Github messages for voidlinux
 help / color / mirror / Atom feed
* [PR PATCH] xbps-triggers:system-accounts: use grep to check for user/group existent
@ 2020-09-08  0:40 sgn
  2020-09-08  1:29 ` ericonr
                   ` (16 more replies)
  0 siblings, 17 replies; 18+ messages in thread
From: sgn @ 2020-09-08  0:40 UTC (permalink / raw)
  To: ml

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

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

https://github.com/sgn/void-packages xbps-triggres-system-account
https://github.com/void-linux/void-packages/pull/24754

xbps-triggers:system-accounts: use grep to check for user/group existent
In `system-accounts` triggers, we're using `getent(1)` to check whether
the username or group in question is existed before doing the heavy
lifting.

However, `getent(1)` will check the database in host system instead of our
rootfs, and by `PATH` manipulation logic, we prefer to use `usr/bin/getent`
inside our rootfs instead of host `getent(1)`.

This is usually not a problem since we mostly run `xbps-triggers` in
a real system instead of running from foreign system.

Except for `base-files` packages, which used to not have group `kvm`
pre-allocated. Thus, requires running this trigger, and lead to all sort
of problems:
- If host system is a musl-based linux system, with gcompat installed,
  and we're bootstrapping a glibc one, `getent(1)` will be executable,
  however, when `getent(1)` attempt to `dlopen(3)` other libraries,
  it'll run into failure.
- If host system doesn't have `kvm` group pre-allocated (bootstrapping
  from foreign distro), we attempt to run `groupadd(1)` on such system,
  thus failing with EPERM.

If we run into one of those cases, `xbps-reconfigure(1)` will stop
configuring `base-files`, not running `base-files`' `INSTALL` and leave
the system in half-baked state, without some requires files and
directories.

Switch to `grep(1)` to check for username and group existence,
since `passwd(5)` and `group(5)` is well-documented.

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

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: github-pr-xbps-triggres-system-account-24754.patch --]
[-- Type: text/x-diff, Size: 3844 bytes --]

From 73d59167ed32631e3e9011d72c0e087ba172418f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C4=90o=C3=A0n=20Tr=E1=BA=A7n=20C=C3=B4ng=20Danh?=
 <congdanhqx@gmail.com>
Date: Tue, 8 Sep 2020 07:12:30 +0700
Subject: [PATCH] xbps-triggers:system-accounts: use grep to check for
 user/group existent

In `system-accounts` triggers, we're using `getent(1)` to check whether
the username or group in question is existed before doing the heavy
lifting.

However, `getent(1)` will check the database in host system instead of our
rootfs, and by `PATH` manipulation logic, we prefer to use `usr/bin/getent`
inside our rootfs instead of host `getent(1)`.

This is usually not a problem since we mostly run `xbps-triggers` in
a real system instead of running from foreign system.

Except for `base-files` packages, which used to not have group `kvm`
pre-allocated. Thus, requires running this trigger, and lead to all sort
of problems:
- If host system is a musl-based linux system, with gcompat installed,
  and we're bootstrapping a glibc one, `getent(1)` will be executable,
  however, when `getent(1)` attempt to `dlopen(3)` other libraries,
  it'll run into failure.
- If host system doesn't have `kvm` group pre-allocated (bootstrapping
  from foreign distro), we attempt to run `groupadd(1)` on such system,
  thus failing with EPERM.

If we run into one of those cases, `xbps-reconfigure(1)` will stop
configuring `base-files`, not running `base-files`' `INSTALL` and leave
the system in half-baked state, without some requires files and
directories.

Switch to `grep(1)` to check for username and group existence,
since `passwd(5)` and `group(5)` is well-documented.
---
 srcpkgs/xbps-triggers/files/system-accounts | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/srcpkgs/xbps-triggers/files/system-accounts b/srcpkgs/xbps-triggers/files/system-accounts
index d48e9b7a2cf..f87fd0d9c03 100755
--- a/srcpkgs/xbps-triggers/files/system-accounts
+++ b/srcpkgs/xbps-triggers/files/system-accounts
@@ -26,7 +26,7 @@ group_add() {
 		use_gid="gid ${_gid}"
 	fi
 
-	if ! getent group ${_grname} >/dev/null; then
+	if ! grep -q "^${_grname}:" etc/group >/dev/null; then
 		if [ -n "$use_gid" ]; then
 			groupadd -r ${_grname} -g ${_gid} >/dev/null 2>&1
 		else
@@ -59,9 +59,6 @@ run)
 	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
 		PASSWD=1
 	fi
