diff --git a/src/cryptonote_config.h b/src/cryptonote_config.h index 962346017..2ec194ef8 100644 --- a/src/cryptonote_config.h +++ b/src/cryptonote_config.h @@ -246,6 +246,8 @@ namespace config const unsigned char HASH_KEY_CLSAG_AGG_1[] = "CLSAG_agg_1"; const char HASH_KEY_MESSAGE_SIGNING[] = "MoneroMessageSignature"; const unsigned char HASH_KEY_MM_SLOT = 'm'; + const constexpr char HASH_KEY_MULTISIG_TX_PRIVKEYS_SEED[] = "multisig_tx_privkeys_seed"; + const constexpr char HASH_KEY_MULTISIG_TX_PRIVKEYS[] = "multisig_tx_privkeys"; // Multisig const uint32_t MULTISIG_MAX_SIGNERS{16}; diff --git a/src/multisig/multisig_tx_builder_ringct.cpp b/src/multisig/multisig_tx_builder_ringct.cpp index cbc556b71..e5c9ac483 100644 --- a/src/multisig/multisig_tx_builder_ringct.cpp +++ b/src/multisig/multisig_tx_builder_ringct.cpp @@ -34,6 +34,7 @@ #include "cryptonote_basic/cryptonote_basic.h" #include "cryptonote_basic/account.h" #include "cryptonote_basic/cryptonote_format_utils.h" +#include "cryptonote_config.h" #include "cryptonote_core/cryptonote_tx_utils.h" #include "device/device.hpp" #include "multisig_clsag_context.h" @@ -47,6 +48,7 @@ #include #include #include +#include #include #include #include @@ -242,6 +244,80 @@ static bool set_tx_extra( } //---------------------------------------------------------------------------------------------------------------------- //---------------------------------------------------------------------------------------------------------------------- +static void make_tx_secret_key_seed(const crypto::secret_key& tx_secret_key_entropy, + const std::vector& sources, + crypto::secret_key& tx_secret_key_seed) +{ + // seed = H(H("domain separator"), entropy, {KI}) + static const std::string domain_separator{config::HASH_KEY_MULTISIG_TX_PRIVKEYS_SEED}; + + rct::keyV hash_context; + hash_context.reserve(2 + sources.size()); + auto hash_context_wiper = epee::misc_utils::create_scope_leave_handler([&]{ + memwipe(hash_context.data(), hash_context.size()); + }); + hash_context.emplace_back(); + rct::cn_fast_hash(hash_context.back(), domain_separator.data(), domain_separator.size()); //domain sep + hash_context.emplace_back(rct::sk2rct(tx_secret_key_entropy)); //entropy + + for (const cryptonote::tx_source_entry& source : sources) + hash_context.emplace_back(source.multisig_kLRki.ki); //{KI} + + // set the seed + tx_secret_key_seed = rct::rct2sk(rct::cn_fast_hash(hash_context)); +} +//---------------------------------------------------------------------------------------------------------------------- +//---------------------------------------------------------------------------------------------------------------------- +static void make_tx_secret_keys(const crypto::secret_key& tx_secret_key_seed, + const std::size_t num_tx_keys, + std::vector& tx_secret_keys) +{ + // make tx secret keys as a hash chain of the seed + // h1 = H_n(seed || H("domain separator")) + // h2 = H_n(seed || h1) + // h3 = H_n(seed || h2) + // ... + static const std::string domain_separator{config::HASH_KEY_MULTISIG_TX_PRIVKEYS}; + + rct::keyV hash_context; + hash_context.resize(2); + auto hash_context_wiper = epee::misc_utils::create_scope_leave_handler([&]{ + memwipe(hash_context.data(), hash_context.size()); + }); + hash_context[0] = rct::sk2rct(tx_secret_key_seed); + rct::cn_fast_hash(hash_context[1], domain_separator.data(), domain_separator.size()); + + tx_secret_keys.clear(); + tx_secret_keys.resize(num_tx_keys); + + for (crypto::secret_key& tx_secret_key : tx_secret_keys) + { + // advance the hash chain + hash_context[1] = rct::hash_to_scalar(hash_context); + + // set this key + tx_secret_key = rct::rct2sk(hash_context[1]); + } +} +//---------------------------------------------------------------------------------------------------------------------- +//---------------------------------------------------------------------------------------------------------------------- +static bool collect_tx_secret_keys(const std::vector& tx_secret_keys, + crypto::secret_key& tx_secret_key, + std::vector& tx_aux_secret_keys) +{ + if (tx_secret_keys.size() == 0) + return false; + + tx_secret_key = tx_secret_keys[0]; + tx_aux_secret_keys.clear(); + tx_aux_secret_keys.reserve(tx_secret_keys.size() - 1); + for (std::size_t tx_key_index{1}; tx_key_index < tx_secret_keys.size(); ++tx_key_index) + tx_aux_secret_keys.emplace_back(tx_secret_keys[tx_key_index]); + + return true; +} +//---------------------------------------------------------------------------------------------------------------------- +//---------------------------------------------------------------------------------------------------------------------- static bool compute_keys_for_destinations( const cryptonote::account_keys& account_keys, const std::uint32_t subaddr_account, @@ -250,6 +326,7 @@ static bool compute_keys_for_destinations( const std::vector& extra, const bool use_view_tags, const bool reconstruction, + const crypto::secret_key& tx_secret_key_seed, crypto::secret_key& tx_secret_key, std::vector& tx_aux_secret_keys, rct::keyV& output_public_keys, @@ -288,8 +365,35 @@ static bool compute_keys_for_destinations( unique_std_recipients.insert(dst_entr.addr); } - if (not reconstruction) { - tx_secret_key = rct::rct2sk(rct::skGen()); + // figure out how many tx secret keys are needed + // - tx aux keys: add if there are > 1 non-change recipients, with at least one to a subaddress + const std::size_t num_destinations = destinations.size(); + const bool need_tx_aux_keys = unique_subbaddr_recipients.size() + bool(unique_std_recipients.size()) > 1; + + const std::size_t num_tx_keys = 1 + (need_tx_aux_keys ? num_destinations : 0); + + // make tx secret keys + std::vector all_tx_secret_keys; + make_tx_secret_keys(tx_secret_key_seed, num_tx_keys, all_tx_secret_keys); + + // split up tx secret keys + crypto::secret_key tx_secret_key_temp; + std::vector tx_aux_secret_keys_temp; + if (not collect_tx_secret_keys(all_tx_secret_keys, tx_secret_key_temp, tx_aux_secret_keys_temp)) + return false; + + if (reconstruction) + { + // when reconstructing, the tx secret keys should be reproducible from input seed + if (!(tx_secret_key == tx_secret_key_temp)) + return false; + if (!(tx_aux_secret_keys == tx_aux_secret_keys_temp)) + return false; + } + else + { + tx_secret_key = tx_secret_key_temp; + tx_aux_secret_keys = std::move(tx_aux_secret_keys_temp); } // tx pub key: R @@ -312,17 +416,6 @@ static bool compute_keys_for_destinations( } // additional tx pubkeys: R_t - // - add if there are > 1 non-change recipients, with at least one to a subaddress - const std::size_t num_destinations = destinations.size(); - - const bool need_tx_aux_keys = unique_subbaddr_recipients.size() + bool(unique_std_recipients.size()) > 1; - if (not reconstruction and need_tx_aux_keys) { - tx_aux_secret_keys.clear(); - tx_aux_secret_keys.reserve(num_destinations); - for(std::size_t i = 0; i < num_destinations; ++i) - tx_aux_secret_keys.push_back(rct::rct2sk(rct::skGen())); - } - output_public_keys.resize(num_destinations); view_tags.resize(num_destinations); std::vector tx_aux_public_keys; @@ -738,6 +831,7 @@ bool tx_builder_ringct_t::init( const bool reconstruction, crypto::secret_key& tx_secret_key, std::vector& tx_aux_secret_keys, + crypto::secret_key& tx_secret_key_entropy, cryptonote::transaction& unsigned_tx ) { @@ -765,6 +859,23 @@ bool tx_builder_ringct_t::init( // sort inputs sort_sources(sources); + // prepare tx secret key seed (must be AFTER sorting sources) + // - deriving the seed from sources plus entropy ensures uniqueness for every new tx attempt + // - the goal is that two multisig txs added to the chain will never have outputs with the same onetime addresses, + // which would burn funds (embedding the inputs' key images guarantees this) + // - it is acceptable if two tx attempts use the same input set and entropy (only a malicious tx proposer will do + // that, but all it can accomplish is leaking information about the recipients - which a malicious proposer can + // easily do outside the signing ritual anyway) + if (not reconstruction) + tx_secret_key_entropy = rct::rct2sk(rct::skGen()); + + // expect not null (note: wallet serialization code may set this to null if handling an old partial tx) + if (tx_secret_key_entropy == crypto::null_skey) + return false; + + crypto::secret_key tx_secret_key_seed; + make_tx_secret_key_seed(tx_secret_key_entropy, sources, tx_secret_key_seed); + // get secret keys for signing input CLSAGs (multisig: or for the initial partial signature) rct::keyV input_secret_keys; auto input_secret_keys_wiper = epee::misc_utils::create_scope_leave_handler([&]{ @@ -791,6 +902,7 @@ bool tx_builder_ringct_t::init( extra, use_view_tags, reconstruction, + tx_secret_key_seed, tx_secret_key, tx_aux_secret_keys, output_public_keys, @@ -921,6 +1033,7 @@ bool tx_builder_ringct_t::finalize_tx( cryptonote::transaction& unsigned_tx ) { + // checks const std::size_t num_sources = sources.size(); if (num_sources != unsigned_tx.rct_signatures.p.CLSAGs.size()) return false; @@ -928,6 +1041,8 @@ bool tx_builder_ringct_t::finalize_tx( return false; if (num_sources != s.size()) return false; + + // finalize tx signatures for (std::size_t i = 0; i < num_sources; ++i) { const std::size_t ring_size = unsigned_tx.rct_signatures.p.CLSAGs[i].s.size(); if (sources[i].real_output >= ring_size) @@ -935,6 +1050,7 @@ bool tx_builder_ringct_t::finalize_tx( unsigned_tx.rct_signatures.p.CLSAGs[i].s[sources[i].real_output] = s[i]; unsigned_tx.rct_signatures.p.CLSAGs[i].c1 = c_0[i]; } + return true; } //---------------------------------------------------------------------------------------------------------------------- diff --git a/src/multisig/multisig_tx_builder_ringct.h b/src/multisig/multisig_tx_builder_ringct.h index 67ef9e065..853934659 100644 --- a/src/multisig/multisig_tx_builder_ringct.h +++ b/src/multisig/multisig_tx_builder_ringct.h @@ -82,6 +82,7 @@ public: const bool reconstruction, crypto::secret_key& tx_secret_key, std::vector& tx_aux_secret_keys, + crypto::secret_key& tx_secret_key_entropy, cryptonote::transaction& unsigned_tx ); diff --git a/src/wallet/wallet2.cpp b/src/wallet/wallet2.cpp index ed153d681..ca6670143 100644 --- a/src/wallet/wallet2.cpp +++ b/src/wallet/wallet2.cpp @@ -7157,6 +7157,7 @@ bool wallet2::sign_multisig_tx(multisig_tx_set &exported_txs, std::vector additional_tx_keys; + crypto::secret_key multisig_tx_key_entropy; LOG_PRINT_L2("constructing tx"); auto sources_copy = sources; multisig::signing::tx_builder_ringct_t multisig_tx_builder; @@ -9029,6 +9031,7 @@ void wallet2::transfer_selected_rct(std::vector additional_tx_keys; std::vector dests; std::vector multisig_sigs; + crypto::secret_key multisig_tx_key_entropy; tx_construction_data construction_data; BEGIN_SERIALIZE_OBJECT() + VERSION_FIELD(1) FIELD(tx) FIELD(dust) FIELD(fee) @@ -648,6 +650,12 @@ private: FIELD(dests) FIELD(construction_data) FIELD(multisig_sigs) + if (version < 1) + { + multisig_tx_key_entropy = crypto::null_skey; + return true; + } + FIELD(multisig_tx_key_entropy) END_SERIALIZE() }; diff --git a/tests/core_tests/multisig.cpp b/tests/core_tests/multisig.cpp index 28d176e56..28b44d293 100644 --- a/tests/core_tests/multisig.cpp +++ b/tests/core_tests/multisig.cpp @@ -307,9 +307,10 @@ bool gen_multisig_tx_validation_base::generate_with(std::vector additional_tx_secret_keys; + crypto::secret_key multisig_tx_key_entropy; auto sources_copy = sources; multisig::signing::tx_builder_ringct_t tx_builder; - CHECK_AND_ASSERT_MES(tx_builder.init(miner_account[creator].get_keys(), {}, 0, 0, {0}, sources, destinations, {}, {rct::RangeProofPaddedBulletproof, 4}, true, false, tx_key, additional_tx_secret_keys, tx), false, "error: multisig::signing::tx_builder_t::init"); + CHECK_AND_ASSERT_MES(tx_builder.init(miner_account[creator].get_keys(), {}, 0, 0, {0}, sources, destinations, {}, {rct::RangeProofPaddedBulletproof, 4}, true, false, tx_key, additional_tx_secret_keys, multisig_tx_key_entropy, tx), false, "error: multisig::signing::tx_builder_ringct_t::init"); // work out the permutation done on sources std::vector ins_order; @@ -398,7 +399,7 @@ bool gen_multisig_tx_validation_base::generate_with(std::vector