Update bitcoin.service to conform to init.md

[dongcarl]

• -datadir option specified.
• Ask systemd to create and set the right mode for PID directory, configuration directory, and data directory.
• Tell systemd our group so it will set the right owner for aforementioned directories.

More information: https://www.freedesktop.org/software/systemd/man/systemd.exec.html

[laanwj]

Last update was in PR #10529 - please try to coordinate the changes with the people replying there.

[dongcarl]

@laanwj Sorry I'm not sure I entirely know what you mean. Am I to bring up this PR on that thread or shall I tag them in a comment here?

[maflcko]

I think the link was supposed to be #12102

[laanwj]

Sorry I'm not sure I entirely know what you mean. Am I to bring up this PR on that thread or shall I tag them in a comment here?

Either, or both, is fine!

[dongcarl]

@laanwj @MarcoFalke should I be waiting for the other PR to get merged? Seems like the OP has been unresponsive.

[maflcko]

I think the only thing left is give people time to review and then merge both whenever they are reviewed.

[dongcarl]

Cool beans.

[Flowdalic]

Thanks for your ping in #12102 (comment), here are my thoughts on this PR:

This PR contains the following changes to the systemd bitcoind.service file

1. Provide -datadir argument to bitcoind via ExecStart
2. Create various runtime directories and set their access mode
3. Set the Group to bitcoin

I deliberately did not put (1) in the systemd file as it can also be set via bitcoind's conf. I believe that it is good practice to put as much as possible in generic service config file (here: bitcoin.conf) as opposed to the system's service management facilities (here: systemd).

(2) is typically done by the distribution's package manager and would hence override settings of distributions that don't use those standard directories (therefore those distributions would need to patch the upstream systemd file or use their own, which is not both not desirable). Nit: I don't think it is necessary to protect the runtime directory with 700 as there a no secrets in it. Also the used settings are only available with systemd >= 235.

(3) is changes the behavior as it explicitly mentions the existence of a group named 'bitcoin', which may or may not exist. I believe that the behavior if 'Group' is not set, using the default group of the user, is more portable and should be used.

Further nits of this PR include that the order of -pid and -conf is changed for no reason, and the (correct) comment telling us that the /run/bitcoind is owned by 'bitcoin' is removed.

In conclusion I am not provided with and don't see any compelling reasons to apply this.

[dongcarl]

@Flowdalic The spirit of my PR is to follow in the footsteps of doc/init.md. Which states that these changes are to be made.

(1) doc/init.md states that the data directory should be at /var/lib/bitcoind, but if it's not indicated in the service file, the data directory is not at /var/lib/bitcoind, but rather at ~/.bitcoin
(2, 3) doc/init.md states that "The configuration file, PID directory (if applicable) and data directory should all be owned by the bitcoin user and group. It is advised for security reasons to make the configuration file and data directory only readable by the bitcoin user and group. Access to bitcoin-cli and other bitcoind rpc clients can then be controlled by group membership.". Perhaps according to the last sentence it'd be better to set the permissions to 750?

If someone was installing from source, and reading from doc/init.md, they would experience all kinds of confusion, as the data directory won't be where doc/init.md indicated it is. With 2 and 3 I think it's reasonable to think that this is optional, but my personal opinion is that what's in the repository should reflect the latest best practices and be consistent.

I'm actually torn for what we should do for (2), while I believe it's better to have consistency on our end and have the distributions figure it out, I don't wanna break things for distributions with alternative hiers.

Happy to discuss more 😄

[randolf]

utACK.

[sugarjig]

I'd like to throw in a vote for (1). I recently stood up an Ubuntu 18.04 server, using the instructions in doc/init.md to install the service. Like @dongcarl is saying, I was a bit confused when it came to configuring the data directory. It worked when I added the -datadir option to the service file.

I tried adding datadir=/var/lib/bitcoind to bitcoin.conf like @Flowdalic suggested, but Bitcoin failed to start. Running journalctl -xe, I see the the error boost::filesystem::create_directories: Permission denied: "/home/bitcoin". It seems this option in the config file has no effect.

