Commit Graph

3214 Commits

Author SHA1 Message Date
Serge Hallyn
fa68441bc4 Improve the login.defs unknown item error message
Closes #746

Only print the 'unknown item' message to syslog if we are
actually parsing a login.defs.  Prefix it with "shadow:" to make
it clear in syslog where it came from.

Also add the source filename to the console message.  I'm not
quite clear on the econf API, so not sure whether in that path we
will end up actually having the path, or printing ''.

Signed-off-by: Serge Hallyn <serge@hallyn.com>
2023-10-20 18:46:23 -05:00
Alejandro Colomar
d73f480ddc autogen.sh: Prepare CFLAGS before ./configure
Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2023-10-20 21:05:33 +02:00
Alejandro Colomar
b3652d8a32 lib/: Add missing #include <config.h>
Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2023-10-20 21:05:33 +02:00
Alejandro Colomar
a6d795bac5 autogen.sh: CFLAGS: Add -Werror=implicit-function-declaration
This is not just a style issue.  This should be a hard error, and never
compile.  ISO C89 already had this feature as deprecated.  ISO C99
removed this deprecated feature, for good reasons.  If we compile
ignoring this warning, shadow is not going to behave well.

Cc: Sam James <sam@gentoo.org>
Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2023-10-20 21:05:33 +02:00
Alejandro Colomar
d5e1c1e475 lib/, src/: Use xasprintf() instead of its pattern
Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2023-10-20 21:05:33 +02:00
Alejandro Colomar
ad3b31a59e lib/, src/: Use asprintf(3) instead of strlen(3)+malloc(3)+snprintf(3)
asprintf(3) is non-standard, but is provided by GNU, the BSDs, and musl.
That makes it portable enough for us to use.

This function is much simpler than the burdensome code for allocating
the right size.  Being simpler, it's thus safer.

I took the opportunity to fix the style to my preferred one in the
definitions of variables used in these calls, and also in the calls to
free(3) with these pointers.  That isn't gratuituous, but has a reason:
it makes those appear in the diff for this patch, which helps review it.
Oh, well, I had an excuse :)

Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2023-10-20 21:05:33 +02:00
Alejandro Colomar
c5e5fee606 lib/copydir.c: Use goto to reduce a conditional branch
Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2023-10-20 21:05:33 +02:00
Alejandro Colomar
2a558bd8cb tests/unit/test_xasprintf.c: Test x[v]asprintf()
Link: <https://github.com/shadow-maint/shadow/pull/816>
Suggested-by: Iker Pedrosa <ipedrosa@redhat.com>
Acked-by: Andreas Schneider <https://github.com/cryptomilk>
Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2023-10-20 21:05:33 +02:00
Alejandro Colomar
83c8a2d3fa lib/sprintf.[ch]: Add x[v]asprintf()
As other x...() wrappers around functions that allocate, these wrappers
are like [v]asprintf(3), but exit on failure.

Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2023-10-20 21:05:33 +02:00
Alejandro Colomar
7c93e1cdce lib/copydir.c: Invert conditional to reduce nesting
Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2023-10-20 21:05:33 +02:00
Dimitri John Ledkov
088fe2618f Fix badname option to be singular just like useradd.
Badnames still accepted, note that previously usage already stated
singular form, whilst manpage and real one was plural only.

Fixes: 45d6746219 ("src: correct "badname" option")

