Commit Graph

3358 Commits

Author SHA1 Message Date
Samanta Navarro
f078412398 src/sulogin.c: Simplify password handling
The password returned by agetpass can be used directly without copying
it into a char array first.

Reviewed-by: Alejandro Colomar <alx@kernel.org>
Signed-off-by: Samanta Navarro <ferivoz@riseup.net>
2024-02-01 14:37:00 +01:00
Samanta Navarro
cb42ee620e src/sulogin.c: Use a do-while loop
Clarify how this endless while(true) loop can be stopped by using a
boolean variable as condition and turn it into a do-while loop.

Suggested-by: Alejandro Colomar <alx@kernel.org>
Reviewed-by: Alejandro Colomar <alx@kernel.org>
Signed-off-by: Samanta Navarro <ferivoz@riseup.net>
2024-02-01 14:37:00 +01:00
Alejandro Colomar
f98e43ee11 tests/unit/test_atoi_strtoi.c: Test strtou_noneg()
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-01-31 22:26:19 -06:00
Alejandro Colomar
f2b240595b lib/atoi/strtou_noneg.[ch]: Add strtou_noneg()
It's like strtou_(), but rejects negative input, instead of silently
converting it to unsigned.

Link: <https://softwareengineering.stackexchange.com/a/449060/332848>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-01-31 22:26:19 -06:00
Alejandro Colomar
f632515581 tests/unit/Makefile.am: tfix
Fix typo.

Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-01-31 22:26:19 -06:00
Alejandro Colomar
40355150c4 tests/unit/test_atoi_strtoi.c: Test strtoi_()
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-01-31 22:26:19 -06:00
Alejandro Colomar
34ff8edb63 lib/atoi/strtoi.[ch]: strtoi_(), strtou_(): Add functions
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.

Link: <https://lists.sr.ht/~hallyn/shadow/%3CZZoQDms6Sv6e5SPE%40debian%3E>
Link: <https://gnats.netbsd.org/cgi-bin/query-pr-single.pl?number=57828>
Cc: Thorsten Glaser <tg@mirbsd.de>
Cc: christos <christos@netbsd.org>
Cc: roy <roy@netbsd.org>
Cc: Guillem Jover <guillem@hadrons.org>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-01-31 22:26:19 -06:00
Tomas Halman
49001ca846 src/passwd.c: implement reading password from pipe
New option --stdin/-t is available for root user. It is useful
for automation/setup and it makes shadow utils passwd more versatile.

Signed-off-by: Tomas Halman <tomas@halman.net>
2024-01-31 22:16:02 -06:00
Tomas Halman
3fff9d7621 lib/agetpass.[ch]: add function ro read from pipe
Add alternative function to agetpass for reading password
from stdin or pipe.

Signed-off-by: Tomas Halman <tomas@halman.net>
2024-01-31 22:16:02 -06:00
loqs
341d80c2c7 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.

Tested-by: David Runge <dvzrv@archlinux.org>
2024-01-30 22:10:32 +01:00
Pablo Saavedra
cd9b4de327 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>
2024-01-30 16:19:56 +01:00
Pablo Saavedra
5d5d212764 lib/, src/: Rename 'unused' macro as 'MAYBE_UNUSED'
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>
2024-01-30 16:19:56 +01:00
Pablo Saavedra
da84d0ede7 Fix Build error 'parameter name omitted' in logind
Fixes #918 by adding the omitted parameter name in
active_sessions_count().

Signed-off-by: Pablo Saavedra <psaavedra@igalia.com>
2024-01-30 16:19:56 +01:00
Alejandro Colomar
1a377e318f src/sulogin.c: pw_entry(): Report errors by returning -1
Cc: Samanta Navarro <ferivoz@riseup.net>
Reviewed-by: Serge Hallyn <serge@hallyn.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-01-29 17:29:59 +01:00
Alejandro Colomar
6fb7fe11f2 src/passwd.c: Remove comments about flags that don't exist
Those flags have never existed, AFAICS.

