Iker Pedrosa [Mon, 10 Mar 2025 08:50:56 +0000 (09:50 +0100)]
tests/: extend basic useradd test
The test framework PoC only provided basic checks. I've added additional
functionality to the framework by checking shadow and gshadow entries
and I've extended the basic useradd test to check those too.
Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com> Reviewed-by: Dan Lavu <dlavu@redhat.com>
Iker Pedrosa [Wed, 12 Mar 2025 13:59:38 +0000 (14:59 +0100)]
tests/: improve distribution detection
openSUSE includes comment lines in `/etc/os-release` file and this can
cause some issues during the distribution detection. Ignore those lines
as they don't cause any effect on the system.
Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com> Reviewed-by: Dan Lavu <dlavu@redhat.com>
Iker Pedrosa [Mon, 31 Mar 2025 15:49:22 +0000 (17:49 +0200)]
tests/: implement feature detection
Implement a general function to detect features in shadow host.
Apparently, musl doesn't provide `getent gshadow`, but shadow still needs
it to check for several group attributes. Thus, check whether it exists
in the host, and if it does run it. If not, let's just skip that part of
the test.
Link: <https://gitlab.alpinelinux.org/alpine/aports/-/issues/16979> Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com> Reviewed-by: Dan Lavu <dlavu@redhat.com>
Serge Hallyn [Wed, 26 Mar 2025 13:23:52 +0000 (08:23 -0500)]
Quick fix: define E_PAM_ERR in lib/pam_pass.c
The exit code situation is a hot mess. Do a
git grep "define.*E_SUCCESS"
Each src/*.c is defining its own set of error codes, and
they are frequently conflicting, e.g. more than one use
10.
We should probably have a common set defined in lib/exitcodes.h.
I'm thinking for a first cut, we just move all the definitions
from src/*.c to lib/exitcodes.h, and let the conflicts stand.
If we later want to change some defines to make them unambiguous
across the project, we can do that separately.
Serge Hallyn [Sat, 22 Mar 2025 12:54:27 +0000 (07:54 -0500)]
passwd: document exit code when PAM has errored
closes #1219
When pam returns an error, we were exiting with exit code 10,
which was hardcoded and not documented. Create a name for it,
and document it in the manpage.
Signed-off-by: Serge Hallyn <serge@hallyn.com> Reported-by: Marc Haber <githubvisible@zugschlus.de> Reviewed-by: Alejandro Colomar <alx@kernel.org>
lib/fs/readlink/: readlinknul(): Use ssize_t to simplify
Consistently using a signed type allows us to avoid sign-mismatch
diagnostics, while keeping the code simple. It feels weird to
accept a ssize_t instead of a size_t, but it's a matter of getting
used to it.
Another way to achieve this with a single 'len' variable and no casts
would be to compare against SIZE_MAX, but that's less readable than -1.
Or one could write a SIZE_C() macro a la UINT64_C(), and compare the
size_t against SIZE_C(-1), but that's still suboptimal (regarding
readability) compared to consistently using signed size types.
Michael Vetter [Mon, 10 Feb 2025 16:43:05 +0000 (17:43 +0100)]
doc/: Remove list of distributions
Since c8e8557803f3 (2025-01-21; "ci: add openSUSE Tumbleweed") we also
run openSUSE in CI. Since the set may grow let's not list each of them
in the documentation.
Iker Pedrosa [Fri, 24 Jan 2025 13:19:04 +0000 (14:19 +0100)]
src/: update group audit messages
Auditing has been broken for a long time upstream and Fedora had some
downstream patches that fixed it, upstreaming that content to fix the
problem for everybody.
Iker Pedrosa [Fri, 24 Jan 2025 13:13:27 +0000 (14:13 +0100)]
lib/, src/: update audit messages
Auditing has been broken for a long time upstream and Fedora had some
downstream patches that fixed it, upstreaming that content to fix the
problem for everybody.
The audit of a user is performed through the AUDIT_USER_* macros.
Similarly, the audit of a group is performed through the AUDIT_GRP_*
macros. Part of the audit performed for groups was incorrectly labeled
as a user, and therefore some changes needed to be made to label them
correctly.
configure.ac: be deterministic about passwd location
Statically set PASSWD_PROGRAM depending on exec_prefix, and not by where
the passwd program was at configure time.
Depending on the specific build situation before, this may or may not
change the embedded passwd program path. Also configure.ac sets
exec_prefix=/ for prefix=/usr, so this might be a bit confusing, but
at least deterministic.
Closes: #1224 Signed-off-by: Chris Hofstaedtler <zeha@debian.org>
lib/getdate.y: Ignore time-zone information and use UTC
There is exactly one caller of this function, and it wants a date, not a
time. It is useless to be able to parse local dates, because we
ultimately store a UTC date. To avoid confusion, unconditionally use
UTC. Since this code had important bugs regarding offset, we can safely
assume that no existing users rely on being able to use their local
date (this never worked correctly).
Also, the code parsing time zones is quite bad, for today's standards.
Link: <https://github.com/shadow-maint/shadow/issues/1202>
Link: <https://github.com/shadow-maint/shadow/issues/1209> Reported-by: Chris Hofstaedtler <zeha@debian.org> Reported-by: Tim Parenti <tim@timtimeonline.com> Reported-by: Lee Garrett <lgarrett@rocketjump.eu> Cc: Gus Kenion <https://github.com/kenion> Cc: Michael Vetter <jubalh@iodoru.org> Cc: Paul Eggert <eggert@cs.ucla.edu> Cc: Iker Pedrosa <ipedrosa@redhat.com> Cc: "Serge E. Hallyn" <serge@hallyn.com> Cc: Brian Inglis <Brian.Inglis@SystematicSW.ab.ca> Signed-off-by: Alejandro Colomar <alx@kernel.org>
The dates are stored as UTC, and are stored as a number of days since
Epoch. We don't have enough precision to translate it into local time.
Using local time has caused endless issues in users.
This patch is not enough for fixing this issue completely, since
printing a date without time-zone information means that the date is a
local date, but what we're printing is a UTC date. A future patch
should add time-zone information to the date.
For now, let's revert this change that has caused so many issues.
Fixes: 3f5b4b562682 (2024-08-01; "lib/, src/: Use local time for human-readable dates")
Link: <https://github.com/ansible/ansible/blob/devel/test/integration/targets/user/tasks/test_expires.yml#L2-L20>
Link: <https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1095430>
Link: <https://lists.iana.org/hyperkitty/list/tz@iana.org/message/ENE5IFV3GAH6WK22UJ6YU57D6TQINSP5/>
Link: <https://github.com/shadow-maint/shadow/issues/1202>
Link: <https://github.com/shadow-maint/shadow/issues/1057>
Link: <https://github.com/shadow-maint/shadow/issues/939>
Link: <https://github.com/shadow-maint/shadow/pull/1058>
Link: <https://github.com/shadow-maint/shadow/pull/1059#issuecomment-2309888519>
Link: <https://github.com/shadow-maint/shadow/pull/952>
Link: <https://github.com/shadow-maint/shadow/pull/942> Reported-by: Chris Hofstaedtler <zeha@debian.org> Reported-by: Gus Kenion <https://github.com/kenion> Reported-by: Alejandro Colomar <alx@kernel.org> Reported-by: Michael Vetter <jubalh@iodoru.org> Reported-by: Lee Garrett <lgarrett@rocketjump.eu> Cc: Paul Eggert <eggert@cs.ucla.edu> Cc: Tim Parenti <tim@timtimeonline.com> Cc: "Serge E. Hallyn" <serge@hallyn.com> Cc: Brian Inglis <Brian.Inglis@SystematicSW.ab.ca> Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com> Signed-off-by: Alejandro Colomar <alx@kernel.org>
was considering an empty string as if it were a number.
strisdigit() does not consider an empty string to be numeric.
I think it will not affect the behavior in either case, as they should
sooner or later result in an error somewhere. And it seems (IMO)
surprising to treat empty strings as numeric strings, so let's not do
it.
lib/string/strspn/, lib/, src/: stprspn(), strrspn_(): Split API into function and macro
This provides a safer and more consistent API.
We had the strrspn(3) function as it was for compatibility with Oracle
Solaris, but let's not repeat their mistake. Nevertheless, name our
function strrspn_() with a trailing underscore, to differentiate it from
the one in Solaris, since it's slightly different.
lib/string/strchr/: strrcspn(), stprcspn(): Add function and macro
These APIs are to strrspn(), like strcspn() is to strspn().
They are like strcspn(3), but search from the end of the string.
The function is meant for internal use, and consistency with libc.
The macro is meant for normal use, since it returns a pointer,
which is what algorithms using this need.
pw_auth()'s $4 was always being specified as NULL. Remove the
parameter. Instead, set a local variable to NULL at function entry, and
remove code that never runs (conditional on $4 != NULL).
Reviewed-by: Sam James <sam@gentoo.org> Signed-off-by: Alejandro Colomar <alx@kernel.org>
Todd C. Miller [Fri, 24 Jan 2025 02:11:09 +0000 (19:11 -0700)]
src/vipw.c: Restore the original terminal pgrp after editing
This fixes a problem when the shell is not in monitor mode (job control
enabled) which resulted in the terminal pgrp being set to an invalid
value once vipw exited.
configure.ac, lib/, src/: Use gid_t instead of GETGROUPS_T
Autoconf's NEWS file says
*** AC_FUNC_GETGROUPS and AC_TYPE_GETGROUPS no longer run test programs.
These macros were testing for OS bugs that we believe are at least
twenty years in the past. Most operating systems are now trusted to
provide an accurate prototype for getgroups in unistd.h, and to
implement it as specified in POSIX.