From 4741f4ea02148ed1dede88affd83edd9a96e2680 Mon Sep 17 00:00:00 2001 From: Christian Grothoff Date: Thu, 20 May 2021 12:31:27 +0200 Subject: implement duplicate reserve_pub detection in fakebank, add test (fails in pybank), for #6863 --- src/bank-lib/fakebank.c | 173 +++++++++++++++++---- src/include/taler_crypto_lib.h | 13 ++ src/include/taler_fakebank_lib.h | 2 +- src/testing/test_bank_api.c | 5 + src/testing/test_exchange_api.c | 18 +-- .../testing_api_cmd_bank_admin_add_incoming.c | 51 +++--- 6 files changed, 195 insertions(+), 67 deletions(-) (limited to 'src') diff --git a/src/bank-lib/fakebank.c b/src/bank-lib/fakebank.c index 88fb22793..5091b4885 100644 --- a/src/bank-lib/fakebank.c +++ b/src/bank-lib/fakebank.c @@ -81,7 +81,12 @@ struct Transaction /** * Transfer FROM the exchange. */ - T_DEBIT + T_DEBIT, + + /** + * Exchange-to-exchange WAD transfer. + */ + T_WAD, } type; /** @@ -121,6 +126,24 @@ struct Transaction } credit; + /** + * Used if @e type is T_WAD. + */ + struct + { + + /** + * Subject of the transfer. + */ + struct TALER_WadIdentifierP wad; + + /** + * Base URL of the originating exchange. + */ + char *origin_base_url; + + } wad; + } subject; /** @@ -168,6 +191,13 @@ struct TALER_FAKEBANK_Handle */ struct GNUNET_SCHEDULER_Task *mhd_task; + /** + * Hashmap of reserve public keys to + * `struct Transaction` with that reserve public + * key. Used to prevent public-key re-use. + */ + struct GNUNET_CONTAINER_MultiPeerMap *rpubs; + /** * Number of transactions. */ @@ -210,19 +240,43 @@ struct TALER_FAKEBANK_Handle static void check_log (struct TALER_FAKEBANK_Handle *h) { - for (struct Transaction *t = h->transactions_head; NULL != t; t = t->next) + for (struct Transaction *t = h->transactions_head; + NULL != t; + t = t->next) { if (GNUNET_YES == t->checked) continue; - GNUNET_log (GNUNET_ERROR_TYPE_ERROR, - "%s -> %s (%s) %s (%s)\n", - t->debit_account, - t->credit_account, - TALER_amount2s (&t->amount), - (T_DEBIT == t->type) - ? t->subject.debit.exchange_base_url - : TALER_B2S (&t->subject.credit.reserve_pub), - (T_DEBIT == t->type) ? "DEBIT" : "CREDIT"); + switch (t->type) + { + case T_DEBIT: + GNUNET_log (GNUNET_ERROR_TYPE_ERROR, + "%s -> %s (%s) %s (%s)\n", + t->debit_account, + t->credit_account, + TALER_amount2s (&t->amount), + t->subject.debit.exchange_base_url, + "DEBIT"); + break; + case T_CREDIT: + GNUNET_log (GNUNET_ERROR_TYPE_ERROR, + "%s -> %s (%s) %s (%s)\n", + t->debit_account, + t->credit_account, + TALER_amount2s (&t->amount), + TALER_B2S (&t->subject.credit.reserve_pub), + "CREDIT"); + break; + case T_WAD: + GNUNET_log (GNUNET_ERROR_TYPE_ERROR, + "%s -> %s (%s) %s[%s] (%s)\n", + t->debit_account, + t->credit_account, + TALER_amount2s (&t->amount), + t->subject.wad.origin_base_url, + TALER_B2S (&t->subject.wad), + "WAD"); + break; + } } } @@ -252,7 +306,9 @@ TALER_FAKEBANK_check_debit (struct TALER_FAKEBANK_Handle *h, { GNUNET_assert (0 == strcasecmp (want_amount->currency, h->currency)); - for (struct Transaction *t = h->transactions_head; NULL != t; t = t->next) + for (struct Transaction *t = h->transactions_head; + NULL != t; + t = t->next) { if ( (0 == strcasecmp (want_debit, t->debit_account)) && @@ -304,7 +360,9 @@ TALER_FAKEBANK_check_credit (struct TALER_FAKEBANK_Handle *h, { GNUNET_assert (0 == strcasecmp (want_amount->currency, h->currency)); - for (struct Transaction *t = h->transactions_head; NULL != t; t = t->next) + for (struct Transaction *t = h->transactions_head; + NULL != t; + t = t->next) { if ( (0 == strcasecmp (want_debit, t->debit_account)) && @@ -371,9 +429,12 @@ TALER_FAKEBANK_make_transfer ( strlen ("payto://"))); if (NULL != request_uid) { - for (struct Transaction *t = h->transactions_head; NULL != t; t = t->next) + for (struct Transaction *t = h->transactions_head; + NULL != t; + t = t->next) { - if (0 != GNUNET_memcmp (request_uid, &t->request_uid)) + if (0 != GNUNET_memcmp (request_uid, + &t->request_uid)) continue; if ( (0 != strcasecmp (debit_account, t->debit_account)) || @@ -442,7 +503,18 @@ TALER_FAKEBANK_make_admin_transfer ( const struct TALER_ReservePublicKeyP *reserve_pub) { struct Transaction *t; - + const struct GNUNET_PeerIdentity *pid; + + GNUNET_assert (sizeof (*pid) == + sizeof (*reserve_pub)); + pid = (const struct GNUNET_PeerIdentity *) reserve_pub; + t = GNUNET_CONTAINER_multipeermap_get (h->rpubs, + pid); + if (NULL != t) + { + GNUNET_break (0); + return 0; + } GNUNET_assert (0 == strcasecmp (amount->currency, h->currency)); GNUNET_assert (NULL != debit_account); @@ -465,6 +537,12 @@ TALER_FAKEBANK_make_admin_transfer ( GNUNET_CONTAINER_DLL_insert_tail (h->transactions_head, h->transactions_tail, t); + GNUNET_assert (GNUNET_OK == + GNUNET_CONTAINER_multipeermap_put ( + h->rpubs, + pid, + t, + GNUNET_CONTAINER_MULTIHASHMAPOPTION_UNIQUE_ONLY)); GNUNET_log (GNUNET_ERROR_TYPE_INFO, "Making transfer from %s to %s over %s and subject %s at row %llu\n", debit_account, @@ -523,8 +601,18 @@ TALER_FAKEBANK_stop (struct TALER_FAKEBANK_Handle *h) t); GNUNET_free (t->debit_account); GNUNET_free (t->credit_account); - if (T_DEBIT == t->type) + switch (t->type) + { + case T_CREDIT: + /* nothing to free */ + break; + case T_DEBIT: GNUNET_free (t->subject.debit.exchange_base_url); + break; + case T_WAD: + GNUNET_free (t->subject.wad.origin_base_url); + break; + } GNUNET_free (t); } if (NULL != h->mhd_task) @@ -541,6 +629,7 @@ TALER_FAKEBANK_stop (struct TALER_FAKEBANK_Handle *h) h->mhd_bank = NULL; } GNUNET_free (h->my_baseurl); + GNUNET_CONTAINER_multipeermap_destroy (h->rpubs); GNUNET_free (h->currency); GNUNET_free (h); } @@ -626,9 +715,12 @@ handle_admin_add_incoming (struct TALER_FAKEBANK_Handle *h, struct TALER_ReservePublicKeyP reserve_pub; char *debit; struct GNUNET_JSON_Specification spec[] = { - GNUNET_JSON_spec_fixed_auto ("reserve_pub", &reserve_pub), - GNUNET_JSON_spec_string ("debit_account", &debit_account), - TALER_JSON_spec_amount ("amount", &amount), + GNUNET_JSON_spec_fixed_auto ("reserve_pub", + &reserve_pub), + GNUNET_JSON_spec_string ("debit_account", + &debit_account), + TALER_JSON_spec_amount ("amount", + &amount), GNUNET_JSON_spec_end () }; @@ -642,6 +734,15 @@ handle_admin_add_incoming (struct TALER_FAKEBANK_Handle *h, /* We're fakebank, no need for nice error handling */ return MHD_NO; } + if (0 != strcasecmp (amount.currency, + h->currency)) + { + return TALER_MHD_reply_with_error ( + connection, + MHD_HTTP_CONFLICT, + TALER_EC_GENERIC_CURRENCY_MISMATCH, + NULL); + } debit = TALER_xtalerbank_account_from_payto (debit_account); GNUNET_log (GNUNET_ERROR_TYPE_INFO, "Receiving incoming wire transfer: %s->%s, subject: %s, amount: %s\n", @@ -655,6 +756,14 @@ handle_admin_add_incoming (struct TALER_FAKEBANK_Handle *h, &amount, &reserve_pub); GNUNET_free (debit); + if (0 == row_id) + { + return TALER_MHD_reply_with_error ( + connection, + MHD_HTTP_CONFLICT, + TALER_EC_BANK_DUPLICATE_RESERVE_PUB_SUBJECT, + NULL); + } } json_decref (json); @@ -821,11 +930,9 @@ handle_home_page (struct TALER_FAKEBANK_Handle *h, (strlen (HELLOMSG), HELLOMSG, MHD_RESPMEM_MUST_COPY); - ret = MHD_queue_response (connection, MHD_HTTP_OK, resp); - MHD_destroy_response (resp); return ret; } @@ -1389,12 +1496,17 @@ schedule_httpd (struct TALER_FAKEBANK_Handle *h) FD_ZERO (&ws); FD_ZERO (&es); max = -1; - if (MHD_YES != MHD_get_fdset (h->mhd_bank, &rs, &ws, &es, &max)) + if (MHD_YES != MHD_get_fdset (h->mhd_bank, + &rs, + &ws, + &es, + &max)) { GNUNET_assert (0); return; } - haveto = MHD_get_timeout (h->mhd_bank, &timeout); + haveto = MHD_get_timeout (h->mhd_bank, + &timeout); if (MHD_YES == haveto) tv.rel_value_us = (uint64_t) timeout * 1000LL; else @@ -1403,8 +1515,12 @@ schedule_httpd (struct TALER_FAKEBANK_Handle *h) { wrs = GNUNET_NETWORK_fdset_create (); wws = GNUNET_NETWORK_fdset_create (); - GNUNET_NETWORK_fdset_copy_native (wrs, &rs, max + 1); - GNUNET_NETWORK_fdset_copy_native (wws, &ws, max + 1); + GNUNET_NETWORK_fdset_copy_native (wrs, + &rs, + max + 1); + GNUNET_NETWORK_fdset_copy_native (wws, + &ws, + max + 1); } else { @@ -1418,7 +1534,8 @@ schedule_httpd (struct TALER_FAKEBANK_Handle *h) tv, wrs, wws, - &run_mhd, h); + &run_mhd, + h); if (NULL != wrs) GNUNET_NETWORK_fdset_destroy (wrs); if (NULL != wws) @@ -1468,6 +1585,8 @@ TALER_FAKEBANK_start (uint16_t port, GNUNET_assert (strlen (currency) < TALER_CURRENCY_LEN); h = GNUNET_new (struct TALER_FAKEBANK_Handle); h->port = port; + h->rpubs = GNUNET_CONTAINER_multipeermap_create (128, + GNUNET_NO); h->currency = GNUNET_strdup (currency); GNUNET_asprintf (&h->my_baseurl, "http://localhost:%u/", diff --git a/src/include/taler_crypto_lib.h b/src/include/taler_crypto_lib.h index 8d1bbec26..71579170a 100644 --- a/src/include/taler_crypto_lib.h +++ b/src/include/taler_crypto_lib.h @@ -591,6 +591,19 @@ struct TALER_WireTransferIdentifierRawP }; +/** + * Raw value of a wire transfer subject for a wad. + */ +struct TALER_WadIdentifierP +{ + + /** + * Wad identifier, in binary encoding. + */ + uint8_t raw[24]; +}; + + /** * Binary information encoded in Crockford's Base32 in wire transfer * subjects of transfers from Taler to a merchant. The actual value diff --git a/src/include/taler_fakebank_lib.h b/src/include/taler_fakebank_lib.h index bd4376695..abef2de52 100644 --- a/src/include/taler_fakebank_lib.h +++ b/src/include/taler_fakebank_lib.h @@ -99,7 +99,7 @@ TALER_FAKEBANK_make_transfer ( * @param credit_account account to credit * @param amount amount to transfer * @param reserve_pub reserve public key to use in subject - * @return serial_id of the transfer + * @return serial_id of the transfer, 0 on error */ uint64_t TALER_FAKEBANK_make_admin_transfer ( diff --git a/src/testing/test_bank_api.c b/src/testing/test_bank_api.c index 7069e44f9..98f97cd44 100644 --- a/src/testing/test_bank_api.c +++ b/src/testing/test_bank_api.c @@ -87,6 +87,11 @@ run (void *cls, "KUDOS:5.01", &bc.exchange_auth, bc.user42_payto), + TALER_TESTING_cmd_admin_add_incoming_with_ref ("credit-1-fail", + "KUDOS:2.01", + &bc.exchange_auth, + bc.user42_payto, + "credit-1"), TALER_TESTING_cmd_sleep ("Waiting 4s for 'credit-1' to settle", 4), TALER_TESTING_cmd_bank_credits ("history-1c", diff --git a/src/testing/test_exchange_api.c b/src/testing/test_exchange_api.c index 18f7237b6..008eefb20 100644 --- a/src/testing/test_exchange_api.c +++ b/src/testing/test_exchange_api.c @@ -121,9 +121,9 @@ run (void *cls, * Move money to the exchange's bank account. */ CMD_TRANSFER_TO_EXCHANGE ("create-reserve-1", - "EUR:4.01"), + "EUR:6.02"), TALER_TESTING_cmd_check_bank_admin_transfer ("check-create-reserve-1", - "EUR:4.01", + "EUR:6.02", bc.user42_payto, bc.exchange_payto, "create-reserve-1"), @@ -132,20 +132,6 @@ run (void *cls, * transfer. */ CMD_EXEC_WIREWATCH ("wirewatch-1"), - /** - * Do another transfer to the same reserve - */ - TALER_TESTING_cmd_admin_add_incoming_with_ref ("create-reserve-1.2", - "EUR:2.01", - &bc.exchange_auth, - bc.user42_payto, - "create-reserve-1"), - TALER_TESTING_cmd_check_bank_admin_transfer ("check-create-reserve-1.2", - "EUR:2.01", - bc.user42_payto, - bc.exchange_payto, - "create-reserve-1.2"), - CMD_EXEC_WIREWATCH ("wirewatch-1.2"), /** * Withdraw EUR:5. */ diff --git a/src/testing/testing_api_cmd_bank_admin_add_incoming.c b/src/testing/testing_api_cmd_bank_admin_add_incoming.c index e467063aa..aaa447fec 100644 --- a/src/testing/testing_api_cmd_bank_admin_add_incoming.c +++ b/src/testing/testing_api_cmd_bank_admin_add_incoming.c @@ -148,6 +148,11 @@ struct AdminAddIncomingState * enable retries? If so, how often should we still retry? */ unsigned int do_retry; + + /** + * Expected HTTP status code. + */ + unsigned int expected_http_status; }; @@ -215,6 +220,13 @@ confirmation_cb (void *cls, switch (http_status) { case MHD_HTTP_OK: + if (fts->expected_http_status != + MHD_HTTP_OK) + { + GNUNET_break (0); + TALER_TESTING_interpreter_fail (is); + return; + } fts->serial_id = serial_id; fts->timestamp = timestamp; TALER_TESTING_interpreter_next (is); @@ -233,6 +245,16 @@ confirmation_cb (void *cls, break; } break; + case MHD_HTTP_CONFLICT: + if (fts->expected_http_status != + MHD_HTTP_CONFLICT) + { + GNUNET_break (0); + TALER_TESTING_interpreter_fail (is); + return; + } + TALER_TESTING_interpreter_next (is); + return; default: if (0 != fts->do_retry) { @@ -405,6 +427,10 @@ admin_add_incoming_traits (void *cls, unsigned int index) { struct AdminAddIncomingState *fts = cls; + + if (MHD_HTTP_OK != + fts->expected_http_status) + return GNUNET_NO; /* requests that failed generate no history */ if (fts->reserve_priv_known) { struct TALER_TESTING_Trait traits[] = { @@ -479,6 +505,7 @@ make_fts (const char *amount, fts->exchange_credit_url = auth->wire_gateway_url; fts->payto_debit_account = payto_debit_account; fts->auth = *auth; + fts->expected_http_status = MHD_HTTP_OK; if (GNUNET_OK != TALER_string_to_amount (amount, &fts->amount)) @@ -515,15 +542,6 @@ make_command (const char *label, } -/** - * Create admin/add-incoming command. - * - * @param label command label. - * @param amount amount to transfer. - * @param payto_debit_account which account sends money. - * @param auth authentication data - * @return the command. - */ struct TALER_TESTING_Command TALER_TESTING_cmd_admin_add_incoming (const char *label, const char *amount, @@ -538,20 +556,6 @@ TALER_TESTING_cmd_admin_add_incoming (const char *label, } -/** - * Create "/admin/add-incoming" CMD, letting the caller specify - * a reference to a command that can offer a reserve private key. - * This private key will then be used to construct the subject line - * of the wire transfer. - * - * @param label command label. - * @param amount the amount to transfer. - * @param payto_debit_account which account sends money - * @param auth authentication data - * @param ref reference to a command that can offer a reserve - * private key or public key. - * @return the command. - */ struct TALER_TESTING_Command TALER_TESTING_cmd_admin_add_incoming_with_ref (const char *label, @@ -566,6 +570,7 @@ TALER_TESTING_cmd_admin_add_incoming_with_ref auth, payto_debit_account); fts->reserve_reference = ref; + fts->expected_http_status = MHD_HTTP_CONFLICT; return make_command (label, fts); } -- cgit v1.2.3