Closes: <https://github.com/shadow-maint/shadow/issues/929>
Cc: Tomas Halman <tomas@halman.net>
Cc: "Serge E. Hallyn" <serge@hallyn.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-01-29 08:10:44 -06:00
Sam James
0f4e59fd00 Link correctly with libdl
This fixes build with glibc-2.33 (newer glibc merged libdl and libpthread
into libc):
```
libtool: link: x86_64-pc-linux-gnu-gcc -isystem /usr/include/bsd -DLIBBSD_OVERLAY -O2 -pipe -Wl,-O1 -o login login.o login_nopam.o  -Wl,--as-needed ../lib/.libs/libshadow.a -lcrypt -lsystemd -lpam -lpam_misc -lbsd
/usr/lib/gcc/x86_64-pc-linux-gnu/13/../../../../x86_64-pc-linux-gnu/bin/ld: ../lib/.libs/libshadow.a(libshadow_la-nss.o): undefined reference to symbol 'dlclose@@GLIBC_2.2.5'
/usr/lib/gcc/x86_64-pc-linux-gnu/13/../../../../x86_64-pc-linux-gnu/bin/ld: /lib64/libdl.so.2: error adding symbols: DSO missing from command line
collect2: error: ld returned 1 exit status
```

In Debian, the needed macro from libtool seems to be in libltdl-dev.

Signed-off-by: Sam James <sam@gentoo.org>
2024-01-26 10:05:40 +01:00
Alejandro Colomar
6fcc0f6756 autogen.sh: CFLAGS: Use -Wno-unknown-attributes; Clang doesn't know [[gnu::access()]]
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-01-26 09:40:10 +01:00
Alejandro Colomar
d74ffd3c29 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>
2024-01-26 09:40:10 +01:00
Alejandro Colomar
d452d1b812 src/usermod.c: grp_update(): Remove scope of variable, and fix const correctness
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-01-26 09:40:10 +01:00
Alejandro Colomar
5a5cd85bd2 src/useradd.c: get_defaults(): Use const temporary pointer to fix const correctness
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-01-26 09:40:10 +01:00
Alejandro Colomar
3e0cdc87b7 src/su.c: Use const_cast() to silence -Wincompatible-pointer-types-discards-qualifiers
argv is passed to execve(3), which for historic reasons is non-const,
but doesn't modify the strings.

Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-01-26 09:40:10 +01:00
Alejandro Colomar
e9fc8fc7ef 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.

Link: <https://software.codidact.com/posts/286575/287345#answer-287345>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-01-26 09:40:10 +01:00
Alejandro Colomar
4ef08548cc lib/must_be.h: is_same_type(): Add macro
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-01-26 09:40:10 +01:00
Alejandro Colomar
9c5e433a3a lib/must_be.h: is_same_typeof(): Rename macro
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-01-26 09:40:10 +01:00
Alejandro Colomar
9340efbb0d src/su.c: do_check_perms(): Fix -Wincompatible-pointer-types bug
Fixes: ef95bb7ed1 ("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>
2024-01-24 14:49:56 +01:00
Alejandro Colomar
0138819b2a tests/unit/test_atoi_strtou_noneg.c: Test strtou[l]l_noneg()
Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-01-22 17:17:15 -06:00
Alejandro Colomar
f14670ee1a 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.

Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com>
Cc: "Serge E. Hallyn" <serge@hallyn.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-01-22 17:17:15 -06:00
Alejandro Colomar
4a2646f676 lib/atoi/strtou_noneg.[ch]: Add strtou[l]l_noneg()
These functions reject negative numbers, instead of silently converting
them into unsigned, which strtou[l]l(3) do.

Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com>
Cc: "Serge E. Hallyn" <serge@hallyn.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-01-22 17:17:15 -06:00
Samanta Navarro
4d835c7ea4 src/sulogin.c: Free previously allocated memory
The sulogin program calls pw_entry in a loop while incorrect root
passwords are entered.

Free the previously allocated memory to avoid memory exhaustion.

Co-developed-by: Alejandro Colomar <alx@kernel.org>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
Signed-off-by: Samanta Navarro <ferivoz@riseup.net>
2024-01-22 15:40:39 -06:00
Alejandro Colomar
08ae7af111 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.

Cc: Samanta Navarro <ferivoz@riseup.net>
Suggested-by: Serge Hallyn <serge@hallyn.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-01-17 18:11:19 -06:00
Alejandro Colomar
4edda5d8ba src/sulogin.c: Remove 'static' from a temporary variable
There's no need to keep 'pass' in .bss:

$ grep -nC3 '\<pass\>' src/sulogin.c
58-/*ARGSUSED*/ int main (int argc, char **argv)
59-{
60-	int     err = 0;
61:	char    pass[BUFSIZ];
62-	char    **envp = environ;
63-	TERMIO  termio;
64-#ifndef USE_PAM
--
166-#endif
167-			exit (0);
168-		}
169:		STRTCPY(pass, cp);
170-		erase_pass (cp);
171-
172:		if (valid (pass, &pwent)) {	/* check encrypted passwords ... */
173-			break;	/* ... encrypted passwords matched */
174-		}
175-
176-		sleep (2);
177-		(void) puts (_("Login incorrect"));
178-	}
179:	MEMZERO(pass);
180-	(void) alarm (0);
181-	(void) signal (SIGALRM, SIG_DFL);
182-	environ = newenvp;	/* make new environment active */

