search

pi-hole / docker-pi-hole

Pi-hole in a docker container
https://pi-hole.net
Other
8.09k stars 1.11k forks source link

Set PUID and PGID as Variables #328

Open Veldkornet opened 5 years ago

Veldkornet commented 5 years ago

This is a...

Description

So I'm addressing a few things in this one issue, which I understand is not ideal. Could you please add the PUID (Process User ID) and PGID (Process Group ID) variables? I tell you why. I use Docker on my Synology NAS and for the life of me, I cannot get it to work. I keep getting the following error: [ERROR]: Unable to parse results from queryads.php: Unhandled error message (Invalid domain!) I've therefore set the logs to be external with "/volume1/docker/pihole/logs/:/var/log/", however I then run into the problem that it cannot start because pihole doesn't have access to the logs (even though I set the permission to 777 for the directory and everything in it.)

` 2018-08-31 17:30:36: (log.c.171) opening errorlog '/var/log/lighttpd/error.log' failed: Permission denied stderr

`

Expected Behavior

Docker processes should run under the user id and group ID values specified in the environmental variables

Actual Behavior

It doesn't

Possible Fix

Add the variables. Many docker images have these variables, especially those from https://www.linuxserver.io/our-images if you need examples

Steps to Reproduce and debugging done

e.g. your docker run command, pages to visit, CLI commands you ran There's not much to do as I can't even access the webpage directly after creating the image. FYI, I do not have DNS or DHCP running on my NAS. This is the create command that I used: sudo docker run -d \ --name pihole \ -p 53:53/tcp -p 53:53/udp \ -p 67:67/udp \ -p 32777:80 \ -p 32778:443 \ -v "/volume1/docker/pihole/pihole/:/etc/pihole/" \ -v "/volume1/docker/pihole/dnsmasq.d/:/etc/dnsmasq.d/" \ -e ServerIP="${IP}" \ -e TZ=Europe/Amsterdam \ -e DNS1=208.67.222.222 \ -e DNS2=208.67.220.220 \ -e PUID=1033 \ -e PGID=65540 \ --restart=unless-stopped \ --cap-add=NET_ADMIN \ pihole/pihole:latest

Debug steps I have tried

Context and extra information

271 #267 #85

Your Environment

Exist2Resist commented 5 years ago

What are the deafult PUID and PGID that Pi-Hole runs under right now? I'm creating an unraid template for myself and I need to know this, thanks.

diginc commented 5 years ago

The services run as mix of root and www-data, with some files owned by the pihole service account:

root@a70f4332e07f:/# id root
uid=0(root) gid=0(root) groups=0(root)
root@a70f4332e07f:/# id www-data
uid=33(www-data) gid=33(www-data) groups=33(www-data)
root@a70f4332e07f:/# id pihole
uid=999(pihole) gid=999(pihole) groups=999(pihole),33(www-data)
Exist2Resist commented 5 years ago

@diginc Thanks

jbpaux commented 5 years ago

That would explain why I can't do anything in the webgui (adding white list) etc. for example as the user is not correctly set

EsEnZeT commented 5 years ago

Similar stuff - I deploy on Qnap NAS + VPS.

I think it would be nice to have ability to specify UID & GID.

I hope this issue is still valid

diginc commented 5 years ago

Still on my board. FTL's service script (docker specific s6) needs to be updated to run as pi-hole user to accommodate not having a mix of 3 different users processes.

EsEnZeT commented 5 years ago

@diginc thank you for clarification and overall great work on that :)

lennyg1 commented 5 years ago

Same thing happens on QNAP NAS, the GUID en PUID aren't set so the files pihole needs in /etc/pihole are pretty much read only to the docker image if you want to mount them to store the data outside of the image. You can clearly see pihole wanting to create files with GID 999, while that one doesn't exist on the NAS.

tzapu commented 5 years ago

same thing happening on docker in ubuntu, every time i restart the service it can't read previosly saved config files i assume, so everything is reset and a new password regenerated.

great job with pi-hole, can t wait to be able to fully use it

arnemoor commented 5 years ago

As this is not a synology specific issue would it be possible to rename that issue? Something like "Use provided host PID / GID to allow host compatible file access" seems to me more meaningful for this feature request.

The mentioned "magic" from linuxserver.io is mainly the usage of a user account "abc" which the tweak to the given PID / GID, see https://github.com/linuxserver/docker-baseimage-alpine/blob/master/root/etc/cont-init.d/10-adduser