@@ -70,8 +67,8 @@ 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)"
+			if [ -z "$GROUPADD" ]; then
+				echo "WARNING: cannot create ${grp} system group (missing groupadd)"
 				echo "The following group must be created manually: $grp"
 				continue
 			fi
@@ -96,8 +93,8 @@ run)
 			[ "${_uid}" != "${_uname}" ] &&
 				use_id="-u ${_uid} -g ${pgroup:-${_uid}}"
 
-			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 "   Description: '${descr}'"
@@ -109,7 +106,7 @@ run)
 
 			group_add ${pgroup:-${acct}}
 
-			if ! getent passwd ${_uname} >/dev/null; then
+			if ! grep -q "^${_uname}:" etc/passwd >/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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: xbps-triggers:system-accounts: use grep to check for user/group existent
  2020-09-08  0:40 [PR PATCH] xbps-triggers:system-accounts: use grep to check for user/group existent sgn
@ 2020-09-08  1:29 ` ericonr
  2020-09-08 12:04 ` sgn
                   ` (15 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: ericonr @ 2020-09-08  1:29 UTC (permalink / raw)
  To: ml

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

New comment by ericonr on void-packages repository

https://github.com/void-linux/void-packages/pull/24754#issuecomment-688567352

Comment:
Remember to change the version / revbump the package.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: xbps-triggers:system-accounts: use grep to check for user/group existent
  2020-09-08  0:40 [PR PATCH] xbps-triggers:system-accounts: use grep to check for user/group existent sgn
  2020-09-08  1:29 ` ericonr
@ 2020-09-08 12:04 ` sgn
  2020-09-09  1:07 ` [PR PATCH] [Updated] " sgn
                   ` (14 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: sgn @ 2020-09-08 12:04 UTC (permalink / raw)
  To: ml

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

New comment by sgn on void-packages repository

https://github.com/void-linux/void-packages/pull/24754#issuecomment-688822519

Comment:
On 2020-09-07 18:29:26-0700, Érico Nogueira Rolim <notifications@github.com> wrote:
> Remember to change the version / revbump the package.

Yup, I forgot to revbump (since no behavior changes).

Also, would wait for other's opinions.


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PR PATCH] [Updated] xbps-triggers:system-accounts: use grep to check for user/group existent
  2020-09-08  0:40 [PR PATCH] xbps-triggers:system-accounts: use grep to check for user/group existent sgn
  2020-09-08  1:29 ` ericonr
  2020-09-08 12:04 ` sgn
@ 2020-09-09  1:07 ` sgn
  2020-09-09 14:04 ` sgn
                   ` (13 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: sgn @ 2020-09-09  1:07 UTC (permalink / raw)
  To: ml

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

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

https://github.com/sgn/void-packages xbps-triggres-system-account
https://github.com/void-linux/void-packages/pull/24754

xbps-triggers:system-accounts: use grep to check for user/group existent
In `system-accounts` triggers, we're using `getent(1)` to check whether
the username or group in question is existed before doing the heavy
lifting.

However, `getent(1)` will check the database in host system instead of our
rootfs, and by `PATH` manipulation logic, we prefer to use `usr/bin/getent`
inside our rootfs instead of host `getent(1)`.

This is usually not a problem since we mostly run `xbps-triggers` in
a real system instead of running from foreign system.

Except for `base-files` packages, which used to not have group `kvm`
pre-allocated. Thus, requires running this trigger, and lead to all sort
of problems:
- If host system is a musl-based linux system, with gcompat installed,
  and we're bootstrapping a glibc one, `getent(1)` will be executable,
  however, when `getent(1)` attempt to `dlopen(3)` other libraries,
  it'll run into failure.
- If host system doesn't have `kvm` group pre-allocated (bootstrapping
  from foreign distro), we attempt to run `groupadd(1)` on such system,
  thus failing with EPERM.

If we run into one of those cases, `xbps-reconfigure(1)` will stop
configuring `base-files`, not running `base-files`' `INSTALL` and leave
the system in half-baked state, without some requires files and
directories.

Switch to `grep(1)` to check for username and group existence,
since `passwd(5)` and `group(5)` is well-documented.

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

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: github-pr-xbps-triggres-system-account-24754.patch --]
[-- Type: text/x-diff, Size: 4325 bytes --]

From 00584650b6267bd7eaee31c53fe08bc90d7bc380 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C4=90o=C3=A0n=20Tr=E1=BA=A7n=20C=C3=B4ng=20Danh?=
 <congdanhqx@gmail.com>
