Merge pull request #3629 from EOSIO/wallet_pw_to_expose_private_key

Require wallet password when revealing private keys

plugins/wallet_api_plugin/wallet_api_plugin.cpp

@@ -91,7 +91,7 @@ void wallet_api_plugin::plugin_startup() {
        CALL(wallet, wallet_mgr, list_wallets,
             INVOKE_R_V(wallet_mgr, list_wallets), 200),
        CALL(wallet, wallet_mgr, list_keys,
-            INVOKE_R_V(wallet_mgr, list_keys), 200),
+            INVOKE_R_R_R(wallet_mgr, list_keys, std::string, std::string), 200),
        CALL(wallet, wallet_mgr, get_public_keys,
             INVOKE_R_V(wallet_mgr, get_public_keys), 200)
    });

plugins/wallet_plugin/include/eosio/wallet_plugin/wallet.hpp

@@ -91,6 +91,15 @@ class wallet_api
        */
       void    unlock(string password);
 
+      /** Checks the password of the wallet
+       *
+       * Validates the password on a wallet even if the wallet is already unlocked,
+       * throws if bad password given.
+       * @param password the password previously set with \c set_password()
+       * @ingroup Wallet Management
+       */
+      void    check_password(string password);
+
       /** Sets a new password on the wallet.
        *
        * The wallet must be either 'new' or 'unlocked' to

plugins/wallet_plugin/include/eosio/wallet_plugin/wallet_manager.hpp

@@ -69,8 +69,8 @@ public:
    /// @return A list of wallet names with " *" appended if the wallet is unlocked.
    std::vector<std::string> list_wallets();
 
-   /// @return A list of private keys from all unlocked wallets in wif format.
-   map<public_key_type,private_key_type> list_keys();
+   /// @return A list of private keys from a wallet provided password is correct to said wallet
+   map<public_key_type,private_key_type> list_keys(const string& name, const string& pw);
 
    /// @return A set of public keys from all unlocked wallets, use with chain_controller::get_required_keys.
    flat_set<public_key_type> get_public_keys();

plugins/wallet_plugin/wallet.cpp

@@ -327,6 +327,16 @@ void wallet_api::unlock(string password)
 } EOS_RETHROW_EXCEPTIONS(chain::wallet_invalid_password_exception,
                           "Invalid password for wallet: \"${wallet_name}\"", ("wallet_name", get_wallet_filename())) }
 
+void wallet_api::check_password(string password)
+{ try {
+   FC_ASSERT(password.size() > 0);
+   auto pw = fc::sha512::hash(password.c_str(), password.size());
+   vector<char> decrypted = fc::aes_decrypt(pw, my->_wallet.cipher_keys);
+   auto pk = fc::raw::unpack<plain_keys>(decrypted);
+   FC_ASSERT(pk.checksum == pw);
+} EOS_RETHROW_EXCEPTIONS(chain::wallet_invalid_password_exception,
+                          "Invalid password for wallet: \"${wallet_name}\"", ("wallet_name", get_wallet_filename())) }
+
 void wallet_api::set_password( string password )
 {
    if( !is_new() )

plugins/wallet_plugin/wallet_manager.cpp

@@ -95,18 +95,16 @@ std::vector<std::string> wallet_manager::list_wallets() {
    return result;
 }
 
-map<public_key_type,private_key_type> wallet_manager::list_keys() {
+map<public_key_type,private_key_type> wallet_manager::list_keys(const string& name, const string& pw) {
    check_timeout();
-   map<public_key_type,private_key_type> result;
-   for (const auto& i : wallets) {
-      if (!i.second->is_locked()) {
-         const auto& keys = i.second->list_keys();
-         for (const auto& i : keys) {
-            result[i.first] = i.second;
-         }
-      }
-   }
-   return result;
+
+   if (wallets.count(name) == 0)
+      EOS_THROW(chain::wallet_nonexistent_exception, "Wallet not found: ${w}", ("w", name));
+   auto& w = wallets.at(name);
+   if (w->is_locked())
+      EOS_THROW(chain::wallet_locked_exception, "Wallet is locked: ${w}", ("w", name));
+   w->check_password(pw); //throws if bad password
+   return w->list_keys();
 }
 
 flat_set<public_key_type> wallet_manager::get_public_keys() {

programs/cleos/main.cpp

@@ -2115,12 +2115,31 @@ int main( int argc, char** argv ) {
    });
 
    // list keys
-   auto listKeys = wallet->add_subcommand("keys", localized("List of private keys from all unlocked wallets in wif format."), false);
+   auto listKeys = wallet->add_subcommand("keys", localized("List of public keys from all unlocked wallets."), false);
    listKeys->set_callback([] {
       // wait for keosd to come up
       try_port(uint16_t(std::stoi(parse_url(wallet_url).port)), 2000);
 
-      const auto& v = call(wallet_url, wallet_list_keys);
+      const auto& v = call(wallet_url, wallet_public_keys);
+      std::cout << fc::json::to_pretty_string(v) << std::endl;
+   });
+
+   // list private keys
+   auto listPrivKeys = wallet->add_subcommand("private_keys", localized("List of private keys from an unlocked wallet in wif or PVT_R1 format."), false);
+   listPrivKeys->add_option("-n,--name", wallet_name, localized("The name of the wallet to list keys from"), true);
+   listPrivKeys->add_option("--password", wallet_pw, localized("The password returned by wallet create"));
+   listPrivKeys->set_callback([&wallet_name, &wallet_pw] {
+      if( wallet_pw.size() == 0 ) {
+         std::cout << localized("password: ");
+         fc::set_console_echo(false);
+         std::getline( std::cin, wallet_pw, '\n' );
+         fc::set_console_echo(true);
+      }
+      // wait for keosd to come up
+      try_port(uint16_t(std::stoi(parse_url(wallet_url).port)), 2000);
+
+      fc::variants vs = {fc::variant(wallet_name), fc::variant(wallet_pw)};
+      const auto& v = call(wallet_url, wallet_list_keys, vs);
       std::cout << fc::json::to_pretty_string(v) << std::endl;
    });
 

tests/nodeos_run_test.py

@@ -194,7 +194,7 @@ try:
         errorExit("Unexpected wallet list: %s" % (wallets))
 
     Print("Getting wallet keys.")
-    actualKeys=walletMgr.getKeys()
+    actualKeys=walletMgr.getKeys(testWallet)
     expectedkeys=[]
     for account in accounts:
         expectedkeys.append(account.ownerPrivateKey)
@@ -219,7 +219,7 @@ try:
         errorExit("Failed to unlock wallet %s" % (testWallet.name))
 
     Print("Getting wallet keys.")
-    actualKeys=walletMgr.getKeys()
+    actualKeys=walletMgr.getKeys(defproduceraWallet)
     expectedkeys=[defproduceraAccount.ownerPrivateKey]
     noMatch=list(set(expectedkeys) - set(actualKeys))
     if len(noMatch) > 0:

tests/testUtils.py

@@ -1272,17 +1272,17 @@ class WalletMgr(object):
 
         return wallets
 
-    def getKeys(self):
+    def getKeys(self, wallet):
         keys=[]
 
         p = re.compile(r'\n\s+\"(\w+)\"\n', re.MULTILINE)
-        cmd="%s %s wallet keys" % (Utils.EosClientPath, self.endpointArgs)
+        cmd="%s %s wallet private_keys --name %s --password %s " % (Utils.EosClientPath, self.endpointArgs, wallet.name, wallet.password)
         if Utils.Debug: Utils.Print("cmd: %s" % (cmd))
         retStr=subprocess.check_output(cmd.split()).decode("utf-8")
         #Utils.Print("retStr: %s" % (retStr))
         m=p.findall(retStr)
         if m is None:
-            Utils.Print("ERROR: wallet keys parser failure")
+            Utils.Print("ERROR: wallet private_keys parser failure")
             return None
         keys=m
 

tests/wallet_tests.cpp

@@ -16,7 +16,6 @@ namespace eosio {
 
 BOOST_AUTO_TEST_SUITE(wallet_tests)
 
-
 /// Test creating the wallet
 BOOST_AUTO_TEST_CASE(wallet_test)
 { try {
@@ -82,7 +81,7 @@ BOOST_AUTO_TEST_CASE(wallet_manager_test)
 
    wallet_manager wm;
    BOOST_CHECK_EQUAL(0, wm.list_wallets().size());
-   BOOST_CHECK_EQUAL(0, wm.list_keys().size());
+   BOOST_CHECK_THROW(wm.get_public_keys(), wallet_not_available_exception);
    BOOST_CHECK_NO_THROW(wm.lock_all());
 
    BOOST_CHECK_THROW(wm.lock("test"), fc::exception);
@@ -94,7 +93,8 @@ BOOST_AUTO_TEST_CASE(wallet_manager_test)
    BOOST_CHECK_EQUAL(0, pw.find("PW")); // starts with PW
    BOOST_CHECK_EQUAL(1, wm.list_wallets().size());
    // eosio key is imported automatically when a wallet is created
-   BOOST_CHECK_EQUAL(1, wm.list_keys().size());
+   BOOST_CHECK_EQUAL(1, wm.get_public_keys().size());
+   BOOST_CHECK_EQUAL(1, wm.list_keys("test", pw).size());
    BOOST_CHECK(wm.list_wallets().at(0).find("*") != std::string::npos);
    wm.lock("test");
    BOOST_CHECK(wm.list_wallets().at(0).find("*") == std::string::npos);
@@ -102,8 +102,8 @@ BOOST_AUTO_TEST_CASE(wallet_manager_test)
    BOOST_CHECK_THROW(wm.unlock("test", pw), chain::wallet_unlocked_exception);
    BOOST_CHECK(wm.list_wallets().at(0).find("*") != std::string::npos);
    wm.import_key("test", key1);
-   BOOST_CHECK_EQUAL(2, wm.list_keys().size());
-   auto keys = wm.list_keys();
+   BOOST_CHECK_EQUAL(2, wm.get_public_keys().size());
+   auto keys = wm.list_keys("test", pw);
 
    auto pub_pri_pair = [](const char *key) -> auto {
        private_key_type prikey = private_key_type(std::string(key));
@@ -113,33 +113,40 @@ BOOST_AUTO_TEST_CASE(wallet_manager_test)
    BOOST_CHECK(std::find(keys.cbegin(), keys.cend(), pub_pri_pair(key1)) != keys.cend());
 
    wm.import_key("test", key2);
-   keys = wm.list_keys();
+   keys = wm.list_keys("test", pw);
    BOOST_CHECK(std::find(keys.cbegin(), keys.cend(), pub_pri_pair(key1)) != keys.cend());
    BOOST_CHECK(std::find(keys.cbegin(), keys.cend(), pub_pri_pair(key2)) != keys.cend());
    // key3 was automatically imported
    BOOST_CHECK(std::find(keys.cbegin(), keys.cend(), pub_pri_pair(key3)) != keys.cend());
    wm.lock("test");
-   BOOST_CHECK_EQUAL(0, wm.list_keys().size());
+   BOOST_CHECK_THROW(wm.list_keys("test", pw), wallet_locked_exception);
+   BOOST_CHECK_THROW(wm.get_public_keys(), wallet_locked_exception);
    wm.unlock("test", pw);
-   BOOST_CHECK_EQUAL(3, wm.list_keys().size());
+   BOOST_CHECK_EQUAL(3, wm.get_public_keys().size());
+   BOOST_CHECK_EQUAL(3, wm.list_keys("test", pw).size());
    wm.lock_all();
-   BOOST_CHECK_EQUAL(0, wm.list_keys().size());
+   BOOST_CHECK_THROW(wm.get_public_keys(), wallet_locked_exception);
    BOOST_CHECK(wm.list_wallets().at(0).find("*") == std::string::npos);
 
    auto pw2 = wm.create("test2");
    BOOST_CHECK_EQUAL(2, wm.list_wallets().size());
    // eosio key is imported automatically when a wallet is created
-   BOOST_CHECK_EQUAL(1, wm.list_keys().size());
+   BOOST_CHECK_EQUAL(1, wm.get_public_keys().size());
    BOOST_CHECK_THROW(wm.import_key("test2", key3), fc::exception);
-   keys = wm.list_keys();
+   keys = wm.list_keys("test2", pw2);
    BOOST_CHECK(std::find(keys.cbegin(), keys.cend(), pub_pri_pair(key1)) == keys.cend());
    BOOST_CHECK(std::find(keys.cbegin(), keys.cend(), pub_pri_pair(key2)) == keys.cend());
    BOOST_CHECK(std::find(keys.cbegin(), keys.cend(), pub_pri_pair(key3)) != keys.cend());
    wm.unlock("test", pw);
-   keys = wm.list_keys();
+   keys = wm.list_keys("test", pw);
+   auto keys2 = wm.list_keys("test2", pw2);
+   keys.insert(keys2.begin(), keys2.end());
    BOOST_CHECK(std::find(keys.cbegin(), keys.cend(), pub_pri_pair(key1)) != keys.cend());
    BOOST_CHECK(std::find(keys.cbegin(), keys.cend(), pub_pri_pair(key2)) != keys.cend());
    BOOST_CHECK(std::find(keys.cbegin(), keys.cend(), pub_pri_pair(key3)) != keys.cend());
+   BOOST_CHECK_EQUAL(3, keys.size());
+
+   BOOST_CHECK_THROW(wm.list_keys("test2", "PWnogood"), wallet_invalid_password_exception);
 
    private_key_type pkey1{std::string(key1)};
    private_key_type pkey2{std::string(key2)};
@@ -158,9 +165,10 @@ BOOST_AUTO_TEST_CASE(wallet_manager_test)
    BOOST_CHECK(find(pks.cbegin(), pks.cend(), pkey2.get_public_key()) != pks.cend());
    BOOST_CHECK(find(pks.cbegin(), pks.cend(), pkey3.get_public_key()) != pks.cend());
 
-   BOOST_CHECK_EQUAL(3, wm.list_keys().size());
+   BOOST_CHECK_EQUAL(3, wm.get_public_keys().size());
    wm.set_timeout(chrono::seconds(0));
-   BOOST_CHECK_EQUAL(0, wm.list_keys().size());
+   BOOST_CHECK_THROW(wm.get_public_keys(), wallet_locked_exception);
+   BOOST_CHECK_THROW(wm.list_keys("test", pw), wallet_locked_exception);
 
    wm.set_timeout(chrono::seconds(15));
 
@@ -168,7 +176,7 @@ BOOST_AUTO_TEST_CASE(wallet_manager_test)
 
    const string test_key_create_types[] = {"K1", "R1", "k1", ""};
    for(const string& key_type_to_create : test_key_create_types) {
-      wm.create("testgen");
+      string pw = wm.create("testgen");
 
       //check that the public key returned looks legit through a string conversion
       // (would throw otherwise)
@@ -176,7 +184,7 @@ BOOST_AUTO_TEST_CASE(wallet_manager_test)
 
       //now pluck out the private key from the wallet and see if the public key of said
       // private key matches what was returned earlier from the create_key() call
-      private_key_type create_key_priv(wm.list_keys().cbegin()->second);
+      private_key_type create_key_priv(wm.list_keys("testgen", pw).cbegin()->second);
       BOOST_CHECK_EQUAL((string)create_key_pub, (string)create_key_priv.get_public_key());
 
       wm.lock("testgen");
@@ -196,7 +204,6 @@ BOOST_AUTO_TEST_CASE(wallet_manager_test)
 } FC_LOG_AND_RETHROW() }
 
 
-
 BOOST_AUTO_TEST_SUITE_END()
 
 } // namespace eos