- useradd: check if subid range exists for user

- find_new_[gu]id: Skip over IDs that are reserved for legacy reasons

Resolves: #2179987
Resolves: #2179988

Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
This commit is contained in:
Iker Pedrosa 2023-05-16 13:01:45 +02:00
parent e4272d78c4
commit a7e5e041d5
3 changed files with 263 additions and 1 deletions

View File

@ -0,0 +1,214 @@
From baae5b4a06c905d9f52ed1f922a0d7d0625d11cf Mon Sep 17 00:00:00 2001
From: Martin Kletzander <nert.pinx@gmail.com>
Date: Wed, 1 Feb 2023 15:36:41 +0100
Subject: [PATCH] find_new_[gu]id(): Skip over IDs that are reserved for legacy
reasons
Some programs don't support `(uint16_t) -1` or `(uint32_t) -1` as user
or group IDs. This is because `-1` is used as an error code or as an
unspecified ID, e.g. in `chown(2)` parameters, and in the past, `gid_t`
and `uid_t` have changed width. For legacy reasons, those values have
been kept reserved in programs today (for example systemd does this; see
the documentation in the link below).
This should not be confused with catching overflow in the ID values,
since that is already caught by our ERANGE checks. This is about not
using reserved values that have been reserved for legacy reasons.
Link: <https://systemd.io/UIDS-GIDS/>
Reviewed-by: Alejandro Colomar <alx@kernel.org>
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
---
libmisc/find_new_gid.c | 38 ++++++++++++++++++++++++++++----------
libmisc/find_new_uid.c | 38 ++++++++++++++++++++++++++++----------
2 files changed, 56 insertions(+), 20 deletions(-)
diff --git a/libmisc/find_new_gid.c b/libmisc/find_new_gid.c
index 70ba95a2..da1d8d55 100644
--- a/libmisc/find_new_gid.c
+++ b/libmisc/find_new_gid.c
@@ -98,6 +98,7 @@ static int get_ranges (bool sys_group, gid_t *min_id, gid_t *max_id,
*
* On success, return 0
* If the ID is in use, return EEXIST
+ * If the ID might clash with -1, return EINVAL
* If the ID is outside the range, return ERANGE
* In other cases, return errno from getgrgid()
*/
@@ -111,6 +112,11 @@ static int check_gid (const gid_t gid,
return ERANGE;
}
+ /* Check for compatibility with 16b and 32b gid_t error codes */
+ if (gid == UINT16_MAX || gid == UINT32_MAX) {
+ return EINVAL;
+ }
+
/*
* Check whether we already detected this GID
* using the gr_next() loop
@@ -182,10 +188,10 @@ int find_new_gid (bool sys_group,
* gr_locate_gid() found the GID in an as-yet uncommitted
* entry. We'll proceed below and auto-set a GID.
*/
- } else if (result == EEXIST || result == ERANGE) {
+ } else if (result == EEXIST || result == ERANGE || result == EINVAL) {
/*
* Continue on below. At this time, we won't
- * treat these two cases differently.
+ * treat these three cases differently.
*/
} else {
/*
@@ -296,8 +302,11 @@ int find_new_gid (bool sys_group,
*gid = id;
free (used_gids);
return 0;
- } else if (result == EEXIST) {
- /* This GID is in use, we'll continue to the next */
+ } else if (result == EEXIST || result == EINVAL) {
+ /*
+ * This GID is in use or unusable, we'll
+ * continue to the next.
+ */
} else {
/*
* An unexpected error occurred.
@@ -339,8 +348,11 @@ int find_new_gid (bool sys_group,
*gid = id;
free (used_gids);
return 0;
- } else if (result == EEXIST) {
- /* This GID is in use, we'll continue to the next */
+ } else if (result == EEXIST || result == EINVAL) {
+ /*
+ * This GID is in use or unusable, we'll
+ * continue to the next.
+ */
} else {
/*
* An unexpected error occurred.
@@ -399,8 +411,11 @@ int find_new_gid (bool sys_group,
*gid = id;
free (used_gids);
return 0;
- } else if (result == EEXIST) {
- /* This GID is in use, we'll continue to the next */
+ } else if (result == EEXIST || result == EINVAL) {
+ /*
+ * This GID is in use or unusable, we'll
+ * continue to the next.
+ */
} else {
/*
* An unexpected error occurred.
@@ -442,8 +457,11 @@ int find_new_gid (bool sys_group,
*gid = id;
free (used_gids);
return 0;
- } else if (result == EEXIST) {
- /* This GID is in use, we'll continue to the next */
+ } else if (result == EEXIST || result == EINVAL) {
+ /*
+ * This GID is in use or unusable, we'll
+ * continue to the next.
+ */
} else {
/*
* An unexpected error occurred.
diff --git a/libmisc/find_new_uid.c b/libmisc/find_new_uid.c
index 6b71dfe5..09885236 100644
--- a/libmisc/find_new_uid.c
+++ b/libmisc/find_new_uid.c
@@ -98,6 +98,7 @@ static int get_ranges (bool sys_user, uid_t *min_id, uid_t *max_id,
*
* On success, return 0
* If the ID is in use, return EEXIST
+ * If the ID might clash with -1, return EINVAL
* If the ID is outside the range, return ERANGE
* In other cases, return errno from getpwuid()
*/
@@ -111,6 +112,11 @@ static int check_uid(const uid_t uid,
return ERANGE;
}
+ /* Check for compatibility with 16b and 32b uid_t error codes */
+ if (uid == UINT16_MAX || uid == UINT32_MAX) {
+ return EINVAL;
+ }
+
/*
* Check whether we already detected this UID
* using the pw_next() loop
@@ -182,10 +188,10 @@ int find_new_uid(bool sys_user,
* pw_locate_uid() found the UID in an as-yet uncommitted
* entry. We'll proceed below and auto-set an UID.
*/
- } else if (result == EEXIST || result == ERANGE) {
+ } else if (result == EEXIST || result == ERANGE || result == EINVAL) {
/*
* Continue on below. At this time, we won't
- * treat these two cases differently.
+ * treat these three cases differently.
*/
} else {
/*
@@ -296,8 +302,11 @@ int find_new_uid(bool sys_user,
*uid = id;
free (used_uids);
return 0;
- } else if (result == EEXIST) {
- /* This UID is in use, we'll continue to the next */
+ } else if (result == EEXIST || result == EINVAL) {
+ /*
+ * This GID is in use or unusable, we'll
+ * continue to the next.
+ */
} else {
/*
* An unexpected error occurred.
@@ -339,8 +348,11 @@ int find_new_uid(bool sys_user,
*uid = id;
free (used_uids);
return 0;
- } else if (result == EEXIST) {
- /* This UID is in use, we'll continue to the next */
+ } else if (result == EEXIST || result == EINVAL) {
+ /*
+ * This GID is in use or unusable, we'll
+ * continue to the next.
+ */
} else {
/*
* An unexpected error occurred.
@@ -399,8 +411,11 @@ int find_new_uid(bool sys_user,
*uid = id;
free (used_uids);
return 0;
- } else if (result == EEXIST) {
- /* This UID is in use, we'll continue to the next */
+ } else if (result == EEXIST || result == EINVAL) {
+ /*
+ * This GID is in use or unusable, we'll
+ * continue to the next.
+ */
} else {
/*
* An unexpected error occurred.
@@ -442,8 +457,11 @@ int find_new_uid(bool sys_user,
*uid = id;
free (used_uids);
return 0;
- } else if (result == EEXIST) {
- /* This UID is in use, we'll continue to the next */
+ } else if (result == EEXIST || result == EINVAL) {
+ /*
+ * This GID is in use or unusable, we'll
+ * continue to the next.
+ */
} else {
/*
* An unexpected error occurred.
--
2.40.1

View File

@ -0,0 +1,38 @@
From e0524e813a3bae2891b33a66f35876841c11cee7 Mon Sep 17 00:00:00 2001
From: Iker Pedrosa <ipedrosa@redhat.com>
Date: Mon, 24 Oct 2022 10:46:36 +0200
Subject: [PATCH] useradd: check if subid range exists for user
Check if a user already has a subid range before assigning one.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2012929
Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
---
src/useradd.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/useradd.c b/src/useradd.c
index 7ea0a9c4..e784d602 100644
--- a/src/useradd.c
+++ b/src/useradd.c
@@ -2188,14 +2188,14 @@ static void usr_update (unsigned long subuid_count, unsigned long subgid_count)
fail_exit (E_PW_UPDATE);
}
#ifdef ENABLE_SUBIDS
- if (is_sub_uid &&
+ if (is_sub_uid && !local_sub_uid_assigned(user_name) &&
(sub_uid_add(user_name, sub_uid_start, subuid_count) == 0)) {
fprintf (stderr,
_("%s: failed to prepare the new %s entry\n"),
Prog, sub_uid_dbname ());
fail_exit (E_SUB_UID_UPDATE);
}
- if (is_sub_gid &&
+ if (is_sub_gid && !local_sub_gid_assigned(user_name) &&
(sub_gid_add(user_name, sub_gid_start, subgid_count) == 0)) {
fprintf (stderr,
_("%s: failed to prepare the new %s entry\n"),
--
2.40.1

View File

@ -1,7 +1,7 @@
Summary: Utilities for managing accounts and shadow password files Summary: Utilities for managing accounts and shadow password files
Name: shadow-utils Name: shadow-utils
Version: 4.9 Version: 4.9
Release: 6%{?dist} Release: 7%{?dist}
Epoch: 2 Epoch: 2
License: BSD and GPLv2+ License: BSD and GPLv2+
URL: https://github.com/shadow-maint/shadow URL: https://github.com/shadow-maint/shadow
@ -74,6 +74,10 @@ Patch26: shadow-4.9-subordinateio-compare-owner-ID.patch
# https://github.com/shadow-maint/shadow/commit/0593b330d8413e9694b5d6783bb90974c9b141c5 # https://github.com/shadow-maint/shadow/commit/0593b330d8413e9694b5d6783bb90974c9b141c5
# https://github.com/shadow-maint/shadow/commit/45d674621918664c8736f94f862e86bddf4c3fd4 # https://github.com/shadow-maint/shadow/commit/45d674621918664c8736f94f862e86bddf4c3fd4
Patch27: shadow-4.9-badname-special-characters.patch Patch27: shadow-4.9-badname-special-characters.patch
# https://github.com/shadow-maint/shadow/commit/e0524e813a3bae2891b33a66f35876841c11cee7
Patch28: shadow-4.9-useradd-check-if-subid-range-exists.patch
# https://github.com/shadow-maint/shadow/commit/baae5b4a06c905d9f52ed1f922a0d7d0625d11cf
Patch29: shadow-4.9-skip-over-reserved-ids.patch
### Dependencies ### ### Dependencies ###
Requires: audit-libs >= 1.6.5 Requires: audit-libs >= 1.6.5
@ -162,6 +166,8 @@ Development files for shadow-utils-subid.
%patch25 -p1 -b .useradd-modify-check-ID-range-for-system-users %patch25 -p1 -b .useradd-modify-check-ID-range-for-system-users
%patch26 -p1 -b .subordinateio-compare-owner-ID %patch26 -p1 -b .subordinateio-compare-owner-ID
%patch27 -p1 -b .badname-special-characters %patch27 -p1 -b .badname-special-characters
%patch28 -p1 -b .useradd-check-if-subid-range-exists
%patch29 -p1 -b .skip-over-reserved-ids
iconv -f ISO88591 -t utf-8 doc/HOWTO > doc/HOWTO.utf8 iconv -f ISO88591 -t utf-8 doc/HOWTO > doc/HOWTO.utf8
cp -f doc/HOWTO.utf8 doc/HOWTO cp -f doc/HOWTO.utf8 doc/HOWTO
@ -332,6 +338,10 @@ rm -f $RPM_BUILD_ROOT/%{_libdir}/libsubid.la
%{_libdir}/libsubid.so %{_libdir}/libsubid.so
%changelog %changelog
* Tue May 16 2023 Iker Pedrosa <ipedrosa@redhat.com> - 2:4.9-7
- useradd: check if subid range exists for user. Resolves: #2179987
- find_new_[gu]id: Skip over IDs that are reserved for legacy reasons. Resolves: #2179988
* Wed Sep 28 2022 Iker Pedrosa <ipedrosa@redhat.com> - 2:4.9-6 * Wed Sep 28 2022 Iker Pedrosa <ipedrosa@redhat.com> - 2:4.9-6
- Change "badnames" to "badname" as this is the accepted option name. Resolves: #2076819 - Change "badnames" to "badname" as this is the accepted option name. Resolves: #2076819