Date: Tue, 8 Sep 2020 07:12:30 +0700
Subject: [PATCH] xbps-triggers:system-accounts: use grep to check for
 user/group existent

In `system-accounts` triggers, we're using `getent(1)` to check whether
the username or group in question is existed before doing the heavy
lifting.

However, `getent(1)` will check the database in host system instead of our
rootfs, and by `PATH` manipulation logic, we prefer to use `usr/bin/getent`
inside our rootfs instead of host `getent(1)`.

This is usually not a problem since we mostly run `xbps-triggers` in
a real system instead of running from foreign system.

Except for `base-files` packages, which used to not have group `kvm`
pre-allocated. Thus, requires running this trigger, and lead to all sort
of problems:
- If host system is a musl-based linux system, with gcompat installed,
  and we're bootstrapping a glibc one, `getent(1)` will be executable,
  however, when `getent(1)` attempt to `dlopen(3)` other libraries,
  it'll run into failure.
- If host system doesn't have `kvm` group pre-allocated (bootstrapping
  from foreign distro), we attempt to run `groupadd(1)` on such system,
  thus failing with EPERM.

If we run into one of those cases, `xbps-reconfigure(1)` will stop
configuring `base-files`, not running `base-files`' `INSTALL` and leave
the system in half-baked state, without some requires files and
directories.

Switch to `grep(1)` to check for username and group existence,
since `passwd(5)` and `group(5)` is well-documented.
---
 srcpkgs/xbps-triggers/files/system-accounts | 15 ++++++---------
 srcpkgs/xbps-triggers/template              |  3 +--
 2 files changed, 7 insertions(+), 11 deletions(-)

diff --git a/srcpkgs/xbps-triggers/files/system-accounts b/srcpkgs/xbps-triggers/files/system-accounts
index d48e9b7a2cf..f87fd0d9c03 100755
--- a/srcpkgs/xbps-triggers/files/system-accounts
+++ b/srcpkgs/xbps-triggers/files/system-accounts
@@ -26,7 +26,7 @@ group_add() {
 		use_gid="gid ${_gid}"
 	fi
 
-	if ! getent group ${_grname} >/dev/null; then
+	if ! grep -q "^${_grname}:" etc/group >/dev/null; then
 		if [ -n "$use_gid" ]; then
 			groupadd -r ${_grname} -g ${_gid} >/dev/null 2>&1
 		else
@@ -59,9 +59,6 @@ run)
 	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
 		PASSWD=1
 	fi
@@ -70,8 +67,8 @@ 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)"
+			if [ -z "$GROUPADD" ]; then
+				echo "WARNING: cannot create ${grp} system group (missing groupadd)"
 				echo "The following group must be created manually: $grp"
 				continue
 			fi
@@ -96,8 +93,8 @@ run)
 			[ "${_uid}" != "${_uname}" ] &&
 				use_id="-u ${_uid} -g ${pgroup:-${_uid}}"
 
-			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 "   Description: '${descr}'"
@@ -109,7 +106,7 @@ run)
 
 			group_add ${pgroup:-${acct}}
 
-			if ! getent passwd ${_uname} >/dev/null; then
+			if ! grep -q "^${_uname}:" etc/passwd >/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
diff --git a/srcpkgs/xbps-triggers/template b/srcpkgs/xbps-triggers/template
index b28d198b1ce..f6325c5f569 100644
--- a/srcpkgs/xbps-triggers/template
+++ b/srcpkgs/xbps-triggers/template
@@ -1,8 +1,7 @@
 # Template file for 'xbps-triggers'
 pkgname=xbps-triggers
 version=0.116
-revision=1
-archs=noarch
+revision=2
 bootstrap=yes
 short_desc="XBPS triggers for Void Linux"
 maintainer="Enno Boland <gottox@voidlinux.org>"

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: xbps-triggers:system-accounts: use grep to check for user/group existent
  2020-09-08  0:40 [PR PATCH] xbps-triggers:system-accounts: use grep to check for user/group existent sgn
                   ` (2 preceding siblings ...)
  2020-09-09  1:07 ` [PR PATCH] [Updated] " sgn
