bitcoin-cli: -stdinwalletpassphrase and non-echo stdin passwords by kallewoof · Pull Request #13716 · bitcoin/bitcoin

kallewoof · 2018-07-19T14:52:07Z

This PR

adds -stdinwalletpassphrase for use with walletpasshprase(change)
adds no-echo for passwords (-stdinrpcpass and above)

It may not be ideal, but it's better than having to clear the screen whenever you unlock the wallet.

practicalswift · 2018-07-19T20:29:27Z

Concept ACK

DrahtBot · 2018-07-22T13:22:22Z

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

No conflicts as of last run.

hebasto · 2018-08-05T08:47:07Z

There are conflicting pull requests:

this one adds new features and, as a side effect, fixes one of two bugs in bitcoin-cli --help output;
#13879 fixes all found bugs in bitcoin-cli --help output and is safe for the whole code.

So, #13879 can be merged regardless of this pull request.

laanwj · 2018-09-10T16:37:32Z

src/util.cpp

As this is only used in bitcoin-cli, it doesn't need to be in util.cpp (which is shared between the server and client), would be better in a client-specific utility, e.g. compat/noecho.{cpp,h} or something like that, that is only linked into -cli.

Good point. Fixing.

laanwj · 2018-09-10T16:38:35Z

src/util.cpp

is this generally supported on all UNIX, or does it need checks in the build system?

termios.h? It seems to be pretty standard FWICT.

going to test on FreeBSD and OpenBSD

laanwj · 2018-09-10T16:40:06Z

Concept ACK

laanwj · 2018-09-11T09:49:25Z

src/bitcoin-cli.cpp

need to re-enable echoing before the normal -stdin processing

make sure that this is not called unless either -stdinrpcpass or -stdinwalletpassphrase is called to avoid interfering with scripts that pipe in data, not attached to a TTY

Good points. Addressed both.

kallewoof · 2019-02-13T06:01:39Z

@Sjors

Nit: return \n after password entry.

Hitting enter adds a newline on my mac. Where do you want one to be added? After the std::getline call?

Sjors · 2019-02-13T09:17:25Z

@kallewoof it didn't return a new line for me (macOS 10.14.3):

re-tACK 110e821

kallewoof · 2019-02-14T05:51:59Z

Oh, okay! So this is the case for pre-existing -stdinrpcpass as well. Fixed in d1688c9.

Sjors · 2019-02-14T15:59:47Z

re-tACK d1688c9

laanwj · 2019-09-30T12:05:45Z

src/bitcoin-cli.cpp

you could use fputs here (same below) and avoid introducing the locale dependency exception

