aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorkoe <ukoe@protonmail.com>2022-05-14 17:07:47 -0500
committerkoe <ukoe@protonmail.com>2022-09-21 12:38:35 -0500
commit4b0785f1c9a6c6d960749938d64420403d70d8c2 (patch)
tree673f718ebb0b0790d7a4b118936303ae7b8bb15c
parentMerge pull request #8435 (diff)
downloadmonero-4b0785f1c9a6c6d960749938d64420403d70d8c2.tar.xz
add an option to force-update multisig key exchange under some circumstances
-rw-r--r--src/gen_multisig/gen_multisig.cpp12
-rw-r--r--src/multisig/multisig_account.cpp7
-rw-r--r--src/multisig/multisig_account.h12
-rw-r--r--src/multisig/multisig_account_kex_impl.cpp151
-rw-r--r--src/simplewallet/simplewallet.cpp28
-rw-r--r--src/simplewallet/simplewallet.h2
-rw-r--r--src/wallet/api/wallet.cpp4
-rw-r--r--src/wallet/api/wallet.h2
-rw-r--r--src/wallet/api/wallet2_api.h3
-rw-r--r--src/wallet/wallet2.cpp31
-rw-r--r--src/wallet/wallet2.h3
-rw-r--r--src/wallet/wallet_rpc_server.cpp9
-rw-r--r--src/wallet/wallet_rpc_server_commands_defs.h4
-rw-r--r--tests/unit_tests/multisig.cpp88
-rw-r--r--utils/python-rpc/framework/wallet.py3
15 files changed, 244 insertions, 115 deletions
diff --git a/src/gen_multisig/gen_multisig.cpp b/src/gen_multisig/gen_multisig.cpp
index f13e74b0f..eedd1511d 100644
--- a/src/gen_multisig/gen_multisig.cpp
+++ b/src/gen_multisig/gen_multisig.cpp
@@ -50,7 +50,6 @@
using namespace std;
using namespace epee;
using namespace cryptonote;
-using boost::lexical_cast;
namespace po = boost::program_options;
#undef MONERO_DEFAULT_LOG_CATEGORY
@@ -84,6 +83,9 @@ static bool generate_multisig(uint32_t threshold, uint32_t total, const std::str
try
{
+ if (total == 0)
+ throw std::runtime_error("Signer group of size 0 is not allowed.");
+
// create M wallets first
std::vector<boost::shared_ptr<tools::wallet2>> wallets(total);
for (size_t n = 0; n < total; ++n)
@@ -118,13 +120,17 @@ static bool generate_multisig(uint32_t threshold, uint32_t total, const std::str
ss << " " << name << std::endl;
}
- //exchange keys unless exchange_multisig_keys returns no extra info
- while (!kex_msgs_intermediate[0].empty())
+ // exchange keys until the wallets are done
+ bool ready{false};
+ wallets[0]->multisig(&ready);
+ while (!ready)
{
for (size_t n = 0; n < total; ++n)
{
kex_msgs_intermediate[n] = wallets[n]->exchange_multisig_keys(pwd_container->password(), kex_msgs_intermediate);
}
+
+ wallets[0]->multisig(&ready);
}
std::string address = wallets[0]->get_account().get_public_address_str(wallets[0]->nettype());
diff --git a/src/multisig/multisig_account.cpp b/src/multisig/multisig_account.cpp
index 9bdcf2dbc..971970277 100644
--- a/src/multisig/multisig_account.cpp
+++ b/src/multisig/multisig_account.cpp
@@ -175,19 +175,20 @@ namespace multisig
// only mutate account if update succeeds
multisig_account temp_account{*this};
temp_account.set_multisig_config(threshold, std::move(signers));
- temp_account.kex_update_impl(expanded_msgs_rnd1);
+ temp_account.kex_update_impl(expanded_msgs_rnd1, false);
*this = std::move(temp_account);
}
//----------------------------------------------------------------------------------------------------------------------
// multisig_account: EXTERNAL
//----------------------------------------------------------------------------------------------------------------------
- void multisig_account::kex_update(const std::vector<multisig_kex_msg> &expanded_msgs)
+ void multisig_account::kex_update(const std::vector<multisig_kex_msg> &expanded_msgs,
+ const bool force_update_use_with_caution /*= false*/)
{
CHECK_AND_ASSERT_THROW_MES(account_is_active(), "multisig account: tried to update kex, but kex isn't initialized yet.");
CHECK_AND_ASSERT_THROW_MES(!multisig_is_ready(), "multisig account: tried to update kex, but kex is already complete.");
multisig_account temp_account{*this};
- temp_account.kex_update_impl(expanded_msgs);
+ temp_account.kex_update_impl(expanded_msgs, force_update_use_with_caution);
*this = std::move(temp_account);
}
//----------------------------------------------------------------------------------------------------------------------
diff --git a/src/multisig/multisig_account.h b/src/multisig/multisig_account.h
index 7b372bbff..794230ffc 100644
--- a/src/multisig/multisig_account.h
+++ b/src/multisig/multisig_account.h
@@ -169,12 +169,20 @@ namespace multisig
* - The main interface for multisig key exchange, this handles all the work of processing input messages,
* creating new messages for new rounds, and finalizing the multisig shared public key when kex is complete.
* param: expanded_msgs - kex messages corresponding to the account's 'in progress' round
+ * param: force_update_use_with_caution - try to force the account to update with messages from an incomplete signer set.
+ * - If this is the post-kex verification round, only require one input message.
+ * - Force updating here should only be done if we can safely assume an honest signer subgroup of size 'threshold'
+ * will complete the account.
+ * - If this is an intermediate round, only require messages from 'num signers - 1 - (round - 1)' other signers.
+ * - If force updating with maliciously-crafted messages, the resulting account will be invalid (either unable
+ * to complete signatures, or a 'hostage' to the malicious signer [i.e. can't sign without his participation]).
*/
- void kex_update(const std::vector<multisig_kex_msg> &expanded_msgs);
+ void kex_update(const std::vector<multisig_kex_msg> &expanded_msgs,
+ const bool force_update_use_with_caution = false);
private:
// implementation of kex_update() (non-transactional)
- void kex_update_impl(const std::vector<multisig_kex_msg> &expanded_msgs);
+ void kex_update_impl(const std::vector<multisig_kex_msg> &expanded_msgs, const bool incomplete_signer_set);
/**
* brief: initialize_kex_update - Helper for kex_update_impl()
* - Collect the local signer's shared keys to ignore in incoming messages, build the aggregate ancillary key
diff --git a/src/multisig/multisig_account_kex_impl.cpp b/src/multisig/multisig_account_kex_impl.cpp
index be9ed9cb2..141b20ead 100644
--- a/src/multisig/multisig_account_kex_impl.cpp
+++ b/src/multisig/multisig_account_kex_impl.cpp
@@ -181,7 +181,8 @@ namespace multisig
* Key aggregation via aggregation coefficients prevents key cancellation attacks.
* See: https://www.getmonero.org/resources/research-lab/pubs/MRL-0009.pdf
* param: final_keys - address components (public keys) obtained from other participants (not shared with local)
- * param: privkeys_inout - private keys of address components known by local; each key will be multiplied by an aggregation coefficient (return by reference)
+ * param: privkeys_inout - private keys of address components known by local; each key will be multiplied by an aggregation
+ * coefficient (return by reference)
* return: final multisig public spend key for the account
*/
//----------------------------------------------------------------------------------------------------------------------
@@ -199,7 +200,8 @@ namespace multisig
for (std::size_t multisig_keys_index{0}; multisig_keys_index < privkeys_inout.size(); ++multisig_keys_index)
{
crypto::public_key pubkey;
- CHECK_AND_ASSERT_THROW_MES(crypto::secret_key_to_public_key(privkeys_inout[multisig_keys_index], pubkey), "Failed to derive public key");
+ CHECK_AND_ASSERT_THROW_MES(crypto::secret_key_to_public_key(privkeys_inout[multisig_keys_index], pubkey),
+ "Failed to derive public key");
own_keys_mapping[pubkey] = multisig_keys_index;
@@ -307,8 +309,7 @@ namespace multisig
* INTERNAL
*
* brief: multisig_kex_msgs_sanitize_pubkeys - Sanitize multisig kex messages.
- * - Removes duplicates from msg pubkeys, ignores pubkeys equal to the local account's signing key,
- * ignores messages signed by the local account, ignores keys found in input 'exclusion set',
+ * - Removes duplicates from msg pubkeys, ignores keys found in input 'exclusion set',
* constructs map of pubkey:origins.
* - Requires that all input msgs have the same round number.
*
@@ -316,15 +317,13 @@ namespace multisig
*
* - If the messages' round numbers are all '1', then only the message signing pubkey is considered
* 'recommended'. Furthermore, the 'exclusion set' is ignored.
- * param: own_pubkey - local account's signing key (key used to sign multisig messages)
* param: expanded_msgs - set of multisig kex messages to process
* param: exclude_pubkeys - pubkeys to exclude from output set
* outparam: sanitized_pubkeys_out - processed pubkeys obtained from msgs, mapped to their origins
* return: round number shared by all input msgs
*/
//----------------------------------------------------------------------------------------------------------------------
- static std::uint32_t multisig_kex_msgs_sanitize_pubkeys(const crypto::public_key &own_pubkey,
- const std::vector<multisig_kex_msg> &expanded_msgs,
+ static std::uint32_t multisig_kex_msgs_sanitize_pubkeys(const std::vector<multisig_kex_msg> &expanded_msgs,
const std::vector<crypto::public_key> &exclude_pubkeys,
multisig_keyset_map_memsafe_t &sanitized_pubkeys_out)
{
@@ -339,10 +338,6 @@ namespace multisig
// - origins = all the signing pubkeys that recommended a given msg pubkey
for (const auto &expanded_msg : expanded_msgs)
{
- // ignore messages from self
- if (expanded_msg.get_signing_pubkey() == own_pubkey)
- continue;
-
// in round 1, only the signing pubkey is treated as a msg pubkey
if (round == 1)
{
@@ -355,10 +350,6 @@ namespace multisig
// copy all pubkeys from message into list
for (const auto &pubkey : expanded_msg.get_msg_pubkeys())
{
- // ignore own pubkey
- if (pubkey == own_pubkey)
- continue;
-
// ignore pubkeys in 'ignore' set
if (std::find(exclude_pubkeys.begin(), exclude_pubkeys.end(), pubkey) != exclude_pubkeys.end())
continue;
@@ -375,6 +366,31 @@ namespace multisig
/**
* INTERNAL
*
+ * brief: remove_key_from_mapped_sets - Remove a specified key from the mapped sets in a multisig keyset map.
+ * param: key_to_remove - specified key to remove
+ * inoutparam: keyset_inout - keyset to update
+ */
+ //----------------------------------------------------------------------------------------------------------------------
+ static void remove_key_from_mapped_sets(const crypto::public_key &key_to_remove,
+ multisig_keyset_map_memsafe_t &keyset_inout)
+ {
+ // remove specified key from each mapped set
+ for (auto keyset_it = keyset_inout.begin(); keyset_it != keyset_inout.end();)
+ {
+ // remove specified key from this set
+ keyset_it->second.erase(key_to_remove);
+
+ // remove empty keyset positions or increment iterator
+ if (keyset_it->second.size() == 0)
+ keyset_it = keyset_inout.erase(keyset_it);
+ else
+ ++keyset_it;
+ }
+ }
+ //----------------------------------------------------------------------------------------------------------------------
+ /**
+ * INTERNAL
+ *
* brief: evaluate_multisig_kex_round_msgs - Evaluate pubkeys from a kex round in order to prepare for the next round.
* - Sanitizes input msgs.
* - Require uniqueness in: 'exclude_pubkeys'.
@@ -392,6 +408,8 @@ namespace multisig
* param: signers - expected participants in multisig kex
* param: expanded_msgs - set of multisig kex messages to process
* param: exclude_pubkeys - derivations held by the local account corresponding to round 'expected_round'
+ * param: incomplete_signer_set - only require the minimum number of signers to complete this round
+ * minimum = num_signers - (round num - 1) (including local signer)
* return: fully sanitized and validated pubkey:origins map for building the account's next kex round message
*/
//----------------------------------------------------------------------------------------------------------------------
@@ -400,7 +418,8 @@ namespace multisig
const std::uint32_t expected_round,
const std::vector<crypto::public_key> &signers,
const std::vector<multisig_kex_msg> &expanded_msgs,
- const std::vector<crypto::public_key> &exclude_pubkeys)
+ const std::vector<crypto::public_key> &exclude_pubkeys,
+ const bool incomplete_signer_set)
{
// exclude_pubkeys should all be unique
for (auto it = exclude_pubkeys.begin(); it != exclude_pubkeys.end(); ++it)
@@ -410,21 +429,31 @@ namespace multisig
}
// sanitize input messages
- multisig_keyset_map_memsafe_t pubkey_origins_map;
- const std::uint32_t round = multisig_kex_msgs_sanitize_pubkeys(base_pubkey, expanded_msgs, exclude_pubkeys, pubkey_origins_map);
+ multisig_keyset_map_memsafe_t pubkey_origins_map; //map: [pubkey : [origins]]
+ const std::uint32_t round = multisig_kex_msgs_sanitize_pubkeys(expanded_msgs, exclude_pubkeys, pubkey_origins_map);
CHECK_AND_ASSERT_THROW_MES(round == expected_round,
"Kex messages were for round [" << round << "], but expected round is [" << expected_round << "]");
+ // remove the local signer from each origins set in the sanitized pubkey map
+ // note: intermediate kex rounds only need keys from other signers to make progress (keys from self are useless)
+ remove_key_from_mapped_sets(base_pubkey, pubkey_origins_map);
+
// evaluate pubkeys collected
- std::unordered_map<crypto::public_key, std::unordered_set<crypto::public_key>> origin_pubkeys_map;
+ std::unordered_map<crypto::public_key, std::unordered_set<crypto::public_key>> origin_pubkeys_map; //map: [origin: [pubkeys]]
// 1. each pubkey should be recommended by a precise number of signers
+ const std::size_t num_recommendations_per_pubkey_required{
+ incomplete_signer_set
+ ? 1
+ : round
+ };
+
for (const auto &pubkey_and_origins : pubkey_origins_map)
{
// expected amount = round_num
// With each successive round, pubkeys are shared by incrementally larger groups,
// starting at 1 in round 1 (i.e. the local multisig key to start kex with).
- CHECK_AND_ASSERT_THROW_MES(pubkey_and_origins.second.size() == round,
+ CHECK_AND_ASSERT_THROW_MES(pubkey_and_origins.second.size() >= num_recommendations_per_pubkey_required,
"A pubkey recommended by multisig kex messages had an unexpected number of recommendations.");
// map (sanitized) pubkeys back to origins
@@ -433,8 +462,18 @@ namespace multisig
}
// 2. the number of unique signers recommending pubkeys should equal the number of signers passed in (minus the local signer)
- CHECK_AND_ASSERT_THROW_MES(origin_pubkeys_map.size() == signers.size() - 1,
- "Number of unique other signers does not equal number of other signers that recommended pubkeys.");
+ // - if an incomplete set is allowed, then we need at least one signer to represent each subgroup in this round that
+ // doesn't include the local signer
+ const std::size_t num_signers_required{
+ incomplete_signer_set
+ ? signers.size() - 1 - (round - 1)
+ : signers.size() - 1
+ };
+
+ CHECK_AND_ASSERT_THROW_MES(origin_pubkeys_map.size() >= num_signers_required,
+ "Number of unique other signers recommending pubkeys does not equal number of required other signers "
+ "(kex round: " << round << ", num signers found: " << origin_pubkeys_map.size() << ", num signers required: " <<
+ num_signers_required << ").");
// 3. each origin should recommend a precise number of pubkeys
@@ -461,19 +500,20 @@ namespace multisig
// other signers: (N - 2) choose (msg_round_num - 1)
// - Each signer recommends keys they share with other signers.
- // - In each round, a signer shares a key with 'round num - 1' other signers.
- // - Since 'origins pubkey map' excludes keys shared with the local account,
- // only keys shared with participants 'other than local and self' will be in the map (e.g. N - 2 signers).
- // - So other signers will recommend (N - 2) choose (msg_round_num - 1) pubkeys (after removing keys shared with local).
- // - Each origin should have a shared key with each group of size 'round - 1'.
- // Note: Keys shared with local are ignored to facilitate kex round boosting, where one or more signers may
+ // - In each round, every group of size 'round num' will have a key. From a single signer's perspective,
+ // they will share a key with every group of size 'round num - 1' of other signers.
+ // - Since 'origins pubkey map' excludes keys shared with the local account, only keys shared with participants
+ // 'other than local and self' will be in the map (e.g. N - 2 signers).
+ // - Other signers will recommend (N - 2) choose (msg_round_num - 1) pubkeys (after removing keys shared with local).
+ // Note: Keys shared with local are filtered out to facilitate kex round boosting, where one or more signers may
// have boosted the local signer (implying they didn't have access to the local signer's previous round msg).
const std::uint32_t expected_recommendations_others = n_choose_k_f(signers.size() - 2, round - 1);
// local: (N - 1) choose (msg_round_num - 1)
const std::uint32_t expected_recommendations_self = n_choose_k_f(signers.size() - 1, round - 1);
- // note: expected_recommendations_others would be 0 in the last round of 1-of-N, but we return early for that case
+ // note: expected_recommendations_others would be 0 in the last round of 1-of-N, but we don't call this function for
+ // that case
CHECK_AND_ASSERT_THROW_MES(expected_recommendations_self > 0 && expected_recommendations_others > 0,
"Bad num signers or round num (possibly numerical limits exceeded).");
@@ -485,7 +525,7 @@ namespace multisig
for (const auto &origin_and_pubkeys : origin_pubkeys_map)
{
CHECK_AND_ASSERT_THROW_MES(origin_and_pubkeys.second.size() == expected_recommendations_others,
- "A pubkey recommended by multisig kex messages had an unexpected number of recommendations.");
+ "A multisig signer recommended an unexpected number of pubkeys.");
// 2 (continued). only expected signers should be recommending keys
CHECK_AND_ASSERT_THROW_MES(std::find(signers.begin(), signers.end(), origin_and_pubkeys.first) != signers.end(),
@@ -507,6 +547,7 @@ namespace multisig
* param: expected_round - expected kex round of input messages
* param: signers - expected participants in multisig kex
* param: expanded_msgs - set of multisig kex messages to process
+ * param: incomplete_signer_set - only require the minimum amount of messages to complete this round (1 message)
* return: sanitized and validated pubkey:origins map
*/
//----------------------------------------------------------------------------------------------------------------------
@@ -514,15 +555,20 @@ namespace multisig
const crypto::public_key &base_pubkey,
const std::uint32_t expected_round,
const std::vector<crypto::public_key> &signers,
- const std::vector<multisig_kex_msg> &expanded_msgs)
+ const std::vector<multisig_kex_msg> &expanded_msgs,
+ const bool incomplete_signer_set)
{
// sanitize input messages
const std::vector<crypto::public_key> dummy;
- multisig_keyset_map_memsafe_t pubkey_origins_map;
- const std::uint32_t round = multisig_kex_msgs_sanitize_pubkeys(base_pubkey, expanded_msgs, dummy, pubkey_origins_map);
+ multisig_keyset_map_memsafe_t pubkey_origins_map; //map: [pubkey : [origins]]
+ const std::uint32_t round = multisig_kex_msgs_sanitize_pubkeys(expanded_msgs, dummy, pubkey_origins_map);
CHECK_AND_ASSERT_THROW_MES(round == expected_round,
"Kex messages were for round [" << round << "], but expected round is [" << expected_round << "]");
+ // note: do NOT remove the local signer from the pubkey origins map, since the post-kex round can be force-updated with
+ // just the local signer's post-kex message (if the local signer were removed, then the post-kex message's pubkeys
+ // would be completely lost)
+
// evaluate pubkeys collected
// 1) there should only be two pubkeys
@@ -533,17 +579,26 @@ namespace multisig
CHECK_AND_ASSERT_THROW_MES(pubkey_origins_map.begin()->second == (++(pubkey_origins_map.begin()))->second,
"Multisig post-kex round messages from other signers did not all recommend the same pubkey pair.");
- // 3) all signers should be present in the recommendation list
+ // 3) all signers should be present in the recommendation list (unless an incomplete list is permitted)
auto origins = pubkey_origins_map.begin()->second;
- origins.insert(base_pubkey); //add self
+ origins.insert(base_pubkey); //add self if missing
- CHECK_AND_ASSERT_THROW_MES(origins.size() == signers.size(),
- "Multisig post-kex round message origins don't line up with multisig signer set.");
+ const std::size_t num_signers_required{
+ incomplete_signer_set
+ ? 1
+ : signers.size()
+ };
+
+ CHECK_AND_ASSERT_THROW_MES(origins.size() >= num_signers_required,
+ "Multisig post-kex round message origins don't line up with multisig signer set "
+ "(num signers found: " << origins.size() << ", num signers required: " << num_signers_required << ").");
- for (const crypto::public_key &signer : signers)
+ for (const crypto::public_key &origin : origins)
{
- CHECK_AND_ASSERT_THROW_MES(origins.find(signer) != origins.end(),
- "Could not find an expected signer in multisig post-kex round messages (all signers expected).");
+ // note: if num_signers_required == signers.size(), then this test will ensure all signers are present in 'origins',
+ // which contains only unique pubkeys
+ CHECK_AND_ASSERT_THROW_MES(std::find(signers.begin(), signers.end(), origin) != signers.end(),
+ "An unknown origin recommended a multisig post-kex verification messsage.");
}
return pubkey_origins_map;
@@ -564,6 +619,7 @@ namespace multisig
* param: expanded_msgs - set of multisig kex messages to process
* param: exclude_pubkeys - keys held by the local account corresponding to round 'current_round'
* - If 'current_round' is the final round, these are the local account's shares of the final aggregate key.
+ * param: incomplete_signer_set - allow messages from an incomplete signer set
* outparam: keys_to_origins_map_out - map between round keys and identity keys
* - If in the final round, these are key shares recommended by other signers for the final aggregate key.
* - Otherwise, these are the local account's DH derivations for the next round.
@@ -578,6 +634,7 @@ namespace multisig
const std::vector<crypto::public_key> &signers,
const std::vector<multisig_kex_msg> &expanded_msgs,
const std::vector<crypto::public_key> &exclude_pubkeys,
+ const bool incomplete_signer_set,
multisig_keyset_map_memsafe_t &keys_to_origins_map_out)
{
check_multisig_config(current_round, threshold, signers.size());
@@ -598,7 +655,8 @@ namespace multisig
current_round,
signers,
expanded_msgs,
- exclude_pubkeys);
+ exclude_pubkeys,
+ incomplete_signer_set);
}
else //(current_round == kex_rounds_required + 1)
{
@@ -606,7 +664,8 @@ namespace multisig
evaluated_pubkeys = evaluate_multisig_post_kex_round_msgs(base_pubkey,
current_round,
signers,
- expanded_msgs);
+ expanded_msgs,
+ incomplete_signer_set);
}
// prepare keys-to-origins map for updating the multisig account
@@ -693,9 +752,9 @@ namespace multisig
{
// post-kex verification round: check that the multisig pubkey and common pubkey were recommended by other signers
CHECK_AND_ASSERT_THROW_MES(result_keys_to_origins_map.count(m_multisig_pubkey) > 0,
- "Multisig post-kex round: expected multisig pubkey wasn't found in other signers' messages.");
+ "Multisig post-kex round: expected multisig pubkey wasn't found in input messages.");
CHECK_AND_ASSERT_THROW_MES(result_keys_to_origins_map.count(m_common_pubkey) > 0,
- "Multisig post-kex round: expected common pubkey wasn't found in other signers' messages.");
+ "Multisig post-kex round: expected common pubkey wasn't found in input messages.");
// save keys that should be recommended to other signers
// - for convenience, re-recommend the post-kex verification message once an account is complete
@@ -790,7 +849,8 @@ namespace multisig
//----------------------------------------------------------------------------------------------------------------------
// multisig_account: INTERNAL
//----------------------------------------------------------------------------------------------------------------------
- void multisig_account::kex_update_impl(const std::vector<multisig_kex_msg> &expanded_msgs)
+ void multisig_account::kex_update_impl(const std::vector<multisig_kex_msg> &expanded_msgs,
+ const bool incomplete_signer_set)
{
// check messages are for the expected kex round
check_messages_round(expanded_msgs, m_kex_rounds_complete + 1);
@@ -816,6 +876,7 @@ namespace multisig
m_signers,
expanded_msgs,
exclude_pubkeys,
+ incomplete_signer_set,
result_keys_to_origins_map);
// finish account update
diff --git a/src/simplewallet/simplewallet.cpp b/src/simplewallet/simplewallet.cpp
index a8f4e5a07..db5be4dc2 100644
--- a/src/simplewallet/simplewallet.cpp
+++ b/src/simplewallet/simplewallet.cpp
@@ -244,7 +244,7 @@ namespace
const char* USAGE_IMPORT_OUTPUTS("import_outputs <filename>");
const char* USAGE_SHOW_TRANSFER("show_transfer <txid>");
const char* USAGE_MAKE_MULTISIG("make_multisig <threshold> <string1> [<string>...]");
- const char* USAGE_EXCHANGE_MULTISIG_KEYS("exchange_multisig_keys <string> [<string>...]");
+ const char* USAGE_EXCHANGE_MULTISIG_KEYS("exchange_multisig_keys [force-update-use-with-caution] <string> [<string>...]");
const char* USAGE_EXPORT_MULTISIG_INFO("export_multisig_info <filename>");
const char* USAGE_IMPORT_MULTISIG_INFO("import_multisig_info <filename> [<filename>...]");
const char* USAGE_SIGN_MULTISIG("sign_multisig <filename>");
@@ -1139,11 +1139,22 @@ 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);
+ bool force_update_use_with_caution = false;
+
+ auto local_args = args;
+ if (args.size() >= 1 && local_args[0] == "force-update-use-with-caution")
+ {
+ force_update_use_with_caution = true;
+ local_args.erase(local_args.begin());
+ }
+
+ exchange_multisig_keys_main(local_args, force_update_use_with_caution, false);
return true;
}
-bool simple_wallet::exchange_multisig_keys_main(const std::vector<std::string> &args, bool called_by_mms) {
+bool simple_wallet::exchange_multisig_keys_main(const std::vector<std::string> &args,
+ const bool force_update_use_with_caution,
+ const bool called_by_mms) {
CHECK_MULTISIG_ENABLED();
bool ready;
if (m_wallet->key_on_device())
@@ -1169,15 +1180,9 @@ bool simple_wallet::exchange_multisig_keys_main(const std::vector<std::string> &
return false;
}
- if (args.size() < 1)
- {
- PRINT_USAGE(USAGE_EXCHANGE_MULTISIG_KEYS);
- return false;
- }
-
try
{
- std::string multisig_extra_info = m_wallet->exchange_multisig_keys(orig_pwd_container->password(), args);
+ std::string multisig_extra_info = m_wallet->exchange_multisig_keys(orig_pwd_container->password(), args, force_update_use_with_caution);
bool ready;
m_wallet->multisig(&ready);
if (!ready)
@@ -11157,7 +11162,8 @@ void simple_wallet::mms_next(const std::vector<std::string> &args)
mms::message m = ms.get_message_by_id(data.message_ids[i]);
sig_args[i] = m.content;
}
- command_successful = exchange_multisig_keys_main(sig_args, true);
+ // todo: update mms to enable 'key exchange force updating'
+ command_successful = exchange_multisig_keys_main(sig_args, false, true);
break;
}
diff --git a/src/simplewallet/simplewallet.h b/src/simplewallet/simplewallet.h
index 6a9fa149d..1b67d9754 100644
--- a/src/simplewallet/simplewallet.h
+++ b/src/simplewallet/simplewallet.h
@@ -235,7 +235,7 @@ namespace cryptonote
bool make_multisig(const std::vector<std::string>& args);
bool make_multisig_main(const std::vector<std::string>& args, bool called_by_mms);
bool exchange_multisig_keys(const std::vector<std::string> &args);
- bool exchange_multisig_keys_main(const std::vector<std::string> &args, bool called_by_mms);
+ bool exchange_multisig_keys_main(const std::vector<std::string> &args, const bool force_update_use_with_caution, const bool called_by_mms);
bool export_multisig(const std::vector<std::string>& args);
bool export_multisig_main(const std::vector<std::string>& args, bool called_by_mms);
bool import_multisig(const std::vector<std::string>& args);
diff --git a/src/wallet/api/wallet.cpp b/src/wallet/api/wallet.cpp
index 1ee2e20b6..3327c2821 100644
--- a/src/wallet/api/wallet.cpp
+++ b/src/wallet/api/wallet.cpp
@@ -1396,12 +1396,12 @@ string WalletImpl::makeMultisig(const vector<string>& info, const uint32_t thres
return string();
}
-std::string WalletImpl::exchangeMultisigKeys(const std::vector<std::string> &info) {
+std::string WalletImpl::exchangeMultisigKeys(const std::vector<std::string> &info, const bool force_update_use_with_caution /*= false*/) {
try {
clearStatus();
checkMultisigWalletNotReady(m_wallet);
- return m_wallet->exchange_multisig_keys(epee::wipeable_string(m_password), info);
+ return m_wallet->exchange_multisig_keys(epee::wipeable_string(m_password), info, force_update_use_with_caution);
} catch (const exception& e) {
LOG_ERROR("Error on exchanging multisig keys: " << e.what());
setStatusError(string(tr("Failed to exchange multisig keys: ")) + e.what());
diff --git a/src/wallet/api/wallet.h b/src/wallet/api/wallet.h
index 018b2a0ed..ec2d7e9b3 100644
--- a/src/wallet/api/wallet.h
+++ b/src/wallet/api/wallet.h
@@ -146,7 +146,7 @@ public:
MultisigState multisig() const override;
std::string getMultisigInfo() const override;
std::string makeMultisig(const std::vector<std::string>& info, uint32_t threshold) override;
- std::string exchangeMultisigKeys(const std::vector<std::string> &info) override;
+ std::string exchangeMultisigKeys(const std::vector<std::string> &info, const bool force_update_use_with_caution = false) override;
bool exportMultisigImages(std::string& images) override;
size_t importMultisigImages(const std::vector<std::string>& images) override;
bool hasMultisigPartialKeyImages() const override;
diff --git a/src/wallet/api/wallet2_api.h b/src/wallet/api/wallet2_api.h
index b67bce60c..0ae84adb9 100644
--- a/src/wallet/api/wallet2_api.h
+++ b/src/wallet/api/wallet2_api.h
@@ -796,9 +796,10 @@ struct Wallet
/**
* @brief exchange_multisig_keys - provides additional key exchange round for arbitrary multisig schemes (like N-1/N, M/N)
* @param info - base58 encoded key derivations returned by makeMultisig or exchangeMultisigKeys function call
+ * @param force_update_use_with_caution - force multisig account to update even if not all signers contribute round messages
* @return new info string if more rounds required or an empty string if wallet creation is done
*/
- virtual std::string exchangeMultisigKeys(const std::vector<std::string> &info) = 0;
+ virtual std::string exchangeMultisigKeys(const std::vector<std::string> &info, const bool force_update_use_with_caution) = 0;
/**
* @brief exportMultisigImages - exports transfers' key images
* @param images - output paramter for hex encoded array of images
diff --git a/src/wallet/wallet2.cpp b/src/wallet/wallet2.cpp
index 195763949..61b34102b 100644
--- a/src/wallet/wallet2.cpp
+++ b/src/wallet/wallet2.cpp
@@ -5075,12 +5075,11 @@ std::string wallet2::make_multisig(const epee::wipeable_string &password,
}
//----------------------------------------------------------------------------------------------------
std::string wallet2::exchange_multisig_keys(const epee::wipeable_string &password,
- const std::vector<std::string> &kex_messages)
+ const std::vector<std::string> &kex_messages,
+ const bool force_update_use_with_caution /*= false*/)
{
bool ready{false};
CHECK_AND_ASSERT_THROW_MES(multisig(&ready), "The wallet is not multisig");
- CHECK_AND_ASSERT_THROW_MES(!ready, "Multisig wallet creation process has already been finished");
- CHECK_AND_ASSERT_THROW_MES(kex_messages.size() > 0, "No key exchange messages passed in.");
// decrypt account keys
epee::misc_utils::auto_scope_leave_caller keys_reencryptor;
@@ -5099,13 +5098,6 @@ std::string wallet2::exchange_multisig_keys(const epee::wipeable_string &passwor
);
}
- // open kex messages
- std::vector<multisig::multisig_kex_msg> expanded_msgs;
- expanded_msgs.reserve(kex_messages.size());
-
- for (const auto &msg : kex_messages)
- expanded_msgs.emplace_back(msg);
-
// reconstruct multisig account
multisig::multisig_keyset_map_memsafe_t kex_origins_map;
@@ -5126,8 +5118,25 @@ std::string wallet2::exchange_multisig_keys(const epee::wipeable_string &passwor
""
};
+ // KLUDGE: early return if there are no kex messages and main kex is complete (will return the post-kex verification round
+ // message) (it's a kludge because this behavior would be more appropriate for a standalone wallet method)
+ if (kex_messages.size() == 0)
+ {
+ CHECK_AND_ASSERT_THROW_MES(multisig_account.main_kex_rounds_done(),
+ "Exchange multisig keys: there are no kex messages but the main kex rounds are not done.");
+
+ return multisig_account.get_next_kex_round_msg();
+ }
+
+ // open kex messages
+ std::vector<multisig::multisig_kex_msg> expanded_msgs;
+ expanded_msgs.reserve(kex_messages.size());
+
+ for (const auto &msg : kex_messages)
+ expanded_msgs.emplace_back(msg);
+
// update multisig kex
- multisig_account.kex_update(expanded_msgs);
+ multisig_account.kex_update(expanded_msgs, force_update_use_with_caution);
// update wallet state
diff --git a/src/wallet/wallet2.h b/src/wallet/wallet2.h
index 16e898ad8..81472c200 100644
--- a/src/wallet/wallet2.h
+++ b/src/wallet/wallet2.h
@@ -862,7 +862,8 @@ private:
* to other participants
*/
std::string exchange_multisig_keys(const epee::wipeable_string &password,
- const std::vector<std::string> &kex_messages);
+ const std::vector<std::string> &kex_messages,
+ const bool force_update_use_with_caution = false);
/*!
* \brief Get initial message to start multisig key exchange (before 'make_multisig()' is called)
* \return string to send to other participants
diff --git a/src/wallet/wallet_rpc_server.cpp b/src/wallet/wallet_rpc_server.cpp
index 7ec5fc7a1..4cb837d38 100644
--- a/src/wallet/wallet_rpc_server.cpp
+++ b/src/wallet/wallet_rpc_server.cpp
@@ -4146,13 +4146,6 @@ namespace tools
er.message = "This wallet is not multisig";
return false;
}
-
- if (ready)
- {
- er.code = WALLET_RPC_ERROR_CODE_ALREADY_MULTISIG;
- er.message = "This wallet is multisig, and already finalized";
- return false;
- }
CHECK_MULTISIG_ENABLED();
if (req.multisig_info.size() + 1 < total)
@@ -4164,7 +4157,7 @@ namespace tools
try
{
- res.multisig_info = m_wallet->exchange_multisig_keys(req.password, req.multisig_info);
+ res.multisig_info = m_wallet->exchange_multisig_keys(req.password, req.multisig_info, req.force_update_use_with_caution);
m_wallet->multisig(&ready);
if (ready)
{
diff --git a/src/wallet/wallet_rpc_server_commands_defs.h b/src/wallet/wallet_rpc_server_commands_defs.h
index ecfc8e673..61b7cf5cd 100644
--- a/src/wallet/wallet_rpc_server_commands_defs.h
+++ b/src/wallet/wallet_rpc_server_commands_defs.h
@@ -47,7 +47,7 @@
// advance which version they will stop working with
// Don't go over 32767 for any of these
#define WALLET_RPC_VERSION_MAJOR 1
-#define WALLET_RPC_VERSION_MINOR 25
+#define WALLET_RPC_VERSION_MINOR 26
#define MAKE_WALLET_RPC_VERSION(major,minor) (((major)<<16)|(minor))
#define WALLET_RPC_VERSION MAKE_WALLET_RPC_VERSION(WALLET_RPC_VERSION_MAJOR, WALLET_RPC_VERSION_MINOR)
namespace tools
@@ -2531,10 +2531,12 @@ namespace wallet_rpc
{
std::string password;
std::vector<std::string> multisig_info;
+ bool force_update_use_with_caution;
BEGIN_KV_SERIALIZE_MAP()
KV_SERIALIZE(password)
KV_SERIALIZE(multisig_info)
+ KV_SERIALIZE_OPT(force_update_use_with_caution, false)
END_KV_SERIALIZE_MAP()
};
typedef epee::misc_utils::struct_init<request_t> request;
diff --git a/tests/unit_tests/multisig.cpp b/tests/unit_tests/multisig.cpp
index 5ddd78955..7015781b4 100644
--- a/tests/unit_tests/multisig.cpp
+++ b/tests/unit_tests/multisig.cpp
@@ -96,8 +96,46 @@ static std::vector<std::string> exchange_round(std::vector<tools::wallet2>& wall
new_infos.reserve(infos.size());
for (size_t i = 0; i < wallets.size(); ++i)
+ new_infos.push_back(wallets[i].exchange_multisig_keys("", infos));
+
+ return new_infos;
+}
+
+static std::vector<std::string> exchange_round_force_update(std::vector<tools::wallet2>& wallets,
+ const std::vector<std::string>& infos,
+ const std::size_t round_in_progress)
+{
+ EXPECT_TRUE(wallets.size() == infos.size());
+ std::vector<std::string> new_infos;
+ std::vector<std::string> temp_force_update_infos;
+ new_infos.reserve(infos.size());
+
+ // when force-updating, we only need at most 'num_signers - 1 - (round - 1)' messages from other signers
+ size_t num_other_messages_required{wallets.size() - 1 - (round_in_progress - 1)};
+ if (num_other_messages_required > wallets.size())
+ num_other_messages_required = 0; //overflow case for post-kex verification round of 1-of-N
+
+ for (size_t i = 0; i < wallets.size(); ++i)
{
- new_infos.push_back(wallets[i].exchange_multisig_keys("", infos));
+ temp_force_update_infos.clear();
+ temp_force_update_infos.reserve(num_other_messages_required + 1);
+ temp_force_update_infos.push_back(infos[i]); //always include the local signer's message for this round
+
+ size_t infos_collected{0};
+ for (size_t wallet_index = 0; wallet_index < wallets.size(); ++wallet_index)
+ {
+ // skip the local signer's message
+ if (wallet_index == i)
+ continue;
+
+ temp_force_update_infos.push_back(infos[wallet_index]);
+ ++infos_collected;
+
+ if (infos_collected == num_other_messages_required)
+ break;
+ }
+
+ new_infos.push_back(wallets[i].exchange_multisig_keys("", temp_force_update_infos, true));
}
return new_infos;
@@ -105,7 +143,7 @@ static std::vector<std::string> exchange_round(std::vector<tools::wallet2>& wall
static void check_results(const std::vector<std::string> &intermediate_infos,
std::vector<tools::wallet2>& wallets,
- std::uint32_t M)
+ const std::uint32_t M)
{
// check results
std::unordered_set<crypto::secret_key> unique_privkeys;
@@ -167,8 +205,9 @@ static void check_results(const std::vector<std::string> &intermediate_infos,
wallets[0].encrypt_keys("");
}
-static void make_wallets(std::vector<tools::wallet2>& wallets, unsigned int M)
+static void make_wallets(const unsigned int M, const unsigned int N, const bool force_update)
{
+ std::vector<tools::wallet2> wallets(N);
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;
@@ -207,9 +246,12 @@ static void make_wallets(std::vector<tools::wallet2>& wallets, unsigned int M)
wallets[0].multisig(&ready);
while (!ready)
{
- intermediate_infos = exchange_round(wallets, intermediate_infos);
- wallets[0].multisig(&ready);
+ if (force_update)
+ intermediate_infos = exchange_round_force_update(wallets, intermediate_infos, rounds_complete + 1);
+ else
+ intermediate_infos = exchange_round(wallets, intermediate_infos);
+ wallets[0].multisig(&ready);
++rounds_complete;
}
@@ -220,38 +262,38 @@ static void make_wallets(std::vector<tools::wallet2>& wallets, unsigned int M)
TEST(multisig, make_1_2)
{
- std::vector<tools::wallet2> wallets(2);
- make_wallets(wallets, 1);
+ make_wallets(1, 2, false);
+ make_wallets(1, 2, true);
}
TEST(multisig, make_1_3)
{
- std::vector<tools::wallet2> wallets(3);
- make_wallets(wallets, 1);
+ make_wallets(1, 3, false);
+ make_wallets(1, 3, true);
}
TEST(multisig, make_2_2)
{
- std::vector<tools::wallet2> wallets(2);
- make_wallets(wallets, 2);
+ make_wallets(2, 2, false);
+ make_wallets(2, 2, true);
}
TEST(multisig, make_3_3)
{
- std::vector<tools::wallet2> wallets(3);
- make_wallets(wallets, 3);
+ make_wallets(3, 3, false);
+ make_wallets(3, 3, true);
}
TEST(multisig, make_2_3)
{
- std::vector<tools::wallet2> wallets(3);
- make_wallets(wallets, 2);
+ make_wallets(2, 3, false);
+ make_wallets(2, 3, true);
}
TEST(multisig, make_2_4)
{
- std::vector<tools::wallet2> wallets(4);
- make_wallets(wallets, 2);
+ make_wallets(2, 4, false);
+ make_wallets(2, 4, true);
}
TEST(multisig, multisig_kex_msg)
@@ -272,9 +314,7 @@ TEST(multisig, multisig_kex_msg)
signing_skey = rct::rct2sk(rct::skGen());
}
- crypto::secret_key ancillary_skey = rct::rct2sk(rct::skGen());
- while (ancillary_skey == crypto::null_skey)
- ancillary_skey = rct::rct2sk(rct::skGen());
+ const crypto::secret_key ancillary_skey{rct::rct2sk(rct::skGen())};
// misc. edge cases
EXPECT_NO_THROW((multisig_kex_msg{}));
@@ -312,8 +352,8 @@ TEST(multisig, multisig_kex_msg)
// test that keys can be recovered if stored in a message and the message's reverse
// round 1
- multisig_kex_msg msg_rnd1{1, signing_skey, std::vector<crypto::public_key>{pubkey1}, ancillary_skey};
- multisig_kex_msg msg_rnd1_reverse{msg_rnd1.get_msg()};
+ const multisig_kex_msg msg_rnd1{1, signing_skey, std::vector<crypto::public_key>{pubkey1}, ancillary_skey};
+ const multisig_kex_msg msg_rnd1_reverse{msg_rnd1.get_msg()};
EXPECT_EQ(msg_rnd1.get_round(), 1);
EXPECT_EQ(msg_rnd1.get_round(), msg_rnd1_reverse.get_round());
EXPECT_EQ(msg_rnd1.get_signing_pubkey(), signing_pubkey);
@@ -324,8 +364,8 @@ TEST(multisig, multisig_kex_msg)
EXPECT_EQ(msg_rnd1.get_msg_privkey(), msg_rnd1_reverse.get_msg_privkey());
// round 2
- multisig_kex_msg msg_rnd2{2, signing_skey, std::vector<crypto::public_key>{pubkey1, pubkey2}, ancillary_skey};
- multisig_kex_msg msg_rnd2_reverse{msg_rnd2.get_msg()};
+ const multisig_kex_msg msg_rnd2{2, signing_skey, std::vector<crypto::public_key>{pubkey1, pubkey2}, ancillary_skey};
+ const multisig_kex_msg msg_rnd2_reverse{msg_rnd2.get_msg()};
EXPECT_EQ(msg_rnd2.get_round(), 2);
EXPECT_EQ(msg_rnd2.get_round(), msg_rnd2_reverse.get_round());
EXPECT_EQ(msg_rnd2.get_signing_pubkey(), signing_pubkey);
diff --git a/utils/python-rpc/framework/wallet.py b/utils/python-rpc/framework/wallet.py
index 01e937627..062166e6b 100644
--- a/utils/python-rpc/framework/wallet.py
+++ b/utils/python-rpc/framework/wallet.py
@@ -524,12 +524,13 @@ class Wallet(object):
}
return self.rpc.send_json_rpc_request(finalize_multisig)
- def exchange_multisig_keys(self, multisig_info, password = ''):
+ def exchange_multisig_keys(self, multisig_info, password = '', force_update_use_with_caution = False):
exchange_multisig_keys = {
'method': 'exchange_multisig_keys',
'params' : {
'multisig_info': multisig_info,
'password': password,
+ 'force_update_use_with_caution': force_update_use_with_caution,
},
'jsonrpc': '2.0',
'id': '0'