@ 2020-09-09 14:04 ` sgn
  2020-09-10  0:37 ` ahesford
                   ` (12 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: sgn @ 2020-09-09 14:04 UTC (permalink / raw)
  To: ml

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

New comment by sgn on void-packages repository

https://github.com/void-linux/void-packages/pull/24754#issuecomment-689584906

Comment:
@void-linux/pkg-committers 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: xbps-triggers:system-accounts: use grep to check for user/group existent
  2020-09-08  0:40 [PR PATCH] xbps-triggers:system-accounts: use grep to check for user/group existent sgn
                   ` (3 preceding siblings ...)
  2020-09-09 14:04 ` sgn
@ 2020-09-10  0:37 ` ahesford
  2020-09-10  0:57 ` sgn
                   ` (11 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: ahesford @ 2020-09-10  0:37 UTC (permalink / raw)
  To: ml

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

New comment by ahesford on void-packages repository

https://github.com/void-linux/void-packages/pull/24754#issuecomment-689898960

Comment:
This is probably a fine replacement, depending on how much we care about sanity checks on values on group and user definitions.

As a totally contrived example, suppose somebody puts an invalid value in `$system_groups`, say `wheel:x:123`. This will be split into a group `wheel:x` and a gid `123`. Currently, when `getent group wheel:x` is run, the output will be empty and the return value nonzero because the group does not (and can never) exist. The trigger will try to create the group, but `groupadd` will fail because `wheel:x` is not a valid name, causing the trigger to complain during install.

The `grep` replacement will instead test a match on `wheel:x:` in `etc/group`, which (in this case) should match the existing group definition for `wheel`, meaning the trigger will not report an error. (It will also not report that a group was created, but that's much harder to notice than a failure message.)

In the end, it seems like the system state would be the same either way, because either a valid group is created or already exists; an invalid group fails to match and the trigger tries unsuccessfully to create it; or an invalid group falsely matches and the trigger doesn't try to run a `groupadd` command that would have failed anyway.

If we care about the failure always appearing when a group should be created but isn't, you might have to `cut` the fields of the files or pull `awk` into the picture.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: xbps-triggers:system-accounts: use grep to check for user/group existent
  2020-09-08  0:40 [PR PATCH] xbps-triggers:system-accounts: use grep to check for user/group existent sgn
                   ` (4 preceding siblings ...)
  2020-09-10  0:37 ` ahesford
@ 2020-09-10  0:57 ` sgn
  2020-09-10  1:03 ` [PR PATCH] [Updated] " sgn
                   ` (10 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: sgn @ 2020-09-10  0:57 UTC (permalink / raw)
  To: ml

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

New comment by sgn on void-packages repository

https://github.com/void-linux/void-packages/pull/24754#issuecomment-689905068

Comment:
> As a totally contrived example, suppose somebody puts an invalid value in `$system_groups`, say `wheel:x:123`. This will be split into a group `wheel:x` and a gid `123`.

Minor correction:
The current code will split into:
```sh
_grname=wheel:x
_gid=x:123
```

> Currently, when `getent group wheel:x` is run, the output will be empty and the return value nonzero because the group does not (and can never) exist. The trigger will try to create the group, but `groupadd` will fail because `wheel:x` is not a valid name, causing the trigger to complain during install.
> 
> The `grep` replacement will instead test a match on `wheel:x:` in `etc/group`, which (in this case) should match the existing group definition for `wheel`, meaning the trigger will not report an error. (It will also not report that a group was created, but that's much harder to notice than a failure message.)

I believe we can do the sanity check for argument by:
```sh
case "$1" in
*:*:*)  echo "Invalid group specification" >&2; exit 1;;
esac
```

> 
> In the end, it seems like the system state would be the same either way, because either a valid group is created or already exists; an invalid group fails to match and the trigger tries unsuccessfully to create it; or an invalid group falsely matches and the trigger doesn't try to run a `groupadd` command that would have failed anyway.
> 
> If we care about the failure always appearing when a group should be created but isn't, you might have to `cut` the fields of the files or pull `awk` into the picture.



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PR PATCH] [Updated] xbps-triggers:system-accounts: use grep to check for user/group existent
  2020-09-08  0:40 [PR PATCH] xbps-triggers:system-accounts: use grep to check for user/group existent sgn
                   ` (5 preceding siblings ...)
  2020-09-10  0:57 ` sgn
@ 2020-09-10  1:03 ` sgn
  2020-09-10  1:17 ` ahesford
                   ` (9 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: sgn @ 2020-09-10  1:03 UTC (permalink / raw)
  To: ml

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

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

https://github.com/sgn/void-packages xbps-triggres-system-account
https://github.com/void-linux/void-packages/pull/24754

xbps-triggers:system-accounts: use grep to check for user/group existent
In `system-accounts` triggers, we're using `getent(1)` to check whether
the username or group in question is existed before doing the heavy
lifting.

However, `getent(1)` will check the database in host system instead of our
rootfs, and by `PATH` manipulation logic, we prefer to use `usr/bin/getent`
inside our rootfs instead of host `getent(1)`.

This is usually not a problem since we mostly run `xbps-triggers` in
a real system instead of running from foreign system.

Except for `base-files` packages, which used to not have group `kvm`
pre-allocated. Thus, requires running this trigger, and lead to all sort
of problems:
- If host system is a musl-based linux system, with gcompat installed,
  and we're bootstrapping a glibc one, `getent(1)` will be executable,
  however, when `getent(1)` attempt to `dlopen(3)` other libraries,
  it'll run into failure.
- If host system doesn't have `kvm` group pre-allocated (bootstrapping
  from foreign distro), we attempt to run `groupadd(1)` on such system,
  thus failing with EPERM.

If we run into one of those cases, `xbps-reconfigure(1)` will stop
configuring `base-files`, not running `base-files`' `INSTALL` and leave
the system in half-baked state, without some requires files and
directories.

Switch to `grep(1)` to check for username and group existence,
since `passwd(5)` and `group(5)` is well-documented.

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

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: github-pr-xbps-triggres-system-account-24754.patch --]
[-- Type: text/x-diff, Size: 4947 bytes --]

