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
nixos/babeld: lock down service #98187
Conversation
@GrahamcOfBorg test babeld |
serviceConfig.ExecStart = "${pkgs.babeld}/bin/babeld -c ${configFile}"; | ||
serviceConfig = { | ||
ExecStart = "${pkgs.babeld}/bin/babeld -c ${configFile} -I /run/babeld/babeld.pid -S /var/lib/babeld/state"; | ||
DeviceAllow = [ "/dev/null" "/dev/urandom" ]; |
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.
Isn't PrivateDevices
enough?
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.
Isn't this more narrow and thus an improvement?
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.
My concern here is that the user is still able to see those devices and we must keep whitelisting them? With PrivateDevices
the process would just get a "standard" layout of /dev without us having to care about which devices they try to access as none of them should be able to cause harm.
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.
Fair enough, I'll drop DeviceAllow completely then.
serviceConfig = { | ||
ExecStart = "${pkgs.babeld}/bin/babeld -c ${configFile} -I /run/babeld/babeld.pid -S /var/lib/babeld/state"; | ||
DeviceAllow = [ "/dev/null" "/dev/urandom" ]; | ||
CapabilityBoundingSet = [ "CAP_NET_ADMIN" ]; |
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.
Can't we use DynamicUser=true
in this case? What other privs does the process need that justify running as root?
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.
Requires setting sysctls.
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.
mhm okay :/
@andir How do we proceed here? |
→ Overall exposure level for babeld.service: 2.2 OK 🙂
94aba19
to
c821e0d
Compare
Motivation for this change
Tested in production in my private mesh network.
Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
./result/bin/
)nix path-info -S
before and after)