[PR PATCH] xbps-triggers: make system-accounts behave in altroots

[PR PATCH] xbps-triggers: make system-accounts behave in altroots

[ahesford]

[-- Attachment #1: Type: text/plain, Size: 2603 bytes --]

There is a new pull request by ahesford against master on the void-packages repository

https://github.com/ahesford/void-packages xbps-triggers
https://github.com/void-linux/void-packages/pull/24812

xbps-triggers: make system-accounts behave in altroots
This attempts to fix some issues uncovered in #24754. The changes:
- Use `command -v` to check for `groupadd`, `usermod`, `useradd` and `passwd` rather than looking for executable files at a fixed path, to match what the shell will actually do when these programs are invoked.
- Uses the `-P|--prefix` options to `groupadd`, `usermod` and `useradd`, as well as the `-R|--root` option to `passwd`, to manipulate files in the target root (which should be `.`) rather than on the host system.
- Skips `getent` altogether, instead letting the `useradd` and `groupadd` programs detect and do the right thing when attempting to create existing users or groups (`getent` is still used in the `pre-remove` hook to try to extract the GECOS field for an account, but this is a non-critical operation and we can tolerate pulling the wrong comment in some edge cases in `xbps-remove -r`).

As a bonus, I cleaned up the hook output a bit, avoiding some strange text artifacts when user or group IDs are unspecified.

There are ~200 packages that specify system accounts or groups. I've tested this on a few that seem to offer a range of different user/group styles:
- sndio (system account with primary group `audio`, tests account creation with no user group)
- dbus (system account with specific UID, tests account creation with user group of matching GID)
- kismet (system group only, no user group)
- pulseaudio (system account with matching group, system group, additional groups for account)
- rtkit (system account with matching group)

These packages were installed on the local host by the superuser, and were also installed by an unprivileged user into an alternate root owned by that user; some packages were also installed to the unprivileged root by the superuser. In all cases, the hooks seem to correctly update `etc/passwd` and `etc/group` with respect to creating, disabling and reenabling accounts.

I have no means to test functionality on a system using a remote directory service but these changes at least prevent an altroot installation from leaking account creation or modification into the local host files while also ensuring that the account modifications are made in the altroot.

cc: @void-linux/pkg-committers @ericonr 


A patch file from https://github.com/void-linux/void-packages/pull/24812.patch is attached

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: github-pr-xbps-triggers-24812.patch --]
[-- Type: text/x-diff, Size: 7430 bytes --]

From ed59c44b91582da3775f979a61d2087c9ef37571 Mon Sep 17 00:00:00 2001
From: "Andrew J. Hesford" <ajh@sideband.org>
Date: Thu, 10 Sep 2020 11:39:44 -0400
Subject: [PATCH] xbps-triggers: make system-accounts behave in altroots

---
 srcpkgs/xbps-triggers/files/system-accounts | 141 ++++++++++----------
 srcpkgs/xbps-triggers/template              |   4 +-
 2 files changed, 75 insertions(+), 70 deletions(-)

diff --git a/srcpkgs/xbps-triggers/files/system-accounts b/srcpkgs/xbps-triggers/files/system-accounts
index d48e9b7a2cf..65485183c70 100755
--- a/srcpkgs/xbps-triggers/files/system-accounts
+++ b/srcpkgs/xbps-triggers/files/system-accounts
@@ -17,28 +17,28 @@ UPDATE="$5"
 export PATH="usr/sbin:usr/bin:/usr/sbin:/usr/bin:/sbin:/bin"
 
 group_add() {
-	local _grname _gid use_gid
+	local _pretty_grname _grname _gid
+
+	if ! command -v groupadd >/dev/null 2>&1; then
+		echo "WARNING: cannot create $1 system group (missing groupadd)"
+		echo "The following group must be created manually: $1"
+		return
+	fi
 
 	_grname="${1%:*}"
-	_gid="${1#*:}"
+	_gid="${1##*:}"
 
-	if [ "${_gid}" != "${_grname}" ]; then
-		use_gid="gid ${_gid}"
-	fi
+	[ "${_grname}" = "${_gid}" ] && _gid=
 
-	if ! getent group ${_grname} >/dev/null; then
-		if [ -n "$use_gid" ]; then
-			groupadd -r ${_grname} -g ${_gid} >/dev/null 2>&1
-		else
-			groupadd -r ${_grname} >/dev/null 2>&1
-		fi
-		if [ $? -eq 0 ]; then
-			echo "Created ${_grname} ($use_gid) system group."
-		else
-			echo "Failed to create ${_grname} ($use_gid) system group!"
-			exit 1
-		fi
-	fi
+	_pretty_grname="${_grname}${_gid:+ (gid: ${_gid})}"
+
+	groupadd -P . -r ${_grname} ${_gid:+-g ${_gid}} >/dev/null 2>&1
+
+	case $? in
+		0) echo "Created ${_pretty_grname} system group." ;;
+		9) ;;
+		*) echo "Failed to create system group ${_pretty_grname}!"; exit 1;;
+	esac
 }
 
 case "$ACTION" in
@@ -46,23 +46,17 @@ targets)
 	echo "post-install pre-remove"
 	;;
 run)
-	if [ -z "$system_accounts" -a -z "$system_groups" ]; then
-		exit 0
-	fi
+	[ -z "$system_accounts" -a -z "$system_groups" ] && exit 0
 
