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

Docker: run as non-root #28849

Merged
merged 6 commits into from
Jan 3, 2025
Merged

Docker: run as non-root #28849

merged 6 commits into from
Jan 3, 2025

Conversation

richvdh
Copy link
Member

@richvdh richvdh commented Jan 2, 2025

Notes: The docker image now runs as a non-root user

Empirically, listening on port 80 with non-root works just fine nowadays, so we may as well do that.

The hacks to nginx.conf are based on "Running nginx as a non-root user" in https://hub.docker.com/_/nginx (and the dockerfile for nginxinc/nginx-unprivileged). Turns out we don't need many hacks, for our usecase.

Based on #28840
Fixes #25926

@richvdh richvdh requested a review from a team as a code owner January 2, 2025 17:03
@richvdh richvdh requested a review from dbkr January 2, 2025 17:03
@richvdh richvdh changed the base branch from develop to rav/docker/configurable_port January 2, 2025 17:03
Dockerfile Outdated Show resolved Hide resolved
Base automatically changed from rav/docker/configurable_port to develop January 3, 2025 12:02
@richvdh richvdh added this pull request to the merge queue Jan 3, 2025
Merged via the queue into develop with commit 3fb10ba Jan 3, 2025
38 checks passed
@richvdh richvdh deleted the rav/docker/unprivileged branch January 3, 2025 23:12
remram44 added a commit to remram44/matrix-helm that referenced this pull request Jan 4, 2025
Comment on lines +29 to +33
RUN sed -i -e 's,/var/run/nginx.pid,/tmp/nginx.pid,' /etc/nginx/nginx.conf

# nginx user must own the cache and etc directory to write cache and tweak the nginx config
RUN chown -R nginx:0 /var/cache/nginx /etc/nginx
RUN chmod -R g+w /var/cache/nginx /etc/nginx

Choose a reason for hiding this comment

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

Rather than having to maintain two tmp file systems (/tmp and /var/cache), why not move both to the same place, e.g. /tmp?

(Sadly it seems impossible to prevent the creation of a PID file at all.)

Choose a reason for hiding this comment

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

Ah, and by the way rather than modifying the config via a scary regexp you could just launch nginx via a parameter override:

nginx -g "pid /tmp/nginx.pid;"

Copy link
Member

Choose a reason for hiding this comment

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

Contributions welcome!

Choose a reason for hiding this comment

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

I tried to contribute under terms of AGPL-3.0, but I won't sign CLAs.

bmwiedemann pushed a commit to bmwiedemann/openSUSE that referenced this pull request Jan 15, 2025
https://build.opensuse.org/request/show/1238072
by user dheidler + anag+factory
- Version 1.11.90
  ## ✨ Features
  * Docker: run as non-root (element-hq/element-web#28849). Contributed by @richvdh.
  * Docker: allow configuration of HTTP listen port via env var (element-hq/element-web#28840). Contributed by @richvdh.
  * Update matrix-wysiwyg to consume WASM asset (element-hq/element-web#28838). Contributed by @t3chguy.
  * OIDC settings tweaks (element-hq/element-web#28787). Contributed by @t3chguy.
  * Delabs native OIDC support (element-hq/element-web#28615). Contributed by @t3chguy.
  * Move room header info button to right-most position (element-hq/element-web#28754). Contributed by @t3chguy.
  * Enable key backup by defa
bmwiedemann pushed a commit to bmwiedemann/openSUSE that referenced this pull request Jan 15, 2025
https://build.opensuse.org/request/show/1238071
by user dheidler + anag+factory
- Version 1.11.90
  ## ✨ Features
  * Docker: run as non-root (element-hq/element-web#28849). Contributed by @richvdh.
  * Docker: allow configuration of HTTP listen port via env var (element-hq/element-web#28840). Contributed by @richvdh.
  * Update matrix-wysiwyg to consume WASM asset (element-hq/element-web#28838). Contributed by @t3chguy.
  * OIDC settings tweaks (element-hq/element-web#28787). Contributed by @t3chguy.
  * Delabs native OIDC support (element-hq/element-web#28615). Contributed by @t3chguy.
  * Move room header info button to right-most position (element-hq/element-web#28754). Contributed by @t3chguy.
  * Enable key backup by defa
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

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