fix alignment issue, ensure we hash over packed structure to avoid non-determinism

5 files changed, 190 insertions, 134 deletions

diff --git a/src/exchange/taler-exchange-httpd_keys.c b/src/exchange/taler-exchange-httpd_keys.c
index debad6f1f..f7bdaae41 100644
--- a/src/exchange/taler-exchange-httpd_keys.c
+++ b/src/exchange/taler-exchange-httpd_keys.c
@@ -2110,8 +2110,8 @@ finish_keys_response (struct TEH_KeyStateHandle *ksh)
   {
     struct TEH_DenominationKey *dk;
     struct GNUNET_CONTAINER_MultiHashMap *denominations_by_group;
-    /* groupData is the value we store for each group meta-data */
-    struct groupData
+    /* GroupData is the value we store for each group meta-data */
+    struct GroupData
     {
       /**
        * The json blob with the group meta-data and list of denominations
@@ -2215,7 +2215,7 @@ finish_keys_response (struct TEH_KeyStateHandle *ksh)
        */
       {
         static const char *denoms_key = "denoms";
-        struct groupData *group;
+        struct GroupData *group;
         json_t *list;
         json_t *entry;
         struct GNUNET_HashCode key;
@@ -2226,28 +2226,19 @@ finish_keys_response (struct TEH_KeyStateHandle *ksh)
           .age_mask = dk->meta.age_mask,
         };
 
-        memset (&meta.hash,
-                0,
-                sizeof(meta.hash));
         /* Search the group/JSON-blob for the key */
-        GNUNET_CRYPTO_hash (&meta,
-                            sizeof(meta),
-                            &key);
-
-        group =
-          (struct groupData *) GNUNET_CONTAINER_multihashmap_get (
-            denominations_by_group,
-            &key);
-
+        TALER_denomination_group_get_key (&meta,
+                                          &key);
+        group = GNUNET_CONTAINER_multihashmap_get (
+          denominations_by_group,
+          &key);
         if (NULL == group)
         {
           /* There is no group for this meta-data yet, so we create a new group */
           bool age_restricted = meta.age_mask.bits != 0;
           const char *cipher;
 
-          group = GNUNET_new (struct groupData);
-          memset (group, 0, sizeof(*group));
-
+          group = GNUNET_new (struct GroupData);
           switch (meta.cipher)
           {
           case TALER_DENOMINATION_RSA:
@@ -2261,9 +2252,12 @@ finish_keys_response (struct TEH_KeyStateHandle *ksh)
           }
 
           group->json = GNUNET_JSON_PACK (
-            GNUNET_JSON_pack_string ("cipher", cipher),
-            TALER_JSON_PACK_DENOM_FEES ("fee", &meta.fees),
-            TALER_JSON_pack_amount ("value", &meta.value));
+            GNUNET_JSON_pack_string ("cipher",
+                                     cipher),
+            TALER_JSON_PACK_DENOM_FEES ("fee",
+                                        &meta.fees),
+            TALER_JSON_pack_amount ("value",
+                                    &meta.value));
           GNUNET_assert (NULL != group->json);
 
           if (age_restricted)
@@ -2354,7 +2348,7 @@ finish_keys_response (struct TEH_KeyStateHandle *ksh)
         GNUNET_CONTAINER_multihashmap_size (denominations_by_group))
     {
       struct GNUNET_CONTAINER_MultiHashMapIterator *iter;
-      struct groupData *group = NULL;
+      struct GroupData *group = NULL;
 
       iter =
         GNUNET_CONTAINER_multihashmap_iterator_create (denominations_by_group);
diff --git a/src/include/taler_crypto_lib.h b/src/include/taler_crypto_lib.h
index 9b0cb604b..ee06f631c 100644
--- a/src/include/taler_crypto_lib.h
+++ b/src/include/taler_crypto_lib.h
@@ -6032,4 +6032,56 @@ TALER_age_restriction_from_secret (
   struct TALER_AgeCommitmentProof *comm_proof);
 
 
+/**
+ * Group of Denominations.  These are the common fields of an array of
+ * denominations.
+ *
+ * The corresponding JSON-blob will also contain an array of particular
+ * denominations with only the timestamps, cipher-specific public key and the
+ * master signature.
+ */
+struct TALER_DenominationGroup
+{
+
+  /**
+   * XOR of all SHA-512 hashes of the public keys in this
+   * group.
+   */
+  struct GNUNET_HashCode hash;
+
+  /**
+   * Value of coins in this denomination group.
+   */
+  struct TALER_Amount value;
+
+  /**
+   * Fee structure for all coins in the group.
+   */
+  struct TALER_DenomFeeSet fees;
+
+  /**
+   * Cipher used for the denomination.
+   */
+  enum TALER_DenominationCipher cipher;
+
+  /**
+   * Age mask for the denomiation.
+   */
+  struct TALER_AgeMask age_mask;
+
+};
+
+
+/**
+ * Compute a unique key for the meta data of a denomination group.
+ *
+ * @param dg denomination group to evaluate
+ * @param[out] key key to set
+ */
+void
+TALER_denomination_group_get_key (
+  const struct TALER_DenominationGroup *dg,
+  struct GNUNET_HashCode *key);
+
+
 #endif
diff --git a/src/include/taler_json_lib.h b/src/include/taler_json_lib.h
index d0749808d..e2d54e825 100644
--- a/src/include/taler_json_lib.h
+++ b/src/include/taler_json_lib.h
@@ -380,26 +380,6 @@ TALER_JSON_spec_amount_any_nbo (const char *name,
   TALER_JSON_pack_amount ("account_fee", &(gfs)->account),     \
   TALER_JSON_pack_amount ("purse_fee", &(gfs)->purse)
 
-/**
- * Group of Denominations.  These are the common fields of an array of
- * denominations.
- *
- * The corresponding JSON-blob will also contain an array of particular
- * denominations with only the timestamps, cipher-specific public key and the
- * master signature.
- *
- **/
-struct TALER_DenominationGroup
-{
-  enum TALER_DenominationCipher cipher;
-  struct TALER_Amount value;
-  struct TALER_DenomFeeSet fees;
-  struct TALER_AgeMask age_mask;
-
-  // hash is/should be the XOR of all SHA-512 hashes of the public keys in this
-  // group
-  struct GNUNET_HashCode hash;
-};
 
 /**
  * Generate a parser for a group of denominations.
diff --git a/src/lib/exchange_api_handle.c b/src/lib/exchange_api_handle.c
index cf7f1b970..9185a6179 100644
--- a/src/lib/exchange_api_handle.c
+++ b/src/lib/exchange_api_handle.c
@@ -763,7 +763,7 @@ decode_keys_json (const json_t *resp_obj,
     key_data->age_mask = TALER_extensions_get_age_restriction_mask ();
   }
 
-  /**
+  /*
    * Parse the denomination keys, merging with the
    * possibly EXISTING array as required (/keys cherry picking).
    *
@@ -793,6 +793,9 @@ decode_keys_json (const json_t *resp_obj,
                                       &denom_keys_array),
         GNUNET_JSON_spec_end ()
       };
+      json_t *denom_key_obj;
+      unsigned int index;
+
       EXITIF (GNUNET_SYSERR ==
               GNUNET_JSON_parse (group_obj,
                                  group_spec,
@@ -800,78 +803,73 @@ decode_keys_json (const json_t *resp_obj,
                                  NULL));
 
       /* Now, parse the individual denominations */
+      json_array_foreach (denom_keys_array, index, denom_key_obj)
       {
-        json_t *denom_key_obj;
-        unsigned int index;
-
-        json_array_foreach (denom_keys_array, index, denom_key_obj)
+        /* Set the common fields from the group for this particular
+           denomination.  Required to make the validity check inside
+           parse_json_denomkey_partially pass */
+        struct TALER_EXCHANGE_DenomPublicKey dk = {
+          .key.cipher = group.cipher,
+          .value = group.value,
+          .fees = group.fees,
+          .key.age_mask = group.age_mask
+        };
+        bool found = false;
+
+        EXITIF (GNUNET_SYSERR ==
+                parse_json_denomkey_partially (&dk,
+                                               group.cipher,
+                                               check_sig,
+                                               denom_key_obj,
+                                               &key_data->master_pub,
+                                               check_sig ? &hash_xor : NULL));
+
+        /* Build the running xor of the SHA512-hash of the public keys for the group */
+        GNUNET_CRYPTO_hash_xor (&dk.h_key.hash,
+                                &group_hash_xor,
+                                &group_hash_xor);
+        for (unsigned int j = 0;
+             j<key_data->num_denom_keys;
+             j++)
         {
-          /* Set the common fields from the group for this particular
-             denomination.  Required to make the validity check inside
-             parse_json_denomkey_partially pass */
-          struct TALER_EXCHANGE_DenomPublicKey dk = {
-            .key.cipher = group.cipher,
-            .value = group.value,
-            .fees = group.fees,
-            .key.age_mask = group.age_mask
-          };
-          bool found = false;
-
-          EXITIF (GNUNET_SYSERR ==
-                  parse_json_denomkey_partially (&dk,
-                                                 group.cipher,
-                                                 check_sig,
-                                                 denom_key_obj,
-                                                 &key_data->master_pub,
-                                                 check_sig ? &hash_xor : NULL));
-
-          /* Build the running xor of the SHA512-hash of the public keys for the group */
-          GNUNET_CRYPTO_hash_xor (&dk.h_key.hash,
-                                  &group_hash_xor,
-                                  &group_hash_xor);
-          for (unsigned int j = 0;
-               j<key_data->num_denom_keys;
-               j++)
+          if (0 == denoms_cmp (&dk,
+                               &key_data->denom_keys[j]))
           {
-            if (0 == denoms_cmp (&dk,
-                                 &key_data->denom_keys[j]))
-            {
-              found = true;
-              break;
-            }
+            found = true;
+            break;
           }
+        }
 
-          if (found)
-          {
-            /* 0:0:0 did not support /keys cherry picking */
-            TALER_LOG_DEBUG ("Skipping denomination key: already know it\n");
-            TALER_denom_pub_free (&dk.key);
-            continue;
-          }
+        if (found)
+        {
+          /* 0:0:0 did not support /keys cherry picking */
+          TALER_LOG_DEBUG ("Skipping denomination key: already know it\n");
+          TALER_denom_pub_free (&dk.key);
+          continue;
+        }
+
+        if (key_data->denom_keys_size == key_data->num_denom_keys)
+          GNUNET_array_grow (key_data->denom_keys,
+                             key_data->denom_keys_size,
+                             key_data->denom_keys_size * 2 + 2);
+        key_data->denom_keys[key_data->num_denom_keys++] = dk;
+
+        /* Update "last_denom_issue_date" */
+        TALER_LOG_DEBUG ("Adding denomination key that is valid_until %s\n",
+                         GNUNET_TIME_timestamp2s (dk.valid_from));
+        key_data->last_denom_issue_date
+          = GNUNET_TIME_timestamp_max (key_data->last_denom_issue_date,
+                                       dk.valid_from);
+      };   /* end of json_array_foreach over denominations */
+
+      /* The calculated group_hash_xor must be the same as group.hash from
+         the JSON. */
+      EXITIF (0 !=
+              GNUNET_CRYPTO_hash_cmp (&group_hash_xor,
+                                      &group.hash));
 
-          if (key_data->denom_keys_size == key_data->num_denom_keys)
-            GNUNET_array_grow (key_data->denom_keys,
-                               key_data->denom_keys_size,
-                               key_data->denom_keys_size * 2 + 2);
-          key_data->denom_keys[key_data->num_denom_keys++] = dk;
-
-          /* Update "last_denom_issue_date" */
-          TALER_LOG_DEBUG ("Adding denomination key that is valid_until %s\n",
-                           GNUNET_TIME_timestamp2s (dk.valid_from));
-          key_data->last_denom_issue_date
-            = GNUNET_TIME_timestamp_max (key_data->last_denom_issue_date,
-                                         dk.valid_from);
-        }; /* end of json_array_foreach over denominations */
-
-        /* The calculated group_hash_xor must be the same as group.hash from
-           the JSON. */
-        EXITIF (0 !=
-                GNUNET_CRYPTO_hash_cmp (&group_hash_xor,
-                                        &group.hash));
-
-      } /* end of block for parsing individual denominations */
     } /* end of json_array_foreach over groups of denominations */