-	if [ -x sbin/useradd -o -x bin/useradd ]; then
+	if command -v useradd >/dev/null 2>&1; then
 		USERADD=1
 	fi
-	if [ -x sbin/usermod -o -x bin/usermod ]; then
+
+	if command -v usermod >/dev/null 2>&1; then
 		USERMOD=1
 	fi
-	if [ -x sbin/groupadd -o -x bin/groupadd ]; then
-		GROUPADD=1
-	fi
-	if [ -x bin/getent -o -x sbin/getent ]; then
-		GETENT=1
-	fi
-	if [ -x bin/passwd -o -x sbin/passwd ]; then
+
+	if command -v passwd >/dev/null 2>&1; then
 		PASSWD=1
 	fi
 
@@ -70,36 +64,39 @@ run)
 	post-install)
 		# System groups required by a package.
 		for grp in ${system_groups}; do
-			if [ -z "$GROUPADD" -a -z "$GETENT" ]; then
-				echo "WARNING: cannot create ${grp} system group (missing groupadd/getent)"
-				echo "The following group must be created manually: $grp"
-				continue
-			fi
 			group_add $grp
 		done
 
 		# System user/group required by a package.
 		for acct in ${system_accounts}; do
 			_uname="${acct%:*}"
-			_uid="${acct#*:}"
+			_uid="${acct##*:}"
+
+			[ "${_uname}" = "${_uid}" ] && _uid=
 
 			eval homedir="\$${_uname}_homedir"
 			eval shell="\$${_uname}_shell"
 			eval descr="\$${_uname}_descr"
 			eval groups="\$${_uname}_groups"
 			eval pgroup="\$${_uname}_pgroup"
+
 			[ -z "$homedir" ] && homedir="/var/empty"
 			[ -z "$shell" ] && shell="/sbin/nologin"
 			[ -z "$descr" ] && descr="${_uname} unprivileged user"
 			[ -n "$groups" ] && user_groups="-G $groups"
 
-			[ "${_uid}" != "${_uname}" ] &&
+			if [ -n "${_uid}" ]; then
 				use_id="-u ${_uid} -g ${pgroup:-${_uid}}"
+				_pretty_uname="${_uname} (uid: ${_uid})"
+			else
+				use_id="-g ${pgroup:-${_uname}}"
+				_pretty_uname="${_uname}"
+			fi
 
-			if [ -z "$USERADD" -a -z "$GETENT" -a -z "$PASSWD" ]; then
-				echo "WARNING: cannot create ${acct} system user/group (missing useradd/getent/passwd)"
+			if [ -z "$USERADD" -a -z "$PASSWD" ]; then
+				echo "WARNING: cannot create ${acct} system user/group (missing useradd/passwd)"
 				echo "The following system account must be created:"
-				echo "   Account: ${uname:-${_uid}} (uid: '${_uid}')"
+				echo "   Account: ${_pretty_uname}"
 				echo "   Description: '${descr}'"
 				echo "   Homedir: '${homedir}'"
 				echo "   Shell: '${shell}'"
@@ -109,45 +106,53 @@ run)
 
 			group_add ${pgroup:-${acct}}
 
-			if ! getent passwd ${_uname} >/dev/null; then
-				useradd -c "$descr" -d "$homedir" -s "$shell" ${user_groups} \
-					${pgroup:+-N} ${use_id:=-g ${pgroup:-${_uname}}} -r ${_uname} && \
-					passwd -l ${_uname} >/dev/null 2>&1
-				if [ $? -eq 0 ]; then
-					echo "Created ${_uname} (${_uid}) system user."
-				else
-					echo "Failed to create ${acct} system user!"
-					exit 1
-				fi
-			else
-				if [ -z "$USERMOD" ]; then
-					echo "WARNING: cannot update ${acct} system user/group (missing usermod)"
-					continue
-				fi
-				usermod -c "${descr}" -d "${homedir}" -s "${shell}" ${user_groups} \
-					-g "${pgroup:-${_uname}}" ${_uname} >/dev/null 2>&1
-				if [ $? -eq 0 ]; then
-					echo "Updated ${_uname} (${_uid}) system user."
-				else
-					echo "Failed to update ${acct} system user!"
+			useradd -P . -c "${descr}" -d "${homedir}" ${use_id} \
+				${pgroup:+-N} -s "${shell}" ${user_groups} \
+				-r ${_uname} >/dev/null 2>&1
+
+			case $? in
+				0)
+					passwd -R . -l ${_uname} >/dev/null 2>&1
+					echo "Created ${_pretty_uname} system user."
+					;;
+				9)
+					if [ -z "$USERMOD" ]; then
+						echo "WARNING: cannot update ${_pretty_uname} system user/group (missing usermod)"
+						continue
+					fi
+					usermod -P . -c "${descr}" -d "${homedir}" \
+						-s "${shell}" ${user_groups} \
+						-g "${pgroup:-${_uname}}" ${_uname} >/dev/null 2>&1
+					if [ $? -eq 0 ]; then
+						echo "Updated ${_uname} system user."
+					else
+						echo "Failed to update ${_uname} system user!"
+						exit 1
+					fi
+					;;
+				*)
+					echo "Failed to create system user ${_pretty_uname}!"
 					exit 1
-				fi
-			fi
+					;;
+			esac
 		done
 		;;
 	pre-remove)
 		if [ "$UPDATE" = "no" ]; then
 			for acct in ${system_accounts}; do
 				_uname="${acct%:*}"
