RPC: Add option -stdinrpcpass to bitcoin-cli to allow RPC password to be read from standard input #10997

jharvell · 2017-08-06T16:14:13Z

Add a new command-line option to bitcoin-cli that allows the RPC password to be read from standard intput. The purpose of this option is to allow secure RPC password input to bitcoin-cli through an external program that is capable of disabling terminal echo.

This option works similarly to the existing -stdin option, and also works when combined with that option.

I have also written a simple ncurses based program that disables echo, gets input from the terminal and writes to standard output. I couldn't find an existing askpass program that doesn't require graphics libraries, since they are primarily used for getting passwords in a graphics environment. Unless someone can point out a suitable existing askpass program, I plan to submit my ncurses program to the contrib directory separately from this pull request.

jharvell · 2017-08-07T06:18:35Z

I created an askpass utility program (https://github.com/jharvell/askpass) since I didn't find one to my liking. In particular, this one supports mult-line input which would be needed for -stdinrpcpass combined with -stdin.

laanwj · 2017-08-08T09:32:00Z

src/bitcoin-cli.cpp

Suggestion: ForceSetArg("-rpcpassword", rpcPass) here, I think this avoids most of the other changes.

laanwj · 2017-08-08T09:35:42Z

src/bitcoin-cli.cpp

Please use throw std::runtime_error("-stdinrpcpass specified but failed to read from standard input"), this will do automatically the right thing (like adding error: in front, adding a newline and setting the return code).

jharvell · 2017-08-08T22:52:51Z

I realized I had committed with the wrong author email. So I did a rebase where I amended the author email. Both commits are the same content, just now with the correct author email (and hash).

Also, let me know if you want me to do one last rebase and squash to a single commit if/before you merge to master.

laanwj · 2017-08-10T08:35:34Z

src/bitcoin-cli.cpp

I don't understand this change. I think it's better to leave it out as it's not relevant to adding -stdinrpcpass, it also duplicates the credentials error which is a bit ugly.

Without this change, it is possible to specify an RPC password (either through explicit option for via -stdinrpcpass) without specifying an RPC user. In this case, the authentication token is ":<rpcpass>". I had assumed that this could never match and was an error. Now that I think about it, perhaps this was an intentional mechanism to allow an empty RPC user.

In any case, I will remove this change.

done.

Also, let me know if/when to rebase to combine these three commits into one.

laanwj · 2017-08-23T10:41:55Z

Looks good to me now utACK. Would be nice if someone could test, though.

Also, let me know if you want me to do one last rebase and squash to a single commit if/before you merge to master.

Yes please.

jonasschnelli · 2017-08-23T10:52:12Z

utACK 4bd8775ec3ea1d32a15aad9ceb6fcf21694df9d6 (yes, please squash).

…rd input

jharvell · 2017-08-23T18:52:11Z

squashed to single commit

laanwj · 2017-08-24T06:53:26Z

Tested ACK 79191f5

…PC password to be read from standard input 79191f5 Add option -stdinrpcpass to allow RPC password to be read from standard input (Joe Harvell) Pull request description: Add a new command-line option to bitcoin-cli that allows the RPC password to be read from standard intput. The purpose of this option is to allow secure RPC password input to bitcoin-cli through an external program that is capable of disabling terminal echo. This option works similarly to the existing -stdin option, and also works when combined with that option. I have also written a simple ncurses based program that disables echo, gets input from the terminal and writes to standard output. I couldn't find an existing askpass program that doesn't require graphics libraries, since they are primarily used for getting passwords in a graphics environment. Unless someone can point out a suitable existing askpass program, I plan to submit my ncurses program to the contrib directory separately from this pull request. Tree-SHA512: 6d426d757de325d928fab42ea8e423273a7dea9f838acb745ccf9f9daa2b47e23044ec1c019cda1a081253f5145fc10f79ae82dfe7f8e952e1f271ec56018e14

promag · 2017-08-24T13:06:41Z

src/bitcoin-cli.cpp

        }
