On 2025-03-05 12:04:04 +0100, Marc Haber wrote:
> On Wed, Mar 05, 2025 at 09:46:53AM +0100, Marc Haber wrote:
> > this is a discussion with Vincent Lefevre on #1099470:
> > >1. For a system account, there would still be an issue if the account↲
> > >has a password (if possible). If EXISTING_ID_MISMATCH is set, this↲
> > >would also yield an issue; I think that this is possible if the --uid↲
> > >adduser option is used, with an id different from the current one for↲
> > >this account. You should add a test for this case.↲
> > >↲
> > >2. For a non-system account, the problematic test would always be↲
> > >false, while it is actually meant to be true (the error message↲
> > >contains "but is not a system user"). So you should add a test↲
> > >↲
> > > adduser --system username↲
> > >↲
> > >where the username account already exists and is not a system account↲
> > >(not sure whether other options, such as --disabled-password, may be↲
> > >needed). One would expect a non-zero exit status because --system is↲
> > >used while username exists and is not a system account.↲
Clarification: by "system account" and "non-system account" above,
I meant: the account that already exists. What will follow is a
"adduser --system username" (possibly with other options).
Note also that actually, there is no issue with EXISTING_ID_MISMATCH
for case (1).
> So that would bascially mean:
>
> (1)
> adduser foo
> adduser --system (must fail, 'already exists as a non-system user')
adduser foo
adduser --system foo
That would actually be case (2). This must fail.
Explanations of the issue on this case with adduser 3.143:
One has
use constant {
EXISTING_NOT_FOUND => 0,
EXISTING_FOUND => 1,
EXISTING_SYSTEM => 2,
EXISTING_ID_MISMATCH => 4,
EXISTING_LOCKED => 8,
};
(these are flags with power-of-two values, thus can be OR'ed) and
the buggy adduser 3.143 has
if ($ret == (EXISTING_FOUND|EXISTING_SYSTEM)) {
# a user with this name already exists; it's a problem when it's not a s
ystem user
log_fatal( mtx("The user `%s' already exists, but is not a system user.
Exiting."), $new_name );
exit( RET_WRONG_OBJECT_PROPERTIES );
}
i.e. it tests whether *only* EXISTING_FOUND and EXISTING_SYSTEM are
set.
If the existing "foo" is a non-system account (as tested above), this
does not fail (while it would be expected to fail) for 2 reasons:
first, because EXISTING_SYSTEM is unset (since the existing account
is *not* a system account) while the test incorrectly requires that
it is set (instead, it should require that it is unset); second,
because EXISTING_ID_MISMATCH is necessarily set (non-system and
system accounts necessarily have 2 different ids), thus the "=="
cannot be satisfied.
You should add a test for case (1), i.e. when the existing system
account has a password:
adduser --system foo
passwd foo
[add a password]
adduser --system foo
or for a script, I suppose that this should work (the passwd(1)
man page says that the -s / --stdin option "is used to indicate
that passwd should read the new password from standard input,
which can be a pipe"):
adduser --system foo
echo mypassword | passwd -s foo
adduser --system foo
This one must not fail.
> adduser --uid 100 foo
> adduser --uid 101 foo (must fail, 'cannot fulfill uid requirement,
> already exists with othe uid')
>
> (we would need to use --disabled-password for all non-system user
> creation since adduser cannot non-interactively create an account with a
> password).
The above is for system accounts (this is handled correctly, even in
adduser 3.143, with the "$ret & EXISTING_ID_MISMATCH" test).
You should add a similar test for non-system accounts, since this
involves a different part of the code. Something like:
adduser --disabled-password --uid 2000 foo
adduser --disabled-password --uid 2001 foo
Regards,
--
Vincent Lefèvre <vinc...@vinc17.net> - Web: <https://www.vinc17.net/>
100% accessible validated (X)HTML - Blog: <https://www.vinc17.net/blog/>
Work: CR INRIA - computer arithmetic / Pascaline project (LIP, ENS-Lyon)