-				_uid="${acct#*:}"
-				comment="$(getent passwd "${_uname}" |cut -d: -f5 |head -n1) - for uninstalled package ${PKGNAME}"
+
+				comment="$( (getent passwd "${_uname}" | cut -d: -f5 | head -n1) 2>/dev/null )"
+				comment="${comment:-unprivileged user} - for uninstalled package ${PKGNAME}"
+
 				if [ -z "$USERMOD" ]; then
-					echo "WARNING: cannot disable ${acct} system user/group (missing usermod)"
+					echo "WARNING: cannot disable ${_uname} system user (missing usermod)"
 					continue
 				fi
-				usermod -L -d /var/empty -s /bin/false -c "${comment}" ${_uname} >/dev/null 2>&1
+
+				usermod -P . -L -d /var/empty -s /bin/false -c "${comment}" ${_uname} >/dev/null 2>&1
 				if [ $? -eq 0 ]; then
-					echo "Disabled ${_uname} (${_uid}) system user/group."
+					echo "Disabled ${_uname} system user."
 				fi
 			done
 		fi
diff --git a/srcpkgs/xbps-triggers/template b/srcpkgs/xbps-triggers/template
index f316b269a7d..4131b479bef 100644
--- a/srcpkgs/xbps-triggers/template
+++ b/srcpkgs/xbps-triggers/template
@@ -1,7 +1,7 @@
 # Template file for 'xbps-triggers'
 pkgname=xbps-triggers
-version=0.116
-revision=2
+version=0.117
+revision=1
 bootstrap=yes
 short_desc="XBPS triggers for Void Linux"
 maintainer="Enno Boland <gottox@voidlinux.org>"

Re: xbps-triggers: make system-accounts behave in altroots

[ericonr]

[-- Attachment #1: Type: text/plain, Size: 885 bytes --]

New comment by ericonr on void-packages repository

https://github.com/void-linux/void-packages/pull/24812#issuecomment-692931476

Comment:
@the-maldridge I don't believe these changes are any less robust for systems using remote directories than the current state. It is using the proper tools anyway.

Even so, there is the argument that these shouldn't be supported, given that these are system accounts and should be available in all situations.

We have discussed the possibility of bringing the `system_{account,group}` functionality into XBPS itself, but I believe it would also break in the alternate root case with remote directory, due to the host system's setup not being necessarily the same as the target system. More importantly, someone using `xbps.static` on a glibc system would lack the extra functionality implemented by glibc, if I understand things correctly.

Re: [PR PATCH] [Updated] xbps-triggers: make system-accounts behave in altroots

[ahesford]

[-- Attachment #1: Type: text/plain, Size: 2608 bytes --]

There is an updated pull request by ahesford against master on the void-packages repository

https://github.com/ahesford/void-packages xbps-triggers
https://github.com/void-linux/void-packages/pull/24812

xbps-triggers: make system-accounts behave in altroots
This attempts to fix some issues uncovered in #24754. The changes:
- Use `command -v` to check for `groupadd`, `usermod`, `useradd` and `passwd` rather than looking for executable files at a fixed path, to match what the shell will actually do when these programs are invoked.
- Uses the `-P|--prefix` options to `groupadd`, `usermod` and `useradd`, as well as the `-R|--root` option to `passwd`, to manipulate files in the target root (which should be `.`) rather than on the host system.
- Skips `getent` altogether, instead letting the `useradd` and `groupadd` programs detect and do the right thing when attempting to create existing users or groups (`getent` is still used in the `pre-remove` hook to try to extract the GECOS field for an account, but this is a non-critical operation and we can tolerate pulling the wrong comment in some edge cases in `xbps-remove -r`).

As a bonus, I cleaned up the hook output a bit, avoiding some strange text artifacts when user or group IDs are unspecified.

There are ~200 packages that specify system accounts or groups. I've tested this on a few that seem to offer a range of different user/group styles:
- sndio (system account with primary group `audio`, tests account creation with no user group)
- dbus (system account with specific UID, tests account creation with user group of matching GID)
- kismet (system group only, no user group)
- pulseaudio (system account with matching group, system group, additional groups for account)
- rtkit (system account with matching group)

These packages were installed on the local host by the superuser, and were also installed by an unprivileged user into an alternate root owned by that user; some packages were also installed to the unprivileged root by the superuser. In all cases, the hooks seem to correctly update `etc/passwd` and `etc/group` with respect to creating, disabling and reenabling accounts.

I have no means to test functionality on a system using a remote directory service but these changes at least prevent an altroot installation from leaking account creation or modification into the local host files while also ensuring that the account modifications are made in the altroot.

cc: @void-linux/pkg-committers @ericonr 


A patch file from https://github.com/void-linux/void-packages/pull/24812.patch is attached

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: github-pr-xbps-triggers-24812.patch --]
[-- Type: text/x-diff, Size: 8151 bytes --]

From 9cc9dafd28fc176a9c045d7461e022996347fd4a Mon Sep 17 00:00:00 2001
From: "Andrew J. Hesford" <ajh@sideband.org>
Date: Thu, 10 Sep 2020 11:39:44 -0400
Subject: [PATCH] xbps-triggers: make system-accounts behave in altroots

---
 srcpkgs/xbps-triggers/files/system-accounts | 168 +++++++++++---------
 srcpkgs/xbps-triggers/template              |   4 +-
 2 files changed, 94 insertions(+), 78 deletions(-)

diff --git a/srcpkgs/xbps-triggers/files/system-accounts b/srcpkgs/xbps-triggers/files/system-accounts
index d48e9b7a2cf..7d04a99e8b0 100755
--- a/srcpkgs/xbps-triggers/files/system-accounts
+++ b/srcpkgs/xbps-triggers/files/system-accounts
@@ -16,29 +16,45 @@ UPDATE="$5"
 
 export PATH="usr/sbin:usr/bin:/usr/sbin:/usr/bin:/sbin:/bin"
 
+# Determine whether useradd/groupadd/usermod need a prefix argument
+if [ "$(readlink -f . 2>/dev/null || echo .)" != "/" ]; then
+	prefix="-P ."
+else
+	prefix=
+fi
+
+# show_acct_details <username> <description> <homedir> <shell> [groups]
+show_acct_details() {
+	echo "   Account: $1"
+	echo "   Description: '$2'"
+	echo "   Homedir: '$3'"
+	echo "   Shell: '$4'"
+	[ -n "$5" ] && echo "   Additional groups: '$5'"
+}
+
 group_add() {
-	local _grname _gid use_gid
+	local _pretty_grname _grname _gid _prefix
+
+	if ! command -v groupadd >/dev/null 2>&1; then
+		echo "WARNING: cannot create $1 system group (missing groupadd)"
+		echo "The following group must be created manually: $1"
+		return
+	fi
 
 	_grname="${1%:*}"
-	_gid="${1#*:}"
+	_gid="${1##*:}"
 
-	if [ "${_gid}" != "${_grname}" ]; then
-		use_gid="gid ${_gid}"
-	fi
+	[ "${_grname}" = "${_gid}" ] && _gid=
 
-	if ! getent group ${_grname} >/dev/null; then
-		if [ -n "$use_gid" ]; then
-			groupadd -r ${_grname} -g ${_gid} >/dev/null 2>&1
-		else
-			groupadd -r ${_grname} >/dev/null 2>&1
-		fi
-		if [ $? -eq 0 ]; then
-			echo "Created ${_grname} ($use_gid) system group."
-		else
-			echo "Failed to create ${_grname} ($use_gid) system group!"
-			exit 1
-		fi
-	fi
+	_pretty_grname="${_grname}${_gid:+ (gid: ${_gid})}"
+
+	groupadd ${prefix} -r ${_grname} ${_gid:+-g ${_gid}} >/dev/null 2>&1
+
+	case $? in
+		0) echo "Created ${_pretty_grname} system group." ;;
+		9) ;;
+		*) echo "ERROR: failed to create system group ${_pretty_grname}!"; exit 1;;
+	esac
 }
 
 case "$ACTION" in
