From 6a483b51ec6b857982a45d5215834c99a827066f Mon Sep 17 00:00:00 2001 From: Christian Grothoff Date: Mon, 10 Jul 2023 16:34:01 +0200 Subject: [PATCH] fix alignment issue, ensure we hash over packed structure to avoid non-determinism --- src/exchange/taler-exchange-httpd_keys.c | 38 +++--- src/include/taler_crypto_lib.h | 52 ++++++++ src/include/taler_json_lib.h | 20 --- src/lib/exchange_api_handle.c | 156 ++++++++++------------- src/util/crypto.c | 50 ++++++++ 5 files changed, 186 insertions(+), 130 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; + /* 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; - json_array_foreach (denom_keys_array, index, denom_key_obj) + 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; + jnum_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; - jnum_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; + 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 */ + /* 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)); + /* 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 */ { @@ -1620,11 +1618,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. */ @@ -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 */