From 067719a3c9a96feeb533d5bd28d586fa31e43dcb Mon Sep 17 00:00:00 2001 From: Christian Grothoff Date: Tue, 17 Mar 2020 01:45:07 +0100 Subject: code cleanup, more consistent handling of libjansson allocation failures --- src/exchange/taler-exchange-httpd_keystate.c | 98 +++++++++++---- .../taler-exchange-httpd_refreshes_reveal.c | 132 +++++++++++---------- src/exchange/taler-exchange-httpd_wire.c | 10 +- src/exchange/taler-exchange-httpd_withdraw.c | 35 +----- 4 files changed, 158 insertions(+), 117 deletions(-) diff --git a/src/exchange/taler-exchange-httpd_keystate.c b/src/exchange/taler-exchange-httpd_keystate.c index 2f6c61874..422bc144e 100644 --- a/src/exchange/taler-exchange-httpd_keystate.c +++ b/src/exchange/taler-exchange-httpd_keystate.c @@ -916,10 +916,18 @@ revocations_iter (void *cls, obj = json_pack ("{s:o}", "h_denom_pub", GNUNET_JSON_from_data_auto (denom_hash)); - GNUNET_assert (NULL != obj); - GNUNET_assert (0 == - json_array_append_new (rfc->recoup_array, - obj)); + if (NULL == obj) + { + GNUNET_break (0); + return GNUNET_SYSERR; + } + if (0 != + json_array_append_new (rfc->recoup_array, + obj)) + { + GNUNET_break (0); + return GNUNET_SYSERR; + } } return GNUNET_OK; } @@ -1016,11 +1024,14 @@ reload_keys_sign_iter (void *cls, /* We use the most recent one, if it is valid now (not just in the near future) */ key_state->current_sign_key_issue = *ski; } - GNUNET_assert (0 == - json_array_append_new (rfc->sign_keys_array, - sign_key_issue_to_json (&ski->issue, - &ski->master_sig))); - + if (0 != + json_array_append_new (rfc->sign_keys_array, + sign_key_issue_to_json (&ski->issue, + &ski->master_sig))) + { + GNUNET_break (0); + return GNUNET_SYSERR; + } return GNUNET_OK; } @@ -1305,6 +1316,29 @@ struct AuditorEntry }; +/** + * Free auditor entry from the hash map. + * + * @param cls NULL + * @param key unused + * @param value a `struct AuditorEntry` to free + * @return #GNUNET_OK (to continue to iterate) + */ +static int +free_auditor_entry (void *cls, + const struct GNUNET_HashCode *key, + void *value) +{ + struct AuditorEntry *ae = value; + + (void) cls; + (void) key; + json_decref (ae->ar); + GNUNET_free (ae); + return GNUNET_OK; +} + + /** * Convert auditor entries from the hash map to entries * in the auditor array, free the auditor entry as well. @@ -1312,7 +1346,7 @@ struct AuditorEntry * @param cls a `struct ResponseBuilderContext *` * @param key unused * @param value a `struct AuditorEntry` to add to the `auditors_array` - * @return #GNUNET_OK (to continue to iterate) + * @return #GNUNET_OK (to continue to iterate), #GNUNET_SYSERR to terminate with error */ static int add_auditor_entry (void *cls, @@ -1328,10 +1362,18 @@ add_auditor_entry (void *cls, "denomination_keys", ae->ar, "auditor_url", ae->auditor_url, "auditor_pub", GNUNET_JSON_from_data_auto (ae->apub)); - GNUNET_assert (NULL != ao); - GNUNET_assert (0 == - json_array_append_new (rbc->auditors_array, - ao)); + if (NULL == ao) + { + GNUNET_break (0); + return GNUNET_SYSERR; + } + if (0 != + json_array_append_new (rbc->auditors_array, + ao)) + { + GNUNET_break (0); + return GNUNET_SYSERR; + } GNUNET_free (ae); return GNUNET_OK; } @@ -1442,15 +1484,25 @@ build_keys_response (const struct ResponseFactoryContext *rfc, ae, GNUNET_CONTAINER_MULTIHASHMAPOPTION_UNIQUE_ONLY)); } - GNUNET_assert (0 == - json_array_append_new (ae->ar, - json_pack ("{s:o, s:o}", - "denom_pub_h", - GNUNET_JSON_from_data_auto ( - denom_key_hash), - "auditor_sig", - GNUNET_JSON_from_data_auto ( - &as->asig)))); + if (0 != + json_array_append_new (ae->ar, + json_pack ("{s:o, s:o}", + "denom_pub_h", + GNUNET_JSON_from_data_auto ( + denom_key_hash), + "auditor_sig", + GNUNET_JSON_from_data_auto ( + &as->asig)))) + { + destroy_response_builder (&rbc); + GNUNET_break (0); + GNUNET_CONTAINER_multihashmap_iterate (auditors, + &free_auditor_entry, + NULL); + GNUNET_CONTAINER_multihashmap_destroy (auditors); + GNUNET_CRYPTO_hash_context_abort (rbc.hash_context); + return GNUNET_SYSERR; + } } } diff --git a/src/exchange/taler-exchange-httpd_refreshes_reveal.c b/src/exchange/taler-exchange-httpd_refreshes_reveal.c index 557935240..377baaa1a 100644 --- a/src/exchange/taler-exchange-httpd_refreshes_reveal.c +++ b/src/exchange/taler-exchange-httpd_refreshes_reveal.c @@ -58,67 +58,58 @@ reply_refreshes_reveal_success (struct MHD_Connection *connection, const struct TALER_DenominationSignature *sigs) { json_t *list; - int ret; list = json_array (); + if (NULL == list) + { + GNUNET_break (0); + return TALER_MHD_reply_with_error (connection, + MHD_HTTP_INTERNAL_SERVER_ERROR, + TALER_EC_JSON_ALLOCATION_FAILURE, + "json_array() call failed"); + } for (unsigned int freshcoin_index = 0; freshcoin_index < num_freshcoins; freshcoin_index++) { json_t *obj; - obj = json_object (); - json_object_set_new (obj, - "ev_sig", - GNUNET_JSON_from_rsa_signature ( - sigs[freshcoin_index].rsa_signature)); - GNUNET_assert (0 == - json_array_append_new (list, - obj)); - } - - { - json_t *root; - - root = json_object (); - json_object_set_new (root, - "ev_sigs", - list); - ret = TALER_MHD_reply_json (connection, - root, - MHD_HTTP_OK); - json_decref (root); + obj = json_pack ("{s:o}", + "ev_sig", + GNUNET_JSON_from_rsa_signature ( + sigs[freshcoin_index].rsa_signature)); + if (NULL == obj) + { + json_decref (list); + GNUNET_break (0); + return TALER_MHD_reply_with_error (connection, + MHD_HTTP_INTERNAL_SERVER_ERROR, + TALER_EC_JSON_ALLOCATION_FAILURE, + "json_pack() failed"); + } + if (0 != + json_array_append_new (list, + obj)) + { + json_decref (list); + GNUNET_break (0); + return TALER_MHD_reply_with_error (connection, + MHD_HTTP_INTERNAL_SERVER_ERROR, + TALER_EC_JSON_ALLOCATION_FAILURE, + "json_array_append_new() failed"); + } } - return ret; -} - -/** - * Send a response for a failed "/refresh/reveal", where the - * revealed value(s) do not match the original commitment. - * - * @param connection the connection to send the response to - * @param rc commitment computed by the exchange - * @return a MHD result code - */ -static int -reply_refreshes_reveal_mismatch (struct MHD_Connection *connection, - const struct TALER_RefreshCommitmentP *rc) -{ return TALER_MHD_reply_json_pack (connection, - MHD_HTTP_CONFLICT, - "{s:s, s:I, s:o}", - "hint", "commitment violation", - "code", - (json_int_t) - TALER_EC_REFRESH_REVEAL_COMMITMENT_VIOLATION, - "rc_expected", - GNUNET_JSON_from_data_auto (rc)); + MHD_HTTP_OK, + "{s:o}", + "ev_sigs", + list); } /** - * State for a /refresh/reveal operation. + * State for a /refreshes/$RCH/reveal operation. */ struct RevealContext { @@ -202,7 +193,12 @@ check_exists_cb (void *cls, GNUNET_break (0); return; } + /* This should be a database invariant for us */ GNUNET_break (TALER_CNC_KAPPA - 1 == num_tprivs); + /* Given that the $RCH value matched, we don't actually need to check these + values (we checked before). However, if a client repeats a request with + invalid values the 2nd time, that's a protocol violation we should at least + log (but it's safe to ignore it). */ GNUNET_break_op (0 == GNUNET_memcmp (tp, &rctx->gamma_tp)); @@ -225,8 +221,8 @@ check_exists_cb (void *cls, /** - * Check if the "/refresh/reveal" was already successful before. - * If so, just return the old result. + * Check if the "/refreshes/$RCH/reveal" request was already successful + * before. If so, just return the old result. * * @param cls closure of type `struct RevealContext` * @param connection MHD request which triggered the transaction @@ -275,7 +271,7 @@ refreshes_reveal_preflight (void *cls, /** - * Execute a "/refresh/reveal". The client is revealing to us the + * Execute a "/refreshes/$RCH/reveal". The client is revealing to us the * transfer keys for @a #TALER_CNC_KAPPA-1 sets of coins. Verify that the * revealed transfer keys would allow linkage to the blinded coins. * @@ -353,8 +349,8 @@ refreshes_reveal_transaction (void *cls, else { /* Reconstruct coin envelopes from transfer private key */ - struct TALER_TransferPrivateKeyP *tpriv = &rctx->transfer_privs[i - - off]; + struct TALER_TransferPrivateKeyP *tpriv + = &rctx->transfer_privs[i - off]; struct TALER_TransferSecretP ts; GNUNET_CRYPTO_ecdhe_key_get_public (&tpriv->ecdhe_priv, @@ -396,7 +392,7 @@ refreshes_reveal_transaction (void *cls, struct TALER_RefreshCommitmentEntry *rce = &rcs[i]; if (i == melt.session.noreveal_index) - continue; /* This offset is special... */ + continue; /* This offset is special: not allocated! */ for (unsigned int j = 0; jnum_fresh_coins; j++) { struct TALER_RefreshCoinData *rcd = &rce->new_coins[j]; @@ -411,8 +407,16 @@ refreshes_reveal_transaction (void *cls, &rc_expected)) { GNUNET_break_op (0); - *mhd_ret = reply_refreshes_reveal_mismatch (connection, - &rc_expected); + *mhd_ret = TALER_MHD_reply_json_pack (connection, + MHD_HTTP_CONFLICT, + "{s:s, s:I, s:o}", + "hint", "commitment violation", + "code", + (json_int_t) + TALER_EC_REFRESH_REVEAL_COMMITMENT_VIOLATION, + "rc_expected", + GNUNET_JSON_from_data_auto ( + &rc_expected)); return GNUNET_DB_STATUS_HARD_ERROR; } } /* end of checking "rc_expected" */ @@ -465,7 +469,7 @@ refreshes_reveal_transaction (void *cls, /** - * Persist result of a "/refresh/reveal". + * Persist result of a "/refreshes/$RCH/reveal" operation. * * @param cls closure of type `struct RevealContext` * @param connection MHD request which triggered the transaction @@ -637,7 +641,7 @@ resolve_refreshes_reveal_denominations (struct TEH_KS_StateHandle *key_state, res = TALER_MHD_reply_with_error (connection, MHD_HTTP_INTERNAL_SERVER_ERROR, TALER_EC_INTERNAL_INVARIANT_FAILURE, - "assertion failed"); + "assertion failed (unexpected database serialization error)"); break; } goto cleanup; @@ -660,13 +664,14 @@ resolve_refreshes_reveal_denominations (struct TEH_KS_StateHandle *key_state, return (GNUNET_NO == res) ? MHD_YES : MHD_NO; /* Check link_sigs[i] signature */ { - struct TALER_LinkDataPS ldp; + struct TALER_LinkDataPS ldp = { + .purpose.size = htonl (sizeof (ldp)), + .purpose.purpose = htonl (TALER_SIGNATURE_WALLET_COIN_LINK), + .h_denom_pub = dki_h[i], + .old_coin_pub = melt.session.coin.coin_pub, + .transfer_pub = rctx->gamma_tp + }; - ldp.purpose.size = htonl (sizeof (ldp)); - ldp.purpose.purpose = htonl (TALER_SIGNATURE_WALLET_COIN_LINK); - ldp.h_denom_pub = dki_h[i]; - ldp.old_coin_pub = melt.session.coin.coin_pub; - ldp.transfer_pub = rctx->gamma_tp; GNUNET_CRYPTO_hash (rcds[i].coin_ev, rcds[i].coin_ev_size, &ldp.coin_envelope_hash); @@ -767,6 +772,9 @@ resolve_refreshes_reveal_denominations (struct TEH_KS_StateHandle *key_state, rctx->ev_sigs); break; } + /* If we get here, the final transaction failed, possibly + due to a conflict between the pre-flight and us persisting + the result, so we go again. */ } /* end for (retries...) */ cleanup: diff --git a/src/exchange/taler-exchange-httpd_wire.c b/src/exchange/taler-exchange-httpd_wire.c index 126c7cdff..cb63355b6 100644 --- a/src/exchange/taler-exchange-httpd_wire.c +++ b/src/exchange/taler-exchange-httpd_wire.c @@ -256,9 +256,13 @@ load_account (void *cls, if (GNUNET_OK == load_fee (ai->method)) { - GNUNET_assert (-1 != - json_array_append_new (wire_accounts_array, - wire_s)); + if (0 != + json_array_append_new (wire_accounts_array, + wire_s)) + { + GNUNET_break (0); + *ret = GNUNET_SYSERR; + } } else { diff --git a/src/exchange/taler-exchange-httpd_withdraw.c b/src/exchange/taler-exchange-httpd_withdraw.c index 4575a8ab5..5665f24fb 100644 --- a/src/exchange/taler-exchange-httpd_withdraw.c +++ b/src/exchange/taler-exchange-httpd_withdraw.c @@ -91,33 +91,6 @@ reply_withdraw_insufficient_funds ( } -/** - * Send blind signature to client. - * - * @param connection connection to the client - * @param collectable blinded coin to return - * @return MHD result code - */ -static int -reply_withdraw_success ( - struct MHD_Connection *connection, - const struct TALER_EXCHANGEDB_CollectableBlindcoin *collectable) -{ - json_t *sig_json; - - sig_json = GNUNET_JSON_from_rsa_signature (collectable->sig.rsa_signature); - if (NULL == sig_json) - return TALER_MHD_reply_with_error (connection, - MHD_HTTP_INTERNAL_SERVER_ERROR, - TALER_EC_JSON_ALLOCATION_FAILURE, - "GNUNET_JSON_from_rsa_signature() failed"); - return TALER_MHD_reply_json_pack (connection, - MHD_HTTP_OK, - "{s:o}", - "ev_sig", sig_json); -} - - /** * Context for #withdraw_transaction. */ @@ -540,8 +513,12 @@ TEH_handler_withdraw (const struct TEH_RequestHandler *rh, { int ret; - ret = reply_withdraw_success (connection, - &wc.collectable); + ret = TALER_MHD_reply_json_pack ( + connection, + MHD_HTTP_OK, + "{s:o}", + "ev_sig", GNUNET_JSON_from_rsa_signature ( + wc.collectable.sig.rsa_signature)); GNUNET_CRYPTO_rsa_signature_free (wc.collectable.sig.rsa_signature); return ret; } -- cgit v1.2.3