@@ -46,108 +62,108 @@ targets)
 	echo "post-install pre-remove"
 	;;
 run)
-	if [ -z "$system_accounts" -a -z "$system_groups" ]; then
-		exit 0
-	fi
+	[ -z "$system_accounts" -a -z "$system_groups" ] && exit 0
 
-	if [ -x sbin/useradd -o -x bin/useradd ]; then
-		USERADD=1
-	fi
-	if [ -x sbin/usermod -o -x bin/usermod ]; then
-		USERMOD=1
-	fi
-	if [ -x sbin/groupadd -o -x bin/groupadd ]; then
-		GROUPADD=1
+	if command -v useradd >/dev/null 2>&1; then
+		USERADD="useradd ${prefix}"
 	fi
-	if [ -x bin/getent -o -x sbin/getent ]; then
-		GETENT=1
-	fi
-	if [ -x bin/passwd -o -x sbin/passwd ]; then
-		PASSWD=1
+
+	if command -v usermod >/dev/null 2>&1; then
+		USERMOD="usermod ${prefix}"
 	fi
 
 	case "$TARGET" in
 	post-install)
 		# System groups required by a package.
 		for grp in ${system_groups}; do
-			if [ -z "$GROUPADD" -a -z "$GETENT" ]; then
-				echo "WARNING: cannot create ${grp} system group (missing groupadd/getent)"
-				echo "The following group must be created manually: $grp"
-				continue
-			fi
 			group_add $grp
 		done
 
 		# System user/group required by a package.
 		for acct in ${system_accounts}; do
 			_uname="${acct%:*}"
-			_uid="${acct#*:}"
+			_uid="${acct##*:}"
+
+			[ "${_uname}" = "${_uid}" ] && _uid=
 
 			eval homedir="\$${_uname}_homedir"
 			eval shell="\$${_uname}_shell"
 			eval descr="\$${_uname}_descr"
 			eval groups="\$${_uname}_groups"
 			eval pgroup="\$${_uname}_pgroup"
+
 			[ -z "$homedir" ] && homedir="/var/empty"
 			[ -z "$shell" ] && shell="/sbin/nologin"
 			[ -z "$descr" ] && descr="${_uname} unprivileged user"
 			[ -n "$groups" ] && user_groups="-G $groups"
 
-			[ "${_uid}" != "${_uname}" ] &&
+			if [ -n "${_uid}" ]; then
 				use_id="-u ${_uid} -g ${pgroup:-${_uid}}"
+				_pretty_uname="${_uname} (uid: ${_uid})"
+			else
+				use_id="-g ${pgroup:-${_uname}}"
+				_pretty_uname="${_uname}"
+			fi
 