-  }
+  } /* end of scope for group_ojb/group_idx */
 
   /* parse the auditor information */
   {
@@ -1621,11 +1619,6 @@ struct GroupData
   json_t *json;
 
   /**
-   * xor of all hashes of denominations in that group
-   */
-  struct GNUNET_HashCode hash_xor;
-
-  /**
    * Meta data for this group.
    */
   struct TALER_DenominationGroup meta;
@@ -1668,7 +1661,7 @@ add_grp (void *cls,
 
   ge = GNUNET_JSON_PACK (
     GNUNET_JSON_pack_data_auto ("hash",
-                                &gd->hash_xor),
+                                &gd->meta.hash),
     GNUNET_JSON_pack_string ("cipher",
                              cipher),
     GNUNET_JSON_pack_array_steal ("denoms",
@@ -1751,8 +1744,7 @@ TALER_EXCHANGE_keys_to_json (const struct TALER_EXCHANGE_Keys *kd)
     json_decref (signkeys);
     return NULL;
   }
-  // FIXME: construct denominations_by_group analogous
-  // to taler-exchange-httpd_keys!
+
   {
     struct GNUNET_CONTAINER_MultiHashMap *dbg;
 
@@ -1776,9 +1768,8 @@ TALER_EXCHANGE_keys_to_json (const struct TALER_EXCHANGE_Keys *kd)
                                      >,
                                      dk->expire_deposit))
         continue; /* skip keys that have expired */
-      GNUNET_CRYPTO_hash (&meta,
-                          sizeof(meta),
-                          &key);
+      TALER_denomination_group_get_key (&meta,
+                                        &key);
       gd = GNUNET_CONTAINER_multihashmap_get (dbg,
                                               &key);
       if (NULL == gd)
@@ -1794,18 +1785,11 @@ TALER_EXCHANGE_keys_to_json (const struct TALER_EXCHANGE_Keys *kd)
                                              gd,
                                              GNUNET_CONTAINER_MULTIHASHMAPOPTION_UNIQUE_ONLY));
 
