rpc: Implement random-cookie based authentication #6388

laanwj · 2015-07-07T13:07:08Z

When no -rpcpassword is specified, use a special 'cookie' file for authentication. This file is generated with random content when the daemon starts, and deleted when it exits. Read access to this file controls who can access through RPC. By default this file is stored in the data directory but it be overridden with -rpccookiefile.

This is similar to Tor CookieAuthentication: see https://www.torproject.org/docs/tor-manual.html.en

Alternative to #6258. Like that pull, this allows running bitcoind without any manual configuration. However, daemons should ideally never write to their configuration files, so I prefer this solution.

laanwj · 2015-07-07T13:29:16Z

src/rpcprotocol.cpp

@@ -287,3 +289,68 @@ UniValue JSONRPCError(int code, const string& message)
    error.push_back(Pair("message", message));
    return error;
 }
+
+/** Username used when cookie authentication is in use (arbitrary, only for


Not convinced that this code actually belongs in rpcprotocol, it is here because that file is shared between bitcoin-cli and bitcoind server, but conceptually it exists on a higher level than the protocol. Feel free to suggest a better place.

I think rpcprotocol.h/.cpp is a good place for this extension. A new cookieauth.h/cpp file would be possible but i think is more compact and neatly arranged if it stays in rpcprotocol.h/cpp

Making a new file pair e.g. rpcauthcookie sounds good to me too. But I don't feel strongly about it either.
The low-level code from rpcprotocol will be unnecessary with the libevent-based HTTP server (#5677), so this will at least leave something there :)

ajweiss · 2015-07-07T17:22:54Z

Nice! Reviewing this...

ajweiss · 2015-07-07T18:01:43Z

I spent some time trying to break this, and was unable to do so. This grinds off one of bitcoind's most annoying burrs, very nice! One thing to note is that some of the init scripts in the contrib directory have checks for a set rpcpassword, so when this goes in those should be removed.

jonasschnelli · 2015-07-07T18:39:55Z

src/rpcserver.cpp

@@ -597,28 +597,16 @@ void StartRPCThreads()
        strAllowed += subnet.ToString() + " ";
    LogPrint("rpc", "Allowing RPC connections from: %s\n", strAllowed);

-    strRPCUserColonPass = mapArgs["-rpcuser"] + ":" + mapArgs["-rpcpassword"];
    if (((mapArgs["-rpcpassword"] == "") ||
         (mapArgs["-rpcuser"] == mapArgs["-rpcpassword"])) && Params().RequireRPCPassword())


I think this if needs to be rewritten because at the moment the cookie system does not work in regtest.

I'd personally love to get rid of Params().RequireRPCPassword(), also for consistency reasons. But I think it's unrelated to this pull - on regtest, setting an empty password is currently just allowed, so takes precedence.

Hmm... but the following way don't work.

jonasschnelli$ ./src/bitcoind --regtest --datadir=/tmp/dummy2 --printtoconsole (fresh datadir)

jonasschnelli$ ./src/bitcoin-cli --regtest --datadir=/tmp/dummy2/ help error: You must set rpcpassword=<password> in the configuration file: /tmp/dummy2/bitcoin.conf If the file does not exist, create it with owner-readable-only file permissions.

Right, so bitcoin-cli should mind that flag as well (for now). Thanks.

Bleh, that'd mean moving the flag to BaseParams...

jonasschnelli · 2015-07-07T18:46:47Z

Nice work!
Tested ACK (nit: doesn't work in regtest atm).

sipa · 2015-07-09T16:32:45Z

src/bitcoin-cli.cpp

+        // Try fall back to cookie-based authentication if no password is provided
+        if (!GetAuthCookie(&strRPCUserColonPass)) {
+            throw runtime_error(strprintf(
+                _("You must set rpcpassword=<password> in the configuration file:\n%s\n"


Mention in this help text that the ability to write to [authfilepath] would also solve the problem? Now it may be confusing as being unable to write a file results in a help text mentioning some unrelated config option.

This message triggers if it is unable to read from the file, e.g. if it doesn't exist, but is somehow able to connect. This is a very strange situation, but it won't be solved by ability to write anywhere.

(bitcoind won't even start if no rpcpassword is set but it is unable to write the cookie file)

I've never liked the chain-specific exception to having to set a password. It gives issues with bitcoin#6388 which makes it valid to set no password in every case (as it enables random cookie authentication). This pull removes the flag, so that all chains are regarded the same. It also removes the username==password test, which doesn't provide any substantial extra security.

ghost · 2015-07-13T04:35:06Z

src/rpcserver.cpp

-        return;
+        LogPrintf("No rpcpassword set - using random cookie authentication\n");
+        if (!GenerateAuthCookie(&strRPCUserColonPass)) {
+            StartShutdown();


Would it make sense to move the rpcpassword warning into here, instead of removing it entirely? This would give the user warning if GenerateAuthCookie() fails to write to disk for example.

GenerateAuthCookie already logs an error when it is unable to create the file. The data directory must be writable (otherwise creating the lock file will fail, earlier), so it is an extremely rare error. IMO it doesn't need an extensive translated message.

I think it is reasonable to show an error here before shutting down, but can just as well be a standard "Initialization failed, check the error log for details".

When no `-rpcpassword` is specified, use a special 'cookie' file for authentication. This file is generated with random content when the daemon starts, and deleted when it exits. Read access to this file controls who can access through RPC. By default this file is stored in the data directory but it be overriden with `-rpccookiefile`. This is similar to Tor CookieAuthentication: see https://www.torproject.org/docs/tor-manual.html.en Alternative to bitcoin#6258. Like that pull, this allows running bitcoind without any manual configuration. However, daemons should ideally never write to their configuration files, so I prefer this solution.

laanwj · 2015-07-13T11:12:44Z

Rebased against #6398, added generic UI error message before shutting down when writing cookie fails.

0937290 doc: mention RPC random cookie authentication in release notes (Wladimir J. van der Laan) 71cbeaa rpc: Implement random-cookie based authentication (Wladimir J. van der Laan)

rpc: Implement random-cookie based authentication Cherry-picked from bitcoin/bitcoin#6388. Closes #1950.

I've never liked the chain-specific exception to having to set a password. It gives issues with #6388 which makes it valid to set no password in every case (as it enables random cookie authentication). This pull removes the flag, so that all chains are regarded the same. It also removes the username==password test, which doesn't provide any substantial extra security.

laanwj added the RPC/REST/ZMQ label Jul 7, 2015

laanwj mentioned this pull request Jul 7, 2015

Create .conf file with RPC credentials if none exists. #6258

Closed

laanwj reviewed Jul 7, 2015
View reviewed changes

jonasschnelli reviewed Jul 7, 2015
View reviewed changes

laanwj mentioned this pull request Jul 8, 2015

rpc: Remove chain-specific RequireRPCPassword #6398

Merged

sipa reviewed Jul 9, 2015
View reviewed changes

ghost reviewed Jul 13, 2015
View reviewed changes

laanwj force-pushed the 2015_07_cookie_monsters branch from 3f84f46 to 71cbeaa Compare July 13, 2015 11:11

doc: mention RPC random cookie authentication in release notes

0937290

laanwj force-pushed the 2015_07_cookie_monsters branch from a046144 to 0937290 Compare July 13, 2015 11:34

laanwj merged commit 0937290 into bitcoin:master Jul 14, 2015

laanwj added a commit that referenced this pull request Jul 14, 2015

Merge pull request #6388

fd5dfda

0937290 doc: mention RPC random cookie authentication in release notes (Wladimir J. van der Laan) 71cbeaa rpc: Implement random-cookie based authentication (Wladimir J. van der Laan)

laanwj mentioned this pull request Jul 27, 2015

Idea: Bitcoin.conf created by default #3902

Closed

fanquake mentioned this pull request Aug 7, 2015

Add note on relative paths, improve formatting #6512

Merged

VoidingWarranties mentioned this pull request Apr 14, 2016

API authentication NebulousLabs/Sia#1065

Closed

This was referenced Jan 5, 2017

Replace rpcuser/password mechanism with 'cookie file' authentication. zcash/zcash#1950

Closed

rpc: Implement random-cookie based authentication zcash/zcash#1999

Merged

zkbot added a commit to zcash/zcash that referenced this pull request Jan 18, 2017

Auto merge of #1999 - str4d:1950-random-cookie-rpc-auth, r=ebfull

94f427a

rpc: Implement random-cookie based authentication Cherry-picked from bitcoin/bitcoin#6388. Closes #1950.

dgenr8 mentioned this pull request Jun 5, 2018

Cherry-pick updates to contrib/linearize bitcoinxt/bitcoinxt#441

Merged

fanquake mentioned this pull request Nov 5, 2018

qt: remove unused SECURE style #14655

Closed

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

rpc: Implement random-cookie based authentication #6388

rpc: Implement random-cookie based authentication #6388

Uh oh!

Conversation

laanwj commented Jul 7, 2015

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!

ajweiss commented Jul 7, 2015

Uh oh!

ajweiss commented Jul 7, 2015

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!

jonasschnelli commented Jul 7, 2015

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 Jul 13, 2015

Uh oh!

Uh oh!