From 1618b92815be217c50ccda0513df02cefaa143d2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C4=90o=C3=A0n=20Tr=E1=BA=A7n=20C=C3=B4ng=20Danh?=
 <congdanhqx@gmail.com>
Date: Tue, 8 Sep 2020 07:12:30 +0700
Subject: [PATCH] xbps-triggers:system-accounts: use grep to check for
 user/group existent

In `system-accounts` triggers, we're using `getent(1)` to check whether
the username or group in question is existed before doing the heavy
lifting.

However, `getent(1)` will check the database in host system instead of our
rootfs, and by `PATH` manipulation logic, we prefer to use `usr/bin/getent`
inside our rootfs instead of host `getent(1)`.

This is usually not a problem since we mostly run `xbps-triggers` in
a real system instead of running from foreign system.

Except for `base-files` packages, which used to not have group `kvm`
pre-allocated. Thus, requires running this trigger, and lead to all sort
of problems:
- If host system is a musl-based linux system, with gcompat installed,
  and we're bootstrapping a glibc one, `getent(1)` will be executable,
  however, when `getent(1)` attempt to `dlopen(3)` other libraries,
  it'll run into failure.
- If host system doesn't have `kvm` group pre-allocated (bootstrapping
  from foreign distro), we attempt to run `groupadd(1)` on such system,
  thus failing with EPERM.

If we run into one of those cases, `xbps-reconfigure(1)` will stop
configuring `base-files`, not running `base-files`' `INSTALL` and leave
the system in half-baked state, without some requires files and
directories.

Switch to `grep(1)` to check for username and group existence,
since `passwd(5)` and `group(5)` is well-documented.

While we're at it, sanity check for `system_groups` and `system_accounts`
specification.
---
 srcpkgs/xbps-triggers/files/system-accounts | 28 ++++++++++++++-------
 srcpkgs/xbps-triggers/template              |  3 +--
 2 files changed, 20 insertions(+), 11 deletions(-)

diff --git a/srcpkgs/xbps-triggers/files/system-accounts b/srcpkgs/xbps-triggers/files/system-accounts
index d48e9b7a2cf..fc1a563a239 100755
--- a/srcpkgs/xbps-triggers/files/system-accounts
+++ b/srcpkgs/xbps-triggers/files/system-accounts
@@ -19,6 +19,12 @@ export PATH="usr/sbin:usr/bin:/usr/sbin:/usr/bin:/sbin:/bin"
 group_add() {
 	local _grname _gid use_gid
 
+	case "$1" in
+	*:*:*)	echo "Invalid system_groups specification"
+		exit 1
+		;;
+	esac
+
 	_grname="${1%:*}"
 	_gid="${1#*:}"
 
@@ -26,7 +32,7 @@ group_add() {
 		use_gid="gid ${_gid}"
 	fi
 
-	if ! getent group ${_grname} >/dev/null; then
+	if ! grep -q "^${_grname}:" etc/group >/dev/null; then
 		if [ -n "$use_gid" ]; then
 			groupadd -r ${_grname} -g ${_gid} >/dev/null 2>&1
 		else
@@ -59,9 +65,6 @@ run)
 	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
 		PASSWD=1
 	fi