Cc: Samanta Navarro <ferivoz@riseup.net>
Cc: Serge Hallyn <serge@hallyn.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-01-17 18:11:19 -06:00
Alejandro Colomar
d2c28a402a src/sulogin.c: Align local variables
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-01-17 18:11:19 -06:00
Alejandro Colomar
1faf4d6469 src/sulogin.c: Make static variables local to main()
Those variables are only used in main().  Restrict their scope.
Keep them static (.bss), as changing that may be dangerous.

Suggested-by: Samanta Navarro <ferivoz@riseup.net>
Cc: Serge Hallyn <serge@hallyn.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-01-17 18:11:19 -06:00
Alejandro Colomar
5214710432 src/sulogin.c: pw_entry(): Don't else after return
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-01-17 18:11:19 -06:00
Alejandro Colomar
8679878c8b lib/, src/, po/: pw_entry(): Move function to src/sulogin.c
That's the only file where it's called, and it's a delicate function.
Reduce the chances that other files call it.

Link: <https://github.com/shadow-maint/shadow/pull/908>
Suggested-by: Samanta Navarro <ferivoz@riseup.net>
Cc: Serge Hallyn <serge@hallyn.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-01-17 18:11:19 -06:00
Alejandro Colomar
2e56af1902 lib/, tests/: addsl(): Add addsl(), a variadic macro
Add a variadic macro addsl() that accepts an arbitrary number of
addends, instead of having specific versions like addsl2() or addsl3().

It is internally implemented by the addslN() function, which itself
calls addsl2().  addsl3() is now obsolete and thus removed.

Code should just call addsl().

Link: <https://github.com/shadow-maint/shadow/pull/882#discussion_r1437155212>
Cc: Serge Hallyn <serge@hallyn.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-01-16 16:58:18 +01:00
Alejandro Colomar
2e5fc4c90b lib/, tests/: addsl2(): Rename addsl() to addsl2()
This is for consistency with addsl3(), and in preparation for the
following commit, which will unify the interface into a single addsl()
macro.

Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-01-16 16:58:18 +01:00
Alejandro Colomar
1356b14a00 lib/defines.h: Don't wrap #undef in #ifdef
ISO C guarantees that #undef is a no-op if there is no such macro.

C11::6.10.3.5p2:
> A preprocessing directive of the form
>
>       # undef identifier new-line
>
> causes the specified identifier no longer to be defined as a macro
> name.  It is ignored if the specified identifier is not currently
> defined as a macro name.

Link: <http://port70.net/~nsz/c/c11/n1570.html#6.10.3.5p2>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-01-15 13:41:06 -06:00
Alejandro Colomar
effdb14786 lib/idmapping.c: write_mapping(): Fixx off-by-one bug
Link: <673c2a6f9a (r136830993)>
Cc: Serge Hallyn <serge@hallyn.com>
Cc: "Eric W. Biederman" <ebiederm@xmission.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-01-15 13:37:09 -06:00
Alejandro Colomar
6bec1cf37c lib/: Use 'restrict' alongside [[gnu::access()]]
const + restrict imply read_only.

