-
-
Notifications
You must be signed in to change notification settings - Fork 367
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
Makes docker images support non-root execution #1031
Changes from all commits
c88a9a2
c87999c
ef58f0e
6aedf45
fcb7154
4830a70
a15ecaa
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 |
---|---|---|
@@ -0,0 +1,40 @@ | ||
#!/usr/bin/env bash | ||
set -Eeuo pipefail | ||
|
||
# Runs the command provided as arguments, but attempts to configure permissions first. | ||
|
||
important_read_dirs=("/grist" "/persist") | ||
write_dir="/persist" | ||
current_user_id=$(id -u) | ||
|
||
# We want to avoid running Grist as root if possible. | ||
# Try to setup permissions and de-elevate to a normal user. | ||
if [[ $current_user_id == 0 ]]; then | ||
target_user=${GRIST_DOCKER_USER:-grist} | ||
target_group=${GRIST_DOCKER_GROUP:-grist} | ||
|
||
# Make sure the target user owns everything that Grist needs write access to. | ||
find $write_dir ! -user "$target_user" -exec chown "$target_user" "{}" + | ||
|
||
# Restart as the target user, replacing the current process (replacement is needed for security). | ||
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. What security does 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. Exactly - exec throws away the parent process running as root. Digging out the issue, it looks like it was a vuln mentioned in Gosu (an alternative to setpriv), concerning TTYs persisting across privilege boundaries: The |
||
# Alternative tools to setpriv are: chroot, gosu. | ||
# Need to use `exec` to close the parent shell, to avoid vulnerabilities: https://github.com/tianon/gosu/issues/37 | ||
exec setpriv --reuid "$target_user" --regid "$target_group" --init-groups /usr/bin/env bash "$0" "$@" | ||
fi | ||
|
||
# Validate that this user has access to the top level of each important directory. | ||
# There might be a benefit to testing individual files, but this is simpler as the dir may start empty. | ||
for dir in "${important_read_dirs[@]}"; do | ||
if ! { test -r "$dir" ;} ; then | ||
echo "Invalid permissions, cannot read '$dir'. Aborting." >&2 | ||
exit 1 | ||
fi | ||
done | ||
for dir in "${important_write_dirs[@]}"; do | ||
if ! { test -r "$dir" && test -w "$dir" ;} ; then | ||
echo "Invalid permissions, cannot write '$dir'. Aborting." >&2 | ||
exit 1 | ||
fi | ||
done | ||
|
||
exec /usr/bin/tini -s -- "$@" |
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.
Is there a reason why this isn't just a separate
ENV
stanza?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.
Figured we may as well not create an extra docker layer for it :)