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

Modify docker file to use non-root user #45

Merged
merged 4 commits into from Oct 10, 2019

Conversation

@ahmedkrmn
Copy link

commented Oct 8, 2019

As requested in issue #43, this PR runs the application in a restricted user as opposed to the current run as root by default.

Changes:
A new group and user with name ohmyformUser and id of 999 are created. Docker is then set to use this new user (non-root) in all of the operations.

Copy link

left a comment

Looking at the order of things here worries me. Switching to the unprivileged user that early in the dockerfile will most certainly break the apk operations. Otherwise adding the user just named ohmyform or app would be fine. I like the idea of using UID / GID 999 since that isn't the typical first user created on a server so thats probably wise.

Thanks for your submission but please make sure that the app works post changes before submitting a PR especially if it's not marked as a work in progress [WIP] in the title.

Dockerfile Outdated Show resolved Hide resolved
@ahmedkrmn ahmedkrmn changed the title Modify docker file to use non-root user [WIP] Modify docker file to use non-root user Oct 9, 2019
ahmedkrmn added 2 commits Oct 9, 2019
@wodka

This comment has been minimized.

Copy link

commented Oct 9, 2019

Could you for ease of use switch the user and the group to be called ohmyform?

@ahmedkrmn

This comment has been minimized.

Copy link
Author

commented Oct 9, 2019

Could you for ease of use switch the user and the group to be called ohmyform?

@wodka Setting the user and the group to the same name would cause the following error:

test

Dockerfile Outdated
@@ -1,6 +1,9 @@
FROM node:10-alpine
MAINTAINER OhMyForm <admin@ohmyform.com>

# Create a group and a user with name "ohmyformUser".
RUN addgroup -g 9999 ohmyformGroup && adduser -u 99999 -D -g ohmyformGroup ohmyformUser

This comment has been minimized.

Copy link
@philipmckenna

philipmckenna Oct 9, 2019

Suggested change
RUN addgroup -g 9999 ohmyformGroup && adduser -u 99999 -D -g ohmyformGroup ohmyformUser
RUN addgroup -g 9999 ohmyform && adduser -u 99999 -D -G ohmyform ohmyform

perhaps this may fulfil @wodka suggestion, unable to test atm due to work

Dockerfile Outdated

# Change to non-root privilege
USER ohmyformUser
RUN addgroup -g 9999 ohmyformGroup && adduser -u 99999 -D -g ohmyformGroup ohmyformUser

This comment has been minimized.

Copy link
@Leopere

Leopere Oct 9, 2019

The nines are multiplying.

This comment has been minimized.

Copy link
@Leopere

Leopere Oct 9, 2019

You should be able to do that screenshot I'm not sure whats wrong here but we should be able to do this.

This comment has been minimized.

Copy link
@Leopere

Leopere Oct 9, 2019

So far running this in the Dockerfile results in a failure but this works somewhat when I copy the commands one at a time into docker run -ti --rm alpine /bin/ash running in the foreground for testing.

FROM  node:10-alpine
MAINTAINER OhMyForm <admin@ohmyform.com>

## https://stackoverflow.com/questions/49955097/how-do-i-add-a-user-when-im-using-alpine-as-a-base-image
export USER=app
export UID=1000
export GID=1000



