Enable systemd sandboxing settings for pihole-FTL.service

[orazioedoardo]

Thank you for your contribution to the Pi-hole Community!

Please read the comments below to help us consider your Pull Request.

We are all volunteers and completing the process outlined will help us review your commits quicker.

Please make sure you

1. Base your code and PRs against the repositories developmental branch.
2. Sign Off all commits as we enforce the DCO for all contributions
3. Sign all your commits as they must have verified signatures
4. File a pull request for any change that requires changes to our documentation at our documentation repo

---

What does this PR aim to accomplish?:

Since Pi-hole is processing untrusted data from the internet all the time, this PR aims to isolate and constrain the pihole-FTL process such that it is more difficult to compromise the rest of the system in case pihole-FTL is compromised.

How does this PR accomplish the above?:

Enable as much systemd sandboxing settings for the pihole-FTL.service without affecting functionality. Here's a link to the manpage which explains what every switch does: https://man.archlinux.org/man/systemd.exec.5

The FTL PID file has been moved to /run/pihole/FTL.pid so that the /run folder can be hidden which prevents a compromised pihole from talking with other processes which may have a unix socket in there.

systemd-analyze security pihole-FTL.service can be used to check the exposure level of the service. With the default pihole-FTL.service file, the exposure level is 9.0. This pull request brings the level down to 2.0 on Debian Bullseye (the exact level depends on the options supported by the system).

Among the options, I also tried to add

• MemoryDenyWriteExecute=true

Works on my test VM, but not on another system where pihole-FTL crashes with /usr/bin/pihole-FTL: error while loading shared libraries: cannot make segment writable for relocation: Operation not permitted even though I think the system should configured the same.

I couldn't set

• SystemCallFilter=~@privileged @resources

because pihole-FTL uses getpriority syscall in the @resources set (figured out by looking at the syscall number in the crash log).

• PrivateUsers=true

pihole-FTL reports permission denied when trying to bind to port 53 and 67.

• ProcSubset=pid

pihole-FTL can't read /proc/net/route, when navigating the web UI (though I don't know where it is used).

---

By submitting this pull request, I confirm the following:

1. I have read and understood the contributors guide, as well as this entire template. I understand which branch to base my commits and Pull Requests against.
2. I have commented my proposed changes within the code and I have tested my changes.
3. I am willing to help maintain this change if there are issues with it later.
4. It is compatible with the EUPL 1.2 license
5. I have squashed any insignificant commits. (git rebase)
6. I have checked that another pull request for this purpose does not exist.
7. I have considered, and confirmed that this submission will be valuable to others.
8. I accept that this submission may not be used, and the pull request closed at the will of the maintainer.
9. I give this submission freely, and claim no ownership to its content.

---

• I have read the above and my PR is ready for review. Check this box to confirm

[dschaper]

Thank you for the submission. I like the concept here, I'm not sure if there's a need for it to this extreme. There's a lot of moving parts for all of Pi-hole and this will require a lot of testing to make sure all those parts will still function.

[orazioedoardo]

Well I don't think it is too extreme. It is mostly making sure that pihole-FTL can't get access to resources it won't use anyway.
From a security point of view this is especially beneficial since pihole-FTL is written in a memory-unsafe language which is prone to memory corruption bugs.
For example, I do not think pihole-FTL is directly or indirectly accessing serial ports, block devices, "SysV IPC as well as POSIX message queues", home directories, kernel logs, control groups, or changing kernel settings, modules, system clock, hostname.
Filtering syscalls may be risky as it may not be easy to enumerate all of them that are required to operate but, at least on Debian, I've been using this service file for more than a week and did not have any issue.
On a side note, I wish pihole used /usr/lib/systemd/system as the location of the service file, as it would allow user overrides to it using systemctl edit --full pihole-FTL.service.

[MichaIng]

On a side note, I wish pihole used /usr/lib/systemd/system as the location of the service file, as it would allow user overrides to it using systemctl edit --full pihole-FTL.service.

What is the benefit of --full? It overwrites the whole service, which means that potentially mandatory changes shipped with updates do not apply. I personally think that /usr (everything but /usr/local) and /lib should be used by system packages only, not but manually created files of installers. /usr/local/lib/systemd/system should work as alternative. One argument for not using /etc is that it as well allows masking the service, while I'm not aware of a reason to mask FTL when Pi-hole is installed.