Sidenote: Best practice is to create a "technical docker user" on the NAS / host and use the values of that account for the containers.

celeroll commented 5 years ago

And is there currently a workaround to overcome the issue with permission 999 on shared folder "pihole"? I'm facing same error as here https://discourse.pi-hole.net/t/php-permissions-error-web-admin-page/14250

diginc commented 5 years ago

My last post in that thread leads to this post and it's reply about tweaking nas permissions to resolve folder share permissions conflicting with docker permissions.

celeroll commented 5 years ago

I solved this problem in the way that I executed this command before docker-compose on the shared folder: chmod 777 /volume1/apps/configs/pihole/

This allowed the system to create all files with necessary permissions to execute.

And my volume share for this looks like this: - /volume1/apps/configs/pihole/:/etc/pihole/

jcass8695 commented 4 years ago

Any movement on this?

dschaper commented 4 years ago

Diginc, do we need to add an entrypoint to chmod directories to a PID/GID? Or is this something more on the core package that needs to be able to install/run with user supplied UIDs?

diginc commented 4 years ago

Using the linuxserver io style adduser script works really well as long as your programs actually run as that same defined user.

Since this was last discussed we added customizing dnsmasq / FTL's user feature which helps. Spitballing the logic: IF PUID / PGID is passed in, it could modify the pre-existing pihole user to user those IDs and then force the dnsmasq user to pihole.

Next up is probably customizing www-data lighttpd user customization before this'll work. That is a fairly simple sed command.

jcass8695 commented 4 years ago

@diginc It sounds like everything I need has been implemented, just sanity checking with you. Adding

-e PUID=1000 \
-e GUID=1000 \
-e DNSMASQ_USER=1000

to my docker run will allow me to volume mount dnsmasq.d locally and be able to write to the conf files? Provided my local user is 1000.

diginc commented 4 years ago

@JCass45 Only the DNSMASQ_USER variable has been added so far. The PUID and GUID scripts haven't been added yet.

HNGamingUK commented 4 years ago

Do we know when PUID and PGID is going to be implemented as I currently keep getting:

cp: cannot create regular file '/etc/dnsmasq.d/01-pihole.conf': Permission denied

Even with DNSMASQ_USER set to 1000

kquinsland commented 4 years ago

I am another +1 for this.

I am trying to use pihole in a rootless container w/ Podman. I currently cant do this because there are three relevant user/group IDs inside the pi-hole container.

The only way to get pihole working is - sadly - to use xx7 level permissions on whichever directories the host mounts into the container. This way, the three IDs inside the container that need access to the volume can access them.

Haarolean commented 4 years ago

This permissions hell is a mess. And chmod'ing 777 is not a proper solution, it's a serious security issue.

matthewdennett commented 4 years ago

It would be awesometo see this fixed up. Having contorll over this make it really easy for this to slot straight into existing convensions.

araemo commented 4 years ago

