Irqbalance / irqbalance

The irqbalance source tree - The new official site for irqbalance
http://irqbalance.github.io/irqbalance/
GNU General Public License v2.0
576 stars 139 forks source link

Additional systemd options #294

Closed RZR7332 closed 7 months ago

RZR7332 commented 7 months ago

Would it be possible/feasible/realistic to update the systemd service file for irqbalance to include some hardening options? I have built a service file to include some additional sandboxing and security toggles.

I am not suggesting that irqbalance is insecure or badly coded, but an extra layer never hurts in my opinion. I am using the below configuration:

IPAddressDeny=any ProtectSystem=strict ReadWritePaths=/proc/irq ProtectHome=true PrivateTmp=yes PrivateNetwork=yes ProtectHostname=yes ProtectClock=yes ProtectKernelTunables=yes ProtectKernelModules=yes ProtectKernelLogs=yes ProtectControlGroups=yes PrivateMounts=yes SystemCallArchitectures=native LockPersonality=yes

None of these settings should have any impact on the operation of irqbalance. What would be helpful is some input on the below options:

Any input would be much appreciated - if these suggestions could be considered for default inclusion, that would be amazing.

nhorman commented 7 months ago

RestrictNamespaces - irqbalance makes no effort to create its own namespaces, so I can't see how this would hurt anything

PrivateUsers - Unsure about this one. As long as irqbalance runs as a root user with write access to /proc/irq/*/smp_affinity, and read access to the sysfs tree, I think its ok

RestrictSUIDSGID - I think this is ok, as irqbalance should really only be run as root, or a simmilarly permissioned user

SystemCallFilter - yes, I think that set is appropriate, definately would need to test this extensively though

RestrictRealtime - no realtime scheduling is needed, so this should be ok

MemoryDenyWriteExecute - This should be fine.

RZR7332 commented 7 months ago

Thanks, I have enabled as per your recommendations. Some further comments below:

For PrivateUsers, the documentation states as below:

If true, sets up a new user namespace for the executed processes and configures a minimal user and group mapping, that maps the "root" user and group as well as the unit's own user and group to themselves and everything else to the "nobody" user and group. This is useful to securely detach the user and group databases used by the unit from the rest of the system, and thus to create an effective sandbox environment. All files, directories, processes, IPC objects and other resources owned by users/groups not equaling "root" or the unit's own will stay visible from within the unit but appear owned by the "nobody" user and group. If this mode is enabled, all unit processes are run without privileges in the host user namespace (regardless if the unit's own user/group is "root" or not). Specifically this means that the process will have zero process capabilities on the host's user namespace, but full capabilities within the service's user namespace. Settings such as CapabilityBoundingSet= will affect only the latter, and there's no way to acquire additional capabilities in the host's user namespace.

Source: https://www.freedesktop.org/software/systemd/man/latest/systemd.exec.html

For RestrictSUIDSGID, irqbalance runs as root so this should not be an issue. For the SystemCallFilter, @resources might also need to be added.

Is there a method of testing or a path to getting these settings incorporated at the source?

nhorman commented 7 months ago

yes...open a PR here. There is a sample irqbalance.service unit file in the misc directory. If you want to be sure to get these changes incorporated to a given distro however, you will have to open issues with those respective distros individually

RZR7332 commented 7 months ago

Got it, thanks - will try to get to it in the next few days (new to actually doing PRs). I think for now I would rather focus on getting it accepted/working at source - then I can run down the distributions. If I go that route right now, I have little backing and not much to stand on; if it gets accepted at source, it becomes a much easier conversation to have if required.

RZR7332 commented 7 months ago

@nhorman I have created a new PR (https://github.com/Irqbalance/irqbalance/pull/295) - this is my first time doing this so hope I have done it correctly.

Just for reference, the below are set by default on Ubuntu Server 22,04:

[Unit] Description=irqbalance daemon Documentation=man:irqbalance(1) Documentation=https://github.com/Irqbalance/irqbalance ConditionVirtualization=!container

[Service] EnvironmentFile=-/usr/lib/irqbalance/defaults.env EnvironmentFile=-/etc/default/irqbalance ExecStart=/usr/sbin/irqbalance --foreground $IRQBALANCE_ARGS CapabilityBoundingSet= NoNewPrivileges=yes ReadOnlyPaths=/ ReadWritePaths=/proc/irq RestrictAddressFamilies=AF_UNIX RuntimeDirectory=irqbalance/

nhorman commented 7 months ago

merged, thanks