feat(unprivileged): run unprivileged nginx to support arbitrarily assigned user ID #3033

m4s-b3n · 2022-04-22T12:19:11Z

Use unprivileged nginx to support arbitrarily assigned user ID to be able to run the image e.g. on Red Hat Openshift

Description

The dockerfile uses the nginxinc/nginx-unprivileged image instead of the standard nginx image.
Moreover the nginx.conf file was extended for some configuration to locate nginx runtime data in the /tmp directory.

This enables the container to run as a non-root user which is an improvement to the container security and enables running the container on distributions such as OpenShift by default.

Motivation and Context

The change is necessary as the previous version required nginx to run as root to be able to read/write so several default paths.

Fixes #2931

How Has This Been Tested?

Dependencies: npm ci
Build: npm build
Docker Build: docker build -t swagger-editor-unprivileged .
Docker Run: docker run -p 8080:8080 -u 100000 swagger-editor-unprivileged
Visit in browser: http://localhost:8080

My PR contains...

No code changes (src/ is unmodified: changes to documentation, CI, metadata, etc.)
Dependency changes (any modification to dependencies in package.json)
Bug fixes (non-breaking change which fixes an issue)
Improvements (misc. changes to existing features)
Features (non-breaking change which adds functionality)

My changes...

are breaking changes to a public API (config options, System API, major UI change, etc).
are breaking changes to a private API (Redux, component props, utility functions, etc.).
are breaking changes to a developer API (npm script behavior changes, new dev system dependencies, etc).
are not breaking changes.

Documentation

My changes do not require a change to the project documentation.
My changes require a change to the project documentation.
If yes to above: I have updated the documentation accordingly.

Automated tests

My changes can not or do not need to be tested.
My changes can and should be tested by unit and/or integration tests.
If yes to above: I have added tests to cover my changes.
If yes to above: I have taken care to cover edge cases in my tests.
All new and existing tests passed.

char0n · 2022-06-13T13:02:28Z

Hi @marius-boden-sva,

Thank you for the PR! I'll have some questions on the changes to fully understand what have been changed and why if that's OK.

char0n

Hi @marius-boden-sva, thanks again for the PR. Can you please address questions I had in the code review of this PR. Thanks!

char0n · 2022-06-13T13:04:15Z

Dockerfile

@@ -1,4 +1,4 @@
-FROM nginx:1.21.6-alpine
+FROM nginxinc/nginx-unprivileged:1.21.6-alpine


This is an official image, so we should be fine. The image difference from nginx is as follows:

The default NGINX listen port is now 8080 instead of 80.

The default NGINX user directive in /etc/nginx/nginx.conf has been removed.

The default NGINX PID has been moved from /var/run/nginx.pid to /tmp/nginx.pid. *Change _temp_path variables to /tmp/.

@marius-boden-sva I want to address specifically The default NGINX listen port is now 8080 instead of 80. Will this affect the end users; will this be a breaking change for them? I assume not because of https://github.com/swagger-api/swagger-editor/pull/3033/files#diff-dd2c0eb6ea5cfc6c4bd4eac30934e2d5746747af48fef6da689e85b752f39557L19 bug just asking to be sure.

The default port has to be changed because ports below 1024 are privileged ports and require root privileges. For running the editor with docker only this results in changing the mapped port only (-p 80:80 will become -p 80:8080). Reminder: need to update the docs about that ;)
For end users running swagger editor e.g. with compose or in kubernetes this will be a breaking change as they need to adapt their ports.

So it is going to be a breaking change. We have a version 5 comming up where we could possibly do this change.

char0n · 2022-06-13T13:05:34Z

Dockerfile

 COPY ./docker-run.sh /usr/share/nginx/

-
-RUN chmod +x /usr/share/nginx/docker-run.sh && \


Can you maybe elaborate why we're removing this chmod?

we do not need this as the docker-run.sh script is already checked into git with -rwxr-xr-x permissions