@@ -70,8 +73,8 @@ 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)"
+			if [ -z "$GROUPADD" ]; then
+				echo "WARNING: cannot create ${grp} system group (missing groupadd)"
 				echo "The following group must be created manually: $grp"
 				continue
 			fi
@@ -80,6 +83,13 @@ run)
 
 		# System user/group required by a package.
 		for acct in ${system_accounts}; do
+
+			case "$acct" in
+			*:*:*)	echo "Invalid system_accounts specification"
+				exit 1
+				;;
+			esac
+
 			_uname="${acct%:*}"
 			_uid="${acct#*:}"
 
@@ -96,8 +106,8 @@ run)
 			[ "${_uid}" != "${_uname}" ] &&
 				use_id="-u ${_uid} -g ${pgroup:-${_uid}}"
 
-			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 "   Description: '${descr}'"
@@ -109,7 +119,7 @@ run)
 
 			group_add ${pgroup:-${acct}}
 
-			if ! getent passwd ${_uname} >/dev/null; then
+			if ! grep -q "^${_uname}:" etc/passwd >/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
diff --git a/srcpkgs/xbps-triggers/template b/srcpkgs/xbps-triggers/template
index b28d198b1ce..f6325c5f569 100644
--- a/srcpkgs/xbps-triggers/template
+++ b/srcpkgs/xbps-triggers/template
@@ -1,8 +1,7 @@
 # Template file for 'xbps-triggers'
 pkgname=xbps-triggers
 version=0.116
-revision=1
-archs=noarch
+revision=2
 bootstrap=yes
 short_desc="XBPS triggers for Void Linux"
 maintainer="Enno Boland <gottox@voidlinux.org>"

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: xbps-triggers:system-accounts: use grep to check for user/group existent
  2020-09-08  0:40 [PR PATCH] xbps-triggers:system-accounts: use grep to check for user/group existent sgn
                   ` (6 preceding siblings ...)
  2020-09-10  1:03 ` [PR PATCH] [Updated] " sgn
