encryptwallet to be entered from stdin, as discussed in #15318.
I know it can be done using the -stdin parameter from , but I think entering the password from stdin should become the default for encryptwallet (as well as for walletpassphrase) as it can be retrieved easily (grep encryptwallet ~/.bash_history).
I dislike the fact that bitcoin-cli does have insight into specific commands and argument positions, etc.
Nevertheless I think this is an improvement.
Concept ACK
Concept ACK
If we should do special handling for encryptwallet, I think something similar should be done with walletpassphrase and walletpassphrasechange as well. Especially walletpassphrase, which is most often used from these three.
Yes, I wanted to add the other commands once this one is accepted, in order to do one thing at a time.
I’d prefer to do this all at once, if we’re going to do this.
One way to avoid client side knowledge of these commands is to support special arguments that are replaced after reading the stdin. For instance:
0# before
1bitcoin-cli encryptwallet myawesomepass
2
3# after
4bitcoin-cli encryptwallet -p
5Enter password: *************
6Verify password: *************
When -p is detected it is replaced after double reading the password from stdin (ideally without echoing).
One way to avoid client side knowledge of these commands is to support special arguments that are replaced after reading the stdin.
Problem is these three RPCs differ - with encryptwallet you would expect being asked for password twice to verify they match, with walletpassphrasechange you expect to be asked first for the old password and then twice for the new password, with walletpassphrase you would expect being asked for a password just once.
Using -p as an universal replacement for any argument with meaning “ask to enter value using password prompt” also would not work, because it will not allow to use “-p” as a string in arguments, so will break compatibilty. And, as mentioned above, there are RPC arguments where you would expect being asked for password just once, and others where you are expected to being asked twice.
So, I don’t see how we can work around special handling for these three there (ok, without introducing some obscure syntax that no one would use, like “-passarg=3,2” before arguments = ask for a third argument with a password prompt, requiring entering it twice).
But -p (or some other name, other bitcoin-cli arguments are long ones, not single letter) as an flag to do special handling for these three commands looks ok to me, examples could be:
0$ bitcoin-cli -p encryptwallet
1Enter password: ***
2Verify password: ***
3$ bitcoin-cli -p walletpassphrasechange
4Enter current password: ***
5Enter new password: ***
6Verify new password: ***
7$ bitcoin-cli -p walletpassphrase 60
8Enter password: ***
I am also not a fan of special handling for some RPCs in client, but I think benefits are worth it here.
448+ if(!std::getline(std::cin, password)) {
449+ throw std::runtime_error("Failed to read password");
450+ }
451+ fprintf(stdout, "Re-enter password : ");
452+ if(!std::getline(std::cin, verification) || password != verification) {
453+ throw std::runtime_error("Passwords do not match");
Yes, I wanted to add the other commands once this one is accepted, in order to do one thing at a time.
I’d prefer to do this all at once, if we’re going to do this.
Ok, I’ll commit changes for the other commands.
One way to avoid client side knowledge of these commands is to support special arguments that are replaced after reading the stdin. For instance:
0# before 1bitcoin-cli encryptwallet myawesomepass 2 3# after 4bitcoin-cli encryptwallet -p 5Enter password: ************* 6Verify password: *************When
-pis detected it is replaced after double reading the password from stdin (ideally without echoing).
This is similar to using -stdin, as it would requires to add an argument. What I propose is to use stdin for passwords in bitcoin-cli as a default : I have seen some people “securising their wallet” by using encryptwallet the default way just because they didn’t know about -stdin, nor they bother to search, and I could retrieve their pass from the bash_history.
About the echoing, I have not found a clean and not too verbose way to achieve it. Maybe now that 3 commands will be covered it worth adding a function (kind of getpass instead of getline) that would not echo the password.
Have you seen #13716?
Yes I have, and I still think it is more convenient for the user to change the behavior of the existing commands instead of adding another. However the two PR are not incompatible and not echoing the password would be nice.
I added a commit to change the behavior of walletpassphrase and walletpassphrasechange too.
0./src/bitcoin-cli encryptwallet
1Enter passphrase to encrypt the wallet : test
2Re-enter passphrase : test
3
4./src/bitcoin-cli walletpassphrasechange
5Enter current passphrase : test
6Enter new passphrase : testt
7Repeat new passphrase : testt
8
9./src/bitcoin-cli walletpassphrase 100
10Enter passphrase to unlock the wallet : testt
440@@ -441,6 +441,45 @@ static int CommandLineRPC(int argc, char *argv[])
441 method = args[0];
442 args.erase(args.begin()); // Remove trailing method name from arguments vector
443 }
444+ // Get wallet encryption passphrases from command line
445+ if (method == "encryptwallet" && !args.size()) {
args.empty() is more idiomatic?
461+ if(!std::getline(std::cin, passphrase)) {
462+ throw std::runtime_error("Failed to read the passphrase");
463+ }
464+ args.insert(args.begin(), passphrase); // Second parameter is timeout
465+ }
466+ else if (method == "walletpassphrasechange" && !args.size()) {