From b6336bd422f1200db8ef0cf44d685212512a402b Mon Sep 17 00:00:00 2001 From: riordant Date: Sun, 14 Oct 2018 14:54:08 +0200 Subject: [PATCH 1/5] Add warning before dumpwallet is exexecuted --- src/wallet/rpcdump.cpp | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index c3061bc628..1ca74b011e 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -572,6 +572,17 @@ UniValue dumpwallet(const UniValue& params, bool fHelp) EnsureWalletIsUnlocked(); + const char* warning = "WARNING! This command prints all your private keys. Anyone with these private keys has complete control of your funds. " + "If anyone is requesting for any of the info or output from this dumpwallet command, chances are high that they are scammers. " + "This command is never needed for any Znode setup. Continue? (Y/n)"; + + printf("%s\n", warning); + char choice; + cin >> choice; + if(choice !='Y'){ + return NullUniValue; + } + ofstream file; file.open(params[0].get_str().c_str()); if (!file.is_open()) From df855fc58264580c3204ac235256af62531cf626 Mon Sep 17 00:00:00 2001 From: riordant Date: Mon, 15 Oct 2018 13:50:51 +0200 Subject: [PATCH 2/5] Add wallet warning argument requirement --- src/wallet/rpcdump.cpp | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index 1ca74b011e..c9d85e752a 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -556,22 +556,30 @@ UniValue dumpwallet(const UniValue& params, bool fHelp) { if (!EnsureWalletIsAvailable(fHelp)) return NullUniValue; + + std::string warningInput = "I'M-AWARE-THAT-THIS-FILE-CONTAINS-KEYS-FOR-ALL-MY-FUNDS-AND-I-CAN-LOSE-THEM-IF-SHARED-WITH-A-3RD-PARTY"; - if (fHelp || params.size() != 1) + if (fHelp || params.size() != 2) throw runtime_error( "dumpwallet \"filename\"\n" "\nDumps all wallet keys in a human-readable format.\n" "\nArguments:\n" "1. \"filename\" (string, required) The filename\n" + "1. \"warning\" (string, required) The warning string. Must be exactly equal to: \"" + warningInput + "\"\n" "\nExamples:\n" - + HelpExampleCli("dumpwallet", "\"test\"") - + HelpExampleRpc("dumpwallet", "\"test\"") + + HelpExampleCli("dumpwallet", "\"test\" \"" + warningInput + "\"") + + HelpExampleRpc("dumpwallet", "\"test\", \"" + warningInput + "\"") ); LOCK2(cs_main, pwalletMain->cs_wallet); EnsureWalletIsUnlocked(); + + if(params[1].get_str() != warningInput){ + return "Warning inputted incorrectly. You must enter the warning shown by running this command without any arguments."; + } + const char* warning = "WARNING! This command prints all your private keys. Anyone with these private keys has complete control of your funds. " "If anyone is requesting for any of the info or output from this dumpwallet command, chances are high that they are scammers. " "This command is never needed for any Znode setup. Continue? (Y/n)"; From 942761beb949b289e66e2015e5b1aef24682dd70 Mon Sep 17 00:00:00 2001 From: riordant Date: Mon, 15 Oct 2018 20:29:20 +0200 Subject: [PATCH 3/5] Update warning msg --- src/wallet/rpcdump.cpp | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index c9d85e752a..7ddcfdf007 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -580,9 +580,16 @@ UniValue dumpwallet(const UniValue& params, bool fHelp) return "Warning inputted incorrectly. You must enter the warning shown by running this command without any arguments."; } - const char* warning = "WARNING! This command prints all your private keys. Anyone with these private keys has complete control of your funds. " - "If anyone is requesting for any of the info or output from this dumpwallet command, chances are high that they are scammers. " - "This command is never needed for any Znode setup. Continue? (Y/n)"; + const char* warning = + "WARNING! This command exports all your private keys. Anyone with these keys has complete control over your funds. \n" + "If someone asked you to type in this command, chances are they want to steal your coins. \n" + "Zcoin team members will never ask for this command's output and it is not needed for Znode setup or diagnosis!\n" + "\n" + " Please seek help on one of our public channels. \n" + " Telegram: https://t.me/zcoinproject \n" + " Discord: https://discordapp.com/invite/4FjnQ2q\n" + " Reddit: https://www.reddit.com/r/zcoin/\n" + " Continue? (Y/n)"; printf("%s\n", warning); char choice; From f96fa8d0f8c3b21d7c9f1ca0be9cd6ba92743730 Mon Sep 17 00:00:00 2001 From: Andrey Bezrukov Date: Wed, 17 Oct 2018 19:33:36 +0400 Subject: [PATCH 4/5] Implemented 2-step auth of dumpprivkey and dumpwallet --- src/Makefile.am | 1 + src/rpc/server.cpp | 2 +- src/wallet/authhelper.cpp | 101 +++++++++++++++++++++++ src/wallet/authhelper.h | 25 ++++++ src/wallet/rpcdump.cpp | 119 ++++++++++++++++++++------- src/wallet/rpcwallet.cpp | 8 +- src/wallet/test/rpc_wallet_tests.cpp | 15 ++++ 7 files changed, 237 insertions(+), 34 deletions(-) create mode 100644 src/wallet/authhelper.cpp create mode 100644 src/wallet/authhelper.h diff --git a/src/Makefile.am b/src/Makefile.am index da98430ac3..89b1fd54d5 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -281,6 +281,7 @@ libbitcoin_wallet_a_SOURCES = \ wallet/rpcwallet.cpp \ wallet/wallet.cpp \ wallet/walletdb.cpp \ + wallet/authhelper.cpp \ policy/rbf.cpp \ $(BITCOIN_CORE_H) diff --git a/src/rpc/server.cpp b/src/rpc/server.cpp index 0e08dbceba..927fd49268 100644 --- a/src/rpc/server.cpp +++ b/src/rpc/server.cpp @@ -472,7 +472,7 @@ std::vector CRPCTable::listCommands() const std::string HelpExampleCli(const std::string& methodname, const std::string& args) { - return "> bitcoin-cli " + methodname + " " + args + "\n"; + return "> zcoin-cli " + methodname + " " + args + "\n"; } std::string HelpExampleRpc(const std::string& methodname, const std::string& args) diff --git a/src/wallet/authhelper.cpp b/src/wallet/authhelper.cpp new file mode 100644 index 0000000000..082fce12d6 --- /dev/null +++ b/src/wallet/authhelper.cpp @@ -0,0 +1,101 @@ +#include +#include +#include + +#include "base58.h" +#include "authhelper.h" + +struct AuthorizationHelper::Impl +{ + using rnd_type = size_t; + using code_type = std::string; + Impl() + : seed() + , generator(seed()) + , rnd_dist(std::numeric_limits::min(), std::numeric_limits::max()) + {} + + bool authorize(code_type const & functionName, code_type code) + { + auto const fn_key_iter = key_store.find(functionName); + if(fn_key_iter == key_store.end()) + { + generateAuthorizationCode(functionName); + return false; + } + + if(code == toCode(fn_key_iter->second)) + { + generateAuthorizationCode(functionName); + return true; + } + + return false; + } + + code_type generateAuthorizationCode(code_type const & functionName) + { + rnd_type key = rnd_dist(generator); + key_store[functionName] = key; + return toCode(key); + } + +private: + std::random_device seed; + std::mt19937 generator; + std::uniform_int_distribution rnd_dist; + std::map key_store; + + code_type toCode(rnd_type code) + { + static_assert(true == std::is_standard_layout::value, "This method can only work with standard_layout data."); + std::string temp = EncodeBase58(reinterpret_cast(&code), reinterpret_cast(&code) + sizeof(code)); + + static std::vector const pick_positions = {13, 17, 19, 23}; + + code_type result; + + size_t position = 0; + + for(auto pick_position : pick_positions) + { + position += pick_position; + position %= temp.size(); + result.push_back(temp[position]); + } + + return result; + } +}; + + +AuthorizationHelper & AuthorizationHelper::inst() +{ + static AuthorizationHelper inst; + return inst; +} + + +AuthorizationHelper::AuthorizationHelper() +: pImpl(*new Impl) +{ +} + + +AuthorizationHelper::~AuthorizationHelper() +{ + delete &pImpl; +} + + +bool AuthorizationHelper::authorize(std::string const & function_name, std::string const & auth_code) +{ + return pImpl.authorize(function_name, auth_code); +} + + +std::string AuthorizationHelper::generateAuthorizationCode(std::string const & function_name) +{ + return pImpl.generateAuthorizationCode(function_name); +} + diff --git a/src/wallet/authhelper.h b/src/wallet/authhelper.h new file mode 100644 index 0000000000..938beeb2e2 --- /dev/null +++ b/src/wallet/authhelper.h @@ -0,0 +1,25 @@ +#ifndef AUTHHELPER_H +#define AUTHHELPER_H + +/** + * Generates and manages one-time authorization keys + * for sensitive + */ +class AuthorizationHelper { +public: + static AuthorizationHelper & inst(); + + bool authorize(std::string const & function_name, std::string const & auth_code); + std::string generateAuthorizationCode(std::string const & function_name); +private: + AuthorizationHelper(); + ~AuthorizationHelper(); + AuthorizationHelper(const AuthorizationHelper&) = delete; + AuthorizationHelper& operator=(const AuthorizationHelper&) = delete; + + struct Impl; + Impl & pImpl; +}; + +#endif /* AUTHHELPER_H */ + diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index 7ddcfdf007..a6a4fa32ac 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -15,6 +15,7 @@ #include "wallet.h" #include "merkleblock.h" #include "core_io.h" +#include "authhelper.h" #include #include @@ -514,6 +515,7 @@ UniValue importwallet(const UniValue& params, bool fHelp) return NullUniValue; } +//This method intentionally left unchanged. UniValue dumpprivkey(const UniValue& params, bool fHelp) { if (!EnsureWalletIsAvailable(fHelp)) @@ -551,53 +553,71 @@ UniValue dumpprivkey(const UniValue& params, bool fHelp) return CBitcoinSecret(vchSecret).ToString(); } +UniValue dumpprivkey_zcoin(const UniValue& params, bool fHelp) +{ + if (fHelp || params.size() < 1 || params.size() > 2) + throw runtime_error( + "dumpprivkey \"bitcoinaddress\"\n" + "\nReveals the private key corresponding to 'bitcoinaddress'.\n" + "Then the importprivkey can be used with this output\n" + "\nArguments:\n" + "1. \"bitcoinaddress\" (string, required) The bitcoin address for the private key\n" + "2. \"one-time-auth-code\" (string, optional) A one time authorization code received from a previous call of dumpprivkey" + "\nResult:\n" + "\"key\" (string) The private key\n" + "\nExamples:\n" + + HelpExampleCli("dumpprivkey", "\"myaddress\"") + + HelpExampleCli("dumpprivkey", "\"myaddress\" \"12aB\"") + + HelpExampleRpc("dumpprivkey", "\"myaddress\"") + + HelpExampleRpc("dumpprivkey", "\"myaddress\",\"12aB\"") + + HelpExampleCli("importprivkey", "\"mykey\"") + ); + + if(params.size() == 1 || !AuthorizationHelper::inst().authorize(__FUNCTION__ + params[0].get_str(), params[1].get_str())) + { + std::string warning = + "WARNING! Your one time authorization code is: " + AuthorizationHelper::inst().generateAuthorizationCode(__FUNCTION__ + params[0].get_str()) + "\n" + "This command exports your wallet private key. Anyone with this key has complete control over your funds. \n" + "If someone asked you to type in this command, chances are they want to steal your coins. \n" + "Zcoin team members will never ask for this command's output and it is not needed for Znode setup or diagnosis!\n" + "\n" + " Please seek help on one of our public channels. \n" + " Telegram: https://t.me/zcoinproject \n" + " Discord: https://discordapp.com/invite/4FjnQ2q\n" + " Reddit: https://www.reddit.com/r/zcoin/\n" + "\n" + ; + throw runtime_error(warning); + } + UniValue dumpParams; + dumpParams.setArray(); + dumpParams.push_back(params[0]); + + return dumpprivkey(dumpParams, false); +} + +//This method intentionally left unchanged. UniValue dumpwallet(const UniValue& params, bool fHelp) { if (!EnsureWalletIsAvailable(fHelp)) return NullUniValue; - std::string warningInput = "I'M-AWARE-THAT-THIS-FILE-CONTAINS-KEYS-FOR-ALL-MY-FUNDS-AND-I-CAN-LOSE-THEM-IF-SHARED-WITH-A-3RD-PARTY"; - - if (fHelp || params.size() != 2) + if (fHelp || params.size() != 1) throw runtime_error( "dumpwallet \"filename\"\n" "\nDumps all wallet keys in a human-readable format.\n" "\nArguments:\n" "1. \"filename\" (string, required) The filename\n" - "1. \"warning\" (string, required) The warning string. Must be exactly equal to: \"" + warningInput + "\"\n" "\nExamples:\n" - + HelpExampleCli("dumpwallet", "\"test\" \"" + warningInput + "\"") - + HelpExampleRpc("dumpwallet", "\"test\", \"" + warningInput + "\"") + + HelpExampleCli("dumpwallet", "\"test\"") + + HelpExampleRpc("dumpwallet", "\"test\"") ); LOCK2(cs_main, pwalletMain->cs_wallet); EnsureWalletIsUnlocked(); - - if(params[1].get_str() != warningInput){ - return "Warning inputted incorrectly. You must enter the warning shown by running this command without any arguments."; - } - - const char* warning = - "WARNING! This command exports all your private keys. Anyone with these keys has complete control over your funds. \n" - "If someone asked you to type in this command, chances are they want to steal your coins. \n" - "Zcoin team members will never ask for this command's output and it is not needed for Znode setup or diagnosis!\n" - "\n" - " Please seek help on one of our public channels. \n" - " Telegram: https://t.me/zcoinproject \n" - " Discord: https://discordapp.com/invite/4FjnQ2q\n" - " Reddit: https://www.reddit.com/r/zcoin/\n" - " Continue? (Y/n)"; - - printf("%s\n", warning); - char choice; - cin >> choice; - if(choice !='Y'){ - return NullUniValue; - } - ofstream file; file.open(params[0].get_str().c_str()); if (!file.is_open()) @@ -665,3 +685,44 @@ UniValue dumpwallet(const UniValue& params, bool fHelp) file.close(); return NullUniValue; } + +UniValue dumpwallet_zcoin(const UniValue& params, bool fHelp) +{ + if (fHelp || params.size() < 1 || params.size() > 2) + throw runtime_error( + "dumpwallet \"filename\"\n" + "\nDumps all wallet keys in a human-readable format.\n" + "\nArguments:\n" + "1. \"filename\" (string, required) The filename\n" + "2. \"one-time-auth-code\" (string, optional) A one time authorization code received from a previous call of dumpwallet" + "\nExamples:\n" + + HelpExampleCli("dumpwallet", "\"test\"") + + HelpExampleCli("dumpwallet", "\"test\" \"12aB\"") + + HelpExampleRpc("dumpwallet", "\"test\"") + + HelpExampleRpc("dumpwallet", "\"test\",\"12aB\"") + ); + + if(params.size() == 1 || !AuthorizationHelper::inst().authorize(__FUNCTION__ + params[0].get_str(), params[1].get_str())) + { + std::string warning = + "WARNING! Your one time authorization code is: " + AuthorizationHelper::inst().generateAuthorizationCode(__FUNCTION__ + params[0].get_str()) + "\n" + "This command exports all your private keys. Anyone with these keys has complete control over your funds. \n" + "If someone asked you to type in this command, chances are they want to steal your coins. \n" + "Zcoin team members will never ask for this command's output and it is not needed for Znode setup or diagnosis!\n" + "\n" + " Please seek help on one of our public channels. \n" + " Telegram: https://t.me/zcoinproject \n" + " Discord: https://discordapp.com/invite/4FjnQ2q\n" + " Reddit: https://www.reddit.com/r/zcoin/\n" + "\n" + ; + throw runtime_error(warning); + } + + UniValue dumpParams; + dumpParams.setArray(); + dumpParams.push_back(params[0]); + + return dumpwallet(dumpParams, false); +} + diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 19552d29a9..8ef283556f 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -3318,11 +3318,11 @@ UniValue removetxwallet(const UniValue& params, bool fHelp) { -extern UniValue dumpprivkey(const UniValue& params, bool fHelp); // in rpcdump.cpp +extern UniValue dumpprivkey_zcoin(const UniValue& params, bool fHelp); // in rpcdump.cpp extern UniValue importprivkey(const UniValue& params, bool fHelp); extern UniValue importaddress(const UniValue& params, bool fHelp); extern UniValue importpubkey(const UniValue& params, bool fHelp); -extern UniValue dumpwallet(const UniValue& params, bool fHelp); +extern UniValue dumpwallet_zcoin(const UniValue& params, bool fHelp); extern UniValue importwallet(const UniValue& params, bool fHelp); extern UniValue importprunedfunds(const UniValue& params, bool fHelp); extern UniValue removeprunedfunds(const UniValue& params, bool fHelp); @@ -3336,8 +3336,8 @@ static const CRPCCommand commands[] = { "wallet", "addmultisigaddress", &addmultisigaddress, true }, { "wallet", "addwitnessaddress", &addwitnessaddress, true }, { "wallet", "backupwallet", &backupwallet, true }, - { "wallet", "dumpprivkey", &dumpprivkey, true }, - { "wallet", "dumpwallet", &dumpwallet, true }, + { "wallet", "dumpprivkey", &dumpprivkey_zcoin, true }, + { "wallet", "dumpwallet", &dumpwallet_zcoin, true }, { "wallet", "encryptwallet", &encryptwallet, true }, { "wallet", "getaccountaddress", &getaccountaddress, true }, { "wallet", "getaccount", &getaccount, true }, diff --git a/src/wallet/test/rpc_wallet_tests.cpp b/src/wallet/test/rpc_wallet_tests.cpp index 4e7d177f51..6d0ce74c2c 100644 --- a/src/wallet/test/rpc_wallet_tests.cpp +++ b/src/wallet/test/rpc_wallet_tests.cpp @@ -10,6 +10,7 @@ #include "wallet/wallet.h" #include "wallet/test/wallet_test_fixture.h" +#include "wallet/authhelper.h" #include #include @@ -226,4 +227,18 @@ BOOST_AUTO_TEST_CASE(rpc_wallet) BOOST_CHECK_THROW(CallRPC("fundrawtransaction 01000000000180969800000000001976a91450ce0a4b0ee0ddeb633da85199728b940ac3fe9488ac00000000"), runtime_error); } +//This code is disabled for now. Anyway, this integrity test can be copied somewhere. +BOOST_AUTO_TEST_CASE(rpc_wallet_authorization) +{ + auto & ah = AuthorizationHelper::inst(); + std::string code = ah.generateAuthorizationCode("function"); + BOOST_CHECK(true == ah.authorize("function", code)); + + code = ah.generateAuthorizationCode("function"); + ++code[0]; + BOOST_CHECK(false == ah.authorize("function", code)); + --code[0]; + BOOST_CHECK(true == ah.authorize("function", code)); +} + BOOST_AUTO_TEST_SUITE_END() From faa806028bfab8c230e088423322fdb87935dc0a Mon Sep 17 00:00:00 2001 From: Andrey Bezrukov Date: Thu, 18 Oct 2018 12:12:34 +0400 Subject: [PATCH 5/5] Avoided the random_device mingw issue --- src/Makefile.am | 1 + src/wallet/authhelper.cpp | 12 +++++++----- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index 89b1fd54d5..356f1383da 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -194,6 +194,7 @@ BITCOIN_CORE_H = \ wallet/rpcwallet.h \ wallet/wallet.h \ wallet/walletdb.h \ + wallet/authhelper.h \ definition.h \ zmq/zmqabstractnotifier.h \ zmq/zmqconfig.h\ diff --git a/src/wallet/authhelper.cpp b/src/wallet/authhelper.cpp index 082fce12d6..186ad75231 100644 --- a/src/wallet/authhelper.cpp +++ b/src/wallet/authhelper.cpp @@ -4,16 +4,19 @@ #include "base58.h" #include "authhelper.h" +#include "wallet.h" +#include "main.h" struct AuthorizationHelper::Impl { using rnd_type = size_t; using code_type = std::string; Impl() - : seed() - , generator(seed()) - , rnd_dist(std::numeric_limits::min(), std::numeric_limits::max()) - {} + : rnd_dist(std::numeric_limits::min(), std::numeric_limits::max()) + { + LOCK2(cs_main, pwalletMain->cs_wallet); + generator.seed(pwalletMain->GetHDChain().masterKeyID.GetUint64(0) ^ time(NULL)); + } bool authorize(code_type const & functionName, code_type code) { @@ -41,7 +44,6 @@ struct AuthorizationHelper::Impl } private: - std::random_device seed; std::mt19937 generator; std::uniform_int_distribution rnd_dist; std::map key_store;