-        /* Build the running xor of the SHA512-hash of the public keys */
       }
-      {
-        struct TALER_DenominationHashP hc;
-
-        TALER_denom_pub_hash (&dk->key,
-                              &hc);
-        GNUNET_CRYPTO_hash_xor (&hc.hash,
-                                &gd->hash_xor,
-                                &gd->hash_xor);
-      }
-
+      /* Build the running xor of the SHA512-hash of the public keys */
+      GNUNET_CRYPTO_hash_xor (&dk->h_key.hash,
+                              &gd->meta.hash,
+                              &gd->meta.hash);
       switch (meta.cipher)
       {
       case TALER_DENOMINATION_RSA:
@@ -1833,10 +1817,6 @@ TALER_EXCHANGE_keys_to_json (const struct TALER_EXCHANGE_Keys *kd)
                                     dk->valid_from),
         GNUNET_JSON_pack_timestamp ("stamp_expire_legal",
                                     dk->expire_legal),
-        TALER_JSON_pack_amount ("value",
-                                &dk->value),
-        TALER_JSON_PACK_DENOM_FEES ("fee",
-                                    &dk->fees),
         GNUNET_JSON_pack_data_auto ("master_sig",
                                     &dk->master_sig),
         key_spec
diff --git a/src/util/crypto.c b/src/util/crypto.c
index bb14b6cdc..8699035c8 100644
--- a/src/util/crypto.c
+++ b/src/util/crypto.c
@@ -461,4 +461,54 @@ TALER_coin_ev_hash (const struct TALER_BlindedPlanchet *blinded_planchet,
 }
 
 