tfm::format(std::cerr, ... should work as well

D'oh, yeah. Removing linter changes and using fputs.

laanwj · 2019-09-30T12:06:56Z

src/bitcoin-cli.cpp

please only print this extra newline if stdin terminal is detected, and not when, say, piping in from a script (same below 2x)

Understood. I think the latest version achieves this. (See StdinTerminal().)

$ ./bitcoin-cli -datadir=d -regtest encryptwallet foobar38 wallet encrypted; The keypool has been flushed and a new HD seed was generated (if you are using HD). You need to make a new backup. $ ./bitcoin-cli -datadir=d -regtest -stdinwalletpassphrase walletpassphrase 10 Wallet passphrase> $ echo foobar38 | ./bitcoin-cli -datadir=d -regtest -stdinwalletpassphrase walletpassphrase 10 $ echo xfoobar38 | ./bitcoin-cli -datadir=d -regtest -stdinwalletpassphrase walletpassphrase 10 error code: -14 error message: Error: The wallet passphrase entered was incorrect.

laanwj · 2019-10-01T11:15:56Z

Thanks, LGTM now
code review ACK 50c4afa

…passwords 50c4afa add newline after -stdin* (Karl-Johan Alm) 7f11fba cli: add -stdinwalletpassphrase for (slightly more) secure CLI (Karl-Johan Alm) 0da503e add stdin helpers for password input support (Karl-Johan Alm) Pull request description: This PR * adds `-stdinwalletpassphrase` for use with `walletpasshprase(change)` * adds no-echo for passwords (`-stdinrpcpass` and above) It may not be ideal, but it's better than having to clear the screen whenever you unlock the wallet. ACKs for top commit: laanwj: code review ACK 50c4afa Tree-SHA512: 473db8a303ff360ffaa36ac81a2f82be2136fa82696df0bc4f33cb44033a3ae258b5aa5bbcc1f101f88ae9abe9598ed564ce52877ab139bd5d709833f5275ec6

…n#13716)

Summary: This helpers will be used for no-echo input when typing passwords in a terminal. This is a backport of Core [[bitcoin/bitcoin#13716 | PR13716]] [1/3] Commit bitcoin/bitcoin@0da503e Test Plan: `cmake .. -GNinja && ninja` Reviewers: O1 Bitcoin ABC, #bitcoin_abc, deadalnix Reviewed By: O1 Bitcoin ABC, #bitcoin_abc, deadalnix Subscribers: deadalnix Differential Revision: https://reviews.bitcoinabc.org/D8106

Summary: > This PR > > - adds -stdinwalletpassphrase for use with walletpasshprase(change) > - adds no-echo for passwords (-stdinrpcpass and above) This is a backport of Core [[bitcoin/bitcoin#13716 | PR13716]] [2/3] Commit: bitcoin/bitcoin@7f11fba Depends D8106 Test Plan: ``` src/bitcoin-cli -help src/bitcoin-cli -stdinrpcpass getnetworkinfo src/bitcoin-cli -stdinrpcpass -stdinwalletpassphrase walletpassphrase ``` Reviewers: O1 Bitcoin ABC, #bitcoin_abc, deadalnix Reviewed By: O1 Bitcoin ABC, #bitcoin_abc, deadalnix Subscribers: deadalnix Differential Revision: https://reviews.bitcoinabc.org/D8107

Summary: This is a backport of Core [[bitcoin/bitcoin#13716 | PR13716]] [3/3] Commit: bitcoin/bitcoin@50c4afa Depends on D8107 Test Plan: Verify that newlines are added between the two passwords: `src/bitcoin-cli -stdinrpcpass -stdinwalletpassphrase walletpassphrase` Reviewers: O1 Bitcoin ABC, #bitcoin_abc, deadalnix Reviewed By: O1 Bitcoin ABC, #bitcoin_abc, deadalnix Subscribers: deadalnix Differential Revision: https://reviews.bitcoinabc.org/D8108

fanquake added the Utils/log/libs label Jul 19, 2018

kallewoof force-pushed the stdinwalletpassphrase branch 2 times, most recently from 6644848 to 64eb9b5 Compare July 19, 2018 18:04

This was referenced Jul 22, 2018

[refactor] Fix the chainparamsbase -> util circular dependency #13639

Closed

[bugfix] Fix encoding issue for Windows #13426

Closed

kallewoof force-pushed the stdinwalletpassphrase branch 2 times, most recently from f4fb12b to 3f49a30 Compare July 23, 2018 01:25

kallewoof closed this Jul 24, 2018

kallewoof reopened this Jul 24, 2018

DrahtBot added the Needs rebase label Jul 24, 2018

kallewoof force-pushed the stdinwalletpassphrase branch from 3f49a30 to eee2aeb Compare July 25, 2018 07:41

DrahtBot removed the Needs rebase label Jul 25, 2018

This was referenced Jul 25, 2018

[WIP] Full unicode support on Windows #13755

Closed

-masterdatadir for datadir bootstrapping #13746

Closed

Test for Windows encoding issue #13787

Merged

DrahtBot mentioned this pull request Aug 5, 2018

doc: fixed bitcoin-cli --help output to compatibility with help2man #13879

Closed

This was referenced Aug 5, 2018

utils: Convert Windows args to utf-8 string #13883

Merged

docs: fixed bitcoin-cli -help output for help2man #13905

Merged

DrahtBot added the Needs rebase label Aug 13, 2018

kallewoof force-pushed the stdinwalletpassphrase branch from eee2aeb to fb1f6c9 Compare August 14, 2018 05:28

DrahtBot removed the Needs rebase label Aug 14, 2018

DrahtBot mentioned this pull request Aug 26, 2018

Use std::unordered_set instead of set in blockfilter interface #14074

Merged

laanwj reviewed Sep 10, 2018

View reviewed changes

kallewoof force-pushed the stdinwalletpassphrase branch from fb1f6c9 to abd986f Compare September 11, 2018 06:35

laanwj reviewed Sep 11, 2018

View reviewed changes

DrahtBot added the Needs rebase label Feb 12, 2019

kallewoof force-pushed the stdinwalletpassphrase branch from b197e57 to 110e821 Compare February 13, 2019 06:02

DrahtBot removed the Needs rebase label Feb 13, 2019

DrahtBot added the Needs rebase label Aug 2, 2019

kallewoof force-pushed the stdinwalletpassphrase branch 2 times, most recently from e80b84a to 404beda Compare August 3, 2019 02:57

DrahtBot removed the Needs rebase label Aug 3, 2019

laanwj added the Feature label Sep 30, 2019

laanwj reviewed Sep 30, 2019

View reviewed changes

kallewoof and others added 3 commits October 1, 2019 12:19

add stdin helpers for password input support

0da503e

cli: add -stdinwalletpassphrase for (slightly more) secure CLI

7f11fba

add newline after -stdin*

50c4afa

kallewoof force-pushed the stdinwalletpassphrase branch from f42176b to 50c4afa Compare October 1, 2019 03:31

laanwj merged commit 50c4afa into bitcoin:master Oct 2, 2019

kallewoof deleted the stdinwalletpassphrase branch October 8, 2019 07:23

laanwj mentioned this pull request Oct 9, 2019

build: Fix #include sys/poll.h to just poll.h (without sys/) #17084

Merged

MarkLTZ added a commit to litecoinz-core/litecoinz that referenced this pull request Nov 17, 2019

[cli] Add -stdinwalletpassphrase and non-echo stdin passwords (bitcoi…

65a4450

…n#13716)

MarkLTZ mentioned this pull request Apr 4, 2020

Bitcoin PR tracking litecoinz-core/litecoinz#14

Closed

adamjonas mentioned this pull request Apr 24, 2020

encryptwallet password entered in clear #15318

Closed

bitcoin locked as resolved and limited conversation to collaborators Dec 16, 2021

Conversation

kallewoof commented Jul 19, 2018

Uh oh!

practicalswift commented Jul 19, 2018

Uh oh!

DrahtBot commented Jul 22, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Conflicts

Uh oh!

hebasto commented Aug 5, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

laanwj Sep 10, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

laanwj commented Sep 10, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kallewoof commented Feb 13, 2019

Uh oh!

Sjors commented Feb 13, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kallewoof commented Feb 14, 2019

Uh oh!

Sjors commented Feb 14, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

laanwj Sep 30, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

laanwj commented Oct 1, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

DrahtBot commented Jul 22, 2018 •

edited

Loading

hebasto commented Aug 5, 2018 •

edited

Loading

laanwj Sep 10, 2018 •

edited

Loading

Sjors commented Feb 13, 2019 •

edited

Loading

laanwj Sep 30, 2019 •

edited

Loading