-			if [ -z "$USERADD" -a -z "$GETENT" -a -z "$PASSWD" ]; then
-				echo "WARNING: cannot create ${acct} system user/group (missing useradd/getent/passwd)"
+			if [ -z "$USERADD" -o -z "$USERMOD" ]; then
+				echo "WARNING: cannot create ${_uname} system account (missing useradd or usermod)"
 				echo "The following system account must be created:"
-				echo "   Account: ${uname:-${_uid}} (uid: '${_uid}')"
-				echo "   Description: '${descr}'"
-				echo "   Homedir: '${homedir}'"
-				echo "   Shell: '${shell}'"
-				echo "   Additional groups: '${groups}'"
+				show_acct_details "${_pretty_uname}" "${descr}" "${homedir}" "${shell}" "${groups}"
 				continue
 			fi
 
 			group_add ${pgroup:-${acct}}
 
-			if ! getent passwd ${_uname} >/dev/null; then
-				useradd -c "$descr" -d "$homedir" -s "$shell" ${user_groups} \
-					${pgroup:+-N} ${use_id:=-g ${pgroup:-${_uname}}} -r ${_uname} && \
-					passwd -l ${_uname} >/dev/null 2>&1
-				if [ $? -eq 0 ]; then
-					echo "Created ${_uname} (${_uid}) system user."
-				else
-					echo "Failed to create ${acct} system user!"
-					exit 1
-				fi
-			else
-				if [ -z "$USERMOD" ]; then
-					echo "WARNING: cannot update ${acct} system user/group (missing usermod)"
-					continue
-				fi
-				usermod -c "${descr}" -d "${homedir}" -s "${shell}" ${user_groups} \
-					-g "${pgroup:-${_uname}}" ${_uname} >/dev/null 2>&1
-				if [ $? -eq 0 ]; then
-					echo "Updated ${_uname} (${_uid}) system user."
-				else
-					echo "Failed to update ${acct} system user!"
+			${USERADD} -c "${descr}" -d "${homedir}" \
+				${use_id} ${pgroup:+-N} -s "${shell}" \
+				${user_groups} -r ${_uname} >/dev/null 2>&1
+
+			case $? in
+				0)
+					echo "Created ${_pretty_uname} system user."
+					${USERMOD} -L ${_uname} >/dev/null 2>&1
+					if [ $? -ne 0 ]; then
+						echo "WARNING: unable to lock password for ${_uname} system account"
+					fi
+					;;
+				9)
+					${USERMOD} -c "${descr}" -d "${homedir}" \
+						-g "${pgroup:-${_uname}}" ${user_groups} \
+						-s "${shell}" ${_uname} >/dev/null 2>&1
+					if [ $? -eq 0 ]; then
+						echo "Updated ${_uname} system user."
+					else
+						echo "WARNING: unable to modify ${_uname} system account"
+						echo "Please verify that account is compatible with these settings:"
+						show_acct_details "${_pretty_uname}" \
+							"${descr}" "${homedir}" "${shell}" "${groups}"
+						continue
+					fi
+					;;
+				*)
+					echo "ERROR: failed to create system user ${_pretty_uname}!"
 					exit 1
-				fi
-			fi
+					;;
+			esac
 		done
 		;;
 	pre-remove)
 		if [ "$UPDATE" = "no" ]; then
 			for acct in ${system_accounts}; do
 				_uname="${acct%:*}"
-				_uid="${acct#*:}"
-				comment="$(getent passwd "${_uname}" |cut -d: -f5 |head -n1) - for uninstalled package ${PKGNAME}"
+
+				comment="$( (getent passwd "${_uname}" | cut -d: -f5 | head -n1) 2>/dev/null )"
+				comment="${comment:-unprivileged user} - for uninstalled package ${PKGNAME}"
+
 				if [ -z "$USERMOD" ]; then
-					echo "WARNING: cannot disable ${acct} system user/group (missing usermod)"
+					echo "WARNING: cannot disable ${_uname} system user (missing usermod)"
 					continue
 				fi
-				usermod -L -d /var/empty -s /bin/false -c "${comment}" ${_uname} >/dev/null 2>&1
+
+				${USERMOD} -L -d /var/empty -s /bin/false \
+				       	-c "${comment}" ${_uname} >/dev/null 2>&1
 				if [ $? -eq 0 ]; then
-					echo "Disabled ${_uname} (${_uid}) system user/group."
+					echo "Disabled ${_uname} system user."
 				fi
 			done
 		fi
diff --git a/srcpkgs/xbps-triggers/template b/srcpkgs/xbps-triggers/template
index f316b269a7d..4131b479bef 100644
--- a/srcpkgs/xbps-triggers/template
+++ b/srcpkgs/xbps-triggers/template
@@ -1,7 +1,7 @@
 # Template file for 'xbps-triggers'
 pkgname=xbps-triggers
-version=0.116
-revision=2
+version=0.117
+revision=1
 bootstrap=yes
 short_desc="XBPS triggers for Void Linux"
 maintainer="Enno Boland <gottox@voidlinux.org>"

Re: xbps-triggers: make system-accounts behave in altroots

[ahesford]