+        std::string rpcPass;
+        if (gArgs.GetBoolArg("-stdinrpcpass", false)) {
+            if(!std::getline(std::cin,rpcPass))


Nit, missing spaces after if and , and missing {.

@laanwj I'm writing a test for bitcoin-cli, maybe add a commit there to fix these?

…rd input Github-Pull: bitcoin#10997 Rebased-From: 79191f5

29e1dfb [test] Add bitcoin-cli -stdin and -stdinrpcpass functional tests (João Barbosa) ce379b4 [test] Replace check_output with low level version (João Barbosa) 232e3e8 [test] Add assert_raises_process_error to assert process errors (João Barbosa) 5c18a84 [test] Add support for custom arguments to TestNodeCLI (João Barbosa) e127494 [test] Improve assert_raises_jsonrpc docstring (João Barbosa) 7696841 Fix style in -stdin and -stdinrpcpass handling (João Barbosa) Pull request description: This patch adds tests for `bitcoin-cli` options `-stdin` (#7550) and `-stdinrpcpass` #10997. Tree-SHA512: fd8133f44876f2b5b41dfd3762b1988598f6b7bf13fb2385ad95876825d9c0b2b896ce4ea6eeb21012158e1f276907f155d37bb967198b609d2d3dddbfa334c1

…allow RPC password to be read from standard input 79191f5 Add option -stdinrpcpass to allow RPC password to be read from standard input (Joe Harvell) Pull request description: Add a new command-line option to bitcoin-cli that allows the RPC password to be read from standard intput. The purpose of this option is to allow secure RPC password input to bitcoin-cli through an external program that is capable of disabling terminal echo. This option works similarly to the existing -stdin option, and also works when combined with that option. I have also written a simple ncurses based program that disables echo, gets input from the terminal and writes to standard output. I couldn't find an existing askpass program that doesn't require graphics libraries, since they are primarily used for getting passwords in a graphics environment. Unless someone can point out a suitable existing askpass program, I plan to submit my ncurses program to the contrib directory separately from this pull request. Tree-SHA512: 6d426d757de325d928fab42ea8e423273a7dea9f838acb745ccf9f9daa2b47e23044ec1c019cda1a081253f5145fc10f79ae82dfe7f8e952e1f271ec56018e14

…nal tests 29e1dfb [test] Add bitcoin-cli -stdin and -stdinrpcpass functional tests (João Barbosa) ce379b4 [test] Replace check_output with low level version (João Barbosa) 232e3e8 [test] Add assert_raises_process_error to assert process errors (João Barbosa) 5c18a84 [test] Add support for custom arguments to TestNodeCLI (João Barbosa) e127494 [test] Improve assert_raises_jsonrpc docstring (João Barbosa) 7696841 Fix style in -stdin and -stdinrpcpass handling (João Barbosa) Pull request description: This patch adds tests for `bitcoin-cli` options `-stdin` (bitcoin#7550) and `-stdinrpcpass` bitcoin#10997. Tree-SHA512: fd8133f44876f2b5b41dfd3762b1988598f6b7bf13fb2385ad95876825d9c0b2b896ce4ea6eeb21012158e1f276907f155d37bb967198b609d2d3dddbfa334c1

…allow RPC password to be read from standard input 79191f5 Add option -stdinrpcpass to allow RPC password to be read from standard input (Joe Harvell) Pull request description: Add a new command-line option to bitcoin-cli that allows the RPC password to be read from standard intput. The purpose of this option is to allow secure RPC password input to bitcoin-cli through an external program that is capable of disabling terminal echo. This option works similarly to the existing -stdin option, and also works when combined with that option. I have also written a simple ncurses based program that disables echo, gets input from the terminal and writes to standard output. I couldn't find an existing askpass program that doesn't require graphics libraries, since they are primarily used for getting passwords in a graphics environment. Unless someone can point out a suitable existing askpass program, I plan to submit my ncurses program to the contrib directory separately from this pull request. Tree-SHA512: 6d426d757de325d928fab42ea8e423273a7dea9f838acb745ccf9f9daa2b47e23044ec1c019cda1a081253f5145fc10f79ae82dfe7f8e952e1f271ec56018e14

…nal tests 29e1dfb [test] Add bitcoin-cli -stdin and -stdinrpcpass functional tests (João Barbosa) ce379b4 [test] Replace check_output with low level version (João Barbosa) 232e3e8 [test] Add assert_raises_process_error to assert process errors (João Barbosa) 5c18a84 [test] Add support for custom arguments to TestNodeCLI (João Barbosa) e127494 [test] Improve assert_raises_jsonrpc docstring (João Barbosa) 7696841 Fix style in -stdin and -stdinrpcpass handling (João Barbosa) Pull request description: This patch adds tests for `bitcoin-cli` options `-stdin` (bitcoin#7550) and `-stdinrpcpass` bitcoin#10997. Tree-SHA512: fd8133f44876f2b5b41dfd3762b1988598f6b7bf13fb2385ad95876825d9c0b2b896ce4ea6eeb21012158e1f276907f155d37bb967198b609d2d3dddbfa334c1 remove duplicate method Signed-off-by: Pasta <pasta@dashboost.org>

Summary: This includes a sentence in the documentation of `-stdin` that was missed while backporting [[bitcoin/bitcoin#10997 | PR10997]] (D1042), and the removal of a redundant `strprintf`. The double space was already removed in D3058. This is a backport of Core [[bitcoin/bitcoin#13905 | PR13905]] Test Plan: `ninja && ninja check` `bitcoin-cli -help` Reviewers: O1 Bitcoin ABC, #bitcoin_abc, deadalnix Reviewed By: O1 Bitcoin ABC, #bitcoin_abc, deadalnix Differential Revision: https://reviews.bitcoinabc.org/D8105

jharvell changed the title ~~Add option -stdinrpcpass to allow RPC password to be read from stdin~~ RPC: Add option -stdinrpcpass to allow RPC password to be read from standard input Aug 6, 2017

jharvell changed the title ~~RPC: Add option -stdinrpcpass to allow RPC password to be read from standard input~~ RPC: Add option -stdinrpcpass to bitcoin-cli to allow RPC password to be read from standard input Aug 7, 2017

fanquake added the RPC/REST/ZMQ label Aug 7, 2017

laanwj reviewed Aug 8, 2017

View reviewed changes

jharvell force-pushed the stdinrpcpass branch from ca1cd10 to cb8cf16 Compare August 8, 2017 22:50

laanwj reviewed Aug 10, 2017

View reviewed changes

laanwj self-assigned this Aug 23, 2017

Add option -stdinrpcpass to allow RPC password to be read from standa…

79191f5

…rd input

jharvell force-pushed the stdinrpcpass branch from 4bd8775 to 79191f5 Compare August 23, 2017 18:51

laanwj merged commit 79191f5 into bitcoin:master Aug 24, 2017

laanwj mentioned this pull request Aug 24, 2017

TODO for release notes 0.16.0 #11054

Closed

promag reviewed Aug 24, 2017

View reviewed changes

promag mentioned this pull request Aug 24, 2017

Add bitcoin-cli -stdin and -stdinrpcpass functional tests #11125

Merged

jharvell deleted the stdinrpcpass branch August 25, 2017 03:03

luke-jr pushed a commit to bitcoinknots/bitcoin that referenced this pull request Sep 2, 2017

Add option -stdinrpcpass to allow RPC password to be read from standa…

3b78bd2

…rd input Github-Pull: bitcoin#10997 Rebased-From: 79191f5

jonasschnelli mentioned this pull request Feb 1, 2019

encryptwallet password entered in clear #15318

Closed

bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021

RPC: Add option -stdinrpcpass to bitcoin-cli to allow RPC password to be read from standard input #10997

RPC: Add option -stdinrpcpass to bitcoin-cli to allow RPC password to be read from standard input #10997

Uh oh!

Conversation

jharvell commented Aug 6, 2017

Uh oh!

jharvell commented Aug 7, 2017

Uh oh!

laanwj Aug 8, 2017 • 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!

jharvell Aug 8, 2017

Choose a reason for hiding this comment

Uh oh!

laanwj Aug 8, 2017

Choose a reason for hiding this comment

Uh oh!

jharvell Aug 8, 2017

Choose a reason for hiding this comment

Uh oh!

jharvell commented Aug 8, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

laanwj Aug 10, 2017

Choose a reason for hiding this comment

Uh oh!

jharvell Aug 10, 2017 • 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!

jharvell Aug 10, 2017

Choose a reason for hiding this comment

Uh oh!

laanwj commented Aug 23, 2017

Uh oh!

jonasschnelli commented Aug 23, 2017

Uh oh!

jharvell commented Aug 23, 2017

Uh oh!

laanwj commented Aug 24, 2017

Uh oh!

promag Aug 24, 2017

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

laanwj Aug 8, 2017 •

edited

Loading

jharvell commented Aug 8, 2017 •

edited

Loading

jharvell Aug 10, 2017 •

edited

Loading