-
Notifications
You must be signed in to change notification settings - Fork 108
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
drop unneeded/broken systemd options #98
Conversation
Needs to be rebased and merged after #95 |
Capabilities=CAP_IPC_LOCK+ep | ||
CapabilityBoundingSet=CAP_SYSLOG CAP_IPC_LOCK | ||
NoNewPrivileges=yes | ||
CapabilityBoundingSet=CAP_IPC_LOCK |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change seems to be similar to #91
When I did a bit of poking in the systemd changelog I found AmbientCapabilities
, do you know how that differs from CapabilityBoundingSet
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I checked https://www.freedesktop.org/software/systemd/man/systemd.exec.html and the local manpage. An advise I got a long time ago: For systemd, always check the local manpage. The stuff changes from version to version and the online docs aren't accurate to all versions. I am not 100% sure about the difference. I am not a native speaker and I don't understand all of the docs. The changes in this PR were needed to make it work on systemd235. SecureBits is redundant with AmbientCapabilities, PrivateDevices can't be used for mlock, and I think Capabilities is deprecated now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The difference between CapabilityBoundingSet
and AmbientCapabilities
is that the first one restrictes CAPs, the second one gives CAPs.
What you want is a unprivileged User=
instead of root
and than giving the appropriate rights with AmbientCapabilities=
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @killermoehre , so based on that we want to use AmbientCapabilities
since we already use a non-root user ($vault::user
).
The other issue with this change is that we are assuming a minimum systemd version which may not be true for all supported platforms (which use systemd).
Discussion over in #91 touched on this too: #91 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I want to replace the reload systemd exec with the camptocamp/systemd module. This also provides a fact with the systemd version: https://github.com/camptocamp/puppet-systemd/blob/master/lib/facter/systemd.rb#L40-L45
so if we can find out the correct versions, we can create a case statement or something similar.
Rebased, should be good to go now. |
I deployed the module today on my fedora nodes and didn't notice any issues. Systemd changes from jsok#98 got cherrypicked for testing.
I deployed the module today on my fedora nodes and didn't notice any issues. Systemd changes from jsok#98 got cherrypicked for testing.
at least on systemd235, PrivateDevices=yes doesn't work together with the CAP_IPC_LOCK. The CAP_SYSLOG capability isn't needed. Capabilities= is deprecated.
It looks like these changes are needed to run vault on Ubuntu 18.04 (otherwise it crashes right away because of securebits). |
It'd be nice to add |
Agree @pcfens @shanemadden , I don't have the bandwidth to test this and ensure it works on both older and newer versions of systemd. Any help would be appreciated. |
Superseded by #140 |
at least on systemd235, PrivateDevices=yes doesn't work together with
the CAP_IPC_LOCK. The CAP_SYSLOG capability isn't needed. Capabilities=
is deprecated.