init: Add option for rpccookie permissions #26088

Adds a bitcoind launch option -rpccookieperms to configure the file permissions of the cookie. Can be used like ./src/bitcoind -rpccookieperms=0660

Closes #25270.

https://github.com/bitcoin/bitcoin/pull/26088/checks?check_run_id=8346907823:

The locale dependent function strtol(...) appears to be used:
src/httprpc.cpp:            const auto perms{strtol(cookie_perms_arg->c_str(), &cookie_perms_end, 8)};

Unnecessary locale depedence can cause bugs that are very tricky to isolate and fix. Please avoid using locale dependent functions if possible.

Fixed lint error.

ACK 857d46e47a18265eb229a1ea20064d634786bc72

This will work only for Linux. Added a few lines in src/rpc/request.cpp from an example on https://en.cppreference.com/w/cpp/filesystem/permissions to test and print permissions in logs:

<details> <summary>diff</summary>

+void perms(fs::perms p)
+{
+   std::cout << ((p & fs::perms::owner_read) != fs::perms::none ? "r" : "-")
+             << ((p & fs::perms::owner_write) != fs::perms::none ? "w" : "-")
+              << ((p & fs::perms::owner_exec) != fs::perms::none ? "x" : "-")
+              << ((p & fs::perms::group_read) != fs::perms::none ? "r" : "-")
+              << ((p & fs::perms::group_write) != fs::perms::none ? "w" : "-")
+              << ((p & fs::perms::group_exec) != fs::perms::none ? "x" : "-")
+              << ((p & fs::perms::others_read) != fs::perms::none ? "r" : "-")
+              << ((p & fs::perms::others_write) != fs::perms::none ? "w" : "-")
+              << ((p & fs::perms::others_exec) != fs::perms::none ? "x" : "-")
+              << '\n';
+}

bool GenerateAuthCookie(std::string* cookie_out, std::optional<fs::perms> cookie_perms)
{
  const size_t COOKIE_SIZE = 32;
  unsigned char rand_pwd[COOKIE_SIZE];
  GetRandBytes(rand_pwd);
  std::string cookie = COOKIEAUTH_USER + ":" + HexStr(rand_pwd);

  /** the umask determines what permissions are used to create this file -
   * these are set to 077 in init.cpp unless overridden with -sysperms.
   */
  std::ofstream file;
  fs::path filepath_tmp = GetAuthCookieFile(true);
  file.open(filepath_tmp);
  if (!file.is_open()) {
      LogPrintf("Unable to open cookie authentication file %s for writing\n", fs::PathToString(filepath_tmp));
      return false;
  }
  file << cookie;
  file.close();

  if (cookie_perms) {
      std::error_code code;
      std::filesystem::permissions(filepath_tmp, *cookie_perms, std::filesystem::perm_options::replace, code);
      if (code) {
          LogPrintf("Unable to set permissions on cookie authentication file %s\n", fs::PathToString(filepath_tmp));
          return false;
      }
  }

  fs::path filepath = GetAuthCookieFile(false);
  if (!RenameOver(filepath_tmp, filepath)) {
      LogPrintf("Unable to rename cookie authentication file %s to %s\n", fs::PathToString(filepath_tmp), fs::PathToString(filepath));
      return false;
  }
  LogPrintf("Generated RPC authentication cookie %s\n", fs::PathToString(filepath));
+ LogPrintf("Permissions used for cookie: ");
+ perms(fs::status(filepath).permissions());

  if (cookie_out)
      *cookie_out = cookie;
  return true;
}

</details>

Windows:

.\bitcoind.exe

2022-09-15T09:57:30Z Using random cookie authentication.
2022-09-15T09:57:30Z Generated RPC authentication cookie C:\Users\test\AppData\Roaming\Bitcoin\signet\.cookie
2022-09-15T09:57:30Z Permissions used for cookie: rw-rw-rw-

.\bitcoind.exe -rpccookieperms=0660

2022-09-15T09:51:36Z Using random cookie authentication.
2022-09-15T09:51:36Z Generated RPC authentication cookie C:\Users\test\AppData\Roaming\Bitcoin\signet\.cookie
2022-09-15T09:51:36Z Permissions used for cookie: rw-rw-rw-

Fedora:

bitcoind

2022-09-15T11:34:59Z Using random cookie authentication.
2022-09-15T11:34:59Z Generated RPC authentication cookie /home/test/.bitcoin/regtest/.cookie
2022-09-15T11:34:59Z Permissions used for cookie: rw-------

bitcoind -rpccookieperms=0660

2022-09-15T11:39:37Z Using random cookie authentication.
2022-09-15T11:39:37Z Generated RPC authentication cookie /home/test/.bitcoin/regtest/.cookie
2022-09-15T11:39:37Z Permissions used for cookie: rw-rw----

Added the log message with a slightly modified version. Thanks @1440000bytes.

ACK https://github.com/bitcoin/bitcoin/pull/26088/commits/cc0d0aeca684c73c8f8538ca67ccc6866b786b25

I don't have the time time to test it furtther

<!--e57a25ab6845829454e8d69fc972939a-->

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

<!--021abf342d371248e50ceaed478a90ca-->

Reviews

See the guideline for information on the review process.

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

<!--174a7506f384e20aa4161008e828411d-->

Conflicts

No conflicts as of last run.

I'm wondering: would it be perhaps simpler to just state in the documentation that you should run bitcoind with a umask of 0002 and specify -sysperms if you want group access to the cookie?

A consideration here is that now all bitcoind created files will be group accessible including wallet files, but given that the group has access to the cookie, if they have the wallet password they can still dumpwallet, so the UNIX permissions don't matter as much (or is just a defense-in-depth thing). It seems that #17127 also solves this as well.

In any case, this is something that I run into time and again when deploying Bitcoin Core alongside other services, so I'd love to see some documentation on it.

@aureleoules did you want to reponds to Carls thoughts here?

It seems that #17127 also solves this as well.

It's probably likey that 17127 will be merged shortly.

I don't have a preference as I don't have much experience using this feature. Happy to close this PR if #17127 gets merged.

Happy to close this PR if #17127 gets merged.

17127 has been merged. Closing this.

@aureleoules @fanquake

I think this could be re-opened to solve #25270 if @aureleoules is happy to pick it up again, otherwise I'd be happy to?

Whilst #17127 has given us more sane and secure defaults, permitting group members to access the cookie without resorting to -startupnotify wortkarounds or similar seems reasonable to me.

One the one hand any processes with access to the cookie could just as well be running as the same user as bitcoind on the host, as the cookie essentially gives them full access to funds. And if users took that approach this change wouldn't be necessary. But I also understand people's desire for compartmentalisation and running different processes as different users, who share a group.

If we don't want to include this feature (but I think we should) then we should opt to close #25270 also.

I don't have much time to work on Core so feel free to pick this up @willcl-ark.