Serge Hallyn [Wed, 20 Mar 2024 22:39:46 +0000 (17:39 -0500)]
getdef: avoid spurious error messages about unknown configuration options
def_find can return NULL for unset, not just unknown, config options. So
move the decision of whether to log an error message about an unknown config
option back into def_find, which knows the difference. Only putdef_str()
will pass a char* srcfile to def_find, so only calls from putdef_str will
cause the message, which was the original intent of fa68441bc4be8.
Also, it was checking for >=0 for success, but since that code is for
opening a different tty as stdin, that was bogus. But since it's
guaranteed to be either 0 or -1, this commit doesn't add any code to
make sure it's 0 (i.e., we could say !=0 instead of ==-1). That's more
appropriate for a different commit.
Remove /*ARGSUSED*/ comments. Instead, use appropriate declarators for
main(). ISO C allows using int main(void) if the parameters are going
to be unused.
Also, do some cosmetic changes in the uses of argc and argv, to show
where they are used.
And use *argv[], instead of **argv. Array notation is friendlier, IMO.
The function should never be used; it's always used via its wrapper
macro. To simplify, and reduce chances of confusion: remove the
function, and implement the macro directly in terms of
stpcpy(mempcpy(strnlen())).
Update the documentation, and improve the example, which was rather
confusing.
lib/utmp.c: Use the appropriate autotools macros for struct utmpx
Recently, we started using utmpx instead of utmp, and we updated
<./configure.ac> to do the checks for 'struct utmpx' instead of
'struct utmp'. However, I forgot to update the preprocessor
conditionals accordingly.
lib/utmp.c: Replace UT_LINESIZE by a NITEMS() calculation
A difference between 'struct utmp' and 'struct utmpx' is that
the former uses UT_LINESIZE for the size of its array members,
while the latter doesn't have a standard variable to get its
size. Therefore, we need to get the number of elements in
the array with NITEMS().
lib/, src/, configure.ac: Use utmpx instead of utmp
utmpx is specified by POSIX as an XSI extension. That's more portable
than utmp, which is unavailable for example in musl libc. The manual
page specifies that in Linux (but it probably means in glibc), utmp and
utmpx (and the functions that use them) are identical, so this commit
shouldn't affect glibc systems.
Assume utmpx is always present.
Also, if utmpx is present, POSIX guarantees that some members exist:
If (maxsize == -1), then ((size_t)maxsize == SIZE_MAX). And no size can
ever be >= SIZE_MAX, so it will never return false if sysconf(3) reports
an unlimited user-name size via returning -1. Well, to be pedantic,
that disallows a user-name siz of precisely SIZE_MAX bytes when
sysconf(3) returns -1. However, that's probably a good thing; such a
long user name might trigger Undefined Behavior somewhere else, so be
cautious and disallow it. I hope nobody will be using the entire
address space for a user name.
The commit that introduced that check missed that this code had always
supported unlimited user-name sizes since it was introduced by Iker in 3b7cc053872c ("lib: replace `USER_NAME_MAX_LENGTH` macro"), and 6be85b0bafb5 ("lib/chkname.c: Use tmp variable to avoid a -Wsign-compare
warning") even clarified this in the commit message.
So, while the code in 6a1f45d932c8 ("lib/chkname.c: Support unlimited
user name lengths") wasn't bad per se, the commit message was incorrect.
What that patch did was adding code for handling EINVAL (or any other
errors that a future kernel might add).
To be more pedantically correct, that commit also allowed (under certain
circumstances, user names of SIZE_MAX bytes, but those were originally
allowed (by accident), and only became disallowed in 403a2e3771be
("lib/chkname.c: Take NUL byte into account"). But again, let's
disallow those, just to be cautious.
Link: <https://github.com/shadow-maint/shadow/pull/935>
Link: <https://github.com/shadow-maint/shadow/pull/935#discussion_r1477429492>
See-also: 6be85b0bafb5 ("lib/chkname.c: Use tmp variable to avoid a -Wsign-compare warning") Fixes: 6a1f45d932c8 ("lib/chkname.c: Support unlimited user name lengths") Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com> Cc: Tobias Stoeckmann <tobias@stoeckmann.org> Cc: Serge Hallyn <serge@hallyn.com> Signed-off-by: Alejandro Colomar <alx@kernel.org>
Before 3b7cc053872c ("lib: replace `USER_NAME_MAX_LENGTH` macro"), this
code did use a length. It used a utmp(5) fixed-width buffer, so the
length matches the buffer size (there was no terminating NUL byte).
However, sysconf(_SC_LOGIN_NAME_MAX) returns a buffer size that accounts
for the terminating null byte; see sysconf(3). Thus, the commit that
introduced the call to sysconf(3), should have taken that detail into
account.
403a2e3771be ("lib/chkname.c: Take NUL byte into account"), by Tobias,
caught that bug in <lib/chkname.c>, but missed that the same commit that
introduced that bug, introduced the same bug in two other places.
This fixes all remaining calls to sysconf(_SC_LOGIN_NAME_MAX).
I still observe some suspicious code after this fix:
if (do_rlogin(hostname, username, max_size - 1, term, sizeof(term)))
...
login_prompt(username, max_size - 1);
We're passing size-1 to functions that want a size. But since the fix
to those will be different, let's do that in the following commits.
Days officially roll over at 00:00 UTC, not at 12:00 UTC. I see no
reason to add that half day.
Also, remove the comment. It's likely to get stale.
So, get_date() gets the number of seconds since the Epoch. I wonder how
that thing works, but I'll assume it's something similar to getdate(3)
+ mktime(3). After that, we need to convert seconds since Epoch to days
since Epoch. That should be a simple division, AFAICS, since Epoch is
"1970‐01‐01 00:00:00 +0000 (UTC)". See mktime(3).
Very large values in /etc/shadow could lead to overflows. Make sure
that these calculations are saturated at LONG_MAX. Since entries are
based on days and not seconds since epoch, saturating won't hurt anyone.
src/chage.c: Unify long overflow checks in print_day_as_date()
The conversion from day to seconds can be done in print_date
(renamed to print_day_as_date for clarification). This has the nice
benefit that DAY multiplication and long to time_t conversion are done
at just one place.
The commit we're fixing mentions that it wanted to move 'chpasswd', but
it removed 'ch_g_passwd' from 'pamd_acct_tools_files' and added
'chpasswd' to 'pamd_files'. It seems it removed the wrong thing by
accident.
Fixes: 341d80c2c751 ("Makefile: move chpasswd and newusers to pamd target")
Link: <https://github.com/shadow-maint/shadow/pull/928#discussion_r1487687347>
Link: <https://github.com/shadow-maint/shadow/issues/926#issuecomment-1941324761> Reported-by: Dominique Leuenberger <dleuenberger@suse.com> Reported-by: Michael Vetter <jubalh@iodoru.org> Cc: David Runge <dvzrv@archlinux.org> Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com> Tested-by: Michael Vetter <jubalh@iodoru.org> Reviewed-by: Michael Vetter <jubalh@iodoru.org> Reviewed-by: loqs <https://github.com/loqs> Co-developed-by: Dominique Leuenberger <dleuenberger@suse.com> Signed-off-by: Dominique Leuenberger <dleuenberger@suse.com> Signed-off-by: Alejandro Colomar <alx@kernel.org>
lib/chkname.c: Support unlimited user name lengths
If the system does not have a user name length limit, support it
accordingly. If the system has no _SC_LOGIN_NAME_MAX, use
LOGIN_NAME_MAX constant instead.
These functions are identical to strtoi(3bsd) and strtou(3bsd), except
for one important thing: if both ERANGE and ENOTSUP conditions happen,
the BSD functions report ENOTSUP, which is bogus; our strtoi_() and
strtou_() report ERANGE.
loqs [Fri, 26 Jan 2024 12:41:09 +0000 (12:41 +0000)]
Makefile: move chpasswd and newusers to pamd target
Install pam configs for chpasswd and newusers when using ./configure --with-libpam --disable-account-tools-setuid.
Fixes https://github.com/shadow-maint/shadow/issues/810.
Pablo Saavedra [Tue, 23 Jan 2024 07:33:37 +0000 (08:33 +0100)]
lib/, src/: Make the use of MAYBE_UNUSED macro consistent
There is an inconsistent use of the MAYBE_UNUSED macro. Sometimes the
`int unused(x)` form is used form and others the `unused int x`. We'd
like to use the second form always.
Related-To: https://github.com/shadow-maint/shadow/issues/918 Suggested-by: Alejandro Colomar <alx@kernel.org> Signed-off-by: Pablo Saavedra <psaavedra@igalia.com>
autogen.sh: CFLAGS: Add some -Werror=... flags that will be default soon
Clang 16 and GCC 14 have upgraded several warnings to errors by default.
Also, there are new warnings that will be requirements of ISO C23. Add
all of those to our build.
Use Clang's -Wno-unknown-attribute-option, to ignore warnings that are
exclusive of GCC. Sadly, GCC doesn't have such an option.
Link: <https://wiki.gentoo.org/wiki/Modern_C_porting#What_changed.3F>
Link: <https://github.com/shadow-maint/shadow/issues/922> Suggested-by: Sam James <sam@gentoo.org> Signed-off-by: Alejandro Colomar <alx@kernel.org>
lib/cast.h: const_cast(): Add macro for dropping 'const'
Uses of this macro indicate a code smell, but in some cases, libc
functions require breaking const correctness. Use this macro to wrap
casts in such cases, so that we limit the danger of the cast.
It only permits discarding const. Discarding any other qualifiers, or
doing other type changes should result in a compile-time error.
Fixes: ef95bb7ed139 ("src/su.c: Fix type of variable") Closes: <https://github.com/shadow-maint/shadow/issues/915> Reported-by: Sam James <sam@gentoo.org> Signed-off-by: Alejandro Colomar <alx@kernel.org>
lib/, src/: Replace strtou[l]l(3) by strtou[l]l_noneg()
strtou[l]l(3) silently converts negative numbers into positive. This
behavior is wrong: a negative value should be parsed as a negative
value, which would underflow unsigned (long) long, and so would return
the smallest possible value, 0, and set errno to ERANGE to report an
error.
src/sulogin.c: Remove 'static' from local variable, but keep initialization
We don't need 'static', because it's in main(), which is only called
once. However, we will need initialization as if it were 'static', so
use ={} to initialize it. This will allow freeing the pointers before
they have been allocated.
projects / thirdparty / shadow.git / log