[orazioedoardo]

What is the benefit of --full? It overwrites the whole service, which means that potentially mandatory changes shipped with updates do not apply.

It's most beneficial when I want to replace most of the unit. With overrides in /etc/systemd/system/name.service.d I would need to set empty ReadWriteDirectories= and ProtectSystem=, potentially more with updates, then the same options with new values.

/usr (everything but /usr/local) and /lib should be used by system packages only

I used to think /usr/lib/systemd/system as a folder for upstream units, so the pihole one would apply too, but yes, /usr/local/lib/systemd/system seems more appropriate.

[MichaIng]

It's most beneficial when I want to replace most of the unit.

The question is whether it is wanted to make this easy for admins, which implies to make it easier breaking FTL, directly or after a future Pi-hole update. On the other hand, one might not want to artificially limit the options for admins. I thought about the same change for my other project, but haven't finally decided, considering as well possibly additional support requests from a tendentially less experienced audience 🤔.

[DL6ER]

this PR aims to isolate and constrain the pihole-FTL process such that it is more difficult to compromise the rest of the system in case pihole-FTL is compromised.

Reading your comment, it seems you are not aware that pihole-FTL is an unprivileged daemon. FTL is running under user pihole which has close to no privileges.

For example, I do not think pihole-FTL is directly or indirectly accessing serial ports, block devices, "SysV IPC as well as POSIX message queues", home directories, kernel logs, control groups, or changing kernel settings, modules, system clock, hostname.

See my point above. It does not, but it also couldn't. When these things are world-write-/readable on the machine this would be an issue on its own outside of Pi-hole. FTL could certainly not mess around with kernel settings, the clock and other things.

TL;DR: Most of the protection you want to add here is already there (in one way or another) as FTL is not running as root

[MichaIng]

I was thinking the same regarding CapabilityBoundingSet, as pihole cannot elevate its own capabilities. However, doing a second layer of protection doesn't hurt either, does it? E.g. as we talked about editing the service, admins might also for some reason start it as root, and then the sandboxing becomes effective.

[DL6ER]

As long as it doesn't limit what we can do in a way that has unpleasant side-effects, I'm surely not against more protection.

[orazioedoardo]

it seems you are not aware that pihole-FTL is an unprivileged daemon. FTL is running under user pihole which has close to no privileges.

I am aware, but running as unprivileged doesn't mean it has close to no privileges. The daemon can still access other world readable folders, home folders (up until Ubuntu 21.04 those were still 0755, and Debian still hasn't moved to 0750 or less), communicate with other processes via unix sockets, access features that may be used to exploit the system.

See my point above. It does not, but it also couldn't. When these things are world-write-/readable on the machine this would be an issue on its own outside of Pi-hole. FTL could certainly not mess around with kernel settings, the clock and other things.

Not assuming the software can do just what has been coded to do is the main point of sandboxing.

[DL6ER]

My point is that your example of FTL being able to mess with kernel settings, the clock and the other examples was way exaggerating and downplaying that Pi-hole is already doing much more to protect the user against bugs that most other software does. Some of the things mentioned are simply not possible right now and have never been. But this does not mean there is no room to improvement, I also already expressed above that this is a welcomed addition but only in case it is not limiting us elsewhere or increases the support burden as things get less easy to handle/modify for users.
That's also the reason why are not shipping AppArmor or SELinux profiles any longer (yes we did some years ago) because it's just to easy to break stuff, e.g., when defining a different log file location. Many users just aren't aware of the things happening backstage.

[orazioedoardo]

FTL being able to mess with kernel settings, the clock and the other examples was way exaggerating and downplaying that Pi-hole is already doing much more to protect

I didn't say that pihole is messing with the kernel or such, quite the opposite, it's not, so it makes sense to enforce those features to be unavailable. The intention is to create a bounding box around the expected functionalities, without removing any, so that if compromised, the blast radius is as small as possible.

I also already expressed above that this is a welcomed addition but only in case it is not limiting us elsewhere or increases the support burden as things get less easy to handle/modify for users.

The sandbox can be loosened if required. I usually start with a very restrictive service file and remove options until the daemon works as expected. IMHO the only option that may be problematic is SystemCallFilter since it may not be obvious which syscalls are required. ProtectProc was also a concern but it doesn't look like pihole-FTL is accessing other processes' information.