PermissionsStartOnly & Group=
#33044
This removes the redundant ‘Group=’ directive and replaces the deprecated ‘PermissionsStartOnly’ directive.
Picks up #16994 / #19513. The concern in both of these PRs was changing this too early, while systemd v240 was still prelevant on supported systems. That was ~5 years ago, and from what I can see, no modern/supported OS is still using an older systemd.
Separately , I am wondering if we should move these files out to https://github.com/bitcoin-core/packaging/.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/33044.
See the guideline for information on the review process.
| Type | Reviewers |
|---|---|
| ACK | willcl-ark |
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
24@@ -25,8 +25,7 @@ ExecStart=/usr/bin/bitcoind -pid=/run/bitcoind/bitcoind.pid \
25 -shutdownnotify='systemd-notify --stopping'
26
27 # Make sure the config directory is readable by the service user
28-PermissionsStartOnly=true
29-ExecStartPre=/bin/chgrp bitcoin /etc/bitcoin
30+ExecStartPre=!/bin/chgrp bitcoin /etc/bitcoin
In b392a513f025ca03f718c3ff2bbb970faf6f8afd
Commit message could be updated to state that we use ! (not + as it says). Using ! here is the correct choice for minimal priviledge escalation for here though.
ACK b392a513f025ca03f718c3ff2bbb970faf6f8afd
Only a single non-blocking nit on the commit message.
Setting Group=bitcoin is redundant. It is typically the default group
of the user and if not explicitly specified, systemd will run the
service with the default group of the user.
PermissionsStartOnly is deprecated [1]. This removes the directives
and instead we prefixes the value of the ExecStartPre directive with
'!', which means the executable, 'chgrp' in this case, is run with
full privileges and able to change the group of /etc/bitcoin.
1: https://github.com/systemd/systemd/blob/60b45a80c1f98bad000bd902d97ecf6c4e3fc315/NEWS#L2434
Separately , I am wondering if we should move these files out to bitcoin-core/packaging.
A web search for the systemd service file shows barely any direct results, most notably some Stacks.co docs
Fewer still for the openrc and init files.
So, I suspect that moving these to the packaging repo would not have much systematic impact. Not to mention that we prefer installs from our releases vs packaged installs.
I suppose the only thing we don’t know is how many users:
Again, my suspicion would be that the answer is very few to none.
reACK 18d1071dd1244cf3252d687bb46f88d65f652e4d
based on: git range-diff b392a513f02…18d1071dd12
0 1: b12a63e71aa = 91: 1caaf650436 init: remove Group= as it will default to the user's default group
1 2: b392a513f02 ! 92: 18d1071dd12 init: replace deprecated PermissionsStartOnly systemd directive
2 @@ Commit message
3
4 PermissionsStartOnly is deprecated [1]. This removes the directives
5 and instead we prefixes the value of the ExecStartPre directive with
6 - '+', which means the executable, 'chgrp' in this case, is run with
7 + '!', which means the executable, 'chgrp' in this case, is run with
8 full privileges and able to change the group of /etc/bitcoin.
9
10 1: https://github.com/systemd/systemd/blob/60b45a80c1f98bad000bd902d97ecf6c4e3fc315/NEWS#L2434