Skip to content
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

[feat] Run Docker container as non-root user #783

Open
Laynezilla opened this issue Mar 1, 2021 · 17 comments
Open

[feat] Run Docker container as non-root user #783

Laynezilla opened this issue Mar 1, 2021 · 17 comments
Assignees
Labels
enhancement New feature or request

Comments

@Laynezilla
Copy link

Is your feature request related to a problem? Please describe.
The Docker container is currently running as root which is a security issue.

Describe the solution you'd like
Have environmental variables specify UID and GID similar to this.

Describe alternatives you've considered
Hardcode user 1001 or something similar to this.

Additional context
Thanks for the great software!

@Laynezilla Laynezilla added the enhancement New feature or request label Mar 1, 2021
@robertsLando
Copy link
Member

@Laynezilla Would you like to submit a PR for this?

@Laynezilla
Copy link
Author

@Laynezilla Would you like to submit a PR for this?

I don't have time at the moment to work on this but I can give it a shot at some point. Also, I don't have any coding background, only tinkering, so if anyone else wants to attempt that might be better haha.

@ChrisRomp
Copy link

The way that linuxserver.io handles it is they run everything in their container as user:group abc:abc. When starting a container they look for the PUID and PGID to set the abc user/group to.

The Docker Node image, which is the base for this image, provides some guidance for running as non-root.

I'll play around with the Dockerfile locally and see if I can get it to run under the node user instead of root, and then assign a custom PUID/PGID to node. If I get that working I'll submit a PR, but additional testing should be done to ensure it won't break anything else.

@ChrisRomp
Copy link

There's a bit of overhead to how linuxserver.io does it. I've done some experimentation and without adding more "things" to the base image I haven't found a good way to do this.

In a nutshell, what needs to happen at container start is:

  1. Modify user and group ID for the node user
  2. chown -R node:node /usr/src/app
    • This takes too long - probably because of node_modules
  3. Run node bin/www from the /usr/src/app directory

The fastest/easiest solution for this is to just run under the existing node user, which as a UID/GID of 1000. This can be handled in the Dockerfile by copying the files as node:node and then setting USER node before executing the app. I'll submit a PR referencing this issue.

I've run the container and it looks like it's ok, but I haven't tested it with a controller attached.

@scyto
Copy link
Contributor

scyto commented Jan 4, 2022

please don't do this by default, let it be configured if folks want, just not by default - its a PITA on many docker platforms (thinking synology here)

linuxserver:io containers are needlessly complex and fragile, i wouldn't hold them up as a good example of creating containers

an example of container that is like this is grafana - it is a absolute nightmare to get running

@scyto
Copy link
Contributor

scyto commented Jan 4, 2022

oh and only privileged containers are actually root, the security aspect of running as root insider a container is overblown outside of that mode

if folks are worried about vulnerabilities in docker they should mitigate using rootless mode of the docker daemon https://docs.docker.com/engine/security/rootless/

note, running in non-root in the container doesn't mitigate (i know everyone says it does, but it doesn;t)

@ChrisRomp
Copy link

@scyto This issue is long closed. I think you're safe. :)

@ChrisRomp
Copy link

Or I guess the PR is closed. Not sure why the issue is still open if the devs don't want to do it.

@scyto
Copy link
Contributor

scyto commented Jan 4, 2022

Or I guess the PR is closed. Not sure why the issue is still open if the devs don't want to do it.

it was closed by the bot due to inactivity, to be clear i think its fine to modify to let users run as any GID/UID they want, but to change default to non-root is my issue as it provides minimal protection to any actual threat but breaks a lot.

@ftc2
Copy link

ftc2 commented Jul 1, 2024

i was surprised to see this running as root. uh, wat???

anyway, i worked around it by adding this to the service definition in compose:

user: "${PUID}:${PGID}"
group_add:
  - ${DIALOUT_GID}

this runs node as PUID:GUID (i do 2000:2000) and adds that user to the audio group inside the container. the group is necessary to access /dev/zwave.

for completeness, here is my solution for rootless HA + zwave-js-ui that i wish i had this morning:

compose project's .env file:

TZ=US/Eastern
RESTART_POLICY=unless-stopped

PUID=2000
PGID=2000

# home-assistant
HA_PROXY_HOST=homeassistant.example.com

# zwave-js-ui
SESSION_SECRET=foo
DIALOUT_GID=18

the last line (DIALOUT_GID) is the gid that has access to your zwave device. you can add it to the file like this if you want:

echo "DIALOUT_GID=$(stat -Lc %g /dev/serial/by-id/usb-Zooz_800)" >> .env

docker-compose.yml:

services:
  home_assistant:
    image: lscr.io/linuxserver/homeassistant
    container_name: home_assistant
    restart: ${RESTART_POLICY}
    networks:
      - wan
      - traefik_home_assistant
    # ports:
    #   - "8123:8123" # webui
    environment:
      - PUID
      - PGID
      - TZ
    volumes:
      - ./ha_conf/:/config/
    labels:
      - "traefik.enable=true"
      - "traefik.docker.network=traefik_home_assistant"
      #
      - "traefik.http.routers.home_assistant.rule=Host(`${HA_PROXY_HOST}`)"

  zwave_js:
    image: zwavejs/zwave-js-ui
    container_name: zwave_js
    restart: ${RESTART_POLICY}
    user: "${PUID}:${PGID}"
    group_add:
      - ${DIALOUT_GID}
    tty: true
    stop_signal: SIGINT
    networks:
      - wan
    ports:
      - "8091:8091" # webui
    healthcheck:
      test: 'wget --no-verbose --spider --no-check-certificate --header "Accept: text/plain" http://localhost:8091/health || exit 1'
      interval: 1m
      timeout: 10s
      start_period: 30s
      retries: 3
    environment:
      - TZ
      - SESSION_SECRET
      - ZWAVEJS_EXTERNAL_CONFIG=/usr/src/app/store/.config-db
    devices:
      - /dev/serial/by-id/usb-Zooz_800:/dev/zwave
    volumes:
      - ./zwave_js_conf/:/usr/src/app/store/