[-- Attachment #1: Type: text/plain, Size: 933 bytes --]

New comment by ahesford on void-packages repository

https://github.com/void-linux/void-packages/pull/24812#issuecomment-693749172

Comment:
The latest push attempts to better accommodate NIS/LDAP directories; if the canonical target prefix is `/`, the `--prefix` option is omitted from `useradd`, `groupadd` and `usermod`, which means these utilities should refuse to create accounts that conflict with entries in the remote directories. When the target prefix is something other than the system root, the `--prefix` argument is used, which ignores NIS/LDAP and creates accounts in the local databases for that root. I believe this to be the most sane option because any assumptions about remote directories for an alternate root are bound to be problematic, and if the user is aware of an intent to configure special NSS behavior in the altroot, leaving packages unconfigured until the full environment is ready seems appropriate.

Re: xbps-triggers: make system-accounts behave in altroots

[the-maldridge]

[-- Attachment #1: Type: text/plain, Size: 179 bytes --]

New comment by the-maldridge on void-packages repository

https://github.com/void-linux/void-packages/pull/24812#issuecomment-693878878

Comment:
No further concerns for this PR.

Re: [PR PATCH] [Updated] xbps-triggers: make system-accounts behave in altroots

[ahesford]

[-- Attachment #1: Type: text/plain, Size: 2608 bytes --]

There is an updated pull request by ahesford against master on the void-packages repository

https://github.com/ahesford/void-packages xbps-triggers
https://github.com/void-linux/void-packages/pull/24812

xbps-triggers: make system-accounts behave in altroots
This attempts to fix some issues uncovered in #24754. The changes:
- Use `command -v` to check for `groupadd`, `usermod`, `useradd` and `passwd` rather than looking for executable files at a fixed path, to match what the shell will actually do when these programs are invoked.
- Uses the `-P|--prefix` options to `groupadd`, `usermod` and `useradd`, as well as the `-R|--root` option to `passwd`, to manipulate files in the target root (which should be `.`) rather than on the host system.
- Skips `getent` altogether, instead letting the `useradd` and `groupadd` programs detect and do the right thing when attempting to create existing users or groups (`getent` is still used in the `pre-remove` hook to try to extract the GECOS field for an account, but this is a non-critical operation and we can tolerate pulling the wrong comment in some edge cases in `xbps-remove -r`).

As a bonus, I cleaned up the hook output a bit, avoiding some strange text artifacts when user or group IDs are unspecified.

There are ~200 packages that specify system accounts or groups. I've tested this on a few that seem to offer a range of different user/group styles:
- sndio (system account with primary group `audio`, tests account creation with no user group)
- dbus (system account with specific UID, tests account creation with user group of matching GID)
- kismet (system group only, no user group)
- pulseaudio (system account with matching group, system group, additional groups for account)
- rtkit (system account with matching group)

These packages were installed on the local host by the superuser, and were also installed by an unprivileged user into an alternate root owned by that user; some packages were also installed to the unprivileged root by the superuser. In all cases, the hooks seem to correctly update `etc/passwd` and `etc/group` with respect to creating, disabling and reenabling accounts.

I have no means to test functionality on a system using a remote directory service but these changes at least prevent an altroot installation from leaking account creation or modification into the local host files while also ensuring that the account modifications are made in the altroot.

cc: @void-linux/pkg-committers @ericonr 


A patch file from https://github.com/void-linux/void-packages/pull/24812.patch is attached

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: github-pr-xbps-triggers-24812.patch --]
[-- Type: text/x-diff, Size: 8151 bytes --]

From 5cc296f314ad477e94f9efba659d2793c00580e9 Mon Sep 17 00:00:00 2001
From: "Andrew J. Hesford" <ajh@sideband.org>
Date: Thu, 10 Sep 2020 11:39:44 -0400
Subject: [PATCH] xbps-triggers: make system-accounts behave in altroots

---
 srcpkgs/xbps-triggers/files/system-accounts | 168 +++++++++++---------
 srcpkgs/xbps-triggers/template              |   4 +-
 2 files changed, 94 insertions(+), 78 deletions(-)

diff --git a/srcpkgs/xbps-triggers/files/system-accounts b/srcpkgs/xbps-triggers/files/system-accounts
index d48e9b7a2cf..d809d00cd83 100755
--- a/srcpkgs/xbps-triggers/files/system-accounts
+++ b/srcpkgs/xbps-triggers/files/system-accounts
@@ -16,29 +16,45 @@ UPDATE="$5"
 
 export PATH="usr/sbin:usr/bin:/usr/sbin:/usr/bin:/sbin:/bin"
 
+# Determine whether useradd/groupadd/usermod need a prefix argument
+if [ "$(readlink -f . 2>/dev/null || echo .)" != "/" ]; then
+	prefix="-P ."
+else
+	prefix=
+fi
+
+# show_acct_details <username> <description> <homedir> <shell> [groups]
+show_acct_details() {
+	echo "   Account: $1"
+	echo "   Description: '$2'"
+	echo "   Homedir: '$3'"
+	echo "   Shell: '$4'"
+	[ -n "$5" ] && echo "   Additional groups: '$5'"
+}
+
 group_add() {
-	local _grname _gid use_gid
+	local _pretty_grname _grname _gid _prefix
+
+	if ! command -v groupadd >/dev/null 2>&1; then
+		echo "WARNING: cannot create $1 system group (missing groupadd)"
+		echo "The following group must be created manually: $1"
+		return
+	fi
 
 	_grname="${1%:*}"
-	_gid="${1#*:}"
+	_gid="${1##*:}"
 
-	if [ "${_gid}" != "${_grname}" ]; then
-		use_gid="gid ${_gid}"
-	fi
+	[ "${_grname}" = "${_gid}" ] && _gid=
 
-	if ! getent group ${_grname} >/dev/null; then
-		if [ -n "$use_gid" ]; then
-			groupadd -r ${_grname} -g ${_gid} >/dev/null 2>&1
-		else
-			groupadd -r ${_grname} >/dev/null 2>&1
-		fi
-		if [ $? -eq 0 ]; then
-			echo "Created ${_grname} ($use_gid) system group."
-		else
-			echo "Failed to create ${_grname} ($use_gid) system group!"
-			exit 1
-		fi
-	fi
+	_pretty_grname="${_grname}${_gid:+ (gid: ${_gid})}"
+
+	groupadd ${prefix} -r ${_grname} ${_gid:+-g ${_gid}} >/dev/null 2>&1
+
+	case $? in
+		0) echo "Created ${_pretty_grname} system group." ;;
+		9) ;;
+		*) echo "ERROR: failed to create system group ${_pretty_grname}!"; exit 1;;
+	esac
 }
 
 case "$ACTION" in
