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.
This commit is contained in:
moneromooo-monero 2022-05-14 19:49:48 +00:00
parent 8349cfe4a6
commit 2979474221
No known key found for this signature in database
GPG key ID: 686F07454D6CEFC3
9 changed files with 111 additions and 7 deletions

View file

@ -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<std::string> &args/* = std:
bool simple_wallet::prepare_multisig(const std::vector<std::string> &args)
{
CHECK_MULTISIG_ENABLED();
prepare_multisig_main(args, false);
return true;
}
bool simple_wallet::prepare_multisig_main(const std::vector<std::string> &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<std::string> &args,
bool simple_wallet::make_multisig(const std::vector<std::string> &args)
{
CHECK_MULTISIG_ENABLED();
make_multisig_main(args, false);
return true;
}
bool simple_wallet::make_multisig_main(const std::vector<std::string> &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<std::string> &args, boo
bool simple_wallet::exchange_multisig_keys(const std::vector<std::string> &args)
{
CHECK_MULTISIG_ENABLED();
exchange_multisig_keys_main(args, false);
return true;
}
bool simple_wallet::exchange_multisig_keys_main(const std::vector<std::string> &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<std::string> &
bool simple_wallet::export_multisig(const std::vector<std::string> &args)
{
CHECK_MULTISIG_ENABLED();
export_multisig_main(args, false);
return true;
}
bool simple_wallet::export_multisig_main(const std::vector<std::string> &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<std::string> &args, b
bool simple_wallet::import_multisig(const std::vector<std::string> &args)
{
CHECK_MULTISIG_ENABLED();
import_multisig_main(args, false);
return true;
}
bool simple_wallet::import_multisig_main(const std::vector<std::string> &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<std::string> &args)
{
CHECK_MULTISIG_ENABLED();
sign_multisig_main(args, false);
return true;
}
bool simple_wallet::sign_multisig_main(const std::vector<std::string> &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<std::string> &args, boo
bool simple_wallet::submit_multisig(const std::vector<std::string> &args)
{
CHECK_MULTISIG_ENABLED();
submit_multisig_main(args, false);
return true;
}
bool simple_wallet::submit_multisig_main(const std::vector<std::string> &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<std::string> &args, b
bool simple_wallet::export_raw_multisig(const std::vector<std::string> &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<std::string> &
return true;
}
bool simple_wallet::set_enable_multisig(const std::vector<std::string> &args/* = std::vector<std::string>()*/)
{
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<std::string> &args/* = std::vector<std::string>()*/)
{
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 <unsigned int>\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<std::string> &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<std::string> &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<std::string> &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<std::string> &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<std::string> &args)
bool simple_wallet::mms(const std::vector<std::string> &args)
{
CHECK_MULTISIG_ENABLED();
try
{
m_wallet->get_multisig_wallet_state();

View file

@ -153,6 +153,7 @@ namespace cryptonote
bool set_device_name(const std::vector<std::string> &args = std::vector<std::string>());
bool set_export_format(const std::vector<std::string> &args = std::vector<std::string>());
bool set_load_deprecated_formats(const std::vector<std::string> &args = std::vector<std::string>());
bool set_enable_multisig(const std::vector<std::string> &args = std::vector<std::string>());
bool set_persistent_rpc_client_id(const std::vector<std::string> &args = std::vector<std::string>());
bool set_auto_mine_for_rpc_payment_threshold(const std::vector<std::string> &args = std::vector<std::string>());
bool set_credits_target(const std::vector<std::string> &args = std::vector<std::string>());

View file

@ -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::keys_file_data> 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<rapidjson::StringBuffer> 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
{

View file

@ -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<crypto::secret_key> &additional_tx_keys) const;
void set_tx_key(const crypto::hash &txid, const crypto::secret_key &tx_key, const std::vector<crypto::secret_key> &additional_tx_keys, const boost::optional<cryptonote::account_public_address> &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<crypto::hash, std::string> m_tx_device;

View file

@ -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<std::string, true> 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<wallet2::pending_tx> 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<wallet_rpc::transfer_destination> 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<wallet_rpc::transfer_destination> 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))

View file

@ -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_t> request;

View file

@ -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

View file

@ -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

View file

@ -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'