From 29794742211b7fbd35b1c7b0b707cb495001b3e8 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Sat, 14 May 2022 19:49:48 +0000 Subject: [PATCH] disable multisig by default There are vulnerabilities in multisig protocol if the parties do not trust each other, and while there is a patch for it, it has not been throroughly reviewed yet, so it is felt safer to disable multisig by default for now. If all parties in a multisig setup trust each other, then it is safe to enable multisig. --- src/simplewallet/simplewallet.cpp | 55 ++++++++++++++++++++ src/simplewallet/simplewallet.h | 1 + src/wallet/wallet2.cpp | 9 +++- src/wallet/wallet2.h | 3 ++ src/wallet/wallet_rpc_server.cpp | 33 ++++++++++++ src/wallet/wallet_rpc_server_commands_defs.h | 3 ++ src/wallet/wallet_rpc_server_error_codes.h | 1 + tests/functional_tests/multisig.py | 10 ++-- utils/python-rpc/framework/wallet.py | 3 +- 9 files changed, 111 insertions(+), 7 deletions(-) diff --git a/src/simplewallet/simplewallet.cpp b/src/simplewallet/simplewallet.cpp index d3e40ab74..05d10b734 100644 --- a/src/simplewallet/simplewallet.cpp +++ b/src/simplewallet/simplewallet.cpp @@ -142,6 +142,19 @@ typedef cryptonote::simple_wallet sw; #define MIN_PAYMENT_RATE 0.01f // per hash #define MAX_MNEW_ADDRESSES 1000 +#define CHECK_MULTISIG_ENABLED() \ + do \ + { \ + if (!m_wallet->is_multisig_enabled()) \ + { \ + 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."); \ + fail_msg_writer() << tr("You can enable it with:"); \ + fail_msg_writer() << tr(" set enable-multisig-experimental 1"); \ + return false; \ + } \ + } while(0) + enum TransferType { Transfer, TransferLocked, @@ -986,12 +999,14 @@ bool simple_wallet::print_fee_info(const std::vector &args/* = std: bool simple_wallet::prepare_multisig(const std::vector &args) { + CHECK_MULTISIG_ENABLED(); prepare_multisig_main(args, false); return true; } bool simple_wallet::prepare_multisig_main(const std::vector &args, bool called_by_mms) { + CHECK_MULTISIG_ENABLED(); if (m_wallet->key_on_device()) { fail_msg_writer() << tr("command not supported by HW wallet"); @@ -1031,12 +1046,14 @@ bool simple_wallet::prepare_multisig_main(const std::vector &args, bool simple_wallet::make_multisig(const std::vector &args) { + CHECK_MULTISIG_ENABLED(); make_multisig_main(args, false); return true; } bool simple_wallet::make_multisig_main(const std::vector &args, bool called_by_mms) { + CHECK_MULTISIG_ENABLED(); if (m_wallet->key_on_device()) { fail_msg_writer() << tr("command not supported by HW wallet"); @@ -1121,11 +1138,13 @@ bool simple_wallet::make_multisig_main(const std::vector &args, boo bool simple_wallet::exchange_multisig_keys(const std::vector &args) { + CHECK_MULTISIG_ENABLED(); exchange_multisig_keys_main(args, false); return true; } bool simple_wallet::exchange_multisig_keys_main(const std::vector &args, bool called_by_mms) { + CHECK_MULTISIG_ENABLED(); bool ready; if (m_wallet->key_on_device()) { @@ -1189,12 +1208,14 @@ bool simple_wallet::exchange_multisig_keys_main(const std::vector & bool simple_wallet::export_multisig(const std::vector &args) { + CHECK_MULTISIG_ENABLED(); export_multisig_main(args, false); return true; } bool simple_wallet::export_multisig_main(const std::vector &args, bool called_by_mms) { + CHECK_MULTISIG_ENABLED(); bool ready; if (m_wallet->key_on_device()) { @@ -1254,12 +1275,14 @@ bool simple_wallet::export_multisig_main(const std::vector &args, b bool simple_wallet::import_multisig(const std::vector &args) { + CHECK_MULTISIG_ENABLED(); import_multisig_main(args, false); return true; } bool simple_wallet::import_multisig_main(const std::vector &args, bool called_by_mms) { + CHECK_MULTISIG_ENABLED(); bool ready; uint32_t threshold, total; if (m_wallet->key_on_device()) @@ -1349,12 +1372,14 @@ bool simple_wallet::accept_loaded_tx(const tools::wallet2::multisig_tx_set &txs) bool simple_wallet::sign_multisig(const std::vector &args) { + CHECK_MULTISIG_ENABLED(); sign_multisig_main(args, false); return true; } bool simple_wallet::sign_multisig_main(const std::vector &args, bool called_by_mms) { + CHECK_MULTISIG_ENABLED(); bool ready; if (m_wallet->key_on_device()) { @@ -1464,12 +1489,14 @@ bool simple_wallet::sign_multisig_main(const std::vector &args, boo bool simple_wallet::submit_multisig(const std::vector &args) { + CHECK_MULTISIG_ENABLED(); submit_multisig_main(args, false); return true; } bool simple_wallet::submit_multisig_main(const std::vector &args, bool called_by_mms) { + CHECK_MULTISIG_ENABLED(); bool ready; uint32_t threshold; if (m_wallet->key_on_device()) @@ -1551,6 +1578,7 @@ bool simple_wallet::submit_multisig_main(const std::vector &args, b bool simple_wallet::export_raw_multisig(const std::vector &args) { + CHECK_MULTISIG_ENABLED(); bool ready; uint32_t threshold; if (m_wallet->key_on_device()) @@ -3074,6 +3102,25 @@ bool simple_wallet::set_load_deprecated_formats(const std::vector & return true; } +bool simple_wallet::set_enable_multisig(const std::vector &args/* = std::vector()*/) +{ + if (args.size() < 2) + { + fail_msg_writer() << tr("Value not specified"); + return true; + } + + const auto pwd_container = get_and_verify_password(); + if (pwd_container) + { + parse_bool_and_use(args[1], [&](bool r) { + m_wallet->enable_multisig(r); + m_wallet->rewrite(m_wallet_file, pwd_container->password()); + }); + } + return true; +} + bool simple_wallet::help(const std::vector &args/* = std::vector()*/) { if(args.empty()) @@ -3391,6 +3438,8 @@ simple_wallet::simple_wallet() " The RPC payment credits balance to target (0 for default).\n " "show-wallet-name-when-locked <1|0>\n " " Set this if you would like to display the wallet name when locked.\n " + "enable-multisig-experimental <1|0>\n " + " Set this to allow multisig commands. Multisig may currently be exploitable if parties do not trust each other.\n " "inactivity-lock-timeout \n " " How many seconds to wait before locking the wallet (0 to disable).")); m_cmd_binder.set_handler("encrypted_seed", @@ -3806,6 +3855,7 @@ bool simple_wallet::set_variable(const std::vector &args) success_msg_writer() << "auto-mine-for-rpc-payment-threshold = " << m_wallet->auto_mine_for_rpc_payment_threshold(); success_msg_writer() << "credits-target = " << m_wallet->credits_target(); success_msg_writer() << "load-deprecated-formats = " << m_wallet->load_deprecated_formats(); + success_msg_writer() << "enable-multisig-experimental = " << m_wallet->is_multisig_enabled(); return true; } else @@ -3872,6 +3922,7 @@ bool simple_wallet::set_variable(const std::vector &args) CHECK_SIMPLE_VARIABLE("persistent-rpc-client-id", set_persistent_rpc_client_id, tr("0 or 1")); CHECK_SIMPLE_VARIABLE("auto-mine-for-rpc-payment-threshold", set_auto_mine_for_rpc_payment_threshold, tr("floating point >= 0")); CHECK_SIMPLE_VARIABLE("credits-target", set_credits_target, tr("unsigned integer")); + CHECK_SIMPLE_VARIABLE("enable-multisig-experimental", set_enable_multisig, tr("0 or 1")); } fail_msg_writer() << tr("set: unrecognized argument(s)"); return true; @@ -6980,6 +7031,7 @@ bool simple_wallet::sweep_unmixable(const std::vector &args_) // actually commit the transactions if (m_wallet->multisig()) { + CHECK_MULTISIG_ENABLED(); bool r = m_wallet->save_multisig_tx(ptx_vector, "multisig_monero_tx"); if (!r) { @@ -7284,6 +7336,7 @@ bool simple_wallet::sweep_main(uint32_t account, uint64_t below, bool locked, co // actually commit the transactions if (m_wallet->multisig()) { + CHECK_MULTISIG_ENABLED(); bool r = m_wallet->save_multisig_tx(ptx_vector, "multisig_monero_tx"); if (!r) { @@ -7518,6 +7571,7 @@ bool simple_wallet::sweep_single(const std::vector &args_) // actually commit the transactions if (m_wallet->multisig()) { + CHECK_MULTISIG_ENABLED(); bool r = m_wallet->save_multisig_tx(ptx_vector, "multisig_monero_tx"); if (!r) { @@ -11549,6 +11603,7 @@ void simple_wallet::mms_auto_config(const std::vector &args) bool simple_wallet::mms(const std::vector &args) { + CHECK_MULTISIG_ENABLED(); try { m_wallet->get_multisig_wallet_state(); diff --git a/src/simplewallet/simplewallet.h b/src/simplewallet/simplewallet.h index 4c005c53a..6c4ddd4e7 100644 --- a/src/simplewallet/simplewallet.h +++ b/src/simplewallet/simplewallet.h @@ -153,6 +153,7 @@ namespace cryptonote bool set_device_name(const std::vector &args = std::vector()); bool set_export_format(const std::vector &args = std::vector()); bool set_load_deprecated_formats(const std::vector &args = std::vector()); + bool set_enable_multisig(const std::vector &args = std::vector()); bool set_persistent_rpc_client_id(const std::vector &args = std::vector()); bool set_auto_mine_for_rpc_payment_threshold(const std::vector &args = std::vector()); bool set_credits_target(const std::vector &args = std::vector()); diff --git a/src/wallet/wallet2.cpp b/src/wallet/wallet2.cpp index f4a5a5855..5076286c4 100644 --- a/src/wallet/wallet2.cpp +++ b/src/wallet/wallet2.cpp @@ -1216,7 +1216,8 @@ wallet2::wallet2(network_type nettype, uint64_t kdf_rounds, bool unattended, std m_rpc_version(0), m_export_format(ExportFormat::Binary), m_load_deprecated_formats(false), - m_credits_target(0) + m_credits_target(0), + m_enable_multisig(false) { set_rpc_client_secret_key(rct::rct2sk(rct::skGen())); } @@ -4051,6 +4052,9 @@ boost::optional wallet2::get_keys_file_data(const epee: value2.SetUint64(m_credits_target); json.AddMember("credits_target", value2, json.GetAllocator()); + value2.SetInt(m_enable_multisig ? 1 : 0); + json.AddMember("enable_multisig", value2, json.GetAllocator()); + // Serialize the JSON object rapidjson::StringBuffer buffer; rapidjson::Writer writer(buffer); @@ -4199,6 +4203,7 @@ bool wallet2::load_keys_buf(const std::string& keys_buf, const epee::wipeable_st m_persistent_rpc_client_id = false; m_auto_mine_for_rpc_payment_threshold = -1.0f; m_credits_target = 0; + m_enable_multisig = false; } else if(json.IsObject()) { @@ -4431,6 +4436,8 @@ bool wallet2::load_keys_buf(const std::string& keys_buf, const epee::wipeable_st m_auto_mine_for_rpc_payment_threshold = field_auto_mine_for_rpc_payment; GET_FIELD_FROM_JSON_RETURN_ON_ERROR(json, credits_target, uint64_t, Uint64, false, 0); m_credits_target = field_credits_target; + GET_FIELD_FROM_JSON_RETURN_ON_ERROR(json, enable_multisig, int, Int, false, false); + m_enable_multisig = field_enable_multisig; } else { diff --git a/src/wallet/wallet2.h b/src/wallet/wallet2.h index 660e6a14b..8a9a2e519 100644 --- a/src/wallet/wallet2.h +++ b/src/wallet/wallet2.h @@ -1297,6 +1297,8 @@ private: void set_rpc_client_secret_key(const crypto::secret_key &key) { m_rpc_client_secret_key = key; m_node_rpc_proxy.set_client_secret_key(key); } uint64_t credits_target() const { return m_credits_target; } void credits_target(uint64_t threshold) { m_credits_target = threshold; } + bool is_multisig_enabled() const { return m_enable_multisig; } + void enable_multisig(bool enable) { m_enable_multisig = enable; } bool get_tx_key_cached(const crypto::hash &txid, crypto::secret_key &tx_key, std::vector &additional_tx_keys) const; void set_tx_key(const crypto::hash &txid, const crypto::secret_key &tx_key, const std::vector &additional_tx_keys, const boost::optional &single_destination_subaddress = boost::none); @@ -1811,6 +1813,7 @@ private: crypto::secret_key m_rpc_client_secret_key; rpc_payment_state_t m_rpc_payment_state; uint64_t m_credits_target; + bool m_enable_multisig; // Aux transaction data from device serializable_unordered_map m_tx_device; diff --git a/src/wallet/wallet_rpc_server.cpp b/src/wallet/wallet_rpc_server.cpp index 57baf428f..7ec5fc7a1 100644 --- a/src/wallet/wallet_rpc_server.cpp +++ b/src/wallet/wallet_rpc_server.cpp @@ -61,6 +61,17 @@ using namespace epee; #define DEFAULT_AUTO_REFRESH_PERIOD 20 // seconds +#define CHECK_MULTISIG_ENABLED() \ + do \ + { \ + if (m_wallet->multisig() && !m_wallet->is_multisig_enabled()) \ + { \ + er.code = WALLET_RPC_ERROR_CODE_DISABLED; \ + er.message = "This wallet is multisig, and multisig is disabled. 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. You can enable it by running this once in monero-wallet-cli: set enable-multisig-experimental 1"; \ + return false; \ + } \ + } while(0) + namespace { const command_line::arg_descriptor arg_rpc_bind_port = {"rpc-bind-port", "Sets bind port for server"}; @@ -1057,6 +1068,8 @@ namespace tools return false; } + CHECK_MULTISIG_ENABLED(); + // validate the transfer requested and populate dsts & extra if (!validate_transfer(req.destinations, req.payment_id, dsts, extra, true, er)) { @@ -1109,6 +1122,8 @@ namespace tools return false; } + CHECK_MULTISIG_ENABLED(); + // validate the transfer requested and populate dsts & extra; RPC_TRANSFER::request and RPC_TRANSFER_SPLIT::request are identical types. if (!validate_transfer(req.destinations, req.payment_id, dsts, extra, true, er)) { @@ -1163,6 +1178,8 @@ namespace tools return false; } + CHECK_MULTISIG_ENABLED(); + cryptonote::blobdata blob; if (!epee::string_tools::parse_hexstr_to_binbuff(req.unsigned_txset, blob)) { @@ -1511,6 +1528,8 @@ namespace tools return false; } + CHECK_MULTISIG_ENABLED(); + try { std::vector ptx_vector = m_wallet->create_unmixable_sweep_transactions(); @@ -1539,6 +1558,8 @@ namespace tools return false; } + CHECK_MULTISIG_ENABLED(); + // validate the transfer requested and populate dsts & extra std::list destination; destination.push_back(wallet_rpc::transfer_destination()); @@ -1604,6 +1625,8 @@ namespace tools return false; } + CHECK_MULTISIG_ENABLED(); + // validate the transfer requested and populate dsts & extra std::list destination; destination.push_back(wallet_rpc::transfer_destination()); @@ -3933,6 +3956,9 @@ namespace tools er.message = "This wallet is already multisig"; return false; } + if (req.enable_multisig_experimental) + m_wallet->enable_multisig(true); + CHECK_MULTISIG_ENABLED(); if (m_wallet->watch_only()) { er.code = WALLET_RPC_ERROR_CODE_WATCH_ONLY; @@ -3959,6 +3985,7 @@ namespace tools er.message = "This wallet is already multisig"; return false; } + CHECK_MULTISIG_ENABLED(); if (m_wallet->watch_only()) { er.code = WALLET_RPC_ERROR_CODE_WATCH_ONLY; @@ -4003,6 +4030,7 @@ namespace tools er.message = "This wallet is multisig, but not yet finalized"; return false; } + CHECK_MULTISIG_ENABLED(); cryptonote::blobdata info; try @@ -4044,6 +4072,7 @@ namespace tools er.message = "This wallet is multisig, but not yet finalized"; return false; } + CHECK_MULTISIG_ENABLED(); if (req.info.size() < threshold - 1) { @@ -4096,6 +4125,7 @@ namespace tools //------------------------------------------------------------------------------------------------------------------------------ bool wallet_rpc_server::on_finalize_multisig(const wallet_rpc::COMMAND_RPC_FINALIZE_MULTISIG::request& req, wallet_rpc::COMMAND_RPC_FINALIZE_MULTISIG::response& res, epee::json_rpc::error& er, const connection_context *ctx) { + CHECK_MULTISIG_ENABLED(); return false; } //------------------------------------------------------------------------------------------------------------------------------ @@ -4123,6 +4153,7 @@ namespace tools er.message = "This wallet is multisig, and already finalized"; return false; } + CHECK_MULTISIG_ENABLED(); if (req.multisig_info.size() + 1 < total) { @@ -4172,6 +4203,7 @@ namespace tools er.message = "This wallet is multisig, but not yet finalized"; return false; } + CHECK_MULTISIG_ENABLED(); cryptonote::blobdata blob; if (!epee::string_tools::parse_hexstr_to_binbuff(req.tx_data_hex, blob)) @@ -4241,6 +4273,7 @@ namespace tools er.message = "This wallet is multisig, but not yet finalized"; return false; } + CHECK_MULTISIG_ENABLED(); cryptonote::blobdata blob; if (!epee::string_tools::parse_hexstr_to_binbuff(req.tx_data_hex, blob)) diff --git a/src/wallet/wallet_rpc_server_commands_defs.h b/src/wallet/wallet_rpc_server_commands_defs.h index fe53e293f..ecfc8e673 100644 --- a/src/wallet/wallet_rpc_server_commands_defs.h +++ b/src/wallet/wallet_rpc_server_commands_defs.h @@ -2416,7 +2416,10 @@ namespace wallet_rpc { struct request_t { + bool enable_multisig_experimental; + BEGIN_KV_SERIALIZE_MAP() + KV_SERIALIZE_OPT(enable_multisig_experimental, false) END_KV_SERIALIZE_MAP() }; typedef epee::misc_utils::struct_init request; diff --git a/src/wallet/wallet_rpc_server_error_codes.h b/src/wallet/wallet_rpc_server_error_codes.h index 914939573..734229380 100644 --- a/src/wallet/wallet_rpc_server_error_codes.h +++ b/src/wallet/wallet_rpc_server_error_codes.h @@ -78,3 +78,4 @@ #define WALLET_RPC_ERROR_CODE_ATTRIBUTE_NOT_FOUND -45 #define WALLET_RPC_ERROR_CODE_ZERO_AMOUNT -46 #define WALLET_RPC_ERROR_CODE_INVALID_SIGNATURE_TYPE -47 +#define WALLET_RPC_ERROR_CODE_DISABLED -48 diff --git a/tests/functional_tests/multisig.py b/tests/functional_tests/multisig.py index 89cb2fdc7..1c5894f47 100755 --- a/tests/functional_tests/multisig.py +++ b/tests/functional_tests/multisig.py @@ -107,7 +107,7 @@ class MultisigTest(): try: self.wallet[i].close_wallet() except: pass res = self.wallet[i].restore_deterministic_wallet(seed = seeds[i]) - res = self.wallet[i].prepare_multisig() + res = self.wallet[i].prepare_multisig(enable_multisig_experimental = True) assert len(res.multisig_info) > 0 info.append(res.multisig_info) @@ -172,7 +172,7 @@ class MultisigTest(): res = wallet2of2[i].restore_deterministic_wallet(seed = seeds[i]) res = wallet2of2[i].is_multisig() assert not res.multisig - res = wallet2of2[i].prepare_multisig() + res = wallet2of2[i].prepare_multisig(enable_multisig_experimental = True) assert len(res.multisig_info) > 0 info2of2.append(res.multisig_info) @@ -187,7 +187,7 @@ class MultisigTest(): assert res.ready ok = False - try: res = wallet2of2[0].prepare_multisig() + try: res = wallet2of2[0].prepare_multisig(enable_multisig_experimental = True) except: ok = True assert ok @@ -205,7 +205,7 @@ class MultisigTest(): res = wallet2of3[i].restore_deterministic_wallet(seed = seeds[i]) res = wallet2of3[i].is_multisig() assert not res.multisig - res = wallet2of3[i].prepare_multisig() + res = wallet2of3[i].prepare_multisig(enable_multisig_experimental = True) assert len(res.multisig_info) > 0 info2of3.append(res.multisig_info) @@ -223,7 +223,7 @@ class MultisigTest(): assert not res.ready ok = False - try: res = wallet2of3[1].prepare_multisig() + try: res = wallet2of3[1].prepare_multisig(enable_multisig_experimental = True) except: ok = True assert ok diff --git a/utils/python-rpc/framework/wallet.py b/utils/python-rpc/framework/wallet.py index 037beee84..01e937627 100644 --- a/utils/python-rpc/framework/wallet.py +++ b/utils/python-rpc/framework/wallet.py @@ -490,10 +490,11 @@ class Wallet(object): } return self.rpc.send_json_rpc_request(is_multisig) - def prepare_multisig(self): + def prepare_multisig(self, enable_multisig_experimental = False): prepare_multisig = { 'method': 'prepare_multisig', 'params' : { + 'enable_multisig_experimental': enable_multisig_experimental, }, 'jsonrpc': '2.0', 'id': '0'