From f3989038a2ab2e7b6f0a335605fc8fcf423070eb Mon Sep 17 00:00:00 2001 From: Erik Sundell Date: Sun, 29 Mar 2020 04:10:27 +0200 Subject: [PATCH 01/10] start.sh: add info about the user executing hooks --- base-notebook/start.sh | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/base-notebook/start.sh b/base-notebook/start.sh index 498b8727c2..5ff629b3dd 100755 --- a/base-notebook/start.sh +++ b/base-notebook/start.sh @@ -11,25 +11,27 @@ else cmd=( "$@" ) fi +# The run-hooks function looks for .sh scripts and executable files +# - .sh scripts will be sourced +# - executable files (+x) will be executed run-hooks () { - # Source scripts or run executable files in a directory if [[ ! -d "${1}" ]] ; then return fi - echo "${0}: running hooks in ${1}" + echo "${0}: running hooks in ${1} as uid / gid: $(id -u) / $(id -g)" for f in "${1}/"*; do case "${f}" in *.sh) - echo "${0}: running ${f}" + echo "${0}: running script ${f}" # shellcheck disable=SC1090 source "${f}" ;; *) if [[ -x "${f}" ]] ; then - echo "${0}: running ${f}" + echo "${0}: running executable ${f}" "${f}" else - echo "${0}: ignoring ${f}" + echo "${0}: ignoring non-executable ${f}" fi ;; esac From 52cc15ccf6dcd85465a52d709927ed053ac6642c Mon Sep 17 00:00:00 2001 From: Erik Sundell Date: Sun, 29 Mar 2020 04:33:36 +0200 Subject: [PATCH 02/10] start.sh: preserve environment properly when starting as root --- base-notebook/start.sh | 44 +++++++++++++++++++++++++++++++++--------- 1 file changed, 35 insertions(+), 9 deletions(-) diff --git a/base-notebook/start.sh b/base-notebook/start.sh index 5ff629b3dd..ec02d876a3 100755 --- a/base-notebook/start.sh +++ b/base-notebook/start.sh @@ -49,6 +49,9 @@ if [ "$(id -u)" == 0 ] ; then echo "Set username to: ${NB_USER}" usermod -d "/home/${NB_USER}" -l "${NB_USER}" jovyan fi + # Update any environment variables we set during build of the + # Dockerfile that contained the home directory path. + export XDG_CACHE_HOME=/home/$NB_USER/.cache # handle home and working directory if the username changed if [[ "${NB_USER}" != "jovyan" ]]; then @@ -92,20 +95,43 @@ if [ "$(id -u)" == 0 ] ; then useradd --home "/home/${NB_USER}" -u "${NB_UID}" -g "${NB_GID}" -G 100 -l "${NB_USER}" fi - # Enable sudo if requested + # Conditionally enable passwordless sudo usage for the jovyan user if [[ "${GRANT_SUDO}" == "1" || "${GRANT_SUDO}" == 'yes' ]]; then - echo "Granting ${NB_USER} sudo access and appending ${CONDA_DIR}/bin to sudo PATH" - echo "${NB_USER} ALL=(ALL) NOPASSWD:ALL" > /etc/sudoers.d/notebook + echo "Granting ${NB_USER} passwordless sudo rights!" + echo "${NB_USER} ALL=(ALL) NOPASSWD:ALL" > /etc/sudoers.d/added-by-start-script fi - # Add ${CONDA_DIR}/bin to sudo secure_path - sed -r "s#Defaults\s+secure_path\s*=\s*\"?([^\"]+)\"?#Defaults secure_path=\"\1:${CONDA_DIR}/bin\"#" /etc/sudoers | grep secure_path > /etc/sudoers.d/path + # Ensure that the initial environment that this container is started with + # is preserved when we run transition from running as root to running as + # NB_USER. + # + # - We use the sudo command to execute the command as NB_USER. But, what + # happens to the environment will be determined by configuration in + # /etc/sudoers and /etc/sudoers.d/* as well as flags we pass to the sudo + # command. The behavior can be inspected with `sudo -V` run as root. + # + # ref: `man sudo` - https://linux.die.net/man/8/sudo ref: `man sudoers` - + # https://www.sudo.ws/man/1.8.15/sudoers.man.html + # + # - We use the `--preserve-env` flag to pass through most environment, but + # understand that exceptions are caused by the sudoers configuration: + # `env_delete`, `env_check`, and `secure_path`. + # + # - We reduce the `env_delete` list of default variables to be deleted by + # default which would ignore the `--preserve-env` flag and `env_keep` + # configuration. + # + # - We manage the PATH variable specifically as `secure_path` is set by + # default in /etc/sudoers and would override the PATH variable. So we + # disable that default. + echo 'Defaults env_delete -= "PATH LD_* PYTHON*"' >> /etc/sudoers.d/added-by-start-script + echo 'Defaults !secure_path' >> /etc/sudoers.d/added-by-start-script - # Exec the command as NB_USER with the PATH and the rest of - # the environment preserved + # NOTE: This hook is run as the root user! run-hooks /usr/local/bin/before-notebook.d - echo "Executing the command:" "${cmd[@]}" - exec sudo -E -H -u "${NB_USER}" PATH="${PATH}" XDG_CACHE_HOME="/home/${NB_USER}/.cache" PYTHONPATH="${PYTHONPATH:-}" "${cmd[@]}" + + echo "Running as ${NB_USER}:" "${cmd[@]}" + exec sudo --preserve-env --set-home --user "${NB_USER}" "${cmd[@]}" else if [[ "${NB_UID}" == "$(id -u jovyan 2>/dev/null)" && "${NB_GID}" == "$(id -g jovyan 2>/dev/null)" ]]; then # User is not attempting to override user/group via environment From 22c3516abd50b793d0e290bafe43bf70f1c2defb Mon Sep 17 00:00:00 2001 From: Erik Sundell Date: Sun, 29 Mar 2020 04:54:18 +0200 Subject: [PATCH 03/10] start.sh: inline documentation --- base-notebook/start.sh | 92 ++++++++++++++++++++++++++---------------- 1 file changed, 58 insertions(+), 34 deletions(-) diff --git a/base-notebook/start.sh b/base-notebook/start.sh index ec02d876a3..cbf474e7f8 100755 --- a/base-notebook/start.sh +++ b/base-notebook/start.sh @@ -39,60 +39,82 @@ run-hooks () { echo "${0}: done running hooks in ${1}" } + + +# NOTE: This hook will run as the user the container was started with! run-hooks /usr/local/bin/start-notebook.d -# Handle special flags if we're root + + +# If we have started the container as the root user, then we have permission to +# manipulate user identities and storage file permissions before we start the +# server as a non-root user. +# +# Environment variables of relevance: +# - NB_UID: the user we want to run as, uniquely identified by an id +# - NB_USER: the username and associated home folder we want for our user +# - NB_GID: a group we want our user to belong to, uniquely identified by an id +# - NB_GROUP: the groupname we want for the group if [ "$(id -u)" == 0 ] ; then - # Only attempt to change the jovyan username if it exists + # Optionally ensure the user we want to run as (NB_UID) get filesystem + # ownership of it's home folder and additional folders. This can be relevant + # for attached NFS storage. + # + # Environment variables: + # - CHOWN_HOME: a boolean ("1" or "yes") to chown the user's home folder + # - CHOWN_EXTRA: a comma separated list of paths to chown + # - CHOWN_HOME_OPTS / CHOWN_EXTRA_OPTS: arguments to the chown command + if [[ "${CHOWN_HOME}" == "1" || "${CHOWN_HOME}" == "yes" ]]; then + echo "Updating ownership of /home/${NB_USER} to ${NB_UID}:${NB_GID} with options '${CHOWN_HOME_OPTS}'" + # shellcheck disable=SC2086 + chown $CHOWN_HOME_OPTS "${NB_UID}:${NB_GID}" "/home/${NB_USER}" + fi + if [ -n "${CHOWN_EXTRA}" ]; then + for extra_dir in $(echo "${CHOWN_EXTRA}" | tr ',' ' '); do + echo "Updating ownership of ${extra_dir} to ${NB_UID}:${NB_GID} with options '${CHOWN_EXTRA_OPTS}'" + # shellcheck disable=SC2086 + chown ${CHOWN_EXTRA_OPTS} "${NB_UID}:${NB_GID}" "${extra_dir}" + done + fi + + # Update the jovyan identity to get desired username and its associated home folder. if id jovyan &> /dev/null ; then - echo "Set username to: ${NB_USER}" - usermod -d "/home/${NB_USER}" -l "${NB_USER}" jovyan + echo "Updating the default jovyan user:" + echo "username: jovyan -> ${NB_USER}" + echo "home dir: /home/jovyan -> /home/${NB_USER}" + usermod --home "/home/${NB_USER}" --login "${NB_USER}" jovyan fi # Update any environment variables we set during build of the # Dockerfile that contained the home directory path. export XDG_CACHE_HOME=/home/$NB_USER/.cache - # handle home and working directory if the username changed + # For non-jovyan username's, populate their home directory with the jovyan's + # home directory as a fallback if they don't have one mounted already. if [[ "${NB_USER}" != "jovyan" ]]; then - # changing username, make sure homedir exists - # (it could be mounted, and we shouldn't create it if it already exists) if [[ ! -e "/home/${NB_USER}" ]]; then echo "Copying home dir to /home/${NB_USER}" mkdir "/home/${NB_USER}" cp -a /home/jovyan/. "/home/${NB_USER}/" || ln -s /home/jovyan "/home/${NB_USER}" fi - # if workdir is in /home/jovyan, cd to /home/${NB_USER} + # Ensure the current working directory is updated if [[ "${PWD}/" == "/home/jovyan/"* ]]; then newcwd="/home/${NB_USER}/${PWD:13}" - echo "Setting CWD to ${newcwd}" + echo "Changing working directory to ${newcwd}" cd "${newcwd}" fi fi - # Handle case where provisioned storage does not have the correct permissions by default - # Ex: default NFS/EFS (no auto-uid/gid) - if [[ "${CHOWN_HOME}" == "1" || "${CHOWN_HOME}" == 'yes' ]]; then - echo "Changing ownership of /home/${NB_USER} to ${NB_UID}:${NB_GID} with options '${CHOWN_HOME_OPTS}'" - # shellcheck disable=SC2086 - chown ${CHOWN_HOME_OPTS} "${NB_UID}:${NB_GID}" "/home/${NB_USER}" - fi - if [ -n "${CHOWN_EXTRA}" ]; then - for extra_dir in $(echo "${CHOWN_EXTRA}" | tr ',' ' '); do - echo "Changing ownership of ${extra_dir} to ${NB_UID}:${NB_GID} with options '${CHOWN_EXTRA_OPTS}'" - # shellcheck disable=SC2086 - chown ${CHOWN_EXTRA_OPTS} "${NB_UID}:${NB_GID}" "${extra_dir}" - done - fi - - # Change UID:GID of NB_USER to NB_UID:NB_GID if it does not match + # Ensure NB_USER gets the NB_UID user id and is a member of the NB_GID group if [ "${NB_UID}" != "$(id -u "${NB_USER}")" ] || [ "${NB_GID}" != "$(id -g "${NB_USER}")" ]; then - echo "Set user ${NB_USER} UID:GID to: ${NB_UID}:${NB_GID}" + echo "Update ${NB_USER}'s UID:GID to ${NB_UID}:${NB_GID}" + # Ensure the group's existence if [ "${NB_GID}" != "$(id -g "${NB_USER}")" ]; then - groupadd -f -g "${NB_GID}" -o "${NB_GROUP:-${NB_USER}}" + groupadd --force --gid "$NB_GID" --non-unique "${NB_GROUP:-${NB_USER}}" fi + # Recreate the user as we want it userdel "${NB_USER}" - useradd --home "/home/${NB_USER}" -u "${NB_UID}" -g "${NB_GID}" -G 100 -l "${NB_USER}" + useradd --home "/home/${NB_USER}" --uid "${NB_UID}" --gid "${NB_GID}" --groups 100 --no-log-init "${NB_USER}" fi # Conditionally enable passwordless sudo usage for the jovyan user @@ -132,6 +154,10 @@ if [ "$(id -u)" == 0 ] ; then echo "Running as ${NB_USER}:" "${cmd[@]}" exec sudo --preserve-env --set-home --user "${NB_USER}" "${cmd[@]}" + + + +# The container didn't start as the root user. else if [[ "${NB_UID}" == "$(id -u jovyan 2>/dev/null)" && "${NB_GID}" == "$(id -g jovyan 2>/dev/null)" ]]; then # User is not attempting to override user/group via environment @@ -147,13 +173,13 @@ else cat /tmp/passwd > /etc/passwd rm /tmp/passwd else - echo 'Container must be run with group "root" to update passwd file' + echo "WARNING: Container must be run with group 'root' to update passwd file" fi fi # Warn if the user isn't going to be able to write files to ${HOME}. if [[ ! -w /home/jovyan ]]; then - echo 'Container must be run with group "users" to update files' + echo "WARNING: Container must be run with group 'users' to update files" fi else # Warn if looks like user want to override uid/gid but hasn't @@ -166,13 +192,11 @@ else fi fi - # Warn if looks like user want to run in sudo mode but hasn't run - # the container as root. + # Warn about a probable misconfiguration of sudo if [[ "${GRANT_SUDO}" == "1" || "${GRANT_SUDO}" == 'yes' ]]; then - echo 'Container must be run as root to grant sudo permissions' + echo "WARNING: container must be started up as root to grant sudo permissions." fi - # Execute the command run-hooks /usr/local/bin/before-notebook.d echo "Executing the command:" "${cmd[@]}" exec "${cmd[@]}" From cd5e45b5735de7e316f7ea15c70ce8b103545fd2 Mon Sep 17 00:00:00 2001 From: Erik Sundell Date: Sun, 29 Mar 2020 04:54:48 +0200 Subject: [PATCH 04/10] start.sh: refactor for readability --- base-notebook/start.sh | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/base-notebook/start.sh b/base-notebook/start.sh index cbf474e7f8..052c9fb20d 100755 --- a/base-notebook/start.sh +++ b/base-notebook/start.sh @@ -93,9 +93,14 @@ if [ "$(id -u)" == 0 ] ; then # home directory as a fallback if they don't have one mounted already. if [[ "${NB_USER}" != "jovyan" ]]; then if [[ ! -e "/home/${NB_USER}" ]]; then - echo "Copying home dir to /home/${NB_USER}" + echo "Attempting to copy /home/jovyan to /home/${NB_USER}..." mkdir "/home/${NB_USER}" - cp -a /home/jovyan/. "/home/${NB_USER}/" || ln -s /home/jovyan "/home/${NB_USER}" + if cp -a /home/jovyan/. "/home/${NB_USER}/"; then + echo "Done" + else + echo "Failed. Attempting to symlink /home/jovyan to /home/${NB_USER}..." + ln -s /home/jovyan "/home/${NB_USER}" && echo "Done" + fi fi # Ensure the current working directory is updated if [[ "${PWD}/" == "/home/jovyan/"* ]]; then From 0365e73889b743ce9d5c1999643db464893ac135 Mon Sep 17 00:00:00 2001 From: Erik Sundell Date: Sun, 29 Mar 2020 07:35:55 +0200 Subject: [PATCH 05/10] start.sh: update logging output --- base-notebook/start.sh | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/base-notebook/start.sh b/base-notebook/start.sh index 052c9fb20d..cb73fbeb4c 100755 --- a/base-notebook/start.sh +++ b/base-notebook/start.sh @@ -3,6 +3,7 @@ # Distributed under the terms of the Modified BSD License. set -e +echo "Running: start.sh" "$@" # Exec the specified command or fall back on bash if [ $# -eq 0 ]; then @@ -80,13 +81,13 @@ if [ "$(id -u)" == 0 ] ; then # Update the jovyan identity to get desired username and its associated home folder. if id jovyan &> /dev/null ; then - echo "Updating the default jovyan user:" - echo "username: jovyan -> ${NB_USER}" - echo "home dir: /home/jovyan -> /home/${NB_USER}" - usermod --home "/home/${NB_USER}" --login "${NB_USER}" jovyan + if ! usermod --home "/home/${NB_USER}" --login "${NB_USER}" jovyan 2>&1 | grep "no changes" > /dev/null; then + echo "Updated the jovyan user:" + echo "username: jovyan -> ${NB_USER}" + echo "home dir location: /home/jovyan -> /home/${NB_USER}" + fi fi - # Update any environment variables we set during build of the - # Dockerfile that contained the home directory path. + # Update env vars set during image build with the current NB_USER value export XDG_CACHE_HOME=/home/$NB_USER/.cache # For non-jovyan username's, populate their home directory with the jovyan's @@ -137,8 +138,8 @@ if [ "$(id -u)" == 0 ] ; then # /etc/sudoers and /etc/sudoers.d/* as well as flags we pass to the sudo # command. The behavior can be inspected with `sudo -V` run as root. # - # ref: `man sudo` - https://linux.die.net/man/8/sudo ref: `man sudoers` - - # https://www.sudo.ws/man/1.8.15/sudoers.man.html + # ref: `man sudo` https://linux.die.net/man/8/sudo + # ref: `man sudoers` https://www.sudo.ws/man/1.8.15/sudoers.man.html # # - We use the `--preserve-env` flag to pass through most environment, but # understand that exceptions are caused by the sudoers configuration: From 018e5fc962a76a9bdbc6327d2c7178c7e6e470dd Mon Sep 17 00:00:00 2001 From: Erik Sundell Date: Mon, 30 Mar 2020 02:16:47 +0200 Subject: [PATCH 06/10] start.sh: tests, docs, and refinements --- base-notebook/start.sh | 227 ++++++++++--------- base-notebook/test/test_container_options.py | 76 +++++-- 2 files changed, 176 insertions(+), 127 deletions(-) diff --git a/base-notebook/start.sh b/base-notebook/start.sh index cb73fbeb4c..2bdfa135ec 100755 --- a/base-notebook/start.sh +++ b/base-notebook/start.sh @@ -12,9 +12,8 @@ else cmd=( "$@" ) fi -# The run-hooks function looks for .sh scripts and executable files -# - .sh scripts will be sourced -# - executable files (+x) will be executed +# The run-hooks function looks for .sh scripts to source and executable files to +# run within a passed directory. run-hooks () { if [[ ! -d "${1}" ]] ; then return @@ -41,99 +40,101 @@ run-hooks () { } - # NOTE: This hook will run as the user the container was started with! run-hooks /usr/local/bin/start-notebook.d - - -# If we have started the container as the root user, then we have permission to -# manipulate user identities and storage file permissions before we start the -# server as a non-root user. +# If the container started as the root user, then we have permission to refit +# the jovyan user, and ensure file permissions, grant sudo rights, and such +# things before we run the command passed to start.sh as the desired user +# (NB_USER). # -# Environment variables of relevance: -# - NB_UID: the user we want to run as, uniquely identified by an id -# - NB_USER: the username and associated home folder we want for our user -# - NB_GID: a group we want our user to belong to, uniquely identified by an id -# - NB_GROUP: the groupname we want for the group if [ "$(id -u)" == 0 ] ; then - - # Optionally ensure the user we want to run as (NB_UID) get filesystem - # ownership of it's home folder and additional folders. This can be relevant - # for attached NFS storage. - # # Environment variables: + # - NB_USER: the desired username and associated home folder + # - NB_UID: the desired user id + # - NB_GID: a group id we want our user to belong to + # - NB_GROUP: the groupname we want for the group + # - GRANT_SUDO: a boolean ("1" or "yes") to grant the user sudo rights # - CHOWN_HOME: a boolean ("1" or "yes") to chown the user's home folder # - CHOWN_EXTRA: a comma separated list of paths to chown - # - CHOWN_HOME_OPTS / CHOWN_EXTRA_OPTS: arguments to the chown command - if [[ "${CHOWN_HOME}" == "1" || "${CHOWN_HOME}" == "yes" ]]; then - echo "Updating ownership of /home/${NB_USER} to ${NB_UID}:${NB_GID} with options '${CHOWN_HOME_OPTS}'" - # shellcheck disable=SC2086 - chown $CHOWN_HOME_OPTS "${NB_UID}:${NB_GID}" "/home/${NB_USER}" - fi - if [ -n "${CHOWN_EXTRA}" ]; then - for extra_dir in $(echo "${CHOWN_EXTRA}" | tr ',' ' '); do - echo "Updating ownership of ${extra_dir} to ${NB_UID}:${NB_GID} with options '${CHOWN_EXTRA_OPTS}'" - # shellcheck disable=SC2086 - chown ${CHOWN_EXTRA_OPTS} "${NB_UID}:${NB_GID}" "${extra_dir}" - done - fi + # - CHOWN_HOME_OPTS / CHOWN_EXTRA_OPTS: arguments to the chown commands - # Update the jovyan identity to get desired username and its associated home folder. + # Refit the jovyan user to the desired the user (NB_USER) if id jovyan &> /dev/null ; then if ! usermod --home "/home/${NB_USER}" --login "${NB_USER}" jovyan 2>&1 | grep "no changes" > /dev/null; then echo "Updated the jovyan user:" - echo "username: jovyan -> ${NB_USER}" - echo "home dir location: /home/jovyan -> /home/${NB_USER}" + echo "- username: jovyan -> ${NB_USER}" + echo "- home dir: /home/jovyan -> /home/${NB_USER}" fi + elif ! id -u "${NB_USER}" &> /dev/null; then + echo "ERROR: Neither the jovyan user or '$NB_USER' exists." + echo " This could be the result of stopping and starting, the" + echo " container with a different NB_USER environment variable." + exit 1 + fi + # Ensure the desired user (NB_USER) gets its desired user id (NB_UID) and is + # a member of the desired group (NB_GROUP, NB_GID) + if [ "${NB_UID}" != "$(id -u "${NB_USER}")" ] || [ "${NB_GID}" != "$(id -g "${NB_USER}")" ]; then + echo "Update ${NB_USER}'s UID:GID to ${NB_UID}:${NB_GID}" + # Ensure the desired group's existence + if [ "${NB_GID}" != "$(id -g "${NB_USER}")" ]; then + groupadd --force --gid "${NB_GID}" --non-unique "${NB_GROUP:-${NB_USER}}" + fi + # Recreate the desired user as we want it + userdel "${NB_USER}" + useradd --home "/home/${NB_USER}" --uid "${NB_UID}" --gid "${NB_GID}" --groups 100 --no-log-init "${NB_USER}" fi - # Update env vars set during image build with the current NB_USER value - export XDG_CACHE_HOME=/home/$NB_USER/.cache - # For non-jovyan username's, populate their home directory with the jovyan's - # home directory as a fallback if they don't have one mounted already. + # Move or symlink the jovyan home directory to the desired users home + # directory if it doesn't already exist, and update the current working + # directory to the new location if needed. if [[ "${NB_USER}" != "jovyan" ]]; then if [[ ! -e "/home/${NB_USER}" ]]; then echo "Attempting to copy /home/jovyan to /home/${NB_USER}..." mkdir "/home/${NB_USER}" if cp -a /home/jovyan/. "/home/${NB_USER}/"; then - echo "Done" + echo "Success!" else - echo "Failed. Attempting to symlink /home/jovyan to /home/${NB_USER}..." - ln -s /home/jovyan "/home/${NB_USER}" && echo "Done" + echo "Failed!" + echo "Attempting to symlink /home/jovyan to /home/${NB_USER}..." + if ln -s /home/jovyan "/home/${NB_USER}"; then + echo "Success!" + else + echo "Failed!" + fi fi fi - # Ensure the current working directory is updated + # Ensure the current working directory is updated to the new path if [[ "${PWD}/" == "/home/jovyan/"* ]]; then - newcwd="/home/${NB_USER}/${PWD:13}" - echo "Changing working directory to ${newcwd}" - cd "${newcwd}" + new_wd="/home/${NB_USER}/${PWD:13}" + echo "Changing working directory to ${new_wd}" + cd "${new_wd}" fi fi - # Ensure NB_USER gets the NB_UID user id and is a member of the NB_GID group - if [ "${NB_UID}" != "$(id -u "${NB_USER}")" ] || [ "${NB_GID}" != "$(id -g "${NB_USER}")" ]; then - echo "Update ${NB_USER}'s UID:GID to ${NB_UID}:${NB_GID}" - # Ensure the group's existence - if [ "${NB_GID}" != "$(id -g "${NB_USER}")" ]; then - groupadd --force --gid "$NB_GID" --non-unique "${NB_GROUP:-${NB_USER}}" - fi - # Recreate the user as we want it - userdel "${NB_USER}" - useradd --home "/home/${NB_USER}" --uid "${NB_UID}" --gid "${NB_GID}" --groups 100 --no-log-init "${NB_USER}" + # Optionally ensure the desired user get filesystem ownership of it's home + # folder and/or additional folders + if [[ "${CHOWN_HOME}" == "1" || "${CHOWN_HOME}" == "yes" ]]; then + echo "Ensuring /home/${NB_USER} is owned by ${NB_UID}:${NB_GID} ${CHOWN_HOME_OPTS:+chown options: ${CHOWN_HOME_OPTS}}" + # shellcheck disable=SC2086 + chown ${CHOWN_HOME_OPTS} "${NB_UID}:${NB_GID}" "/home/${NB_USER}" fi - - # Conditionally enable passwordless sudo usage for the jovyan user - if [[ "${GRANT_SUDO}" == "1" || "${GRANT_SUDO}" == 'yes' ]]; then - echo "Granting ${NB_USER} passwordless sudo rights!" - echo "${NB_USER} ALL=(ALL) NOPASSWD:ALL" > /etc/sudoers.d/added-by-start-script + if [ -n "${CHOWN_EXTRA}" ]; then + for extra_dir in $(echo "${CHOWN_EXTRA}" | tr ',' ' '); do + echo "Ensuring ${extra_dir} is owned by ${NB_UID}:${NB_GID} ${CHOWN_HOME_OPTS:+(chown options: ${CHOWN_HOME_OPTS})}" + # shellcheck disable=SC2086 + chown ${CHOWN_EXTRA_OPTS} "${NB_UID}:${NB_GID}" "${extra_dir}" + done fi - # Ensure that the initial environment that this container is started with - # is preserved when we run transition from running as root to running as - # NB_USER. + # Update potentially outdated environment variables since image build + export XDG_CACHE_HOME=/home/$NB_USER/.cache + + # Notes on how we ensure that the environment that this container is started + # with is preserved (except vars listen in JUPYTER_ENV_VARS_TO_UNSET) when + # we transition from running as root to running as NB_USER. # - # - We use the sudo command to execute the command as NB_USER. But, what + # - We use `sudo` to execute the command as NB_USER. What then # happens to the environment will be determined by configuration in # /etc/sudoers and /etc/sudoers.d/* as well as flags we pass to the sudo # command. The behavior can be inspected with `sudo -V` run as root. @@ -141,19 +142,26 @@ if [ "$(id -u)" == 0 ] ; then # ref: `man sudo` https://linux.die.net/man/8/sudo # ref: `man sudoers` https://www.sudo.ws/man/1.8.15/sudoers.man.html # - # - We use the `--preserve-env` flag to pass through most environment, but - # understand that exceptions are caused by the sudoers configuration: - # `env_delete`, `env_check`, and `secure_path`. + # - We use the `--preserve-env` flag to pass through most environment + # variables, but understand that exceptions are caused by the sudoers + # configuration: `env_delete`, `env_check`, and `secure_path`. + # + # - We use the `--set-home` flag to set the HOME variable appropriatly. # - # - We reduce the `env_delete` list of default variables to be deleted by - # default which would ignore the `--preserve-env` flag and `env_keep` + # - We reduce the `env_delete` list of default variables to be deleted. It + # has higher priority than the `--preserve-env` flag and `env_keep` # configuration. # - # - We manage the PATH variable specifically as `secure_path` is set by - # default in /etc/sudoers and would override the PATH variable. So we - # disable that default. + # - We disable the `secure_path` which is set by default in /etc/sudoers as + # it would override the PATH variable. + echo 'Defaults !secure_path' > /etc/sudoers.d/added-by-start-script echo 'Defaults env_delete -= "PATH LD_* PYTHON*"' >> /etc/sudoers.d/added-by-start-script - echo 'Defaults !secure_path' >> /etc/sudoers.d/added-by-start-script + + # Optionally grant passwordless sudo rights for the desired user + if [[ "$GRANT_SUDO" == "1" || "$GRANT_SUDO" == 'yes' ]]; then + echo "Granting $NB_USER passwordless sudo rights!" + echo "$NB_USER ALL=(ALL) NOPASSWD:ALL" >> /etc/sudoers.d/added-by-start-script + fi # NOTE: This hook is run as the root user! run-hooks /usr/local/bin/before-notebook.d @@ -161,48 +169,49 @@ if [ "$(id -u)" == 0 ] ; then echo "Running as ${NB_USER}:" "${cmd[@]}" exec sudo --preserve-env --set-home --user "${NB_USER}" "${cmd[@]}" - - -# The container didn't start as the root user. +# The container didn't start as the root user, so we will have to act as the +# user we started as. else - if [[ "${NB_UID}" == "$(id -u jovyan 2>/dev/null)" && "${NB_GID}" == "$(id -g jovyan 2>/dev/null)" ]]; then - # User is not attempting to override user/group via environment - # variables, but they could still have overridden the uid/gid that - # container runs as. Check that the user has an entry in the passwd - # file and if not add an entry. - STATUS=0 && whoami &> /dev/null || STATUS=$? && true - if [[ "${STATUS}" != "0" ]]; then - if [[ -w /etc/passwd ]]; then - echo "Adding passwd file entry for $(id -u)" - sed -e "s/^jovyan:/nayvoj:/" /etc/passwd > /tmp/passwd - echo "jovyan:x:$(id -u):$(id -g):,,,:/home/jovyan:/bin/bash" >> /tmp/passwd - cat /tmp/passwd > /etc/passwd - rm /tmp/passwd - else - echo "WARNING: Container must be run with group 'root' to update passwd file" - fi - fi + # Warn about misconfiguration of: desired username, user id, or group id + if [[ -n "${NB_USER}" && "${NB_USER}" != "$(id -un)" ]]; then + echo "WARNING: container must be started as root to change the desired user's name with NB_USER!" + fi + if [[ -n "${NB_UID}" && "${NB_UID}" != "$(id -u)" ]]; then + echo "WARNING: container must be started as root to change the desired user's id with NB_UID!" + fi + if [[ -n "${NB_GID}" && "${NB_GID}" != "$(id -g)" ]]; then + echo "WARNING: container must be started as root to change the desired user's group id with NB_GID!" + fi - # Warn if the user isn't going to be able to write files to ${HOME}. - if [[ ! -w /home/jovyan ]]; then - echo "WARNING: Container must be run with group 'users' to update files" - fi - else - # Warn if looks like user want to override uid/gid but hasn't - # run the container as root. - if [[ -n "${NB_UID}" && "${NB_UID}" != "$(id -u)" ]]; then - echo "Container must be run as root to set NB_UID to ${NB_UID}" - fi - if [[ -n "${NB_GID}" && "${NB_GID}" != "$(id -g)" ]]; then - echo "Container must be run as root to set NB_GID to ${NB_GID}" + # Warn about misconfiguration of: granting sudo rights + if [[ "${GRANT_SUDO}" == "1" || "${GRANT_SUDO}" == 'yes' ]]; then + echo "WARNING: container must be started as root to grant sudo permissions!" + fi + + # Attempt to ensure the user uid we currently run as has a named entry in + # the /etc/passwd file, as it avoids software crashing on hard assumptions + # on such entry. Writing to the /etc/passwd was allowed for the root group + # from the Dockerfile during build. + # + # ref: https://github.com/jupyter/docker-stacks/issues/552 + if ! whoami &> /dev/null; then + if [[ -w /etc/passwd ]]; then + sed --in-place "s/^jovyan:/nayvoj:/" /etc/passwd + echo "Renamed old jovyan user to nayvoj (1000:100)" + + echo "jovyan:x:$(id -u):$(id -g):,,,:/home/jovyan:/bin/bash" >> /etc/passwd + echo "Added new jovyan user ($(id -u):$(id -g))" + else + echo "WARNING: container must be started with group 'root' (0) to add a user entry in /etc/passwd!" fi fi - # Warn about a probable misconfiguration of sudo - if [[ "${GRANT_SUDO}" == "1" || "${GRANT_SUDO}" == 'yes' ]]; then - echo "WARNING: container must be started up as root to grant sudo permissions." + # Warn if the user isn't able to write files to ${HOME} + if [[ ! -w /home/jovyan ]]; then + echo "WARNING: container must be started with group 'users' (100) to be allowed to write to /home/jovyan!" fi + # NOTE: This hook is run as the user we started the container as! run-hooks /usr/local/bin/before-notebook.d echo "Executing the command:" "${cmd[@]}" exec "${cmd[@]}" diff --git a/base-notebook/test/test_container_options.py b/base-notebook/test/test_container_options.py index 2212de6ea9..31e529fa81 100644 --- a/base-notebook/test/test_container_options.py +++ b/base-notebook/test/test_container_options.py @@ -44,7 +44,8 @@ def test_uid_change(container): command=["start.sh", "bash", "-c", "id && touch /opt/conda/test-file"], ) # usermod is slow so give it some time - c.wait(timeout=120) + rv = c.wait(timeout=120) + assert rv == 0 or rv["StatusCode"] == 0 assert "uid=1010(jovyan)" in c.logs(stdout=True).decode("utf-8") @@ -56,7 +57,8 @@ def test_gid_change(container): environment=["NB_GID=110"], command=["start.sh", "id"], ) - c.wait(timeout=10) + rv = c.wait(timeout=10) + assert rv == 0 or rv["StatusCode"] == 0 logs = c.logs(stdout=True).decode("utf-8") assert "gid=110(jovyan)" in logs assert "groups=110(jovyan),100(users)" in logs @@ -77,7 +79,9 @@ def test_nb_user_change(container): time.sleep(10) LOGGER.info(f"Checking if the user is changed to {nb_user} by the start script ...") output = running_container.logs(stdout=True).decode("utf-8") - assert f"Set username to: {nb_user}" in output, f"User is not changed to {nb_user}" + assert ( + f"username: jovyan -> {nb_user}" in output + ), f"User is not changed to {nb_user}" LOGGER.info(f"Checking {nb_user} id ...") command = "id" @@ -108,21 +112,30 @@ def test_nb_user_change(container): def test_chown_extra(container): - """Container should change the UID/GID of CHOWN_EXTRA.""" + """Container should change the UID/GID of a comma separated + CHOWN_EXTRA list of folders.""" c = container.run( tty=True, user="root", environment=[ "NB_UID=1010", "NB_GID=101", - "CHOWN_EXTRA=/opt/conda", + "CHOWN_EXTRA=/home/jovyan,/opt/conda/bin", "CHOWN_EXTRA_OPTS=-R", ], - command=["start.sh", "bash", "-c", "stat -c '%n:%u:%g' /opt/conda/LICENSE.txt"], + command=[ + "start.sh", + "bash", + "-c", + "stat -c '%n:%u:%g' /home/jovyan/.bashrc /opt/conda/bin/jupyter", + ], ) # chown is slow so give it some time - c.wait(timeout=120) - assert "/opt/conda/LICENSE.txt:1010:101" in c.logs(stdout=True).decode("utf-8") + rv = c.wait(timeout=120) + assert rv == 0 or rv["StatusCode"] == 0 + logs = c.logs(stdout=True).decode("utf-8") + assert "/home/jovyan/.bashrc:1010:101" in logs + assert "/opt/conda/bin/jupyter:1010:101" in logs def test_chown_home(container): @@ -131,18 +144,19 @@ def test_chown_home(container): c = container.run( tty=True, user="root", - environment=["CHOWN_HOME=yes", "CHOWN_HOME_OPTS=-R"], - command=[ - "start.sh", - "bash", - "-c", - "chown root:root /home/jovyan && ls -alsh /home", + environment=[ + "CHOWN_HOME=yes", + "CHOWN_HOME_OPTS=-R", + "NB_USER=kitten", + "NB_UID=1010", + "NB_GID=101", ], + command=["start.sh", "bash", "-c", "stat -c '%n:%u:%g' /home/kitten/.bashrc"], ) - c.wait(timeout=120) - assert "Changing ownership of /home/jovyan to 1000:100 with options '-R'" in c.logs( - stdout=True - ).decode("utf-8") + rv = c.wait(timeout=120) + assert rv == 0 or rv["StatusCode"] == 0 + logs = c.logs(stdout=True).decode("utf-8") + assert "/home/kitten/.bashrc:1010:101" in logs def test_sudo(container): @@ -224,3 +238,29 @@ def test_container_not_delete_bind_mount(container, tmp_path): assert rv == 0 or rv["StatusCode"] == 0 assert p.read_text() == "some-content" assert len(list(tmp_path.iterdir())) == 1 + + +@pytest.mark.skip(reason="not yet implemented; TODO: cherry-pick b44b7ab") +def test_jupyter_env_vars_to_unset_as_root(container): + """Environment variables names listed in JUPYTER_ENV_VARS_TO_UNSET + should be unset in the final environment.""" + c = container.run( + tty=True, + user="root", + environment=[ + "JUPYTER_ENV_VARS_TO_UNSET=SECRET_ANIMAL,UNUSED_ENV,SECRET_FRUIT", + "FRUIT=bananas", + "SECRET_FRUIT=mango", + "SECRET_ANIMAL=cats", + ], + command=[ + "start.sh", + "bash", + "-c", + "echo I like $FRUIT and ${SECRET_FRUIT:-stuff}, and love ${SECRET_LOVE:-to keep secrets}!", + ], + ) + rv = c.wait(timeout=10) + assert rv == 0 or rv["StatusCode"] == 0 + logs = c.logs(stdout=True).decode("utf-8") + assert "I like bananas and stuff, and love to keep secrets!" in logs From 4ef8a7816dbb3f8b6f3c69c8c2a640db001a45aa Mon Sep 17 00:00:00 2001 From: Ben Mares Date: Sun, 7 Nov 2021 20:01:34 +0100 Subject: [PATCH 07/10] Undo changes which require further examination --- base-notebook/start.sh | 33 ++++++--------------------------- 1 file changed, 6 insertions(+), 27 deletions(-) diff --git a/base-notebook/start.sh b/base-notebook/start.sh index 2bdfa135ec..40d13f51ef 100755 --- a/base-notebook/start.sh +++ b/base-notebook/start.sh @@ -130,32 +130,8 @@ if [ "$(id -u)" == 0 ] ; then # Update potentially outdated environment variables since image build export XDG_CACHE_HOME=/home/$NB_USER/.cache - # Notes on how we ensure that the environment that this container is started - # with is preserved (except vars listen in JUPYTER_ENV_VARS_TO_UNSET) when - # we transition from running as root to running as NB_USER. - # - # - We use `sudo` to execute the command as NB_USER. What then - # happens to the environment will be determined by configuration in - # /etc/sudoers and /etc/sudoers.d/* as well as flags we pass to the sudo - # command. The behavior can be inspected with `sudo -V` run as root. - # - # ref: `man sudo` https://linux.die.net/man/8/sudo - # ref: `man sudoers` https://www.sudo.ws/man/1.8.15/sudoers.man.html - # - # - We use the `--preserve-env` flag to pass through most environment - # variables, but understand that exceptions are caused by the sudoers - # configuration: `env_delete`, `env_check`, and `secure_path`. - # - # - We use the `--set-home` flag to set the HOME variable appropriatly. - # - # - We reduce the `env_delete` list of default variables to be deleted. It - # has higher priority than the `--preserve-env` flag and `env_keep` - # configuration. - # - # - We disable the `secure_path` which is set by default in /etc/sudoers as - # it would override the PATH variable. - echo 'Defaults !secure_path' > /etc/sudoers.d/added-by-start-script - echo 'Defaults env_delete -= "PATH LD_* PYTHON*"' >> /etc/sudoers.d/added-by-start-script + # Add ${CONDA_DIR}/bin to sudo secure_path + sed -r "s#Defaults\s+secure_path\s*=\s*\"?([^\"]+)\"?#Defaults secure_path=\"\1:${CONDA_DIR}/bin\"#" /etc/sudoers | grep secure_path > /etc/sudoers.d/path # Optionally grant passwordless sudo rights for the desired user if [[ "$GRANT_SUDO" == "1" || "$GRANT_SUDO" == 'yes' ]]; then @@ -167,7 +143,10 @@ if [ "$(id -u)" == 0 ] ; then run-hooks /usr/local/bin/before-notebook.d echo "Running as ${NB_USER}:" "${cmd[@]}" - exec sudo --preserve-env --set-home --user "${NB_USER}" "${cmd[@]}" + exec sudo --preserve-env --set-home --user "${NB_USER}" \ + PATH="${PATH}" XDG_CACHE_HOME="/home/${NB_USER}/.cache" \ + PYTHONPATH="${PYTHONPATH:-}" \ + "${cmd[@]}" # The container didn't start as the root user, so we will have to act as the # user we started as. From 1ce72b55e273f1c566382125a6a0677534fcd502 Mon Sep 17 00:00:00 2001 From: Erik Sundell Date: Tue, 9 Nov 2021 13:44:06 +0100 Subject: [PATCH 08/10] Apply suggestions from code review Co-authored-by: Ayaz Salikhov --- base-notebook/start.sh | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/base-notebook/start.sh b/base-notebook/start.sh index 40d13f51ef..e11604dcb0 100755 --- a/base-notebook/start.sh +++ b/base-notebook/start.sh @@ -67,7 +67,7 @@ if [ "$(id -u)" == 0 ] ; then echo "- home dir: /home/jovyan -> /home/${NB_USER}" fi elif ! id -u "${NB_USER}" &> /dev/null; then - echo "ERROR: Neither the jovyan user or '$NB_USER' exists." + echo "ERROR: Neither the jovyan user or '${NB_USER}' exists." echo " This could be the result of stopping and starting, the" echo " container with a different NB_USER environment variable." exit 1 @@ -128,15 +128,15 @@ if [ "$(id -u)" == 0 ] ; then fi # Update potentially outdated environment variables since image build - export XDG_CACHE_HOME=/home/$NB_USER/.cache + export XDG_CACHE_HOME="/home/${NB_USER}/.cache" # Add ${CONDA_DIR}/bin to sudo secure_path sed -r "s#Defaults\s+secure_path\s*=\s*\"?([^\"]+)\"?#Defaults secure_path=\"\1:${CONDA_DIR}/bin\"#" /etc/sudoers | grep secure_path > /etc/sudoers.d/path # Optionally grant passwordless sudo rights for the desired user - if [[ "$GRANT_SUDO" == "1" || "$GRANT_SUDO" == 'yes' ]]; then - echo "Granting $NB_USER passwordless sudo rights!" - echo "$NB_USER ALL=(ALL) NOPASSWD:ALL" >> /etc/sudoers.d/added-by-start-script + if [[ "$GRANT_SUDO" == "1" || "$GRANT_SUDO" == "yes" ]]; then + echo "Granting ${NB_USER} passwordless sudo rights!" + echo "${NB_USER} ALL=(ALL) NOPASSWD:ALL" >> /etc/sudoers.d/added-by-start-script fi # NOTE: This hook is run as the root user! @@ -146,7 +146,7 @@ if [ "$(id -u)" == 0 ] ; then exec sudo --preserve-env --set-home --user "${NB_USER}" \ PATH="${PATH}" XDG_CACHE_HOME="/home/${NB_USER}/.cache" \ PYTHONPATH="${PYTHONPATH:-}" \ - "${cmd[@]}" + "${cmd[@]}" # The container didn't start as the root user, so we will have to act as the # user we started as. @@ -163,7 +163,7 @@ else fi # Warn about misconfiguration of: granting sudo rights - if [[ "${GRANT_SUDO}" == "1" || "${GRANT_SUDO}" == 'yes' ]]; then + if [[ "${GRANT_SUDO}" == "1" || "${GRANT_SUDO}" == "yes" ]]; then echo "WARNING: container must be started as root to grant sudo permissions!" fi From 87ad5ffdf60e24b4d5cd8e86a0283eb6dec3c611 Mon Sep 17 00:00:00 2001 From: Ben Mares Date: Tue, 9 Nov 2021 20:52:59 +0100 Subject: [PATCH 09/10] start.sh: improve some messages --- base-notebook/start.sh | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/base-notebook/start.sh b/base-notebook/start.sh index e11604dcb0..2749e8cd6f 100755 --- a/base-notebook/start.sh +++ b/base-notebook/start.sh @@ -174,20 +174,21 @@ else # # ref: https://github.com/jupyter/docker-stacks/issues/552 if ! whoami &> /dev/null; then + echo "There is no entry in /etc/passwd for our UID. Attempting to fix..." if [[ -w /etc/passwd ]]; then + echo "Renaming old jovyan user to nayvoj ($(id -u jovyan):$(id -g jovyan))" sed --in-place "s/^jovyan:/nayvoj:/" /etc/passwd - echo "Renamed old jovyan user to nayvoj (1000:100)" echo "jovyan:x:$(id -u):$(id -g):,,,:/home/jovyan:/bin/bash" >> /etc/passwd - echo "Added new jovyan user ($(id -u):$(id -g))" + echo "Added new jovyan user ($(id -u):$(id -g)). Fixed UID!" else - echo "WARNING: container must be started with group 'root' (0) to add a user entry in /etc/passwd!" + echo "WARNING: unable to fix missing /etc/passwd entry because we don't have write permission." fi fi # Warn if the user isn't able to write files to ${HOME} if [[ ! -w /home/jovyan ]]; then - echo "WARNING: container must be started with group 'users' (100) to be allowed to write to /home/jovyan!" + echo "WARNING: no write access to /home/jovyan. Try starting the container with group 'users' (100)." fi # NOTE: This hook is run as the user we started the container as! From 4c47de93e758e9df4844d7f61d77c19503b786f0 Mon Sep 17 00:00:00 2001 From: Ben Mares Date: Wed, 10 Nov 2021 00:56:34 +0100 Subject: [PATCH 10/10] start.sh: add parens for CHOWN_HOME_OPTS Co-authored-by: Erik Sundell --- base-notebook/start.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/base-notebook/start.sh b/base-notebook/start.sh index 2749e8cd6f..bf0c03cadb 100755 --- a/base-notebook/start.sh +++ b/base-notebook/start.sh @@ -115,7 +115,7 @@ if [ "$(id -u)" == 0 ] ; then # Optionally ensure the desired user get filesystem ownership of it's home # folder and/or additional folders if [[ "${CHOWN_HOME}" == "1" || "${CHOWN_HOME}" == "yes" ]]; then - echo "Ensuring /home/${NB_USER} is owned by ${NB_UID}:${NB_GID} ${CHOWN_HOME_OPTS:+chown options: ${CHOWN_HOME_OPTS}}" + echo "Ensuring /home/${NB_USER} is owned by ${NB_UID}:${NB_GID} ${CHOWN_HOME_OPTS:+(chown options: ${CHOWN_HOME_OPTS})}" # shellcheck disable=SC2086 chown ${CHOWN_HOME_OPTS} "${NB_UID}:${NB_GID}" "/home/${NB_USER}" fi