char0n · 2022-06-13T13:08:14Z

nginx.conf

-worker_processes      1;
+worker_processes      auto;
+
+error_log  /var/log/nginx/error.log notice;


Can you collaborate why changing the error_log?

this ist just a merge of your your configuration an the one from the nginxinc/nginx-unprivileged:1.21.6-alpine image

char0n · 2022-06-13T13:09:58Z

nginx.conf

 }

 http {
+  proxy_temp_path /tmp/proxy_temp;


I assume these changes are all necessary to be able to run as a non root, unprivileged user, right?

see my answer above. my thought was to change as little as possible and only integrate what's necessary to run swagger editor

char0n · 2022-06-13T13:10:09Z

nginx.conf

+  uwsgi_temp_path /tmp/uwsgi_temp;
+  scgi_temp_path /tmp/scgi_temp;
+
+  include /etc/nginx/conf.d/*.conf;


Why we're including the default config here?

see my answer above

char0n · 2022-11-16T08:38:50Z

@marius-boden-sva would you mind sending this PR against next branch? Now it's a good opportunity to push this effort forward.

Thanks!

…igned user ID

m4s-b3n · 2022-11-21T09:49:08Z

@char0n done: #3697

Co-authored-by: marius-boden-sva Refs #3033 Refs #3697

char0n · 2022-11-24T12:50:10Z

Superseded by #3705

Co-authored-by: marius-boden-sva Refs #3033 Refs #3697

# [5.0.0-alpha.38](v5.0.0-alpha.37...v5.0.0-alpha.38) (2022-11-24) ### Features * **docker:** build and publish unpriviledged image ([#3705](#3705)) ([30ce794](30ce794)), closes [#3033](#3033) [#3697](#3697)

char0n self-assigned this May 6, 2022

char0n added type: enhancement cat: docker labels May 6, 2022

char0n self-requested a review June 13, 2022 13:01

char0n reviewed Jun 13, 2022

View reviewed changes

char0n added the swagger-editor@next label Jul 6, 2022

feat(unprivileged): run unprivileged nginx to support arbitrarily ass…

486c560

…igned user ID

char0n added a commit that referenced this pull request Nov 24, 2022

feat(docker): build and publish unpriviledged image

93f297b

Co-authored-by: marius-boden-sva Refs #3033 Refs #3697

char0n mentioned this pull request Nov 24, 2022

feat(docker): build and publish unpriviledged image #3705

Merged

char0n closed this Nov 24, 2022

char0n added a commit that referenced this pull request Nov 24, 2022

feat(docker): build and publish unpriviledged image (#3705)

30ce794

Co-authored-by: marius-boden-sva Refs #3033 Refs #3697

		@@ -1,4 +1,4 @@
		FROM nginx:1.21.6-alpine
		FROM nginxinc/nginx-unprivileged:1.21.6-alpine

		COPY ./docker-run.sh /usr/share/nginx/


		RUN chmod +x /usr/share/nginx/docker-run.sh && \

feat(unprivileged): run unprivileged nginx to support arbitrarily assigned user ID #3033

feat(unprivileged): run unprivileged nginx to support arbitrarily assigned user ID #3033

Uh oh!

Conversation

m4s-b3n commented Apr 22, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Motivation and Context

How Has This Been Tested?

My PR contains...

My changes...

Documentation

Automated tests

Uh oh!

char0n commented Jun 13, 2022

Uh oh!

char0n left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

m4s-b3n Jun 16, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

m4s-b3n Jun 16, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

char0n commented Nov 16, 2022

Uh oh!

m4s-b3n commented Nov 21, 2022

Uh oh!

char0n commented Nov 24, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

m4s-b3n commented Apr 22, 2022 •

edited

Loading

m4s-b3n Jun 16, 2022 •

edited

Loading

m4s-b3n Jun 16, 2022 •

edited

Loading