Github messages for voidlinux
 help / color / mirror / Atom feed
From: ahesford <ahesford@users.noreply.github.com>
To: ml@inbox.vuxu.org
Subject: Re: [PR PATCH] [Updated] xbps-triggers: make system-accounts behave in altroots
Date: Fri, 18 Sep 2020 15:44:16 +0200	[thread overview]
Message-ID: <20200918134416.r2eM3hvV-XjPTVr5I1WBE0OxTJ2usUK_vZcgSIArBkk@z> (raw)
In-Reply-To: <gh-mailinglist-notifications-41a7ca26-5023-4802-975b-f1789d68868e-void-packages-24812@inbox.vuxu.org>

[-- 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>"

  parent reply	other threads:[~2020-09-18 13:44 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-10 18:37 [PR PATCH] " ahesford
2020-09-15 19:32 ` ericonr
2020-09-17  1:04 ` [PR PATCH] [Updated] " ahesford
2020-09-17  1:15 ` ahesford
2020-09-17  5:18 ` the-maldridge
2020-09-18 13:44 ` ahesford [this message]
2020-09-18 14:00 ` [PR PATCH] [Closed]: " ahesford

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200918134416.r2eM3hvV-XjPTVr5I1WBE0OxTJ2usUK_vZcgSIArBkk@z \
    --to=ahesford@users.noreply.github.com \
    --cc=ml@inbox.vuxu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).