From 3b1e312f884cad2185c3b57dcde470a5d7914c61 Mon Sep 17 00:00:00 2001 From: Pavin Joseph Date: Fri, 2 Feb 2024 02:48:05 +0530 Subject: [PATCH] Fix systemd/init container startup (#1069) Change user login to prevent issues with firefox segfaulting (IPC I/O) related errors. TODO: improve init wait depending on whether container has init or not. Improved waiting for systemd to start system Fix creation of systemd login sessions to prevent conflict across multiple active sessions. some containers are not allowing passthrough of certain environment variables without explicitly passing them using sudo's --preserve-env flag run podman exec command as root since some distros like suse or arch would ask for user password when using su to login even though the user has no password in /etc/shadow fix pam_systemd not being checked for su on certain distros like suse or arch. --------- Signed-off-by: Luca Di Maio Co-authored-by: Luca Di Maio --- .markdownlint.yaml | 1 + distrobox-create | 1 + distrobox-enter | 10 +++---- distrobox-ephemeral | 2 +- distrobox-export | 31 +++++++++++++-------- distrobox-init | 46 +++++++++++++++++++++++--------- distrobox-stop | 5 +++- distrobox-upgrade | 4 +-- docs/usage/distrobox-assemble.md | 42 ++++++++++++++--------------- 9 files changed, 89 insertions(+), 53 deletions(-) diff --git a/.markdownlint.yaml b/.markdownlint.yaml index 5bc9252e83..171fca0eeb 100644 --- a/.markdownlint.yaml +++ b/.markdownlint.yaml @@ -7,3 +7,4 @@ MD013: headers: false MD033: false MD041: false +MD045: false diff --git a/distrobox-create b/distrobox-create index 64e9911a15..4087e768d7 100755 --- a/distrobox-create +++ b/distrobox-create @@ -178,6 +178,7 @@ Options: --pre-init-hooks: additional commands to execute prior to container initialization --init/-I: use init system (like systemd) inside the container. this will make host's processes not visible from within the container. (assumes --unshare-process) + may require additional packages depending on the container image: https://github.com/89luca89/distrobox/blob/main/docs/useful_tips.md#using-init-system-inside-a-distrobox --nvidia: try to integrate host's nVidia drivers in the guest --unshare-devsys: do not share host devices and sysfs dirs from host --unshare-ipc: do not share ipc namespace with host diff --git a/distrobox-enter b/distrobox-enter index 6d8998a6bf..96a97a4dee 100755 --- a/distrobox-enter +++ b/distrobox-enter @@ -131,7 +131,7 @@ Usage: Options: --name/-n: name for the distrobox default: my-distrobox - --/-e: end arguments execute the rest as command to execute at login default: bash -l + --/-e: end arguments execute the rest as command to execute at login default: su ${USER} --no-tty/-T: do not instantiate a tty --no-workdir/-nw: always start the container from container's home directory --additional-flags/-a: additional flags to pass to the container manager command @@ -305,7 +305,7 @@ generate_command() { result_command="${result_command} --detach-keys=\"\"" result_command="${result_command} - --user=\"${USER}\"" + --user='root'" # For some usage, like use in service, or launched by non-terminal # eg. from desktop files, TTY can fail to instantiate, and fail to enter @@ -346,7 +346,7 @@ generate_command() { set +o xtrace # disable logging for this snippet, or it will be too talkative. for i in $(printenv | grep '=' | grep -Ev ' |"|`|\$' | - grep -Ev '^(HOST|HOSTNAME|HOME|PATH|PROFILEREAD|SHELL|XDG_.*_DIRS|^_)'); do + grep -Ev '^(HOST|HOSTNAME|HOME|PATH|PROFILEREAD|SHELL|XDG_SEAT|XDG_VTNR|XDG_.*_DIRS|^_)'); do # We filter the environment so that we do not have strange variables, # multiline or containing spaces. # We also NEED to ignore the HOME variable, as this is set at create time @@ -435,12 +435,12 @@ generate_command() { if [ -n "${container_command}" ]; then result_command="${result_command} - ${container_command}" + su ${USER} -c \"${container_command}\"" else # if no command was specified, let's execute a command that will find # and run the default shell for the user result_command="${result_command} - sh -c \"\\\$(getent passwd ${USER} | cut -f 7 -d :) -l"\" + su ${USER}" fi # Return generated command. diff --git a/distrobox-ephemeral b/distrobox-ephemeral index 47a1486dd0..67b9c812c2 100755 --- a/distrobox-ephemeral +++ b/distrobox-ephemeral @@ -74,7 +74,7 @@ Options: specify it through the DBX_SUDO_PROGRAM env variable, or 'distrobox_sudo_program' config variable) --verbose/-v: show more verbosity --help/-h: show this message - --/-e: end arguments execute the rest as command to execute at login default: bash -l + --/-e: end arguments execute the rest as command to execute at login default: su ${USER} --version/-V: show version See also: diff --git a/distrobox-export b/distrobox-export index 77c928c21d..c05f2673d8 100755 --- a/distrobox-export +++ b/distrobox-export @@ -39,7 +39,6 @@ host_home="${DISTROBOX_HOST_HOME:-"${HOME}"}" dest_path="${host_home}/.local/bin" is_sudo=0 rootful="" -start_shell="/bin/sh -l -c" verbose=0 version="1.6.0.1" @@ -180,8 +179,7 @@ fi # Ensure the foundamental variables are set and not empty, we will not proceed # if they are not all set. -if [ -z "${exported_app}" ] && - [ -z "${exported_bin}" ]; then +if [ -z "${exported_app}" ] && [ -z "${exported_bin}" ]; then printf >&2 "Error: Invalid arguments.\n" printf >&2 "Error: missing export target. Run\n" printf >&2 "\tdistrobox-export --help\n" @@ -216,17 +214,29 @@ if [ -z "${container_name}" ]; then container_name=$(uname -n | cut -d'.' -f1) fi -# -if [ "${is_sudo}" -ne 0 ]; then +# Command to execute +container_command_suffix="${exported_bin} ${extra_flags} \"\$@\"" + +# Edge case for systems without sudo +if command -v su-exec > /dev/null >&1; then + sudo_prefix="su-exec ${USER}" + container_command_suffix="sh -l -c \"${exported_bin} ${extra_flags} \$*\"" +fi + +# Check if exported application/binary should be run with sudo privileges +if [ "${is_sudo}" -ne 0 ]; then + sudo_prefix="sudo -S" + + # Edge case for systems without sudo if command -v su-exec > /dev/null >&1; then - start_shell="su-exec root su - root ${start_shell}" - elif command -v sudo > /dev/null 2>&1; then - start_shell="$(command -v sudo) -i ${start_shell}" + sudo_prefix="su-exec root" + container_command_suffix="sh -l -c \"${exported_bin} ${extra_flags} \$*\"" fi fi # Prefix to add to an existing command to work through the container -container_command_prefix="${DISTROBOX_ENTER_PATH:-"distrobox-enter"} ${rootful} -n ${container_name} -- ${start_shell} " +container_command_prefix="${DISTROBOX_ENTER_PATH:-"distrobox-enter"} ${rootful} -n ${container_name} -- ${sudo_prefix:-} " + if [ -z "${exported_app_label}" ]; then exported_app_label=" (on ${container_name})" elif [ "${exported_app_label}" = "none" ]; then @@ -247,8 +257,7 @@ generate_script() { # distrobox_binary # name: ${container_name} if [ -z "\${CONTAINER_ID}" ]; then - exec "${DISTROBOX_ENTER_PATH:-"distrobox-enter"}" ${rootful} -n ${container_name} -- \ - ${start_shell} '${exported_bin} ${extra_flags} \$@' -- "\$@" + exec "${DISTROBOX_ENTER_PATH:-"distrobox-enter"}" ${rootful} -n ${container_name} -- ${sudo_prefix:-} ${container_command_suffix} elif [ -n "\${CONTAINER_ID}" ] && [ "\${CONTAINER_ID}" != "${container_name}" ]; then exec distrobox-host-exec ${dest_path}/$(basename "${exported_bin}") ${extra_flags} "\$@" else diff --git a/distrobox-init b/distrobox-init index f2153b99d2..6c7e3f11db 100755 --- a/distrobox-init +++ b/distrobox-init @@ -1713,7 +1713,7 @@ fi ############################################################################### ############################################################################### -printf "distrobox: Setting up distrobox profile...\n" +printf "distrobox: Setting up distrobox profile...\n" # This ensures compatibility with prompts and tools between toolbx and distrobox touch /run/.toolboxenv @@ -1768,8 +1768,8 @@ fi ############################################################################### ############################################################################### -printf "distrobox: Setting up sudo...\n" -mkdir -p /etc/sudoers.d +printf "distrobox: Setting up sudo...\n" +mkdir -p /etc/sudoers.d # Ensure we're using the user's password for sudo, not root if [ -e /etc/sudoers ]; then sed -i "s|^Defaults targetpw.*||g" /etc/sudoers @@ -1778,12 +1778,29 @@ fi # Do not check fqdn when doing sudo, it will not work anyways # Also allow canonical groups to use sudo cat << EOF > /etc/sudoers.d/sudoers +Defaults !targetpw Defaults !fqdn %wheel ALL=(ALL:ALL) ALL %sudo ALL=(ALL:ALL) ALL %root ALL=(ALL:ALL) ALL EOF +# PAM config for "su" command +if [ ! -e /etc/pam.d/su ]; then + mkdir -p /etc/pam.d + cat << EOF > /etc/pam.d/su +auth sufficient pam_rootok.so +auth required pam_unix.so +account required pam_unix.so +session required pam_unix.so +-session optional pam_systemd.so +EOF +fi + +if ! grep -q "pam_systemd.so" /etc/pam.d/su; then + printf "%s" '-session optional pam_systemd.so' >> /etc/pam.d/su +fi + # If we're running this script as root in a login shell (sudoless), we don't # have to bother setting up sudo. # @@ -1923,7 +1940,7 @@ if [ "$(getent passwd "${container_user_name}" | cut -d: -f6)" != "${container_u fi # If we're rootless, delete password for root and user -if [ ! -e /etc/passwd.done ]; then +if [ ! -e /etc/passwd.done ]; then temporary_password="$(cat /proc/sys/kernel/random/uuid)" # We generate a random password to initialize the entry for the user. printf "%s:%s" "${container_user_name}" "${temporary_password}" | chpasswd -e @@ -1965,7 +1982,7 @@ if [ "${rootful}" -eq 1 ] && fi # Now we're done -touch /etc/passwd.done +touch /etc/passwd.done ############################################################################### ############################################################################### @@ -2078,16 +2095,16 @@ fi # Instantiate a new pty to mount over /dev/console # this way we will have init output right of the logs [ -e /dev/console ] || touch /dev/console -rm -f /var/console -mkfifo /var/console -script -c "cat /var/console" /dev/null & +rm -f /var/console +mkfifo /var/console +script -c "cat /var/console" /dev/null & # Ensure the pty is created -sleep 0.5 +sleep 0.5 # Mount the created pty over /dev/console in order to have systemd logs # right into container logs -if ! mount --bind /dev/pts/0 /dev/console; then +if ! mount --bind /dev/pts/0 /dev/console; then # Fallback to older behaviour or fake plaintext file in case it fails # this ensures rootful + initful boxes do not interfere with host's /dev/console rm -f /var/console @@ -2175,8 +2192,6 @@ fi printf "distrobox: Firing up init system...\n" if [ -e /usr/lib/systemd/systemd ] || [ -e /lib/systemd/systemd ]; then - printf "container_setup_done\n" - # Start user Systemd unit, this will attempt until Systemd is ready sh -c "while true; do \ sleep 1; \ @@ -2185,8 +2200,15 @@ if [ -e /usr/lib/systemd/systemd ] || [ -e /lib/systemd/systemd ]; then exit 0; \ done" & + sh -c " while true; do \ + systemctl is-system-running && break; \ + printf >&2 'waiting for systemd to come up...\n' && sleep 1; \ + done; \ + printf container_setup_done" & + [ -e /usr/lib/systemd/systemd ] && exec /usr/lib/systemd/systemd --system --log-target=console --unit=multi-user.target [ -e /lib/systemd/systemd ] && exec /lib/systemd/systemd --system --log-target=console --unit=multi-user.target + elif [ -e /sbin/init ]; then printf "container_setup_done\n" diff --git a/distrobox-stop b/distrobox-stop index fd17cc1d8e..891fb61db3 100755 --- a/distrobox-stop +++ b/distrobox-stop @@ -246,11 +246,14 @@ fi # check if we have containers to delete if [ -z "${container_name_list}" ]; then container_name_list="${container_name_default}" +else + # strip leading whitespace from container name + container_name_list="$(echo "${container_name_list}" | sed -E 's/^[[:space:]]+//')" fi if [ "${non_interactive}" -eq 0 ]; then # Prompt to stop the container. - printf "Do you really want to stop%s? [Y/n]: " "${container_name_list}" + printf "Do you really want to stop %s? [Y/n]: " "${container_name_list}" read -r response response="${response:-"Y"}" else diff --git a/distrobox-upgrade b/distrobox-upgrade index ab5e96fc2f..dafaa87e0f 100755 --- a/distrobox-upgrade +++ b/distrobox-upgrade @@ -240,7 +240,7 @@ fi for container in ${container_name}; do printf >&2 "\033[1;31m Upgrading %s...\n\033[0m" "${container}" # shellcheck disable=SC2086,SC2248 - "${distrobox_path}"/distrobox-enter --additional-flags "--user root:root" \ + "${distrobox_path}"/distrobox-enter \ ${distrobox_flags} ${container} -- \ - /usr/bin/entrypoint --upgrade + "command -v su-exec 2>/dev/null && su-exec root /usr/bin/entrypoint --upgrade || sudo -S -i /usr/bin/entrypoint --upgrade" done diff --git a/docs/usage/distrobox-assemble.md b/docs/usage/distrobox-assemble.md index 35150ca418..807ae5091a 100644 --- a/docs/usage/distrobox-assemble.md +++ b/docs/usage/distrobox-assemble.md @@ -97,27 +97,27 @@ This is a list of available options with the corresponding type: | Flag Name | Type | | - | - | -| additional_flags | string -| additional_packages | string -| home | string -| image | string -| init_hooks | string -| pre_init_hooks | string -| volume | string -| exported_apps | string -| exported_bins | string -| exported_bins_path | string -| entry | bool -| start_now | bool -| init | bool -| nvidia | bool -| pull | bool -| root | bool -| unshare_ipc | bool -| unshare_netns | bool -| unshare_process | bool -| unshare_devsys | bool -| unshare_all | bool +| additional_flags | string | +| additional_packages | string | +| home | string | +| image | string | +| init_hooks | string | +| pre_init_hooks | string | +| volume | string | +| exported_apps | string | +| exported_bins | string | +| exported_bins_path | string | +| entry | bool | +| start_now | bool | +| init | bool | +| nvidia | bool | +| pull | bool | +| root | bool | +| unshare_ipc | bool | +| unshare_netns | bool | +| unshare_process | bool | +| unshare_devsys | bool | +| unshare_all | bool | boolean options default to false if not specified. string options can be broken in multiple declarations additively in order to improve