# Install some needed packages
RUN apk add --no-cache git python \
	&& rm -rf /tmp/* \
	&& npm install --quiet -g grunt bower pm2 \
	&& npm cache clean --force \
	&& mkdir -p /opt/app/public/lib

# to expose the public folder to other containers
# VOLUME /opt/app

WORKDIR /opt/app

## https://stackoverflow.com/questions/49955097/how-do-i-add-a-user-when-im-using-alpine-as-a-base-image
RUN addgroup --gid "$GID" "$USER" \
    && adduser \
    --disabled-password \
    --gecos "" \
    --home "$(pwd)" \
    --ingroup "$USER" \
    --no-create-home \
    --uid "$UID" \
    "$USER"

This comment has been minimized.

Copy link
@philipmckenna

philipmckenna Oct 9, 2019

looks like alot of effort than just doing USER 1000:1000

This comment has been minimized.

Copy link
@ahmedkrmn

ahmedkrmn Oct 9, 2019

Author

I retested this. It seems like the issue had to do with using 999 for the group id as it is used elsewhere in the container.
Setting both the user id and the group id to 9999 worked fine.

This comment has been minimized.

Copy link
@Leopere

Leopere Oct 9, 2019

@philipmckenna optimizations would be welcomed on this suggestion the only thinking I had behind this suggestion was that it was a bit more flexible but yes its more work.

Dockerfile Outdated
@@ -54,5 +51,8 @@ RUN npm install --only=production \
&& bower install --allow-root -f \
&& grunt build

# Change to non-root privilege
USER ohmyformUser

This comment has been minimized.

Copy link
@Leopere

Leopere Oct 9, 2019

Good place to put this just before final execution. I think that this should be fine however file permissions may argue having had the install functions run prior to switching to the unprivileged user, however this I think I would find acceptable as the user shouldn't really need to modify the running container's code however it might cause read write execute issues so we'll have to see.

This comment has been minimized.

Copy link
@ahmedkrmn

ahmedkrmn Oct 9, 2019

Author

Putting USER ohmyformUser before RUN apk add ... causes this error:

ERROR: Unable to lock database: Permission denied
ERROR: Failed to open apk database: Permission denied

Putting USER ohmyformUser before RUN npm install ... causes this error:

npm ERR!  { [Error: EACCES: permission denied, access '/opt/app']
npm ERR!   stack: 'Error: EACCES: permission denied, access \'/opt/app\'',
npm ERR!   errno: -13,
npm ERR!   code: 'EACCES',
npm ERR!   syscall: 'access',
npm ERR!   path: '/opt/app' }

So I believe putting it just before CMD ["node", "server.js"] is the most suitable place.

This comment has been minimized.

Copy link
@Leopere

Leopere Oct 9, 2019

Only execution needs to be done as the unprivileged user in the Dockerfile. The only thing that might be tricky is ensuring anything in the application files copied over from the git repository are set to the correct ownership.

@wodka

This comment has been minimized.

Copy link

commented Oct 9, 2019

Could you for ease of use switch the user and the group to be called ohmyform?

@wodka Setting the user and the group to the same name would cause the following error:

test

the reason for the error is beause adduser per default also tries to create the user, just add the flag -N to only add it to the group

@Leopere

This comment has been minimized.

Copy link

commented Oct 9, 2019

Could you for ease of use switch the user and the group to be called ohmyform?

@wodka Setting the user and the group to the same name would cause the following error:
test

the reason for the error is beause adduser per default also tries to create the user, just add the flag -N to only add it to the group

If it by default tries to create the user is there any point in the adduser when the addgroup does it or can we not set as many features for the user with addgroup

@ahmedkrmn

This comment has been minimized.

Copy link
Author

commented Oct 9, 2019

Could you for ease of use switch the user and the group to be called ohmyform?

@wodka Setting the user and the group to the same name would cause the following error:
test

the reason for the error is beause adduser per default also tries to create the user, just add the flag -N to only add it to the group

If it by default tries to create the user is there any point in the adduser when the addgroup does it or can we not set as many features for the user with addgroup

As stated in the official docker docs, when the user doesn’t have a primary group then the image (or the next instructions) will be run with the root group.

@Leopere

This comment has been minimized.

Copy link

commented Oct 9, 2019

As stated in the official docker docs, when the user doesn’t have a primary group then the image (or the next instructions) will be run with the root group.

Right but the key words here are in @wodka's reply the reason for the error is beause adduser per default also tries to create the user, just add the flag -N to only add it to the group

Copy link

left a comment

This looks much better, does is work still as is?

@ahmedkrmn

This comment has been minimized.

Copy link
Author

commented Oct 9, 2019

This looks much better, does is work still as is?

Yes 🚀

@Leopere

This comment has been minimized.

Copy link

commented Oct 9, 2019

Now I'm getting an entirely different issue but that might just be a @wodka issue

> bcrypt@3.0.6 install /opt/app/node_modules/bcrypt
> node-pre-gyp install --fallback-to-build

node-pre-gyp WARN Using request for node-pre-gyp https download
node-pre-gyp WARN Tried to download(404): https://github.com/kelektiv/node.bcrypt.js/releases/download/v3.0.6/bcrypt_lib-v3.0.6-node-v64-linux-x64-musl.tar.gz
node-pre-gyp WARN Pre-built binaries not found for bcrypt@3.0.6 and node@10.16.3 (node-v64 ABI, musl) (falling back to source compile with node-gyp)
gyp ERR! build error
gyp ERR! stack Error: not found: make
gyp ERR! stack     at getNotFoundError (/usr/local/lib/node_modules/npm/node_modules/which/which.js:13:12)
gyp ERR! stack     at F (/usr/local/lib/node_modules/npm/node_modules/which/which.js:68:19)
gyp ERR! stack     at E (/usr/local/lib/node_modules/npm/node_modules/which/which.js:80:29)
gyp ERR! stack     at /usr/local/lib/node_modules/npm/node_modules/which/which.js:89:16
gyp ERR! stack     at /usr/local/lib/node_modules/npm/node_modules/isexe/index.js:42:5
gyp ERR! stack     at /usr/local/lib/node_modules/npm/node_modules/isexe/mode.js:8:5
gyp ERR! stack     at FSReqWrap.oncomplete (fs.js:153:21)
gyp ERR! System Linux 4.9.184-linuxkit
gyp ERR! command "/usr/local/bin/node" "/usr/local/lib/node_modules/npm/node_modules/node-gyp/bin/node-gyp.js" "build" "--fallback-to-build" "--module=/opt/app/node_modules/bcrypt/lib/binding/bcrypt_lib.node" "--module_name=bcrypt_lib" "--module_path=/opt/app/node_modules/bcrypt/lib/binding" "--napi_version=4" "--node_abi_napi=napi" "--napi_build_version=0" "--node_napi_label=node-v64"
gyp ERR! cwd /opt/app/node_modules/bcrypt
gyp ERR! node -v v10.16.3
gyp ERR! node-gyp -v v3.8.0
gyp ERR! not ok
node-pre-gyp ERR! build error
node-pre-gyp ERR! stack Error: Failed to execute '/usr/local/bin/node /usr/local/lib/node_modules/npm/node_modules/node-gyp/bin/node-gyp.js build --fallback-to-build --module=/opt/app/node_modules/bcrypt/lib/binding/bcrypt_lib.node --module_name=bcrypt_lib --module_path=/opt/app/node_modules/bcrypt/lib/binding --napi_version=4 --node_abi_napi=napi --napi_build_version=0 --node_napi_label=node-v64' (1)
node-pre-gyp ERR! stack     at ChildProcess.<anonymous> (/opt/app/node_modules/node-pre-gyp/lib/util/compile.js:83:29)
node-pre-gyp ERR! stack     at ChildProcess.emit (events.js:198:13)
node-pre-gyp ERR! stack     at maybeClose (internal/child_process.js:982:16)
node-pre-gyp ERR! stack     at Process.ChildProcess._handle.onexit (internal/child_process.js:259:5)
node-pre-gyp ERR! System Linux 4.9.184-linuxkit
node-pre-gyp ERR! command "/usr/local/bin/node" "/opt/app/node_modules/.bin/node-pre-gyp" "install" "--fallback-to-build"
node-pre-gyp ERR! cwd /opt/app/node_modules/bcrypt
node-pre-gyp ERR! node -v v10.16.3
node-pre-gyp ERR! node-pre-gyp -v v0.12.0
node-pre-gyp ERR! not ok
Failed to execute '/usr/local/bin/node /usr/local/lib/node_modules/npm/node_modules/node-gyp/bin/node-gyp.js build --fallback-to-build --module=/opt/app/node_modules/bcrypt/lib/binding/bcrypt_lib.node --module_name=bcrypt_lib --module_path=/opt/app/node_modules/bcrypt/lib/binding --napi_version=4 --node_abi_napi=napi --napi_build_version=0 --node_napi_label=node-v64' (1)
npm WARN The package grunt-contrib-uglify is included as both a dev and production dependency.
npm WARN optional SKIPPING OPTIONAL DEPENDENCY: fsevents@1.2.9 (node_modules/grunt-html2js/node_modules/fsevents):
npm WARN notsup SKIPPING OPTIONAL DEPENDENCY: Unsupported platform for fsevents@1.2.9: wanted {"os":"darwin","arch":"any"} (current: {"os":"linux","arch":"x64"})

npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! bcrypt@3.0.6 install: `node-pre-gyp install --fallback-to-build`
npm ERR! Exit status 1
npm ERR!
npm ERR! Failed at the bcrypt@3.0.6 install script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     /root/.npm/_logs/2019-10-09T14_36_35_858Z-debug.log
ERROR: Service 'ohmyform' failed to build: The command '/bin/sh -c npm install --only=production     && bower install --allow-root -f     && grunt build' returned a non-zero code: 1```
@Leopere
Leopere approved these changes Oct 9, 2019
@Leopere

This comment has been minimized.

Copy link

commented Oct 9, 2019

At this point we just need to resolve the build error. I'll see if I can't poke @wodka via the Discord server.

Dockerfile Show resolved Hide resolved
@Leopere Leopere added this to In progress in Needs via automation Oct 9, 2019
@Leopere Leopere added this to In progress in OhMyForm 1.0 via automation Oct 9, 2019
@Leopere Leopere changed the title [WIP] Modify docker file to use non-root user Modify docker file to use non-root user Oct 10, 2019
@Leopere Leopere merged commit 44f5d68 into ohmyform:master Oct 10, 2019
Needs automation moved this from In progress to Done Oct 10, 2019
OhMyForm 1.0 automation moved this from In progress to Done Oct 10, 2019
@Leopere

This comment has been minimized.

Copy link

commented Oct 10, 2019

Thank you everyone for your participation in this PR! Happy Hacktoberfest!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Needs
  
Done
OhMyForm 1.0
  
Done
4 participants
You can’t perform that action at this time.