networks:
  wan:
  traefik_home_assistant: # docker network create --internal traefik_home_assistant
    external: true

n.b.

chown zwave-js' conf dir as needed

also, to make traefik work, add this to HA's configuration.yaml

http:
  use_x_forwarded_for: true
  trusted_proxies:
    - 0.0.0.0/0 # disable the security check, lol

@scyto
Copy link
Contributor

scyto commented Jul 2, 2024

i was surprised to see this running as root. uh, wat???

when a container runs internally as root it doesn't mean it has root access to the host process or host file system in general - you do know that right? it isn't running as host root

also groups in containers / hosts can have random naming based on the groupIDs so its not very portable in terms of other folks group names, not to mention the docker daemon still runs as root and you still have the same risks of privilege escalation if there is a docker zero-day...

if you think there is a potential security issue look into running docker as rootless as a solution

@ftc2
Copy link

ftc2 commented Jul 2, 2024

i'm familiar with namespaces, yes. that doesn't mean running container processes as root is without risk, so doing that without a good, articulable reason is not a defensible practice. running docker rootless is also wise (i do this myself when possible), but that's out of scope.

it isn't running as host root

actually, i don't think it's common for docker to use userns-remap by default, lol. so yes root insider the container is actually root outside

as for the group name, how portable does it need to be? it just needs to apply to this image, right?

it looks like the image has a group audio with gid 18 (which i'm relying on to grant rw access to the zwave device). on my docker host, audio has gid 63, and this appears to be irrelevant. in my testing, supplying group_add with the group name instead of gid produces the desired result:

$ docker exec -it zwave_js id
uid=2000 gid=2000 groups=18(audio),2000

ok, but which is more likely to be more stable in the image over time – the name or the gid? that's a fair question, and i'm too lazy to look at the image to answer that. like are the permissions on /dev/zwave set up based on name or gid, and what created the audio group to begin with and how?

anyway, my solution's there in my prev post in case it helps someone.

@robertsLando
Copy link
Member

@ftc2 Thanks for sharing your rootless setup!

I'm not a docker expert and I would be open to any suggestion that improves security. Anyway I also don't want users to start blaming me because something isn't working like before in their setup, for lot users even installing docker on the PC is an hard task so we need to keep everything as simple as possible but also let advanced users like you to be able doing this kind of things

@ftc2
Copy link

ftc2 commented Jul 2, 2024

I also don't want users to start blaming me

i can sympathize.

there are probably 3 options in descending order of preference:

  1. if you care about this admittedly low priority issue, i'd probably update the documentation so that new users run it as rootless by doing something like my solution. i'd also have a note for existing users that may run into permissions issues by using the newly supplied docker config blindly: "be sure to chown your persistent data volume to restore access". this way, new users are rootless by default, and users with existing data re-doing their docker stuff are (optimistically) likely to see that note alongside the setup steps in the documentation and avoid that pitfall. not sure i'd bother with this last part, but there could also be some mechanism in your container to allow node to start if permission to the persistent data is denied, and it could serve a helpful error via the webui with a hint to fix the permissions. that might be more visible to clueless users than checking the docker logs, idk.

  2. have some in-container mechanism to migrate existing users to rootless. i'm not an expert in this, but i guess i could look into it if you're curious. the problem with this approach is it's somewhat less secure and also adds complexity. you'd probably rather invest your time in something else, ha. but if i had to guess, a reasonable solution may involve using cap_drop (--cap-drop in docker run) ALL and then cap_add the minimal set of capabilities back in to accomplish this task – probably just enough for your init script to re-chown the persistent data and then run node as a safe UID spec'd by an env var.

  3. make the image rootless as a breaking change without implementing a migration-to-rootless feature. lol

@robertsLando
Copy link
Member

I'm ok for point 1 if you wish to submit a PR to improve docs about this, the right place is: https://github.com/zwave-js/zwave-js-ui/blob/d66bcb9cff1e48bf6e6a95fdbcd411f3c4a9dfae/docs/getting-started/docker.md

@ftc2
Copy link

ftc2 commented Jul 2, 2024

also groups in containers / hosts can have random naming based on the groupIDs so its not very portable in terms of other folks group names

thanks, @scyto, you are right. i looked at it again, and i am no longer convinced that what i said is completely correct. i'm trying to come up with a better way to do that part now.

i guess what you would really want to do is something like:

docker run --group-add="$(stat -Lc %g /dev/serial/by-id/usb-Zooz_800)" ...

but docker compose does not support statement execution like that. you can only do interpolation of variables from static sources.

i suppose something like this could be an option:

echo "DIALOUT_GID=$(stat -Lc %g /dev/serial/by-id/usb-Zooz_800)" >> .env

and then in the compose file:

group_add:
  - ${DIALOUT_GID}

and then on top of that, if the docker implementation is using userns (e.g. the userns-remap feature in docker), i think you have to manually fix the mapping yourself.

@mhrivnak
Copy link

It is important and valuable to run as non-root inside a container, even if the container runtime is running as non-root on the host (aka "rootless docker" or just the default mode with podman). Among other things, it limits what an attacker could do if they compromise the application. If the application is running as root, it can modify its entire userspace, including replacing binaries or application code with compromised versions. That's a big reason, among others, why it is a widely-accepted best practice to set a non-root user inside containers.

More references:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants