Skip to content

Non root docker #11323

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.

Sign up for GitHub

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

Merged
mattklein123 merged 3 commits into from
Jun 4, 2020
Merged

Non root docker #11323

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
384c953
Update Dockerfile recipes to install su-exec
phlax May 26, 2020
57e272b
Update docker-entrypoint to use su-exec
phlax May 26, 2020
0d020df
Add release note for #11323
phlax Jun 2, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 22 additions & 1 deletion ci/Dockerfile-envoy
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,22 @@
FROM ubuntu:18.04
ARG BUILD_FROM=ubuntu:18.04


# Build stage
FROM $BUILD_FROM as build

RUN apt-get update \
&& apt-get upgrade -y \
&& apt-get install --no-install-recommends -y ca-certificates curl gcc libc-dev \
Comment thread
lizan marked this conversation as resolved.
Outdated
&& echo "d6c40440609a23483f12eb6295b5191e94baf08298a856bab6e15b10c3b82891 /tmp/su-exec.c" > /tmp/checksum \
&& curl -o /tmp/su-exec.c https://raw.githubusercontent.com/ncopa/su-exec/212b75144bbc06722fbd7661f651390dc47a43d1/su-exec.c \
&& sha256sum -c /tmp/checksum \
&& gcc -Wall /tmp/su-exec.c -o/usr/local/bin/su-exec \
&& chown root:root /usr/local/bin/su-exec \
&& chmod 0755 /usr/local/bin/su-exec


# Final stage
FROM $BUILD_FROM

RUN apt-get update \
&& apt-get upgrade -y \
Expand All @@ -8,6 +26,9 @@ RUN apt-get update \
&& rm -rf /tmp/* /var/tmp/* \
&& rm -rf /var/lib/apt/lists/*

COPY --from=build /usr/local/bin/su-exec /usr/local/bin/su-exec
RUN adduser --group --system envoy

RUN mkdir -p /etc/envoy

ADD build_release_stripped/envoy /usr/local/bin/envoy
Expand Down
2 changes: 2 additions & 0 deletions ci/Dockerfile-envoy-alpine
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ RUN mkdir -p /etc/envoy

ADD build_release_stripped/envoy /usr/local/bin/envoy
ADD configs/google_com_proxy.v2.yaml /etc/envoy/envoy.yaml
RUN apk add --no-cache shadow su-exec \
&& addgroup -S envoy && adduser --no-create-home -S envoy -G envoy

EXPOSE 10000

Expand Down
2 changes: 2 additions & 0 deletions ci/Dockerfile-envoy-alpine-debug
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ RUN mkdir -p /etc/envoy

ADD build_release/envoy /usr/local/bin/envoy
ADD configs/google_com_proxy.v2.yaml /etc/envoy/envoy.yaml
RUN apk add --no-cache shadow su-exec \
&& addgroup -S envoy && adduser --no-create-home -S envoy -G envoy

EXPOSE 10000

Expand Down
12 changes: 11 additions & 1 deletion ci/docker-entrypoint.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,14 @@ if [ "$1" = 'envoy' ]; then
fi
fi

exec "$@"
if [ "$ENVOY_UID" != "0" ]; then

@mattklein123 mattklein123 Jun 4, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What is the default here? If not set somehow by default should this default to "0"? I think by default we will enter this statement but the string will be empty?

@phlax phlax Jun 4, 2020

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

thats correct. The default is to use the envoy user with the uid/gid that is set at build time

@mattklein123 mattklein123 Jun 4, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Where are these env variables set?

@phlax phlax Jun 4, 2020

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

they are runtime env vars for the container - so they can be set eg on the docker command line or in a compose file etc

@mattklein123 mattklein123 Jun 4, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It's not clear that this now has to be set. Can we please change this so that the default stays wat it was and we document how to set the right variables? By default I don't think this makes sense.

@phlax phlax Jun 4, 2020

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

in the default case nothing should need to be set and all should work as it does now - save for the fact that process inside the container doesnt run as root.

The cases where you would want to set these vars are:

  • opening well-known ports inside the container and running as root - containers kind of negate the need to ever do this
  • setting a specific uid/gid to get the correct permissions on in or out sockets

my own opinion is that it is better to not run as root by default. Im happy to make whatever changes others feel necessary though

@mattklein123 mattklein123 Jun 4, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If both of these env vars are empty strings which would be the defualt, do the commands even work?

@phlax phlax Jun 4, 2020

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

yep for sure.

the default case is that ENVOY_UID is not set - ie also not set to 0

in this case it goes into this code block, but as ENVOY_UID is not set (same with gid) it goes straight to

     su-exec envoy "${@}"

@mattklein123 mattklein123 Jun 4, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ah OK, gotcha. Alright thanks for the explanation. LGTM.

if [ -n "$ENVOY_UID" ]; then
usermod -u "$ENVOY_UID" envoy
fi
if [ -n "$ENVOY_GID" ]; then
groupmod -g "$ENVOY_GID" envoy
fi
su-exec envoy "${@}"
else
exec "${@}"
fi
1 change: 1 addition & 0 deletions docs/root/version_history/current.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ Minor Behavior Changes
*Changes that may cause incompatibilities for some users, but should not for most*

* access loggers: applied existing buffer limits to access logs, as well as :ref:`stats <config_access_log_stats>` for logged / dropped logs. This can be reverted temporarily by setting runtime feature `envoy.reloadable_features.disallow_unbounded_access_logs` to false.
* build: run as non-root inside Docker containers. Existing behaviour can be restored by setting the environment variable `ENVOY_UID` to `0`. `ENVOY_UID` and `ENVOY_GID` can be used to set the envoy user's `uid` and `gid` respectively.
* hot restart: added the option :option:`--use-dynamic-base-id` to select an unused base ID at startup and the option :option:`--base-id-path` to write the base id to a file (for reuse with later hot restarts).
* http: fixed several bugs with applying correct connection close behavior across the http connection manager, health checker, and connection pool. This behavior may be temporarily reverted by setting runtime feature `envoy.reloadable_features.fix_connection_close` to false.
* http: fixed a bug where the upgrade header was not cleared on responses to non-upgrade requests.
Expand Down