cli: encryptwallet password entered from stdin. fixes #15318

[darosior]

This PR adds the possibility to enter the password from 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).

[jonasschnelli]

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

[kristapsk]

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.

[darosior]

Yes, I wanted to add the other commands once this one is accepted, in order to do one thing at a time.

[laanwj]

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.

[kristapsk]

I agree with @laanwj, less time spent on testing and code review.

[promag]

One way to avoid client side knowledge of these commands is to support special arguments that are replaced after reading the stdin. For instance:

# before
bitcoin-cli encryptwallet myawesomepass

# after
bitcoin-cli encryptwallet -p
Enter password: *************
Verify password: *************

When -p is detected it is replaced after double reading the password from stdin (ideally without echoing).

[kristapsk]

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:

$ bitcoin-cli -p encryptwallet
Enter password: ***
Verify password: ***
$ bitcoin-cli -p walletpassphrasechange
Enter current password: ***
Enter new password: ***
Verify new password: ***
$ bitcoin-cli -p walletpassphrase 60
Enter password: ***

I am also not a fan of special handling for some RPCs in client, but I think benefits are worth it here.

[darosior]

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:

# before
bitcoin-cli encryptwallet myawesomepass

# after
bitcoin-cli encryptwallet -p
Enter password: *************
Verify password: *************

When -p is 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.

[promag]

Have you seen #13716?

[darosior]

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.

[darosior]

I added a commit to change the behavior of walletpassphrase and walletpassphrasechange too.

./src/bitcoin-cli encryptwallet
Enter passphrase to encrypt the wallet : test
Re-enter passphrase : test

./src/bitcoin-cli walletpassphrasechange
Enter current passphrase : test
Enter new passphrase : testt
Repeat new passphrase : testt

./src/bitcoin-cli walletpassphrase 100
Enter passphrase to unlock the wallet : testt

[kristapsk]

Tested this and #13716 and I like this approach better, apart from no-echo password part from by @kallewoof, which would be cool have here too.

[darosior]

Closing in favor of #13716