Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
Already on GitHub? Sign in to your account
RPC: Add option -stdinrpcpass to bitcoin-cli to allow RPC password to be read from standard input #10997
Conversation
jharvell
changed the title from
Add option -stdinrpcpass to allow RPC password to be read from stdin
to
RPC: Add option -stdinrpcpass to allow RPC password to be read from standard input
Aug 6, 2017
jharvell
changed the title from
RPC: Add option -stdinrpcpass to allow RPC password to be read from standard input
to
RPC: Add option -stdinrpcpass to bitcoin-cli to allow RPC password to be read from standard input
Aug 7, 2017
|
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. |
fanquake
added
the
RPC/REST/ZMQ
label
Aug 7, 2017
| + if(!std::getline(std::cin,rpcPass)) { | ||
| + std::cerr<<"error: -stdinrpcpass specified but failed to read from standard input\n"; | ||
| + return EXIT_FAILURE; | ||
| + } |
laanwj
Aug 8, 2017
•
Edited 1 time
-
laanwj
Aug 8, 2017
Owner
-
Aug 8, 2017
Suggestion: ForceSetArg("-rpcpassword", rpcPass) here, I think this avoids most of the other changes.
| + std::string rpcPass; | ||
| + if (GetBoolArg("-stdinrpcpass", false)) { | ||
| + if(!std::getline(std::cin,rpcPass)) { | ||
| + std::cerr<<"error: -stdinrpcpass specified but failed to read from standard input\n"; |
laanwj
Aug 8, 2017
Owner
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).
|
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. |
| + strRPCUserColonPass = GetArg("-rpcuser", strRPCUserColonPass); | ||
| + if (strRPCUserColonPass.empty()) { | ||
| + throw std::runtime_error(strprintf( | ||
| + _("Could not locate RPC credentials. No authentication cookie could be found, and RPC user is not set. See -rpcuser. Configuration file: (%s)"), |
laanwj
Aug 10, 2017
Owner
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.
jharvell
Aug 10, 2017
•
Edited 1 time
-
jharvell
Aug 10, 2017
Contributor
-
Aug 10, 2017
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.
jharvell
Aug 10, 2017
Contributor
done.
Also, let me know if/when to rebase to combine these three commits into one.
|
Looks good to me now utACK. Would be nice if someone could test, though.
Yes please. |
laanwj
self-assigned this
Aug 23, 2017
|
utACK 4bd8775 (yes, please squash). |
|
squashed to single commit |
|
Tested ACK 79191f5 |
laanwj
merged commit 79191f5
into
bitcoin:master
Aug 24, 2017
1 check passed
added a commit
that referenced
this pull request
Aug 24, 2017
| @@ -293,6 +294,12 @@ int CommandLineRPC(int argc, char *argv[]) | ||
| argc--; | ||
| argv++; | ||
| } | ||
| + std::string rpcPass; | ||
| + if (gArgs.GetBoolArg("-stdinrpcpass", false)) { | ||
| + if(!std::getline(std::cin,rpcPass)) |
promag
Aug 24, 2017
Contributor
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?
promag
referenced this pull request
Aug 24, 2017
Merged
Add bitcoin-cli -stdin and -stdinrpcpass functional tests #11125
jharvell
deleted the
jharvell:stdinrpcpass
branch
Aug 25, 2017
added a commit
to bitcoinknots/bitcoin
that referenced
this pull request
Sep 2, 2017
jharvell commentedAug 6, 2017
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.