@@ -46,108 +62,108 @@ targets)
 	echo "post-install pre-remove"
 	;;
 run)
-	if [ -z "$system_accounts" -a -z "$system_groups" ]; then
-		exit 0
-	fi
+	[ -z "$system_accounts" -a -z "$system_groups" ] && exit 0
 
-	if [ -x sbin/useradd -o -x bin/useradd ]; then
-		USERADD=1
-	fi
-	if [ -x sbin/usermod -o -x bin/usermod ]; then
-		USERMOD=1
-	fi
-	if [ -x sbin/groupadd -o -x bin/groupadd ]; then
-		GROUPADD=1
+	if command -v useradd >/dev/null 2>&1; then
+		USERADD="useradd ${prefix}"
 	fi
-	if [ -x bin/getent -o -x sbin/getent ]; then
-		GETENT=1
-	fi
-	if [ -x bin/passwd -o -x sbin/passwd ]; then
-		PASSWD=1
+
+	if command -v usermod >/dev/null 2>&1; then
+		USERMOD="usermod ${prefix}"
 	fi
 
 	case "$TARGET" in
 	post-install)
 		# System groups required by a package.
 		for grp in ${system_groups}; do
-			if [ -z "$GROUPADD" -a -z "$GETENT" ]; then
-				echo "WARNING: cannot create ${grp} system group (missing groupadd/getent)"
-				echo "The following group must be created manually: $grp"
-				continue
-			fi
 			group_add $grp
 		done
 
 		# System user/group required by a package.
 		for acct in ${system_accounts}; do
 			_uname="${acct%:*}"
-			_uid="${acct#*:}"
+			_uid="${acct##*:}"
+
+			[ "${_uname}" = "${_uid}" ] && _uid=
 
 			eval homedir="\$${_uname}_homedir"
 			eval shell="\$${_uname}_shell"
 			eval descr="\$${_uname}_descr"
 			eval groups="\$${_uname}_groups"
 			eval pgroup="\$${_uname}_pgroup"
+
 			[ -z "$homedir" ] && homedir="/var/empty"
 			[ -z "$shell" ] && shell="/sbin/nologin"
 			[ -z "$descr" ] && descr="${_uname} unprivileged user"
 			[ -n "$groups" ] && user_groups="-G $groups"
 
-			[ "${_uid}" != "${_uname}" ] &&
+			if [ -n "${_uid}" ]; then
 				use_id="-u ${_uid} -g ${pgroup:-${_uid}}"
+				_pretty_uname="${_uname} (uid: ${_uid})"
+			else
+				use_id="-g ${pgroup:-${_uname}}"
+				_pretty_uname="${_uname}"
+			fi
 
-			if [ -z "$USERADD" -a -z "$GETENT" -a -z "$PASSWD" ]; then
-				echo "WARNING: cannot create ${acct} system user/group (missing useradd/getent/passwd)"
+			if [ -z "$USERADD" -o -z "$USERMOD" ]; then
+				echo "WARNING: cannot create ${_uname} system account (missing useradd or usermod)"
 				echo "The following system account must be created:"
-				echo "   Account: ${uname:-${_uid}} (uid: '${_uid}')"
-				echo "   Description: '${descr}'"
-				echo "   Homedir: '${homedir}'"
-				echo "   Shell: '${shell}'"
-				echo "   Additional groups: '${groups}'"
+				show_acct_details "${_pretty_uname}" "${descr}" "${homedir}" "${shell}" "${groups}"
 				continue
 			fi
 
 			group_add ${pgroup:-${acct}}
 
