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

Jump to bottom

Harden ympd.service #185

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Harden ympd.service #185

wants to merge 1 commit into from

Conversation

cpitclaudel
Copy link

This offers a measure of protection against potential ympd vulnerabilities. See
https://www.freedesktop.org/software/systemd/man/systemd.exec.html for
documentation.

@cpitclaudel
Harden ympd.service
0917b46 
This offers a measure of protection against potential ympd vulnerabilities.  See
https://www.freedesktop.org/software/systemd/man/systemd.exec.html for
documentation.
@cpitclaudel
Copy link
Author

See also: https://gist.github.com/ageis/f5595e59b1cddb1513d1b425a323db04, https://github.com/konstruktoid/hardening/blob/master/systemd.adoc#unit-configuration, and https://www.freedesktop.org/software/systemd/man/systemd.exec.html#Sandboxing:

The following sandboxing options are an effective way to limit the exposure of the system towards the unit's processes. It is recommended to turn on as many of these options for each unit as is possible without negatively affecting the process' ability to operate

@SuperBFG7
Copy link
Contributor

When I add these, ympd fails to start for me, since it cannot change the user (drop priviledges).

@cpitclaudel
Copy link
Author

Are you setting up ympd.service as a user service?

@SuperBFG7
Copy link
Contributor

No, but I figured it out in the meantime: the user name has (of course) to be the same as in /etc/defaults/ympd for $YMPD_USER which in my case was mpd

@cpitclaudel
Copy link
Author

Ah, it makes sense then :)

@gdamjan
Copy link

gdamjan commented Oct 29, 2020

I'd also remove the --user $YMPD_USER argument in this PR, since it will/can clash with the DynamicUser=yes/User=ympd, and is unnecessary.

Great work otherwise. Should it be merged already? :)

gdamjan
gdamjan reviewed Oct 29, 2020
View reviewed changes
contrib/ympd.service
PrivateUsers=yes
PrivateTmp=yes
PrivateDevices=yes
ProtectSystem=strict
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is implied by DynamicUser=yes

contrib/ympd.service
CapabilityBoundingSet=
LockPersonality=yes
PrivateUsers=yes
PrivateTmp=yes
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is implied by DynamicUser=yes

contrib/ympd.service
User=ympd
DynamicUser=yes
MountAPIVFS=yes
RemoveIPC=yes
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is implied by DynamicUser=yes

contrib/ympd.service
PrivateTmp=yes
PrivateDevices=yes
ProtectSystem=strict
NoNewPrivileges=yes
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is implied by DynamicUser=yes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gdamjan gdamjan gdamjan left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@cpitclaudel @SuperBFG7 @gdamjan