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.
Since 'list' is used for a comma/colon-separated-value list, grouplist
is incorrect and inconsistent. grouplist is not a list, but an array.
Use the more common convention of just using plural. Also, use 'gids'
to distinguish it from other group representations.
We can calculate an upper bound of the number of added groups by
counting the number of delimiters in the string (plus one for the
element after the last delimiter). This avoids reallocating +1 in a
loop.
lib/, src/: Replace redundant checks by actual error handling
setgroups(2) already performs a test to check if the number of groups is
too large. Don't do that ourselves, and also don't do it for every
iteration. Just let setgroups(2) do it once.
Instead of our check, let's report errors from setgroups(2).
Call it regardless of having added any groups. If the group list is the
same that getgroups(3) gave us, setgroups(3) will be a no-op, and it
simplifies the surrounding code, by removing the 'added' variable, and
allowing to call lsearch(3) instead of lfind(3).
getgroups(0, NULL) returns the number of groups, so that we can allocate
at once. This might fail if there's a race and the number of users
grows while we're allocating, but if that happens, failing is probably a
good thing to do.
There was some comment saying it doesn't work on some systems, but
according to gnulib, that's only NeXTstep 3.2, which we don't support.
This originates from a typo in this project, which was later copied by
glibc, and so the typo was set in stone. The typo was eventually fixed
in shadow, but glibc had already set the name in stone, so we should
just learn to live with it.
$ grep -rn -C3 sg_name ChangeLog
1607-
1608-2011-07-30 Nicolas François <nicolas.francois@centraliens.net>
1609-
1610: * src/chgpasswd.c: Fix typo sp -> sg. sg_namp -> sg_name
1611- * src/chgpasswd.c: Always update the group file when SHADOWGRP is
1612- not enabled.
1613-
This is a scripted change:
$ find lib* src -type f \
| xargs sed -i 's/\<sg_name\>/sg_namp/g';
If we are not interested in the amount of errors but only if errors
exist, use a flag instead of a counter. This eliminates the chance of
signed integer overflows and better reflects the meaning of variable.
Keeping variable name and basically copied from src/faillog.c.
projects / thirdparty / shadow.git / log
