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
Enable systemd sandboxing settings for pihole-FTL.service #5155
base: development
Are you sure you want to change the base?
Conversation
…pihole/FTL.pid Signed-off-by: Orazio <22700499+orazioedoardo@users.noreply.github.com>
Signed-off-by: Orazio <22700499+orazioedoardo@users.noreply.github.com>
Signed-off-by: Orazio <22700499+orazioedoardo@users.noreply.github.com>
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. |
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. |
Signed-off-by: Orazio <22700499+orazioedoardo@users.noreply.github.com>
What is the benefit of |
It's most beneficial when I want to replace most of the unit. With overrides in
I used to think |
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 🤔. |
Reading your comment, it seems you are not aware that
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 |
I was thinking the same regarding |
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. |
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.
Not assuming the software can do just what has been coded to do is the main point of sandboxing. |
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. |
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.
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 |
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
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.5The 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 defaultpihole-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:
git rebase
)