Fix permissions on unitd tmp directory

[jhujhiti]

New Behavior

This fixes file permissions on the nginx unitd tmp directory. The existing Docker build creates this directory as root:root/0775, so unitd can't write there.

Contrast to Current Behavior

I discovered these incorrect permissions when attempting to create a very large number of devices with a single, gigantic POST. The request would fail with a 500 and the container would log [alert] 15#22 *204 mkstemp(/opt/unit/tmp//req-XXXXXXXX) failed (13: Permission denied). The unitd installation docs say that this location is "used to dump large request bodies", so it makes sense that it needs to be writable by the user running the daemon.

Discussion: Benefits and Drawbacks

Probably goes without saying that there are no drawbacks to a bugfix :)

Changes to the Wiki

None

Proposed Release Note Entry

"Fixed an issue where requests with large bodies would result in a 500 error."

Double Check

• I have read the comments and followed the PR template.
• I have explained my PR according to the information in the comments.
• My PR targets the develop branch.

[cimnine]

Are you sure this helps? Because unit is not run as user unit, but as whatever we tell it to in the docker-compose.yml, by default with the user id 101.

Shouldn't rather the next line be changed to chmod -R o+w media /opt/unit/tmp, i.e. so that others (to which uid 101 IMO belongs) can write to these directories.

@tobiasge, wdyt? Can you recall, why we chose g+w?

[tobiasge]

We used g+w because the containers run with the group ID 0 (root). The directories belong to that group.
We also did this because the user ID is chosen as a random value in Openshift or Kubernetes but the group ID in those systems is also set to 0.

[jhujhiti]

Oops - I run this in K8s and I completely forgot to check what docker-compose was doing. I'll figure out how to get both working.

[jhujhiti]

I see what's happening now. If the container is started as root, which it is by default, unitd is able to drop privileges, and it drops to using the group unit, which means that g+w on the directories owned by root doesn't work. If I execute the container as unit (ie., 101), it can't drop privileges, so it ends up retaining the root group and can write to those directories.

Is there any reason not to set USER unit in the Dockerfile?

[maximumG]

I raised github issue #506 to alert about this problem. We ran into the same issue as you when trying to make some big bulk update queries, that do not fit into Nginx memory buffer.

We run the container as root as well (the default), and had to use a customized Dockerfile to introduce the same change you proposed and its working just fine now.

chown unit:unit -R /opt/unit/tmp

[jhujhiti]

If you're running in K8s, set the securityContext.runAsUser to 101 like the docker-compose does instead, which is what I did as soon as I saw the first reply to this PR and realized that docker-compose wasn't running with the default user.

[cimnine]

If the container is started as root, which it is by default, unitd is able to drop privileges, and it drops to using the group unit, which means that g+w on the directories owned by root doesn't work. If I execute the container as unit (ie., 101), it can't drop privileges, so it ends up retaining the root group and can write to those directories.

Wouldn't the actual fix then be to tell unit to become the user 101 instead of the user unit when it's started as root? Either on the NetBox application or during startup, of which I lean towards the latter.

Which would mean to change the lines in docker/launch-netbox.sh to something like the following:

exec unitd \
          --no-daemon \
          --control unix:$UNIT_SOCKET \
          --pid /opt/unit/unit.pid \
          --log /dev/stdout \
          --state /opt/unit/state/ \
          --tmp /opt/unit/tmp/ \
          --user 101

or maybe

exec unitd \
          --no-daemon \
          --control unix:$UNIT_SOCKET \
          --pid /opt/unit/unit.pid \
          --log /dev/stdout \
          --state /opt/unit/state/ \
          --tmp /opt/unit/tmp/ \
          --group root

[jhujhiti]

If done that way, I would personally specify both --user and --group. That's a possible solution, although I'm not sure what happens when unitd tries to start with those explicit settings and finds that it can't drop privileges since it's not root... Right now it logs a warning and continues, but the settings aren't explicit.

But it seems safest to set the USER in the Dockerfile unless there's a reason not to. There are mechanisms in K8s to disallow running as root (actually, any UID, but 0 is surely the most commonly disallowed UID), so even with the --user/--group fix, someone might find that the container would refuse to start with default settings in such an environment. Somebody migrating from docker-compose to K8s might be surprised by this since docker-compose would be doing it for them.

Reference to the K8s policy I have in mind: https://kubernetes.io/docs/concepts/policy/pod-security-policy/#users-and-groups

MustRunAsNonRoot - Requires that the pod be submitted with a non-zero runAsUser or have the USER directive defined (using a numeric UID) in the image. Pods which have specified neither runAsNonRoot nor runAsUser settings will be mutated to set runAsNonRoot=true, thus requiring a defined non-zero numeric USER directive in the container. No default provided. Setting allowPrivilegeEscalation=false is strongly recommended with this strategy.

[cimnine]

@tobiasge the suggestions in this PR were addressed with the change in #546, weren't they?

[tobiasge]

@tobiasge the suggestions in this PR were addressed with the change in #546, weren't they?

Yes