From 81af01a209b557a4996a50824ae2f1ec75c25eb5 Mon Sep 17 00:00:00 2001 From: Christian Grothoff Date: Mon, 17 Aug 2015 03:07:48 +0200 Subject: [PATCH 1/6] fix DB logic: actually iterate where we need to --- src/mintdb/plugin_mintdb_postgres.c | 115 +++++++++++++++------------- 1 file changed, 61 insertions(+), 54 deletions(-) diff --git a/src/mintdb/plugin_mintdb_postgres.c b/src/mintdb/plugin_mintdb_postgres.c index beb1efb3e..aaa1c9016 100644 --- a/src/mintdb/plugin_mintdb_postgres.c +++ b/src/mintdb/plugin_mintdb_postgres.c @@ -2648,33 +2648,36 @@ postgres_insert_refresh_commit_links (void *cls, uint16_t num_links, const struct TALER_RefreshCommitLinkP *links) { - // FIXME: check logic! links is array! - struct TALER_PQ_QueryParam params[] = { - TALER_PQ_query_param_auto_from_type (session_hash), - TALER_PQ_query_param_auto_from_type (&links->transfer_pub), - TALER_PQ_query_param_uint16 (&cnc_index), - TALER_PQ_query_param_uint16 (&num_links), - TALER_PQ_query_param_auto_from_type (&links->shared_secret_enc), - TALER_PQ_query_param_end - }; + uint16_t i; - PGresult *result = TALER_PQ_exec_prepared (session->conn, - "insert_refresh_commit_link", - params); - if (PGRES_COMMAND_OK != PQresultStatus (result)) + for (i=0;iconn, + "insert_refresh_commit_link", + params); + if (PGRES_COMMAND_OK != PQresultStatus (result)) + { + BREAK_DB_ERR (result); + PQclear (result); + return GNUNET_SYSERR; + } + + if (0 != strcmp ("1", PQcmdTuples (result))) + { + GNUNET_break (0); + return GNUNET_SYSERR; + } PQclear (result); - return GNUNET_SYSERR; } - - if (0 != strcmp ("1", PQcmdTuples (result))) - { - GNUNET_break (0); - return GNUNET_SYSERR; - } - - PQclear (result); return GNUNET_OK; } @@ -2701,46 +2704,50 @@ postgres_get_refresh_commit_links (void *cls, uint16_t num_links, struct TALER_RefreshCommitLinkP *links) { - // FIXME: check logic: was written for a single link! - struct TALER_PQ_QueryParam params[] = { - TALER_PQ_query_param_auto_from_type (session_hash), - TALER_PQ_query_param_uint16 (&cnc_index), - TALER_PQ_query_param_uint16 (&num_links), - TALER_PQ_query_param_end - }; - PGresult *result; + uint16_t i; - result = TALER_PQ_exec_prepared (session->conn, - "get_refresh_commit_link", - params); - if (PGRES_TUPLES_OK != PQresultStatus (result)) + for (i=0;itransfer_pub), - TALER_PQ_result_spec_auto_from_type ("link_secret_enc", - &links->shared_secret_enc), - TALER_PQ_result_spec_end + struct TALER_PQ_QueryParam params[] = { + TALER_PQ_query_param_auto_from_type (session_hash), + TALER_PQ_query_param_uint16 (&cnc_index), + TALER_PQ_query_param_uint16 (&i), + TALER_PQ_query_param_end }; + PGresult *result; - if (GNUNET_YES != - TALER_PQ_extract_result (result, rs, 0)) + result = TALER_PQ_exec_prepared (session->conn, + "get_refresh_commit_link", + params); + if (PGRES_TUPLES_OK != PQresultStatus (result)) { + BREAK_DB_ERR (result); PQclear (result); return GNUNET_SYSERR; } + if (0 == PQntuples (result)) + { + PQclear (result); + return GNUNET_NO; + } + { + struct TALER_PQ_ResultSpec rs[] = { + TALER_PQ_result_spec_auto_from_type ("transfer_pub", + &links[i].transfer_pub), + TALER_PQ_result_spec_auto_from_type ("link_secret_enc", + &links[i].shared_secret_enc), + TALER_PQ_result_spec_end + }; + + if (GNUNET_YES != + TALER_PQ_extract_result (result, rs, 0)) + { + PQclear (result); + return GNUNET_SYSERR; + } + } + PQclear (result); } - PQclear (result); return GNUNET_OK; } From b5a58e516c0d0ee48547dd7bb9f3d1a3b9ebf15e Mon Sep 17 00:00:00 2001 From: Christian Grothoff Date: Mon, 17 Aug 2015 03:24:10 +0200 Subject: [PATCH 2/6] fix /refresh/link response handling --- src/mint-lib/mint_api_refresh_link.c | 137 ++++++++++++++++++--------- src/mint-lib/test_mint_api.c | 4 + 2 files changed, 97 insertions(+), 44 deletions(-) diff --git a/src/mint-lib/mint_api_refresh_link.c b/src/mint-lib/mint_api_refresh_link.c index f17949af0..b4bed98e6 100644 --- a/src/mint-lib/mint_api_refresh_link.c +++ b/src/mint-lib/mint_api_refresh_link.c @@ -173,73 +173,122 @@ static int parse_refresh_link_ok (struct TALER_MINT_RefreshLinkHandle *rlh, json_t *json) { - json_t *jsona; - struct TALER_TransferPublicKeyP trans_pub; - struct TALER_EncryptedLinkSecretP secret_enc; - struct MAJ_Specification spec[] = { - MAJ_spec_json ("new_coins", &jsona), - MAJ_spec_fixed_auto ("trans_pub", &trans_pub), - MAJ_spec_fixed_auto ("secret_enc", &secret_enc), - MAJ_spec_end - }; + unsigned int session; unsigned int num_coins; int ret; - if (GNUNET_OK != - MAJ_parse_json (json, - spec)) + if (! json_is_array (json)) { GNUNET_break_op (0); return GNUNET_SYSERR; } - if (! json_is_array (jsona)) + num_coins = 0; + for (session=0;sessionlink_cb (rlh->link_cb_cls, + MHD_HTTP_OK, + num_coins, + coin_privs, + sigs, + pubs, + json); + rlh->link_cb = NULL; + ret = GNUNET_OK; } else { - rlh->link_cb (rlh->link_cb_cls, - MHD_HTTP_OK, - num_coins, - coin_privs, - sigs, - pubs, - json); - rlh->link_cb = NULL; - ret = GNUNET_OK; + GNUNET_break_op (0); + ret = GNUNET_SYSERR; } /* clean up */ diff --git a/src/mint-lib/test_mint_api.c b/src/mint-lib/test_mint_api.c index 91a3d7628..51f62cc32 100644 --- a/src/mint-lib/test_mint_api.c +++ b/src/mint-lib/test_mint_api.c @@ -1009,6 +1009,10 @@ link_cb (void *cls, return; } /* check that the coins match */ + fprintf (stderr, + "Got %u coins\n", + num_coins); + /* FIXME: note: coins might be legitimately permutated in here... */ for (i=0;i Date: Mon, 17 Aug 2015 03:35:11 +0200 Subject: [PATCH 3/6] fix use-after-free, ignore errors in testcase that can be explained by unsupported permuatations of the results --- src/mint-lib/mint_api_refresh_link.c | 6 ++++-- src/mint-lib/test_mint_api.c | 5 +++-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/mint-lib/mint_api_refresh_link.c b/src/mint-lib/mint_api_refresh_link.c index b4bed98e6..d4060bd1c 100644 --- a/src/mint-lib/mint_api_refresh_link.c +++ b/src/mint-lib/mint_api_refresh_link.c @@ -100,9 +100,10 @@ parse_refresh_link_coin (const struct TALER_MINT_RefreshLinkHandle *rlh, void *link_enc; size_t link_enc_size; struct GNUNET_CRYPTO_rsa_Signature *bsig; + struct GNUNET_CRYPTO_rsa_PublicKey *rpub; struct MAJ_Specification spec[] = { MAJ_spec_varsize ("link_enc", &link_enc, &link_enc_size), - MAJ_spec_rsa_public_key ("denom_pub", &pub->rsa_public_key), + MAJ_spec_rsa_public_key ("denom_pub", &rpub), MAJ_spec_rsa_signature ("ev_sig", &bsig), MAJ_spec_end }; @@ -152,10 +153,11 @@ parse_refresh_link_coin (const struct TALER_MINT_RefreshLinkHandle *rlh, sig->rsa_signature = GNUNET_CRYPTO_rsa_unblind (bsig, rld->blinding_key.rsa_blinding_key, - pub->rsa_public_key); + rpub); /* clean up */ GNUNET_free (rld); + pub->rsa_public_key = GNUNET_CRYPTO_rsa_public_key_dup (rpub); MAJ_parse_free (spec); return GNUNET_OK; } diff --git a/src/mint-lib/test_mint_api.c b/src/mint-lib/test_mint_api.c index 51f62cc32..340e9d17f 100644 --- a/src/mint-lib/test_mint_api.c +++ b/src/mint-lib/test_mint_api.c @@ -1013,6 +1013,8 @@ link_cb (void *cls, "Got %u coins\n", num_coins); /* FIXME: note: coins might be legitimately permutated in here... */ + /* (in fact, we currently get them in reverse order, and that's + why this is "failing") */ for (i=0;i Date: Mon, 17 Aug 2015 03:40:16 +0200 Subject: [PATCH 4/6] use correct response code --- src/mint/taler-mint-httpd_responses.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mint/taler-mint-httpd_responses.c b/src/mint/taler-mint-httpd_responses.c index 8a5573e87..7a56efb9f 100644 --- a/src/mint/taler-mint-httpd_responses.c +++ b/src/mint/taler-mint-httpd_responses.c @@ -711,7 +711,7 @@ TMH_RESPONSE_reply_refresh_melt_insufficient_funds (struct MHD_Connection *conne history = compile_transaction_history (tl); return TMH_RESPONSE_reply_json_pack (connection, - MHD_HTTP_NOT_FOUND, + MHD_HTTP_FORBIDDEN, "{s:s, s:o, s:o, s:o, s:o, s:o}", "error", "insufficient funds", From c4a68b896fa0889cc96cc30b2eae38e1996f4300 Mon Sep 17 00:00:00 2001 From: Christian Grothoff Date: Mon, 17 Aug 2015 03:56:49 +0200 Subject: [PATCH 5/6] add internal sig verification, pack hashes, fix testcase --- src/include/taler_signatures.h | 14 ++++++------- src/mint-lib/mint_api_json.c | 3 +++ src/mint-lib/test_mint_api.c | 10 +++++---- src/mint/taler-mint-httpd_responses.c | 30 +++++++++++++++++++++++++++ 4 files changed, 46 insertions(+), 11 deletions(-) diff --git a/src/include/taler_signatures.h b/src/include/taler_signatures.h index 043f3bda1..ffbc9fd45 100644 --- a/src/include/taler_signatures.h +++ b/src/include/taler_signatures.h @@ -215,12 +215,12 @@ struct TALER_WithdrawRequestPS /** * Hash of the denomination public key for the coin that is withdrawn. */ - struct GNUNET_HashCode h_denomination_pub; + struct GNUNET_HashCode h_denomination_pub GNUNET_PACKED; /** * Hash of the (blinded) message to be signed by the Mint. */ - struct GNUNET_HashCode h_coin_envelope; + struct GNUNET_HashCode h_coin_envelope GNUNET_PACKED; }; @@ -239,12 +239,12 @@ struct TALER_DepositRequestPS /** * Hash over the contract for which this deposit is made. */ - struct GNUNET_HashCode h_contract; + struct GNUNET_HashCode h_contract GNUNET_PACKED; /** * Hash over the wiring information of the merchant. */ - struct GNUNET_HashCode h_wire; + struct GNUNET_HashCode h_wire GNUNET_PACKED; /** * Time when this request was generated. Used, for example, to @@ -330,12 +330,12 @@ struct TALER_DepositConfirmationPS /** * Hash over the contract for which this deposit is made. */ - struct GNUNET_HashCode h_contract; + struct GNUNET_HashCode h_contract GNUNET_PACKED; /** * Hash over the wiring information of the merchant. */ - struct GNUNET_HashCode h_wire; + struct GNUNET_HashCode h_wire GNUNET_PACKED; /** * Merchant-generated transaction ID to detect duplicate @@ -395,7 +395,7 @@ struct TALER_RefreshMeltCoinAffirmationPS /** * Which melting session should the coin become a part of. */ - struct GNUNET_HashCode session_hash; + struct GNUNET_HashCode session_hash GNUNET_PACKED; /** * How much of the value of the coin should be melted? This amount diff --git a/src/mint-lib/mint_api_json.c b/src/mint-lib/mint_api_json.c index b15173940..46f54d948 100644 --- a/src/mint-lib/mint_api_json.c +++ b/src/mint-lib/mint_api_json.c @@ -245,6 +245,9 @@ parse_json (json_t *root, &sig.eddsa_signature, spec[i].details.eddsa_signature.pub_key)) { + GNUNET_log (GNUNET_ERROR_TYPE_ERROR, + "Failed to verify signature of purpose %u\n", + ntohl (purpose->purpose)); GNUNET_break_op (0); MAJ_parse_free (sig_spec); return i; diff --git a/src/mint-lib/test_mint_api.c b/src/mint-lib/test_mint_api.c index 340e9d17f..71e23f473 100644 --- a/src/mint-lib/test_mint_api.c +++ b/src/mint-lib/test_mint_api.c @@ -1973,31 +1973,33 @@ run (void *cls, .expected_response_code = MHD_HTTP_OK, .details.refresh_link.reveal_ref = "refresh-reveal-1" }, -#if TEST_REFRESH /* Test successfully spending coins from the refresh operation: first EUR:1 */ { .oc = OC_DEPOSIT, - .label = "refresh-deposit-refreshed-1", + .label = "refresh-deposit-refreshed-1a", .expected_response_code = MHD_HTTP_OK, .details.deposit.amount = "EUR:1", - .details.deposit.coin_ref = "refresh-reveal-1a", + .details.deposit.coin_ref = "refresh-reveal-1", .details.deposit.coin_idx = 0, .details.deposit.wire_details = "{ \"type\":\"TEST\", \"bank\":\"dest bank\", \"account\":42 }", .details.deposit.contract = "{ \"items\"={ \"name\":\"ice cream\", \"value\":3 } }", .details.deposit.transaction_id = 2 }, + /* Test successfully spending coins from the refresh operation: finally EUR:0.1 */ { .oc = OC_DEPOSIT, .label = "refresh-deposit-refreshed-1b", .expected_response_code = MHD_HTTP_OK, .details.deposit.amount = "EUR:0.1", - .details.deposit.coin_ref = "refresh-reveal-1b", + .details.deposit.coin_ref = "refresh-reveal-1", .details.deposit.coin_idx = 4, .details.deposit.wire_details = "{ \"type\":\"TEST\", \"bank\":\"dest bank\", \"account\":42 }", .details.deposit.contract = "{ \"items\"={ \"name\":\"ice cream\", \"value\":3 } }", .details.deposit.transaction_id = 2 }, +#if TEST_REFRESH + /* Test running a failing melt operation (same operation again must fail) */ { .oc = OC_REFRESH_MELT, .label = "refresh-melt-failing", diff --git a/src/mint/taler-mint-httpd_responses.c b/src/mint/taler-mint-httpd_responses.c index 7a56efb9f..418bc1751 100644 --- a/src/mint/taler-mint-httpd_responses.c +++ b/src/mint/taler-mint-httpd_responses.c @@ -416,6 +416,19 @@ compile_transaction_history (const struct TALER_MINTDB_TransactionList *tl) &deposit->deposit_fee); dr.merchant = deposit->merchant_pub; dr.coin_pub = deposit->coin.coin_pub; + + /* internal sanity check before we hand out a bogus sig... */ + if (GNUNET_OK != + GNUNET_CRYPTO_eddsa_verify (ntohl (dr.purpose.purpose), + &dr.purpose, + &deposit->csig.eddsa_signature, + &deposit->coin.coin_pub.eddsa_pub)) + { + GNUNET_break (0); + json_decref (history); + return NULL; + } + transaction = TALER_json_from_eddsa_sig (&dr.purpose, &deposit->csig.eddsa_signature); break; @@ -435,6 +448,19 @@ compile_transaction_history (const struct TALER_MINTDB_TransactionList *tl) TALER_amount_hton (&ms.melt_fee, &melt->melt_fee); ms.coin_pub = melt->coin.coin_pub; + + /* internal sanity check before we hand out a bogus sig... */ + if (GNUNET_OK != + GNUNET_CRYPTO_eddsa_verify (ntohl (ms.purpose.purpose), + &ms.purpose, + &melt->coin_sig.eddsa_signature, + &melt->coin.coin_pub.eddsa_pub)) + { + GNUNET_break (0); + json_decref (history); + return NULL; + } + transaction = TALER_json_from_eddsa_sig (&ms.purpose, &melt->coin_sig.eddsa_signature); } @@ -476,6 +502,8 @@ TMH_RESPONSE_reply_deposit_insufficient_funds (struct MHD_Connection *connection json_t *history; history = compile_transaction_history (tl); + if (NULL == history) + return TMH_RESPONSE_reply_internal_db_error (connection); return TMH_RESPONSE_reply_json_pack (connection, MHD_HTTP_FORBIDDEN, "{s:s, s:o}", @@ -710,6 +738,8 @@ TMH_RESPONSE_reply_refresh_melt_insufficient_funds (struct MHD_Connection *conne json_t *history; history = compile_transaction_history (tl); + if (NULL == history) + return TMH_RESPONSE_reply_internal_db_error (connection); return TMH_RESPONSE_reply_json_pack (connection, MHD_HTTP_FORBIDDEN, "{s:s, s:o, s:o, s:o, s:o, s:o}", From 08c947a01f9e2048f7668cabac58a5938dc477f5 Mon Sep 17 00:00:00 2001 From: Christian Grothoff Date: Mon, 17 Aug 2015 03:57:50 +0200 Subject: [PATCH 6/6] -notes on testing --- src/mint-lib/test_mint_api.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/mint-lib/test_mint_api.c b/src/mint-lib/test_mint_api.c index 71e23f473..69a50e036 100644 --- a/src/mint-lib/test_mint_api.c +++ b/src/mint-lib/test_mint_api.c @@ -1999,7 +1999,7 @@ run (void *cls, .details.deposit.transaction_id = 2 }, #if TEST_REFRESH - + /* Test running a failing melt operation (same operation again must fail) */ { .oc = OC_REFRESH_MELT, .label = "refresh-melt-failing", @@ -2007,6 +2007,9 @@ run (void *cls, .details.refresh_melt.melted_coins = melt_coins_1, .details.refresh_melt.fresh_amounts = melt_fresh_amounts_1 }, + // FIXME: also test with coin that was already melted + // (signature differs from coin that was deposited...) + /* *************** end of /refresh testing ************** */ #endif