-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Using non root user in controller #3579
Conversation
Which security risks are you talking about specifically? Note that we setup docker to use usernamespaces. The root user in the container does not equal to a root user on the system itself. |
Got it. I will check the namespace config if that serves my purpose (say prevent socket access and thereby prevent it from being able to start other docker containers). |
I think doing this will break DockerClientWithFileAccess because it relies on the user who is running the invoker's ability to read the log files written by the docker daemon, which is of course running as root. |
I checked the namespaces but on the host the process still runs as root:
When I run the changed image it already fails at deployment while copying jmxremote to @mcdan Do you mean the container logs? Cause they are owned by the non-root user. If the invoker reads the daemon log at |
@fmaschler okay, sounds good. |
Is there any update on this? |
@fmaschler |
I'm not an expert in namespaces but I see what you mean. Though those container processes run as root on the host. Even if they may not have the same privilege inside the container this should be changed if there is no reason about it. |
@fmaschler |
This is good when you talk about INSIDE containers but this is about the OUTSIDE: The containers process on the host runs as root.
The question is if controller, kafka, invoker and the runtime containers all need to run as root? |
@Himavanth @fmaschler using this discussion, we will open an issue to document security-related best practices for an openhisk deployment. In this case, creating namespace so that root on the host is not root inside the container. See #3746. |
@rabbah @fmaschler My 2 cents.. I have found that both the approaches have their limitations. A User namespace has its own limitations documented here. https://docs.docker.com/engine/security/userns-remap/#user-namespace-known-limitations The best practice as per docker seems to " configure your container’s applications to run as unprivileged users" |
Codecov Report
@@ Coverage Diff @@
## master #3579 +/- ##
==========================================
- Coverage 86.05% 81.12% -4.93%
==========================================
Files 147 147
Lines 7162 7162
Branches 457 457
==========================================
- Hits 6163 5810 -353
- Misses 999 1352 +353
Continue to review full report at Codecov.
|
core/controller/Dockerfile
Outdated
# Copy app jars | ||
ADD build/distributions/controller.tar / | ||
|
||
COPY init.sh / | ||
RUN chmod +x init.sh | ||
RUN chmod 777 /root |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this be more restrictive? Which part requires the permissions to be open up in conntroller
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The jmx files are moved to this dir at init stage and then their permissions are changed to 600 by copyJMXFiles.sh. This is the only permission combination that i found working. There are no other files under root folder. If required we could create a different folder just to store these files.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
May be change its owner to NOT_ROOT_USER
or make it 640 i.e. avoid execute bit
From discussion I understand that proper setting up of namespace would avoid change user for process. However as @Himavanth mentioned that its a good practice to follow per docker docs
Unless there is any cons/downside in doing this change we should make this change. @markusthoemmes @rabbah @ddragosd Thoughts? |
Ping! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, @chetanmeh if you think this doesn't break anything, feel free to merge.
@chetanmeh OK with me. |
There is a PG fail that needs to be debugged here. Following is the stacktrace. Thx @vvraskin for this. |
Tracking this at #4012 |
Have done some basic testing. Would like some feedback.
Using chown instead of giving full permissions
Permissions to create coverage folder
The root folder has permission issues in IBM PG build. So using /home/owuser instead of /root to store jmxremote files. owuser is the new user we create to avoid using root user. Not switching the user in invoker because it is a privileged container.
Triggering build
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for addressing the comments as per our slack discussion!
With the change apache#3579, controller logs file was not being created due to lack of permissions when OW is deployed on docker-machine. This is because ansible is not able to create a folder with full permissions within a mounted folder in docker-machine. However creating a file with full permissions through ansible does work.
* Fix controller logs in docker-machine With the change #3579, controller logs file was not being created due to lack of permissions when OW is deployed on docker-machine. This is because ansible is not able to create a folder with full permissions within a mounted folder in docker-machine. However creating a file with full permissions through ansible does work. * Adding condition to check for docker-machine This change is needed only for docker-machine
* Using non root user in controller Have done some basic testing. Would like some feedback. * Fixing jmxremote file permissions * Triggering build * Using chown instead of giving full permissions Using chown instead of giving full permissions * Permissions to create coverage folder Permissions to create coverage folder * Using user's home folder instead of root The root folder has permission issues in IBM PG build. So using /home/owuser instead of /root to store jmxremote files. owuser is the new user we create to avoid using root user. Not switching the user in invoker because it is a privileged container. * Triggering build Triggering build
* Fix controller logs in docker-machine With the change apache#3579, controller logs file was not being created due to lack of permissions when OW is deployed on docker-machine. This is because ansible is not able to create a folder with full permissions within a mounted folder in docker-machine. However creating a file with full permissions through ansible does work. * Adding condition to check for docker-machine This change is needed only for docker-machine
Using a different user than root would be helpful in mitigating security risks that could arise because of root privileges.
Have done some basic testing.
Would like some feedback.
Related issue and scope
My changes affect the following components
Types of changes
Checklist: