From ca66a1d1af2412e3ad04c18150db7a259dc69b5e Mon Sep 17 00:00:00 2001 From: Christian Grothoff Date: Sun, 10 Jan 2021 00:54:12 +0100 Subject: fix major bug with SQL statement missing ORDER BY freshcoin_index resulting in possible link failures (but usually lucky with old DB schema) --- src/exchange/taler-exchange-httpd_link.c | 4 +- .../taler-exchange-httpd_refreshes_reveal.c | 42 +++++------- src/exchangedb/exchange-0002.sql | 1 - src/exchangedb/plugin_exchangedb_postgres.c | 9 ++- src/include/taler_crypto_lib.h | 41 +++++++++++ src/include/taler_exchangedb_plugin.h | 1 + src/lib/exchange_api_link.c | 41 +++++------ src/lib/exchange_api_refreshes_reveal.c | 30 +++----- src/util/Makefile.am | 1 + src/util/wallet_signatures.c | 80 ++++++++++++++++++++++ 10 files changed, 175 insertions(+), 75 deletions(-) create mode 100644 src/util/wallet_signatures.c diff --git a/src/exchange/taler-exchange-httpd_link.c b/src/exchange/taler-exchange-httpd_link.c index 3edb25b29..b93d2e718 100644 --- a/src/exchange/taler-exchange-httpd_link.c +++ b/src/exchange/taler-exchange-httpd_link.c @@ -83,8 +83,8 @@ handle_link_data (void *cls, obj = json_pack ("{s:o, s:o, s:o}", "denom_pub", - GNUNET_JSON_from_rsa_public_key - (pos->denom_pub.rsa_public_key), + GNUNET_JSON_from_rsa_public_key ( + pos->denom_pub.rsa_public_key), "ev_sig", GNUNET_JSON_from_rsa_signature (pos->ev_sig.rsa_signature), diff --git a/src/exchange/taler-exchange-httpd_refreshes_reveal.c b/src/exchange/taler-exchange-httpd_refreshes_reveal.c index 930de0820..d7ec02c88 100644 --- a/src/exchange/taler-exchange-httpd_refreshes_reveal.c +++ b/src/exchange/taler-exchange-httpd_refreshes_reveal.c @@ -349,7 +349,7 @@ refreshes_reveal_transaction (void *cls, else { /* Reconstruct coin envelopes from transfer private key */ - struct TALER_TransferPrivateKeyP *tpriv + const struct TALER_TransferPrivateKeyP *tpriv = &rctx->transfer_privs[i - off]; struct TALER_TransferSecretP ts; @@ -695,32 +695,22 @@ resolve_refreshes_reveal_denominations (struct MHD_Connection *connection, if (GNUNET_OK != res) return (GNUNET_NO == res) ? MHD_YES : MHD_NO; /* Check link_sigs[i] signature */ + if (GNUNET_OK != + TALER_wallet_link_verify ( + &dk_h[i], + &rctx->gamma_tp, + rcds[i].coin_ev, + rcds[i].coin_ev_size, + &melt.session.coin.coin_pub, + &link_sigs[i])) { - struct TALER_LinkDataPS ldp = { - .purpose.size = htonl (sizeof (ldp)), - .purpose.purpose = htonl (TALER_SIGNATURE_WALLET_COIN_LINK), - .h_denom_pub = dk_h[i], - .old_coin_pub = melt.session.coin.coin_pub, - .transfer_pub = rctx->gamma_tp - }; - - GNUNET_CRYPTO_hash (rcds[i].coin_ev, - rcds[i].coin_ev_size, - &ldp.coin_envelope_hash); - if (GNUNET_OK != - GNUNET_CRYPTO_eddsa_verify ( - TALER_SIGNATURE_WALLET_COIN_LINK, - &ldp, - &link_sigs[i].eddsa_signature, - &melt.session.coin.coin_pub.eddsa_pub)) - { - GNUNET_break_op (0); - ret = TALER_MHD_reply_with_error (connection, - MHD_HTTP_FORBIDDEN, - TALER_EC_EXCHANGE_REFRESHES_REVEAL_LINK_SIGNATURE_INVALID, - NULL); - goto cleanup; - } + GNUNET_break_op (0); + ret = TALER_MHD_reply_with_error ( + connection, + MHD_HTTP_FORBIDDEN, + TALER_EC_EXCHANGE_REFRESHES_REVEAL_LINK_SIGNATURE_INVALID, + NULL); + goto cleanup; } } diff --git a/src/exchangedb/exchange-0002.sql b/src/exchangedb/exchange-0002.sql index 3d17395bc..9b66d4d18 100644 --- a/src/exchangedb/exchange-0002.sql +++ b/src/exchangedb/exchange-0002.sql @@ -280,7 +280,6 @@ COMMENT ON COLUMN refunds.deposit_serial_id IS 'Identifies ONLY the merchant_pub, h_contract_terms and known_coin_id. Multiple deposits may match a refund, this only identifies one of them.'; - -- Create additional tables... CREATE TABLE IF NOT EXISTS auditors diff --git a/src/exchangedb/plugin_exchangedb_postgres.c b/src/exchangedb/plugin_exchangedb_postgres.c index d8dbd2241..b81fb7c1b 100644 --- a/src/exchangedb/plugin_exchangedb_postgres.c +++ b/src/exchangedb/plugin_exchangedb_postgres.c @@ -794,7 +794,7 @@ postgres_get_session (void *cls) ",h_coin_ev" ",ev_sig" ") SELECT rcx.melt_serial_id, $2, $3, " - " denominations_serial, $5, $6, $7 " + " denominations_serial, $5, $6, $7" " FROM denominations" " CROSS JOIN rcx" " WHERE denom_pub_hash=$4;", @@ -1108,7 +1108,7 @@ postgres_get_session (void *cls) " (SELECT known_coin_id " " FROM known_coins" " WHERE coin_pub=$1)" - " ORDER BY tp.transfer_pub", + " ORDER BY tp.transfer_pub, rrc.freshcoin_index ASC", 1), /* Used in #postgres_lookup_wire_transfer */ GNUNET_PQ_make_prepare ("lookup_transactions", @@ -2323,6 +2323,11 @@ postgres_preflight (void *cls, }; (void) cls; + if (NULL == session) + { + GNUNET_break (0); + return; + } if (NULL == session->transaction_name) return; /* all good */ if (GNUNET_OK == diff --git a/src/include/taler_crypto_lib.h b/src/include/taler_crypto_lib.h index 70e49697c..4ea845b49 100644 --- a/src/include/taler_crypto_lib.h +++ b/src/include/taler_crypto_lib.h @@ -1050,6 +1050,47 @@ TALER_CRYPTO_helper_esign_disconnect ( struct TALER_CRYPTO_ExchangeSignHelper *esh); +/* ********************* wallet signing ************************** */ + +/** + * Sign link data. + * + * @param h_denom_pub hash of the denomiantion public key of the new coin + * @param transfer_pub transfer public key + * @param coin_ev coin envelope + * @param coin_ev_size number of bytes in @a coin_ev + * @param old_coin_priv private key to sign with + * @param[out] coin_sig resulting signature + */ +void +TALER_wallet_link_sign (const struct GNUNET_HashCode *h_denom_pub, + const struct TALER_TransferPublicKeyP *transfer_pub, + const void *coin_ev, + size_t coin_ev_size, + const struct TALER_CoinSpendPrivateKeyP *old_coin_priv, + struct TALER_CoinSpendSignatureP *coin_sig); + + +/** + * Verify link signature. + * + * @param h_denom_pub hash of the denomiantion public key of the new coin + * @param transfer_pub transfer public key + * @param coin_ev coin envelope + * @param coin_ev_size number of bytes in @a coin_ev + * @param old_coin_priv private key to sign with + * @param coin_sig resulting signature + * @return #GNUNET_OK if the signature is valid + */ +enum GNUNET_GenericReturnValue +TALER_wallet_link_verify ( + const struct GNUNET_HashCode *h_denom_pub, + const struct TALER_TransferPublicKeyP *transfer_pub, + const void *coin_ev, + size_t coin_ev_size, + const struct TALER_CoinSpendPublicKeyP *old_coin_pub, + const struct TALER_CoinSpendSignatureP *coin_sig); + /* ********************* offline signing ************************** */ diff --git a/src/include/taler_exchangedb_plugin.h b/src/include/taler_exchangedb_plugin.h index 83d283401..da320d39c 100644 --- a/src/include/taler_exchangedb_plugin.h +++ b/src/include/taler_exchangedb_plugin.h @@ -1274,6 +1274,7 @@ struct TALER_EXCHANGEDB_LinkList * link data, of type #TALER_SIGNATURE_WALLET_COIN_LINK */ struct TALER_CoinSpendSignatureP orig_coin_link_sig; + }; diff --git a/src/lib/exchange_api_link.c b/src/lib/exchange_api_link.c index 23a79d80c..5deecadaf 100644 --- a/src/lib/exchange_api_link.c +++ b/src/lib/exchange_api_link.c @@ -75,7 +75,7 @@ struct TALER_EXCHANGE_LinkHandle * * @param lh link handle * @param json json reply with the data for one coin - * @param coin_num number of the coin to decode + * @param coin_num number of the coin * @param trans_pub our transfer public key * @param[out] coin_priv where to return private coin key * @param[out] sig where to return private coin signature @@ -85,7 +85,7 @@ struct TALER_EXCHANGE_LinkHandle static int parse_link_coin (const struct TALER_EXCHANGE_LinkHandle *lh, const json_t *json, - unsigned int coin_num, + uint32_t coin_num, const struct TALER_TransferPublicKeyP *trans_pub, struct TALER_CoinSpendPrivateKeyP *coin_priv, struct TALER_DenominationSignature *sig, @@ -112,7 +112,6 @@ parse_link_coin (const struct TALER_EXCHANGE_LinkHandle *lh, GNUNET_break_op (0); return GNUNET_SYSERR; } - TALER_link_recover_transfer_secret (trans_pub, &lh->coin_priv, &secret); @@ -130,14 +129,10 @@ parse_link_coin (const struct TALER_EXCHANGE_LinkHandle *lh, { struct TALER_PlanchetDetail pd; struct GNUNET_HashCode c_hash; - struct TALER_LinkDataPS ldp = { - .purpose.size = htonl (sizeof (ldp)), - .purpose.purpose = htonl (TALER_SIGNATURE_WALLET_COIN_LINK), - .transfer_pub = *trans_pub - }; + struct TALER_CoinSpendPublicKeyP old_coin_pub; GNUNET_CRYPTO_eddsa_key_get_public (&lh->coin_priv.eddsa_priv, - &ldp.old_coin_pub.eddsa_pub); + &old_coin_pub.eddsa_pub); pub->rsa_public_key = rpub; if (GNUNET_OK != TALER_planchet_prepare (pub, @@ -149,22 +144,20 @@ parse_link_coin (const struct TALER_EXCHANGE_LinkHandle *lh, GNUNET_JSON_parse_free (spec); return GNUNET_SYSERR; } - ldp.h_denom_pub = pd.denom_pub_hash; - GNUNET_CRYPTO_hash (pd.coin_ev, - pd.coin_ev_size, - &ldp.coin_envelope_hash); - GNUNET_free (pd.coin_ev); - if (GNUNET_OK != - GNUNET_CRYPTO_eddsa_verify (TALER_SIGNATURE_WALLET_COIN_LINK, - &ldp, - &link_sig.eddsa_signature, - &ldp.old_coin_pub.eddsa_pub)) + TALER_wallet_link_verify (&pd.denom_pub_hash, + trans_pub, + pd.coin_ev, + pd.coin_ev_size, + &old_coin_pub, + &link_sig)) { GNUNET_break_op (0); + GNUNET_free (pd.coin_ev); GNUNET_JSON_parse_free (spec); return GNUNET_SYSERR; } + GNUNET_free (pd.coin_ev); } /* clean up */ @@ -457,11 +450,11 @@ TALER_EXCHANGE_link (struct TALER_EXCHANGE_Handle *exchange, char pub_str[sizeof (struct TALER_CoinSpendPublicKeyP) * 2]; char *end; - end = GNUNET_STRINGS_data_to_string (&coin_pub, - sizeof (struct - TALER_CoinSpendPublicKeyP), - pub_str, - sizeof (pub_str)); + end = GNUNET_STRINGS_data_to_string ( + &coin_pub, + sizeof (struct TALER_CoinSpendPublicKeyP), + pub_str, + sizeof (pub_str)); *end = '\0'; GNUNET_snprintf (arg_str, sizeof (arg_str), diff --git a/src/lib/exchange_api_refreshes_reveal.c b/src/lib/exchange_api_refreshes_reveal.c index c51e30b01..e551b482e 100644 --- a/src/lib/exchange_api_refreshes_reveal.c +++ b/src/lib/exchange_api_refreshes_reveal.c @@ -391,30 +391,20 @@ TALER_EXCHANGE_refreshes_reveal ( json_array_append_new (coin_evs, GNUNET_JSON_from_data (pd.coin_ev, pd.coin_ev_size))); - - /* compute link signature */ { struct TALER_CoinSpendSignatureP link_sig; - struct TALER_LinkDataPS ldp; - - ldp.purpose.size = htonl (sizeof (ldp)); - ldp.purpose.purpose = htonl (TALER_SIGNATURE_WALLET_COIN_LINK); - ldp.h_denom_pub = denom_hash; - GNUNET_CRYPTO_eddsa_key_get_public (&md->melted_coin.coin_priv.eddsa_priv, - &ldp.old_coin_pub.eddsa_pub); - ldp.transfer_pub = transfer_pub; - GNUNET_CRYPTO_hash (pd.coin_ev, - pd.coin_ev_size, - &ldp.coin_envelope_hash); - GNUNET_CRYPTO_eddsa_sign (&md->melted_coin.coin_priv.eddsa_priv, - &ldp, - &link_sig.eddsa_signature); + + TALER_wallet_link_sign (&denom_hash, + &transfer_pub, + pd.coin_ev, + pd.coin_ev_size, + &md->melted_coin.coin_priv, + &link_sig); GNUNET_assert (0 == - json_array_append_new (link_sigs, - GNUNET_JSON_from_data_auto ( - &link_sig))); + json_array_append_new ( + link_sigs, + GNUNET_JSON_from_data_auto (&link_sig))); } - GNUNET_free (pd.coin_ev); } diff --git a/src/util/Makefile.am b/src/util/Makefile.am index fcaab8279..c4df708b1 100644 --- a/src/util/Makefile.am +++ b/src/util/Makefile.am @@ -76,6 +76,7 @@ libtalerutil_la_SOURCES = \ taler_error_codes.c \ url.c \ util.c \ + wallet_signatures.c \ yna.c \ os_installation.c diff --git a/src/util/wallet_signatures.c b/src/util/wallet_signatures.c new file mode 100644 index 000000000..ef343d179 --- /dev/null +++ b/src/util/wallet_signatures.c @@ -0,0 +1,80 @@ +/* + This file is part of TALER + Copyright (C) 2020 Taler Systems SA + + TALER is free software; you can redistribute it and/or modify it under the + terms of the GNU General Public License as published by the Free Software + Foundation; either version 3, or (at your option) any later version. + + TALER is distributed in the hope that it will be useful, but WITHOUT ANY + WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR + A PARTICULAR PURPOSE. See the GNU General Public License for more details. + + You should have received a copy of the GNU General Public License along with + TALER; see the file COPYING. If not, see +*/ +/** + * @file secmod_signatures.c + * @brief Utility functions for Taler security module signatures + * @author Christian Grothoff + */ +#include "platform.h" +#include "taler_util.h" +#include "taler_signatures.h" + + +void +TALER_wallet_link_sign (const struct GNUNET_HashCode *h_denom_pub, + const struct TALER_TransferPublicKeyP *transfer_pub, + const void *coin_ev, + size_t coin_ev_size, + const struct TALER_CoinSpendPrivateKeyP *old_coin_priv, + struct TALER_CoinSpendSignatureP *coin_sig) +{ + struct TALER_LinkDataPS ldp = { + .purpose.size = htonl (sizeof (ldp)), + .purpose.purpose = htonl (TALER_SIGNATURE_WALLET_COIN_LINK), + .h_denom_pub = *h_denom_pub, + .transfer_pub = *transfer_pub + }; + + GNUNET_CRYPTO_hash (coin_ev, + coin_ev_size, + &ldp.coin_envelope_hash); + GNUNET_CRYPTO_eddsa_key_get_public (&old_coin_priv->eddsa_priv, + &ldp.old_coin_pub.eddsa_pub); + GNUNET_CRYPTO_eddsa_sign (&old_coin_priv->eddsa_priv, + &ldp, + &coin_sig->eddsa_signature); +} + + +enum GNUNET_GenericReturnValue +TALER_wallet_link_verify ( + const struct GNUNET_HashCode *h_denom_pub, + const struct TALER_TransferPublicKeyP *transfer_pub, + const void *coin_ev, + size_t coin_ev_size, + const struct TALER_CoinSpendPublicKeyP *old_coin_pub, + const struct TALER_CoinSpendSignatureP *coin_sig) +{ + struct TALER_LinkDataPS ldp = { + .purpose.size = htonl (sizeof (ldp)), + .purpose.purpose = htonl (TALER_SIGNATURE_WALLET_COIN_LINK), + .h_denom_pub = *h_denom_pub, + .old_coin_pub = *old_coin_pub, + .transfer_pub = *transfer_pub + }; + + GNUNET_CRYPTO_hash (coin_ev, + coin_ev_size, + &ldp.coin_envelope_hash); + return + GNUNET_CRYPTO_eddsa_verify (TALER_SIGNATURE_WALLET_COIN_LINK, + &ldp, + &coin_sig->eddsa_signature, + &old_coin_pub->eddsa_pub); +} + + +/* end of wallet_signatures.c */ -- cgit v1.2.3