diff options
author | Christian Grothoff <christian@grothoff.org> | 2022-02-12 14:38:24 +0100 |
---|---|---|
committer | Christian Grothoff <christian@grothoff.org> | 2022-02-12 14:38:27 +0100 |
commit | f6ecf7458ab4a0e23233439ca6c878fd93921083 (patch) | |
tree | 1b23fe66bfdac54ba36266eb3960d375a2b7238e | |
parent | 7cedf3f0bf0bd9c4f89c26bfbb4e276423860f65 (diff) | |
download | exchange-f6ecf7458ab4a0e23233439ca6c878fd93921083.tar.xz |
-refactor melt API, add FIXME for discovered bug
-rw-r--r-- | src/include/taler_exchange_service.h | 28 | ||||
-rw-r--r-- | src/lib/exchange_api_melt.c | 132 | ||||
-rw-r--r-- | src/lib/exchange_api_refresh_common.c | 11 | ||||
-rw-r--r-- | src/lib/exchange_api_refresh_common.h | 6 | ||||
-rw-r--r-- | src/testing/testing_api_cmd_refresh.c | 78 |
5 files changed, 97 insertions, 158 deletions
diff --git a/src/include/taler_exchange_service.h b/src/include/taler_exchange_service.h index 3b227fe3e..3961aaa10 100644 --- a/src/include/taler_exchange_service.h +++ b/src/include/taler_exchange_service.h @@ -1650,6 +1650,7 @@ struct TALER_EXCHANGE_MeltBlindingDetail /** * Blinding keys used to blind the fresh coins */ + /* FIXME: uninitialized! */ union TALER_DenominationBlindingKeyP bks; }; @@ -1678,12 +1679,6 @@ struct TALER_EXCHANGE_MeltResponse { /** - * Length of the @a mbds array with the exchange values - * and blinding keys we are using. - */ - unsigned int num_mbds; - - /** * Information returned per coin. */ const struct TALER_EXCHANGE_MeltBlindingDetail *mbds; @@ -1694,6 +1689,12 @@ struct TALER_EXCHANGE_MeltResponse struct TALER_ExchangePublicKeyP sign_key; /** + * Length of the @a mbds array with the exchange values + * and blinding keys we are using. + */ + unsigned int num_mbds; + + /** * Gamma value chosen by the exchange. */ uint32_t noreveal_index; @@ -1710,23 +1711,12 @@ struct TALER_EXCHANGE_MeltResponse * #TALER_EXCHANGE_refreshes_reveal(). * * @param cls closure - * @param hr HTTP response data - * @param num_coins number of fresh coins to be created, length of the @a exchange_vals array, 0 if the operation failed - * @param alg_values array @a num_coins of exchange values contributed to the refresh operation - * @param bks array of @a num_coins blinding keys used to blind the fresh coins - * @param noreveal_index choice by the exchange in the cut-and-choose protocol, - * UINT32_MAX on error - * @param sign_key exchange key used to sign the response, or NULL + * @param mr response details */ typedef void (*TALER_EXCHANGE_MeltCallback) ( void *cls, - const struct TALER_EXCHANGE_HttpResponse *hr, - unsigned int num_coins, - const struct TALER_ExchangeWithdrawValues *alg_values, - const union TALER_DenominationBlindingKeyP *bks, - uint32_t noreveal_index, - const struct TALER_ExchangePublicKeyP *sign_key); + const struct TALER_EXCHANGE_MeltResponse *mr); /** diff --git a/src/lib/exchange_api_melt.c b/src/lib/exchange_api_melt.c index 3c608328a..07034f144 100644 --- a/src/lib/exchange_api_melt.c +++ b/src/lib/exchange_api_melt.c @@ -86,16 +86,10 @@ struct TALER_EXCHANGE_MeltHandle const struct TALER_EXCHANGE_RefreshData *rd; /** - * Array of `num_fresh_coins` contributory values of - * the exchange to the melt operation. + * Array of `num_fresh_coins` per-coin values + * returned from melt operation. */ - struct TALER_ExchangeWithdrawValues *alg_values; - - /** - * Array of `num_fresh_coins` blinding secrets - * used for blinding the coins. - */ - union TALER_DenominationBlindingKeyP *bks; + struct TALER_EXCHANGE_MeltBlindingDetail *mbds; /** * Handle for the preflight request, or NULL. @@ -336,58 +330,45 @@ handle_melt_finished (void *cls, const void *response) { struct TALER_EXCHANGE_MeltHandle *mh = cls; - struct TALER_ExchangePublicKeyP exchange_pub; const json_t *j = response; - struct TALER_EXCHANGE_HttpResponse hr = { - .reply = j, - .http_status = (unsigned int) response_code + struct TALER_EXCHANGE_MeltResponse mr = { + .hr.reply = j, + .hr.http_status = (unsigned int) response_code }; mh->job = NULL; switch (response_code) { case 0: - hr.ec = TALER_EC_GENERIC_INVALID_RESPONSE; + mr.hr.ec = TALER_EC_GENERIC_INVALID_RESPONSE; break; case MHD_HTTP_OK: if (GNUNET_OK != verify_melt_signature_ok (mh, j, - &exchange_pub)) + &mr.details.success.sign_key)) { GNUNET_break_op (0); - hr.http_status = 0; - hr.ec = TALER_EC_EXCHANGE_MELT_INVALID_SIGNATURE_BY_EXCHANGE; - } - if (NULL != mh->melt_cb) - { - mh->melt_cb (mh->melt_cb_cls, - &hr, - (0 == hr.http_status) - ? 0 - : mh->rd->fresh_pks_len, - (0 == hr.http_status) - ? NULL - : mh->alg_values, - (0 == hr.http_status) - ? NULL - : mh->bks, - mh->noreveal_index, - (0 == hr.http_status) - ? NULL - : &exchange_pub); - mh->melt_cb = NULL; + mr.hr.http_status = 0; + mr.hr.ec = TALER_EC_EXCHANGE_MELT_INVALID_SIGNATURE_BY_EXCHANGE; + break; } + mr.details.success.noreveal_index = mh->noreveal_index; + mr.details.success.num_mbds = mh->rd->fresh_pks_len; + mr.details.success.mbds = mh->mbds; + mh->melt_cb (mh->melt_cb_cls, + &mr); + mh->melt_cb = NULL; break; case MHD_HTTP_BAD_REQUEST: /* This should never happen, either us or the exchange is buggy (or API version conflict); just pass JSON reply to the application */ - hr.ec = TALER_JSON_get_error_code (j); - hr.hint = TALER_JSON_get_error_hint (j); + mr.hr.ec = TALER_JSON_get_error_code (j); + mr.hr.hint = TALER_JSON_get_error_hint (j); break; case MHD_HTTP_CONFLICT: - hr.ec = TALER_JSON_get_error_code (j); - switch (hr.ec) + mr.hr.ec = TALER_JSON_get_error_code (j); + switch (mr.hr.ec) { case TALER_EC_EXCHANGE_GENERIC_INSUFFICIENT_FUNDS: /* Double spending; check signatures on transaction history */ @@ -396,9 +377,9 @@ handle_melt_finished (void *cls, j)) { GNUNET_break_op (0); - hr.http_status = 0; - hr.ec = TALER_EC_EXCHANGE_MELT_INVALID_SIGNATURE_BY_EXCHANGE; - hr.hint = TALER_JSON_get_error_hint (j); + mr.hr.http_status = 0; + mr.hr.ec = TALER_EC_EXCHANGE_MELT_INVALID_SIGNATURE_BY_EXCHANGE; + mr.hr.hint = TALER_JSON_get_error_hint (j); } break; case TALER_EC_EXCHANGE_GENERIC_COIN_CONFLICTING_DENOMINATION_KEY: @@ -407,16 +388,16 @@ handle_melt_finished (void *cls, j)) { GNUNET_break_op (0); - hr.http_status = 0; - hr.ec = TALER_EC_EXCHANGE_MELT_INVALID_SIGNATURE_BY_EXCHANGE; - hr.hint = TALER_JSON_get_error_hint (j); + mr.hr.http_status = 0; + mr.hr.ec = TALER_EC_EXCHANGE_MELT_INVALID_SIGNATURE_BY_EXCHANGE; + mr.hr.hint = TALER_JSON_get_error_hint (j); } break; default: GNUNET_break_op (0); - hr.http_status = 0; - hr.ec = TALER_EC_EXCHANGE_MELT_INVALID_SIGNATURE_BY_EXCHANGE; - hr.hint = TALER_JSON_get_error_hint (j); + mr.hr.http_status = 0; + mr.hr.ec = TALER_EC_EXCHANGE_MELT_INVALID_SIGNATURE_BY_EXCHANGE; + mr.hr.hint = TALER_JSON_get_error_hint (j); break; } break; @@ -424,40 +405,35 @@ handle_melt_finished (void *cls, /* Nothing really to verify, exchange says one of the signatures is invalid; assuming we checked them, this should never happen, we should pass the JSON reply to the application */ - hr.ec = TALER_JSON_get_error_code (j); - hr.hint = TALER_JSON_get_error_hint (j); + mr.hr.ec = TALER_JSON_get_error_code (j); + mr.hr.hint = TALER_JSON_get_error_hint (j); break; case MHD_HTTP_NOT_FOUND: /* Nothing really to verify, this should never happen, we should pass the JSON reply to the application */ - hr.ec = TALER_JSON_get_error_code (j); - hr.hint = TALER_JSON_get_error_hint (j); + mr.hr.ec = TALER_JSON_get_error_code (j); + mr.hr.hint = TALER_JSON_get_error_hint (j); break; case MHD_HTTP_INTERNAL_SERVER_ERROR: /* Server had an internal issue; we should retry, but this API leaves this to the application */ - hr.ec = TALER_JSON_get_error_code (j); - hr.hint = TALER_JSON_get_error_hint (j); + mr.hr.ec = TALER_JSON_get_error_code (j); + mr.hr.hint = TALER_JSON_get_error_hint (j); break; default: /* unexpected response code */ - hr.ec = TALER_JSON_get_error_code (j); - hr.hint = TALER_JSON_get_error_hint (j); + mr.hr.ec = TALER_JSON_get_error_code (j); + mr.hr.hint = TALER_JSON_get_error_hint (j); GNUNET_log (GNUNET_ERROR_TYPE_ERROR, "Unexpected response code %u/%d for exchange melt\n", (unsigned int) response_code, - hr.ec); + mr.hr.ec); GNUNET_break_op (0); break; } if (NULL != mh->melt_cb) mh->melt_cb (mh->melt_cb_cls, - &hr, - 0, - NULL, - NULL, - UINT32_MAX, - NULL); + &mr); TALER_EXCHANGE_melt_cancel (mh); } @@ -479,11 +455,16 @@ start_melt (struct TALER_EXCHANGE_MeltHandle *mh) struct TALER_CoinSpendSignatureP confirm_sig; char arg_str[sizeof (struct TALER_CoinSpendPublicKeyP) * 2 + 32]; struct TALER_DenominationHash h_denom_pub; + struct TALER_ExchangeWithdrawValues alg_values[mh->rd->fresh_pks_len]; + for (unsigned int i = 0; i<mh->rd->fresh_pks_len; i++) + alg_values[i] = mh->mbds[i].alg_value; + /* FIXME: get_melt_data computes the 'bks' which + we should return, but leave uninitialized => refactor logic! */ if (GNUNET_OK != TALER_EXCHANGE_get_melt_data_ (&mh->rms, mh->rd, - mh->alg_values, + alg_values, &mh->md)) { GNUNET_break (0); @@ -574,11 +555,6 @@ fail_mh (struct TALER_EXCHANGE_MeltHandle *mh) { // FIXME: do return more than NULLs if the /csr failed! mh->melt_cb (mh->melt_cb_cls, - NULL, - 0, - NULL, - NULL, - UINT32_MAX, NULL); TALER_EXCHANGE_melt_cancel (mh); } @@ -603,7 +579,7 @@ csr_cb (void *cls, { const struct TALER_EXCHANGE_DenomPublicKey *fresh_pk = &mh->rd->fresh_pks[i]; - struct TALER_ExchangeWithdrawValues *wv = &mh->alg_values[i]; + struct TALER_ExchangeWithdrawValues *wv = &mh->mbds[i].alg_value; switch (fresh_pk->key.cipher) { @@ -656,21 +632,18 @@ TALER_EXCHANGE_melt (struct TALER_EXCHANGE_Handle *exchange, mh->rms = *rms; mh->melt_cb = melt_cb; mh->melt_cb_cls = melt_cb_cls; - mh->alg_values = GNUNET_new_array (rd->fresh_pks_len, - struct TALER_ExchangeWithdrawValues); - mh->bks = GNUNET_new_array (rd->fresh_pks_len, - union TALER_DenominationBlindingKeyP); + mh->mbds = GNUNET_new_array (rd->fresh_pks_len, + struct TALER_EXCHANGE_MeltBlindingDetail); for (unsigned int i = 0; i<rd->fresh_pks_len; i++) { const struct TALER_EXCHANGE_DenomPublicKey *fresh_pk = &rd->fresh_pks[i]; - struct TALER_ExchangeWithdrawValues *wv = &mh->alg_values[i]; + struct TALER_ExchangeWithdrawValues *wv = &mh->mbds[i].alg_value; switch (fresh_pk->key.cipher) { case TALER_DENOMINATION_INVALID: GNUNET_break (0); - GNUNET_free (mh->alg_values); - GNUNET_free (mh->bks); + GNUNET_free (mh->mbds); GNUNET_free (mh); return NULL; case TALER_DENOMINATION_RSA: @@ -726,8 +699,7 @@ TALER_EXCHANGE_melt_cancel (struct TALER_EXCHANGE_MeltHandle *mh) mh->csr = NULL; } TALER_EXCHANGE_free_melt_data_ (&mh->md); /* does not free 'md' itself */ - GNUNET_free (mh->alg_values); - GNUNET_free (mh->bks); + GNUNET_free (mh->mbds); GNUNET_free (mh->url); TALER_curl_easy_post_finished (&mh->ctx); GNUNET_free (mh); diff --git a/src/lib/exchange_api_refresh_common.c b/src/lib/exchange_api_refresh_common.c index 4c65e390f..02af2a993 100644 --- a/src/lib/exchange_api_refresh_common.c +++ b/src/lib/exchange_api_refresh_common.c @@ -146,14 +146,15 @@ TALER_EXCHANGE_get_melt_data_ ( TALER_planchet_blinding_secret_create (fc, &alg_values[j], &bks); - /* Note: we already did this for the /csr request, + /* FIXME: we already did this for the /csr request, so this computation is redundant, and here additionally repeated KAPPA times. Could be avoided with slightly more bookkeeping in the future */ - TALER_cs_refresh_nonce_derive ( - rms, - j, - &pd.blinded_planchet.details.cs_blinded_planchet.nonce); + if (TALER_DENOMINATION_CS == alg_values[j].cipher) + TALER_cs_refresh_nonce_derive ( + rms, + j, + &pd.blinded_planchet.details.cs_blinded_planchet.nonce); if (GNUNET_OK != TALER_planchet_prepare (&md->fresh_pks[j], &alg_values[j], diff --git a/src/lib/exchange_api_refresh_common.h b/src/lib/exchange_api_refresh_common.h index 923fb76c4..2115b5a19 100644 --- a/src/lib/exchange_api_refresh_common.h +++ b/src/lib/exchange_api_refresh_common.h @@ -103,12 +103,6 @@ struct MeltData struct TALER_DenominationPublicKey *fresh_pks; /** - * Array of @e num_fresh_coins with exchange contributions - * made during the refresh. - */ - struct TALER_ExchangeWithdrawValues *exchange_vals; - - /** * Arrays of @e num_fresh_coins with information about the fresh * coins to be created, for each cut-and-choose dimension. */ diff --git a/src/testing/testing_api_cmd_refresh.c b/src/testing/testing_api_cmd_refresh.c index 8b0329b38..94fade945 100644 --- a/src/testing/testing_api_cmd_refresh.c +++ b/src/testing/testing_api_cmd_refresh.c @@ -117,15 +117,10 @@ struct RefreshMeltState struct TALER_EXCHANGE_DenomPublicKey *fresh_pks; /** - * Array of @e num_fresh_coins of exchange values contributed to the refresh operation + * Array of @e num_fresh_coins of results from + * the melt operation. */ - struct TALER_ExchangeWithdrawValues *alg_values; - - /** - * Array of @e num_fresh_coins of blinding key secrets - * created during the melt operation. - */ - union TALER_DenominationBlindingKeyP *bks; + struct TALER_EXCHANGE_MeltBlindingDetail *mbds; /** * Entropy seed for the refresh-melt operation. @@ -499,15 +494,20 @@ refresh_reveal_run (void *cls, } // FIXME: use trait for 'rms'! rms = melt_cmd->cls; - rrs->rrh = TALER_EXCHANGE_refreshes_reveal (is->exchange, - &rms->rms, - &rms->refresh_data, - rms->num_fresh_coins, - rms->alg_values, - rms->noreveal_index, - &reveal_cb, - rrs); - + { + struct TALER_ExchangeWithdrawValues alg_values[rms->num_fresh_coins]; + + for (unsigned int i = 0; i<rms->num_fresh_coins; i++) + alg_values[i] = rms->mbds[i].alg_value; + rrs->rrh = TALER_EXCHANGE_refreshes_reveal (is->exchange, + &rms->rms, + &rms->refresh_data, + rms->num_fresh_coins, + alg_values, + rms->noreveal_index, + &reveal_cb, + rrs); + } if (NULL == rrs->rrh) { GNUNET_break (0); @@ -917,26 +917,15 @@ do_melt_retry (void *cls) * CMD was set to do so. * * @param cls closure. - * @param hr HTTP response details - * @param num_coins number of fresh coins to be created, length of the @a exchange_vals array, 0 if the operation failed - * @param alg_values array @a num_coins of exchange values contributed to the refresh operation - * @param bks array of @a num_coins blinding keys used to blind the fresh coins - * @param noreveal_index choice by the exchange in the - * cut-and-choose protocol, UINT16_MAX on error. - * @param exchange_pub public key the exchange used for signing. + * @param mr melt response details */ static void melt_cb (void *cls, - const struct TALER_EXCHANGE_HttpResponse *hr, - unsigned int num_coins, - const struct TALER_ExchangeWithdrawValues *alg_values, - const union TALER_DenominationBlindingKeyP *bks, - uint32_t noreveal_index, - const struct TALER_ExchangePublicKeyP *exchange_pub) + const struct TALER_EXCHANGE_MeltResponse *mr) { struct RefreshMeltState *rms = cls; + const struct TALER_EXCHANGE_HttpResponse *hr = &mr->hr; - (void) exchange_pub; rms->rmh = NULL; if (rms->expected_response_code != hr->http_status) { @@ -981,24 +970,18 @@ melt_cb (void *cls, } if (MHD_HTTP_OK == hr->http_status) { - rms->noreveal_index = noreveal_index; - if (num_coins != rms->num_fresh_coins) + rms->noreveal_index = mr->details.success.noreveal_index; + if (mr->details.success.num_mbds != rms->num_fresh_coins) { GNUNET_break (0); TALER_TESTING_interpreter_fail (rms->is); return; } - GNUNET_free (rms->alg_values); - rms->alg_values = GNUNET_new_array (num_coins, - struct TALER_ExchangeWithdrawValues); - memcpy (rms->alg_values, - alg_values, - num_coins * sizeof (struct TALER_ExchangeWithdrawValues)); - rms->bks = GNUNET_new_array (num_coins, - union TALER_DenominationBlindingKeyP); - memcpy (rms->bks, - bks, - num_coins * sizeof (union TALER_DenominationBlindingKeyP)); + GNUNET_free (rms->mbds); + rms->mbds = GNUNET_memdup (mr->details.success.mbds, + mr->details.success.num_mbds + * sizeof (struct + TALER_EXCHANGE_MeltBlindingDetail)); } if (0 != rms->total_backoff.rel_value_us) { @@ -1199,8 +1182,7 @@ melt_cleanup (void *cls, TALER_denom_pub_free (&rms->fresh_pks[i].key); GNUNET_free (rms->fresh_pks); } - GNUNET_free (rms->alg_values); - GNUNET_free (rms->bks); + GNUNET_free (rms->mbds); GNUNET_free (rms->melt_fresh_amounts); GNUNET_free (rms); } @@ -1235,9 +1217,9 @@ melt_traits (void *cls, TALER_TESTING_make_trait_coin_priv (0, rms->melt_priv), TALER_TESTING_make_trait_blinding_key (index, - &rms->bks[index]), + &rms->mbds[index].bks), TALER_TESTING_make_trait_exchange_wd_value (index, - &rms->alg_values[index]), + &rms->mbds[index].alg_value), TALER_TESTING_make_trait_refresh_secret (&rms->rms), TALER_TESTING_trait_end () }; |