[Flowdalic]

I had a look at the code and it appears that bitcoind init first checks the datadir and then reads the config file, which would explain why datadir in the config file has no effect if the default datadir (or the datadir specified by command line argument) is for some reason inaccessible. I have created #13621 in an attempt to fix this.

[DrahtBot]

The last travis run for this pull request was 177 days ago and is thus outdated. To trigger a fresh travis build, this pull request should be closed and re-opened.

[DrahtBot]

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

Conflicts

No conflicts as of last run.

[ryanofsky]

utACK 8b04bf40f7396518d638a7feacf2d8e8e9931780. The inconsistencies between doc/init.md and contrib/init/bitcoind.service and contrib/init/bitcoind.service and the other init files are confusing and this PR gets rid of them and is an improvement over the status quo.

But I think @Flowdalic makes very good points, especially about this change breaking the ability to specify datadir in the config file in #12255 (comment). So to follow up, I would suggest the following changes (either here or in separate PRs):

• Add release-notes-pr12255.md file mentioning that contrib/init/bitcoind.service has changed to use /var/lib/bitcoind as the datadir instead of $HOME/.bitcoin (which is typically /var/lib/bitcoin/.bitcoin or /home/bitcoin/.bitcoin). The change makes bitcoin more consistent with other services, makes the systemd config more consistent with existing upstart and openrc configs. The release notes should also mention that datadir is now managed entirely in the bitcoind.service file and that trying to override it in /etc/bitcoin/bitcoin.conf will have no effect.

• Mention somewhere in doc/init.md that it is not possible to override datadir in /etc/bitcoin/bitcoin.conf with the current systemd, openrc, and upstart init files.

• (Option for a future PR.) I think it's probably better to just document inability to override datadir in /etc/bitcoin/bitcoin.conf, rather than try to do anything about it. But if there are use-cases for wanting to use our service files but also needing the ability to change datadir from the config file, there are changes we could make to bitcoind to support removing the -datadir=/var/lib/bitcoind startup argument. For example we could change GetDefaultDataDir to return "$BITCOIN_HOME" if it is set, instead of "$HOME/.bitcoin", with no loss of compatibility. (GnuPG has a similar GNUPGHOME variable.) The init files could then set $BITCOIN_HOME instead of overriding -datadir.

• I think Flowdalic is right that dropping the Group=bitcoin line would be less fragile. This should be safe because we aren't giving the group any permissions, and systemd.exec specifies "If no group is set, the default group of the user is used". But I would want the group to be specified explicitly if we were granting the group any permissions (even read only).

[dongcarl]

I think Flowdalic is right that dropping the Group=bitcoin line would be less fragile. This should be safe because we aren't giving the group any permissions, and systemd.exec specifies "If no group is set, the default group of the user is used". But I would want the group to be specified explicitly if we were granting the group any permissions (even read only).

I believe that there are valid uses for the bitcoin group. Some examples:

• elements uses the bitcoin auth cookie file thru its -mainchainrpccookiefile flag for its RPC calls to bitcoind
• c-lightning allows specifying the bitcoin data directory thru its --bitcoin-datadir flag

Perhaps what should be done is to make the permissions for the datadir 770 or 750, so that these applications can run under the bitcoin group and automatically get rw/ro access to it.

[dongcarl]

Add release-notes-pr12255.md file mentioning that contrib/init/bitcoind.service has changed to use /var/lib/bitcoind as the datadir instead of $HOME/.bitcoin (which is typically /var/lib/bitcoin/.bitcoin or /home/bitcoin/.bitcoin). The change makes bitcoin more consistent with other services, makes the systemd config more consistent with existing upstart and openrc configs. The release notes should also mention that datadir is now managed entirely in the bitcoind.service file and that trying to override it in /etc/bitcoin/bitcoin.conf will have no effect.