-			if ! getent passwd ${_uname} >/dev/null; then
-				useradd -c "$descr" -d "$homedir" -s "$shell" ${user_groups} \
-					${pgroup:+-N} ${use_id:=-g ${pgroup:-${_uname}}} -r ${_uname} && \
-					passwd -l ${_uname} >/dev/null 2>&1
-				if [ $? -eq 0 ]; then
-					echo "Created ${_uname} (${_uid}) system user."
-				else
-					echo "Failed to create ${acct} system user!"
-					exit 1
-				fi
-			else
-				if [ -z "$USERMOD" ]; then
-					echo "WARNING: cannot update ${acct} system user/group (missing usermod)"
-					continue
-				fi
-				usermod -c "${descr}" -d "${homedir}" -s "${shell}" ${user_groups} \
-					-g "${pgroup:-${_uname}}" ${_uname} >/dev/null 2>&1
-				if [ $? -eq 0 ]; then
-					echo "Updated ${_uname} (${_uid}) system user."
-				else
-					echo "Failed to update ${acct} system user!"
+			${USERADD} -c "${descr}" -d "${homedir}" \
+				${use_id} ${pgroup:+-N} -s "${shell}" \
+				${user_groups} -r ${_uname} >/dev/null 2>&1
+
+			case $? in
+				0)
+					echo "Created ${_pretty_uname} system user."
+					${USERMOD} -L ${_uname} >/dev/null 2>&1
+					if [ $? -ne 0 ]; then
+						echo "WARNING: unable to lock password for ${_uname} system account"
+					fi
+					;;
+				9)
+					${USERMOD} -c "${descr}" -d "${homedir}" \
+						-s "${shell}" -g "${pgroup:-${_uname}}" \
+						${user_groups} ${_uname} >/dev/null 2>&1
+					if [ $? -eq 0 ]; then
+						echo "Updated ${_uname} system user."
+					else
+						echo "WARNING: unable to modify ${_uname} system account"
+						echo "Please verify that account is compatible with these settings:"
+						show_acct_details "${_pretty_uname}" \
+							"${descr}" "${homedir}" "${shell}" "${groups}"
+						continue
+					fi
+					;;
+				*)
+					echo "ERROR: failed to create system user ${_pretty_uname}!"
 					exit 1
-				fi
-			fi
+					;;
+			esac
 		done
 		;;
 	pre-remove)
 		if [ "$UPDATE" = "no" ]; then
 			for acct in ${system_accounts}; do
 				_uname="${acct%:*}"
-				_uid="${acct#*:}"
-				comment="$(getent passwd "${_uname}" |cut -d: -f5 |head -n1) - for uninstalled package ${PKGNAME}"
+
+				comment="$( (getent passwd "${_uname}" | cut -d: -f5 | head -n1) 2>/dev/null )"
+				comment="${comment:-unprivileged user} - for uninstalled package ${PKGNAME}"
+
 				if [ -z "$USERMOD" ]; then
-					echo "WARNING: cannot disable ${acct} system user/group (missing usermod)"
+					echo "WARNING: cannot disable ${_uname} system user (missing usermod)"
 					continue
 				fi
-				usermod -L -d /var/empty -s /bin/false -c "${comment}" ${_uname} >/dev/null 2>&1
+
+				${USERMOD} -L -d /var/empty -s /bin/false \
+				       	-c "${comment}" ${_uname} >/dev/null 2>&1
 				if [ $? -eq 0 ]; then
-					echo "Disabled ${_uname} (${_uid}) system user/group."
+					echo "Disabled ${_uname} system user."
 				fi
 			done
 		fi
diff --git a/srcpkgs/xbps-triggers/template b/srcpkgs/xbps-triggers/template
index f316b269a7d..4131b479bef 100644
--- a/srcpkgs/xbps-triggers/template
+++ b/srcpkgs/xbps-triggers/template
@@ -1,7 +1,7 @@
 # Template file for 'xbps-triggers'
 pkgname=xbps-triggers
-version=0.116
-revision=2
+version=0.117
+revision=1
 bootstrap=yes
 short_desc="XBPS triggers for Void Linux"
 maintainer="Enno Boland <gottox@voidlinux.org>"

Re: [PR PATCH] [Closed]: xbps-triggers: make system-accounts behave in altroots

[ahesford]

[-- Attachment #1: Type: text/plain, Size: 2444 bytes --]

There's a closed pull request on the void-packages repository

xbps-triggers: make system-accounts behave in altroots
https://github.com/void-linux/void-packages/pull/24812

Description:
This attempts to fix some issues uncovered in #24754. The changes:
- Use `command -v` to check for `groupadd`, `usermod`, `useradd` and `passwd` rather than looking for executable files at a fixed path, to match what the shell will actually do when these programs are invoked.
- Uses the `-P|--prefix` options to `groupadd`, `usermod` and `useradd`, as well as the `-R|--root` option to `passwd`, to manipulate files in the target root (which should be `.`) rather than on the host system.
- Skips `getent` altogether, instead letting the `useradd` and `groupadd` programs detect and do the right thing when attempting to create existing users or groups (`getent` is still used in the `pre-remove` hook to try to extract the GECOS field for an account, but this is a non-critical operation and we can tolerate pulling the wrong comment in some edge cases in `xbps-remove -r`).

As a bonus, I cleaned up the hook output a bit, avoiding some strange text artifacts when user or group IDs are unspecified.

There are ~200 packages that specify system accounts or groups. I've tested this on a few that seem to offer a range of different user/group styles:
- sndio (system account with primary group `audio`, tests account creation with no user group)
- dbus (system account with specific UID, tests account creation with user group of matching GID)
- kismet (system group only, no user group)
- pulseaudio (system account with matching group, system group, additional groups for account)
- rtkit (system account with matching group)

These packages were installed on the local host by the superuser, and were also installed by an unprivileged user into an alternate root owned by that user; some packages were also installed to the unprivileged root by the superuser. In all cases, the hooks seem to correctly update `etc/passwd` and `etc/group` with respect to creating, disabling and reenabling accounts.

I have no means to test functionality on a system using a remote directory service but these changes at least prevent an altroot installation from leaking account creation or modification into the local host files while also ensuring that the account modifications are made in the altroot.

cc: @void-linux/pkg-committers @ericonr