-refactor melt API, add FIXME for discovered bug

This commit is contained in:
Christian Grothoff 2022-02-12 14:38:24 +01:00
parent 7cedf3f0bf
commit f6ecf7458a
No known key found for this signature in database
GPG Key ID: 939E6BE1E29FC3CC
5 changed files with 96 additions and 157 deletions

View File

@ -1650,6 +1650,7 @@ struct TALER_EXCHANGE_MeltBlindingDetail
/** /**
* Blinding keys used to blind the fresh coins * Blinding keys used to blind the fresh coins
*/ */
/* FIXME: uninitialized! */
union TALER_DenominationBlindingKeyP bks; union TALER_DenominationBlindingKeyP bks;
}; };
@ -1677,12 +1678,6 @@ struct TALER_EXCHANGE_MeltResponse
struct struct
{ {
/**
* Length of the @a mbds array with the exchange values
* and blinding keys we are using.
*/
unsigned int num_mbds;
/** /**
* Information returned per coin. * Information returned per coin.
*/ */
@ -1693,6 +1688,12 @@ struct TALER_EXCHANGE_MeltResponse
*/ */
struct TALER_ExchangePublicKeyP sign_key; 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. * Gamma value chosen by the exchange.
*/ */
@ -1710,23 +1711,12 @@ struct TALER_EXCHANGE_MeltResponse
* #TALER_EXCHANGE_refreshes_reveal(). * #TALER_EXCHANGE_refreshes_reveal().
* *
* @param cls closure * @param cls closure
* @param hr HTTP response data * @param mr 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,
* UINT32_MAX on error
* @param sign_key exchange key used to sign the response, or NULL
*/ */
typedef void typedef void
(*TALER_EXCHANGE_MeltCallback) ( (*TALER_EXCHANGE_MeltCallback) (
void *cls, void *cls,
const struct TALER_EXCHANGE_HttpResponse *hr, const struct TALER_EXCHANGE_MeltResponse *mr);
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);
/** /**

View File

@ -86,16 +86,10 @@ struct TALER_EXCHANGE_MeltHandle
const struct TALER_EXCHANGE_RefreshData *rd; const struct TALER_EXCHANGE_RefreshData *rd;
/** /**
* Array of `num_fresh_coins` contributory values of * Array of `num_fresh_coins` per-coin values
* the exchange to the melt operation. * returned from melt operation.
*/ */
struct TALER_ExchangeWithdrawValues *alg_values; struct TALER_EXCHANGE_MeltBlindingDetail *mbds;
/**
* Array of `num_fresh_coins` blinding secrets
* used for blinding the coins.
*/
union TALER_DenominationBlindingKeyP *bks;
/** /**
* Handle for the preflight request, or NULL. * Handle for the preflight request, or NULL.
@ -336,58 +330,45 @@ handle_melt_finished (void *cls,
const void *response) const void *response)
{ {
struct TALER_EXCHANGE_MeltHandle *mh = cls; struct TALER_EXCHANGE_MeltHandle *mh = cls;
struct TALER_ExchangePublicKeyP exchange_pub;
const json_t *j = response; const json_t *j = response;
struct TALER_EXCHANGE_HttpResponse hr = { struct TALER_EXCHANGE_MeltResponse mr = {
.reply = j, .hr.reply = j,
.http_status = (unsigned int) response_code .hr.http_status = (unsigned int) response_code
}; };
mh->job = NULL; mh->job = NULL;
switch (response_code) switch (response_code)
{ {
case 0: case 0:
hr.ec = TALER_EC_GENERIC_INVALID_RESPONSE; mr.hr.ec = TALER_EC_GENERIC_INVALID_RESPONSE;
break; break;
case MHD_HTTP_OK: case MHD_HTTP_OK:
if (GNUNET_OK != if (GNUNET_OK !=
verify_melt_signature_ok (mh, verify_melt_signature_ok (mh,
j, j,
&exchange_pub)) &mr.details.success.sign_key))
{ {
GNUNET_break_op (0); GNUNET_break_op (0);
hr.http_status = 0; mr.hr.http_status = 0;
hr.ec = TALER_EC_EXCHANGE_MELT_INVALID_SIGNATURE_BY_EXCHANGE; mr.hr.ec = TALER_EC_EXCHANGE_MELT_INVALID_SIGNATURE_BY_EXCHANGE;
} break;
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.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; break;
case MHD_HTTP_BAD_REQUEST: case MHD_HTTP_BAD_REQUEST:
/* This should never happen, either us or the exchange is buggy /* This should never happen, either us or the exchange is buggy
(or API version conflict); just pass JSON reply to the application */ (or API version conflict); just pass JSON reply to the application */
hr.ec = TALER_JSON_get_error_code (j); mr.hr.ec = TALER_JSON_get_error_code (j);
hr.hint = TALER_JSON_get_error_hint (j); mr.hr.hint = TALER_JSON_get_error_hint (j);
break; break;
case MHD_HTTP_CONFLICT: case MHD_HTTP_CONFLICT:
hr.ec = TALER_JSON_get_error_code (j); mr.hr.ec = TALER_JSON_get_error_code (j);
switch (hr.ec) switch (mr.hr.ec)
{ {
case TALER_EC_EXCHANGE_GENERIC_INSUFFICIENT_FUNDS: case TALER_EC_EXCHANGE_GENERIC_INSUFFICIENT_FUNDS:
/* Double spending; check signatures on transaction history */ /* Double spending; check signatures on transaction history */
@ -396,9 +377,9 @@ handle_melt_finished (void *cls,
j)) j))
{ {
GNUNET_break_op (0); GNUNET_break_op (0);
hr.http_status = 0; mr.hr.http_status = 0;
hr.ec = TALER_EC_EXCHANGE_MELT_INVALID_SIGNATURE_BY_EXCHANGE; mr.hr.ec = TALER_EC_EXCHANGE_MELT_INVALID_SIGNATURE_BY_EXCHANGE;
hr.hint = TALER_JSON_get_error_hint (j); mr.hr.hint = TALER_JSON_get_error_hint (j);
} }
break; break;
case TALER_EC_EXCHANGE_GENERIC_COIN_CONFLICTING_DENOMINATION_KEY: case TALER_EC_EXCHANGE_GENERIC_COIN_CONFLICTING_DENOMINATION_KEY:
@ -407,16 +388,16 @@ handle_melt_finished (void *cls,
j)) j))
{ {
GNUNET_break_op (0); GNUNET_break_op (0);
hr.http_status = 0; mr.hr.http_status = 0;
hr.ec = TALER_EC_EXCHANGE_MELT_INVALID_SIGNATURE_BY_EXCHANGE; mr.hr.ec = TALER_EC_EXCHANGE_MELT_INVALID_SIGNATURE_BY_EXCHANGE;
hr.hint = TALER_JSON_get_error_hint (j); mr.hr.hint = TALER_JSON_get_error_hint (j);
} }
break; break;
default: default:
GNUNET_break_op (0); GNUNET_break_op (0);
hr.http_status = 0; mr.hr.http_status = 0;
hr.ec = TALER_EC_EXCHANGE_MELT_INVALID_SIGNATURE_BY_EXCHANGE; mr.hr.ec = TALER_EC_EXCHANGE_MELT_INVALID_SIGNATURE_BY_EXCHANGE;
hr.hint = TALER_JSON_get_error_hint (j); mr.hr.hint = TALER_JSON_get_error_hint (j);
break; break;
} }
break; break;
@ -424,40 +405,35 @@ handle_melt_finished (void *cls,
/* Nothing really to verify, exchange says one of the signatures is /* Nothing really to verify, exchange says one of the signatures is
invalid; assuming we checked them, this should never happen, we invalid; assuming we checked them, this should never happen, we
should pass the JSON reply to the application */ should pass the JSON reply to the application */
hr.ec = TALER_JSON_get_error_code (j); mr.hr.ec = TALER_JSON_get_error_code (j);
hr.hint = TALER_JSON_get_error_hint (j); mr.hr.hint = TALER_JSON_get_error_hint (j);
break; break;
case MHD_HTTP_NOT_FOUND: case MHD_HTTP_NOT_FOUND:
/* Nothing really to verify, this should never /* Nothing really to verify, this should never
happen, we should pass the JSON reply to the application */ happen, we should pass the JSON reply to the application */
hr.ec = TALER_JSON_get_error_code (j); mr.hr.ec = TALER_JSON_get_error_code (j);
hr.hint = TALER_JSON_get_error_hint (j); mr.hr.hint = TALER_JSON_get_error_hint (j);
break; break;
case MHD_HTTP_INTERNAL_SERVER_ERROR: case MHD_HTTP_INTERNAL_SERVER_ERROR:
/* Server had an internal issue; we should retry, but this API /* Server had an internal issue; we should retry, but this API
leaves this to the application */ leaves this to the application */
hr.ec = TALER_JSON_get_error_code (j); mr.hr.ec = TALER_JSON_get_error_code (j);
hr.hint = TALER_JSON_get_error_hint (j); mr.hr.hint = TALER_JSON_get_error_hint (j);
break; break;
default: default:
/* unexpected response code */ /* unexpected response code */
hr.ec = TALER_JSON_get_error_code (j); mr.hr.ec = TALER_JSON_get_error_code (j);
hr.hint = TALER_JSON_get_error_hint (j); mr.hr.hint = TALER_JSON_get_error_hint (j);
GNUNET_log (GNUNET_ERROR_TYPE_ERROR, GNUNET_log (GNUNET_ERROR_TYPE_ERROR,
"Unexpected response code %u/%d for exchange melt\n", "Unexpected response code %u/%d for exchange melt\n",
(unsigned int) response_code, (unsigned int) response_code,
hr.ec); mr.hr.ec);
GNUNET_break_op (0); GNUNET_break_op (0);
break; break;
} }
if (NULL != mh->melt_cb) if (NULL != mh->melt_cb)
mh->melt_cb (mh->melt_cb_cls, mh->melt_cb (mh->melt_cb_cls,
&hr, &mr);
0,
NULL,
NULL,
UINT32_MAX,
NULL);
TALER_EXCHANGE_melt_cancel (mh); TALER_EXCHANGE_melt_cancel (mh);
} }
@ -479,11 +455,16 @@ start_melt (struct TALER_EXCHANGE_MeltHandle *mh)
struct TALER_CoinSpendSignatureP confirm_sig; struct TALER_CoinSpendSignatureP confirm_sig;
char arg_str[sizeof (struct TALER_CoinSpendPublicKeyP) * 2 + 32]; char arg_str[sizeof (struct TALER_CoinSpendPublicKeyP) * 2 + 32];
struct TALER_DenominationHash h_denom_pub; 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 != if (GNUNET_OK !=
TALER_EXCHANGE_get_melt_data_ (&mh->rms, TALER_EXCHANGE_get_melt_data_ (&mh->rms,
mh->rd, mh->rd,
mh->alg_values, alg_values,
&mh->md)) &mh->md))
{ {
GNUNET_break (0); GNUNET_break (0);
@ -574,11 +555,6 @@ fail_mh (struct TALER_EXCHANGE_MeltHandle *mh)
{ {
// FIXME: do return more than NULLs if the /csr failed! // FIXME: do return more than NULLs if the /csr failed!
mh->melt_cb (mh->melt_cb_cls, mh->melt_cb (mh->melt_cb_cls,
NULL,
0,
NULL,
NULL,
UINT32_MAX,
NULL); NULL);
TALER_EXCHANGE_melt_cancel (mh); TALER_EXCHANGE_melt_cancel (mh);
} }
@ -603,7 +579,7 @@ csr_cb (void *cls,
{ {
const struct TALER_EXCHANGE_DenomPublicKey *fresh_pk = const struct TALER_EXCHANGE_DenomPublicKey *fresh_pk =
&mh->rd->fresh_pks[i]; &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) switch (fresh_pk->key.cipher)
{ {
@ -656,21 +632,18 @@ TALER_EXCHANGE_melt (struct TALER_EXCHANGE_Handle *exchange,
mh->rms = *rms; mh->rms = *rms;
mh->melt_cb = melt_cb; mh->melt_cb = melt_cb;
mh->melt_cb_cls = melt_cb_cls; mh->melt_cb_cls = melt_cb_cls;
mh->alg_values = GNUNET_new_array (rd->fresh_pks_len, mh->mbds = GNUNET_new_array (rd->fresh_pks_len,
struct TALER_ExchangeWithdrawValues); struct TALER_EXCHANGE_MeltBlindingDetail);
mh->bks = GNUNET_new_array (rd->fresh_pks_len,
union TALER_DenominationBlindingKeyP);
for (unsigned int i = 0; i<rd->fresh_pks_len; i++) for (unsigned int i = 0; i<rd->fresh_pks_len; i++)
{ {
const struct TALER_EXCHANGE_DenomPublicKey *fresh_pk = &rd->fresh_pks[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) switch (fresh_pk->key.cipher)
{ {
case TALER_DENOMINATION_INVALID: case TALER_DENOMINATION_INVALID:
GNUNET_break (0); GNUNET_break (0);
GNUNET_free (mh->alg_values); GNUNET_free (mh->mbds);
GNUNET_free (mh->bks);
GNUNET_free (mh); GNUNET_free (mh);
return NULL; return NULL;
case TALER_DENOMINATION_RSA: case TALER_DENOMINATION_RSA:
@ -726,8 +699,7 @@ TALER_EXCHANGE_melt_cancel (struct TALER_EXCHANGE_MeltHandle *mh)
mh->csr = NULL; mh->csr = NULL;
} }
TALER_EXCHANGE_free_melt_data_ (&mh->md); /* does not free 'md' itself */ TALER_EXCHANGE_free_melt_data_ (&mh->md); /* does not free 'md' itself */
GNUNET_free (mh->alg_values); GNUNET_free (mh->mbds);
GNUNET_free (mh->bks);
GNUNET_free (mh->url); GNUNET_free (mh->url);
TALER_curl_easy_post_finished (&mh->ctx); TALER_curl_easy_post_finished (&mh->ctx);
GNUNET_free (mh); GNUNET_free (mh);

View File

@ -146,14 +146,15 @@ TALER_EXCHANGE_get_melt_data_ (
TALER_planchet_blinding_secret_create (fc, TALER_planchet_blinding_secret_create (fc,
&alg_values[j], &alg_values[j],
&bks); &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 so this computation is redundant, and here additionally
repeated KAPPA times. Could be avoided with slightly repeated KAPPA times. Could be avoided with slightly
more bookkeeping in the future */ more bookkeeping in the future */
TALER_cs_refresh_nonce_derive ( if (TALER_DENOMINATION_CS == alg_values[j].cipher)
rms, TALER_cs_refresh_nonce_derive (
j, rms,
&pd.blinded_planchet.details.cs_blinded_planchet.nonce); j,
&pd.blinded_planchet.details.cs_blinded_planchet.nonce);
if (GNUNET_OK != if (GNUNET_OK !=
TALER_planchet_prepare (&md->fresh_pks[j], TALER_planchet_prepare (&md->fresh_pks[j],
&alg_values[j], &alg_values[j],

View File

@ -102,12 +102,6 @@ struct MeltData
*/ */
struct TALER_DenominationPublicKey *fresh_pks; 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 * Arrays of @e num_fresh_coins with information about the fresh
* coins to be created, for each cut-and-choose dimension. * coins to be created, for each cut-and-choose dimension.

View File

@ -117,15 +117,10 @@ struct RefreshMeltState
struct TALER_EXCHANGE_DenomPublicKey *fresh_pks; 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; struct TALER_EXCHANGE_MeltBlindingDetail *mbds;
/**
* Array of @e num_fresh_coins of blinding key secrets
* created during the melt operation.
*/
union TALER_DenominationBlindingKeyP *bks;
/** /**
* Entropy seed for the refresh-melt operation. * Entropy seed for the refresh-melt operation.
@ -499,15 +494,20 @@ refresh_reveal_run (void *cls,
} }
// FIXME: use trait for 'rms'! // FIXME: use trait for 'rms'!
rms = melt_cmd->cls; rms = melt_cmd->cls;
rrs->rrh = TALER_EXCHANGE_refreshes_reveal (is->exchange, {
&rms->rms, struct TALER_ExchangeWithdrawValues alg_values[rms->num_fresh_coins];
&rms->refresh_data,
rms->num_fresh_coins,
rms->alg_values,
rms->noreveal_index,
&reveal_cb,
rrs);
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) if (NULL == rrs->rrh)
{ {
GNUNET_break (0); GNUNET_break (0);
@ -917,26 +917,15 @@ do_melt_retry (void *cls)
* CMD was set to do so. * CMD was set to do so.
* *
* @param cls closure. * @param cls closure.
* @param hr HTTP response details * @param mr melt 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.
*/ */
static void static void
melt_cb (void *cls, melt_cb (void *cls,
const struct TALER_EXCHANGE_HttpResponse *hr, const struct TALER_EXCHANGE_MeltResponse *mr)
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)
{ {
struct RefreshMeltState *rms = cls; struct RefreshMeltState *rms = cls;
const struct TALER_EXCHANGE_HttpResponse *hr = &mr->hr;
(void) exchange_pub;
rms->rmh = NULL; rms->rmh = NULL;
if (rms->expected_response_code != hr->http_status) if (rms->expected_response_code != hr->http_status)
{ {
@ -981,24 +970,18 @@ melt_cb (void *cls,
} }
if (MHD_HTTP_OK == hr->http_status) if (MHD_HTTP_OK == hr->http_status)
{ {
rms->noreveal_index = noreveal_index; rms->noreveal_index = mr->details.success.noreveal_index;
if (num_coins != rms->num_fresh_coins) if (mr->details.success.num_mbds != rms->num_fresh_coins)
{ {
GNUNET_break (0); GNUNET_break (0);
TALER_TESTING_interpreter_fail (rms->is); TALER_TESTING_interpreter_fail (rms->is);
return; return;
} }
GNUNET_free (rms->alg_values); GNUNET_free (rms->mbds);
rms->alg_values = GNUNET_new_array (num_coins, rms->mbds = GNUNET_memdup (mr->details.success.mbds,
struct TALER_ExchangeWithdrawValues); mr->details.success.num_mbds
memcpy (rms->alg_values, * sizeof (struct
alg_values, TALER_EXCHANGE_MeltBlindingDetail));
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));
} }
if (0 != rms->total_backoff.rel_value_us) 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); TALER_denom_pub_free (&rms->fresh_pks[i].key);
GNUNET_free (rms->fresh_pks); GNUNET_free (rms->fresh_pks);
} }
GNUNET_free (rms->alg_values); GNUNET_free (rms->mbds);
GNUNET_free (rms->bks);
GNUNET_free (rms->melt_fresh_amounts); GNUNET_free (rms->melt_fresh_amounts);
GNUNET_free (rms); GNUNET_free (rms);
} }
@ -1235,9 +1217,9 @@ melt_traits (void *cls,
TALER_TESTING_make_trait_coin_priv (0, TALER_TESTING_make_trait_coin_priv (0,
rms->melt_priv), rms->melt_priv),
TALER_TESTING_make_trait_blinding_key (index, TALER_TESTING_make_trait_blinding_key (index,
&rms->bks[index]), &rms->mbds[index].bks),
TALER_TESTING_make_trait_exchange_wd_value (index, 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_make_trait_refresh_secret (&rms->rms),
TALER_TESTING_trait_end () TALER_TESTING_trait_end ()
}; };