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.

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

Configure systemd security features in keyd.service #616

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Expertcoderz
Copy link

@Expertcoderz Expertcoderz commented Dec 10, 2023

This PR adds directives in the keyd.service unit definition file to enable all appropriate sandboxing/hardening features offered by systemd. This ensures that the daemon runs with least privileges (without breaking any functionality). The overall exposure level as reported by systemd-analyze security for keyd.service thence improves from "9.6 UNSAFE" to "1.4 OK".

References for the various security options: systemd.exec(5), systemd.resource-control(5)

@ThinkChaos
Copy link

It's probably worth taking a look at NixOS' service definition and copy more of what's done there since it's should be well tested. From a quick eyeball diff, SystemCallFilter and UMask are more restrictive there.

@Expertcoderz
Copy link
Author

It's probably worth taking a look at NixOS' service definition and copy more of what's done there since it's should be well tested.

Thanks for the suggestion! I've looked through it and decided to increase the ProtectProc setting from noaccess to invisible, accordingly.

From a quick eyeball diff, SystemCallFilter and UMask are more restrictive there.

keyd requires access to the setgid and nice syscalls (under what systemd considers @resources). Removing @resources from the allowlist (or leaving CAP_SETGID/CAP_SYS_NICE out of CapabilityBoundingSet) seems to prevent keyd from starting, at least from my testing on Arch Linux.

As for the umask value, (0)177 is actually more restrictive than the 0077 given in NixOS. Umask specifies which permission bits to take away from newly created files/directories, and the 1 in 177 means to remove execute permission for the file owner by default. (Not that it's a significant issue anyway; keyd probably doesn't create any new files to begin with.)

@ThinkChaos
Copy link

ThinkChaos commented Jan 5, 2024

Thanks for comparing and the details!

After taking a closer look I noticed nixpkgs only has keyd v2.4.2 ATM, so it never calls setgid or nice since that code was added in v2.4.3.
BTW according to the systemd docs, setgid is in @setuid, not that it changes anything here :)

EDIT: found a recent PR that updates to v2.4.3 NixOS/nixpkgs#245327, it's missing setgid it seems, but not hitting an issue because the group names it uses aren't keyd which is what ipc.c checks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants