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 <luca.dimaio1@gmail.com>
Co-authored-by: Luca Di Maio <luca.dimaio1@gmail.com>

.markdownlint.yaml

@@ -7,3 +7,4 @@ MD013:
   headers: false
 MD033: false
 MD041: false
+MD045: false

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

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.

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:

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

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"
 

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

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

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