Mention somewhere in doc/init.md that it is not possible to override datadir in /etc/bitcoin/bitcoin.conf with the current systemd, openrc, and upstart init files.

Sounds good to me. Will do.

(Option for a future PR.) I think it's probably better to just document inability to override datadir in /etc/bitcoin/bitcoin.conf, rather than try to do anything about it. But if there are use-cases for wanting to use our service files but also needing the ability to change datadir from the config file, there are changes we could make to bitcoind to support removing the -datadir=/var/lib/bitcoind startup argument. For example we could change GetDefaultDataDir to return "$BITCOIN_HOME" if it is set, instead of "$HOME/.bitcoin", with no loss of compatibility. (GnuPG has a similar GNUPGHOME variable.) The init files could then set $BITCOIN_HOME instead of overriding -datadir.

Just to verify I understand what you're saying:

• In the case that both $BITCOIN_HOME and -datadir are set, -datadir would override $BITCOIN_HOME
• An unset $BITCOIN_HOME effectively is env BITCOIN_HOME=$HOME/.bitcoin as it is currently

[ryanofsky]

I believe that there are valid uses for the bitcoin group

Sure, I'm just agreeing with previous commenters that bitcoin group should either be required to exist and given permissions or it should be dropped. It's unnecessarily fragile to reference a group that doesn't need to exist and doesn't have any permissions for no reason.

Just to verify I understand what you're saying

I'm not really saying the second thing. The GetDefaultDataDir function is more complex than that. I'm just saying we could add a one-line override at the top of that function:

if (char* ret = getenv("BITCOIN_HOME")) return ret;

that would give init systems a way to set up a default data location without causing datadir= options in /etc/bitcoin/bitcoin.conf to get silently ignored. I'm not saying we should do this. It's fine IMO to just document the limitations that exist right now. But if they are a problem that needs to be solved, this would be a simple, backwards compatible way to solve them.

[fanquake]

utACK 3b6b72a

nits can be fixed up when the release notes are merged.

[fanquake]

re-utACK ca07fa8

[ryanofsky]

utACK ca07fa8. Only changes since previous review are dropping the first commit (in response to #12255 (comment)) and tweaking the release notes.

[luke-jr]

At least OpenRC's init allows for setting BITCOIND_DATADIR to override it.

[ryanofsky]

re: #12255 (comment)

At least OpenRC's init allows for setting BITCOIND_DATADIR to override it.

re: http://www.erisian.com.au/bitcoin-core-dev/log-2019-01-17.html#l-365

I see... I think I'll make it easy to override for systemd as well then. Thanks!

It would be good to mention /etc/conf.d/bitcoind for OpenRC, but I wouldn't put a huge amount effort into implementing a configuration mechanism for the systemd service (unless your really want to). RC scripts tend to support external configuration because can they can be pretty long and complicated. Systemd service files are (in theory) supposed to stay simple and rely on more coarse override mechanisms like systemctl edit.

[ryanofsky]

I think this could be merged. It looks like the only outstanding suggestion is to mention the openrc config file in the documentation (#12255 (comment)), which could be done in a followup later.

[dongcarl]

@ryanofsky Perhaps I should just omit the other init systems in the release notes, as it is unrelated to this PR and I don't have enough confidence that they don't have a way of overriding. If that seems acceptable I will make the wording changes.

[ryanofsky]

The sentence:

It is not currently possible to override datadir in /etc/bitcoin/bitcoin.conf with the current systemd, OpenRC, and Upstart init files

is correct as written. If you want to mention that OpenRC and Upstart have other config mechanisms that would be wonderful but not necessary. I do think it is important to mention that out of the box in all three init systems trying to set datadir in /etc/bitcoin/bitcoin.conf is not supported and will have no effect.

[dongcarl]

Pushed clearer wording

[laanwj]

Thanks for adding a release notes item.
I'd suggest systemd init file as title, the PR number won't tell most users much

[dongcarl]

Fixed @laanwj's suggestion.

[laanwj]

Thanks! utACK bad1716

[ryanofsky]

utACK bad1716