[Pkg-shadow-devel] [Git][debian/adduser][debian-bug-1099397] 7 commits: refactor existing_*_ok
Marc Haber (@zugschlus)
gitlab at salsa.debian.org
Mon Mar 3 18:06:52 GMT 2025
Marc Haber pushed to branch debian-bug-1099397 at Debian / adduser
Commits:
ee5cee1f by Matt Barry at 2025-03-03T18:52:43+01:00
refactor existing_*_ok
replace with new existing_(user|group)_status, which return a bitmask
value corresponding to these constants:
EXISTING_NOT_FOUND EXISTING_FOUND EXISTING_SYSTEM EXISTING_ID_MISMATCH
(and EXISTING_LOCKED, which is unused in this branch)
- - - - -
81b8eadf by Marc Haber at 2025-03-03T18:52:43+01:00
change logic regarding error messages
Git-Dch: ignore
- - - - -
4b51c786 by Marc Haber at 2025-03-03T18:52:43+01:00
add more debugging
Git-Dch: ignore
- - - - -
e5c4b6ec by Marc Haber at 2025-03-03T19:02:06+01:00
add test to check for bug #1099073
Git-Dch: ignore
- - - - -
eeed860a by Marc Haber at 2025-03-03T19:02:06+01:00
dont issue warnings for --home /nonexistent
Thanks: Sven Joachim
Closes: #1099073
- - - - -
0ae94f1c by Marc Haber at 2025-03-03T19:02:06+01:00
add new test to check two argument adduser with underscores
Git-Dch: ignore
- - - - -
717ac5a2 by Marc Haber at 2025-03-03T19:02:06+01:00
allow adding system accounts to system groups
Closes: #1099397
- - - - -
3 changed files:
- adduser
- + debian/tests/f/addusertogroup_underscore.t
- + debian/tests/f/home-nonexistent-no-create-home.t
Changes:
=====================================
adduser
=====================================
@@ -76,6 +76,14 @@ BEGIN {
}
}
+use constant {
+ EXISTING_NOT_FOUND => 0,
+ EXISTING_FOUND => 1,
+ EXISTING_SYSTEM => 2,
+ EXISTING_ID_MISMATCH => 4,
+ EXISTING_LOCKED => 8,
+};
+
my $yesexpr = langinfo(YESEXPR());
my $charset = langinfo($codeset);
if ($encode_loaded) {
@@ -273,8 +281,8 @@ if (@names == 2) { # must be addusertogroup
exit( RET_INVALID_CALL );
}
$action = "addusertogroup";
- $existing_user = sanitize_name( shift (@names) );
- $existing_group = sanitize_name( shift (@names) );
+ $existing_user = sanitize_string( shift (@names), anynamere );
+ $existing_group = sanitize_string( shift (@names), anynamere );
}
else { # 1 parameter, must be adduser
$new_name = sanitize_name( shift (@names) );
@@ -388,7 +396,7 @@ if ((defined($special_home)) && ($special_home !~ m+^/+ )) {
exit( RET_INVALID_HOME_DIRECTORY );
}
-if (defined($special_home)) {
+if (defined($special_home) && $special_home ne "/nonexistent" ) {
if (!defined($no_create_home) && -d $special_home) {
log_warn( mtx("The home dir %s you specified already exists.\n"), $special_home );
}
@@ -431,23 +439,23 @@ if ($action eq "addsysgroup") {
acquire_lock();
# Check if requested group already exists and we can exit safely
- my $ret = existing_group_ok($new_name, $gid_option);
-
- if ($ret == 3) {
- log_warn( mtx("The group `%s' already exists as a system group. Exiting."), $new_name );
- exit( RET_OK );
- }
+ my $asgret = existing_group_status($new_name, $gid_option);
+ log_trace( "existing_group_status( %s, %s ) returns %s", $new_name, $gid_option, $asgret );
- if ($ret == 1) {
- log_err( mtx("The group `%s' already exists and is not a system group. Exiting."), $new_name );
- exit( RET_WRONG_OBJECT_PROPERTIES );
- }
-
- if ($ret == 2) {
+ if ($asgret & EXISTING_ID_MISMATCH) {
log_err( mtx("The group `%s' already exists, but has a different GID. Exiting."), $new_name );
exit( RET_WRONG_OBJECT_PROPERTIES );
}
-
+ if ($asgret & EXISTING_FOUND) {
+ log_trace( "existing_found" );
+ if ($asgret & (EXISTING_SYSTEM)) {
+ log_info( mtx("The group `%s' already exists as a system group."), $new_name );
+ exit( RET_OK );
+ } else {
+ log_err( mtx("The group `%s' already exists and is not a system group. Exiting."), $new_name );
+ exit( RET_WRONG_OBJECT_PROPERTIES );
+ }
+ }
if (defined($gid_option) && defined(getgrgid($gid_option))) {
log_fatal( mtx("The GID `%s' is already in use."), $gid_option );
exit( RET_ID_IN_USE );
@@ -470,8 +478,8 @@ if ($action eq "addsysgroup") {
log_info( mtx("Adding group `%s' (GID %d) ..."), $new_name, $gid_option);
my $groupadd = which('groupadd');
- $ret = systemcall_useradd($name_check_level, $groupadd, '-g', $gid_option, $new_name);
- if( $ret == RET_INVALID_NAME_FROM_USERADD ) {
+ my $ga_ret = systemcall_useradd($name_check_level, $groupadd, '-g', $gid_option, $new_name);
+ if( $ga_ret == RET_INVALID_NAME_FROM_USERADD ) {
$returnvalue = RET_INVALID_NAME_FROM_USERADD;
}
release_lock(0);
@@ -515,8 +523,8 @@ if ($action eq "addgroup") {
log_info( mtx("Adding group `%s' (GID %d) ..."), $new_name, $gid_option);
my $groupadd = which('groupadd');
- my $ret = systemcall_useradd($name_check_level, $groupadd, '-g', $gid_option, $new_name);
- if( $ret == RET_INVALID_NAME_FROM_USERADD ) {
+ my $ga_ret = systemcall_useradd($name_check_level, $groupadd, '-g', $gid_option, $new_name);
+ if( $ga_ret == RET_INVALID_NAME_FROM_USERADD ) {
$returnvalue = RET_INVALID_NAME_FROM_USERADD;
}
release_lock(0);
@@ -557,21 +565,21 @@ if ($action eq 'addusertogroup') {
################
if ($action eq "addsysuser") {
acquire_lock();
- if (existing_user_ok($new_name, $new_uid) == 1) {
+ my $ret = existing_user_status($new_name, $new_uid);
+ if ($ret == (EXISTING_FOUND|EXISTING_SYSTEM)) {
# a user with this name already exists; it's a problem when it's not a system user
- my $tmp_u = egetpwnam($new_name);
- if (($tmp_u >= $config{"first_system_uid"}) and ($tmp_u <= $config{"last_system_uid"})) {
- log_fatal( mtx("The system user `%s' already exists. Exiting.\n"), $new_name );
- exit( RET_OK );
- }
log_fatal( mtx("The user `%s' already exists, but is not a system user. Exiting."), $new_name );
exit( RET_WRONG_OBJECT_PROPERTIES );
}
- if (existing_user_ok($new_name, $new_uid) == 2) {
+ if ($ret & EXISTING_ID_MISMATCH) {
log_fatal( mtx("The user `%s' already exists with a different UID. Exiting."), $new_name );
exit( RET_WRONG_OBJECT_PROPERTIES );
}
+ if ($ret & EXISTING_FOUND) {
+ log_fatal( mtx("The system user `%s' already exists. Exiting.\n"), $new_name );
+ exit( RET_OK );
+ }
if (!$ingroup_name && !defined($gid_option) && !$make_group_also) {
$gid_option = $nogroup_id;
@@ -633,8 +641,8 @@ if ($action eq "addsysuser") {
log_info( mtx("Adding new group `%s' (GID %d) ..."), $new_name, $gid_option );
$undogroup = $new_name;
my $groupadd = which('groupadd');
- my $ret = systemcall_useradd($name_check_level, $groupadd, '-g', $gid_option, $new_name);
- if( $ret == RET_INVALID_NAME_FROM_USERADD ) {
+ my $ga_ret = systemcall_useradd($name_check_level, $groupadd, '-g', $gid_option, $new_name);
+ if( $ga_ret == RET_INVALID_NAME_FROM_USERADD ) {
$returnvalue = RET_INVALID_NAME_FROM_USERADD;
}
}
@@ -648,7 +656,7 @@ if ($action eq "addsysuser") {
$undouser = $new_name;
my $useradd = which('useradd');
- my $ret = systemcall_useradd($name_check_level,
+ my $ua_ret = systemcall_useradd($name_check_level,
$useradd,
'-r',
'-K', sprintf('SYS_UID_MIN=%d', $new_firstuid || $config{'first_system_uid'}),
@@ -658,7 +666,7 @@ if ($action eq "addsysuser") {
'-s', $shell,
'-u', $new_uid,
$new_name);
- if( $ret == RET_INVALID_NAME_FROM_USERADD ) {
+ if( $ua_ret == RET_INVALID_NAME_FROM_USERADD ) {
$returnvalue = RET_INVALID_NAME_FROM_USERADD;
}
@@ -1111,76 +1119,59 @@ sub mktree {
return 1;
}
-# existing_user_ok: check if there is already a user present
+# existing_user_status: check if there is already a user present
# on the system which satisfies the requirements
# parameter:
# new_name: the name of the user to check
# new_uid : the UID of the user
-# return values:
-# 0 if the the user does not exist
-# 1 if the user already exists with the specified uid
-# (or $new_uid was not specified)
-# 2 if the user already exists, but $new_uid does not match its uid
-sub existing_user_ok {
- my($new_name,$new_uid) = @_;
- log_trace( "existing_user_ok called with new_name %s, new_uid %s", $new_name, $new_uid );
- my ($dummy1,$dummy2,$uid);
- log_trace( "new_name %s", $new_name);
- if (($dummy1,$dummy2,$uid) = egetpwnam($new_name)) {
- if( defined($new_uid) && $uid == $new_uid ) {
- return 1;
- }
- if (! defined($new_uid)) {
- return 1;
- }
- # TODO: do we really need this code? Range check should not be performed here
- # also, we might be checking a regular user as well here
- if( $uid >= $config{"first_system_uid"} &&
- $uid <= $config{"last_system_uid" } ) {
- return 2;
- }
- } else {
- return 0;
- }
+# return value:
+# bitwise combination of these constants:
+# EXISTING_NOT_FOUND => 0
+# EXISTING_FOUND => 1
+# EXISTING_SYSTEM => 2
+# EXISTING_ID_MISMATCH => 4
+# EXISTING_LOCKED => 8
+# e.g. if the requested account name exists as a locked system user,
+# return 8|2|1 == 11
+sub existing_user_status {
+ my ($new_name,$new_uid) = @_;
+ my ($dummy1,$pw,$uid);
+ my $ret = EXISTING_NOT_FOUND;
+ log_trace( "existing_user_status called with new_name %s, new_uid %s", $new_name, $new_uid );
+ if (($dummy1,$pw,$uid) = egetpwnam($new_name)) {
+ $ret |= EXISTING_FOUND;
+ $ret |= EXISTING_ID_MISMATCH if (defined($new_uid) && $uid != $new_uid);
+ $ret |= EXISTING_SYSTEM if \
+ ($uid >= $config{"first_system_uid"} && $uid <= $config{"last_system_uid"});
+ $ret |= EXISTING_LOCKED if (substr($pw,0,1) eq "!"); # TODO: also check expiry?
+ }
+ return $ret;
}
-# existing_group_ok: check if there is already a group which satiesfies the requirements
+# existing_group_status: check if there is already a group which satisfies the requirements
# parameter:
# new_name: the name of the group
# new_gid : the GID of the group
-# return values:
-# 0 if the group does not exist
-# 1 if the group already exists with the specified gid
-# (or $gid_option was not specified)
-# 2 if the group already exists, but $gid_option does not match its gid
-# 3 if the group already exists inside the system range
-sub existing_group_ok {
- my($new_name,$new_gid) = @_;
+# return value:
+# bitwise combination of these constants:
+# EXISTING_NOT_FOUND => 0
+# EXISTING_FOUND => 1
+# EXISTING_SYSTEM => 2
+# EXISTING_ID_MISMATCH => 4
+sub existing_group_status {
+ my ($new_name,$new_gid) = @_;
my ($dummy1,$dummy2,$gid);
+ my $ret = EXISTING_NOT_FOUND;
if (($dummy1,$dummy2,$gid) = egetgrnam($new_name)) {
-
- # TODO: is this check required? There should not be any
- # gid outside of our allowed range anyways ...
- # also, we might be checking a regular user as well here
- if( $gid >= $config{"first_system_gid"} &&
- $gid <= $config{"last_system_gid" } ) {
- return 3;
- }
- if (! defined($new_gid)) {
- return 1;
- }
- if ($gid == $new_gid) {
- return 1;
- } else {
- return 2;
- }
- } else {
- return 0;
- }
+ log_trace("egetgrnam %s returned successfully, $gid = %s", $new_name, $gid);
+ $ret |= EXISTING_FOUND;
+ $ret |= EXISTING_ID_MISMATCH if (defined($new_gid) && $gid != $new_gid);
+ $ret |= EXISTING_SYSTEM if \
+ ($gid >= $config{"first_system_gid"} && $gid <= $config{"last_system_gid"});
+ }
+ return $ret;
}
-
-
# check_user_group: ???
# parameters:
# system: 0 if the user is not a system user, 1 otherwise
@@ -1190,7 +1181,7 @@ sub existing_group_ok {
sub check_user_group {
my ($system) = @_;
log_debug( "check_user_group %s called, make_group_also %s", $system, $make_group_also );
- if( !$system || !existing_user_ok($new_name, $new_uid) ) {
+ if( !$system || !existing_user_status($new_name, $new_uid) ) {
if( defined egetpwnam($new_name) ) {
if( $system ) {
log_fatal( mtx("The user `%s' already exists, and is not a system user."), $new_name);
@@ -1207,7 +1198,7 @@ sub check_user_group {
}
if ($make_group_also) {
log_trace( "make_group_also 1, new_name %s, new_uid %s", $new_name, $new_uid );
- if( !$system || !existing_group_ok($new_name, $new_uid) ) {
+ if( !$system || !existing_group_status($new_name, $new_uid) ) {
if (defined egetgrnam($new_name)) {
log_fatal( mtx("The group `%s' already exists."),$new_name );
exit( RET_OBJECT_EXISTS );
@@ -1230,7 +1221,6 @@ sub check_user_group {
log_debug( "return from check_user_group" );
}
-
# copy_to_dir :
# parameters:
# fromdir
@@ -1276,10 +1266,9 @@ sub copy_to_dir {
log_err( "open >%s/%s: %s", $todir, $file, $!);
&cleanup();
}
-
if( !print $newfilefh (<$filefh>) ) {
- log_err( "print %s/%s: %s", $todir, $file, $!);
- &cleanup();
+ log_err( "print %s/%s: %s", $todir, $file, $!);
+ &cleanup();
}
close($filefh);
if( !close($newfilefh) ) {
=====================================
debian/tests/f/addusertogroup_underscore.t
=====================================
@@ -0,0 +1,118 @@
+#! /usr/bin/perl -Idebian/tests/lib
+
+# check adduser username to group
+# Debian Bug #1099397
+
+use diagnostics;
+use strict;
+use warnings;
+
+use AdduserTestsCommon;
+
+# create user and group
+my $test_user="u1099397";
+my $test_sysuser="_u1099397";
+my $test_group="g1099397";
+my $test_sysgroup="_g1099397";
+my $nonexistent="/nonexistent";
+assert_user_does_not_exist($test_user);
+assert_user_does_not_exist($test_sysuser);
+assert_group_does_not_exist($test_group);
+assert_group_does_not_exist($test_sysgroup);
+assert_command_success('/usr/sbin/adduser',
+ '--stdoutmsglevel=warn', '--stderrmsglevel=warn',
+ '--home', '/nonexistent',
+ '--ingroup', 'nogroup',
+ '--disabled-password',
+ '--comment', '',
+ $test_user);
+assert_user_exists($test_user);
+assert_command_success('/usr/sbin/adduser',
+ '--stdoutmsglevel=warn', '--stderrmsglevel=warn',
+ '--system',
+ $test_sysuser);
+assert_user_exists($test_sysuser);
+assert_command_success('/usr/sbin/addgroup',
+ '--stdoutmsglevel=warn', '--stderrmsglevel=warn',
+ $test_group);
+assert_group_exists($test_group);
+assert_command_success('/usr/sbin/addgroup',
+ '--stdoutmsglevel=warn', '--stderrmsglevel=warn',
+ '--system',
+ $test_sysgroup);
+assert_group_exists($test_sysgroup);
+
+assert_group_membership_does_not_exist($test_user, $test_group);
+assert_group_membership_does_not_exist($test_user, $test_sysgroup);
+assert_group_membership_does_not_exist($test_sysuser, $test_group);
+assert_group_membership_does_not_exist($test_sysuser, $test_sysgroup);
+
+assert_command_success('/usr/sbin/adduser',
+ '--stdoutmsglevel=error', '--stderrmsglevel=error',
+ $test_user,
+ $test_group);
+assert_group_membership_exists($test_user, $test_group);
+assert_group_membership_does_not_exist($test_user, $test_sysgroup);
+assert_group_membership_does_not_exist($test_sysuser, $test_group);
+assert_group_membership_does_not_exist($test_sysuser, $test_sysgroup);
+
+
+assert_command_success('/usr/sbin/adduser',
+ '--stdoutmsglevel=error', '--stderrmsglevel=error',
+ $test_user,
+ $test_sysgroup);
+assert_group_membership_exists($test_user, $test_group);
+assert_group_membership_exists($test_user, $test_sysgroup);
+assert_group_membership_does_not_exist($test_sysuser, $test_group);
+assert_group_membership_does_not_exist($test_sysuser, $test_sysgroup);
+
+assert_command_success('/usr/sbin/adduser',
+ '--stdoutmsglevel=error', '--stderrmsglevel=error',
+ $test_sysuser,
+ $test_group);
+assert_group_membership_exists($test_user, $test_group);
+assert_group_membership_exists($test_user, $test_sysgroup);
+assert_group_membership_exists($test_sysuser, $test_group);
+assert_group_membership_does_not_exist($test_sysuser, $test_sysgroup);
+
+assert_command_success('/usr/sbin/adduser',
+ '--stdoutmsglevel=error', '--stderrmsglevel=error',
+ $test_sysuser,
+ $test_sysgroup);
+assert_group_membership_exists($test_user, $test_group);
+assert_group_membership_exists($test_user, $test_sysgroup);
+assert_group_membership_exists($test_sysuser, $test_group);
+assert_group_membership_exists($test_sysuser, $test_sysgroup);
+
+assert_command_success('/usr/sbin/delgroup',
+ '--stdoutmsglevel=warn', '--stderrmsglevel=warn',
+ $test_group);
+assert_group_does_not_exist($test_group);
+assert_group_membership_does_not_exist($test_user, $test_group);
+assert_group_membership_exists($test_user, $test_sysgroup);
+assert_group_membership_does_not_exist($test_sysuser, $test_group);
+assert_group_membership_exists($test_sysuser, $test_sysgroup);
+
+
+assert_command_success('/usr/sbin/delgroup',
+ '--stdoutmsglevel=warn', '--stderrmsglevel=warn',
+ $test_sysgroup);
+assert_group_does_not_exist($test_sysgroup);
+assert_group_membership_does_not_exist($test_user, $test_group);
+assert_group_membership_does_not_exist($test_user, $test_sysgroup);
+assert_group_membership_does_not_exist($test_sysuser, $test_group);
+assert_group_membership_does_not_exist($test_sysuser, $test_sysgroup);
+
+assert_command_success('/usr/sbin/deluser',
+ '--stdoutmsglevel=warn', '--stderrmsglevel=warn',
+ $test_user);
+assert_user_does_not_exist($test_user);
+
+assert_command_success('/usr/sbin/deluser',
+ '--stdoutmsglevel=warn', '--stderrmsglevel=warn',
+ $test_sysuser);
+assert_user_does_not_exist($test_sysuser);
+
+
+# end of test
+# vim: tabstop=4 shiftwidth=4 expandtab
=====================================
debian/tests/f/home-nonexistent-no-create-home.t
=====================================
@@ -0,0 +1,42 @@
+#! /usr/bin/perl -Idebian/tests/lib
+
+# check adduser with --home /nonexistent and explicit --no-create-home
+# Debian Bug #1099073
+
+use diagnostics;
+use strict;
+use warnings;
+
+use AdduserTestsCommon;
+
+# create user and group
+my $test_user="u1099073";
+my $test_home="/nonexistent";
+assert_user_does_not_exist($test_user);
+assert_command_success('/usr/sbin/adduser',
+ '--stdoutmsglevel=warn', '--stderrmsglevel=warn',
+ '--home', '/nonexistent',
+ '--system',
+ $test_user);
+assert_user_exists($test_user);
+assert_user_has_home_directory($test_user, $test_home);
+assert_command_success('/usr/sbin/deluser',
+ '--stdoutmsglevel=error', '--stderrmsglevel=error',
+ $test_user);
+
+assert_command_success('/usr/sbin/adduser',
+ '--stdoutmsglevel=warn', '--stderrmsglevel=warn',
+ '--home', '/nonexistent',
+ '--no-create-home',
+ '--system',
+ $test_user);
+assert_user_exists($test_user);
+assert_user_has_home_directory($test_user, $test_home);
+assert_command_success('/usr/sbin/deluser',
+ '--stdoutmsglevel=error', '--stderrmsglevel=error',
+ $test_user);
+assert_user_does_not_exist($test_user);
+
+
+# end of test
+# vim: tabstop=4 shiftwidth=4 expandtab
View it on GitLab: https://salsa.debian.org/debian/adduser/-/compare/bcce8e7be63df6faabfcb235994b8469506e53a4...717ac5a232876ecf0e3c34dcb0eed3989fe1ffd8
--
View it on GitLab: https://salsa.debian.org/debian/adduser/-/compare/bcce8e7be63df6faabfcb235994b8469506e53a4...717ac5a232876ecf0e3c34dcb0eed3989fe1ffd8
You're receiving this email because of your account on salsa.debian.org.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://alioth-lists.debian.net/pipermail/pkg-shadow-devel/attachments/20250303/5f6cef95/attachment-0001.htm>
More information about the Pkg-shadow-devel
mailing list