Signed-off-by: Dimitri John Ledkov <dimitri.ledkov@canonical.com>
2023-10-16 12:45:21 -05:00
Dimitri John Ledkov
2e45fff44b Fix mixed-whitespace
Signed-off-by: Dimitri John Ledkov <dimitri.ledkov@canonical.com>
2023-10-16 12:45:21 -05:00
Iker Pedrosa
0d50e1e15f Remove TODO
Sad to remove this file, but things are going on and it doesn't seem to
be up to date.

Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
2023-10-04 13:31:38 -05:00
Iker Pedrosa
fe299017b1 Remove shadow.spec.in
The file isn't up to date with the latest development, the last change
was made 15 years ago, so I'm removing it.

Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
2023-10-04 13:31:38 -05:00
Iker Pedrosa
a0546212c0 Remove .travis.yml
It isn't used anywhere so let's remove it.

Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
2023-10-04 13:31:38 -05:00
Iker Pedrosa
c883786f4f doc: remove WISHLIST
Another file that I remove with sadness. We were unable to complete the
first item but we are working hard on it.

Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
2023-10-04 13:31:38 -05:00
Iker Pedrosa
bc35dfe4ec doc: remove README.platforms
I remove this file with sadness, as it contains data from old times.
Unfortunately, this data is no longer relevant. The source code
management tool will keep it in memory.

Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
2023-10-04 13:31:38 -05:00
Iker Pedrosa
2cfa1743d3 doc: remove cracklib26.diff
Keeping a patch for a file no longer maintained is a bad idea, so I'm
removing it.

Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
2023-10-04 13:31:38 -05:00
Iker Pedrosa
3a43d72e42 doc: remove console.c.spec.txt
I guess we are keeping this for historical purposes more than anything
else. If so, anybody can check the git history to recover the
specification.

Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
2023-10-04 13:31:38 -05:00
Iker Pedrosa
b91b3793a9 contrib: remove udbachk.tgz
Having source code in a compressed file doesn't seem like a good idea. I
checked several distributions and they don't distribute this binary, so
let's remove it.

Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
2023-10-04 13:31:38 -05:00
Iker Pedrosa
d702e08097 contrib: remove shadow-anonftp.patch
The patch is never applied upstream. If I were to take a gamble, I would
even say that it throws an error when trying to patch.

Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
2023-10-04 13:31:38 -05:00
Iker Pedrosa
52d2198252 contrib: remove groupmems.shar
Not sure what this file is exactly, but there's already a groupmems.c
that should generate the binary responsible for managing  the members of
a user's primary group.

Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
2023-10-04 13:31:38 -05:00
Iker Pedrosa
fbcd8b536a contrib: remove atudel
AFAIK, it isn't included in any distribution and it isn't used
internally in the project, so let's remove it.

Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
2023-10-04 13:31:38 -05:00
Iker Pedrosa
13a7713384 CI: remove .builds folder
We stopped using the CI relying on this folder and moved to Github's, so
I'm removing these files.

Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
2023-10-04 13:31:38 -05:00
Johannes Segitz
48aa12af31 useradd: Set proper SELinux labels for def_usrtemplate
Fixes: 74c17c716 ("Add support for skeleton files from /usr/etc/skel")

Signed-off-by: Johannes Segitz <jsegitz@suse.com>
2023-10-03 09:24:47 +02:00
Iker Pedrosa
4f49e3fd3e doc: add unit tests
Brief description of the unit testing framework and how to create test
cases with it.

Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
2023-09-29 09:24:01 +02:00
Iker Pedrosa
0fc697a4b1 CI: build and run unit tests
Run `make check` after the project is built in every runner.

Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
2023-09-29 09:24:01 +02:00
Iker Pedrosa
015448b049 tests: happy path for active_sessions_count()
Simple test to check the recently implemented logind functionality. It
also contains the changes to the build infrastructure, and the
gitignore.

Resolves: https://github.com/shadow-maint/shadow/issues/790

Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
2023-09-29 09:24:01 +02:00
Iker Pedrosa
163c424999 configure: add cmocka for unit tests
Prepare the ground for unit tests.

Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
2023-09-29 09:24:01 +02:00
Christian Göttsche
247a869ccd faillog: check for overflows
Check for arithmetic overflows when computing offsets to avoid file
corruptions for huge UIDs.

Refactor the file lookup into a separate function.
2023-09-29 09:20:43 +02:00
Iker Pedrosa
5178f8c5af utmp: call prepare_utmp() even if utent is NULL
update_utmp() should also return 0 when success.

