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.
If crypt fails, pw_encrypt calls exit. This has the consequence that the
plaintext password is not cleared.
A valid password can fail if the underlying library does not support it.
One such example is SHA512, for which the password must not be longer
than 256 characters on musl. A password longer than this with glibc
works, so it is actually possible that a user, running passwd, tries to
enter the old password but the musl-based passwd binary simply exits.
Let passwd clear the password before exiting.
src/login_nopam.c: list_match(): Use iteration instead of recursion
The recursive nature of list_match() triggered regression during
refactoring. In Linux-PAM, the same code exists which could lead to
stack overflow because <access.conf> could be arbitrarily long.
Use an iterative approach for easier refactoring, to support long
lines in the future and to stay in sync with Linux-PAM.
If passwd is called with -P, then PAM handling is disabled
(src/passwd.c line 749). The manual page claims that host files would
be used, which is not true.
The PAM support was only enabled with configure option
--enable-account-tools-setuid. The other account tools would use PAM
then to verify that the user is granted elevated permissions for
actions which normally only root can do.
In chage, however, any non-root user who does not specify the -l
command line option is denied access in check_perms. The check for
being root or not is done with getuid, so non-root users cannot
change user account's aging information in any possible way since
more than 18 years by now.
It's safe to say that nobody misses this non-existing feature. Biggest
benefit is to get chage out of the ACCT_TOOLS_SETUID group of tools.
The nusers variable could, in theory, overflow and trigger an out of
boundary access if a huge amount of entries is added. Realistically,
this is not possible with current systems because way too much data
would be involved.
But let's better be safe than sorry and use correct data types.
Huge files could trigger signed integer overflows if enough lines are
within the file. Use intmax_t which is at least 64 bit to move this
event far into the future.
Support for /etc/suauth only exists if su is installed without
PAM support. If su is not installed (--without-su) or if PAM
support is enabled (default), do not install suauth.5 manual
page.
The SU_ACCESS preprocessor definition is used to decide if
feature exists or not. See links for more details.
Serge Hallyn [Sat, 11 Jan 2025 21:35:01 +0000 (15:35 -0600)]
add and use a login.defs.test with CREATE_HOME set
I suspect this is not a big deal, and most distributions just ship their own
version verbatim like debian/login.defs. But if there is a distro - or even a
person - using this as is from upstream, then we dont' want to break them. So
let's undo this and use an etc/login.defs.test for the testing if needed.
Changelog: 01/13: move etc/login.defs.test to tests/system/etc/login.defs per
suggestion.
Iker Pedrosa [Fri, 22 Nov 2024 09:28:48 +0000 (10:28 +0100)]
etc/login.defs: enable CREATE_HOME
In order to have consistent behaviour among all distributions, the same
configuration needs to be shared. That is why we are going to use the
`etc/login.defs` file and enable CREATE_HOME so that the home dir is
created automatically. This is not the default configuration used in all
distributions, but it is the most common one.
Iker Pedrosa [Wed, 20 Nov 2024 09:41:10 +0000 (10:41 +0100)]
tests: basic group deletion
This is the transformation to Python of the test located in
`tests/grouptools/groupdel/01_groupdel_delete_group/groupdel.test`,
which checks that `groupdel` is able to delete a group.