fix clean up, double free and errors in keygen

4 files changed, 258 insertions, 51 deletions

diff --git a/src/cli/frosix-cli.c b/src/cli/frosix-cli.c
index 2cb9a8d..2f7663e 100644
--- a/src/cli/frosix-cli.c
+++ b/src/cli/frosix-cli.c
@@ -163,7 +163,7 @@ action_cb (void *cls,
            json_t *result_state)
 {
   (void) cls;
-  ra = NULL;
+  // ra = NULL;
   if (NULL != result_state)
 
     persist_new_state (result_state,
diff --git a/src/libfrosix/frosix_api_keygen.c b/src/libfrosix/frosix_api_keygen.c
index 8b23f89..15202e3 100644
--- a/src/libfrosix/frosix_api_keygen.c
+++ b/src/libfrosix/frosix_api_keygen.c
@@ -279,8 +279,6 @@ free_dkg_data_struct (struct FROSIX_DkgData *dd)
 
   if (NULL != dd->providers_public_keys)
     GNUNET_free (dd->providers_public_keys);
-
-  GNUNET_free (dd);
 }
 
 
@@ -297,7 +295,10 @@ keygen_cancel_cb (void *cls)
 
   /* free dkg data */
   if (NULL != ds->dkg_data)
+  {
     free_dkg_data_struct (ds->dkg_data);
+    GNUNET_free (ds->dkg_data);
+  }
 
   /* free operations */
   if (NULL != ds->co)
@@ -404,8 +405,21 @@ keygen_prepare_result (struct FROSIX_DkgState *ds)
   /* check if every reported public key is the same */
   for (unsigned int i = 0; i < ds->dkg_data->num_of_participants; i++)
   {
-    GNUNET_assert (FROST_point_cmp (&ds->dkg_data->providers[i].public_key.pk,
-                                    &ds->dkg_data->providers[0].public_key.pk));
+    if (GNUNET_OK != FROST_point_cmp (
+          &ds->dkg_data->providers[i].public_key.pk,
+          &ds->dkg_data->providers[0].public_key.pk))
+    {
+      json_t *error_message;
+      error_message = GNUNET_JSON_PACK (
+        GNUNET_JSON_pack_string ("Frosix_client_error",
+                                 "Public keys do not match"));
+
+      ds->cb (ds->cb_cls,
+              0,
+              error_message);
+
+      return;
+    }
   }
 
   /* calculate encryption keys */
@@ -458,7 +472,6 @@ keygen_prepare_result (struct FROSIX_DkgState *ds)
                      provider));
   }
 
