element-web icon indicating copy to clipboard operation
element-web copied to clipboard

Switch container image to nginx-unprivileged

Open Lykos153 opened this issue 3 years ago • 4 comments

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 https://github.com/vector-im/element-web/issues/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 #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.

Lykos153 avatar Jan 18 '22 17:01 Lykos153

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

ibotty avatar Oct 04 '22 14:10 ibotty

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.

t3chguy avatar Aug 04 '23 15:08 t3chguy

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.

CLAassistant avatar Sep 06 '24 17:09 CLAassistant

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.

CLAassistant avatar Sep 06 '24 17:09 CLAassistant

Thanks for the contribution, and sorry for letting this sit for two years without progress.

I talked to the team about this today, and we aren't happy to land it as-is, due to the fact it changes behaviour in an incompatible way.

As an alternative, I've created https://github.com/element-hq/element-web/pull/28849 which I hope will achieve the same result without breaking backwards compatibility,.

richvdh avatar Jan 02 '25 17:01 richvdh