From fc8a5d68f1f4cbddeddac8a5f0c8f59c2b68229a Mon Sep 17 00:00:00 2001 From: j-berman Date: Thu, 1 Sep 2022 16:25:28 -0700 Subject: [PATCH 1/3] multisig: fix #8537 seed restore (suggestions by @UkoeHB) - spend secret key is no longer the sum of multisig key shares; no need to check that is the case upon restore. - restoring a multisig wallet from multisig info means that the wallet must have already completed all setup rounds. Upon restore, set the number of rounds completed accordingly. --- src/multisig/multisig_account.cpp | 9 ++++++++- src/multisig/multisig_account.h | 9 +++++++++ src/multisig/multisig_account_kex_impl.cpp | 2 +- src/wallet/wallet2.cpp | 12 +++++------- src/wallet/wallet2.h | 3 ++- tests/unit_tests/multisig.cpp | 2 +- 6 files changed, 26 insertions(+), 11 deletions(-) diff --git a/src/multisig/multisig_account.cpp b/src/multisig/multisig_account.cpp index 9bdcf2dbc8..f3e78da187 100644 --- a/src/multisig/multisig_account.cpp +++ b/src/multisig/multisig_account.cpp @@ -127,7 +127,7 @@ namespace multisig bool multisig_account::multisig_is_ready() const { if (main_kex_rounds_done()) - return m_kex_rounds_complete >= multisig_kex_rounds_required(m_signers.size(), m_threshold) + 1; + return m_kex_rounds_complete >= multisig_setup_rounds_required(m_signers.size(), m_threshold); else return false; } @@ -200,4 +200,11 @@ namespace multisig return num_signers - threshold + 1; } //---------------------------------------------------------------------------------------------------------------------- + // EXTERNAL + //---------------------------------------------------------------------------------------------------------------------- + std::uint32_t multisig_setup_rounds_required(const std::uint32_t num_signers, const std::uint32_t threshold) + { + return multisig_kex_rounds_required(num_signers, threshold) + 1; + } + //---------------------------------------------------------------------------------------------------------------------- } //namespace multisig diff --git a/src/multisig/multisig_account.h b/src/multisig/multisig_account.h index 7b372bbff1..7beb594b4d 100644 --- a/src/multisig/multisig_account.h +++ b/src/multisig/multisig_account.h @@ -245,4 +245,13 @@ namespace multisig * return: number of kex rounds required */ std::uint32_t multisig_kex_rounds_required(const std::uint32_t num_signers, const std::uint32_t threshold); + + /** + * brief: multisig_setup_rounds_required - The number of setup rounds required to produce an M-of-N shared key. + * - A participant must complete all kex rounds and 1 initialization round. + * param: num_signers - number of participants in multisig (N) + * param: threshold - threshold of multisig (M) + * return: number of setup rounds required + */ + std::uint32_t multisig_setup_rounds_required(const std::uint32_t num_signers, const std::uint32_t threshold); } //namespace multisig diff --git a/src/multisig/multisig_account_kex_impl.cpp b/src/multisig/multisig_account_kex_impl.cpp index be9ed9cb21..443e846313 100644 --- a/src/multisig/multisig_account_kex_impl.cpp +++ b/src/multisig/multisig_account_kex_impl.cpp @@ -74,7 +74,7 @@ namespace multisig "Multisig threshold may not be larger than number of signers."); CHECK_AND_ASSERT_THROW_MES(threshold > 0, "Multisig threshold must be > 0."); CHECK_AND_ASSERT_THROW_MES(round > 0, "Multisig kex round must be > 0."); - CHECK_AND_ASSERT_THROW_MES(round <= multisig_kex_rounds_required(num_signers, threshold) + 1, + CHECK_AND_ASSERT_THROW_MES(round <= multisig_setup_rounds_required(num_signers, threshold), "Trying to process multisig kex for an invalid round."); } //---------------------------------------------------------------------------------------------------------------------- diff --git a/src/wallet/wallet2.cpp b/src/wallet/wallet2.cpp index 1957639494..6234427a6e 100644 --- a/src/wallet/wallet2.cpp +++ b/src/wallet/wallet2.cpp @@ -4684,7 +4684,8 @@ void wallet2::init_type(hw::device::device_type device_type) } /*! - * \brief Generates a wallet or restores one. + * \brief Generates a wallet or restores one. Assumes the multisig setup + * has already completed for the provided multisig info. * \param wallet_ Name of wallet file * \param password Password of wallet file * \param multisig_data The multisig restore info and keys @@ -4743,11 +4744,6 @@ void wallet2::generate(const std::string& wallet_, const epee::wipeable_string& crypto::public_key local_signer; THROW_WALLET_EXCEPTION_IF(!crypto::secret_key_to_public_key(spend_secret_key, local_signer), error::invalid_multisig_seed); THROW_WALLET_EXCEPTION_IF(std::find(multisig_signers.begin(), multisig_signers.end(), local_signer) == multisig_signers.end(), error::invalid_multisig_seed); - rct::key skey = rct::zero(); - for (const auto &msk: multisig_keys) - sc_add(skey.bytes, skey.bytes, rct::sk2rct(msk).bytes); - THROW_WALLET_EXCEPTION_IF(!(rct::rct2sk(skey) == spend_secret_key), error::invalid_multisig_seed); - memwipe(&skey, sizeof(rct::key)); m_account.make_multisig(view_secret_key, spend_secret_key, spend_public_key, multisig_keys); @@ -4758,6 +4754,8 @@ void wallet2::generate(const std::string& wallet_, const epee::wipeable_string& m_multisig = true; m_multisig_threshold = threshold; m_multisig_signers = multisig_signers; + // wallet is assumed already finalized + m_multisig_rounds_passed = multisig::multisig_setup_rounds_required(m_multisig_signers.size(), m_multisig_threshold); setup_keys(password); create_keys_file(wallet_, false, password, m_nettype != MAINNET || create_address_file); @@ -5208,7 +5206,7 @@ bool wallet2::multisig(bool *ready, uint32_t *threshold, uint32_t *total) const if (ready) { *ready = !(get_account().get_keys().m_account_address.m_spend_public_key == rct::rct2pk(rct::identity())) && - (m_multisig_rounds_passed == multisig::multisig_kex_rounds_required(m_multisig_signers.size(), m_multisig_threshold) + 1); + (m_multisig_rounds_passed == multisig::multisig_setup_rounds_required(m_multisig_signers.size(), m_multisig_threshold)); } return true; } diff --git a/src/wallet/wallet2.h b/src/wallet/wallet2.h index 16e898ad83..3fce616e3b 100644 --- a/src/wallet/wallet2.h +++ b/src/wallet/wallet2.h @@ -794,7 +794,8 @@ namespace tools }; /*! - * \brief Generates a wallet or restores one. + * \brief Generates a wallet or restores one. Assumes the multisig setup + * has already completed for the provided multisig info. * \param wallet_ Name of wallet file * \param password Password of wallet file * \param multisig_data The multisig restore info and keys diff --git a/tests/unit_tests/multisig.cpp b/tests/unit_tests/multisig.cpp index 5ddd78955d..8f8ad52e1c 100644 --- a/tests/unit_tests/multisig.cpp +++ b/tests/unit_tests/multisig.cpp @@ -171,7 +171,7 @@ static void make_wallets(std::vector& wallets, unsigned int M) { ASSERT_TRUE(wallets.size() > 1 && wallets.size() <= KEYS_COUNT); ASSERT_TRUE(M <= wallets.size()); - std::uint32_t total_rounds_required = multisig::multisig_kex_rounds_required(wallets.size(), M) + 1; + std::uint32_t total_rounds_required = multisig::multisig_setup_rounds_required(wallets.size(), M); std::uint32_t rounds_complete{0}; // initialize wallets, get first round multisig kex msgs From 401f5d967b283aa9d74da65cef1e275221542535 Mon Sep 17 00:00:00 2001 From: j-berman Date: Mon, 5 Sep 2022 18:39:21 -0600 Subject: [PATCH 2/3] Require user ack multisig is experimental to restore --- src/simplewallet/simplewallet.cpp | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/simplewallet/simplewallet.cpp b/src/simplewallet/simplewallet.cpp index a8f4e5a07b..3242a9f052 100644 --- a/src/simplewallet/simplewallet.cpp +++ b/src/simplewallet/simplewallet.cpp @@ -4134,6 +4134,17 @@ bool simple_wallet::init(const boost::program_options::variables_map& vm) if(!ask_wallet_create_if_needed()) return false; } + bool enable_multisig = false; + if (m_restore_multisig_wallet) { + fail_msg_writer() << tr("Multisig is disabled."); + fail_msg_writer() << tr("Multisig is an experimental feature and may have bugs. Things that could go wrong include: funds sent to a multisig wallet can't be spent at all, can only be spent with the participation of a malicious group member, or can be stolen by a malicious group member."); + if (!command_line::is_yes(input_line("Do you want to continue restoring a multisig wallet?", true))) { + message_writer() << tr("You have canceled restoring a multisig wallet."); + return false; + } + enable_multisig = true; + } + if (!m_generate_new.empty() || m_restoring) { if (!m_subaddress_lookahead.empty() && !parse_subaddress_lookahead(m_subaddress_lookahead)) @@ -4667,6 +4678,8 @@ bool simple_wallet::init(const boost::program_options::variables_map& vm) } m_wallet->set_refresh_from_block_height(m_restore_height); } + if (enable_multisig) + m_wallet->enable_multisig(true); m_wallet->rewrite(m_wallet_file, password); } else From 848a0c05b0c52c2c08fcd346d7f821b496f2d044 Mon Sep 17 00:00:00 2001 From: j-berman Date: Tue, 6 Sep 2022 23:21:25 -0600 Subject: [PATCH 3/3] Fix segfault restoring encrypted multisig seed --- src/simplewallet/simplewallet.cpp | 32 +++++++++++++++---------------- src/simplewallet/simplewallet.h | 2 +- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/src/simplewallet/simplewallet.cpp b/src/simplewallet/simplewallet.cpp index 3242a9f052..2fb538c730 100644 --- a/src/simplewallet/simplewallet.cpp +++ b/src/simplewallet/simplewallet.cpp @@ -4118,6 +4118,7 @@ bool simple_wallet::init(const boost::program_options::variables_map& vm) epee::wipeable_string multisig_keys; epee::wipeable_string password; + epee::wipeable_string seed_pass; if (!handle_command_line(vm)) return false; @@ -4224,19 +4225,9 @@ bool simple_wallet::init(const boost::program_options::variables_map& vm) auto pwd_container = password_prompter(tr("Enter seed offset passphrase, empty if none"), false); if (std::cin.eof() || !pwd_container) return false; - epee::wipeable_string seed_pass = pwd_container->password(); - if (!seed_pass.empty()) - { - if (m_restore_multisig_wallet) - { - crypto::secret_key key; - crypto::cn_slow_hash(seed_pass.data(), seed_pass.size(), (crypto::hash&)key); - sc_reduce32((unsigned char*)key.data); - multisig_keys = m_wallet->decrypt(std::string(multisig_keys.data(), multisig_keys.size()), key, true); - } - else - m_recovery_key = cryptonote::decrypt_key(m_recovery_key, seed_pass); - } + seed_pass = pwd_container->password(); + if (!seed_pass.empty() && !m_restore_multisig_wallet) + m_recovery_key = cryptonote::decrypt_key(m_recovery_key, seed_pass); } if (!m_generate_from_view_key.empty()) { @@ -4579,7 +4570,7 @@ bool simple_wallet::init(const boost::program_options::variables_map& vm) m_wallet_file = m_generate_new; boost::optional r; if (m_restore_multisig_wallet) - r = new_wallet(vm, multisig_keys, old_language); + r = new_wallet(vm, multisig_keys, seed_pass, old_language); else r = new_wallet(vm, m_recovery_key, m_restore_deterministic_wallet, m_non_deterministic, old_language); CHECK_AND_ASSERT_MES(r, false, tr("account creation failed")); @@ -5070,7 +5061,7 @@ boost::optional simple_wallet::new_wallet(const boost::pr } //---------------------------------------------------------------------------------------------------- boost::optional simple_wallet::new_wallet(const boost::program_options::variables_map& vm, - const epee::wipeable_string &multisig_keys, const std::string &old_language) + const epee::wipeable_string &multisig_keys, const epee::wipeable_string &seed_pass, const std::string &old_language) { std::pair, tools::password_container> rc; try { rc = tools::wallet2::make_new(vm, false, password_prompter); } @@ -5104,7 +5095,16 @@ boost::optional simple_wallet::new_wallet(const boost::pr try { - m_wallet->generate(m_wallet_file, std::move(rc.second).password(), multisig_keys, create_address_file); + if (seed_pass.empty()) + m_wallet->generate(m_wallet_file, std::move(rc.second).password(), multisig_keys, create_address_file); + else + { + crypto::secret_key key; + crypto::cn_slow_hash(seed_pass.data(), seed_pass.size(), (crypto::hash&)key); + sc_reduce32((unsigned char*)key.data); + const epee::wipeable_string &msig_keys = m_wallet->decrypt(std::string(multisig_keys.data(), multisig_keys.size()), key, true); + m_wallet->generate(m_wallet_file, std::move(rc.second).password(), msig_keys, create_address_file); + } bool ready; uint32_t threshold, total; if (!m_wallet->multisig(&ready, &threshold, &total) || !ready) diff --git a/src/simplewallet/simplewallet.h b/src/simplewallet/simplewallet.h index 6a9fa149d7..b18fa2dd64 100644 --- a/src/simplewallet/simplewallet.h +++ b/src/simplewallet/simplewallet.h @@ -101,7 +101,7 @@ namespace cryptonote boost::optional new_wallet(const boost::program_options::variables_map& vm, const cryptonote::account_public_address& address, const boost::optional& spendkey, const crypto::secret_key& viewkey); boost::optional new_wallet(const boost::program_options::variables_map& vm, - const epee::wipeable_string &multisig_keys, const std::string &old_language); + const epee::wipeable_string &multisig_keys, const epee::wipeable_string &seed_pass, const std::string &old_language); boost::optional new_wallet(const boost::program_options::variables_map& vm); boost::optional open_wallet(const boost::program_options::variables_map& vm); bool close_wallet();