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

Switch container image to nginx-unprivileged #20627

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

Lykos153
Copy link

@Lykos153 Lykos153 commented Jan 18, 2022

This is another take on #17927 which was reverted in #17990. I'm approaching this by using the alpine version of the [official nginx-unprivileged image]

Fixes #25926

(https://hub.docker.com/r/nginxinc/nginx-unprivileged) which uses port 8080 by default.

Why use an unprivileged container?

  • It reduces attack surface. Though isolated by namespaces, being root in a container is still more powerful than being user in a container.
  • As commented in Don't run nginx as root in docker #17927, the container as it is already drops root privileges after opening the TCP socket. However, some environments (eg. OpenShift) don't allow root containers to be run at all, even if they drop privileges afterwards.
  • Unprivileged containers can be more complicated to handle when dealing with persistent volumes. As this container only serves static content, this isn't an issue and there is no reason to run it as root.

Changes in this PR:

  • Base container image on nginxinc/nginx-unprivileged
  • Move document root to /app
  • Switch port to 8080
  • Update the commands in the documentation so the result is the same as before

I understand that there might be concern that changing the container port could break existing deployments. I can see multiple ways to deal with this:

  • Bump the version in a way that indicates incompatibility. Though, as the tags mirror the app version, this is probably not feasible.
  • Build two versions of the image, so we have eg. v1.9.9 and v1.9.9-unprivileged. Optionally, there could be a deprecation phase after which there won't be any root-versions anymore.
  • Create an image that can handle being run as root or as a user. This image would by default run as root and provide the service on port 80, but if run as a user, would not fail but use a configuration that uses port 8080.

This PR currently has none of the required changelog labels.

A reviewer can add one of: T-Deprecation, T-Enhancement, T-Defect, T-Task to indicate what type of change this is, or add Type: [enhancement/defect/task] to the description and I'll add them for you.

Signed-off-by: Silvio Ankermann <silvio@booq.org>
@Lykos153 Lykos153 requested a review from a team as a code owner January 18, 2022 17:29
Copy link
Member

@turt2live turt2live left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks fine, though the port change means we need to communicate this in advance. Will talk to the app team about this.

@ibotty
Copy link

ibotty commented Oct 4, 2022

Any news? This would make it possible to run in constrainted environments (e.g. OpenShift).

@t3chguy
Copy link
Member

t3chguy commented Aug 4, 2023

According to the image docs

The default NGINX listen port is now 8080 instead of 80 (this is no longer necessary as of Docker 20.03 but it's still required in other container runtimes).

We can revert back to port 80 to minimise the migration risk or provide two versions of the image as per your suggestion

Build two versions of the image, so we have eg. v1.9.9 and v1.9.9-unprivileged. Optionally, there could be a deprecation phase after which there won't be any root-versions anymore.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

1 similar comment
@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

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

Successfully merging this pull request may close these issues.

Consider providing a Docker image that doesn't run as root
5 participants