+GNUNET_NETWORK_STRUCT_BEGIN
+/**
+ * Structure we hash to compute the group key for
+ * a denomination group.
+ */
+struct DenominationGroupP
+{
+  /**
+   * Value of coins in this denomination group.
+   */
+  struct TALER_AmountNBO value;
+
+  /**
+   * Fee structure for all coins in the group.
+   */
+  struct TALER_DenomFeeSetNBOP fees;
+
+  /**
+   * Age mask for the denomiation, in NBO.
+   */
+  uint32_t age_mask GNUNET_PACKED;
+
+  /**
+   * Cipher used for the denomination, in NBO.
+   */
+  uint32_t cipher GNUNET_PACKED;
+};
+GNUNET_NETWORK_STRUCT_END
+
+
+void
+TALER_denomination_group_get_key (
+  const struct TALER_DenominationGroup *dg,
+  struct GNUNET_HashCode *key)
+{
+  struct DenominationGroupP dgp = {
+    .age_mask = htonl (dg->age_mask.bits),
+    .cipher = htonl (dg->cipher)
+  };
+
+  TALER_amount_hton (&dgp.value,
+                     &dg->value);
+  TALER_denom_fee_set_hton (&dgp.fees,
+                            &dg->fees);
+  GNUNET_CRYPTO_hash (&dgp,
+                      sizeof (dgp),
+                      key);
+}
+
+
 /* end of crypto.c */