Adding my vote here... it would be very good to be able to specify what UIDs/GIDs are chosen for pihole and www-data (www-data is less important, since it's somewhat standardized across distros, but 999 for pihole is in-use in both of my main distros for other services, and it complicates the permissions setup on the persistent files)

With the security changes in v5, I had to add the www-data user (UID 33 on ubuntu, and in this container) to the group systemd-coredump (GID 999 on ubuntu.. which is what is the 'pihole' GID is in this container) on my ubuntu NFS host so that the pihole www-data user (UID 33) could update files with 664 perms and uid:gid of 999:999. Being able to choose the UIDs/GIDs would allow me to set the permissions so that they don't overlap with other services on the same host. Then you don't have to worry so much about what the user names are inside the container - IDs just need to match what your external users/groups are set to.

kquinsland commented 4 years ago

The only reason this ticket is open is because the cardinal sin of putting more than one $thing in a container was broken. One user/group per $thing and one $thing per container.

The proper fix would be to break out the web server and web app from the rest of the piHole components into a series of containers that are composed into the service.

I think that keeping this ticket open is a good idea as it's currently the best place to discuss the issue as well as the fix but ultimately there will have to be some substantial work to de-couple the various components of the piHole service.

Is there any documentation on what binaries are used by piHole and how they're configured? I am only vaguely aware of the massive installer script and:

akusei commented 4 years ago

I'm having the same issue with this and find it quite annoying that you can't properly control permissions. I came up with a workaround though. I've only applied it for the pihole and www-data user but can be extended for any user in the container. Forgive the bad code, I didn't make it look pretty.

30-fix-permissions.sh

#!/bin/bash

modifyUser()
{
  declare username=${1:-} newId=${2:-}
  [[ -z ${username} || -z ${newId} ]] && return

  local currentId=$(id -u ${username})

  echo "user ${username} ${currentId} => ${newId}"
  usermod -o -u ${newId} ${username}

  find / -user ${currentId} -print0 2> /dev/null | \
    xargs -0 -n1 chown -h ${username} 2> /dev/null
}

modifyGroup()
{
  declare groupname=${1:-} newId=${2:-}
  [[ -z ${groupname} || -z ${newId} ]] && return

  local currentId=$(id -g ${groupname})

  echo "group ${groupname} ${currentId} => ${newId}"
  groupmod -o -g ${newId} ${groupname}

  find / -group ${currentId} -print0 2> /dev/null | \
    xargs -0 -n1 chgrp -h ${groupname} 2> /dev/null
}

modifyUser www-data ${WEB_UID}
modifyGroup www-data ${WEB_GID}
modifyUser pihole ${PIHOLE_UID}
modifyGroup pihole ${PIHOLE_GID}

Then in the docker-compose.yml file, add a volume that binds to the above file:

volumes:
  - 30-fix-permissions.sh:/etc/cont-init.d/30-permissions.sh:ro

The web user must be in the pihole group on the host and you have to specify the following environment variables

DNSMASQ_USER must be set to pihole and when this runs there will still be root owned files in your pihole volume but the actual processes will be running as whatever user you specify. I'm not sure if this will cause long term issues but so far it's working for me

neybar commented 3 years ago

Any progress? I'm getting this issue also. Would love to be able to specify UID/GID

imajes commented 3 years ago

Would also be a fan of this... on my machine, 999 maps to systemd-coredump which honestly i don't care for ... would much rather be able to specify the pihole user! thanks 👍

k-matti commented 3 years ago

Any progress on this? I have the issue with permission when using nfs share as a volume for pihole configs

modem7 commented 3 years ago

For me this would be useful so I don't have to run docker volumes (which makes backing up and restoring a PITA (I backup my folders daily for easier restore)) and can instead save pihole data in relevant folders with everything else whilst being in the docker group, rather than root.

katbyte commented 3 years ago

Adding another me to +1, its preventing me from running within docker using a shared networked CIFS volume (#750) and i'm not willing to use 777 permissions.

Also would probably fix #749

evanre commented 3 years ago

+1 looking forward to seeing this being fixed.

uysct commented 3 years ago

+1 it would be a great pleasure for me too

c0m3d1an commented 3 years ago

I solved this problem (finally) after running the container with --cap-add=setuid --cap-add=setgid

In kubernetes, add this to the appropriate spot in your YAML:

    securityContext:
      allowPrivilegeEscalation: true
      capabilities:
        add:
        - SETGID
        - SETUID
Klarstein commented 3 years ago

+1 It’s a good security practice to run rootless and use namespace remapping in Docker. The current workarounds (777, setgid/setuid capabilities) to use pihole, completely defy that principle and weaken security in case of a container breakout.

modem7 commented 3 years ago

@diginc @PromoFaux

Do you know if we have any official update on this particular feature from the dev team?

Would be good to see if there has been any updates behind the scenes, and where this particular feature may lie within the roadmap.

Is there anything the devs need from us for further info or to assist in implementation/testing?

kquinsland commented 3 years ago

Is there anything the devs need from us for further info or to assist in implementation/testing?

The only fix is to break up the various binaries inside the image. The current pihole docker image is an anti pattern but because most people running pihole on docker actually want the simpicity that comes from that anti pattern, i suspect very little will change any time soon.

In the short term, it probably wouldn't be too hard to create a set of 'decomposed' containers using multi-stage builds, but in reverse:

# Load the 'composed' pihole image
FROM pihole as donor

FROM alpine:latest as dns
COPY --from=donor /usr/bin/pihole-ftl  ...
...

FROM nginx-php-fpm:latest as web
COPY --from=donor /etc/lighttpd/...
...

Not quite as elegant as building proper containers from the beginning, but probably how it'll get done if anybody can find the time to donate / get it done.

dschaper commented 3 years ago

Might be something to address in https://github.com/pi-hole/docker-pi-hole/issues/735

kquinsland commented 3 years ago

@dschaper I believe it was addressed:

I had started to work on updating the workflow but I find it very difficult to understand and apprehend this repository. We depend on shell and python scripts whereas a simple docker build should be enough.

So I started again and made a new image from scratch but the study of the basic-install.sh script is complicated to take into account. A lot of static items and system dependencies prevent us from a generic and agnostic approach to create a simple image.

de-composing PiHole is non trivial.

PromoFaux commented 3 years ago

The only fix is to break up the various binaries inside the image.

Just to note that:

  • the web server
  • the php process
  • the pihole fork of dnsmasq which also? reads the DB file that the php process manages?

Work is happening in the background currently for Pi-hole 6.0, in which the web server will be embedded in to FTL (our dnsmasq "fork") and PHP will also hopefully be going away entirely....

Klarstein commented 3 years ago

Awesome! Thank you for your hard work! In the meantime I just use pihole by manually editing the config files.

On 27 Feb 2021, at 13:36, Adam Warner notifications@github.com wrote:

 The only fix is to break up the various binaries inside the image.

Just to note that:

the web server the php process the pihole fork of dnsmasq which also? reads the DB file that the php process manages? Work is happening in the background currently for Pi-hole 6.0, in which the web server will be embedded in to FTL (our dnsmasq "fork") and PHP will also hopefully be going away entirely....

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or unsubscribe.

kquinsland commented 3 years ago

The only fix is to break up the various binaries inside the image.

Just to note that:

  • the web server
  • the php process
  • the pihole fork of dnsmasq which also? reads the DB file that the php process manages?

Work is happening in the background currently for Pi-hole 6.0, in which the web server will be embedded in to FTL (our dnsmasq "fork") and PHP will also hopefully be going away entirely....

Good to know! Just curious, why not have a standalone web server if the FTL binary has an API?

PromoFaux commented 3 years ago

If FTL can act as it's own webserver, why have a separate one? Obviously nothing stopping users, then, from reverse proxying via nginx or whatever.

work is happening here https://github.com/pi-hole/ftl/tree/new/http and here https://github.com/pi-hole/AdminLTE/tree/new/FTL_is_my_new_home for those following along at home

github-actions[bot] commented 2 years ago

This issue is stale because it has been open 30 days with no activity. Please comment or update this issue or it will be closed in 5 days.

kquinsland commented 2 years ago

This issue is stale because it has been open 30 days with no activity. Please comment or update this issue or it will be closed in 5 days.

Yep. Still an issue.

Somebody should make a bot that watches for the "about to be stale" bot and keeps things open.

PromoFaux commented 2 years ago

keeps things open.

Added an exemption label.

No status update to share on this - lots of other things to untangle first (including tidying up old and stale issues)

dsm1212 commented 2 years ago

A couple messages back it mentions setting DNSMASQ_USER to 1000. I can see in the start script that this will be used also for starting pihole-FTL which seems odd, but anyhow it doesn't work because there is no 1000 user in the passwd file. The container just prints an error about no user 1000 and restarts. I don't know how that feature is working for anyone. Is there a pihole docker that supports UID/GID correctly? I hate having this mess on my nas. Is too hard to think through the security risk. I'm tempted to overlay my own password file just to change the numbers but then I'll have to manually update the container all the time.

PromoFaux commented 2 years ago

I can see in the start script that this will be used also for starting pihole-FTL which seems odd,

Maybe that variable can be renamed. pihole-FTL is a drop-in replacement for dnsmasq, indeed dnsmasq is embedded directly into pihole-FTL

By default, this user is set to pihole (was root up until recently)

dsm1212 commented 2 years ago

Ah, that makes sense. A pihole user is good, we just need two variables for setting the UID/GID. I think www-data for lighthttpd is not so bad. Heck if the pihole user just had a big number UID that is not already present in these liniux distros it would be manageable. Tying it to systemd-coredumper, or whatever, means we have to try to figure out how to secure what that account can do.

PromoFaux commented 2 years ago

just

Some things are easier said than done. But we hear you - things "just" take time to get sorted. We've been working on various improvements lately

imajes commented 2 years ago

it's a fair point... but it's worth noting that given the widespread use of pihole, and it's leading-edge presence, it does feel like getting this right from a security POV is fairly high priority....