-
   result = GNUNET_JSON_PACK (
     GNUNET_JSON_pack_data_auto ("public_key",
                                 &ds->dkg_data->providers[0].public_key),
@@ -491,13 +504,33 @@ dkg_key_request_cb (void *cls,
   GNUNET_assert (NULL != cls);
   struct FROSIX_DkgState *ds = cls;
 
+  if (0 >= ds->counter)
+  {
+    /* We haven't expected this callback, thus just ignore it */
+    return;
+  }
+
+  /* check if the call was successful */
+  if (0 == dkd->http_status)
+  {
+    ds->counter = 0;
+
+    // FIXME: cancel all other running and not yet returned requests
+    json_t *error_message;
+    error_message = GNUNET_JSON_PACK (
+      GNUNET_JSON_pack_string ("Frosix_client_error",
+                               "Error requesting dkg key from provider"));
+
+    ds->cb (ds->cb_cls,
+            0,
+            error_message);
+
+    return;
+  }
+
   /* set error code */
   ds->error_code = dkd->ec;
 
-  /* check if this is a valid callback */
-  GNUNET_assert (0 < ds->counter);
-  GNUNET_assert (0 != dkd->http_status);
-
   /* check if provider index is valid */
   GNUNET_assert (0 < dkd->provider_index);
   GNUNET_assert (ds->dkg_data->num_of_participants >= dkd->provider_index);
@@ -547,11 +580,24 @@ start_dkg_key (struct FROSIX_DkgState *ds)
     {
       if (j < i)
       {
-        GNUNET_assert (dp->provider_index ==
-                       dd->providers[j].secret_shares[i - 1].target);
+        /* check if target of the secret share matches */
+        if (dp->provider_index != dd->providers[j].secret_shares[i - 1].target)
+        {
+          json_t *error_message;
+          error_message = GNUNET_JSON_PACK (
+            GNUNET_JSON_pack_string ("Frosix_client_error",
+                                     "Error while assigning the secret shares"));
+
+          ds->cb (ds->cb_cls,
+                  0,
+                  error_message);
+
+          return;
+        }
+
         secret_shares[j].identifier =
           dd->providers[j].secret_shares[i - 1].issuer;
-        // FIXME: memory problem?
+
         memcpy (&secret_shares[j].secret_share,
                 &dd->providers[j].secret_shares[i - 1].encrypted_share,
                 sizeof (dd->providers[j].secret_shares[i - 1].encrypted_share));
@@ -561,8 +607,21 @@ start_dkg_key (struct FROSIX_DkgState *ds)
       }
       else if (j > i)
       {
-        GNUNET_assert (dp->provider_index ==
-                       dd->providers[j].secret_shares[i].target);
+        /* check if target of the secret share matches */
+        if (dp->provider_index != dd->providers[j].secret_shares[i].target)
+        {
+          json_t *error_message;
+          error_message = GNUNET_JSON_PACK (
+            GNUNET_JSON_pack_string ("Frosix_client_error",
+                                     "Error while assigning the secret shares"));
+
+          ds->cb (ds->cb_cls,
+                  0,
+                  error_message);
+
+          return;
+        }
+
         secret_shares[j - 1].identifier =
           dd->providers[j].secret_shares[i].issuer;
         memcpy (&secret_shares[j - 1].secret_share,
@@ -620,24 +679,74 @@ dkg_share_request_cb (void *cls,
   GNUNET_assert (NULL != cls);
   struct FROSIX_DkgState *ds = cls;
 
+  if (0 >= ds->counter)
+  {
+    /* We haven't expected this callback, thus just ignore it */
+    return;
+  }
+
+  /* check if the call was successful */
+  if (0 == dsd->http_status)
+  {
+    ds->counter = 0;
+
+    // FIXME: cancel all other running and not yet returned requests
+    json_t *error_message;
+    error_message = GNUNET_JSON_PACK (
+      GNUNET_JSON_pack_string ("Frosix_client_error",
+                               "Error requesting dkg shares from provider"));
+
+    ds->cb (ds->cb_cls,
+            0,
+            error_message);
+
+    return;
+  }
+
   /* set error code */
   ds->error_code = dsd->ec;
 
-  /* check if this is a valid callback */
-  GNUNET_assert (0 < ds->counter);
-  GNUNET_assert (0 != dsd->http_status);
-
   /* check if provider index is valid */
   GNUNET_assert (0 < dsd->provider_index);
   GNUNET_assert (ds->dkg_data->num_of_participants >= dsd->provider_index);
 
   /* check number of secret shares */
-  GNUNET_assert (ds->dkg_data->num_of_participants - 1 == dsd->shares_len);
+  if (ds->dkg_data->num_of_participants - 1 != dsd->shares_len)
+  {
+    ds->counter = 0;
+
+    // FIXME: cancel all other running and not yet returned requests
+    json_t *error_message;
+    error_message = GNUNET_JSON_PACK (
+      GNUNET_JSON_pack_string ("Frosix_client_error",
+                               "Error in dkg shares response"));
+
+    ds->cb (ds->cb_cls,
+            0,
+            error_message);
+
+    return;
+  }
 
   /* check if our provider index is same as in the response */
   for (unsigned int i = 0; i < dsd->shares_len; i++)
   {
-    GNUNET_assert (dsd->provider_index == dsd->shares[i].issuer);
+    if (dsd->provider_index != dsd->shares[i].issuer)
+    {
+      ds->counter = 0;
+
+      // FIXME: cancel all other running and not yet returned requests
+      json_t *error_message;
+      error_message = GNUNET_JSON_PACK (
+        GNUNET_JSON_pack_string ("Frosix_client_error",
+                                 "Error in dkg shares response"));
+
+      ds->cb (ds->cb_cls,
+              0,
+              error_message);
+
+      return;
+    }
   }
 
   /* copy commitment in our struct - just assign pointer */
@@ -735,23 +844,72 @@ dkg_commitment_request_cb (void *cls,
   GNUNET_assert (NULL != cls);
   struct FROSIX_DkgState *ds = cls;
 
+  if (0 >= ds->counter)
+  {
+    /* We haven't expected this callback, thus just ignore it */
+    return;
+  }
+
+  /* check if the call was successful */
+  if (0 == dcd->http_status)
+  {
+    ds->counter = 0;
+
+    // FIXME: cancel all other running and not yet returned requests
+    json_t *error_message;
+    error_message = GNUNET_JSON_PACK (
+      GNUNET_JSON_pack_string ("Frosix_client_error",
+                               "Error requesting dkg commitment from provider"));
+
+    ds->cb (ds->cb_cls,
+            0,
+            error_message);
+
+    return;
+  }
+
   /* set error code */
   ds->error_code = dcd->ec;
 
-  /* check if this is a valid callback */
-  GNUNET_assert (0 < ds->counter);
-  GNUNET_assert (0 != dcd->http_status);
-
   /* check if provider index is valid */
   GNUNET_assert (0 < dcd->provider_index);
   GNUNET_assert (ds->dkg_data->num_of_participants >= dcd->provider_index);
 
   /* check if our provider index is same as in the response */
-  GNUNET_assert (dcd->provider_index == dcd->dkg_commitment.identifier);
+  if (dcd->provider_index != dcd->dkg_commitment.identifier)
+  {
+    ds->counter--;
+
+    // FIXME: cancel all other running and not yet returned requests
+    json_t *error_message;
+    error_message = GNUNET_JSON_PACK (
+      GNUNET_JSON_pack_string ("Frosix_client_error",
+                               "Error in dkg commitment response"));
+
+    ds->cb (ds->cb_cls,
+            0,
+            error_message);
+
+    return;
+  }
 
   /* check if length of commitments equals our threshold value */
-  GNUNET_assert (dcd->dkg_commitment.shares_commitments_length ==
-                 ds->dkg_data->threshold);
+  if (dcd->dkg_commitment.shares_commitments_length != ds->dkg_data->threshold)
+  {
+    ds->counter = 0;
+
+    // FIXME: cancel all other running and not yet returned requests
+    json_t *error_message;
+    error_message = GNUNET_JSON_PACK (
+      GNUNET_JSON_pack_string ("Frosix_client_error",
+                               "Error in dkg commitment response"));
+
+    ds->cb (ds->cb_cls,
+            0,
+            error_message);
+
+    return;
+  }
 
   /* copy commitment in our struct */
   struct FROSIX_DkgProvider *dp = &ds->dkg_data->providers[dcd->provider_index
@@ -956,7 +1114,7 @@ start_dkg_commitments (struct FROSIX_DkgState *ds)
                          &client_entropy,
                          &provider_entropy);
 
-  /* computer salted hash of authentication data */
+  /* compute salted hash of authentication data */
   for (unsigned int i = 0; i < ds->dkg_data->num_of_participants; i++)
   {
     struct FROSIX_DkgProvider *dp = &ds->dkg_data->providers[i];
@@ -1030,9 +1188,29 @@ seed_request_cb (void *cls,
   GNUNET_assert (NULL != cls);
   struct FROSIX_DkgState *ds = cls;
 
-  /* check if this is a valid callback */
-  GNUNET_assert (0 < ds->counter);
-  GNUNET_assert (0 != http_status);
+  if (0 >= ds->counter)
+  {
+    /* We haven't expected this callback, thus just ignore it */
+    return;
+  }
+
+  /* check if the call was successful */
+  if (0 == http_status)
+  {
+    ds->counter = 0;
+
+    // FIXME: cancel all other running and not yet returned requests
+    json_t *error_message;
+    error_message = GNUNET_JSON_PACK (
+      GNUNET_JSON_pack_string ("Frosix_client_error",
+                               "Error requesting seed from provider"));
+
+    ds->cb (ds->cb_cls,
+            0,
+            error_message);
+
+    return;
+  }
 
   /* check if provider index is valid */
   GNUNET_assert (0 < ps->provider_index);
@@ -1064,9 +1242,8 @@ static void
 start_seed_requests (struct FROSIX_DkgState *ds)
 {
   ds->counter = 0;
-  // FIXME: pointer to array of pointers?
   ds->sgo = GNUNET_malloc (ds->dkg_data->num_of_participants * sizeof (char *));
-  // FIXME: implement free of config operations
+
   for (unsigned int i = 0; i < ds->dkg_data->num_of_participants; i++)
   {
     ds->sgo[i] = FROSIX_seed_get (FROSIX_REDUX_ctx_,
@@ -1093,9 +1270,29 @@ config_request_cb (void *cls,
   GNUNET_assert (NULL != cls);
   struct FROSIX_DkgState *ds = cls;
 
-  /* check if this is a valid callback */
-  GNUNET_assert (0 < ds->counter); // FIXME: abort correctly
-  GNUNET_assert (0 != http_status);
+  if (0 >= ds->counter)
+  {
+    /* We haven't expected this callback, thus just ignore it */
+    return;
+  }
+
+  /* check if the call was successful */
+  if (0 == http_status)
+  {
+    ds->counter = 0;
+
+    // FIXME: cancel all other running and not yet returned requests
+    json_t *error_message;
+    error_message = GNUNET_JSON_PACK (
+      GNUNET_JSON_pack_string ("Frosix_client_error",
+                               "Error requesting config from provider"));
+
+    ds->cb (ds->cb_cls,
+            0,
+            error_message);
+
+    return;
+  }
 
   /* check if provider index is valid */
   GNUNET_assert (0 < fcfg->provider_index);
@@ -1106,7 +1303,9 @@ config_request_cb (void *cls,
     struct FROSIX_DkgProvider *dp =
       &ds->dkg_data->providers[fcfg->provider_index - 1];
 
-    dp->provider_name = strdup (fcfg->business_name);
+    size_t len = strlen (fcfg->business_name);
+    dp->provider_name = strndup (fcfg->business_name,
+                                 len);
 
     memcpy (&dp->provider_salt,
             &fcfg->provider_salt,
@@ -1139,7 +1338,6 @@ static void
 start_config_requests (struct FROSIX_DkgState *ds)
 {
   ds->counter = 0;
-  // FIXME: pointer to array of pointers?
   ds->co = GNUNET_malloc (ds->dkg_data->num_of_participants * sizeof (char *));
 
   for (unsigned int i = 0; i < ds->dkg_data->num_of_participants; i++)
@@ -1191,10 +1389,11 @@ parse_keygen_arguments (struct FROSIX_DkgData *dkg_data,
   size_t num_of_providers = json_array_size (providers);
 
   /* validate number of providers */
-  GNUNET_assert (255 > num_of_providers);
-  GNUNET_assert (1 < num_of_providers);
-  GNUNET_assert (0 < dkg_data->threshold);
-  GNUNET_assert (num_of_providers > dkg_data->threshold);
+  if (num_of_providers > 254
+      || num_of_providers < 2
+      || dkg_data->threshold < 1
+      || num_of_providers <= dkg_data->threshold)
+    return GNUNET_NO;
 
   dkg_data->num_of_participants = num_of_providers;
 
@@ -1258,8 +1457,16 @@ FROSIX_redux_keygen_start (const json_t *arguments,
                                            arguments))
   {
     free_dkg_data_struct (dkg_data);
+    GNUNET_free (dkg_data);
+    json_t *error_message;
+    error_message = GNUNET_JSON_PACK (
+      GNUNET_JSON_pack_string ("Frosix_client_error",
+                               "Error while parsing input"));
+
+    cb (cb_cls,
+        0,
+        error_message);
 
-    // FIXME: Return some useful error message
     return NULL;
   }
 
diff --git a/src/libfrosix/frosix_api_sign.c b/src/libfrosix/frosix_api_sign.c
index cd38d92..e227fef 100644
--- a/src/libfrosix/frosix_api_sign.c
+++ b/src/libfrosix/frosix_api_sign.c
@@ -198,8 +198,6 @@ free_signature_data_struct (struct FROSIX_SignatureData *sd)
 
     GNUNET_free (sd->providers);
   }
-
-  GNUNET_free (sd);
 }
 
 
@@ -215,7 +213,11 @@ sign_cancel_cb (void *cls)
   struct FROSIX_SignatureState *ss = cls;
 
   if (NULL != ss->sig_data)
+  {
     free_signature_data_struct (ss->sig_data);
+    GNUNET_free (ss->sig_data);
+  }
+
 
   /* free operations */
   if (NULL != ss->co)
diff --git a/src/restclient/frosix_api_dkg-commitment_request.c b/src/restclient/frosix_api_dkg-commitment_request.c
index 45bcac2..83b210b 100644
--- a/src/restclient/frosix_api_dkg-commitment_request.c
+++ b/src/restclient/frosix_api_dkg-commitment_request.c
@@ -340,17 +340,15 @@ FROSIX_dkg_commitment_request (
                                   context_string),
       GNUNET_JSON_pack_data_auto ("auth_hash",
                                   challenge_hash),
-      GNUNET_JSON_pack_array_incref ("providers_public_keys",
-                                     json_public_keys));
+      GNUNET_JSON_pack_array_steal ("providers_public_keys",
+                                    json_public_keys));
     json_str = json_dumps (dkg_commitment_data,
                            JSON_COMPACT);
 
-    json_decref (json_public_keys);
+    json_decref (dkg_commitment_data);
 
     /* check if we have a json object, abort if it was not successful */
     GNUNET_assert (NULL != json_str);
-
-    json_decref (dkg_commitment_data);
   }
 
   /* prepare curl options and fire the request */