Fixes: 1f368e1c18 ("utmp: update
`update_utmp()")
Resolves: https://github.com/shadow-maint/shadow/issues/805

Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
2023-09-15 12:57:16 -05:00
Vasil Velichkov
bef4da47be groupadd: Improve error message when opening group file fails.
Both gr_open and sgr_open are using commonio_open function and when
there is a failure this function sets errno accordingly.
2023-09-04 16:04:42 +02:00
Alejandro Colomar
c1fd94d7d5 lib/mempcpy.[ch]: Remove our definition of mempcpy(3)
It is provided by glibc, musl, and FreeBSD.

Reported-by: Sam James <sam@gentoo.org>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2023-09-04 08:57:43 -05:00
Alejandro Colomar
9b0f8ddc30 lib/pwauth.c: Replace getpass(3) by agetpass()
Closes: <https://github.com/shadow-maint/shadow/issues/797>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2023-09-04 08:57:18 -05:00
Alejandro Colomar
7c45a6e8ba lib/agetpass.h: Move prototypes to dedicated header
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2023-09-04 08:57:18 -05:00
Alejandro Colomar
158866bfdc lib/pwauth.c: Simplify empty string
And do not set 'clear' to point to the empty string.  After this commit,
'clear' only stores the result of getpass(3).  This will be useful to
change the code to use agetpass().

$ grep '\<clear\>' lib/pwauth.c;
	char *clear = NULL;
		clear = getpass (prompt);
		input = (clear == NULL) ? "" : clear;
		clear = getpass (prompt);
		input = (clear == NULL) ? "" : clear;
	if (NULL != clear) {
		strzero (clear);

Signed-off-by: Alejandro Colomar <alx@kernel.org>
2023-09-04 08:57:18 -05:00
Alejandro Colomar
adbdd086a2 lib/pwauth.c: Remove dead code
There are no users of 'clear_pass' and 'wipe_clear_pass'.

$ grep -rn '\<clear_pass\>'
lib/pwauth.c:35:/*@null@*/char *clear_pass = NULL;
lib/pwauth.c:199:	 * not wipe it (the caller should wipe clear_pass when it is
lib/pwauth.c:203:	clear_pass = clear;

$ grep -rn wipe_clear_pass
lib/pwauth.c:34:bool wipe_clear_pass = true;
lib/pwauth.c:198:	 * if the external variable wipe_clear_pass is zero, we will
lib/pwauth.c:204:	if (wipe_clear_pass && (NULL != clear) && ('\0' != *clear)) {
ChangeLog:3813:	* lib/pwauth.c: Use a boolean for wipe_clear_pass and use_skey.

Remove them.

Signed-off-by: Alejandro Colomar <alx@kernel.org>
2023-09-04 08:57:18 -05:00
Alejandro Colomar
2b393114c7 lib/pwauth.c: Remove dead code
If the string is "", then strzero() is a no-op.  We don't need to test
that.

Signed-off-by: Alejandro Colomar <alx@kernel.org>
2023-09-04 08:57:18 -05:00
Alejandro Colomar
8893c51480 autogen.sh: Support out-of-tree builds
This allows to do the following:

~/src/shadow/shadow/master$ mkdir .tmp/ && cd .tmp/
~/src/shadow/shadow/master/.tmp$ ../autogen.sh

Link: <https://github.com/shadow-maint/shadow/issues/795>
Reviewed-by: Sam James <sam@gentoo.org>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2023-09-04 15:47:14 +02:00
Alejandro Colomar
9514a841bc zustr2stp.h: Assert some assumptions about the size
If the destination buffer is an array, we can check our assumptions.
This adds a readable way to explain that dsize must be strictly > ssize.
The reason is that the destination string is the source + '\0'.

If the destination is not an array, it's up to _FORTIFY_SOURCE or
-fanalyzer to catch newly introduced errors.  There's nothing we can do;
at least not portably.

Suggested-by: Serge Hallyn <serge@hallyn.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2023-09-01 09:39:23 +02:00
Alejandro Colomar
3bf8d68f10 strlcpy.[ch]: Add strlcpy_()
This function is like strlcpy(3), but returns -1 on truncation, which
makes it much easier to test.  strlcpy(3) is useful in two cases:

-  We don't care if the output is truncated.  strlcpy(3) is fine for
   those, and the return value can be ignored.

-  Truncation is bad.  In that case, we just want to signal truncation,
   and the length of the original string is quite useless.  Return the
   length iff no truncation so that we can use it if necessary.

This simplifies the definition of the STRLCPY() macro.

Signed-off-by: Alejandro Colomar <alx@kernel.org>
2023-09-01 09:39:23 +02:00
Alejandro Colomar
e7a292ed4f Use bzero(3) instead of its pattern
It was blessed by POSIX.1-2001, and GCC says that it won't go away,
possibly ever.

memset(3) is dangerous, as the 2nd and 3rd arguments can be accidentally
swapped --who remembers what's the order of the 2nd and 3rd parameters
to memset(3) without checking the manual page or some code that uses
it?--.  Some recent compilers may be able to catch that via some
warnings, but those are not infalible.  And even if compiler warnings
could always catch that, the time lost in fixing or checking the docs is
lost for no clear gain.  Having a sane API that is unambiguous is the
Right Thing (tm); and that API is bzero(3).

If someone doesn't believe memset(3) is error-prone, please read the
book "Unix Network Programming", Volume 1, 3rd Edition by Stevens, et
al., Section 1.2.  See a stackoverflow reference in the link below[1].

bzero(3) had a bad fame in the bad old days, because some ancient
systems (I'm talking of many decades ago) shipped a broken version of
bzero(3).  We can assume that all systems in which current shadow utils
can be built, have a working version of bzero(3) --if not, please fix
your broken system; don't blame the programmer--.

One reason that some use today to avoid bzero(3) in favor of memset(3)
is that memset(3) is more often used; but that's a circular reasoning.
Even if bzero(3) wasn't supported by the system, it would need to be
invented.  It's the right API.

Another reason that some argue is that POSIX.1-2008 removed the
specification of bzero(3).  That's not a problem, because GCC will
probably support it forever, and even if it didn't, we can redefine it
like we do with memzero().  bzero(3) is just a one-liner wrapper around
memset(3).

Link: [1] <https://stackoverflow.com/a/17097978>
Cc: Christian Göttsche <cgzones@googlemail.com>
Cc: Serge Hallyn <serge@hallyn.com>
Cc: Iker Pedrosa <ipedrosa@redhat.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2023-09-01 09:39:23 +02:00
Alejandro Colomar
624bacfbd8 Use CALLOC() instead of its pattern
MALLOC() + memset() is simpler written as CALLOC().

Cc: Christian Göttsche <cgzones@googlemail.com>
Cc: Serge Hallyn <serge@hallyn.com>
Cc: Iker Pedrosa <ipedrosa@redhat.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2023-09-01 09:39:23 +02:00
Alejandro Colomar
24367027d6 Use STRLCPY() instead of its pattern
This makes it harder to make mistakes while editing the code.  Since the
sizeof's can be autocalculated, let the machine do that.  It also
reduces the cognitive load while reading the code.

Cc: Christian Göttsche <cgzones@googlemail.com>
Cc: Serge Hallyn <serge@hallyn.com>
Cc: Iker Pedrosa <ipedrosa@redhat.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2023-09-01 09:39:23 +02:00
Alejandro Colomar
370652ba05 defines.h: Remove definition of STRFCPY()
It's not being used anymore.  We got rid of it in favor of better APIs.

Well, it's still being used in one place: a contrib/ patch, but I
explicitly want to break it, so that someone reviews it.  I don't want
to modify it, since it's not being tested, so it would be very risky for
me to touch it.  Instead, let it bitrot, and if someone cares, they'll
update it correctly.

BTW, the comment that said /* danger -side effects */ was wrong:
sizeof() doesn't evaluate the argument (unless it's a VLA), so there
wasn't really a double-evaluation issue.

Cc: Christian Göttsche <cgzones@googlemail.com>
Cc: Serge Hallyn <serge@hallyn.com>
Cc: Iker Pedrosa <ipedrosa@redhat.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2023-09-01 09:39:23 +02:00
Alejandro Colomar
3029883888 passwd: Replace STRFCPY() by STRLCPY()
The variables are only being read as strings (char *), so data after the
'\0' can't be leaked.

Cc: Christian Göttsche <cgzones@googlemail.com>
Cc: Serge Hallyn <serge@hallyn.com>
Cc: Iker Pedrosa <ipedrosa@redhat.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2023-09-01 09:39:23 +02:00
Alejandro Colomar
7bfcf1724c gpasswd: Replace STRFCPY() by STRLCPY()
The variable is only being read as a string (char *), so data after the
'\0' can't be leaked.

Cc: Christian Göttsche <cgzones@googlemail.com>
Cc: Serge Hallyn <serge@hallyn.com>
Cc: Iker Pedrosa <ipedrosa@redhat.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2023-09-01 09:39:23 +02:00
Alejandro Colomar
fcc25a03cd login: Replace STRFCPY() by STRLCPY()
The variable is only being read as a string (char *), so data after the
'\0' can't be leaked.

Cc: Christian Göttsche <cgzones@googlemail.com>
Cc: Serge Hallyn <serge@hallyn.com>
Cc: Iker Pedrosa <ipedrosa@redhat.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2023-09-01 09:39:23 +02:00
Alejandro Colomar
6dacb154e5 su: Replace STRFCPY() by STRLCPY()
The variables are only being read as strings (char *), so data after the
'\0' can't be leaked.

Cc: Christian Göttsche <cgzones@googlemail.com>
Cc: Serge Hallyn <serge@hallyn.com>
Cc: Iker Pedrosa <ipedrosa@redhat.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2023-09-01 09:39:23 +02:00
Alejandro Colomar
3e0913f119 sulogin: Replace STRFCPY() by STRLCPY()
The variable is only being read as a string (char *), so data after the
'\0' can't be leaked.

Cc: Christian Göttsche <cgzones@googlemail.com>
Cc: Serge Hallyn <serge@hallyn.com>
Cc: Iker Pedrosa <ipedrosa@redhat.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2023-09-01 09:39:23 +02:00