@ 2020-09-10  1:17 ` ahesford
  2020-09-10  6:54 ` the-maldridge
                   ` (8 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: ahesford @ 2020-09-10  1:17 UTC (permalink / raw)
  To: ml

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

New comment by ahesford on void-packages repository

https://github.com/void-linux/void-packages/pull/24754#issuecomment-689912321

Comment:
Looks good to me

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: xbps-triggers:system-accounts: use grep to check for user/group existent
  2020-09-08  0:40 [PR PATCH] xbps-triggers:system-accounts: use grep to check for user/group existent sgn
                   ` (7 preceding siblings ...)
  2020-09-10  1:17 ` ahesford
@ 2020-09-10  6:54 ` the-maldridge
  2020-09-10  7:04 ` ericonr
                   ` (7 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: the-maldridge @ 2020-09-10  6:54 UTC (permalink / raw)
  To: ml

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

New comment by the-maldridge on void-packages repository

https://github.com/void-linux/void-packages/pull/24754#issuecomment-690029777

Comment:
Just to point out that this can break in really hard to diagnose ways on systems that are configured with NSS.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: xbps-triggers:system-accounts: use grep to check for user/group existent
  2020-09-08  0:40 [PR PATCH] xbps-triggers:system-accounts: use grep to check for user/group existent sgn
                   ` (8 preceding siblings ...)
  2020-09-10  6:54 ` the-maldridge
@ 2020-09-10  7:04 ` ericonr
  2020-09-10  7:09 ` ericonr
                   ` (6 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: ericonr @ 2020-09-10  7:04 UTC (permalink / raw)
  To: ml

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

New comment by ericonr on void-packages repository

https://github.com/void-linux/void-packages/pull/24754#issuecomment-690034825

Comment:
Could we instead make this trigger run only inside a chroot? Add some sort of early failure if `ROOTDIR != /`

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: xbps-triggers:system-accounts: use grep to check for user/group existent
  2020-09-08  0:40 [PR PATCH] xbps-triggers:system-accounts: use grep to check for user/group existent sgn
                   ` (9 preceding siblings ...)
  2020-09-10  7:04 ` ericonr
@ 2020-09-10  7:09 ` ericonr
  2020-09-10 13:06 ` sgn
                   ` (5 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: ericonr @ 2020-09-10  7:09 UTC (permalink / raw)
  To: ml

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

New comment by ericonr on void-packages repository

https://github.com/void-linux/void-packages/pull/24754#issuecomment-690034825

Comment:
Could we instead make this trigger run only inside chroot (for the masterdir case, as well as installing new systems) and in a real system? Add some sort of early failure if `ROOTDIR != /`

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: xbps-triggers:system-accounts: use grep to check for user/group existent
  2020-09-08  0:40 [PR PATCH] xbps-triggers:system-accounts: use grep to check for user/group existent sgn
                   ` (10 preceding siblings ...)
  2020-09-10  7:09 ` ericonr
@ 2020-09-10 13:06 ` sgn
  2020-09-10 13:48 ` ahesford
                   ` (4 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: sgn @ 2020-09-10 13:06 UTC (permalink / raw)
  To: ml

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

New comment by sgn on void-packages repository

https://github.com/void-linux/void-packages/pull/24754#issuecomment-690274101

Comment:
On 2020-09-09 23:54:26-0700, Michael Aldridge <notifications@github.com> wrote:
> Just to point out that this can break in really hard to diagnose
> ways on systems that are configured with NSS.

Hm, I've never used such system.
I'm in a fence, now.

I'm not sure what to do now.
- Use both grep and getent to check
- ignore xbps-trigger's system-accounts outside of chroot

I'm really to prefer the second approach since it's the sanner
approach.


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: xbps-triggers:system-accounts: use grep to check for user/group existent
  2020-09-08  0:40 [PR PATCH] xbps-triggers:system-accounts: use grep to check for user/group existent sgn
                   ` (11 preceding siblings ...)
  2020-09-10 13:06 ` sgn
@ 2020-09-10 13:48 ` ahesford
  2020-09-10 13:53 ` sgn
                   ` (3 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: ahesford @ 2020-09-10 13:48 UTC (permalink / raw)
  To: ml

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

New comment by ahesford on void-packages repository

https://github.com/void-linux/void-packages/pull/24754#issuecomment-690300970

Comment:
The more I look at this, the more I see breakage when populating an alternate root. Won't `useradd`, `usermod` and `groupadd` always act on the system root? These commands provide a `--root` option that seems to redirect their actions in the way we want, but we aren't using that option now. It's also not clear from their man pages how well these commands get along with directory services.

Ignoring `system-accounts` unless `$ROOTDIR = /` like @ericonr suggests is a reasonable way to avoid the worst of these issues. If you go that route, the hook should at least print all of the users and groups that should exist (along with ID numbers, if provided) for the package to function as expected.

If we really want to do this the "right" way, it's not obvious to me that the existence tests add value. For groups, just `groupadd --root ...` and catch the return code to know the difference between a real failure and an attempt to create a group that already exists. For users, just `useradd --root ...` and, if the return code indicates that the username already exists, try `usermod --root ...` instead.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: xbps-triggers:system-accounts: use grep to check for user/group existent
  2020-09-08  0:40 [PR PATCH] xbps-triggers:system-accounts: use grep to check for user/group existent sgn
                   ` (12 preceding siblings ...)
  2020-09-10 13:48 ` ahesford
@ 2020-09-10 13:53 ` sgn
  2020-09-10 13:57 ` ericonr
                   ` (2 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: sgn @ 2020-09-10 13:53 UTC (permalink / raw)
  To: ml

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

New comment by sgn on void-packages repository

https://github.com/void-linux/void-packages/pull/24754#issuecomment-690304351

Comment:
On 2020-09-10 06:48:52-0700, "Andrew J. Hesford" <notifications@github.com> wrote:
> The more I look at this, the more I see breakage when populating an
> alternate root. Won't `useradd`, `usermod` and `groupadd` always act
> on the system root? These commands provide a `--root` option that
> seems to redirect their actions in the way we want, but we aren't
> using that option now. It's also not clear from their man pages how
> well these commands get along with directory services.

Yes, it's broken.

> Ignoring `system-accounts` unless `$ROOTDIR = /` like @ericonr
> suggests is a reasonable way to avoid the worst of these issues. If
> you go that route, the hook should at least print all of the users
> and groups that should exist (along with ID numbers, if provided)
> for the package to function as expected.

Agree.

> If we really want to do this the "right" way, it's not obvious to me
> that the existence tests add value. For groups, just `groupadd
> --root ...` and catch the return code to know the difference between
> a real failure and an attempt to create a group that already exists.
> For users, just `useradd --root ...` and, if the return code
> indicates that the username already exists, try `usermod --root ...`
> instead.

Hm, I think we need `groupadd --prefix` and `useradd --prefix` instead.


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: xbps-triggers:system-accounts: use grep to check for user/group existent
  2020-09-08  0:40 [PR PATCH] xbps-triggers:system-accounts: use grep to check for user/group existent sgn
                   ` (13 preceding siblings ...)
  2020-09-10 13:53 ` sgn
@ 2020-09-10 13:57 ` ericonr
  2020-09-10 14:01 ` sgn
  2020-09-10 14:01 ` [PR PATCH] [Closed]: " sgn
  16 siblings, 0 replies; 18+ messages in thread
From: ericonr @ 2020-09-10 13:57 UTC (permalink / raw)
  To: ml

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

New comment by ericonr on void-packages repository

https://github.com/void-linux/void-packages/pull/24754#issuecomment-690307261

Comment:
Wouldn't all of this still be broken if messing with a device that uses NSS? afaik that would require a proper service and all

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: xbps-triggers:system-accounts: use grep to check for user/group existent
  2020-09-08  0:40 [PR PATCH] xbps-triggers:system-accounts: use grep to check for user/group existent sgn
                   ` (14 preceding siblings ...)
  2020-09-10 13:57 ` ericonr
@ 2020-09-10 14:01 ` sgn
  2020-09-10 14:01 ` [PR PATCH] [Closed]: " sgn
  16 siblings, 0 replies; 18+ messages in thread
From: sgn @ 2020-09-10 14:01 UTC (permalink / raw)
  To: ml

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

New comment by sgn on void-packages repository

https://github.com/void-linux/void-packages/pull/24754#issuecomment-690309880

Comment:
Yeah, this PR is fundamentally broken, closing.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PR PATCH] [Closed]: xbps-triggers:system-accounts: use grep to check for user/group existent
  2020-09-08  0:40 [PR PATCH] xbps-triggers:system-accounts: use grep to check for user/group existent sgn
                   ` (15 preceding siblings ...)
  2020-09-10 14:01 ` sgn
@ 2020-09-10 14:01 ` sgn
  16 siblings, 0 replies; 18+ messages in thread
From: sgn @ 2020-09-10 14:01 UTC (permalink / raw)
  To: ml

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

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

xbps-triggers:system-accounts: use grep to check for user/group existent
https://github.com/void-linux/void-packages/pull/24754

Description:
In `system-accounts` triggers, we're using `getent(1)` to check whether
the username or group in question is existed before doing the heavy
lifting.

However, `getent(1)` will check the database in host system instead of our
rootfs, and by `PATH` manipulation logic, we prefer to use `usr/bin/getent`
inside our rootfs instead of host `getent(1)`.

This is usually not a problem since we mostly run `xbps-triggers` in
a real system instead of running from foreign system.

Except for `base-files` packages, which used to not have group `kvm`
pre-allocated. Thus, requires running this trigger, and lead to all sort
of problems:
- If host system is a musl-based linux system, with gcompat installed,
  and we're bootstrapping a glibc one, `getent(1)` will be executable,
  however, when `getent(1)` attempt to `dlopen(3)` other libraries,
  it'll run into failure.
- If host system doesn't have `kvm` group pre-allocated (bootstrapping
  from foreign distro), we attempt to run `groupadd(1)` on such system,
  thus failing with EPERM.

If we run into one of those cases, `xbps-reconfigure(1)` will stop
configuring `base-files`, not running `base-files`' `INSTALL` and leave
the system in half-baked state, without some requires files and
directories.

Switch to `grep(1)` to check for username and group existence,
since `passwd(5)` and `group(5)` is well-documented.

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2020-09-10 14:01 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-08  0:40 [PR PATCH] xbps-triggers:system-accounts: use grep to check for user/group existent sgn
2020-09-08  1:29 ` ericonr
2020-09-08 12:04 ` sgn
2020-09-09  1:07 ` [PR PATCH] [Updated] " sgn
2020-09-09 14:04 ` sgn
2020-09-10  0:37 ` ahesford
2020-09-10  0:57 ` sgn
2020-09-10  1:03 ` [PR PATCH] [Updated] " sgn
2020-09-10  1:17 ` ahesford
2020-09-10  6:54 ` the-maldridge
2020-09-10  7:04 ` ericonr
2020-09-10  7:09 ` ericonr
2020-09-10 13:06 ` sgn
2020-09-10 13:48 ` ahesford
2020-09-10 13:53 ` sgn
2020-09-10 13:57 ` ericonr
2020-09-10 14:01 ` sgn
2020-09-10 14:01 ` [PR PATCH] [Closed]: " sgn

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).