-
Notifications
You must be signed in to change notification settings - Fork 87
[RAPTOR-12499] SemGrep detected root
user in dronin environments
#1423
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
base: master
Are you sure you want to change the base?
Changes from all commits
66a5735
124dd02
0a7ccb9
1f31a64
2f204fa
3ce4e1f
c90d44d
0ce4c85
98b854b
bd26dab
7228032
1c016e8
0215004
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,15 +5,23 @@ FROM ${BASE_ROOT_IMAGE} AS build | |
|
||
USER root | ||
|
||
# The JDK (not just JRE) is required because Py4J calls a Java method (o0.configure) that tries to execute 'javac'. | ||
RUN apk add --no-cache openjdk-11 | ||
|
||
ENV VIRTUAL_ENV=/opt/venv | ||
|
||
RUN python -m venv ${VIRTUAL_ENV} && \ | ||
${VIRTUAL_ENV}/bin/python -m ensurepip --default-pip --upgrade | ||
|
||
COPY requirements.txt requirements.txt | ||
RUN ${VIRTUAL_ENV}/bin/python -m pip install --no-cache-dir -r requirements.txt && \ | ||
find ${VIRTUAL_ENV} -type d -name '__pycache__' -exec rm -rf {} + | ||
|
||
# The JDK (not just JRE) is required because Py4J calls a Java method (o0.configure) that tries to execute 'javac'. | ||
|
||
# This is a private production chain-guard image that is stored in DataRobot's private registry. | ||
# Replace it with your own production chain-gaurd image if you build your own. | ||
FROM datarobotdev/mirror_chainguard_datarobot.com_python-fips:3.11 | ||
|
||
USER root | ||
|
||
# Copy the JRE from the development stage | ||
COPY --from=build /usr/lib/jvm/java-11-openjdk /usr/lib/jvm/java-11-openjdk | ||
|
||
|
@@ -32,25 +40,13 @@ COPY --from=build /bin/mkdir /bin/mkdir | |
# Required for custom-models to install dependencies | ||
COPY --from=build /usr/bin/pip /usr/bin/pip | ||
|
||
# Cleanup '__pycache__' directories. It solves an AsymmetricPrivateKey scanning error. | ||
COPY --from=build /usr/bin/rm /usr/bin/rm | ||
COPY --from=build /usr/bin/find /usr/bin/find | ||
Comment on lines
-36
to
-37
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No cleanup needed as we remove |
||
|
||
# Just for convenience | ||
COPY --from=build /bin/ls /bin/ls | ||
|
||
|
||
COPY requirements.txt requirements.txt | ||
|
||
ENV VIRTUAL_ENV=/opt/venv | ||
|
||
RUN sh -c "python -m venv ${VIRTUAL_ENV} && \ | ||
. ${VIRTUAL_ENV}/bin/activate && \ | ||
python -m ensurepip --default-pip && \ | ||
python -m pip install --upgrade pip && \ | ||
python -m pip install --no-cache-dir -r requirements.txt && \ | ||
find ${VIRTUAL_ENV} -type d -name '__pycache__' -exec rm -rf {} + && \ | ||
rm -rf /usr/bin/rm /usr/bin/find" | ||
Comment on lines
-46
to
-53
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. All that commands do not need a separate shell when building Docker image. I moved the dependencies installation step into |
||
# Copy preinstalled virtualenv | ||
COPY --from=build --chown=nonroot ${VIRTUAL_ENV} ${VIRTUAL_ENV} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this definitely needs testing with actual datarobot. we use 1000:1000 to run by default. |
||
|
||
ENV JAVA_HOME=/usr/lib/jvm/java-11-openjdk | ||
ENV PATH=${VIRTUAL_ENV}/bin:${JAVA_HOME}/bin:${PATH} | ||
|
@@ -68,4 +64,9 @@ ENV WITH_ERROR_SERVER=1 \ | |
COPY ./*.sh ${CODE_DIR}/ | ||
WORKDIR ${CODE_DIR} | ||
|
||
USER root | ||
RUN chmod -R 777 $HOME | ||
# Security scanners require non-root user to be explicitly specified for the Docker image | ||
USER nonroot | ||
|
||
ENTRYPOINT ["sh", "-c", "exec ${CODE_DIR}/start_server.sh \"$@\"", "--"] |
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.
This might not matter too much. In LRS we run the container with a specific set of user id / group id (defaults are 1000:1000), also we force chown on the /opt/code to these.
I'm not sure if setting USER to something else would do it.
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 you test building such image and testing on staging and/or locally?
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.
not sure I can test this on staging before merging the PR. Will try
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.
@klichukb I tested this image on Staging. It really lacks permissions if running for any non-root user (when tried to install model dependencies in the exec env).
The only solution to this is to grant permissions to all users for the
$HOME
directory which is "/opt". That directory keeps pip cache, virtualenvs the the code.We can't know which user will run this container but we definitely can't run the containers from
root
. Chainguard will never let us to use1000
as a user - there is no way to create custom users, it only gives usnonroot
user that we can use when running the container - that's requirement from STIG/FIPS compliance.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.
Also sorry, I didnt realize I commented on specifically julia.. Its an irrelevant env, so if any tests - definitely not the Julia one
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.
Right, for that reason I updated all the Dockerfiles in this PR.