Skip to content
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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

drop unneeded/broken systemd options #98

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 1 addition & 5 deletions templates/vault.systemd.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ After=basic.target network.target
[Service]
User=<%= scope['vault::user'] %>
Group=<%= scope['vault::group'] %>
PrivateDevices=yes
PrivateTmp=yes
ProtectSystem=full
ProtectHome=read-only
Expand All @@ -22,10 +21,7 @@ ProtectHome=read-only
CapabilityBoundingSet=CAP_SYSLOG
NoNewPrivileges=yes
<% else -%>
SecureBits=keep-caps
Capabilities=CAP_IPC_LOCK+ep
CapabilityBoundingSet=CAP_SYSLOG CAP_IPC_LOCK
NoNewPrivileges=yes
CapabilityBoundingSet=CAP_IPC_LOCK
Copy link
Owner

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?

Copy link
Contributor Author

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.

Copy link

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=.

Copy link
Owner

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)

Copy link
Contributor Author

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.

<% end -%>
Environment=GOMAXPROCS=<%= scope['vault::num_procs'] %>
ExecStart=<%= scope['vault::bin_dir'] %>/vault server -config=<%= scope['vault::config_dir'] %>/config.json <%= scope['vault::service_options'] %>
Expand Down