Cc: Serge Hallyn <serge@hallyn.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-01-15 13:14:28 -06:00
Alejandro Colomar
76e7de3fbb lib/: Use ATTR_ACCESS() instead of /*@out@*/
The compiler seems to ignore the attribute in a function pointer,
though.

Link: <https://splint.org/manual/manual.html#undefined>
Cc: Serge Hallyn <serge@hallyn.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-01-15 13:14:28 -06:00
Alejandro Colomar
561448443f lib/: get[u]long(): Use ATTR_ACCESS() instead of /*@out@*/
Link: <https://splint.org/manual/manual.html#undefined>
Cc: Serge Hallyn <serge@hallyn.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-01-15 13:14:28 -06:00
Alejandro Colomar
9ca6b71e76 lib/: Remove incorrect /*@out@*/ comment from functions that read the pointee
These functions (e.g., gr_free()), explicitly dereference the pointer
and read the pointee.

The /@out@/ comment, which is (almost) analogous to the
[[gnu::access(write_only, ...)]] attribute, means that the pointee can
be uninitialized, since it won't read it.  There's a difference between
/@out@/ and the GCC attribute: the attribute doesn't require that the
call writes to the pointee, while /@out@/ requires that the pointee be
fully initialized after the call, so it _must_ write to it.

A guess of why it was used is that these functions are similar to
free(3), which does not read the memory it frees, and so one would
assume that if it doesn't read, write_only (or equivalents) are good.
That's wrong in several ways:

-  free(3) does not read _nor_ write to the memory, so it would
   be slightly inappropriate to use write_only with it.  It wouldn't be
   "wrong", but [[gnu::access(none, ...)]] would be more appropriate.

-  Because /@out@/ requires that the call writes to the pointee, it
   would be wrong to use it in free(3), which doesn't write to the
   pointee.

-  Our functions are similar to free(3) conceptually, but they don't
   behave like free(3), since they do read the memory (pointee) (and
   also write to it), and thus they're actually read_write.

Link: <https://splint.org/manual/manual.html#undefined>
Cc: Serge Hallyn <serge@hallyn.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-01-15 13:14:28 -06:00
Alejandro Colomar
f1b9f8d829 lib/: Remove /*@out@*/ comments in return type
/*@out@*/ makes no sense in the return of a function, AFAICS.

Link: <https://splint.org/manual/manual.html#undefined>
Cc: Serge Hallyn <serge@hallyn.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-01-15 13:14:28 -06:00
Alejandro Colomar
7c1576cfb6 lib/: fgetsx(): Use ATTR_ACCESS() instead of /*@out@*/
Link: <https://splint.org/manual/manual.html#undefined>
Cc: Serge Hallyn <serge@hallyn.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-01-15 13:14:28 -06:00
Alejandro Colomar
a070b84f2e lib/: run_command(): Use ATTR_ACCESS() instead of /*@out@*/
Link: <https://splint.org/manual/manual.html#undefined>
Cc: Serge Hallyn <serge@hallyn.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-01-15 13:14:28 -06:00
Alejandro Colomar
9ac5b2fc5a lib/attr.h: Add ATTR_ACCESS()
This will replace the existing comments like /*@out@*/

Link: <https://splint.org/manual/manual.html#undefined>
Cc: Serge Hallyn <serge@hallyn.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-01-15 13:14:28 -06:00
Samanta Navarro
a9e07c0feb lib/sgetgrent.c: fix null pointer dereference
If reallocation fails in function list, then reset the size to 0 again.
Without the reset, the next call assumes that `members` points to
a memory location with reserved space.

Also use size_t instead of int for size to prevent signed integer
overflows. The length of group lines is not limited.

Fixes 45c0003e53 (4.14 release series)

Reviewed-by: Alejandro Colomar <alx@kernel.org>
Signed-off-by: Samanta Navarro <ferivoz@riseup.net>
2024-01-15 13:06:35 -06:00
Alejandro Colomar
4c0c7c52f1 lib/: get_pid(): Use the usual -1 as an error code
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-01-05 16:54:55 -06:00
Alejandro Colomar
18c428a6c9 lib/, src/: get_uid(): Use the usual -1 as an error code
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-01-05 16:54:55 -06:00