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
3b7cc05387 ("lib: replace `USER_NAME_MAX_LENGTH` macro"), and
6be85b0baf ("lib/chkname.c: Use tmp variable to avoid a -Wsign-compare
warning") even clarified this in the commit message.
So, while the code in 6a1f45d932 ("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 403a2e3771
("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: 6be85b0baf ("lib/chkname.c: Use tmp variable to avoid a -Wsign-compare warning")
Fixes: 6a1f45d932 ("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 3b7cc05387 ("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.
403a2e3771 ("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.
Link: <https://github.com/shadow-maint/shadow/pull/935>
Link: <https://github.com/shadow-maint/shadow/issues/920#issuecomment-1926002209>
Link: <https://github.com/shadow-maint/shadow/pull/757>
Link: <https://github.com/shadow-maint/shadow/issues/674>
See-also: 403a2e3771 ("lib/chkname.c: Take NUL byte into account")
Fixes: 3b7cc05387 ("lib: replace `USER_NAME_MAX_LENGTH` macro")
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>
It is slightly confusing to allow adding these only to later refuse them.
Here is a (lightly tested :) patch to also refuse them when adding.
Signed-off-by: Tycho Andersen <tycho@tycho.pizza>
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).
Fixes: 45c6603cc8 ("[svn-upgrade] Integrating new upstream version, shadow (19990709)")
Link: <https://github.com/shadow-maint/shadow/issues/939>
Reported-by: Michael Vetter <jubalh@iodoru.org>
Tested-by: Gus Kenion <https://github.com/kenion>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
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.
Co-developed-by: Tobias Stoeckmann <tobias@stoeckmann.org>
Co-developed-by: Alejandro Colomar <alx@kernel.org>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
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.
Co-developed-by: Tobias Stoeckmann <tobias@stoeckmann.org>
Co-developed-by: Alejandro Colomar <alx@kernel.org>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
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.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
The _SC_LOGIN_NAME_MAX value includes space for the NUL byte. The length
of name must smaller than this value to be valid.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
The size of time_t varies across systems, but since data type long is
more than enough to calculate with days (precision of shadow file),
use it instead.
Just in case a shadow file contains huge values, check for a possible
signed integer overflow.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
Link: <https://github.com/shadow-maint/shadow/pull/876>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
Contrary to the comment in ENV_HZ.xml, ENV_HZ is not even used in
sulogin (anymore) if PAM support is enabled.
Skip paragraphs of sulogin if PAM support is enabled, since they would
be empty now.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
Special care has to be taken for 32 bit systems with a 64 bit time_t,
since their long data type is still 32 bit.
Since this macro expresses a number of seconds, and seconds are in units
of 'time_t' in C, the appropriate type for the multiplication is
'time_t'.
Reported-by: Tobias Stoeckmann <tobias@stoeckmann.org>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
The values are retrieved from login.defs files, which normally do not
contain negative values. In fact, negative value -1 is used in many
code places as "feature disabled", which is normally achieved by
simply commenting out the key from the file.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
The variable declarations for the buffers have been aligned in this
commit, so that they appear in the diff, making it easier to review.
Some important but somewhat tangent changes included in this commit:
- lib/nss.c: The size was being defined as 65, but then used as 64.
That was a bug, although not an important one; we were just wasting
one byte. Fix that while we replace snprintf() by SNPRINTF(), which
will get the size from sizeof(), and thus will use the real size.
Signed-off-by: Alejandro Colomar <alx@kernel.org>
These functions are like [v]snprintf(3), but return -1 on truncation,
which makes it easier to test. In fact, the API of swprintf(3), which
was invented later than snprintf(3), and is the wide-character version
of it, is identical to this snprintf_().
snprintf(3) is iseful in two cases:
- We don't care if the output is truncated. snprintf(3) is fine for
those, and the return value can be ignored. But snprintf_() is also
fine for those.
- Truncation is bad. In that case, it's as bad as a hard error (-1)
from snprintf, so merging both problems into the same error code
makes it easier to handle errors. Return the length if no truncation
so that we can use it if necessary.
Not returning the whole length before truncation makes a better API,
which need not read the entire input, so it's less vulnerable to DoS
attacks when a malicious user controls the input.
Use these functions to implement SNPRINTF().
Cc: Samanta Navarro <ferivoz@riseup.net>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
It wraps snprintf(3) so that it performs some steps that one might
forget, or might be prone to accidents:
- It calculates the size of the destination buffer, and makes sure it's
an array (otherwise, using sizeof(s) would be very bad).
- It calculates if there's truncation or an error, returning -1 if so.
BTW, this macro doesn't have any issues of double evaluation, because
sizeof() doesn't evaluate its argument (unless it's a VLA, but then the
static_assert(3) within NITEMS() makes sure VLAs are not allowed).
This macro is very similar to STRTCPY(), defined in
<lib/string/strtcpy.h>.
Signed-off-by: Alejandro Colomar <alx@kernel.org>
commonio.c: In function 'commonio_unlock':
commonio.c:487:49: warning: '.lock' directive output may be truncated writing 5 bytes into a region of size between 1 and 1024 [-Wformat-truncation=]
487 | snprintf (lock, sizeof lock, "%s.lock", db->filename);
| ^~~~~
commonio.c:487:17: note: 'snprintf' output between 6 and 1029 bytes into a destination of size 1024
487 | snprintf (lock, sizeof lock, "%s.lock", db->filename);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Check for close(2) failure at more places closing a file descriptor
written to.
Also ignore failures with errno set to EINTR (see man:close(2) for
details).
sprintf(3) does not take the destination buffer into account. Although
the destination in these case is large enough, sprintf(3) indicates a
code smell.
Use the xasprintf() wrapper.
Group them at the end of the list of variable definitions, and use
'#if defined()' instead of '#if[n]def'. Also indent nested ones.
Signed-off-by: Alejandro Colomar <alx@kernel.org>
ITI_AGING is not set through any build environment. If it would be set,
then timings in /etc/shadow would not fit anymore.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
su.c:678:26: warning: format ‘%s’ expects argument of type ‘char *’, but argument 4 has type ‘const void *’ [-Wformat=]
su.c:681:44: warning: format ‘%s’ expects argument of type ‘char *’, but argument 3 has type ‘const void *’ [-Wformat=]
su.c:683:46: warning: format ‘%s’ expects argument of type ‘char *’, but argument 3 has type ‘const void *’ [-Wformat=]
Reported-by: Christian Göttsche <cgzones@googlemail.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
We don't need to terminate them manually after the call. Remove all
that paranoid code, which in some cases was even wrong. While at it,
let's do a few more things:
- Use sizeof(buf) for the size of the buffer. I found that a few cases
were passing one less byte (probably because the last one was
manually zeroed later). This caused a double NUL. snprintf(3) wants
the size of the entire buffer to properly terminate it. Passing the
exact value hardcoded is brittle, so use sizeof().
- Align and improve style of variable declarations. This makes them
appear in this diff, which will help review the patch.
Signed-off-by: Alejandro Colomar <alx@kernel.org>
These functions consume a source string. Document that. There's no way
to mark that they also produce a string in dst, though. That will be up
to the static analyzer to guess.
Signed-off-by: Alejandro Colomar <alx@kernel.org>
It signals that a function parameter is a string _before_ the call.
Suggested-by: Serge Hallyn <serge@hallyn.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
- Set errno = 0 before the call. Otherwise, it may contain anything.
- ERANGE is not the only possible errno value of these functions. They
can also set it to EINVAL.
- Any errno value after these calls is bad; just compare against 0.
- Don't check for the return value; just errno. This function is
guaranteed to not modify errno on success (POSIX).
- Check endptr == str, which may or may not set EINVAL.
Suggested-by: Iker Pedrosa <ipedrosa@redhat.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
Since failure() is [[noreturn]], we can invert the conditional so that
we don't need an else. This silences a -Wunused-parameter warning.
Signed-off-by: Alejandro Colomar <alx@kernel.org>
This simplifies the code a little bit, and prepares for the next
commits, which will clean up further.
Signed-off-by: Alejandro Colomar <alx@kernel.org>
The multiplication was already invoking UB. The test was flawed.
Use __builtin_mul_overflow() instead.
Signed-off-by: Alejandro Colomar <alx@kernel.org>
strtol(3) doesn't specify a return value if (value == endptr).
It is always an error, if (value==endptr).
Signed-off-by: Alejandro Colomar <alx@kernel.org>
I used size_t because:
sysconf(3) can return -1 if the value is not supported, but then it can
only mean that there's no limit. Having no limit is the same as having
a limit of SIZE_MAX (to which -1 is converted).
Signed-off-by: Alejandro Colomar <alx@kernel.org>
By writing the terminating null byte via stpcpy(3), we take advantage of
_FORTIFY_SOURCE for the last byte, which was unprotected before this
commit.
Reported-by: Serge Hallyn <serge@hallyn.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
We were using strncpy(3), which is designed to copy from a string into a
(null-padded) fixed-size character array. However, we were doing the
opposite: copying from a known-size array (which was a prefix of a
string), into a string. That's why we had to manually zero the buffer
afterwards.
Use instead mempcpy(3) to copy the non-null bytes, and then terminate
with a null byte with stpcpy(..., "").
Cc: "Serge E. Hallyn" <serge@hallyn.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
Upcoming `gcc-14` enabled a few warnings into errors, like
`-Wimplicit-function-declaration`. This caused `shadow` build to fail
as:
pwunconv.c: In function 'main':
pwunconv.c:132:13: error: implicit declaration of function 'getdef_bool' [-Wimplicit-function-declaration]
132 | if (getdef_bool("USE_TCB")) {
| ^~~~~~~~~~~
The change adds missing include headers.
If an entry in /etc/shells is not an absolute path (comments or
partial reads due to fgets), the line should not be considered as
a valid login shell.
In general all systems should have getusershells, but let's better
be safe than sorry.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
For arrays of char, both NITEMS() and SIZEOF_ARRAY() return the same
value. However, NITEMS() is more appropriate. Think of wide-character
equivalents of the same code; with NITEMS(), they would continue to be
valid, while with SIZEOF_ARRAY(), they would be wrong.
In the implementation of ZUSTR2STP(), we want SIZEOF_ARRAY() within the
static assert, because we're just comparing the sizes of the source and
destination buffers, and we don't care if we compare sizes or numbers of
elements, and using sizes is just simpler. But we want NITEMS() in the
zustr2stp() call, where we want to copy a specific number of characters.
Signed-off-by: Alejandro Colomar <alx@kernel.org>
We've recently fixed several bugs in the calculation of the size in this
function call. Use this wrapper to prevent similar mistakes in the
future.
Signed-off-by: Alejandro Colomar <alx@kernel.org>
This wrapper calculates the destination buffer's size, to avoid errors
in the size calculation.
A curious fact: this macro did exist in Version 7 Unix (with a slightly
different name). I found it by chance, investigating the origins of
strncpy(3) and strncat(3) in V7, after Branden suggested me to do so,
related to recent discussions about string_copying(7).
alx@debian:~/src/unix/unix/Research-V7$ grepc SCPYN .
./usr/src/cmd/login.c:#define SCPYN(a, b) strncpy(a, b, sizeof(a))
Our implementation is slightly better, because using nitems() we're
protected against passing a pointer instead of an array, and it's also
conceptually more appropriate: for wide characters, it would be
#define WCSNCPY(dst, src) wcsncpy(dst, src, NITEMS(dst))
Cc: "G. Branden Robinson" <branden@debian.org>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
We're not even zeroing the last byte after this call. This was a
completely gratuitous truncation of one byte, and the resulting
character array still wasn't guaranteed to be null terminated, because
strncpy(3) can't do that.
Just to clarify, none of these structures needed zeroing, as they are
treated as null-padded fixed-size character arrays. Calling strncpy(3)
was actually the correct call, and the only problem was unnecessarily
truncating strings by one byte more than necessary.
Cc: Matthew House <mattlloydhouse@gmail.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
This call was too clever. It relied on the last byte of ll_line
being 0 due to a previous memzero() and not writing to it later.
Write an explicit terminating null byte, by using STRTCPY().
Signed-off-by: Alejandro Colomar <alx@kernel.org>
This call was way too clever. It relied on the last byte of fail_line
being 0 due to it being in a static structure and never writing to it.
Write an explicit terminating null byte, by using STRTCPY().
Cc: Matthew House <mattlloydhouse@gmail.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
We were copying from a (zero-padded) fixed-width character array to a
string, but strncpy(3) is meant to do the opposite thing. ZUSTR2STP()
is designed to be used in this case (like strncat(3)).
Fixes: f40bdfa66a ("libmisc: implement `get_session_host()`")
Cc: Iker Pedrosa <ipedrosa@redhat.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
These functions don't seem to exist anymore. I can't find them in
Debian, nor in a web search. They probably were functions from an
ancient implementation of cracklib that doesn't exist anymore.
$ git remote -v
origin git@github.com:cracklib/cracklib.git (fetch)
origin git@github.com:cracklib/cracklib.git (push)
$ grep -rni fascisthistory
$ git log --grep FascistHistory
$ git log -S FascistHistory
Closes: <https://codesearch.debian.net/search?q=FascistHistory&literal=1>
Cc: Mike Frysinger <vapier@gentoo.org>
Acked-by: Michael Vetter <jubalh@iodoru.org>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
strftime(3) makes no guarantees about the contents of the buffer if the
formatted string wouldn't fit in the buffer. It simply returns 0, and
it's the programmer's responsibility to do the right thing after that.
Let's write the string "future" if there's an error, similar to what we
do with gmtime(3)'s errors.
Also, `buf[size - 1] = '\0';` didn't make sense. If the copy fits,
strftime(3) guarantees to terminate with NUL. If it doesn't, the entire
contents of buf are undefined, so adding a NUL at the end of the buffer
would be dangerous: the string could contain anything, such as
"gimme root access now". Remove that, now that we set the string to
"future", as with gmtime(3) errors. This setting to '\0' comes from the
times when we used strncpy(3) in the implementation, and should have
been removed when I changed it to use strlcpy(3); however, I didn't
check we didn't need it anymore.
Cc: Serge Hallyn <serge@hallyn.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
ut_line doesn't hold a string. It is a null-padded fixed-width array.
Luckily, I don't think there has ever existed a ut_line ("/dev/tty*")
that was 32 bytes long. That would have resulted in a buffer overrun.
Anyway, do the right thing, which is copying into a temporary string.
Signed-off-by: Alejandro Colomar <alx@kernel.org>
There's been a very long and interesting discussion in linux-man@ and
libc-alpha@, where we've discussed all the string-copying functions,
their pros and cons, when should each be used and avoided, etc.
Paul Eggert pointed out an important problem of strlcpy(3): it is
vulnerable to DoS attacks if an attacker controls the length of the
source string. And even if it doesn't control it, the function is dead
slow (because its API forces it to calculate strlen(src)).
We've agreed that the general solution for a truncating string-copying
function is to write a wrapper over strnlen(3)+memcpy(3), which is
limited to strnlen(src, sizeof(dst)). This is not vulnerable to DoS,
and is very fast for all buffer sizes. string_copying(7) has been
updated to reflect this, and provides a reference implementation for
this wrapper function.
This strtcpy(3) (t for truncation) wrapper happens to have the same API
that our strlcpy_() function had, so replace it with the better
implementation. We don't need to update callers nor tests, since the
API is the same.
A future commit will rename STRLCPY() to STRTCPY(), and replace
remaining calls to strlcpy(3) by calls to this strtcpy(3).
Link: <https://lore.kernel.org/linux-man/ZU4SDh-Se5gjPny5@debian/T/#mfb5a3fdeb35487dec6f8d9e3d8548bd0d92c4975/>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
This test fails now, due to a bug: the return type of strlcpy_() is
size_t, but it should be ssize_t. The next commit will pass the test,
by fixing the bug.
Signed-off-by: Alejandro Colomar <alx@kernel.org>
libbsd is unwanted in Fedora and RHEL, and the recently released Fedora
39 doesn't contain this dependency in the base image.
shadow removed libbsd from its dependencies for Fedora 39, so let's
build without it to avoid compilation errors.
Resolves: https://github.com/shadow-maint/shadow/issues/839
Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
Reviewed-by: Alejandro Colomar <alx@kernel.org>
free(NULL) is valid; there's no need to check for NULL. Simplify.
Fixes: 5178f8c5af ("utmp: call prepare_utmp() even if utent is NULL")
Signed-off-by: Alejandro Colomar <alx@kernel.org>
copydir.c:429:4: warning: Value stored to 'err' is never read [deadcode.DeadStores]
Also reduce indentation by bailing out early.
(cherry picked from commit d89f2fb06d1b81b56299f9d0bfe7a927a2282f19)
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
It wraps strlcpy(3bsd) so that it performs some steps that one might
forget, or might be prone to accidents:
- It calculates the size of the destination buffer, and makes sure it's
an array (otherwise, using sizeof(dst) would be very bad).
- It calculates if there's truncation, returning an easy-to-use value.
BTW, this macro doesn't have any issues of double evaluation, because
sizeof() doesn't evaluate its argument (unless it's a VLA, but then
the static_assert(3) within SIZEOF_ARRAY() makes sure VLAs are not
allowed).
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>
It's a wrapper around zustr2stp() that calls SIZEOF_ARRAY() internally.
The function call is usually --in our code base, always-- called with an
array as the second argument. For such an argument, one should call
SIZEOF_ARRAY(). To avoid mistakes, and simplify usage, let's add this
macro that does it internally.
BTW, this macro doesn't have any issues of double evaluation, because
sizeof() doesn't evaluate its argument (unless it's a VLA, but then
the static_assert(3) within SIZEOF_ARRAY() makes sure VLAs are not
allowed).
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>
These calls were intending to copy from a NUL-padded (possibly
non-NUL-terminated) character sequences contained in fixed-width arrays,
into a string, where extra padding is superfluous. Use the appropriate
call, which removes the superfluous work. That reduces the chance of
confusing maintainers about the intention of the code.
While at it, use the appropriate third parameter, which is the size of
the source buffer, and not the one of the destination buffer. As a side
effect, this reduces the use of '-1', which itself reduces the chance of
off-by-one bugs.
Also, since using sizeof() on an array is dangerous, use SIZEOF_ARRAY().
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>
There's no standard function that copies from a null-padded character
sequence into a string.
A few standard functions can be workarounded to do that:
- strncat(3): This function is designed to catenate from a null-padded
character sequence into a string. The catch is that there's no
*cpy() equivalent of it --strncpy(3) is not at all related to
strncat(3); don't be fooled by the confusing name--, so one would
need to zero the first byte before the call to strncat(3). It also
has the inconvenient that it returns a useless value.
- strncpy(3): This function is designed to copy from a string to a
null-padded character sequence; the opposite of what we want to do.
If one passes the size of src instead of the size of dst, and then
manually zeroes the last byte of the dst buffer, something similar
to what we want happens. However, this does more than what we want:
it also padds with NUL the remaining bytes after the terminating NUL.
That extra work can confuse maintainers to believe that it's
necessary. That is exactly what happens in logout.c.
src/logoutd.c-46- /*
src/logoutd.c-47- * ut_user may not have the terminating NUL.
src/logoutd.c-48- */
src/logoutd.c:49: strncpy (user, ut->ut_user, sizeof (ut->ut_user));
src/logoutd.c-50- user[sizeof (ut->ut_user)] = '\0';
In that logout.c case --and in most invocations of strncpy(3), which
is usually a wrong tool-- the extra work is not wanted, so it's
preferrable to use the right tool, a function that does exactly
what's needed and nothing more than that. That tool is zustr2stp().
Read string_copying(7) for a more complete comparison of string copying
functions.
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>
This makes it safe to call sizeof() on an array. Calling sizeof()
directly on an array is dangerous, because if the array changes to be a
pointer, the behavior will unexpectedly change. It's the same problem
as with NITEMS().
Link: <https://stackoverflow.com/a/57537491>
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>
There's no need to have these as macros, so use functions, which are a
lot safer: there's no need to worry about multiple evaluation of args,
and there's also more type safety. Compiler warnings are also simpler,
as they don't dump all the nested macros.
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>
These comments were wrong. Remove them instead of fixing them, since
now that we have this small header file, it's much easier to follow the
preprocessor conditionals.
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>
sprintf(3) does not take the destination buffer into account. Although
the destination in these case is large enough, sprintf(3) indicates a
code smell.
Use snprintf(3).
commonio.c:522:15: warning: Although the value stored to 'cp' is used in the enclosing expression, the value is never actually read from 'cp' [deadcode.DeadStores]
Reviewed-by: Alejandro Colomar <alx@kernel.org>
Documentation:
- Correct the comment documenting the function:
write_full() doesn't write "up to" count bytes (which is write(2)'s
behavior, and exactly what this function is designed to avoid), but
rather exactly count bytes (on success).
- While fixing the documentation, take the time to add a man-page-like
comment as in other APIs. Especially, since we'll have to document
a few other changes from this patch, such as the modified return
values.
- Partial writes are still possible on error. It's the caller's
responsibility to handle that possibility.
API:
- In write(2), it's useful to know how many bytes were transferred,
since it can have short writes. In this API, since it either writes
it all or fails, that value is useless, and callers only want to know
if it succeeded or not. Thus, just return 0 or -1.
Implementation:
- Use `== -1` instead of `< 0` to check for write(2) syscall errors.
This is wisdom from Michael Kerrisk. This convention is useful
because it more explicitly tells maintainers that the only value
which can lead to that path is -1. Otherwise, a maintainer of the
code might be confused to think that other negative values are
possible. Keep it simple.
- The path under `if (res == 0)` was unreachable, since the loop
condition `while (count > 0)` precludes that possibility. Remove the
dead code.
- Use a temporary variable of type `const char *` to avoid a cast.
- Rename `res`, which just holds the result from write(2), to `w`,
which more clearly shows that it's just a very-short-lived variable
(by it's one-letter name), and also relates itself more to write(2).
I find it more readable.
- Move the definition of `w` to the top of the function. Now that the
function is significantly shorter, the lifetime of the variable is
clearer, and I find it more readable this way.
Use:
- Also use `== -1` to check errors.
Cc: Christian Göttsche <cgzones@googlemail.com>
Cc: Serge Hallyn <serge@hallyn.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
If shadow is built without libbsd support, then readpassphrase() needs
to be provided from the project.
Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
With the recent changes both login and su compilation fail because there
are some missing dependencies from SELINUX library. Thus, add LIBSELINUX
to su and login for those cases where the library is used.
Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
Remove `utmp` structure as an argument and include its logic inside the
function. This will help remove any reference to utmp from login.
Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
The functionality from this function is related to utmp. Restrict access
to `setutmp()` to the same file.
Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
In some embedded systems, users only use the ps
provided by the busybox. But the ps provided by
the busybox does not support the -eo option by
default. As a result, an error is reported when
the userdel is used. So add a judgment on ps.
If there is no ps -eo, traverse the process directly.
The error information is as follows:
# userdel xsl
ps: invalid option -- 'e'
Signed-off-by: xiongshenglan <xiongshenglan@huawei.com>
Print a warning even for the root user if the provided shell isn't
listed in /etc/shells, but continue to execute the action.
In case of non root user exit.
See https://github.com/shadow-maint/shadow/issues/535
Add a comment at the top of that file explaining how to
regenerate it.
We should add a README, but I don't have time to draft one
right now.
Signed-off-by: Serge Hallyn <serge@hallyn.com>
Since newgrp is setuid-root, any write() system calls it does in order
to print error messages will be done as the root user.
Unprivileged users can get newgrp to print essentially arbitrary strings
to any open file in this way by passing those strings as argv[0] when
calling execve(). For example:
$ setpid() { (exec -a $1$'\n:' newgrp '' 2>/proc/sys/kernel/ns_last_pid & wait) >/dev/null; }
$ setpid 31000
$ readlink /proc/self
31001
This is not a vulnerability in newgrp; it is a bug in the Linux kernel.
However, this type of bug is not new [1] and it makes sense to try to
mitigate these types of bugs in userspace where possible.
[1]: https://lwn.net/Articles/476947/
Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>
b1282224 (Add maximum padding to fit IPv6-Addresses, 2020-05-24) pads
the From field header using `maxIPv6Addrlen - 3`. This leaves the
Latest field header misaligned. Subtract 4 (the length of "From").
Fixes build error:
newusers.c: In function 'update_passwd':
newusers.c:433:21: error: 'sflg' undeclared (first use in this function); did you mean 'rflg'?
introduced by
5cd04d03f9
which forgot to define sflg for these configure options:
--without-sha-crypt --without-bcrypt --with-yescrypt
Using the --sha-rounds option without first giving a crypt method via the --crypt-method option results in comparisons with a NULL pointer and thus make chgpasswd segfault:
$ chgpasswd -s 1
zsh: segmentation fault chgpasswd -s 1
Current patch add a sanity check before these comparisons to ensure there is a defined encryption method.
How to trigger this password leak?
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
When gpasswd(1) asks for the new password, it asks twice (as is usual
for confirming the new password). Each of those 2 password prompts
uses agetpass() to get the password. If the second agetpass() fails,
the first password, which has been copied into the 'static' buffer
'pass' via STRFCPY(), wasn't being zeroed.
agetpass() is defined in <./libmisc/agetpass.c> (around line 91), and
can fail for any of the following reasons:
- malloc(3) or readpassphrase(3) failure.
These are going to be difficult to trigger. Maybe getting the system
to the limits of memory utilization at that exact point, so that the
next malloc(3) gets ENOMEM, and possibly even the OOM is triggered.
About readpassphrase(3), ENFILE and EINTR seem the only plausible
ones, and EINTR probably requires privilege or being the same user;
but I wouldn't discard ENFILE so easily, if a process starts opening
files.
- The password is longer than PASS_MAX.
The is plausible with physical access. However, at that point, a
keylogger will be a much simpler attack.
And, the attacker must be able to know when the second password is being
introduced, which is not going to be easy.
How to read the password after the leak?
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Provoking the leak yourself at the right point by entering a very long
password is easy, and inspecting the process stack at that point should
be doable. Try to find some consistent patterns.
Then, search for those patterns in free memory, right after the victim
leaks their password.
Once you get the leak, a program should read all the free memory
searching for patterns that gpasswd(1) leaves nearby the leaked
password.
On 6/10/23 03:14, Seth Arnold wrote:
> An attacker process wouldn't be able to use malloc(3) for this task.
> There's a handful of tools available for userspace to allocate memory:
>
> - brk / sbrk
> - mmap MAP_ANONYMOUS
> - mmap /dev/zero
> - mmap some other file
> - shm_open
> - shmget
>
> Most of these return only pages of zeros to a process. Using mmap of an
> existing file, you can get some of the contents of the file demand-loaded
> into the memory space on the first use.
>
> The MAP_UNINITIALIZED flag only works if the kernel was compiled with
> CONFIG_MMAP_ALLOW_UNINITIALIZED. This is rare.
>
> malloc(3) doesn't zero memory, to our collective frustration, but all the
> garbage in the allocations is from previous allocations in the current
> process. It isn't leftover from other processes.
>
> The avenues available for reading the memory:
> - /dev/mem and /dev/kmem (requires root, not available with Secure Boot)
> - /proc/pid/mem (requires ptrace privileges, mediated by YAMA)
> - ptrace (requires ptrace privileges, mediated by YAMA)
> - causing memory to be swapped to disk, and then inspecting the swap
>
> These all require a certain amount of privileges.
How to fix it?
~~~~~~~~~~~~~~
memzero(), which internally calls explicit_bzero(3), or whatever
alternative the system provides with a slightly different name, will
make sure that the buffer is zeroed in memory, and optimizations are not
allowed to impede this zeroing.
This is not really 100% effective, since compilers may place copies of
the string somewhere hidden in the stack. Those copies won't get zeroed
by explicit_bzero(3). However, that's arguably a compiler bug, since
compilers should make everything possible to avoid optimizing strings
that are later passed to explicit_bzero(3). But we all know that
sometimes it's impossible to have perfect knowledge in the compiler, so
this is plausible. Nevertheless, there's nothing we can do against such
issues, except minimizing the time such passwords are stored in plain
text.
Security concerns
~~~~~~~~~~~~~~~~~
We believe this isn't easy to exploit. Nevertheless, and since the fix
is trivial, this fix should probably be applied soon, and backported to
all supported distributions, to prevent someone else having more
imagination than us to find a way.
Affected versions
~~~~~~~~~~~~~~~~~
All. Bug introduced in shadow 19990709. That's the second commit in
the git history.
Fixes: 45c6603cc8 ("[svn-upgrade] Integrating new upstream version, shadow (19990709)")
Reported-by: Alejandro Colomar <alx@kernel.org>
Cc: Serge Hallyn <serge@hallyn.com>
Cc: Iker Pedrosa <ipedrosa@redhat.com>
Cc: Seth Arnold <seth.arnold@canonical.com>
Cc: Christian Brauner <christian@brauner.io>
Cc: Balint Reczey <rbalint@debian.org>
Cc: Sam James <sam@gentoo.org>
Cc: David Runge <dvzrv@archlinux.org>
Cc: Andreas Jaeger <aj@suse.de>
Cc: <~hallyn/shadow@lists.sr.ht>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
- Invert conditional to reduce indentation.
- Reduce use of whitespace and newlines while unindenting.
- Reorder variable declarations.
Signed-off-by: Alejandro Colomar <alx@kernel.org>
- Invert conditional to reduce indentation.
- Rewrite while loop calling strtok(3) as a for loop. This allows
doing more simplification inside the loop (see next commit).
Signed-off-by: Alejandro Colomar <alx@kernel.org>
- Fix indentation. It was very broken.
- Move variable declaration to the top of the block in which it's used.
- Reduce use of whitespace and newlines.
Signed-off-by: Alejandro Colomar <alx@kernel.org>
- Invert conditional, to reduce indentation.
- Reduce use of whitespace and newlines while unindenting.
Signed-off-by: Alejandro Colomar <alx@kernel.org>
- Merge nested conditionals into a single if, to reduce indentation.
- Indent (1 SP) nested preprocessor conditionals.
- Reduce use of whitespace and newlines while unindenting.
Signed-off-by: Alejandro Colomar <alx@kernel.org>
- Use the temporary variable more, as it helps readability: it removes
a derefecence, which itself allows removing some parentheses.
- Use a shorter name, which is more common with temporaries, and so
there's less to read.
- Assign to *ranges at the end of the function. It's the same, but
with the other changes, I think this makes it slightly clearer.
Signed-off-by: Alejandro Colomar <alx@kernel.org>
If we consider simple objects as arrays of size 1, we can considerably
simplify these APIs, merging the *ARRAY and the non-array variants.
That will produce more readable code, since lines will be shorter (by
not having ARRAY in the macro names, as all macros will consistently
handle arrays), and the allocated size will be also more explicit.
The syntax will now be of the form:
p = MALLOC(42, foo_t); // allocate 42 elements of type foo_t.
p = MALLOC(1, bar_t); // allocate 1 element of type foo_t.
The _array() allocation functions should _never_ be called directly, and
instead these macros should be used.
The non-array functions (e.g., malloc(3)) still have their place, but
are limited to allocating structures with flexible array members. For
any other uses, the macros should be used.
Thus, we don't use any array or ARRAY variants in any code any more, and
they are only used as implementation details of these macros.
Link: <https://software.codidact.com/posts/285898/288023#answer-288023>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
alloca(3) fails silently if not enough memory can be allocated on the
stack. Use checked dynamic allocation instead.
Also drop unnecessary manual NUL assignment, ensured by snprintf(3).
Co-developed-by: Alejandro Colomar <alx@kernel.org>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
Allocate enough memory for the strings, two slashes and the NUL
terminator.
Reported-by: Alejandro Colomar <alx@kernel.org>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
Those comments were written when this function used 64 bits (and
temporary variables of 128 bits). Now it uses 32 bits, with temporaries
of 64 bits, so some values have changed.
Fixes: 2a61122b5e ("Unoptimize the higher part of the domain of csrand_uniform()")
Signed-off-by: Alejandro Colomar <alx@kernel.org>
This makes the function fit in less screens. This is to avoid consuming
more natural resources than we have available, and everyone knows the
supply of new-lines on a screen is not a renewable source[1].
Some transformations have been done thanks to free(NULL) being an alias
for loopity_loop(), as defined three comits ago. The real definition of
free(3) that everyone has been hiding is this:
void
free(void *p)
{
if (p == NULL)
loopity_loop();
else
real_free(p);
}
Link: [1] <https://www.kernel.org/doc/html/v6.3/process/coding-style.html#placing-braces-and-spaces>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
This was to a loop, as "1234" is to computer security.
No really; a loop that ends in a (forward) goto, and has no continue in it.
Still want a loop? Take two:
#define loopity_loop() do { for (;;) { break; } continue; } while (0-0)
Closes: <https://github.com/shadow-maint/shadow/issues/736>
Easter-egg: 8492dee663 ("subids: support nsswitch")
Signed-off-by: Alejandro Colomar <alx@kernel.org>
The permission also need to be checked before process_root_flag() since
that can chroot into non-selinux environment (unavailable selinux mount
point for example).
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
- Use SIZE_MAX rather than (size_t)-1, to improve readability.
- Move the only branch that breaks to the first place, so that we
remove an else. This reduces nesting while parsing the code.
- Now that we only have a 2-branch conditional where both branches
assign to the same variable, rewrite it as a ternary, to shorten.
Signed-off-by: Alejandro Colomar <alx@kernel.org>
The error handling is performed after the loop. By just calling break it
is possible to reuse the error handling if status is not ERANGE.
Signed-off-by: Samanta Navarro <ferivoz@riseup.net>
A failure of DUP_FUNCTION is already handled for non-reentrant
function wrapper. Perform the check for reentrant version as well.
Signed-off-by: Samanta Navarro <ferivoz@riseup.net>
On failure, these are meant to return 0 with errno set. But if
an nss module is loaded, they were returning -ERRNO instead.
Signed-off-by: Serge Hallyn <serge@hallyn.com>
- Don't else after return or fail_exit().
- Prefer == over != (negated logic is more complex to think about it).
- Reduce nesting when reasonable.
Signed-off-by: Alejandro Colomar <alx@kernel.org>
There's no reason to report all errors. Bail out at the first one,
which is simpler.
Suggested-by: Serge Hallyn <serge@hallyn.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
Some errors were being reported in stderr, but then they weren't really
being treated as errors.
If mkdir(2) for EEXIST, it's possible that the sysadmin pre-created the
user dir; don't fail. However, let's keep a log line, for having some
notice that it happened.
Also, run chmod(2) if mkdir(2) failed for EEXIST (so transform the
'else if' into an 'if').
Cc: Serge Hallyn <serge@hallyn.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
OpenSSH and coreutils' chroot call chroot first and then chdir. Doing it
this way is a bit safer because otherwise something could happen between
chdir and chroot to the specified path (like exchange of links) so the
working directory would not end up within the chroot environment.
This is a purely defensive measure.
Signed-off-by: Samanta Navarro <ferivoz@riseup.net>
This is no real world security fix.
The overflow could occur if too many layered subsystems are encountered
because the function check_perms calls itself recursively.
It would already take a misconfigured system for this to achieve it.
Use an iterative approach by calling the do_check_perms in a loop
instead of calling itself recursively.
As a side note: At least GCC 13 optimized this code and already uses
a jmp in its assembler code. I could only see the stack overflow by
activating address sanitizer which prevented the optimization.
Co-developed-by: Serge Hallyn <serge@hallyn.com>
Signed-off-by: Samanta Navarro <ferivoz@riseup.net>
If a user has home directory "/" and login shell "*" then login and su
enter an endless loop by constantly switching to the next subsystem.
This could also be achieved with a layered approach so just checking
for "/" as home directory is not enough to protect against such a
misconfiguration.
Just break the loop if it progressed too far. I doubt that this has
negative impact on any real setup.
Signed-off-by: Samanta Navarro <ferivoz@riseup.net>
If econf_getStringValue() fails, it will return an error and
set value to NULL. Look for the error and avoid dereferencing
value in that case.
Signed-off-by: Serge Hallyn <serge@hallyn.com>
The function is completely different based on USE_CONF. Either copy
will be easier to read if we just keep them completely separate.
Signed-off-by: Serge Hallyn <serge@hallyn.com>
You can see the memory leaks with address sanitizer if shadow is
compiled with `--enable-vendordir=/usr/etc`.
How to reproduce:
1. Prepare a custom shell file as root
```
mkdir -p /etc/shells.d
echo /bin/myshell > /etc/shells.d/custom
```
2. Run chsh as regular user
```
chsh
```
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
The getusershell implementation of musl returns every line within the
/etc/shells file, which even includes comments. Only consider absolute
paths for login shells.
Signed-off-by: Samanta Navarro <ferivoz@riseup.net>
Using --prefix in a setuid binary is quite dangerous. An unprivileged
user could prepare a custom shadow file in home directory. During a data
race the user could exchange directories with links which could lead to
exchange of shadow file in system's /etc directory.
This could be used for local privilege escalation.
Signed-off-by: Samanta Navarro <ferivoz@riseup.net>
Please find attached the french updated translation of shadow-man-page,
proofread by the debian-l10n-french mailing list contributors.
Signed-off-by: Serge Hallyn <serge@hallyn.com>
Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
Neither a pid_t below 1 nor a negative fd could be valid in this context.
Proof of Concept:
$ newuidmap -1 1 1 1
newuidmap: Could not open proc directory for target 4294967295
Signed-off-by: Samanta Navarro <ferivoz@riseup.net>
Some strings are first written into static char arrays before passed to
functions which expect a const char pointer anyway.
It is easier to pass these strings directly as arguments.
Signed-off-by: Samanta Navarro <ferivoz@riseup.net>
The fcntl call to set FD_CLOEXEC can be performed directly with the
previously performed open call by using the O_CLOEXEC flag.
O_CLOEXEC is required by POSIX.1-2008.
Signed-off-by: Samanta Navarro <ferivoz@riseup.net>
The only user of login_prompt is the login tool. This implies that the
first argument is always the same.
It is much easier to verify printf's format string and its argument if
both are next to each other.
Signed-off-by: Samanta Navarro <ferivoz@riseup.net>
Parsing optional environment variables after a login name is a feature
which is neither documented nor available in util-linux or busybox
login which are other wide spread login utilities used in Linux
distributions as reference.
Removing this feature resolves two issues:
- A memory leak exists if variables without an equal sign are used,
because set_env creates copies on its own. This could lead to OOM
situations in privileged part of login or may lead to heap spraying.
- Environment variables are not reset between login attempts. This
could lead to additional environment variables set for a user who
never intended to do so.
Proof of Concept on a system with shadow login without PAM and
util-linux agetty:
1. Provoke an invalid login, e.g. user `noone` and password `invalid`.
This starts shadow login and subsequent inputs are passed through
the function login_prompt.
2. Provoke an invalid login with environment variables, e.g.
user `noone HISTFILE=/tmp/owo` and password `invalid`.
3. Log in correctly with user `root`.
Now you can see with `echo $HISTFILE` that `/tmp/owo` has been set for
the root user.
This requires a malicious failed login attempt and a successful login
within the configured login timeout (default 60 seconds).
Signed-off-by: Samanta Navarro <ferivoz@riseup.net>
The getline function does not return a pointer but the amount of read
characters. The error return value to check for is -1.
Set buf to NULL to avoid dereference of an uninitialized stack value.
The getline function returns -1 if size argument is NULL. Always use
a valid pointer even if size is unimportant.
Signed-off-by: Samanta Navarro <ferivoz@riseup.net>
Add the relevant XKCD to the passwd(1) manual page. It already explains
most of the rationale behind this patch.
Add also reference to makepasswd(1), which is a good way to generate
strong passwords. Personally, I commonly run `makepasswd --chars 64` to
create my passwords, or 32 for passwords I need to type interactively
often.
The strength of a password is an exponential formula, where the base is
the size of the character set, and the exponent is the length of the
password. That already shows why long passwords of just lowercase
letters are better than short Pa$sw0rdZ3. But an even more important
point is that humans, when forced to use symbols in a password, are more
likely to do trivial substitutions on simple passwords, which doesn't
increase strength, and can instead give a false sense of strength, which
is dangerous.
Closes: <https://github.com/shadow-maint/shadow/issues/688>
Link: <https://xkcd.com/936/>
Cc: Mike Frysinger <vapier@gentoo.org>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
Destroying the handle does not actually disconnect, see [1].
Also free the key on user removal.
[1]: e9072e7d45/libsemanage/src/direct_api.c (L330)
Example adduser leak:
Direct leak of 1008 byte(s) in 14 object(s) allocated from:
#0 0x5638f2e782ae in __interceptor_malloc (./src/useradd+0xee2ae)
#1 0x7fb5cfffad09 in dbase_file_init src/database_file.c:170:45
Direct leak of 392 byte(s) in 7 object(s) allocated from:
#0 0x5638f2e782ae in __interceptor_malloc (./src/useradd+0xee2ae)
#1 0x7fb5cfffc929 in dbase_policydb_init src/database_policydb.c:187:27
Direct leak of 144 byte(s) in 2 object(s) allocated from:
#0 0x5638f2e782ae in __interceptor_malloc (./src/useradd+0xee2ae)
#1 0x7fb5cfffb519 in dbase_join_init src/database_join.c:249:28
[...]
Free the actual struct of the removed entry.
Example userdel report:
Direct leak of 40 byte(s) in 1 object(s) allocated from:
#0 0x55b230efe857 in reallocarray (./src/userdel+0xda857)
#1 0x55b230f6041f in mallocarray ./lib/./alloc.h:97:9
#2 0x55b230f6041f in commonio_open ./lib/commonio.c:563:7
#3 0x55b230f39098 in open_files ./src/userdel.c:555:6
#4 0x55b230f39098 in main ./src/userdel.c:1189:2
#5 0x7f9b48c64189 in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
run_parts currently exists in useradd and userdel, this commit mirrors
the functionality with groupadd and groupdel
Hook for group{add,del} to include killing processes that have group
membership that would no longer exist to avoid membership ID reuse.
getline(3) is much more readable than manually looping. It has some
overhead due to the allocation of a buffer, but that shouldn't be a
problem here. If that was a problem, we could reuse the buffer (thus
making the function non-reentrant), but I don't think that's worth the
extra complexity.
Using rpmatch(3) instead of a simple y/n test provides i18n to the
response checking. We have a fall-back minimalistic implementation for
systems that lack this function (e.g., musl libc).
While we're at it, apply some other minor improvements to this file:
- Remove comment saying which files use this function. That's likely
to get outdated. And anyway, it's just a grep(1) away, so it doesn't
really add any value.
- Remove unnecessary casts to (void) that were used to verbosely ignore
errors from stdio calls. They add clutter without really adding much
value to the code (or I don't see it).
- Remove comments from the function body. They make the function less
readable. Instead, centralize the description of the function into a
man-page-like comment before the function definition. This keeps the
function body short and sweet.
- Add '#include <stdbool.h>', which was missing.
- Minor whitespace style changes (it doesn't hurt the diff at this
point, since most of the affected lines were already touched by other
changes, so I applied my preferred style :).
Acked-by: Samanta Navarro <ferivoz@riseup.net>
Cc: Serge Hallyn <serge@hallyn.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
The tools newgrp and useradd expect waitpid to behave as described in
its manual page. But the notes indicate that if SIGCHLD is ignored,
waitpid behaves differently.
A user could set SIGCHLD to ignore before starting newgrp through exec.
Children of newgrp would not become zombies and their PIDs could be
reassigned before newgrp could call kill with the child pid and SIGCONT.
The useradd tool is not installed setuid, but I have added the default
there as well (copied from vipw).
Signed-off-by: Samanta Navarro <ferivoz@riseup.net>
Do not stop after 79 characters. Read the complete line to avoid
arbitrary limitations.
Proof of Concept:
```
cat > passwd-poc << EOF
root:x:0:0:root:/root:/bin/bash
root:x:0:0:root:/root:/bin/bash
root:x:0:0:root:/root:/bin/bash
EOF
python -c "print(80*'y')" | pwck passwd-poc
```
Two lines should still be within the file because we agreed only once
to remove a duplicated line.
Signed-off-by: Samanta Navarro <ferivoz@riseup.net>
Reviewed-by: Alejandro Colomar <alx@kernel.org>
Reviewed-by: Serge Hallyn <serge@hallyn.com>
If make fails in a multi-process invocation, the log is pretty much
unreadable. To make it readable, build as much as can be built without
failing. Then run a single-process make again. If we succeeded
previously, this should be a no-op. If not, this run will stop at the
first error, which should be more readable, and will only print the few
lines we're interested in.
This has some side effects: Now we build as much as we can, instead of
failing as early as possible; this may make CI a bit slower. However,
it also has the benefit that you see _all_ the error messages that could
be given, instead of needing to fix the first error to see the next and
so on.
Cc: Iker Pedrosa <ipedrosa@redhat.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
libbsd-devel libeconf-devel have already been added to the spec file and
they should be installed by the `dnf builddep` command.
Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
e5905c4b ("Added control character check") introduced checking for
control characters but had the logic inverted, so it rejects all
characters that are not control ones.
Cast the character to `unsigned char` before passing to the character
checking functions to avoid UB.
Use strpbrk(3) for the illegal character test and return early.
Both semanage and libsemanage actually set the user's mls range to the
default of the seuser, which makes more sense and removes a bit of code
for usermod and useradd. More fine-grained details must always be set
with some other tool
(semanage) anyway.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
The --gid option accepts a group name or id. When a name is provided, it
is resolved to an id by looking up the name in the group database
(/etc/group).
The --prefix option overides the location of the passwd and group
databases. I suspect the --gid option was overlooked when wiring up the
--prefix option.
useradd --gid already respects --prefix; this change makes usermod
behave the same way.
Fixes: b6b2c756c9
Signed-off-by: Mike Gilbert <floppym@gentoo.org>
This commit will serve to document why we shouldn't worry about the
truncation in the call to strlcpy(3). Since we have one more byte in
tmptty than in full_tty, truncation will produce a string that is at
least one byte longer than full_tty. Such a string could never compare
equal, so we're actually handling the truncation in a clever way. Maybe
too clever, but that's why I'm documenting it here.
Now, about the simplification itself:
Since we made sure that both full_tty and tmptty are null-terminated, we
can call strcmp(3) instead of strncmp(3). We can also simplify the
return logic avoiding one branch.
Cc: Paul Eggert <eggert@cs.ucla.edu>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com>
* libmisc/utmp.c (is_my_tty): Declare the parameter as a char array,
not char *, as it is not necessarily null-terminated.
Avoid a read overrun when reading 'tty', which comes from
'ut_utname'.
Reported-by: Paul Eggert <eggert@cs.ucla.edu>
Co-developed-by: Paul Eggert <eggert@cs.ucla.edu>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com>
* libmisc/date_to_str.c (date_to_str): Do not crash if gmtime(3)
returns NULL because the timestamp is far in the future.
Reported-by: Paul Eggert <eggert@cs.ucla.edu>
Co-developed-by: Paul Eggert <eggert@cs.ucla.edu>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com>
* lib/gshadow.c (sgetsgent): Use strcpy(3) not strlcpy(3),
since the string is known to fit.
Signed-off-by: Paul Eggert <eggert@cs.ucla.edu>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com>
* lib/fields.c (change_field): Don't point
before array start; that has undefined behavior.
Signed-off-by: Paul Eggert <eggert@cs.ucla.edu>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com>
* lib/fields.c (change_field): Since we know the string fits,
use strcpy(3) rather than strlcpy(3).
Signed-off-by: Paul Eggert <eggert@cs.ucla.edu>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com>
gettime.c:25:30: warning: a function declaration without a prototype is deprecated in all versions of C [-Wstrict-prototypes]
/*@observer@*/time_t gettime ()
^
void
shellcheck warns against using echo with flags, as posix sh won't
support it. It suggests using printf, so let's do that.
Signed-off-by: Serge Hallyn <serge@hallyn.com>
bc github...
For some reason, the first test - ONLY on github - seems to not
give the '$ ' prompt expected when you spawn 'su testsuite'.
So just run the first test twice, and ignore the first failure.
It messes with the expected results.
We can do better than this in the expect scripts, but let's
get things running for now.
Signed-off-by: Serge Hallyn <serge@hallyn.com>
write_mapping() will do the following:
openat(proc_dir_fd, map_file, O_WRONLY);
An attacker could create a directory containing a symlink named
"uid_map" pointing to any file owned by root, and thus allow him to
overwrite any root-owned file.
Closes#635
newuidmap and newgidmap currently take an integner pid as
the first argument, determining the process id on which to
act. Accept also "fd:N", where N must be an open file
descriptor to the /proc/pid directory for the process to
act upon. This way, if you
exec 10</proc/99
newuidmap fd:10 100000 0 65536
and pid 99 dies and a new process happens to take pid 99 before
newuidmap happens to do its work, then since newuidmap will use
openat() using fd 10, it won't change the mapping for the new
process.
Example:
// terminal 1:
serge@jerom ~/src/nsexec$ ./nsexec -W -s 0 -S 0 -U
about to unshare with 10000000
Press any key to exec (I am 129176)
// terminal 2:
serge@jerom ~/src/shadow$ exec 10</proc/129176
serge@jerom ~/src/shadow$ sudo chown root src/newuidmap src/newgidmap
serge@jerom ~/src/shadow$ sudo chmod u+s src/newuidmap
serge@jerom ~/src/shadow$ sudo chmod u+s src/newgidmap
serge@jerom ~/src/shadow$ ./src/newuidmap fd:10 0 100000 10
serge@jerom ~/src/shadow$ ./src/newgidmap fd:10 0 100000 10
// Terminal 1:
uid=0(root) gid=0(root) groups=0(root)
Signed-off-by: Serge Hallyn <serge@hallyn.com>
We can't use a pointer that was input to realloc(3), nor any pointers
that point to reallocated memory, without making sure that the memory
wasn't moved. If we do, the Behavior is Undefined.
Signed-off-by: Alejandro Colomar <alx@kernel.org>
Use of these macros, apart from the benefits mentioned in the commit
that adds the macros, has some other good side effects:
- Consistency in getting the size of the object from sizeof(type),
instead of a mix of sizeof(type) sometimes and sizeof(*p) other
times.
- More readable code: no casts, and no sizeof(), so also shorter lines
that we don't need to cut.
- Consistency in using array allocation calls for allocations of arrays
of objects, even when the object size is 1.
Cc: Valentin V. Bartenev <vbartenev@gmail.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
This macros have several benefits over the standard functions:
- The type of the allocated object (not the pointer) is specified as an
argument, which improves readability:
- It is directly obvious what is the type of the object just by
reading the macro call.
- It allows grepping for all allocations of a given type.
This is admittedly similar to using sizeof() to get the size of the
object, but we'll see why this is better.
- In the case of reallocation macros, an extra check is performed to
make sure that the previous pointer was compatible with the allocated
type, which can avoid some mistakes.
- The cast is performed automatically, with a pointer type derived from
the type of the object. This is the best point of this macro, since
it does an automatic cast, where there's no chance of typos.
Usually, programmers have to decide whether to cast or not the result
of malloc(3). Casts usually hide warnings, so are to be avoided.
However, these functions already return a void *, so a cast doesn't
really add much danger. Moreover, a cast can even add warnings in
this exceptional case, if the type of the cast is different than the
type of the assigned pointer. Performing a manual cast is still not
perfect, since there are chances that a mistake will be done, and
even ignoring accidents, they clutter code, hurting readability.
And now we have a cast that is synced with sizeof.
- Whenever the type of the object changes, since we perform an explicit
cast to the old type, there will be a warning due to type mismatch in
the assignment, so we'll be able to see all lines that are affected
by such a change. This is especially important, since changing the
type of a variable and missing to update an allocation call far away
from the declaration is easy, and the consequences can be quite bad.
Cc: Valentin V. Bartenev <vbartenev@gmail.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
We'll expand the contents in a following commit, so let's move the file
to a more generic name, have a dedicated header, and update includes.
Signed-off-by: Alejandro Colomar <alx@kernel.org>
Use the new header for xstrdup()
Signed-off-by: Alejandro Colomar <alx@kernel.org>
This is guaranteed by ISO C. Now that we require ISO C (and even POSIX)
to compile, we can simplify this code.
Signed-off-by: Alejandro Colomar <alx@kernel.org>
On 2/19/23 18:09, David Mudrich wrote:
> I am working on a RAM based Linux OS from source, and try to use
> latest versions of all software. I found shadow needs libbsd's
> readpassphrase(3) as superior alternative to getpass(3). While
> considering if I a) include libbsd, or include libbsd's code of
> readpassphrase(3) into shadow, found, that libbsd's readpassphrase(3)
> never returns \n or \r
> <https://cgit.freedesktop.org/libbsd/tree/src/readpassphrase.c>
> line 122, while agetpass() uses a check for \n in agetpass.c line 108.
> I assume it always fails.
Indeed, it always failed. I made a mistake when writing agetpass(),
assuming that readpassphrase(3) would keep newlines.
>
> I propose a check of len == PASS_MAX - 1, with false positive error for
> exactly PASS_MAX - 1 long passwords.
Instead, I added an extra byte to the allocation to allow a maximum
password length of PASS_MAX (which is the maximum for getpass(3), which
we're replacing.
While doing that, I notice that my previous implementation also had
another bug (minor): The maximum password length was PASS_MAX - 1
instead of PASS_MAX. That's also fixed in this commit.
Reported-by: David Mudrich <dmudrich@gmx.de>
Fixes: 155c9421b9 ("libmisc: agetpass(), erase_pass(): Add functions for getting passwords safely")
Cc: Iker Pedrosa <ipedrosa@redhat.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
Some programs don't support `(uint16_t) -1` or `(uint32_t) -1` as user
or group IDs. This is because `-1` is used as an error code or as an
unspecified ID, e.g. in `chown(2)` parameters, and in the past, `gid_t`
and `uid_t` have changed width. For legacy reasons, those values have
been kept reserved in programs today (for example systemd does this; see
the documentation in the link below).
This should not be confused with catching overflow in the ID values,
since that is already caught by our ERANGE checks. This is about not
using reserved values that have been reserved for legacy reasons.
Link: <https://systemd.io/UIDS-GIDS/>
Reviewed-by: Alejandro Colomar <alx@kernel.org>
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
These comments should indicate which functions they really wrap.
An alternative would be to remove the line completely to avoid
future copy&paste mistakes.
Signed-off-by: Samanta Navarro <ferivoz@riseup.net>
This function simplifies the calculation of the bounds of the buffer for
catenating strings. It would also reduce error checking, but we don't
care about truncation in this specific code. :)
Signed-off-by: Alejandro Colomar <alx@kernel.org>
strncat(3), strlcpy(3), and many other functions are often misused for
catenating strings, when they should never be used for that. strlcat(3)
is good. However, there's no equivalent to strlcat(3) similar to
snprintf(3). Let's add stpecpy(), which is similar to strlcat(3), but
it is also the only function compatible with stpeprintf(), which makes
it more useful than strlcat(3).
Signed-off-by: Alejandro Colomar <alx@kernel.org>
All the string-copying functions called above do terminate the strings
they create with a NUL byte. Writing it again at the end of the buffer
is unnecessary paranoid code. Let's remove it.
Signed-off-by: Alejandro Colomar <alx@kernel.org>
This function allows reducing error checking (since errors are
propagated across chained calls), and also simplifies the calculation of
the start and end of the buffer where the string should be written.
Moreover, the new code is more optimized, since many calls to strlen(3)
have been removed.
Signed-off-by: Alejandro Colomar <alx@kernel.org>
[v]stpeprintf() are similar to [v]snprintf(3), but they allow chaining.
[v]snprintf(3) are very dangerous for catenating strings, since the
obvious ways to do it invoke Undefined Behavior, and the ways that avoid
UB are very error-prone.
Cc: Iker Pedrosa <ipedrosa@redhat.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
When trying to build shadow in a different directory I stumbled upon few
issues, this commit aims to fix all of them:
- The `subid.h` file is generated and hence in the build directory and
not in the source directory, so use `$(builddir)` instead of
`$(srcdir)`.
- Using `$<` instead of filenames utilises autotools to locate the files
in either the source or build directory automatically.
- `xsltproc` needs to access the files in login.defs.d in either the
source directory or the symlink in a language subdirectory, but it
does not interpret the `--path` as prefix of the entity path, but
rather a path under which to locate the basename of the entity
from the XML file. So specify the whole path to login.defs.d.
- The above point could be used to make the symlinks of login.defs.d
and entity path specifications in the XMLs obsolete, but I trying
not to propose possibly disrupting patches, so for the sake of
simplicity just specify `$(srcdir)` when creating the symlink.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
The intention of the code is just to not report an error message when
'typefile' doesn't exist. If we call access(2) and then fopen(2),
there's a race. It's not a huge problem, and the worst thing that can
happen is reporting an error when the file has been removed after
access(2). It's not a problem, but we can fix the race and at the same
time clarify the intention of not warning about ENOENT and also remove
one syscall. Seems like a win-win.
Suggested-by: Christian Göttsche <cgzones@googlemail.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
- Every non-const pointer converts automatically to void *.
- Every pointer converts automatically to void *.
- void * converts to any other pointer.
- const void * converts to any other const pointer.
- Integer variables convert to each other.
I changed the declaration of a few variables in order to allow removing
a cast.
However, I didn't attempt to edit casts inside comparisons, since they
are very delicate. I also kept casts in variadic functions, since they
are necessary, and in allocation functions, because I have other plans
for them.
I also changed a few casts to int that are better as ptrdiff_t.
This change has triggered some warnings about const correctness issues,
which have also been fixed in this patch (see for example src/login.c).
Signed-off-by: Alejandro Colomar <alx@kernel.org>
We could use the standard (C11) _Noreturn qualifier, but it will be
deprecated in C23, and replaced by C++'s [[noreturn]], which is
compatible with the GCC attribute, so let's directly use the attribute,
and in the future we'll be able to switch to [[]].
Signed-off-by: Alejandro Colomar <alx@kernel.org>
Recently, we removed support for 'struct utmpx'. We did it because utmp
and utmpx are identical, and while POSIX specifies utmpx (and not utmp),
GNU/Linux documentation seems to favor utmp. Also, this project
defaulted to utmp, so changing to utmpx would be more dangerous than
keeping old defaults, even if it's supposed to be the same.
Now, I just found more code that didn't make much sense: lib/utent.c
provides definitions for getutent(3) and friends in case the system
doesn't provide them, but we don't provide prototypes for those
definitions, so code using the functions would have never compiled.
Let's just remove these definitions as dead code.
Fixes: 3be7b9d75a ("Remove traces of utmpx")
Fixes: 170b76cdd1 ("Disable utmpx permanently")
Cc: Serge Hallyn <serge@hallyn.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
Closes#457
The existing prose was confusing, or simply wrong. Make it clear
that only the group ownership of the tty is affected, and how.
Also move the paragraph about defaults after the discussion of
acceptable TTYGROUPs, as this seems more natural.
Signed-off-by: Serge Hallyn <serge@hallyn.com>
In variadic functions we still do the cast. In POSIX, it's not
necessary, since NULL is required to be of type 'void *', and 'void *'
is guaranteed to have the same alignment and representation as 'char *'.
However, since ISO C still doesn't mandate that, and moreover they're
doing dubious stuff by adding nullptr, let's be on the cautious side.
Also, C++ requires that NULL is _not_ 'void *', but either plain 0 or
some magic stuff.
Signed-off-by: Alejandro Colomar <alx@kernel.org>
If lines start with '\0' then it is possible to trigger out of
boundary accesses.
Check if indices are valid before accessing them.
Signed-off-by: Samanta Navarro <ferivoz@riseup.net>
If the file referenced by ENV_TZ has a zero length string, then an out
of boundary write occurs. Also the result can be wrong because it is
assumed that the file will always end with a newline.
Only override a newline character with '\0' to avoid these cases.
This cannot be considered to be security relevant because login.defs
and its contained references to system files should be trusted to begin
with.
Proof of Concept:
1. Compile shadow's su with address sanitizer and --without-libpam
2. Setup your /etc/login.defs to contain ENV_TZ=/etc/tzname
3. Prepare /etc/tzname to contain a '\0' byte at the beginning
`python -c "print('\x00')" > /etc/tzname`
4. Use su
`su -l`
You can see the following output:
`tz.c:45:8: runtime error: index 18446744073709551615 out of bounds for type 'char [8192]'`
Signed-off-by: Samanta Navarro <ferivoz@riseup.net>
We do need the unoptimized version of csrand_uniform() for high values
of `n`, since the optimized version depends on having __int128, and it's
not available on several platforms, including ARMv7, IA32, and MK68k.
This reverts commit 848f53c1d3c1362c86d3baab6906e1e4419d2634; however,
I applied some tweaks to the reverted commit.
Reported-by: Adam Sampson <ats@offog.org>
Cc: Iker Pedrosa <ipedrosa@redhat.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
Now that we optimized csrand_uniform(), we don't need these functions.
This reverts commit 7c8fe291b1260e127c10562bfd7616961013730f.
Signed-off-by: Alejandro Colomar <alx@kernel.org>
Use a different algorithm to minimize rejection. This is essentially
the same algorithm implemented in the Linux kernel for
__get_random_u32_below(), but written in a more readable way, and
avoiding microopimizations that make it less readable.
Which (the Linux kernel implementation) is itself based on Daniel
Lemire's algorithm from "Fast Random Integer Generation in an Interval",
linked below. However, I couldn't really understand that paper very
much, so I had to reconstruct the proofs from scratch, just from what I
could understand from the Linux kernel implementation source code.
I constructed some graphical explanation of how it works, and why it
is optimal, because I needed to visualize it to understand it. It is
published in the GitHub pull request linked below.
Here goes a wordy explanation of why this algorithm based on
multiplication is better optimized than my original implementation based
on masking.
masking:
It discards the extra bits of entropy that are not necessary for
this operation. This works as if dividing the entire space of
possible csrand() values into smaller spaces of a size that is
a smaller power of 2. Each of those smaller spaces has a
rejection band, so we get as many rejection bands as spaces
there are. For smaller values of 'n', the size of each
rejection band is smaller, but having more rejection bands
compensates for this, and results in the same inefficiency as
for large values of 'n'.
multiplication:
It divides the entire space of possible random numbers in
chunks of size exactly 'n', so that there is only one rejection
band that is the remainder of `2^64 % n`. The worst case is
still similar to the masking algorithm, a rejection band that is
almost half the entire space (n = 2^63 + 1), but for lower
values of 'n', by only having one small rejection band, it is
much faster than the masking algorithm.
This algorithm, however, has one caveat: the implementation
is harder to read, since it relies on several bitwise tricky
operations to perform operations like `2^64 % n`, `mult % 2^64`,
and `mult / 2^64`. And those operations are different depending
on the number of bits of the maximum possible random number
generated by the function. This means that while this algorithm
could also be applied to get uniform random numbers in the range
[0, n-1] quickly from a function like rand(3), which only
produces 31 bits of (non-CS) random numbers, it would need to be
implemented differently. However, that's not a concern for us,
it's just a note so that nobody picks this code and expects it
to just work with rand(3) (which BTW I tried for testing it, and
got a bit confused until I realized this).
Finally, here's some light testing of this implementation, just to know
that I didn't goof it. I pasted this function into a standalone
program, and run it many times to find if it has any bias (I tested also
to see how many iterations it performs, and it's also almost always 1,
but that test is big enough to not paste it here).
int main(int argc, char *argv[])
{
printf("%lu\n", csrand_uniform(atoi(argv[1])));
}
$ seq 1 1000 | while read _; do ./a.out 3; done | grep 1 | wc -l
341
$ seq 1 1000 | while read _; do ./a.out 3; done | grep 1 | wc -l
339
$ seq 1 1000 | while read _; do ./a.out 3; done | grep 1 | wc -l
338
$ seq 1 1000 | while read _; do ./a.out 3; done | grep 2 | wc -l
336
$ seq 1 1000 | while read _; do ./a.out 3; done | grep 2 | wc -l
328
$ seq 1 1000 | while read _; do ./a.out 3; done | grep 2 | wc -l
335
$ seq 1 1000 | while read _; do ./a.out 3; done | grep 0 | wc -l
332
$ seq 1 1000 | while read _; do ./a.out 3; done | grep 0 | wc -l
331
$ seq 1 1000 | while read _; do ./a.out 3; done | grep 0 | wc -l
327
This isn't a complete test for a cryptographically-secure random number
generator, of course, but I leave that for interested parties.
Link: <https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=e9a688bcb19348862afe30d7c85bc37c4c293471>
Link: <https://github.com/shadow-maint/shadow/pull/624#discussion_r1059574358>
Link: <https://arxiv.org/abs/1805.10941>
Cc: "Jason A. Donenfeld" <Jason@zx2c4.com>
Cc: Cristian Rodríguez <crrodriguez@opensuse.org>
Cc: Adhemerval Zanella <adhemerval.zanella@linaro.org>
Cc: Björn Esser <besser82@fedoraproject.org>
Cc: Yann Droneaud <ydroneaud@opteya.com>
Cc: Joseph Myers <joseph@codesourcery.com>
Cc: Sam James <sam@gentoo.org>
Cc: Serge Hallyn <serge@hallyn.com>
Cc: Iker Pedrosa <ipedrosa@redhat.com>
[Daniel Lemire: Added link to research paper in source code]
Cc: Daniel Lemire <daniel@lemire.me>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
It is common to use the expression 'sizeof(x) * CHAR_BIT' to mean the
width in bits of a type or object. Now that there are _WIDTH macros for
some types, indicating the number of bits that they use, it makes sense
to wrap this calculation in a macro of a similar name.
Signed-off-by: Alejandro Colomar <alx@kernel.org>
This API is similar to arc4random_uniform(3). However, for an input of
0, this function is equivalent to csrand(), while arc4random_uniform(0)
returns 0.
This function will be used to reimplement csrand_interval() as a wrapper
around this one.
The current implementation of csrand_interval() doesn't produce very good
random numbers. It has a bias. And that comes from performing some
unnecessary floating-point calculations that overcomplicate the problem.
Looping until the random number hits within bounds is unbiased, and
truncating unwanted bits makes the overhead of the loop very small.
We could reduce loop overhead even more, by keeping unused bits of the
random number, if the width of the mask is not greater than
ULONG_WIDTH/2, however, that complicates the code considerably, and I
prefer to be a bit slower but have simple code.
BTW, Björn really deserves the copyright for csrand() (previously known
as read_random_bytes()), since he rewrote it almost from scratch last
year, and I kept most of its contents. Since he didn't put himself in
the copyright back then, and BSD-3-Clause doesn't allow me to attribute
derived works, I won't add his name, but if he asks, he should be put in
the copyright too.
Cc: "Jason A. Donenfeld" <Jason@zx2c4.com>
Cc: Cristian Rodríguez <crrodriguez@opensuse.org>
Cc: Adhemerval Zanella <adhemerval.zanella@linaro.org>
Cc: Björn Esser <besser82@fedoraproject.org>
Cc: Yann Droneaud <ydroneaud@opteya.com>
Cc: Joseph Myers <joseph@codesourcery.com>
Cc: Sam James <sam@gentoo.org>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
These functions implement bit manipulation APIs, which will be added to
C23, so that in the far future, we will be able to replace our functions
by the standard ones, just by adding the stdc_ prefix, and including
<stdbit.h>.
However, we need to avoid UB for an input of 0, so slightly deviate from
C23, and use a different name (with _wrap) for distunguishing our API
from the standard one.
Cc: Joseph Myers <joseph@codesourcery.com>
Cc: Yann Droneaud <ydroneaud@opteya.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
arc4random(3) returns a number.
arc4random_buf(3) fills a buffer.
arc4random_uniform(3) returns a number less than a bound.
and I'd add a hypothetical one which we use:
*_interval() should return a number within the interval [min, max].
In reality, the function being called csrand() in this patch is not
really cryptographically secure, since it had a bias, but a subsequent
patch will fix that.
Signed-off-by: Alejandro Colomar <alx@kernel.org>
We were always casting the result to u_long. Better just use that type
in the function. Since we're returning u_long, it makes sense to also
specify the input as u_long. In fact, that'll help for doing bitwise
operations inside this function.
Signed-off-by: Alejandro Colomar <alx@kernel.org>
I have plans to split this function in smaller functions that implement
bits of this functionallity, to simplify the implementation. So, let's
use names that distinguish them.
This one produces a number within an interval, so make that clear. Also
make clear that the function produces cryptographically-secure numbers.
Signed-off-by: Alejandro Colomar <alx@kernel.org>
strlcpy(3) might not be visible since it is declared in <bsd/string.h>.
This can lead to warnings, like:
fields.c: In function 'change_field':
fields.c:103:17: warning: implicit declaration of function 'strlcpy'; did you mean 'strncpy'? [-Wimplicit-function-declaration]
103 | strlcpy (buf, cp, maxsize);
| ^~~~~~~
| strncpy
../lib/fields.c:103:17: warning: type of 'strlcpy' does not match original declaration [-Wlto-type-mismatch]
103 | strlcpy (buf, cp, maxsize);
| ^
/usr/include/bsd/string.h:44:8: note: return value type mismatch
44 | size_t strlcpy(char *dst, const char *src, size_t siz);
| ^
/usr/include/bsd/string.h:44:8: note: type 'size_t' should match type 'int'
/usr/include/bsd/string.h:44:8: note: 'strlcpy' was previously declared here
/usr/include/bsd/string.h:44:8: note: code may be misoptimized unless '-fno-strict-aliasing' is used
Comparisons if different signedness can result in unexpected results.
Add casts to ensure operants are of the same type.
gettime.c: In function 'gettime':
gettime.c:58:26: warning: comparison of integer expressions of different signedness: 'long long unsigned int' and 'time_t' {aka 'long int'} [-Wsign-compare]
58 | } else if (epoch > fallback) {
| ^
Cast to time_t, since epoch is less than ULONG_MAX at this point.
idmapping.c: In function 'write_mapping':
idmapping.c:202:48: warning: comparison of integer expressions of different signedness: 'int' and 'long unsigned int' [-Wsign-compare]
202 | if ((written <= 0) || (written >= (bufsize - (pos - buf)))) {
| ^~
newgidmap.c: In function ‘main’:
newgidmap.c:178:40: warning: comparison of integer expressions of different signedness: ‘int’ and ‘long unsigned int’ [-Wsign-compare]
178 | if ((written <= 0) || (written >= sizeof(proc_dir_name))) {
| ^~
newuidmap.c: In function ‘main’:
newuidmap.c:107:40: warning: comparison of integer expressions of different signedness: ‘int’ and ‘long unsigned int’ [-Wsign-compare]
107 | if ((written <= 0) || (written >= sizeof(proc_dir_name))) {
| ^~
Instead of using volatile pointers to prevent the compiler from
optimizing the call away, use a memory barrier.
This requires support for embedded assembly, which should be fine after
the recent requirement bumps.
memset_s() has a different signature than memset(3) or explicit_bzero(),
thus the current code would not compile. Also memset_s()
implementations are quite rare.
Use the C23 standardized version memset_explicit(3).
Fixes: 7a799ebb ("Ensure memory cleaning")
Login timed out message prints only first few bytes when write is immediately followed by exit.
Calling exit from new handler provides enough time to display full message.
Commit 90424e7c ("Don't warn when failed to open /etc/nsswitch.conf")
removed the logging for failing to read /etc/nsswitch.conf to reduce the
noise in the case the file does not exists (e.g. musl based systems).
Reintroduce a warning if /etc/nsswitch.conf exists but we failed to read
it (e.g. permission denied).
Improves: 90424e7c ("Don't warn when failed to open /etc/nsswitch.conf")
gethostbyname(3) was removed in POSIX.1-2008. It has been obsoleted,
and replaced by getaddrinfo(3), which is superior in several ways:
- gethostbyname(3) is not reentrant. There's a GNU extension,
gethostbyname_r(3) which is reentrant, but it's not likely to be
standardized for the following reason. And we don't care too much
about this point either.
- gethostbyname(3) only supports IPv4, but getaddrinfo(3) supports both
IPv4 and IPv6 (and may support other address families in the future).
We don't care about reentrancy, so for keeping the code simple (i.e.,
not touch call site to add code to free(3) an allocated buffer), I added
a static buffer for inet_ntop(3). We could address that in the future,
but I don't think it's worth it.
BTW, we also replace inet_ntoa(3) by inet_ntop(3), as a consequence of
using getaddrinfo(3). inet_ntoa(3) is also marked as deprecated, but
that deprecation seems to have been documented only in the manual page,
and POSIX doesn't mark it as deprecated. The deprecation notice goes
back to when the inet_ntop(3) manual page was added by Sam Varshavchik
to the Linux man-pages in version 1.30 (year 2000).
So, this, apart from updating the code to POSIX.1-2008, is also adding
support for IPv6 :) Although, probably many other parts of the code are
written for IPv4 only, so I wouldn't yet claim support for it.
A few notes:
- I didn't check the return value of inet_ntop(3), since it can't fail
for the given input:
- EAFNOSUPPORT: We only call it with AF_INET and AF_INET6.
- ENOSPC: We calculate the size of the buffer to be wide enough:
MAX(INET_ADDRSTRLEN, INET6_ADDRSTRLEN) so it always fits.
Cc: Dave Hagewood <admin@arrowweb.com>
Cc: Sam Varshavchik
Cc: Jakub Jelinek <jakub@redhat.com>
Cc: Iker Pedrosa <ipedrosa@redhat.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
Systems can suffer power interruptions whilst .lock files are in /etc,
preventing scripts and other automation tools from updating shadow's
files which persist across boots.
This commit replaces that mechanism with file locking to avoid problems
of power interruption/crashing.
Minor tweak to groupmems man page, requested by 'xx' on IRC.
Signed-off-by: ed neville <ed@s5h.net>
When the caller may not change the room number, work phone, or
home number, then rather than prompting for the new one it will
print the existing one. But due to a typo it printed the full name
in place of each of those.
Fix the fields being printed.
Signed-off-by: Serge Hallyn <serge@hallyn.com>
- Since strncpy(3) is not designed to write strings, but rather
(null-padded) character sequences (a.k.a. unterminated strings), we
had to manually append a '\0'. strlcpy(3) creates strings, so they
are always terminated. This removes dependencies between lines, and
also removes chances of accidents.
- Repurposing strncpy(3) to create strings requires calculating the
location of the terminating null byte, which involves a '-1'
calculation. This is a source of off-by-one bugs. The new code has
no '-1' calculations, so there's almost-zero chance of these bugs.
- strlcpy(3) doesn't padd with null bytes. Padding is relevant when
writing fixed-width buffers to binary files, when interfacing certain
APIs (I believe utmpx requires null padding at lease in some
systems), or when sending them to other processes or through the
network. This is not the case, so padding is effectively ignored.
- strlcpy(3) requires that the input string is really a string;
otherwise it crashes (SIGSEGV). Let's check if the input strings are
really strings:
- lib/fields.c:
- 'cp' was assigned from 'newft', and 'newft' comes from fgets(3).
- lib/gshadow.c:
- strlen(string) is calculated a few lines above.
- libmisc/console.c:
- 'cons' comes from getdef_str, which is a bit cryptic, but seems
to generate strings, I guess.1
- libmisc/date_to_str.c:
- It receives a string literal. :)
- libmisc/utmp.c:
- 'tname' comes from ttyname(3), which returns a string.
- src/su.c:
- 'tmp_name' has been passed to strcmp(3) a few lines above.
Signed-off-by: Alejandro Colomar <alx@kernel.org>
Since the project is supposed to be POSIX.1-2001 compliant it doesn't
make sense to have that added conditionally.
Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
It is Undefined Behavior to declare errno (see NOTES in its manual page).
Instead of using the errno dummy declaration, use one that doesn't need
a comment.
Signed-off-by: Alejandro Colomar <alx@kernel.org>
Previous commits, to keep readability of the diffs, left the code that
was previously wrapped by preprocessor coditionals untouched. Apply
some minor cosmetic changes to merge it in the surrounding code.
Signed-off-by: Alejandro Colomar <alx@kernel.org>
- USER_NAME_MAX_LENGTH was being calculated in terms of utmpx. Do it
in terms of utmp.
- Remove utmpx support from the whishlist.
- Remove unused tests about utmpx members.
Signed-off-by: Alejandro Colomar <alx@kernel.org>
On Linux, utmpx and utmp are identical. However, documentation (manual
pages) covers utmp, and just says about utmpx that it's identical to
utmp. It seems that it's preferred to use utmp, at least by reading the
manual pages.
Moreover, we were defaulting to utmp (utmpx had to be explicitly enabled
at configuration time). So, it seems safer to just make it permanent,
which should not affect default builds.
Signed-off-by: Alejandro Colomar <alx@kernel.org>
We already made that assumption in commit b47aa1e9aa. While the
header is not required by POSIX (it is an XSI extension), it is defined
in systems that are of interest to this project (GNU/Linux).
Fixes: b47aa1e9aa ("Assume <utmpx.h> exists")
Cc: Serge Hallyn <serge@hallyn.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
When useradd sends its ADD_USER event, it is filling in the id field. This is not yet written to disk. When auditd sees the event and the log format is enriched, auditd tries to lookup the user name but it does not exist. This causes the event to never be resolvable since ausearch relies on the lookup information attached by auditd.
The fix is to not send the id information for any event until after close_files() is called. Just the acct field is all that is
Patch by Steve Grubb (afaik).
Reported at https://bugzilla.redhat.com/show_bug.cgi?id=1713432
The OSes that are referred to by these comments, are extinct, but
their comments survived, fossilized in amber.
Reported-by: Iker Pedrosa <ipedrosa@redhat.com>
Cc: Christian Göttsche <cgzones@googlemail.com>
Cc: Mike Frysinger <vapier@gentoo.org>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
In a previous commit, we made USE_TERMIOS unconditionally defined.
Let's just remove it, and remove the condition everywhere.
Reported-by: Iker Pedrosa <ipedrosa@redhat.com>
Cc: Christian Göttsche <cgzones@googlemail.com>
Cc: Mike Frysinger <vapier@gentoo.org>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
All of the macros we're using are required by POSIX.1-2001.
Cc: Christian Göttsche <cgzones@googlemail.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
The function is obsolete. It is recommended to use getrlimit(2) instead
(see the manual page for ulimit(3) or the POSIX manual for it). Since
getrlimit(2) is required by POSIX.1-2001, we can rely on it.
Cc: Christian Göttsche <cgzones@googlemail.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
Clang doesn't implement this attribute and reports an error. Work
around it by hiding it in a macro that will be empty in clang.
Reported-by: Christian Göttsche <cgzones@googlemail.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
There are several issues with getpass(3).
Many implementations of it share the same issues that the infamous
gets(3). In glibc it's not so terrible, since it's a wrapper
around getline(3). But it still has an important bug:
If the password is long enough, getline(3) will realloc(3) memory,
and prefixes of the password will be laying around in some
deallocated memory.
See the getpass(3) manual page for more details, and especially
the commit that marked it as deprecated, which links to a long
discussion in the linux-man@ mailing list.
So, readpassphrase(3bsd) is preferrable, which is provided by
libbsd on GNU systems. However, using readpassphrase(3) directly
is a bit verbose, so we can write our own wrapper with a simpler
interface similar to that of getpass(3).
One of the benefits of writing our own interface around
readpassphrase(3) is that we can hide there any checks that should
be done always and which would be error-prone to repeat every
time. For example, check that there was no truncation in the
password.
Also, use malloc(3) to get the buffer, instead of using a global
buffer. We're not using a multithreaded program (and it wouldn't
make sense to do so), but it's nice to know that the visibility of
our passwords is as limited as possible.
erase_pass() is a clean-up function that handles all clean-up
correctly, including zeroing the entire buffer, and then
free(3)ing the memory. By using [[gnu::malloc(erase_pass)]], we
make sure that we don't leak the buffers in any case, since the
compiler will be able to enforce clean up.
Link: <https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/commit?id=7ca189099d73bde954eed2d7fc21732bcc8ddc6b>
Reported-by: Christian Göttsche <cgzones@googlemail.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
The missing #include <gshadow.h> causes the configure check to fail
spuriously, resulting in HAVE_SHADOWGRP not being defined even
on systems that actually have sgetsgent (such as current glibc).
Allow supplementary groups to be set via the /etc/default/useradd config
file. Allowing an administrator to set additonal groups via the GROUPS
configurable and control the default behaviour of useradd.
This program has 10 calls to gets(3) according to grep(1). That
makes it a very unsafe program which should not be used at all.
Let's kill the program already.
See what gets(3) has to say:
SYNOPSIS
#include <stdio.h>
[[deprecated]] char *gets(char *s);
DESCRIPTION
Never use this function.
...
BUGS
Never use gets(). Because it is impossible to tell with‐
out knowing the data in advance how many characters
gets() will read, and because gets() will continue to
store characters past the end of the buffer, it is ex‐
tremely dangerous to use. It has been used to break com‐
puter security. Use fgets() instead.
For more information, see CWE‐242 (aka "Use of Inherently
Dangerous Function") at http://cwe.mitre.org/data/defini‐
tions/242.html
Acked-by: "Serge E. Hallyn" <serge@hallyn.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
The minimum id allocation for system accounts shouldn't be 0 as this is
reserved for root.
Signed-off-by: Tomáš Mráz <tm@t8m.info>
Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
Report error if usermod asked for moving homedir and it does not exist.
Signed-off-by: Tomáš Mráz <tm@t8m.info>
Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
Introduced by c6c8130db4
After removing snprintf, the format string should get unescaped once.
Fixes#564
Reporter and patch author: DerMouse (github.com/DerMouse)
glibc, musl, FreeBSD, and OpenBSD define the MAX() and MIN()
macros in <sys/param.h> with the same definition that we use.
Let's not redefine it here and use the system one, as it's
effectively the same as we define (modulo whitespace).
See:
shadow (previously):
alx@asus5775:~/src/shadow/shadow$ grepc -ktm MAX
./lib/defines.h:318:#define MAX(x,y) (((x) > (y)) ? (x) : (y))
glibc:
alx@asus5775:~/src/gnu/glibc$ grepc -ktm -x 'sys/param.h$' MAX
./misc/sys/param.h:103:#define MAX(a,b) (((a)>(b))?(a):(b))
musl:
alx@asus5775:~/src/musl/musl$ grepc -ktm -x 'sys/param.h$' MAX
./include/sys/param.h:19:#define MAX(a,b) (((a)>(b))?(a):(b))
OpenBSD:
alx@asus5775:~/src/bsd/openbsd/src$ grepc -ktm -x 'sys/param.h$' MAX
./sys/sys/param.h:193:#define MAX(a,b) (((a)>(b))?(a):(b))
FreeBSD:
alx@asus5775:~/src/bsd/freebsd/freebsd-src$ grepc -ktm -x 'sys/param.h$' MAX
./sys/sys/param.h:333:#define MAX(a,b) (((a)>(b))?(a):(b))
Signed-off-by: Alejandro Colomar <alx@kernel.org>
free(3) accepts NULL, since the oldest ISO C. I guess the
paranoid code was taking care of prehistoric implementations of
free(3). I've never known of an implementation that doesn't
conform to this, so let's simplify this.
Remove xfree(3), which was effectively an equivalent of free(3).
Signed-off-by: Alejandro Colomar <alx@kernel.org>
This tweaks the database locking logic so that failures in the
link-checking paths are more detailed.
The rationale for this is that I've experienced a non-deterministic
bug which seems to be coming from this logic, and I'd like to get
more details about the actual failing condition.
The setuid, setgid, and sticky bits are not copied during copy_tree.
Also start with very restrictive permissions before setting ownerships.
This prevents situations in which users in a group with less permissions
than others could win a race in opening the file before permissions are
removed again.
Proof of concept:
$ echo $HOME
/home/uwu
$ install -o uwu -g fandom -m 604 /dev/null /home/uwu/owo
$ ls -l /home/uwu/owo
-rw----r-- 1 uwu fandom 0 Sep 4 00:00 /home/uwu/owo
If /tmp is on another filesystem, then "usermod -md /tmp/uwu uwu" leads
to this temporary situation:
$ ls -l /tmp/uwu/owo
-rw----r-- 1 root root 0 Sep 4 00:00 /tmp/uwu/owo
This means that between openat and chownat_if_needed a user of group
fandom could open /tmp/uwu/owo and read the content when it is finally
written into the file.
Fixes regression introduced in faeab50e71.
If a directory contains fifos, then openat blocks until the other side
of the fifo is connected as well.
This means that users can prevent "usermod -m" from completing if their
home directories contain at least one fifo.
The groupadd from shadow does not allow upper case group names, the
same is true for the upstream shadow. But distributions like
Debian/Ubuntu/CentOS has their own way to cope with this problem,
this patch is picked up from Fedora [1] to relax the usernames
restrictions to allow the upper case group names, and the relaxation is
POSIX compliant because POSIX indicate that usernames are composed of
characters from the portable filename character set [A-Za-z0-9._-].
[1] https://src.fedoraproject.org/rpms/shadow-utils/blob/rawhide/f/shadow-4.8-goodname.patch
Signed-off-by: Alexander Kanavin <alex@linutronix.de>
useradd does not create the files if they don't exist, but if they exist
it will reset user data even if the data did not exist before creating
a hole and an explicitly zero'd data point resulting (especially for
high UIDs) in a lot of zeros ending up in containers and tarballs.
As rbalint points out, this was an exported fn. It also is
the only way for a libsubid user to do what it does, so let's
not drop it.
This reverts commit 477c8e6f42.
Use *at() functions to pin the directory operating in to avoid being
redirected by unprivileged users replacing parts of paths by symlinks to
privileged files.
Introduce a path_info struct with the full path and dirfd and name
information for *at() functions, since the full path is needed for link
resolution, SELinux label lookup and ACL attributes.
Use *at() functions to pin the directory operating in to avoid being
redirected by unprivileged users replacing parts of paths by symlinks to
privileged files.
Use *at() functions to pin the directory operating in to avoid being
redirected by unprivileged users replacing parts of paths by symlinks to
privileged files.
Allow the compiler to verify the format string against the supplied
arguments.
chage.c:239:51: warning: format not a string literal, format string not checked [-Wformat-nonliteral]
239 | (void) strftime (buf, sizeof buf, format, tp);
| ^~~~~~
salt.c:102:22: warning: type qualifiers ignored on function return type [-Wignored-qualifiers]
102 | static /*@observer@*/const unsigned long SHA_get_salt_rounds (/*@null@*/int *prefered_rounds);
| ^~~~~
salt.c:110:22: warning: type qualifiers ignored on function return type [-Wignored-qualifiers]
110 | static /*@observer@*/const unsigned long YESCRYPT_get_salt_cost (/*@null@*/int *prefered_cost);
| ^~~~~
subordinateio.c:160:8: warning: type qualifiers ignored on function return type [-Wignored-qualifiers]
160 | static const bool range_exists(struct commonio_db *db, const char *owner)
| ^~~~~
Compilers are free to ignore the indented hint and modern optimizations
should create good code by themself.
(As such it is for example deprecated in C++17.)
Version 1.19.1 was released in June 2014.
configure.ac:697: warning: AM_PROG_MKDIR_P: this macro is deprecated, and will soon be removed.
configure.ac:697: You should use the Autoconf-provided 'AC_PROG_MKDIR_P' macro instead,
configure.ac:697: and use '$(MKDIR_P)' instead of '$(mkdir_p)'in your Makefile.am files.
./lib/autoconf/general.m4:2434: AC_DIAGNOSE is expanded from...
aclocal.m4:780: AM_PROG_MKDIR_P is expanded from...
m4/po.m4:23: AM_PO_SUBDIRS is expanded from...
m4/gettext.m4:57: AM_GNU_GETTEXT is expanded from...
configure.ac:697: the top level
configure.ac:697: warning: The macro `AC_TRY_LINK' is obsolete.
configure.ac:697: You should run autoupdate.
./lib/autoconf/general.m4:2920: AC_TRY_LINK is expanded from...
lib/m4sugar/m4sh.m4:692: _AS_IF_ELSE is expanded from...
lib/m4sugar/m4sh.m4:699: AS_IF is expanded from...
./lib/autoconf/general.m4:2249: AC_CACHE_VAL is expanded from...
./lib/autoconf/general.m4:2270: AC_CACHE_CHECK is expanded from...
m4/gettext.m4:365: gt_INTL_MACOSX is expanded from...
m4/gettext.m4:57: AM_GNU_GETTEXT is expanded from...
configure.ac:697: the top level
configure.ac:697: warning: The macro `AC_TRY_LINK' is obsolete.
configure.ac:697: You should run autoupdate.
./lib/autoconf/general.m4:2920: AC_TRY_LINK is expanded from...
lib/m4sugar/m4sh.m4:692: _AS_IF_ELSE is expanded from...
lib/m4sugar/m4sh.m4:699: AS_IF is expanded from...
./lib/autoconf/general.m4:2249: AC_CACHE_VAL is expanded from...
./lib/autoconf/general.m4:2270: AC_CACHE_CHECK is expanded from...
m4/gettext.m4:57: AM_GNU_GETTEXT is expanded from...
configure.ac:697: the top level
configure.ac:697: warning: The macro `AC_TRY_LINK' is obsolete.
configure.ac:697: You should run autoupdate.
./lib/autoconf/general.m4:2920: AC_TRY_LINK is expanded from...
lib/m4sugar/m4sh.m4:692: _AS_IF_ELSE is expanded from...
lib/m4sugar/m4sh.m4:699: AS_IF is expanded from...
./lib/autoconf/general.m4:2249: AC_CACHE_VAL is expanded from...
./lib/autoconf/general.m4:2270: AC_CACHE_CHECK is expanded from...
m4/iconv.m4:20: AM_ICONV_LINK is expanded from...
m4/gettext.m4:57: AM_GNU_GETTEXT is expanded from...
configure.ac:697: the top level
See https://www.gnu.org/software/libtool/manual/html_node/LT_005fINIT.html
configure.ac:25: warning: The macro `AM_ENABLE_STATIC' is obsolete.
configure.ac:25: You should run autoupdate.
m4/ltoptions.m4:259: AM_ENABLE_STATIC is expanded from...
configure.ac:25: the top level
configure.ac:26: warning: The macro `AM_ENABLE_SHARED' is obsolete.
configure.ac:26: You should run autoupdate.
m4/ltoptions.m4:205: AM_ENABLE_SHARED is expanded from...
configure.ac:26: the top level
It was hard to extend the option specification string passed to
getopt_long as the third argument.
The origian code had a branch with WITH_SELINUX ifdef condition. If
one wants to add one more option char with another ifdef condition
like ENABLE_SUBIDS to the spec, the one must enumerate the specs for
all combinations of the conditions:
* WITH_SELINUX && ENABLE_SUBIDS
* WITH_SELINUX && !ENABLE_SUBIDS
* !WITH_SELINUX && ENABLE_SUBIDS
* !WITH_SELINUX && !ENABLE_SUBIDS
With this change, you can append an option char to the spec.
Signed-off-by: Masatake YAMATO <yamato@redhat.com>
On systems with Linux kernel < 3.17, getentropy() and getrandom() may
exist but return ENOSYS. Use /dev/urandom as a fallback to avoid a hard
requirement on Linux kernel version.
Fixes#512.
Signed-off-by: Xi Ruoyao <xry111@xry111.site>
In order to remove some of the FIXMEs it was necessary to change the
code and call getulong() instead of getlong().
Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
No need to redeclare a variable with the same name and type. Just keep
the one with the biggest scope.
Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
"egrep" is an obsolete alias for grep -E and newer greps will warn on usage
of egrep, so let's just swap it out.
Signed-off-by: Sam James <sam@gentoo.org>
Git wants to ensure that you do not read .git owned by other users.
But we fetch+build as 'build' user, and run tests as root user. Those
tests calculate git topdir using git rev-parse --show-toplevel, which
git now fails.
Setting safe.directory, seems wrong. Let's just use bash to figure
out the top dir.
Generating salt value depends on /dev/urandom. But after the
function process_root_flag changed the root directory, It does
not exist.
So, generate salt value before changeing the directory.
Fixes: #514
The markdown output for the maintainers, authors and contributors list
was wrapped in a single line and it was difficult to read. I've created
an unordered list to get a better output. On top of that I've also added
myself as a maintainer.
Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
This used to be 16 for historical reasons but these days basically every
distro configures --with-group-name-max-length=32 to make it match the
max Linux username length, make it default.
Signed-off-by: Jami Kettunen <jami.kettunen@protonmail.com>
C++ requires extern "C" linkage specification to call functions from a C
library. Enclose the function definitions in subid.h in an extern "C"
block if compiling in C++ mode to achieve this.
Signed-off-by: Alois Wohlschlager <alois1@gmx-topmail.de>
useradd warns that a system user ID less than SYS_UID_MIN is outside the
expected range, even though that ID has been specifically selected with
the "-u" option.
In my opinion all the user ID's below SYS_UID_MAX are for the system,
thus I change the condition to take that into account.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2004911
Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
If /etc/nsswitch.conf doesn't exist podman crashes because shadow_logfd
is NULL. In order to avoid that load the log file descriptor with the
log_get_logfd() helper function.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2038811
Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
C89 and POSIX.1-2001 define signal(2) as returning a pointer to a
function returning 'void'. K&R C signal(2) signature is obsolete.
Use 'void' directly.
Also, instead of writing the function pointer type explicitly, use
POSIX's 'sighandler_t'.
Signed-off-by: Alejandro Colomar <alx.manpages@gmail.com>
Systems on which <sys/time.h> conflicted with <time.h> are obsolete.
This macro has been marked as obsolete by autoconf documentation.
Signed-off-by: Alejandro Colomar <alx.manpages@gmail.com>
All current compilers support C89's 'const' keyword.
Autoconf declares this macro as obsolescent.
Signed-off-by: Alejandro Colomar <alx.manpages@gmail.com>
As autoconf documentation says, this macro is obsolescent, as no
current systems have the bug in S_ISDIR, S_ISREG, etc..
The affected systems were Tektronix UTekV, Amdahl UTS, and
Motorola System V/88.
Signed-off-by: Alejandro Colomar <alx.manpages@gmail.com>
INTERACTIVE Systems Corporation Unix is no longer sold, and Sun
said (long ago) that it would drop support for it on 2006-07-23.
So this macro has been obsolete for more than a decade.
Signed-off-by: Alejandro Colomar <alx.manpages@gmail.com>
'mode_t' is defined by POSIX.1-2001 in <sys/types.h>.
It's unlikely to be missing.
See mode_t(3).
Signed-off-by: Alejandro Colomar <alx.manpages@gmail.com>
'pid_t' is defined by POSIX.1-2001 in <sys/types.h>.
It's unlikely to be missing.
See pid_t(3).
Signed-off-by: Alejandro Colomar <alx.manpages@gmail.com>
'off_t' is defined by POSIX.1-2001 in <sys/types.h>.
It's unlikely to be missing.
See off_t(3).
Signed-off-by: Alejandro Colomar <alx.manpages@gmail.com>
'uid_t' is defined by POSIX.1-2001 in <sys/types.h>.
It's unlikely to be missing.
See uid_t(3).
Signed-off-by: Alejandro Colomar <alx.manpages@gmail.com>
The macro HAVE_RUSEROK is not being used anywhere.
As the Linux manual page says, ruserok(3) is present on the BDSs, Solaris, and many other systems. This function appeared in 4.2BSD. So we probably can rely on its existence.
Signed-off-by: Alejandro Colomar <alx.manpages@gmail.com>
The macro HAVE_GETADDRINFO is not being used anywhere.
BTW, the function is defined by POSIX.1-2001 and RFC 2553, so it's likely that it is always available.
Signed-off-by: Alejandro Colomar <alx.manpages@gmail.com>
The macro HAVE_SIGACTION is not being used anywhere.
BTW, the function is defined by SVr4 and POSIX.1-2001, so it's likely that it is always available.
Signed-off-by: Alejandro Colomar <alx.manpages@gmail.com>
The macro HAVE_GETTIMEOFDAY is not being used anywhere.
BTW, the function is defined by SVr4, 4.3BSD, and POSIX.1-2001, so
it's likely that it is always available.
POSIX.1-2008 marks it as obsolete, but only because
clock_gettime(2) provides more precission. Since gettimeofday(3)
is in use by many big projects, and it has no obvious dangers,
it's likely that it will continue to exist even if it's outside of
the POSIX standard.
Signed-off-by: Alejandro Colomar <alx.manpages@gmail.com>
The macro HAVE_GETHOSTNAME is not being used anywhere.
BTW, the function is defined by SVr4, 4.4BSD, and POSIX.1-2001, so
it's likely that it is always available.
Signed-off-by: Alejandro Colomar <alx.manpages@gmail.com>
The only place where the check was used was removed in 4e1afcd66.
BTW, it was unnecessary, since strchr(3) is defined by:
POSIX.1-2001, C89, SVr4, and 4.3BSD. Enough to rely on it.
Signed-off-by: Alejandro Colomar <alx.manpages@gmail.com>
GNU autoconf documentation marks this macro as obsolescent, as
current systems are compatible with POSIX.
Simplify code to unconditionally include <sys/wait.h>, and don't
redefine WIFEXITSTATUS() and WIFEXITED(), since they are mandated
by POSIX.
Signed-off-by: Alejandro Colomar <alx.manpages@gmail.com>
POSIX.1-2001 defines 'struct dirent' in <dirent.h>. It replaces
the old 'struct direct' found in BSDs. All of the systems that I
checked (including FreeBSD, NetBSD, and OpenBSD), now provide
<dirent.h> with 'struct dirent', as mandated by POSIX.
Since autoconf first checks <dirent.h> and only if it's missing it
checks other header files, it's clear that it will always find
<dirent.h>, so let's simplify.
GNU autoconf documentation declares this macro as obsolescent, and
acknowledges that all current systems with directory libraries
have <dirent.h>:
<https://www.gnu.org/software/autoconf/manual/autoconf-2.70/html_node/Particular-Headers.html>
Signed-off-by: Alejandro Colomar <alx.manpages@gmail.com>
Compilers are allowed to and do optimize memset(3) calls away for
pointers not accessed in the future. Since the memzero wrappers purpose
is exactly to unconditionally override memory (e.g. for stored
passwords) do not implement via regular memset(3), but via either
memset_s(3), explicit_bzero(3) or a hand written implementation using
volatile pointers.
See https://wiki.sei.cmu.edu/confluence/display/c/MSC06-C.+Beware+of+compiler+optimizations
run_part() and run_parts() do not modify their directory, name and
action arguments.
Also include the header in the implementation to provide the prototypes.
useradd.c:2495:59: warning: cast discards ‘const’ qualifier from pointer target type [-Wcast-qual]
2495 | if (run_parts ("/etc/shadow-maint/useradd-pre.d", (char*)user_name,
| ^
useradd.c:2495:24: warning: passing argument 1 of ‘run_parts’ discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
2495 | if (run_parts ("/etc/shadow-maint/useradd-pre.d", (char*)user_name,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from useradd.c:45:
../lib/run_part.h:2:22: note: expected ‘char *’ but argument is of type ‘const char *’
2 | int run_parts (char *directory, char *name, char *action);
| ~~~~~~^~~~~~~~~
useradd.c:2496:25: warning: passing argument 3 of ‘run_parts’ discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
2496 | "useradd")) {
| ^~~~~~~~~
nss_init() does not modify its path argument, thus declare it const.
Also drop superfluous prototype.
nss.c:54:31: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
54 | nsswitch_path = NSSWITCH;
| ^
Function declarations with no argument declare functions taking an
arbitrary number of arguments. Use the special type void to declare
functions taking no argument.
C89 defined isdigit as a function that tests for any decimal-digit
character, defining the decimal digits as 0 1 2 3 4 5 6 7 8 9.
I don't own a copy of C89 to check, but check in C17:
7.4.1.5
5.2.1
More specifically:
> In both the source and execution basic character sets, the value
> of each character after 0 in the above list of decimal digits
> shall be one greater than the value of the previous.
And since in ascii(7), the character after '9' is ':', it's highly
unlikely that any implementation will ever accept any
_decimal digit_ other than 0..9.
POSIX simply defers to the ISO C standard.
This is exactly what we wanted from ISDIGIT(c), so just use it.
Non-standard implementations might have been slower or considered
other characters as digits in the past, but let's assume
implementations available today conform to ISO C89.
Signed-off-by: Alejandro Colomar <alx.manpages@gmail.com>
It wasn't being used at all. Let's remove it.
Use isdigit(3) directly in comments that referenced it.
Also, in those comments, remove an outdated reference to the fact
that ISDIGIT_LOCALE(c) might evaluate its argument more than once,
which could be true a few commits ago, until
IN_CTYPE_DEFINITION(c) was removed. Previously, the definition
for ISDIGIT_LOCALE(c) was:
#if defined (STDC_HEADERS) || (!defined (isascii) && !defined (HAVE_ISASCII))
# define IN_CTYPE_DOMAIN(c) 1
#else
# define IN_CTYPE_DOMAIN(c) isascii(c)
#endif
#define ISDIGIT_LOCALE(c) (IN_CTYPE_DOMAIN (c) && isdigit (c))
Which could evaluate 'c' twice on pre-C89 systems (which I hope
don't exist nowadays).
Signed-off-by: Alejandro Colomar <alx.manpages@gmail.com>
Due to the recent removal of IN_CTYPE_DOMAIN(), the uppercase
macros that wrapped these standard calls are now defined to be
equivalent. Therefore, there's no need for the wrappers, and it
is much more readable to use the standard calls directly.
However, hold on with ISDIGIT*(), since it's not so obvious what
to do with it.
Signed-off-by: Alejandro Colomar <alx.manpages@gmail.com>
The recent removal of STDC_HEADERS made IN_CTYPE_DOMAIN be defined
to 1 unconditionally. Remove the now unnecessary definition, and
propagate its truthness to expressions where it was used.
Signed-off-by: Alejandro Colomar <alx.manpages@gmail.com>
We're in 2021. C89 is everywhere; in fact, there are many other
assumptions in the code that wouldn't probably hold on
pre-standard C environments. Let's simplify and assume that C89
is available.
The specific assumptions are that:
- <string.h>, and <stdlib.h> are available
- strchr(3), strrchr(3), and strtok(3) are available
- isalpha(3), isspace(3), isdigit(3), and isupper(3) are available
I think we can safely assume we have all of those.
Signed-off-by: Alejandro Colomar <alx.manpages@gmail.com>
memset(3) has been in standard C since C89. It is also in
POSIX.1-2001, in SVr4, and in 4.3BSD (see memset(3) and memset(3p)).
We can assume that this function is always available.
Signed-off-by: Alejandro Colomar <alx.manpages@gmail.com>
memcpy(3) has been in standard C since C89. It is also in
POSIX.1-2001, in SVr4, and in 4.3BSD (see memcpy(3) and memcpy(3p)).
We can assume that this function is always available.
Signed-off-by: Alejandro Colomar <alx.manpages@gmail.com>
getgroups(2) has been in POSIX since POSIX.1-2001. It is also in
in SVr4 and in 4.3BSD (see getgroups(2) and getgroups(3p)).
We can assume that this function is always available.
Signed-off-by: Alejandro Colomar <alx.manpages@gmail.com>
strftime(3) has been in standard C since C89. It is also in
POSIX.1-2001, and in SVr4 (see strftime(3) and strftime(3p)).
We can assume that this function is always available.
Signed-off-by: Alejandro Colomar <alx.manpages@gmail.com>
sleep 2s before running newxidmap - it seems we were sometimes
racing, causing newxidmap to fail.
Make sure to remove /tmp/test-xidmap, for some reason they
were sometimes still there, causing test to fail.
Fix some irregular tabbing.
Signed-off-by: Serge Hallyn <serge@hallyn.com>
PARAMETERS:
According to the C2x charter, I reordered the parameters 'size'
and 'buf' from previously existing date_to_str() definitions.
C2x charter:
> 15. Application Programming Interfaces (APIs) should be
> self-documenting when possible. In particular, the order of
> parameters in function declarations should be arranged such that
> the size of an array appears before the array. The purpose is to
> allow Variable-Length Array (VLA) notation to be used. This not
> only makes the code's purpose clearer to human readers, but also
> makes static analysis easier. Any new APIs added to the Standard
> should take this into consideration.
I used 'long' for the date parameter, as some uses of the function
need to pass a negative value meaning "never".
FUNCTION BODY:
I didn't check '#ifdef HAVE_STRFTIME', which old definitions did,
since strftime(3) is guaranteed by the C89 standard, and all of
the conversion specifiers that we use are also specified by that
standard, so we don't need any extensions at all.
Signed-off-by: Alejandro Colomar <alx.manpages@gmail.com>
The build was failing with duplicate symbol errors with -fno-common.
This is the default in GCC 10 and later, and explicitly enabled in some
distributions to catch problems like this. There were two causes:
- Prog and shadow_logfd were defined in a header file that was included
in multiple other files. Fix this by defining them once in
shadowlog.c, and having extern declarations in the header.
- Most of the tools (except id/nologin) also define a Prog variable,
which is not intended to alias the one in the library. Fix
this by renaming Prog in the library to shadow_progname, which also
matches the new accessor functions for it.
We were overriding this when --enable-shared was passed. We can actually
just dump the conditional logic as libtool will do the right thing for
us here anyway.
Without this patch, libsubid is installed as .0.
Signed-off-by: Sam James <sam@gentoo.org>
The SIGCHLD handler could have been ignored by parent process.
Make sure that we have default handling activated.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
This conforms to PAM documentation and it is needed to support
ambient capabilities with PAM + libcap-2.58+.
Signed-off-by: Björn Fischer <bf@CeBiTec.Uni-Bielefeld.DE>
Rename libsubid symbols to all be prefixed with subid_.
Don't export anything but the subid_*.
Closes#443
Signed-off-by: Serge Hallyn <serge@hallyn.com>
* Change to markdown format
* Include an introduction
* Remove the commit mailing list from the contacts
* Add the IRC channel to the contacts
* Move 'S/Key' section to doc/README.skey
* Move authors and maintainers to AUTHORS.md
Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
Add an additional NULL check condition in spw_free() and pw_free() to
avoid freeing an already empty pointer.
Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
Rename list_subid_ranges to getsubids to provide a system binary to
check the sub*ids of a user. The intention is to provide this binary
with any distribution that includes the subid feature, so that system
administrators can check the subid ranges of a given user.
Finally, add a man page to explain the behaviour of getsubids.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1980780
Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
Always set SIGCHLD handler to default, even if the caller of vipw has
set SIGCHLD to ignore. If SIGCHLD is ignored no zombie processes would
be created, which in turn could mean that kill is called with an already
recycled pid.
Proof of Concept:
1. Compile nochld:
--
#include <signal.h>
#include <unistd.h>
int main(void) {
char *argv[] = { "vipw", NULL };
signal(SIGCHLD, SIG_IGN);
execvp("vipw", argv);
return 1;
}
--
2. Run nochld
3. Suspend child vi, which suspends vipw too:
`kill -STOP childpid`
4. Kill vi:
`kill -9 childpid`
5. You can see with ps that childpid is no zombie but disappeared
6. Bring vipw back into foreground
`fg`
The kill call sends SIGCONT to "childpid" which in turn could have been
already recycled for another process.
This is definitely not a vulnerability. It would take super user
operations, at which point an attacker would have already elevated
permissions.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
Change SELinux labels for files copied from the skeleton directory to
the home directory.
This could cause gnome's graphical user adding to fail without copying
the full skeleton files.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2022658
Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
When using groupdel with a prefix, groupdel will attempt to read a
passwd file to look for any user in the group. When the file does not
exist it cores with segmentation fault.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1986111
If a line in hushlogins file, e.g. /etc/hushlogins, starts with
'\0', then current code performs an out of boundary write.
If the line lacks a newline at the end, then another character is
overridden.
With strcspn both cases are solved.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
During shadowtcb_move() the directory is temporarily changed to be
owned by root:root with permissions 0700. After the change is done,
the ownership and permissions were supposed to be restored. The
call for chown() was there, but the chmod() call was missing. This
resulted in the broken TCB functionality. The added chmod() fixes
the issue.
Close the selabel handle to update the file_context. This means that the
file_context will be remmaped and used by selabel_lookup() to return
the appropriate context to label the home folder.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1993081
Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
Create the home and mail folders after the SELinux user has been set for
the added user. This will allow the folders to be created with the
SELinux user label.
Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
The buggy code was introduced nearly 5 years ago at the
commit 08fd4b69e8. The
desired behavior is that SIGKILL will be sent to the
child if it does not exit within 2 seconds after it
receives SIGTERM. However, SIGALRM is masked while
waiting for the child so it cannot wake the program
up after 2 seconds to send SIGKILL.
An example shows the buggy behavior, which exists in
Ubuntu 18.04 LTS (with login 1:4.5-1ubuntu2).
```bash
user1@localhost:~$ su user2 -c '
_term() {
echo SIGTERM received
}
trap _term TERM
while true; do
sleep 1
echo still alive
done'
Password:
still alive
Session terminated, terminating shell...Terminated
SIGTERM received
still alive
still alive
still alive
still alive
```
(SIGTERM is sent in another user1's terminal by
executing `killall su`.)
Here is the desired behavior, which shows what the
commit fixes.
```bash
user1@localhost:~$ su user2 -c '
_term() {
echo SIGTERM received
}
trap _term TERM
while true; do
sleep 1
echo still alive
done'
Password:
still alive
Session terminated, terminating shell...Terminated
SIGTERM received
still alive
still alive
...killed.
user1@localhost:~$ echo $?
255
```
In some circumstances I want the default behaviour of useradd to
not add user entries to the lastlog and faillog databases. Allowing
this options behaviour to be controlled by the config file
/etc/default/useradd.
`sgent` is only initialized in `get_group()` if `is_shadowgrp` is true.
So we should also only attempt to free it if this is actually the case.
Can otherwise lead to:
```
free() double free detected in tcache 2 (gpasswd)
```
The GitHub and Debian permanently moved to HTTPS URLs and redirect
there. The Gentoo URL does not redirect to HTTPS, but still use it to
address certain kinds of attacks. Lastly, the NetBSD URL is only
available using HTTP.
subuid_count won't get used by usr_update(), but since we're passing it
as an argument we have to make sure it's always defined. So just define
it as pre-set to 0.
Closes#402
Signed-off-by: Serge Hallyn <serge@hallyn.com>
xml2po is deprecated. We've previously replaced xml2po with
itstool in man/generate_translations.mak, but there was still
an instance of it that only is exercised for 'make dist'.
Update that one. Now 'make dist' succeeds on a ubuntu focal
or newer host where xml2po is not available.
Signed-off-by: Serge Hallyn <serge@hallyn.com>
If SHA_CRYPT_MIN_ROUNDS and SHA_CRYPT_MAX_ROUNDS are both unspecified,
use SHA_ROUNDS_DEFAULT.
Previously, the code fell through, calling shadow_random(-1, -1). This
ultimately set rounds = (unsigned long) -1, which ends up being a very
large number! This then got capped to SHA_ROUNDS_MAX later in the
function.
The new behavior matches BCRYPT_get_salt_rounds().
Bug: https://bugs.gentoo.org/808195
Fixes: https://github.com/shadow-maint/shadow/issues/393
useradd generates an empty subid range when adding a new user. This is
caused because there are two variables, one local and the other one
global, that have a very similar name and they are used indistinctly in
the code. The local variable loads the SUB_*ID_COUNT configuration from
the login.defs file, while the global variable, which holds a value of
0, is used to generate the subid range. Causing the empty subid range
problem.
I've merged the two variables in the local one and removed the global
variable. I prefer to do it this way to reduce the scope of it but I'm
open to doing it the other way round.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1990653
Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
When the -S and -a options are used for passwd to list the status
of all passwords, there is a chance the pw_passwd field of struct
passwd will be NULL. This can be due to 'files compat' being set
for passwd in /etc/nsswitch.conf and the usage of some features
not available in the 'files' mode (e.g. a plus sign at the start
of a line).
Example:
germ161:~ # grep passwd /etc/nsswitch.conf
passwd: files compat
germ161:~ # rpm -qa shadow
shadow-4.2.1-34.20.x86_64
germ161:~ # grep passwd /etc/nsswitch.conf
passwd: files compat
germ161:~ # grep + /etc/passwd
+@nisgroup
germ161:~ # passwd -S -a > /dev/null
Segmentation fault (core dumped)
With this commit:
germ161:~ # passwd -S -a > /dev/null
passwd: malformed password data obtained for user +@nisgroup
The only way of removing a group from the supplementary list is to use
-G option, and list all groups that the user is a member of except for
the one that wants to be removed. The problem lies when there's a user
that contains both local and remote groups, and the group to be removed
is a local one. As we need to include the remote group with -G option
the command will fail.
This reverts commit 140510de9d. This way,
it would be possible to remove the remote groups from the supplementary
list.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1967641
Resolves: https://github.com/shadow-maint/shadow/issues/338
Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
new*idmap has a dependency with libeconf since commit
c464ec5570. I'm just adding it to the
Makefile to be able to compile in distributions that include libeconf.
Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
Since bbf4b79, we stopped shipping /etc/default/useradd, and therefore
install of shadow does not auto-create /etc/default. So when useradd
tries to save a new default, it needs to create the directory.
Closes#390.
Signed-off-by: Serge Hallyn <serge@hallyn.com>
libsubid's Makefile.am was always setting enable-shared in its LDFLAGS.
Do that only if not building static.
Closes#387
Signed-off-by: Serge Hallyn <shallyn@cisco.com>
There's a better way to do this, and I hope to clean that up,
but this fixes out of tree builds for me right now.
Closes#386
Signed-off-by: Serge Hallyn <serge@hallyn.com>
In 9eb191edc4 I included a free() that
frees the members variable, which in turn causes the comma_to_list()
function to return an array of empty elements. The array variable holds
a list of pointers that point to offsets of the members variable. When
the function succeeds freeing members variable causes the elements of
the array variable to point to an empty string.
This is causing several regressions in our internal testing environment.
So, I'm reverting the change.
Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
Most Linux distributions, including Fedora and RHEL 8, are shipping
with libxcrypt >= 4.0.
Since that version of libxcrypt the provided family of crypt_gensalt()
functions are able to use automatic entropy drawn from secure system
ressources, like arc4random(), getentropy() or getrandom().
Anyways, the settings generated by crypt_gensalt() are always
guaranteed to works with the crypt() function.
Using crypt_gensalt() is also needed to make proper use of newer
hashing methods, like yescrypt, provided by libxcrypt.
Signed-off-by: Björn Esser <besser82@fedoraproject.org>
The functions crypt(3), crypt_gensalt(3), and their
feature test macros may be defined in there.
Signed-off-by: Björn Esser <besser82@fedoraproject.org>
In a previous commit we introduced /dev/urandom as a source to obtain
random bytes from. This may not be available on all systems, or when
operating inside of a chroot.
Almost all systems provide functions to obtain random bytes from
secure system ressources. Thus we should prefer to use these, and
fall back to /dev/urandom, if there is no such function present, as
a last resort.
Signed-off-by: Björn Esser <besser82@fedoraproject.org>
Error: RESOURCE_LEAK (CWE-772): [#def1]
shadow-4.8.1/lib/commonio.c:320: alloc_fn: Storage is returned from allocation function "fopen_set_perms".
shadow-4.8.1/lib/commonio.c:320: var_assign: Assigning: "bkfp" = storage returned from "fopen_set_perms(backup, "w", &sb)".
shadow-4.8.1/lib/commonio.c:329: noescape: Resource "bkfp" is not freed or pointed-to in "putc".
shadow-4.8.1/lib/commonio.c:334: noescape: Resource "bkfp" is not freed or pointed-to in "fflush".
shadow-4.8.1/lib/commonio.c:339: noescape: Resource "bkfp" is not freed or pointed-to in "fileno".
shadow-4.8.1/lib/commonio.c:342: leaked_storage: Variable "bkfp" going out of scope leaks the storage it points to.
340| || (fclose (bkfp) != 0)) {
341| /* FIXME: unlink the backup file? */
342|-> return -1;
343| }
344|
Error: RESOURCE_LEAK (CWE-772): [#def2]
shadow-4.8.1/libmisc/addgrps.c:69: alloc_fn: Storage is returned from allocation function "malloc".
shadow-4.8.1/libmisc/addgrps.c:69: var_assign: Assigning: "grouplist" = storage returned from "malloc(i * 4UL)".
shadow-4.8.1/libmisc/addgrps.c:73: noescape: Resource "grouplist" is not freed or pointed-to in "getgroups". [Note: The source code implementation of the function has been overridden by a builtin model.]
shadow-4.8.1/libmisc/addgrps.c:126: leaked_storage: Variable "grouplist" going out of scope leaks the storage it points to.
124| }
125|
126|-> return 0;
127| }
128| #else /* HAVE_SETGROUPS && !USE_PAM */
Error: RESOURCE_LEAK (CWE-772): [#def3]
shadow-4.8.1/libmisc/chowntty.c:62: alloc_fn: Storage is returned from allocation function "getgr_nam_gid".
shadow-4.8.1/libmisc/chowntty.c:62: var_assign: Assigning: "grent" = storage returned from "getgr_nam_gid(getdef_str("TTYGROUP"))".
shadow-4.8.1/libmisc/chowntty.c:98: leaked_storage: Variable "grent" going out of scope leaks the storage it points to.
96| */
97| #endif
98|-> }
99|
Error: RESOURCE_LEAK (CWE-772): [#def4]
shadow-4.8.1/libmisc/copydir.c:742: open_fn: Returning handle opened by "open". [Note: The source code implementation of the function has been overridden by a user model.]
shadow-4.8.1/libmisc/copydir.c:742: var_assign: Assigning: "ifd" = handle returned from "open(src, 0)".
shadow-4.8.1/libmisc/copydir.c:748: leaked_handle: Handle variable "ifd" going out of scope leaks the handle.
746| #ifdef WITH_SELINUX
747| if (set_selinux_file_context (dst, NULL) != 0) {
748|-> return -1;
749| }
750| #endif /* WITH_SELINUX */
Error: RESOURCE_LEAK (CWE-772): [#def5]
shadow-4.8.1/libmisc/copydir.c:751: open_fn: Returning handle opened by "open". [Note: The source code implementation of the function has been overridden by a user model.]
shadow-4.8.1/libmisc/copydir.c:751: var_assign: Assigning: "ofd" = handle returned from "open(dst, 577, statp->st_mode & 0xfffU)".
shadow-4.8.1/libmisc/copydir.c:752: noescape: Resource "ofd" is not freed or pointed-to in "fchown_if_needed".
shadow-4.8.1/libmisc/copydir.c:775: leaked_handle: Handle variable "ofd" going out of scope leaks the handle.
773| ) {
774| (void) close (ifd);
775|-> return -1;
776| }
777|
Error: RESOURCE_LEAK (CWE-772): [#def7]
shadow-4.8.1/libmisc/idmapping.c:188: alloc_fn: Storage is returned from allocation function "xmalloc".
shadow-4.8.1/libmisc/idmapping.c:188: var_assign: Assigning: "buf" = storage returned from "xmalloc(bufsize)".
shadow-4.8.1/libmisc/idmapping.c:188: var_assign: Assigning: "pos" = "buf".
shadow-4.8.1/libmisc/idmapping.c:213: noescape: Resource "buf" is not freed or pointed-to in "write".
shadow-4.8.1/libmisc/idmapping.c:219: leaked_storage: Variable "pos" going out of scope leaks the storage it points to.
shadow-4.8.1/libmisc/idmapping.c:219: leaked_storage: Variable "buf" going out of scope leaks the storage it points to.
217| }
218| close(fd);
219|-> }
Error: RESOURCE_LEAK (CWE-772): [#def8]
shadow-4.8.1/libmisc/list.c:211: alloc_fn: Storage is returned from allocation function "xstrdup".
shadow-4.8.1/libmisc/list.c:211: var_assign: Assigning: "members" = storage returned from "xstrdup(comma)".
shadow-4.8.1/libmisc/list.c:217: var_assign: Assigning: "cp" = "members".
shadow-4.8.1/libmisc/list.c:218: noescape: Resource "cp" is not freed or pointed-to in "strchr".
shadow-4.8.1/libmisc/list.c:244: leaked_storage: Variable "cp" going out of scope leaks the storage it points to.
shadow-4.8.1/libmisc/list.c:244: leaked_storage: Variable "members" going out of scope leaks the storage it points to.
242| if ('\0' == *members) {
243| *array = (char *) 0;
244|-> return array;
245| }
246|
Error: RESOURCE_LEAK (CWE-772): [#def11]
shadow-4.8.1/libmisc/myname.c:61: alloc_fn: Storage is returned from allocation function "xgetpwnam".
shadow-4.8.1/libmisc/myname.c:61: var_assign: Assigning: "pw" = storage returned from "xgetpwnam(cp)".
shadow-4.8.1/libmisc/myname.c:67: leaked_storage: Variable "pw" going out of scope leaks the storage it points to.
65| }
66|
67|-> return xgetpwuid (ruid);
68| }
69|
Error: RESOURCE_LEAK (CWE-772): [#def12]
shadow-4.8.1/libmisc/user_busy.c:260: alloc_fn: Storage is returned from allocation function "opendir".
shadow-4.8.1/libmisc/user_busy.c:260: var_assign: Assigning: "task_dir" = storage returned from "opendir(task_path)".
shadow-4.8.1/libmisc/user_busy.c:262: noescape: Resource "task_dir" is not freed or pointed-to in "readdir".
shadow-4.8.1/libmisc/user_busy.c:278: leaked_storage: Variable "task_dir" going out of scope leaks the storage it points to.
276| _("%s: user %s is currently used by process %d\n"),
277| Prog, name, pid);
278|-> return 1;
279| }
280| }
Error: RESOURCE_LEAK (CWE-772): [#def20]
shadow-4.8.1/src/newgrp.c:162: alloc_fn: Storage is returned from allocation function "xgetspnam".
shadow-4.8.1/src/newgrp.c:162: var_assign: Assigning: "spwd" = storage returned from "xgetspnam(pwd->pw_name)".
shadow-4.8.1/src/newgrp.c:234: leaked_storage: Variable "spwd" going out of scope leaks the storage it points to.
232| }
233|
234|-> return;
235|
236| failure:
Error: RESOURCE_LEAK (CWE-772): [#def21]
shadow-4.8.1/src/passwd.c:530: alloc_fn: Storage is returned from allocation function "xstrdup".
shadow-4.8.1/src/passwd.c:530: var_assign: Assigning: "cp" = storage returned from "xstrdup(crypt_passwd)".
shadow-4.8.1/src/passwd.c:551: noescape: Resource "cp" is not freed or pointed-to in "strlen".
shadow-4.8.1/src/passwd.c:554: noescape: Resource "cp" is not freed or pointed-to in "strcat". [Note: The source code implementation of the function has been overridden by a builtin model.]
shadow-4.8.1/src/passwd.c:555: overwrite_var: Overwriting "cp" in "cp = newpw" leaks the storage that "cp" points to.
553| strcpy (newpw, "!");
554| strcat (newpw, cp);
555|-> cp = newpw;
556| }
557| return cp;
Using the random() function to obtain pseudo-random bytes
for generating salt strings is considered to be dangerous.
See CWE-327.
We really should use a more reliable source for obtaining
pseudo-random bytes like /dev/urandom.
Fixes#376.
Signed-off-by: Björn Esser <besser82@fedoraproject.org>
In the previous commit we refactored the functions converting the
rounds number into a string for use with the crypt() function, to
not require any static buffer anymore.
Add some clarifying comments about how the minimum required buffer
length is computed inside of these functions.
Signed-off-by: Björn Esser <besser82@fedoraproject.org>
* Move all pre-processor defines to the top of the file.
* Unify the gensalt() function to be useable for all supported
hash methods.
* Drop the gensalt_{b,yes}crypt() functions in favor of the
previous change.
* Refactor the functions converting the rounds number into
a string for use with the crypt() function, to not require
any static buffer anymore.
* Clarify the comment about how crypt_make_salt() chooses the used
hash method from the settings in the login.defs file.
* Use memset() to fill static buffers with zero before using them.
* Use a fixed amount of 16 random base64-chars for the
sha{256,512}crypt hash methods, which is effectively still less
than the recommendation from NIST (>= 128 bits), but the maximum
those methods can effectively use (approx. 90 bits).
* Rename ROUNDS_{MIN,MAX} to SHA_ROUNDS_{MIN,MAX}.
* Bugfixes in the logic of setting rounds in BCRYPT_salt_rounds().
* Likewise for YESCRYPT_salt_cost().
* Fix formatting and white-space errors.
Signed-off-by: Björn Esser <besser82@fedoraproject.org>
The corresponding functions for the other hash methods all take
a pointer to an integer value as the only paramater, so this
particular function should do so as well.
Signed-off-by: Björn Esser <besser82@fedoraproject.org>
Error: RESOURCE_LEAK (CWE-772): [#def28]
shadow-4.8.1/src/useradd.c:1905: open_fn: Returning handle opened by "open". [Note: The source code implementation of the function has been overridden by a user model.]
shadow-4.8.1/src/useradd.c:1905: var_assign: Assigning: "fd" = handle returned from "open("/var/log/faillog", 2)".
shadow-4.8.1/src/useradd.c:1906: noescape: Resource "fd" is not freed or pointed-to in "lseek".
shadow-4.8.1/src/useradd.c:1917: leaked_handle: Handle variable "fd" going out of scope leaks the handle.
1915| /* continue */
1916| }
1917|-> }
1918|
1919| static void lastlog_reset (uid_t uid)
Error: RESOURCE_LEAK (CWE-772): [#def29]
shadow-4.8.1/src/useradd.c:1938: open_fn: Returning handle opened by "open". [Note: The source code implementation of the function has been overridden by a user model.]
shadow-4.8.1/src/useradd.c:1938: var_assign: Assigning: "fd" = handle returned from "open("/var/log/lastlog", 2)".
shadow-4.8.1/src/useradd.c:1939: noescape: Resource "fd" is not freed or pointed-to in "lseek".
shadow-4.8.1/src/useradd.c:1950: leaked_handle: Handle variable "fd" going out of scope leaks the handle.
1948| /* continue */
1949| }
1950|-> }
1951|
1952| static void tallylog_reset (const char *user_name)
Error: RESOURCE_LEAK (CWE-772): [#def30]
shadow-4.8.1/src/useradd.c:2109: alloc_fn: Storage is returned from allocation function "strdup".
shadow-4.8.1/src/useradd.c:2109: var_assign: Assigning: "bhome" = storage returned from "strdup(prefix_user_home)".
shadow-4.8.1/src/useradd.c:2131: noescape: Resource "bhome" is not freed or pointed-to in "strtok".
shadow-4.8.1/src/useradd.c:2207: leaked_storage: Variable "bhome" going out of scope leaks the storage it points to.
2205| }
2206| #endif
2207|-> }
2208| }
2209|
Following the discussion https://github.com/shadow-maint/shadow/pull/345
I have changed the documentation to clarify the behaviour of subid
delegation when any subid source except files is configured.
Error: RESOURCE_LEAK (CWE-772): [#def31]
shadow-4.8.1/src/usermod.c:813: alloc_fn: Storage is returned from allocation function "__gr_dup".
shadow-4.8.1/src/usermod.c:813: var_assign: Assigning: "ngrp" = storage returned from "__gr_dup(grp)".
shadow-4.8.1/src/usermod.c:892: leaked_storage: Variable "ngrp" going out of scope leaks the storage it points to.
890| }
891| }
892|-> }
893|
894| #ifdef SHADOWGRP
Error: RESOURCE_LEAK (CWE-772): [#def32]
shadow-4.8.1/src/usermod.c:933: alloc_fn: Storage is returned from allocation function "__sgr_dup".
shadow-4.8.1/src/usermod.c:933: var_assign: Assigning: "nsgrp" = storage returned from "__sgr_dup(sgrp)".
shadow-4.8.1/src/usermod.c:1031: leaked_storage: Variable "nsgrp" going out of scope leaks the storage it points to.
1029| }
1030| }
1031|-> }
1032| #endif /* SHADOWGRP */
1033|
Error: RESOURCE_LEAK (CWE-772): [#def34]
shadow-4.8.1/src/usermod.c:1161: alloc_fn: Storage is returned from allocation function "getgr_nam_gid".
shadow-4.8.1/src/usermod.c:1161: var_assign: Assigning: "grp" = storage returned from "getgr_nam_gid(optarg)".
shadow-4.8.1/src/usermod.c:1495: leaked_storage: Variable "grp" going out of scope leaks the storage it points to.
1493| }
1494| #endif /* ENABLE_SUBIDS */
1495|-> }
1496|
1497| /*
Error: RESOURCE_LEAK (CWE-772): [#def35]
shadow-4.8.1/src/usermod.c:1991: open_fn: Returning handle opened by "open". [Note: The source code implementation of the function has been overridden by a user model.]
shadow-4.8.1/src/usermod.c:1991: var_assign: Assigning: "fd" = handle returned from "open("/var/log/lastlog", 2)".
shadow-4.8.1/src/usermod.c:2000: noescape: Resource "fd" is not freed or pointed-to in "lseek".
shadow-4.8.1/src/usermod.c:2000: noescape: Resource "fd" is not freed or pointed-to in "read". [Note: The source code implementation of the function has been overridden by a builtin model.]
shadow-4.8.1/src/usermod.c:2003: noescape: Resource "fd" is not freed or pointed-to in "lseek".
shadow-4.8.1/src/usermod.c:2032: leaked_handle: Handle variable "fd" going out of scope leaks the handle.
2030| }
2031| }
2032|-> }
2033|
2034| /*
Error: RESOURCE_LEAK (CWE-772): [#def36]
shadow-4.8.1/src/usermod.c:2052: open_fn: Returning handle opened by "open". [Note: The source code implementation of the function has been overridden by a user model.]
shadow-4.8.1/src/usermod.c:2052: var_assign: Assigning: "fd" = handle returned from "open("/var/log/faillog", 2)".
shadow-4.8.1/src/usermod.c:2061: noescape: Resource "fd" is not freed or pointed-to in "lseek".
shadow-4.8.1/src/usermod.c:2061: noescape: Resource "fd" is not freed or pointed-to in "read". [Note: The source code implementation of the function has been overridden by a builtin model.]
shadow-4.8.1/src/usermod.c:2064: noescape: Resource "fd" is not freed or pointed-to in "lseek".
shadow-4.8.1/src/usermod.c:2092: leaked_handle: Handle variable "fd" going out of scope leaks the handle.
2090| }
2091| }
2092|-> }
2093|
2094| #ifndef NO_MOVE_MAILBOX
Clarify that the subid delegation can only come from one source.
Moreover, add an example of what might happen if the subid source is NSS
and useradd is executed.
Related: https://github.com/shadow-maint/shadow/issues/331
Closes#331
1. drop 'has_any_range' nss method as it is not useful
2. do not try to create a subid range in newusers when using nss for
subids, since that's not possible.
Signed-off-by: Serge Hallyn <serge@hallyn.com>
(cherry picked from commit 88a434adbdcf4a8640793fd58bcd2ba77598349d)
Following alexey-tikhonov's suggestion.
Since we've dropped the 'owner' field in the data returned for
get_subid_ranges, we can just return a single allocated array of
simple structs. This means we can return a ** instead of ***, and
we can get rid of the subid_free_ranges() helper, since the caller
can just free() the returned data.
Signed-off-by: Serge Hallyn <serge@hallyn.com>
The rest of the run isn't likely to get much better, is it?
Thanks to Alexey for pointing this out.
Signed-off-by: Serge Hallyn <serge@hallyn.com>
Cc: Alexey Tikhonov <atikhono@redhat.com>
Closes: 339
struct subordinate_range is pretty closely tied to the existing
subid code and /etc/subuid format, so it includes an owner. Dropping
that or even renaming it is more painful than I'd first thought.
So introduce a 'struct subid_range' which is only the start and
count, leaving 'struct subordinate_range' as the owner, start and
count.
Signed-off-by: Serge Hallyn <serge@hallyn.com>
Closes#325
Add a new subid_init() function which can be used to specify the
stream on which error messages should be printed. (If you want to
get fancy you can redirect that to memory :) If subid_init() is
not called, use stderr. If NULL is passed, then /dev/null will
be used.
This patch also fixes up the 'Prog', which previously had to be
defined by any program linking against libsubid. Now, by default
in libsubid it will show (subid). Once subid_init() is called,
it will use the first variable passed to subid_init().
Signed-off-by: Serge Hallyn <serge@hallyn.com>
When uid 0 maps host uid 0 into the child userns newer kernels require
CAP_SETFCAP be retained as this allows the caller to create fscaps that
are valid in the ancestor userns. This was a security issue (in very
rare circumstances). So whenever host uid 0 is mapped, retain
CAP_SETFCAP if the caller had it.
Userspace won't need to set CAP_SETFCAP on newuidmap as this is really
only a scenario that real root should be doing which always has
CAP_SETFCAP. And if they don't then they are in a locked-down userns.
(LXC sometimes maps host uid 0 during chown operations in a helper
userns but will not rely on newuidmap for that. But we don't want to
risk regressing callers that want to rely on this behavior.)
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
Include the new HMAC_CRYPTO_ALGO key that is needed by pam_timestamp to
select the algorithm that is going to be used to calculate the message
authentication code.
pam_timestamp is currently using an embedded algorithm to calculate the
HMAC message, but the idea is to improve this behaviour by relying on
openssl's implementation. On top of that, the ability to change the
algorithm with a simple configuration change allows to simplify the
process of removing unsecure algorithms.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1947294
Once opened, keep the selabel database open for further lookups.
Register an exit handler to close the database.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
Search the SELinux selabel database for the file type to be created.
Not specifying the file mode can cause an incorrect file context to be
returned.
Also prepare contexts in commonio_close() for the generic database
filename, not with the backup suffix appended, to ensure the desired
file context after the final rename.
Closes: #322
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
matchpathcon(3) is deprecated in favor of selabel_lookup(3).
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
Return 0 on setfscreatecon(3) failure, like set_selinux_file_context().
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
These retrieved contexts are just passed to libselinux functions and not
printed or otherwise made available to the outside, so a context
translation to human readable MCS/MLS labels is not needed.
(see man:setrans.conf(5))
The typedef security_context_t is deprecated, see
9eb9c93275
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
This retrieved context is just passed to libselinux functions and not
printed or otherwise made available to the outside, so a context
translation to human readable MCS/MLS labels is not needed.
(see man:setrans.conf(5))
The typedef security_context_t is deprecated, see
9eb9c93275
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
Currently, supplying a relative path via the --prefix flag to the
useradd command triggers a bug in the creation of home directories. The
code seems to unintentionally prepend a leading "/" to all paths,
quietly transforming a relative prefixed home path into an absolute
path. This can be seen in the following strace logs from running
"useradd --create-home --prefix tmp/root squat":
```
access("tmp/root//home/squat", F_OK) = -1 ENOENT (No such file or directory)
access("/mp", F_OK) = 0
access("/mp/root", F_OK) = 0
access("/mp/root/home", F_OK) = 0
access("/mp/root/home/squat", F_OK) = -1 ENOENT (No such file or directory)
mkdir("/mp/root/home/squat", 000) = 0
chown("/mp/root/home/squat", 0, 0) = 0
chmod("/mp/root/home/squat", 0755) = 0
chown("tmp/root//home/squat", 1000, 1000) = -1 ENOENT (No such file or directory)
chmod("tmp/root//home/squat", 0700) = -1 ENOENT (No such file or directory)
```
Note that the relative path is correctly probed in the beginning and it
is only during the recursive creation that the path is turned into an
absolute path. This invocation results in the creation of a "/mp"
hierarchy in the root of the filesystem.
Similar problems occur when using `--prefix ./tmp/root`.
This commit fixes the handling of relative paths by not assuming that
the given path is anchored with a "/".
Signed-off-by: Lucas Servén Marín <lserven@gmail.com>
Otherwise our su -p uses bash if that is what root was
configured to use, and then fails to read /root/ for
.bash_profile. This caused an unexpected error message
in /tmp/err, failing the test.
Signed-off-by: Serge Hallyn <serge@hallyn.com>
Closes#154
When starting any operation to do with subuid delegation, check
nsswitch for a module to use. If none is specified, then use
the traditional /etc/subuid and /etc/subgid files.
Currently only one module is supported, and there is no fallback
to the files on errors. Several possibilities could be considered:
1. in case of connection error, fall back to files
2. in case of unknown user, also fall back to files
etc...
When non-files nss module is used, functions to edit the range
are not supported. It may make sense to support it, but it also
may make sense to require another tool to be used.
libsubordinateio also uses the nss_ helpers. This is how for instance
lxc could easily be converted to supporting nsswitch.
Add a set of test cases, including a dummy libsubid_zzz module. This
hardcodes values such that:
'ubuntu' gets 200000 - 300000
'user1' gets 100000 - 165536
'error' emulates an nss module error
'unknown' emulates a user unknown to the nss module
'conn' emulates a connection error ot the nss module
Changes to libsubid:
Change the list_owner_ranges api: return a count instead of making the array
null terminated.
This is a breaking change, so bump the libsubid abi major number.
Rename free_subuid_range and free_subgid_range to ungrant_subuid_range,
because otherwise it's confusing with free_subid_ranges which frees
memory.
Run libsubid tests in jenkins
Switch argument order in find_subid_owners
Move the db locking into subordinateio.c
Signed-off-by: Serge Hallyn <serge@hallyn.com>
Issue #297 reported seeing
*** Warning: Linking the shared library libsubid.la against the
*** static library ../libmisc/libmisc.a is not portable!
which commit b5fb1b38ee was supposed
to fix. But a few commits later it's back. So try to fix it
in the way the bug reporter suggested. This broke builds some
other ways, namely a few missing library specifications, so add
those.
Signed-off-by: Serge Hallyn <serge@hallyn.com>
* login & su: Treat an empty passwd field as invalid
Otherwise it's treated like the “require no password” clause while it probably
should be treated like a normal su that can't validate anyway.
A similar change should be done for USE_PAM.
* su & login: Introduce PREVENT_NO_AUTH
man/usermod.8.xml: specify what happens when the current home directory
doesn't exist if using -d and -m options. Moreover, specify what happens
when the group ownership is changed and the uid's don't match in -u and
-g options.
man/shadow.5.xml: indicate the exact time and timezone for the dates.
Moreover, clarify that when the password expires the user won't be able
to login.
man/chage.1.xml: Indicate that -d option with a value of 0 forces the
user to change his password. Besides, set an example on how to use -E
option. Finally, add a general note to clarify that chage only takes
charge of local users and another note to indicate that it doesn't check
inconsistencies between shadow and passwd files.
covscan issue:
Error: RESOURCE_LEAK (CWE-772): [#def39] [important]
src/useradd.c:728: alloc_fn: Storage is returned from allocation function "get_local_group".
src/useradd.c:728: var_assign: Assigning: "grp" = storage returned from "get_local_group(list)".
src/useradd.c:728: overwrite_var: Overwriting "grp" in "grp = get_local_group(list)" leaks the storage that "grp" points to.
726| * GID values, otherwise the string is looked up as is.
727| */
728|-> grp = get_local_group (list);
729|
730| /*
Existing help output advertises --force as a long opt.
-f, --force delete group even if it is the primary group of a user
But errors when the long opt is used.
groupdel: unrecognized option '--force'
Signed-off-by: Jamin W. Collins <jamin.collins@gmail.com>
The login.defs is shared between more upstream projects (util-linux,
etc.). We need to improve compatibility between the projects do not
report valid, but foreign items.
Addresses: https://github.com/shadow-maint/shadow/issues/276
Signed-off-by: Karel Zak <kzak@redhat.com>
Some of these tests seem wrong. The assume that
su -- -c command
should work, whereas -- should mean pass all remaining arguments
along to the command.
Add some new tests based on examples in Issue 253
Signed-off-by: Serge Hallyn <shallyn@cisco.com>
It's now possible to run commands as other users without shell
interpolation by using "--exec":
Read /etc/shadow as root without specifying user:
```
su --exec /bin/cat -- /etc/shadow
```
Or specify user:
```
su --exec /bin/cat root -- /etc/shadow
```
Mechanical rename distinguishing this variable from intended changes
supporting executing commands without using an interpretive shell
(i.e. no '/bin/sh -c').
In preparation for supporting --exec I was testing the robustness
of "--" handling and it became apparent that things are currently
a bit broken in `su`.
Since "--" is currently of limited utility, as the subsequent
words are simply passed to the shell after "-c","command_string",
it seems to have gone unnoticed for ages.
However, with --exec, it's expected that "--" would be an almost
required separator with every such usage, considering the
following flags must be passed verbatim to execve() and will
likely begin with hyphens looking indistinguishable from any
other flags in lieu of shell interpolation to worry about.
For some practical context of the existing situation, this
invocation doesn't work today:
```
$ su --command ls -- flags for shell
No passwd entry for user 'flags'
$
```
This should just run ls as root with "flags","for","shell"
forwarded to the shell after "-c","ls".
The "--" should block "flags" from being treated as the user.
That particular issue isn't a getopt one per-se, it's arguably
just a bug in su.c's implementation.
It *seemed* like an easy fix for this would be to add a check if
argv[optind-1] were "--" before treating argv[optind] as USER.
But testing that fix revealed getopt was rearranging things when
encountering "--", the "--" would always separate the handled
opts from the unhandled ones. USER would become shifted to
*after* "--" even when it occurred before it!
If we change the command to specify the user, it works as-is:
```
$ su --command ls root -- flags for shell
Password:
testfile
$
```
But what's rather surprising is how that works; the argv winds up:
"su","--command","ls","--","root","flags","for","shell"
with optind pointing at "root".
That arrangement of argv is indistinguishable from omitting the
user and having "root","flags","for","shell" as the stuff after
"--".
This makes it non-trivial to fix the bug of omitting user
treating the first word after "--" as the user, which one could
argue is a potentially serious security bug if you omit the user,
expect the command to run as root, and the first word after "--"
is a valid user, and what follows that something valid and
potentially destructive not only running in unintended form but
as whatever user happened to be the first word after "--".
So, it seems like something important to fix, and getopt seems to
be getting in the way of fixing it properly without being more
trouble than replacing getopt.
In disbelief of what I was seeing getopt doing with argv here, I
took a glance at the getopt source and found the following:
```
/* The special ARGV-element '--' means premature end of options.
Skip it like a null option,
then exchange with previous non-options as if it were an option,
then skip everything else like a non-option. */
if (d->optind != argc && !strcmp (argv[d->optind], "--"))
```
I basically never use getopt personally because ages ago it
annoyed me with its terrible API for what little it brought to
the table, and this brings it to a whole new level of awful.
This is a stability fix, not a security fix, because the affected -o
option can only be used by root and it takes a modified passwd file.
If a gecos field for a user has BUFSIZ characters without commas and an
equals sign (i.e. a huge slop/extra field) and chfn is called with -o,
then a buffer overflow occurs.
It is not possible to trigger this with shadow tools. Therefore, the
passwd file must be modified manually.
I have fixed this unlikely case the easiest and cleanest way possible.
Since chfn bails out if more than 80 characters excluding commas are
supposed to be written into gecos field, we can stop processing early on
if -o argument is too long.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
This is merely a stability fix, not a security fix.
As the root user, it is possible to set time values which later on
result in signed integer overflows.
For this to work, an sgetspent implementation must be used which
supports long values (glibc on amd64 only parses 32 bit, not 64).
Either use musl or simply call configure with following environment
variable:
$ ac_cv_func_sgetspent=no ./configure
Also it is recommended to compile with -fsanitize=undefined or
-ftrapv to see these issues easily.
Examples to trigger issues when calling "chage -l user":
$ chage -d 9223372036854775807 user
$ chage -d 106751991167300 user
$ chage -M 9999 user
$ chage -d 90000000000000 user
$ chage -I 90000000000000 user
$ chage -M 9999 user
$ chage -E 9223372036854775807 user
While at it, I fixed casting issues which could lead to signed integer
overflows on systems which still have a 32 bit time_t.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
Closes#154
Currently this has three functions: one which returns the
list of subuid ranges for a user, one returning the subgids,
and one which frees the ranges lists.
I might be mistaken about what -disable-man means; some of
the code suggests it means just don't re-generate them, but
not totally ignore them. But that doesn't seem to really work,
so let's just ignore man/ when -disable-man.
Remove --disable-shared. I'm not sure why it was there, but it stems
from long, long ago, and I suspect it comes from some ancient
toolchain bug.
Create a tests/run_some, a shorter version of run_all. I'll
slowly add tests to this as I verify they work, then I can
work on fixing the once which don't.
Also, don't touch man/ if not -enable-man.
Changelog:
Apr 22: change the subid list api as recomended by Dan Walsh.
Apr 23: implement get_subid_owner
Apr 24: implement range add/release
Apr 25: finish tests and rebase
May 10: make @owner const
Signed-off-by: Serge Hallyn <serge@hallyn.com>
Now that travis supports more architectures let's make sure we test on
all of them and that we enable Coverity too.
Signed-off-by: Christian Brauner <christian@brauner.io>
Do not include <sys/prctl.h> we don't have <sys/capability.h>, we don't
need prctl in that case anyway.
Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
According to crypt(5), MD5 and DES should not be used for new
hashes. Also the default number of SHA rounds chosen by libc is orders
of magnitude too low for modern hardware. Let's warn the users about
weak choices.
Signed-off-by: Topi Miettinen <toiwoton@gmail.com>
Explanation: clarify the useradd -d parameter as it does create directory HOME_DIR if it doesn't exit.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1677005
Changelog: [serge] minor tweak to the text
The useradd program should be consistent with userdel and usermod and use the
MAIL_SPOOL_DIR variable as the default spool, if it is defined. Otherwise,
don't create a new mailbox, because it won't be cleaned up by userdel when run
with the -r flag.
In case there is a regular user with a process running on a system
with uid falling into a namespaced uid range of another user.
The user with the colliding namespaced uid range will not be
allowed to be deleted without forcing the action with -f.
The user_busy() is adjusted to check whether the suspected process
is really a namespaced process in a different namespace.
updated Dutch translation for shadow version 4.8 (pot file from 2019-12-01).
I updated the translation for Debian
and on request of the Debian package maintainer Bálint Réczey I am creating this pull request
Although it is a good idea to check for an inadvertent typo
in the shell name it is possible that the shell might not be present
on the system yet when the user is added.
This option can be used to set a separate mode for useradd(8) and
newusers(8) to create the home directories with.
If this option is not set, the current behavior of using UMASK
or the default umask is preserved.
There are many distributions that set UMASK to 077 by default just
to create home directories not readable by others and use things like
/etc/profile, bashrc or sudo configuration files to set a less
restrictive
umask. This has always resulted in bug reports because it is hard
to follow as users tend to change files like bashrc and are not about
setting the umask to counteract the umask set in /etc/login.defs.
A recent change in sudo has also resulted in many bug reports about
this. sudo now tries to respect the umask set by pam modules and on
systems where pam does not set a umask, the login.defs UMASK value is
used.
This commit adds a from= field to the end of the useradd log entry.
Casting user_name to tallylog_reset to silence a compiler warning.
Changelog: Fixing tabs
Changelog: Changing function prototype to const char* to match user_name declaration.
This option can be used to set a separate mode for useradd(8) and
newusers(8) to create the home directories with.
If this option is not set, the current behavior of using UMASK
or the default umask is preserved.
There are many distributions that set UMASK to 077 by default just
to create home directories not readable by others and use things like
/etc/profile, bashrc or sudo configuration files to set a less
restrictive
umask. This has always resulted in bug reports because it is hard
to follow as users tend to change files like bashrc and are not about
setting the umask to counteract the umask set in /etc/login.defs.
A recent change in sudo has also resulted in many bug reports about
this. sudo now tries to respect the umask set by pam modules and on
systems where pam does not set a umask, the login.defs UMASK value is
used.
If SSH_ORIGINAL_COMMAND is set, it will be added to the syslog entry.
Closes#123.
Changelog: (SEH squashed commit): Fixing indentation
Changelog: (SEH) break up long line
`make` runs each line in a shell and bails out on error,
however, the shell is not started with `-e`, so commands in
`for` loops can fail without the error actually causing
`make` to bail out with a failure status.
For instance, the following make snippet will end
successfully, printing 'SUCCESS', despite the first `chmod`
failing:
all:
touch a b
for i in a-missing-file a b; do \
chmod 666 $$i; \
done
@echo SUCCESS
To prevent wrong paths in install scripts from remaining
unnoticed, let's activate `set -e` in the `for` loop
subshells.
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
Suggesting mode 2770 is dangerous because it makes the binary writeable
by all members of the owning group which is supposed to be normal
end-users. Suggest 2710 instead as is usual for s[ug]id binaries,
allowing execution but neither reading nor writing.
Signed-off-by: Michael Weiser <michael.weiser@gmx.de>
Here's a sad story:
* 70971457 is merged into shadow, allowing newgidmap/newuidmap to be
installed with file caps rather than setuid.
* https://bugs.archlinux.org/task/63248 is filed to take advantage of
this.
* The arch maintainer of the 'shadow' package notices that this doesn't
work, and submits a pull request to fix this in shadow.
* edf7547ad5 is merged, fixing the post install hooks.
The problem here is that distros have been building shadow with PAM for
O(years), but the install hooks have silently failed due to the
combination of the directory mismatch (suidubins vs suidsbins) and later
success with setuid'ing newgidmap/newuidmap.
With the install hooks fixed, those of us (Arch[1] and Gentoo[2] so far)
who never built shadow explicitly with --enable-account-tools-setuid are
now getting setuid account tools, and don't have PAM configuration
suitable for use with setuid account management tools.
It's entirely unclear to me why you'd want this, but I assume there's
some reason out there for it existing. Regardless, setuid binaries are
dangerous and shouldn't be enabled by default without good reason.
[1] https://bugs.archlinux.org/task/64836
[2] https://bugs.gentoo.org/702252
This reverts commit e293aa9cfc.
See https://github.com/shadow-maint/shadow/issues/196
Some distros still care about `/bin` vs `/usr/bin`. This commit makes
it so all binaries are always installed to `/bin`/`/sbin`. The only way to
restore the previous behaviour of installing some binaries to
`/usr/bin`/`/usr/sbin` is to revert the patch.
Synchronize how passwd(5) and shadow(5) describe the password field.
Reorder the descriptions more logically.
Signed-off-by: Topi Miettinen <toiwoton@gmail.com>
Closes#185
If vipw is suspended (e.g. via control-Z) and then resumed, it often gets
immediately suspended. This is easier to reproduce on a multi-core system.
root@buster:~# /usr/sbin/vipw
[1]+ Stopped /usr/sbin/vipw
root@buster:~# fg
/usr/sbin/vipw
[1]+ Stopped /usr/sbin/vipw
root@buster:~# fg
[vipw resumes on the second fg]
The problem is that vipw forks a child process and calls waitpid() with the
WUNTRACED flag. When the child process (running the editor) is suspended, the
parent sends itself SIGSTOP to suspend the main vipw process. However, because
the main vipw is in the same process group as the editor which received the ^Z,
the kernel already sent the main vipw SIGTSTP.
If the main vipw receives SIGTSTP before the child, it will be suspended and
then, once resumed, will proceed to suspend itself again.
To fix this, run the child process in its own process group as the foreground
process group. That way, control-Z will only affect the child process and the
parent can use the existing logic to suspend the parent.
Using hard-coded access vector ids is deprecated and can lead to issues with custom SELinux policies.
Switch to `selinux_check_access()`.
Also use the libselinux log callback and log if available to audit.
This makes it easier for users to catch SELinux denials.
Drop legacy shortcut logic for passwd, which avoided a SELinux check if uid 0 changes a password of a user which username equals the current SELinux user identifier.
Nowadays usernames rarely match SELinux user identifiers and the benefit of skipping a SELinux check is negligible.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
With this, it is possible for Linux distributors to store their
supplied default configuration files somewhere below /usr, while
/etc only contains the changes made by the user. The new option
--enable-vendordir defines where the shadow suite should additional
look for login.defs if this file is not in /etc.
libeconf is a key/value configuration file reading library, which
handles the split of configuration files in different locations
and merges them transparently for the application.
suidubins should be suidusbins, since these binaries are installed
${prefix}/sbin. This historically hasn't broken the build because
chmod of newgidmap/newuidmap succeeds, causing make to think the command
succeeded. Configuring shadow with --with-fcaps removes these final two
entries and exposes the chmod failure to make.
Some distros don't care about the split between /bin, /sbin, /usr/bin,
and /usr/sbin, so let them easily stuff binaries wherever they want.
This also fixes a problem during installation where-in a loop of 'chmod
4755' calls will mostly fail. However, because the last two succeed
(newuidmap/newgidmap), make considers the command to be a success.
Somewhat not-amusingly, configuring shadow with --with-fcaps will cause
installation to fail because the final chmod call is now a failing one.
Fix formatting of login.defs comments. Variables are preceeded by "#"
without space, comments are preceeded by "# ". It makes the file machine
parseable again.
Signed-off-by: Stanislav Brabec <sbrabec@suse.cz>
A configure error occurs when /bin/sh -> dash:
checking for is_selinux_enabled in -lselinux... yes
checking for semanage_connect in -lsemanage... yes
configure: 16322: test: yesyes: unexpected operator
Use "=" instead of "==" since dash doesn't support this operator.
Signed-off-by: Yi Zhao <yi.zhao@windriver.com>
new switch added to useradd command, --btrfs-subvolume-home. When
specified *and* the filesystem is detected as btrfs, it will create a
subvolume for user's home instead of a plain directory. This is done via
`btrfs subvolume` command. Specifying the new switch while trying to
create home on non-btrfs will result in an error.
userdel -r will handle and remove this subvolume transparently via
`btrfs subvolume` command. Previosuly this failed as you can't rmdir a
subvolume.
usermod, when moving user's home across devices, will detect if the home
is a subvolume and issue an error messages instead of copying it. Moving
user's home (as subvolume) on same btrfs works transparently.
As the lockfiles have PID in the name, there can be no conflict
in the name with other process, so there is no point in using
O_EXCL and it only can fail if there is a stale lockfile from
previous execution that crashed for some reason.
The implementation of prefix option dropped the use of lckpwdf().
However that is incorrect as other tools manipulating the shadow passwords
such as PAM use lckpwdf() and do not know anything about the
shadow's own locking mechanism.
This reverts the implementation to use lckpwdf() if prefix option
is not used.
From <https://github.com/shadow-maint/shadow/pull/71>:
```
The third field in the /etc/shadow file (sp_lstchg) contains the date of
the last password change expressed as the number of days since Jan 1, 1970.
As this is a relative time, creating a user today will result in:
username:17238:0:99999:7:::
whilst creating the same user tomorrow will result in:
username:17239:0:99999:7:::
This has an impact for the Reproducible Builds[0] project where we aim to
be independent of as many elements the build environment as possible,
including the current date.
This patch changes the behaviour to use the SOURCE_DATE_EPOCH[1]
environment variable (instead of Jan 1, 1970) if valid.
```
This updated PR adds some missing calls to gettime (). This was originally
filed by Johannes Schauer in Debian as #917773 [2].
[0] https://reproducible-builds.org/
[1] https://reproducible-builds.org/specs/source-date-epoch/
[2] https://bugs.debian.org/917773
- translated by Jean-Philippe MENGUAL
- proofread by the debian-l10n-french mailing list contributors
Signed-off-by: Alban VIDAL <alban.vidal@zordhak.fr>
In case the home directory is not a real home directory
(owned by the user) but things like / or /var or similar,
it is unsafe to change ownership of home directory content.
The test checks whether the home directory is owned by the
user him/herself, if not no ownership modification of contents
is performed.
As the large uids are usually provided by remote user identity and
authentication service, which also provide user login tracking,
there is no need to create a huge sparse file for them on every local
machine.
fixup! login.defs: Add LASTLOG_UID_MAX variable to limit lastlog to small uids.
simplify the condition for setting the euid of the process. Now it is
always set when we are running as root, the issue was introduced with
the commit 52c081b02c
Changelog: 2018-11-24 - seh - enforce that euid only gets set to ruid if
it currently == 0 (i.e. really was setuid-*root*).
Closes: https://github.com/genuinetools/img/issues/191
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Signed-off-by: Serge Hallyn <shallyn@cisco.com>
Commit 1ecca8439d ("new[ug]idmap: not require CAP_SYS_ADMIN in the parent userNS")
does contain a wrong commit message, is lacking an explanation of the
issue, misses some simplifications and hardening features. This commit
tries to rectify this.
In (crazy) environment where all capabilities are dropped from the
capability bounding set apart from CAP_SET{G,U}ID setuid- and
fscaps-based new{g,u}idmap binaries behave differently when writing
complex mappings for an unprivileged user:
1. newuidmap is setuid
unshare -U sleep infinity &
newuidmap $? 0 100000 65536
First file_ns_capable(file, ns, CAP_SYS_ADMIN) is hit. This calls into
cap_capable() and hits the loop
for (;;) {
/* Do we have the necessary capabilities? */
if (ns == cred->user_ns)
return cap_raised(cred->cap_effective, cap) ? 0 : -EPERM;
/*
* If we're already at a lower level than we're looking for,
* we're done searching.
*/
if (ns->level <= cred->user_ns->level)
return -EPERM;
/*
* The owner of the user namespace in the parent of the
* user namespace has all caps.
*/
if ((ns->parent == cred->user_ns) && uid_eq(ns->owner, cred->euid))
return 0;
/*
* If you have a capability in a parent user ns, then you have
* it over all children user namespaces as well.
*/
ns = ns->parent;
}
The first check fails and falls through to the end of the loop and
retrieves the parent user namespace and checks whether CAP_SYS_ADMIN is
available there which isn't.
2. newuidmap has CAP_SETUID as fscaps set
unshare -U sleep infinity &
newuidmap $? 0 100000 65536
The first file_ns_capable() check for CAP_SYS_ADMIN is passed since the
euid has not been changed:
if ((ns->parent == cred->user_ns) && uid_eq(ns->owner, cred->euid))
return 0;
Now new_idmap_permitted() is hit which calls ns_capable(ns->parent,
CAP_SET{G,U}ID). This check passes since CAP_SET{G,U}ID is available in
the parent user namespace.
Now file_ns_capable(file, ns->parent, CAP_SETUID) is hit and the
cap_capable() loop (see above) is entered again. This passes
if (ns == cred->user_ns)
return cap_raised(cred->cap_effective, cap) ? 0 : -EPERM;
since CAP_SET{G,U}ID is available in the parent user namespace. Now the
mapping can be written.
There is no need for this descrepancy between setuid and fscaps based
new{g,u}idmap binaries. The solution is to do a
seteuid() back to the unprivileged uid and PR_SET_KEEPCAPS to keep
CAP_SET{G,U}ID. The seteuid() will cause the
file_ns_capable(file, ns, CAP_SYS_ADMIN) check to pass and the
PR_SET_KEEPCAPS for CAP_SET{G,U}ID will cause the CAP_SET{G,U}ID to
pass.
Fixes: 1ecca8439d ("new[ug]idmap: not require CAP_SYS_ADMIN in the parent userNS")
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
do not install newuidmap/newgidmap as suid binaries. Running these
tools with the same euid as the owner of the user namespace to
configure requires only CAP_SETUID and CAP_SETGID instead of requiring
CAP_SYS_ADMIN when it is installed as a suid binary.
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
if the euid!=owner of the userns, the kernel returns EPERM when trying
to write the uidmap and there is no CAP_SYS_ADMIN in the parent
namespace.
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Some distributions, notably Fedora, have the following order of nsswitch
modules by default:
passwd: sss files
group: sss files
The advantage of serving local users through SSSD is that the nss_sss
module has a fast mmapped-cache that speeds up NSS lookups compared to
accessing the disk an opening the files on each NSS request.
Traditionally, this has been done with the help of nscd, but using nscd
in parallel with sssd is cumbersome, as both SSSD and nscd use their own
independent caching, so using nscd in setups where sssd is also serving
users from some remote domain (LDAP, AD, ...) can result in a bit of
unpredictability.
More details about why Fedora chose to use sss before files can be found
on e.g.:
https://fedoraproject.org//wiki/Changes/SSSDCacheForLocalUsers
or:
https://docs.pagure.org/SSSD.sssd/design_pages/files_provider.html
Now, even though sssd watches the passwd and group files with the help
of inotify, there can still be a small window where someone requests a
user or a group, finds that it doesn't exist, adds the entry and checks
again. Without some support in shadow-utils that would explicitly drop
the sssd caches, the inotify watch can fire a little late, so a
combination of commands like this:
getent passwd user || useradd user; getent passwd user
can result in the second getent passwd not finding the newly added user
as the racy behaviour might still return the cached negative hit from
the first getent passwd.
This patch more or less copies the already existing support that
shadow-utils had for dropping nscd caches, except using the "sss_cache"
tool that sssd ships.
Sometimes getlogin() may fail, e.g., in a chroot() environment or due to NSS
misconfiguration. Loggin UID allows for investigation and troubleshooting in
such situation.
When "su -l" is used the behaviour is described as similar to
a direct login. However login.c is doing a setup_env(pw) and then a
pam_getenvlist() in this scenario. But su.c is doing it the other
way around. Which means that the value of PATH from /etc/environment
is overriden. I think this is a bug because:
The man-page claims that "-l": "provides an environment similar
to what the user would expect had the user logged in directly."
And login.c is using the PATH from /etc/environment.
This will fix:
https://bugs.launchpad.net/ubuntu/+source/shadow/+bug/984390
This allows shadow-utils to build on systems like Adélie, which have no
<utmp.h> header or `struct utmp`. We use a <utmpx.h>-based daemon,
utmps[1], which uses `struct utmpx` only.
Tested both `login` and `logoutd` with utmps and both work correctly.
[1]: http://skarnet.org/software/utmps/
Equivalent of `mkdir -p`. It will create all parent directories.
Example: `useradd -d /home2/testu1 -m testu1`
Based on https://github.com/shadow-maint/shadow/pull/2 by Thorsten Kukuk
and Thorsten Behrens which was Code from pwdutils 3.2.2 with slight adaptations.
Adapted to so it applies to current code.
Otherwise our spw_next() will cause us to skip an entry.
Ideally we'd be able to do an swp_rewind(1), but I don't
see a helper for this.
Closes#60
Signed-off-by: Serge Hallyn <shallyn@cisco.com>
Translate remaining strings to Norwegian bokmål (nb). Also, cure previous translation of excessive anglicism and apply a more consistent use of actual Norwegian syntax.
This is necessary to match the kernel-side policy of "self-mapping in a
user namespace is fine, but you cannot drop groups" -- a policy that was
created in order to stop user namespaces from allowing trivial privilege
escalation by dropping supplementary groups that were "blacklisted" from
certain paths.
This is the simplest fix for the underlying issue, and effectively makes
it so that unless a user has a valid mapping set in /etc/subgid (which
only administrators can modify) -- and they are currently trying to use
that mapping -- then /proc/$pid/setgroups will be set to deny. This
workaround is only partial, because ideally it should be possible to set
an "allow_setgroups" or "deny_setgroups" flag in /etc/subgid to allow
administrators to further restrict newgidmap(1).
We also don't write anything in the "allow" case because "allow" is the
default, and users may have already written "deny" even if they
technically are allowed to use setgroups. And we don't write anything if
the setgroups policy is already "deny".
Ref: https://bugs.launchpad.net/ubuntu/+source/shadow/+bug/1729357
Fixes: CVE-2018-7169
Reported-by: Craig Furman <craig.furman89@gmail.com>
Signed-off-by: Aleksa Sarai <asarai@suse.de>
In case a system uses remote identity server (LDAP) the group lookup
can be very slow. We avoid it when we already know the user has the
group membership.
Do not reset the pid_child to 0 if the child process is still
running. This else-condition can be reached with pid being -1,
therefore explicitly test this condition.
This is a regression fix for CVE-2017-2616. If su receives a
signal like SIGTERM, it is not propagated to the child.
Reported-by: Radu Duta <raduduta@gmail.com>
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
The third field in the /etc/shadow file (sp_lstchg) contains the date of
the last password change expressed as the number of days since Jan 1, 1970.
As this is a relative time, creating a user today will result in:
username:17238:0:99999:7:::
whilst creating the same user tomorrow will result in:
username:17239:0:99999:7:::
This has an impact for the Reproducible Builds[0] project where we aim to
be independent of as many elements the build environment as possible,
including the current date.
This patch changes the behaviour to use the SOURCE_DATE_EPOCH[1]
environment variable (instead of Jan 1, 1970) if valid.
[0] https://reproducible-builds.org/
[1] https://reproducible-builds.org/specs/source-date-epoch/
Signed-off-by: Chris Lamb <lamby@debian.org>
If ptr->line == NULL for an entry, the first cycle will exit,
but the second one will happily write past entries buffer.
We actually do not want to exit the first cycle prematurely
on ptr->line == NULL.
Signed-off-by: Tomas Mraz <tmraz@fedoraproject.org>
If su is compiled with PAM support, it is possible for any local user
to send SIGKILL to other processes with root privileges. There are
only two conditions. First, the user must be able to perform su with
a successful login. This does NOT have to be the root user, even using
su with the same id is enough, e.g. "su $(whoami)". Second, SIGKILL
can only be sent to processes which were executed after the su process.
It is not possible to send SIGKILL to processes which were already
running. I consider this as a security vulnerability, because I was
able to write a proof of concept which unlocked a screen saver of
another user this way.
This reverts the behavior of "useradd --root" to using the settings
from login.defs in the target root directory, not the root of the
executed useradd command.
Without this patch, PAM enabled builds crash when encountering an
invalid key in login.defs or key overrides because of array overflows
To reproduce, simply
useradd -K Windows=broken
Signed-off-by: Bernhard Rosenkränzer <bero@lindev.ch>
Signed-off-by: Serge Hallyn <serge@hallyn.com>
Previously, the allocation was optimized for an outdated
deployment style (that of /etc/group alongside nss_db). The issue
here is that this results in extremely poor performance when using
SSSD, Winbind or nss_ldap.
There were actually two serious bugs here that have been addressed:
1) Running getgrent() loops won't work in most SSSD or Winbind
environments, as full group enumeration is disabled by default.
This could easily result in auto-allocating a group that was
already in use. (This might result in a security issue as well, if
the shared GID is a privileged group).
2) For system groups, the loop was always iterating through the
complete SYS_GID_MIN->SYS_GID_MAX range. On SSSD and Winbind, this
means hundreds of round-trips to LDAP (unless the GIDs were
specifically configured to be ignored by the SSSD or winbindd).
To a user with a slow connection to their LDAP server, this would
appear as if groupadd -r was hung. (Though it would eventually
complete).
This patch changes the algorithm to be more favorable for LDAP
environments, at the expense of some performance when using nss_db.
Given that the DB is a local service, this should have a negligible
effect from a user's perspective.
With the new algorithm, we simply first iterate through all entries
in the local database with gr_next(), recording the IDs that are in
use. We then start from the highest presumed-available entry and
call getgrgid() to see if it is available. We continue this until
we come to the first unused GID. We then select that and return it.
If we make it through all the remaining IDs without finding a free
one, we start over from the beginning of the range and try to find
room in one of the gaps in the range.
The patch was originally written by Stephen Gallagher and applied
identically also to the user allocation by Tomáš Mráz.
Signed-off-by: Serge Hallyn <serge@hallyn.com>
The useradd application resets the user data in /var/log/faillog, if it
exists and a new user is created.
pam_tally2 is used in many distributions.
Check for /var/log/tallylog and reset the user there.
Patch was written by Josef Moellers <jmoellers@suse.de>.
https://bugzilla.suse.com/show_bug.cgi?id=980486
Otherwise we get build warnings like:
sgroupio.c:255:6: warning: implicit declaration of function 'getdef_bool' [-Wimplicit-function-declaration]
shadowio.c:131:6: warning: implicit declaration of function 'getdef_bool' [-Wimplicit-function-declaration]
Enable the automake feature to produce silent output by default.
When compiling code, we now see things like:
$ make
CC addgrps.o
CC age.o
CC audit_help.o
...
This can be disabled via configure's --disable-silent-rules or
by passing V=1 to make.
Custom output (like in the man subdirs) don't (yet) respect this
feature. More work will be needed to clean those up.
Since xz is fairly common nowadays, and is typically smaller/faster than
bzip2 for people to decompress, switch shadow over too. We also merge
the two init locations into configure.ac to match newer autotools style.
The min automake version is bumped to 1.11 too since that's when xz was
released.
The autoconf/automake guys want AC_INIT to be passed the details of the
package directly rather than going through AM_INIT_AUTOMAKE. Update them
both to use the newer style.
This also allows us to pass in contact details for the project.
We set the minimum autoconf version to 2.64 as that's the first one to
support passing the homepage URL in to AC_INIT. That's a pretty old
release by now, so it shouldn't be a problem.
These assignments were pasted as is into the Makefile and
ended up as part of a rule. (Usually the .PRECIOUS rule
which is why the build system never attempted to execute it
as commands, hiding the problem.)
Signed-off-by: Wolfgang Bumiller <wry.git@bumiller.com>
Reported-by: Rahel A <ra00177@surrey.ac.uk>
Should have been: '[...] but only checkS [...]'.
So there was a missing 's'. Architectures isn't the right word either.
I decided to write the whole sentence new.
Some of the supplied tools use functions which are not signal-safe.
Most of the times it's exit() vs. _exit().
In other times it's how the standard output or standard error is
handled. FILE-related functions shall be avoided, therefore I replaced
them with write().
Also there is no need to call closelog(). At worst, it allows to
trigger a deadlock by issuing different signal types at bad timings.
But as these fixes are about race conditions, expect bad timings in
general for these bugs to be triggered. :)
Catch up with Automake's [1], which was part of v1.6b, cut 2002-07-28
[2]. Avoids:
$ autoreconf -v -f --install
...
libmisc/Makefile.am:4: warning: 'INCLUDES' is the old name for 'AM_CPPFLAGS' (or '*_CPPFLAGS')
...
src/Makefile.am:10: warning: 'INCLUDES' is the old name for 'AM_CPPFLAGS' (or '*_CPPFLAGS')
...
Consolidating with the earlier AM_CPPFLAGS avoids:
$ autoreconf -v -f --install
src/Makefile.am:72: warning: AM_CPPFLAGS multiply defined in condition TRUE ...
src/Makefile.am:10: ... 'AM_CPPFLAGS' previously defined here
autoreconf-2.69: Leaving directory `.'
[1]: http://git.savannah.gnu.org/cgit/automake.git/commit/?id=1415d22f6203206bc393fc4ea233123ba579222d
Summary: automake.in (generate_makefile): Suggest using AM_CPPFLAGS instead of INCLUDES
Date: 2002-07-09
[2]: http://git.savannah.gnu.org/cgit/automake.git/tag/?id=Release-1-6b
This functionality is useful because there is now a feature
of Linux-PAM's pam_lastlog module to block expired users (users
which did not login recently enough) from login. This commit
complements it so the sysadmin is able to unblock such expired user.
Signed-off-by: Tomáš Mráz <tmraz@fedoraproject.org>
We intend to not create subuids for system users. However we are
checking for command line flags after we check whether -r flag
was set, so it was never found to be true. Fix that.
Signed-off-by: Serge Hallyn <serge.hallyn@ubuntu.com>
- Use an allocation of 65536 uids and gids to allow for POSIX-compliant
user owned namespaces.
- Don't allocate a uid/gid map to system users.
Unfortunately checking for --system isn't quite enough as some
distribution wrappers always call useradd without --system and take care
of choosing a uid and gid themselves, so also check whether the
requested uid/gid is in the user range.
This is taken from a patch I wrote for Ubuntu a couple years ago and
which somehow didn't make it upstream.
Signed-off-by: Stéphane Graber <stgraber@ubuntu.com>
The current implementation of subuid/subgid support in usermod requires the
user to be a local user present in /etc/passwd. There doesn't seem to be a
good reason for this; subuids should work equally well for users whose
records are in other NSS databases.
Bug-Ubuntu: https://bugs.launchpad.net/bugs/1475749
Author: Steve Langasek <steve.langasek@ubuntu.com>
Acked-by: Serge Hallyn <serge.hallyn@ubuntu.com>
The functions __gr_dup and __pw_dup do not explicitly zero the
memory which hold the passwords after free. The gr_free and pw_free
functions do this explicitly.
To guarantee same behaviour, it's possible to call these *_free
functions directly from __*_dup, because the memory is initialized
with zeros at the beginning. Calling free(NULL) has no negative
effect and can be considered safe these days.
This is helpful when using configuration management tools such as
Puppet, where you are managing the groups in a central location and you
don't need this safeguard.
Signed-off-by: "Jesse W. Hathaway" <jesse@mbuki-mvuki.org>
Acked-by: Serge Hallyn <serge.hallyn@ubuntu.com>
Currently the error is just:
newuidmap: Target [pid] is owned by a different user
With this patch it will be like:
newuidmap: Target [pid] is owned by a different user: uid:0 pw_uid:0 st_uid:0, gid:0 pw_gid:0 st_gid:99
Why is this useful? Well, in my case...
The grsecurity kernel-hardening patch includes an option to make parts
of /proc unreadable, such as /proc/pid/ dirs for processes not owned by
the current uid. This comes with an option to make /proc/pid/
directories readable by a specific gid; sysadmins and the like are then
put into that group so they can see a full 'ps'.
This means that the check in new[ug]idmap fails, as in the above quoted
error - /proc/[targetpid] is owned by root, but the group is 99 so that
users in group 99 can see the process.
Some Googling finds dozens of people hitting this problem, but not
*knowing* that they have hit this problem, because the errors and
circumstances are non-obvious.
Some graceful way of handling this and not failing, will be next ;) But
in the meantime it'd be nice to have new[ug]idmap emit a more useful
error, so that it's easier to troubleshoot.
Thanks!
Signed-off-by: Hank Leininger <hlein@korelogic.com>
Signed-off-by: Serge Hallyn <serge.hallyn@ubuntu.com>
passwd, shadow, group, gshadow etc. can be managed via nss -
e.g. system default accounts can be specified using nss_altfiles,
rather than in /etc/. Thus despite having default accounts, these
files can be missing on disk and thus should be opened with O_CREATE
whenever they are attempted to be opened in O_RDWR modes.
When compiled with PAM certain settings are not used, however they are
still defined in the stock login.defs file. Thus every command reports
them as "unknown setting contact administrator".
Alternative would be to parse stock login.defs and comment out/remove
settings that are not applied, when compiled with PAM.
Prevent chmod failure message from displaying if the failure
was due to the backup file not existing.
If there is no backup file present and if no changes have been
made, then this error would always appear since the backup
file isn't created in this situation.
Signed-off-by: Duncan Eastoe <deastoe@Brocade.com>
Signed-off-by: Serge Hallyn <serge.hallyn@ubuntu.com>
When referring to USERGROUPS_ENAB, the German mentions /etc/default/useradd
when it should be /etc/login.defs (like the original English does).
Reported-by: Stefan Kiesler <heavymetal@gmx.de>
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
Change suggested by Nicolas François as performance optimization.
Performance penalty would be really noticeable when usernames are
stored in remote databases (ldap).
Signed-off-by: Serge Hallyn <serge.hallyn@ubuntu.com>
Until now only exact username specification in /etc/sub[ug]id file allowed the
mapping. This prevented normal use for those users who use multiple usernames
with the same UID, as it rejected mapping even though it was allowed for
another username with the same UID.
This patch initially retains the old behaviour, for performance's sake. In the
first pass, new[ug]idmap only searches for exact username match.
If that yields no valid results, it continues into another loop, which does UID
resolution and comparison. If either definition (numeric UID mapping
specification or mapping specification for another username with the same UID as
current username) is found, it is used.
Signed-off-by: Serge Hallyn <serge.hallyn@ubuntu.com>
We're losing the svn history (which we could probably keep if we tried
hard enough) but don't consider that worthwhile.
Note these tests are destructive, so run them only in a throwaway
environment like a chroot, container, or vm.
The tests/run.all script should be the one which launches all the tests.
Signed-off-by: Serge Hallyn <serge.hallyn@ubuntu.com>
This built-in check is simpler than the previous method and, most
importantly, works when cross-compiling.
Signed-off-by: Serge Hallyn <serge.hallyn@ubuntu.com>
Currently shadow fails to build from source and is flagged as
out-of-date. This is due to a usage of PATH_MAX, which is not defined
on GNU/Hurd. The attached patch solves this problem by allocating a
fixed number of 32 bytes for the string proc_dir_name in files
src/procuidmap.c and src/procgidmap.c. (In fact only 18 bytes are
needed)
Signed-off-by: Serge Hallyn <serge.hallyn@ubuntu.com>
Users may otherwise be confused and think that because the kernel
does not restrict uid mappings to the root user (within his
current uid mappings), newuidmap will ignore /etc/subuid for the
root user. It will not.
Reported-by: Philippe Grégoire <gregoirep@hotmail.com>
Signed-off-by: Serge Hallyn <serge.hallyn@ubuntu.com>
Acked-by: "Eric W. Biederman" <ebiederm@xmission.com>
The number of ranges should be the ceiling of the number of arguments divided
by three.
Without this fix newuidmap and newgidmap always report and error and fail,
which is very much not what we want.
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
Signed-off-by: Serge Hallyn <serge.hallyn@ubuntu.com>
* man/login.defs.d/SUB_GID_COUNT.xml: Document newusers behavior
when the user already have subordinate group IDs.
* man/login.defs.d/SUB_UID_COUNT.xml: Likewise.
* man/login.defs.d/SUB_GID_COUNT.xml: Fix typo (MAX<->MIN).
* man/login.defs.d/SUB_UID_COUNT.xml: Likewise.
* libmisc/find_new_sub_gids.c: Remove dead code.
find_new_sub_gids() is always called with *range_count set to 0.
It's more difficult to keep the subordinate GIDs and UIDs
synchronized, than for the user's UID/GId because the count of
subordinate IDs may differ.
* libmisc/find_new_sub_uids.c: Likewise.
* lib/subordinateio.h, lib/subordinateio.c: Remove APIs that are
no more needed: is_sub_uid_range_free(), is_sub_gid_range_free(),
is_range_free().
* src/su.c: Terminate the child (if needed) before closing the PAM
session. This is probably more correct, and avoid reporting
termination from signals possibly sent by PAM modules (e.g. former
versions of pam_systemd). Debian#670132
* src/su.c: When a SIGTSTP is caught, reset caught to 0. There is
no need to kill the child in such case after su is resumed. This
remove the "Session terminated, terminating shell...
...terminated." messages in such case.
* libmisc/idmapping.h: Document what the upper and lower fields
are in struct map_range.
* man/newgidmap.1.xml: Document when the gid, gidlower and count
argument are.
* man/newuidmap.1.xml: Likewise for uid, uidlower and count.
* libmisc/salt.c (shadow_random): Use long instead of size_t.
Compatibility with size_t is easier to check since it's used for
smaller numbers (salt size).
* man/login.defs.d/SUB_GID_COUNT.xml: Document that the behavior
of useradd and newusers depends on the existence of /etc/subgid.
* man/login.defs.d/SUB_UID_COUNT.xml: Likewise for /etc/subuid.
* src/useradd.c: Change message in case of find_new_sub_uids /
find_new_sub_gids failure. This complements the messages already
provided by these APIs.
* configure.in: Add configure options --enable-subordinate-ids /
--disable-subordinate-ids. Enabled by default.
* lib/prototypes.h: Include <config.h> before using its macros.
* lib/commonio.h, lib/commonio.c: Define commonio_append only when
ENABLE_SUBIDS is defined.
* lib/prototypes.h, libmisc/find_new_sub_gids.c,
libmisc/find_new_sub_uids.c: Likewise.
* lib/subordinateio.h, lib/subordinateio.c: Likewise.
* libmisc/user_busy.c: Only check if subordinate IDs are in use if
ENABLE_SUBIDS is defined.
* src/Makefile.am: Create newgidmap and newuidmap only if
ENABLE_SUBIDS is defined.
* src/newusers.c: Check for ENABLE_SUBIDS to enable support for
subordinate IDs.
* src/useradd.c: Likewise.
* src/userdel.c: Likewise.
* src/usermod.c: Likewise.
* man/Makefile.am: Install man1/newgidmap.1, man1/newuidmap.1,
man5/subgid.5, and man5/subuid.5 only if ENABLE_SUBIDS is defined.
* man/fr/Makefile.am: Install man1/newgidmap.1, man1/newuidmap.1,
man5/subgid.5, and man5/subuid.5 (not translated yet).
* man/generate_mans.mak: Add xsltproc conditionals
subids/no_subids.
* man/login.defs.d/SUB_GID_COUNT.xml: Add dependency on subids
condition.
* man/login.defs.d/SUB_UID_COUNT.xml: Likewise.
* man/usermod.8.xml: Document options for subordinate IDs and
reference subgid(5) / subuid(5) depending on the subids condition.
rlogin support is old school code, so newer C libraries drop support for
ruserok (like uClibc, optionally). But shadow doesn't build with that,
so have it check for ruserok to optionally enable rlogin.
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
* lib/groupmem.c (__gr_dup): Support libc which define other
fields in struct group.
* lib/pwmem.c: Likewise for struct passwd.
* lib/shadowmem.c: Likewise for struct spwd.
* lib/sgroupio.c: Apply same logic, even if this structure is
defined internally.
In find_new_sub_{u,g}ids, check for min, count and max values.
In idmapping.c:get_map_ranges(), make sure that the value passed
in for ranges did not overflow. Couldn't happen with the current
code, but this is a sanity check for any future potential mis-uses.
Signed-off-by: Serge Hallyn <serge.hallyn@ubuntu.com>
subordinate_parse is supposed to return a static structure that
represents one line in /etc/subuid or /etc/subgid. I goofed and
failed to make the variable rangebuf that holds the username of
in the returned structure static.
Add this missing static specification.
Author: <Eric W. Biederman" <ebiederm@xmission.com>
Signed-off-by: Serge Hallyn <serge.hallyn@ubuntu.com>
These files list the set of subordinate uids and gids that users are allowed
to use. The expect use case is with the user namespace but other uses are
allowed.
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
To support files that do not have a simple unique key implement
commonio_append to allow new entries to be added.
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
* libmisc/salt.c: random() max value is 2^31-1 (same as RAND_MAX
on GNU). As it is not clear whether on some systems the max value
can exceed this number and whether some systems have max values
which would be lower, we take this into account when defining the
salt size and number of rounds for SHA encrypted passwords. Higher
values are favored.
Review b10cba0e0a and 7a16f4582d:
* lib/groupio.c (merge_group_entries): Do not allocate more than
necessary (sizeof char* instead of char).
Thanks for Tomáš Mráz (alioth#313962)
* lib/groupio.c (merge_group_entries): Document that new_members
is correctly NULL terminated. (alioth:#313940)
Align previous ChangeLog entries.
* Changelog: Update documentation of 2013-07-28 mancha entry.
* lib/prototypes.h, lib/encrypt.c: Update splint marker,
pw_encrypt can return NULL.
* lib/encrypt.c: Fix outdated statement on GNU crypt.
* src/chgpasswd.c: Improve diagnostic to user when pw_encrypt
fails and use fail_exit() instead of exit().
* src/chpasswd.c: Likewise.
* src/newusers.c: Likewise.
* src/passwd.c: Likewise when new password is encrypted.
* src/newgrp.c: Improve diagnostic to user and syslog when
pw_encrypt fails. Do not apply 1s penalty as this is not an
invalid password issue.
* src/passwd.c: Likewise when password is checked.
* libmisc/setupenv.c: xstrdup the static char* temp_pw_dir /
temp_pw_shell. That way we can continue to use pw_free() without
segving. Thanks to Serge Hallyn for the patch.
a salt that violates specs. On Linux, crypt() also fails with
DES/MD5 salts in FIPS140 mode. Rather than exit() on NULL returns
we send them back to the caller for appropriate handling.
man5, man8 subdirectories. This fix the generation of .so links
which did not point to a path relative to the top-level manual
hierarchy.
* man/generate_mans.mak: Update man paths accordingly.
* man/Makefile.am: Likewise.
* man/da/Makefile.am: Likewise.
* man/de/Makefile.am: Likewise.
* man/fr/Makefile.am: Likewise.
* man/it/Makefile.am: Likewise.
* man/pl/Makefile.am: Likewise.
* man/ru/Makefile.am: Likewise.
* man/sv/Makefile.am: Likewise.
* man/zh_CN/Makefile.am: Likewise.
enabled. This is not done by pam_lastlog. This was broken on
2011-07-23.
* NEWS, libmisc/utmp.c: Do not log in wtmp when PAM is enabled.
This is done by pam_lastlog.
* lib/nscd.c: Add missing newline to error message.
* lib/nscd.c: If nscd is installed but not in use, then running
`nscd -i` will exit(1). We shouldn't warn in this case since this
is not abnormal behavior.
libmisc/copydir.c, lib/selinux.c: Move set_selinux_file_context()
and reset_selinux_file_context() from libmisc/copydir.c to
lib/selinux.c.
* lib/commonio.c: Use set_selinux_file_context() and
reset_selinux_file_context() instead of using the existing
database SELinux context to set the context for the newly created
files.
* lib/commonio.c: Avoid multiple statements per line.
* lib/commonio.c: Ignore fclose return value when the file was
open read only or was not changed, or if an error is already
reported.
annotations.
* src/pwconv.c, src/pwunconv.c, src/grpconv.c, src/grpunconv.c:
Ignore return value of spw_rewind, pw_rewind, sgr_rewind, and
gr_rewind.
* lib/commonio.h: Both head and tail cannot be owned. Set tail as
dependent.
* src/expiry.c: Ignore return value of expire ().
* src/expiry.c: The catch_signals function does not use its sig
parameter.
* src/userdel.c: Last audit_logger parameter is a
shadow_audit_result, use SHADOW_AUDIT_FAILURE instead of 0.
src/chsh.c, src/groupadd.c, src/groupdel.c, src/groupmems.c,
src/groupmod.c, src/newusers.c, src/useradd.c, src/userdel.c,
src/usermod.c: Provide the PAM error
message instead of our own, and log error to syslog.
* src/groupmems.c: Exit with exit rather than fail_exit in usage().
* src/newusers.c: Check the number of arguments.
* src/newusers.c: Do not create the home directory when it is not
changed.
* src/useradd.c: Set the group password to "!" rather "x" if there
are no gshadow file.
* src/pwck.c: optind cannot be greater than argc.
* src/pwck.c: If spw_opened, then is_shadow is implicitly set.
* src/pwck.c: Do not report passwd entry without x password and a
shadow entry in --quiet mode (no interaction with the caller)
* src/pwck.c: Do not check if the last password change is in the
future if the time is set to 0.
audit after the potential chroot.
* src/groupadd.c: Check atexit failures.
* src/groupadd.c: Return E_SUCCESS instead of exit'ing at the end
of main().
po/POTFILES.in: Add process_root_flag() to process the --root
option and chroot so that the chroot config is used and changes
are applied to the chroot.
man/sv/Makefile.am, man/pl/Makefile.am, man/zh_CN/Makefile.am,
man/it/Makefile.am, man/Makefile.am: Include man_nopam in
EXTRA_DIST only when USE_PAM (they are already in man_MANS
otherwise).
* lib/prototypes, libmisc/basename.c (Basename): Input is a
constant string.
* lib/prototypes.h, lib/spawn.h, lib/spawn.c, src/userdel.c,
lib/nscd.c, lib/Makefile.am: Delete spawn.h. Move from spawn.h to
prototypes.h.
* src/userdel.c: Remove unused variables.
* lib/nscd.c: Remove unused header files.
* lib/nscd.c: Add the program name to error messages.
* lib/nscd.c: Indicate when nscd does not terminate normally (signal).
* lib/spawn.c: Updated header.
* lib/spawn.c: Flush stdout and stderr to avoid inheriting from
ongoing buffers.
* lib/spawn.c: Avoid implicit conversion of pointer to boolean.
* lib/spawn.c: Replace perror by a complete message.
* lib/spawn.c: Continue to wait for the child if another child
terminates.
* lib/prototypes.h: The name field from cleanup_info_mod is a
constant string. (username).
* src/grpconv.c: At the end of main, the passwd and shadow files
are locked. No need to check before unlocking. No need to set the
lock as false neither since there cannot be anymore failures.
strtoday(). But we need to support "-1" specifically.
* src/chage.c: Fix usage: LOGIN is mandatory.
* src/chage.c: Display disabled expiry or last change as "-1"
instead of 1969-12-31. 1969-12-31 is still supported as input from
the user.
* src/chage.c: Exit cleanly with fail_exit() (lock files were not
removed).
USER_DEFAULTS_FILE.
* src/useradd.c: Fix cut&paste issue causing bad warning when
the useradd.default file contains an invalid INACTIVE= value.
* src/useradd.c: Added missing end of line for rename errors.
* src/useradd.c: Added -D synopsis to the usage message.
* src/useradd.c: Do not scale_age(-1), just use -1.
* src/useradd.c: Added FIXME to be fixed later.
* src/useradd.c: Allow -e -1 when there is no shadow file.
* src/useradd.c: Fail, but do not print the usage message when the
-e argument is not valid.
* src/useradd.c: No need to check for oflg since uflg is
already checked.
save_caller_context() is allocated and freed.
* src/su.c: Added missing #endif indication
* src/su.c save_caller_context(): password only needed if
SU_ACCESS and !USE_PAM.
instead of 'x'. Only when it is confirmed that a shadow entry is
(will be) added, set the passwd's password to 'x'.
* src/newusers.c: An invalid line is an error. A failure needs to
be reported.
* lib/Makefile.am: Added lib/spawn.c and lib/spawn.h.
* lib/nscd.c, lib/spawn.c, lib/spawn.h: It is not possible to
differentiate between an nscd failure, and a failure to execute
due to no nscd with posix_spawn. Use our own run_command routine.
* src/userdel.c: Use run_command()
* src/usermod.c (date_to_str): buf needs to be unique (e.g.
independent from negativ), and is an out buffer.
* src/usermod.c: Ignore return value from snprintf, and force
nul-termination of buffer.
* src/usermod.c: Improve memory management.
* src/usermod.c: An audit bloc was not reachable, moved above on
success to move the home directory.
* src/usermod.c: Ignore close() return value for the mailbox
(opened read only).
set to 'x' in passwd and there are no entry in shadow for the
user.
* NEWS, src/chgpasswd.c: Create a gshadow entry if the password is
set to 'x' in group and there are no entry in gshadow for the
group.
set to 'x' in passwd and there are no entry in shadow for the
user.
* NEWS, src/chgpasswd.c: Create a gshadow entry if the password is
set to 'x' in group and there are no entry in gshadow for the
group.
options are provided.
* src/pwunconv.c: Re-indent.
* src/pwunconv.c: Open the shadow file read only.
* src/grpunconv.c: Exit after printing usage when arguments or
options are provided.
* src/grpunconv.c: Open the gshadow file read only.
not return.
* src/chpasswd.c: Reindent.
* src/chpasswd.c: Remove dead code. No need to set crypt_method
to NULL when it is already NULL. sflg is only set if crypt_method
is not NULL.
return.
* src/faillog.c: Fix message: this is faillog, not lastlog.
* src/faillog.c: Check that there are no extra arguments after
parsing the options.
not return.
* src/chgpasswd.c: Split usage in smaller parts. Those parts are
already translated for chpasswd. Usage is now closer to
chpasswd's.
* src/chgpasswd.c: Remove dead code. No need to set crypt_method
to NULL when it is already NULL. sflg is only set if crypt_method
is not NULL.
* src/grpck.c: Avoid implicit conversion of pointer to boolean.
* src/grpck.c: Remove dead code. argc cannot be lower than optind.
Avoid checking twice in a row for NULL != list[i].
invalid configuration.
* libmisc/find_new_gid.c, libmisc/find_new_uid.c: Updated
comments.
* libmisc/find_new_gid.c, libmisc/find_new_uid.c: Be more strict
on the loop stop conditions. Stop if we passed the limit, even if
the limit itself was never noticed.
indicate why locking failed.
* NEWS, lib/commonio.c: Fix the sort algorithm in case of NIS. NIS
entries were dropped.
* lib/commonio.c: NIS entries can start by '+' or '-'.
exists but there are no shadow entries, an entry has to be created
if the password is changed and passwd requires a shadow entry, or
if aging features are used (-e or -f). Document this and also that
-e and -f require a shadow file.
provided. Update the error message.
* src/usermod.c (process_flags): Check option compatibility and
dependency before options are discarded when no changes are
requested.
* src/su.c: Added function prototypes.
* src/su.c: Rename shellstr parameter to shellname to avoid
collision with static variable.
* NEWS, src/su.c: Added support for PAM modules which change
PAM_USER.
child and listening for signal in the parent from run_shell().
prepare_pam_close_session() is now executed before the creation of
the pam session and before the UID is changed. This allows to
close the session as root.
use the pwent global variable to communicate between APIs of
different files. Added boolean parameter su_to_root to
check_su_auth().
* src/su.c (check_perms): Return the passwd entry of the finally
authenticated user. Remove usage of the pwent variable.
* src/su.c: The password of the caller is the one from the
spwd structure only if the passwd's password is 'x'.
after the switch to the final subsystem. The previous architecture
forced to always change the environment (the shell starts with a
'*' and was thus restricted, and change_environment could not be
reset to false).
definition of shellstr, PATH and IFS is not influenced (getenv,
getdef, restricted_shell) by and does not influence (addenv does
not change environ) the authentication. And the authentication
does not overwrite those definitions. This will ease an extraction
from the big main() function.
commands executed with -c. This prevents tty hijacking which could
lead to execution with the caller's privileges. This required to
forward signals from the terminal (SIGINT, SIGQUIT, SIGTSTP) to
the executed command.
Makefile.
* man/Makefile.am: Build zh_CN pages.
* man/generate_translations.mak: Add config.xml to CLEANFILES.
* man/po/zh_CN.po: limits, groups, faillog, expiry should not be
translated (command name, file name), also this broke the build
system as they are used to derive manpage names.
Makefile.
* man/Makefile.am: Build zh_CN pages.
* man/generate_translations.mak: Add config.xml to CLEANFILES.
* man/po/zh_CN.po: limits, groups, faillog, expiry should not be
translated (command name, file name), also this broke the build
system as they are used to derive manpage names.
entry validity before commits to databases.
* libmisc/fields.c, libmisc/Makefile.am, lib/fields.c,
lib/Makefile.am, po/POTFILES.in: fields.c moved from libmisc to
lib.
for autogen.sh)
* configure.in, man/po/Makefile.in.in, man/po/Makefile.in:
Makefile.in.in replaced by Makefile.in to avoid config.status
issues when there are multiple po directories.
* man/po/LINGUAS: Adapted to above change.
* man/po/POTFILES.in, man/po/XMLFILES: POTFILES.in replaced by
XMLFILES.
* man/Makefile.am: generate_mans.deps shall be included by make,
not automake.
* libmisc/copydir.c: Missing parenthesis in comment.
* libmisc/chowndir.c: Fixed memory leak on failed realloc().
* libmisc/chowndir.c: Make sure the buffer for the path is large
enough.
* libmisc/remove_tree.c: Remove check for NULL before free().
selinux_file_context renamed set_selinux_file_context.
* lib/protoypes.h, libmisc/copydir.c, src/useradd.c:
Added reset_selinux_file_context.
* src/useradd.c: Check the return value of
set_selinux_file_context and reset_selinux_file_context.
* libmisc/copydir.c: Check the return value of
reset_selinux_file_context.
boolean.
* libmisc/basename.c: Avoid implicit conversion of pointer to
boolean.
* libmisc/basename.c, lib/prototypes.h (Basename): Return a
constant string.
* libmisc/basename.c, libmisc/obscure.c, lib/prototypes.h,
libmisc/xmalloc.c, libmisc/getdate.h, libmisc/system.c,
libmisc/getgr_nam_gid.c, libmisc/failure.c, libmisc/valid.c: Add
splint annotations.
* libmisc/chowndir.c: Avoid memory leak.
* libmisc/chowndir.c: Do not check *printf/*puts return value.
* libmisc/chowntty.c: Avoid implicit conversion between integer
types.
* libmisc/obscure.c: Return a bool when possible instead of int.
* libmisc/shell.c: Do not check *printf/*puts return value.
* libmisc/shell.c: Do not check execle return value.
* libmisc/setupenv.c: Avoid implicit conversion between integer
types.
* libmisc/xmalloc.c: size should not be zero to avoid returning
NULL pointers.
* libmisc/hushed.c: Do not check *printf/*puts return value.
* libmisc/system.c: Avoid implicit conversion of integer to
boolean. safe_system last argument is a boolean.
* libmisc/system.c: Check return value of dup2.
* libmisc/system.c: Do not check *printf/*puts return value.
* libmisc/system.c: Do not check execve return value.
* libmisc/salt.c: Do not check *printf/*puts return value.
* libmisc/loginprompt.c: Do not check gethostname return value.
* libmisc/find_new_gid.c, libmisc/find_new_uid.c: Do not check
gr_rewind/pw_rewind return value.
* libmisc/ttytype.c: Limit the number of parsed characters in the
sscanf format.
* libmisc/ttytype.c: Test if a type was really read.
* libmisc/sub.c: Do not check *printf/*puts return value.
* libmisc/sub.c: Avoid implicit conversion of integer to boolean.
* src/userdel.c: Fix typo in comment.
* src/userdel.c: Avoid implicit conversion of boolean to integer.
* src/userdel.c: safe_system last argument is a boolean.
* src/newusers.c: Avoid implicit conversion of boolean to integer.
* src/newusers.c: Avoid implicit conversion of integer to boolean.
* src/usermod.c: Add brackets.
* src/usermod.c: Avoid implicit conversion of characters or
integers to booleans.
* src/vipw.c: Avoid implicit conversion of integer to boolean.
* src/su.c: Avoid implicit conversion of integer to boolean.
* src/su.c: Add brackets.
* src/useradd.c: Avoid implicit conversion of characters or
integers to booleans.
instead of an int.
* libmisc/obscure.c, libmisc/tz.c, src/passwd.c, lib/encrypt.c,
libmisc/copydir.c, lib/prototypes.h: Add splint annotations.
* libmisc/tz.c: Fix some const issues.
* libmisc/tz.c: Avoid multi-statements lines.
* libmisc/tz.c: Add brackets.
* libmisc/copydir.c: Do not check *printf/*puts return value.
* libmisc/copydir.c: Fail if we cannot set or reset the SELinux
fscreate context.
* libmisc/copydir.c: Use xmalloc instead of malloc.
* libmisc/copydir.c: Do not check lutimes return value
* src/vipw.c: Avoid implicit conversion of integer to boolean.
* src/su.c (iswheel): Return a bool instead of an int.
* src/passwd.c: Remove insert_crypt_passwd(). Use xstrdup instead.
* src/passwd.c: Return constant strings when sufficient.
* src/passwd.c: Do not check *printf/*puts return value.
* src/passwd.c: Avoid implicit conversion of character to boolean.
* src/passwd.c: Do not check sleep return value.
* src/sulogin.c: Do not check *printf/*puts return value.
* lib/encrypt.c: Do not check fprintf return value.
copy the extended attributes (the SELinux context is hence reset)
* src/usermod.c: Copy the extended attributes.
* src/useradd.c: Only reset the SELinux context.
libmisc/sulog.c, libmisc/hushed.c, libmisc/failure.c,
libmisc/loginprompt.c, libmisc/ttytype.c,
libmisc/pam_pass_non_interractive.c, src/userdel.c, src/login.c,
lib/commonio.c, lib/commonio.h: Fix some const issues.
* libmisc/motd.c: Avoid multi-statements lines.
* libmisc/motd.c: Support long MOTD_FILE.
* libmisc/list.c, lib/prototypes.h: Revert previous change.
dup_list and is_on_list are used with members as defined for the
group structure, and thus even if the list is not modified, the
list elements cannot be constant strings.
* libmisc/system.c: Avoid C++ comments.
* src/vipw.c: WITH_TCB cannot be tested inside a gettextized
string. Split the Usage string.
* lib/commonio.h: Re-indent.
* src/useradd.c: spool is a constant string.
* src/useradd.c: Set the new copy_tree's paramater 'copy_root' to false
2010-04-04 Nicolas François <nicolas.francois@centraliens.net>
* src/usermod.c: move_home() is only called if mflg is set.
* src/usermod.c: Fail is -m is provided but the old home directory
is not a directory.
* src/usermod.c: Use the previous improvement of copy_tree to
provide better error diagnosis.
* src/usermod.c: When rename() is used, also change the ownership.
* src/usermod.c: Do not change the ownership of the root directory
twice.
* src/usermod.c: When -u is provided, only change the ownership of
the home directory if it is a directory.
* src/usermod.c: Also change ownerships when -g is used.
2010-04-04 Nicolas François <nicolas.francois@centraliens.net>
* lib/prototypes.h, libmisc/copydir.c: Add the old UID and GID to
copy_tree to detect when ownership shall be changed.
* libmisc/copydir.c: Document the behavior when the IDs are set to
-1.
* lib/prototypes.h, libmisc/copydir.c (copy_tree): Add parameter
copy_root.
* libmisc/copydir.c: error() and ctx can be static.
* libmisc/copydir.c (copy_hardlink): Remove parameter src.
2010-04-04 Nicolas François <nicolas.francois@centraliens.net>
* libmisc/chowndir.c: Dynamically allocate memory to support
path longer than 1024 characters.
* libmisc/chowndir.c: Fix typos in documentation.
* libmisc/chowndir.c: Support and document the behavior when a old
or new ID is set to -1.
* libmisc/chowndir.c: Improved error detection when chown fails.
* libmisc/chowndir.c: Harmonize error handling strategy when an
error occurs: stop changing ownership as soon as an error was
detected.
* src/login_nopam.c: Limit the scope of variables end, lineno, i,
str_len.
* src/logoutd.c: Limit the scope of variable c.
* src/vipw.c: Re-indent.
* src/vipw.c: Close the file after the creation of the backup.
* src/useradd.c (set_default): Close input file on failure.
* src/useradd.c: Limit the scope of variables spool, file, fd, gr,
gid, mode.
* src/passwd.c: Limit the scope of variables last and ok.
* src/chage.c: Fix typo (non breaking space).
* src/login.c: Limit the scope of variables erasechar killchar, c,
failed.
* src/groups.c: Limit the scope of variable ngroups, pri_grp, i.
* src/id.c: Limit the scope of variable i.
no more used.
* lib/groupmem.c: Limit the scope of variable i.
* lib/shadow.c: Avoid implicit conversion of pointers and integers
to booleans.
* lib/shadow.c: Added brackets.
* libmisc/limits.c: Limit the scope of variable tmpmask.
* libmisc/copydir.c: Close opened file on failure.
* libmisc/loginprompt.c: Limit the scope of variable envc.
* libmisc/find_new_uid.c, libmisc/find_new_gid.c: Limit the scope
of variable id.
* NEWS, libmisc/limits.c, man/limits.5.xml: Add support for
infinite limits.
* NEWS, libmisc/limits.c, man/limits.5.xml: Add support for @group
syntax.
they are already specified in number of days since Epoch. Return
-2 in case of errors to support the specification of -1.
* src/usermod.c, src/useradd.c: Adapt to the new error value of
strtoday().
* src/chage.c: Remove isnum(). Adapt to the new error value of
strtoday(). Support for numerical dates is moved to strtoday().
* src/pwck.c: Add support for TCB.
* src/pwck.c: Use spw_dbname() instead of spw_file since TCB
changes from a file to another depending on the user. Also use
pw_dbname() for consistency.
-a option.
* NEWS, man/faillog.8.xml, src/faillog.c: Extend the -a option to
the non-display mode. This changes the default behavior of the -l,
-m, -r, -t options when -a is not specified (restrict to existing
users).
man/pwconv.8.xml, man/useradd.8.xml, man/userdel.8.xml,
man/usermod.8.xml, man/vipw.8.xml: Document the usage of the
TCB_AUTH_GROUP, TCB_SYMLINKS, and USE_TCB configuration
parameters.
* man/pwconv.8.xml, man/pwck.8.xml: Document the behavior when
USE_TCB is enabled.
translation.
* lib/tcbfuncs.c: Indicate the name of the program in error
messages. Avoid perror.
* src/useradd.c: Re-indent.
* src/useradd.c: Add more strings for translation. Indicate the
name of the program in error messages.
* src/userdel.c: Re-indent.
* src/userdel.c: Add more strings for translation. Indicate the
name of the program in error messages.
* lib/shadowio.c: Added brackets and parenthesis.
* lib/shadowio.c: Document the sections closed by #endif
* lib/shadowio.c: Avoid negation of comparisons.
* lib/shadowio.c: Avoid implicit conversion of integer to booleans
and booleans to integers.
* src/vipw.c: Avoid implicit conversion of pointers or integers to
booleans.
* src/vipw.c: Added brackets and parenthesis.
* src/vipw.c: Limit the definition of some variables and macros to
the WITH_TCB scope.
* src/vipw.c: Avoid assignment in comparisons.
* src/vipw.c: Replace PASSWD_FILE (resp. GROUP_FILE and
SGROUP_FILE) by pw_dbname () (resp. gr_dbname () and sgr_dbname ()).
This is more consistent with the shadow file handling and may be
useful to allow edition of another partition's files.
* NEWS: Add support for TCB.
* lib/tcbfuncs.h, lib/tcbfuncs.c, lib/Makefile.am: New library to
support TCB.
* lib/prototypes, libmisc/copydir.c (remove_tree): Add boolean
parameter remove_root.
* configure.in: Add conditional WITH_TCB.
* src/userdel.c, src/usermod.c: Add support for TCB. Update call to
remove_tree().
* src/pwconv.c, src/pwunconv.c: Should not be used with TCB enabled.
* src/vipw.c: Add support for TCB. Update call to remove_tree().
* src/useradd.c: Add support for TCB. Open the shadow file outside
of open_files().
* src/chage.c: Add support for TCB.
* src/Makefile.am: Install passwd sgid shadow when TCB is enabled.
* lib/getdefs.c, man/vipw.8.xml, man/login.defs.5.xml,
man/login.defs/TCB_AUTH_GROUP.xml, man/login.defs/USE_TCB.xml,
man/login.defs/TCB_SYMLINKS.xml, man/generate_mans.mak,
man/generate_mans.deps, man/Makefile.am: New configuration
parameters: TCB_AUTH_GROUP, TCB_SYMLINKS, USE_TCB.
* lib/shadowio.c, lib/commonio.c: Add support for TCB.
--preserve-environment. This sanitation was disabled on Debian
since quite some time with no reported issues, and sanitize_env()
documentation agrees that it should be useless as all modern
Unixes will handle setuid executables properly. This Fixes
Alioth#312287.
* src/su.c: Add more messages for translation.
* src/su.c: Ignore kill() return value when sending the TERM
signal. If it fails, a KILL should be sent anyway.
terminate (after sending a SIGTERM), and kill it only if it did
not terminate by itself. No delay will be enforced if the child
cooperates. See http://bugs.gentoo.org/282094
* NEWS, man/su.1.xml: Document su's exit values.
po/Makefile.in.in: Removing and restoring the config.xml file
broke parallel builds. Build the manpages based on *.xml-config
files instead of *.xml files. The *.xml do not include config.xml
anymore, which permits to run xml2po without needing to remove
config.xml. The config.xml is restored in the *.xml-config files.
* man/groupadd.8.xml: Implementation of the above.
* man/generate_mans.deps: Updated dependencies
spwd. (start with the primitive types)
* lib/shadowmem.c: Avoid memzero() on a possibly NULL pointer.
* lib/groupmem.c: Only copy the required fields of the struct
group. (start with the primitive types)
* lib/groupmem.c: Avoid memzero() on a possibly NULL pointer.
* lib/groupmem.c: Free gr_mem in addition to its elements.
* lib/sgroupio.c: The struct sgrp has no primitive types to be
copied initially.
* lib/sgroupio.c: Avoid memzero() on a possibly NULL pointer.
* lib/sgroupio.c: Free sg_mem and sg_add in addition to their
elements.
* lib/pwmem.c: Only copy the required fields of the struct
passwd. (start with the primitive types)
src/chpasswd.c, src/groupmems.c, src/usermod.c, src/chgpasswd.c,
src/vipw.c, src/su.c, src/useradd.c, src/groupmod.c, src/passwd.c,
src/groupadd.c, src/chage.c, src/faillog.c, src/chsh.c: Use
booleans for tests.
* src/userdel.c, src/gpasswd.c, src/groupmems.c, src/usermod.c,
src/groupmod.c, src/passwd.c: Use a break even after usage().
src/newusers.c, src/chpasswd.c, src/groupmems.c, src/usermod.c,
src/chgpasswd.c, src/vipw.c, src/su.c, src/useradd.c,
src/groupmod.c, src/passwd.c, src/groupadd.c, src/chage.c,
src/faillog.c, src/chsh.c: If someone uses the -h/--help options,
the usage should not go to stderr nor should the utility exit with
non-zero status. All of the shadow utils do just this
unfortunately, so convert them over to sanity.
* man/groupmems.8.xml, man/gpasswd.1.xml: Added option -h/--help.
* NEWS, libmisc/find_new_gid.c, libmisc/find_new_uid.c: Since
system accounts are allocated from SYS_?ID_MIN to SYS_?ID_MAX in
reverse order, accounts are packed close to SYS_?ID_MAX if
SYS_?ID_MIN is already used but there are still dome gaps.
getpwent / getgrent for system accounts. Trying the low-IDs with
getpwuid / getgrgid should be more efficient on LDAP configured
systems with many accounts.
libmisc/xgetpwuid.c, libmisc/xgetgrgid.c, libmisc/xgetspnam.c: Do
not limit the size of the buffer to hold the group or user
structure. It used to be limited to 16k, which caused issues with
groups having many users.
* man/su.1.xml: Improve the documentation of the su behavior
regarding environment variables.
* man/su.1.xml: Document that the login.defs file is used.
* po/shadow.pot, man/po/shadow-man-pages.pot: Regenerated.
* po/*.po, man/po/*.po: Updated PO files.
* man/passwd.1.xml: passwd -u does not reset the expiry field.
* NEWS, src/userdel.c: When USERGROUPS_ENAB is enabled, remove the
user's group when the user was the only member. This is still not
complete, as the user could have been specified twice in the
members.
* NEWS, src/userdel.c: Do not fail when -r is used and the home
directory does not exist.
Harmonize formatting of login.defs and default/useradd variables.
Use an <option> tag.
* man/usermod.8.xml: Added reference to gshadow(5).
* man/login.defs.d/USERDEL_CMD.xml: Shorten the lines of the
USERDEL_CMD example.
man/groupadd.8.xml, man/usermod.8.xml, man/chgpasswd.8.xml,
man/groupmod.8.xml: Added warning: passwords set with these tools
may not respect the password policy.
lib/prototypes.h: Move user_busy() to libmisc/user_busy.c.
* NEWS, libmisc/user_busy.c: On Linux, do not check if an user is
logged in with utmp, but check if the user is running some
processes. If not on Linux, continue to search for an utmp record,
but make sure the process recorded in the utmp entry is still
running.
shell as a shell script when the direct execution of the user's
shell failed with ENOEXEC and the user's shell has a shebang. The
interpreter might not be the right one. Executing the user's
shell with sh -c might be better, but I'm not sure we should try
harder when there is a failure. Note: The removed code was only
included #ifndef __linux__.
with a NULL argument.
* src/useradd.c: Replace PATH_MAX by a fixed constant. The buffer
was not meant as a storage for a path.
* src/useradd.c, src/newusers.c, src/chpasswd.c: Better detection
of fgets errors. Lines shall end with a \n, unless we reached the
end of file.
* libmisc/copydir.c: Avoid PATH_MAX. Support file paths with any
length. Added readlink_malloc().
and the password field in passwd is not 'x'.
* src/grpck.c: Warn if a group has an entry in group and gshadow,
and the password field in group is not 'x'.
man/login.defs.d/MD5_CRYPT_ENAB.xml,
man/login.defs.d/SHA_CRYPT_MIN_ROUNDS.xml: Updated note for PAM
enabled versions. These variables are only used for group
passwords in this case.
changing the passwords with PAM.
* src/newusers.c: Split the usage string in smaller parts to
allow enabling single parts.
* man/newusers.8.xml: Indicate the options and configuration
variables valid for PAM and non-PAM versions.
* man/newusers.8.xml: Added pointer to /etc/pam.d/chpasswd.
libmisc/pam_pass_non_interractive.c, libmisc/Makefile.am: Renamed.
* libmisc/pam_pass_non_interractive.c, lib/prototypes.h:
non_interactive_password and non_interactive_pam_conv do not need
to be externally visible.
* libmisc/pam_pass_non_interractive.c: Added declaration of
ni_conv.
* libmisc/pam_pass_non_interractive.c: Only compile ifdef USE_PAM.
* libmisc/pam_pass_non_interractive.c, lib/prototypes.h:
Added do_pam_passwd_non_interractive().
* src/chpasswd.c: Use do_pam_passwd_non_interractive().
* man/chpasswd.8.xml: Describe the PAM enabled chpasswd behavior.
* man/chpasswd.8.xml: Differentiate the files and configurations
needed for PAM and non PAM versions.
instead of only the username.
* src/chpasswd.c: PAM enabled chpasswd do may change the password
database (for the user where the password update succeeded) even
if there were a failure for one user. Do not indicate that changes
were ignored.
unlink() in case of failure of fopen_set_perms() or
create_backup().
* lib/commonio.c: Should the backup file be unlink'ed in case of
failure of create_backup()?
* libmisc/chowntty.c, libmisc/rlogin.c, libmisc/sub.c,
src/newusers.c, libmisc/sulog.c, libmisc/system.c, src/logoutd.c,
src/groups.c, src/id.c, lib/encrypt.c, libmisc/audit_help.c,
libmisc/limits.c: Return EXIT_FAILURE instead of 1, and
EXIT_SUCCESS instead of 0.
* libmisc/audit_help.c: Replace an fprintf() by fputs().
* libmisc/audit_help.c: Remove documentation of the audit_logger
returned values. The function returns void.
* libmisc/system.c: Only return status if waitpid succeeded.
Return -1 otherwise.
with PAM.
* src/chpasswd.c: Split the usage string in smaller parts to
allows enabling single parts.
* src/chpasswd.c: Do not set a global lock on the password files.
This is done by PAM each time a password is updated.
man/useradd.8.xml: Added note to warn about insecurity in using
--password.
* man/groupmod.8.xml: Removed not regarding default if --password
is not used. This was a cut&paste from groupadd.8.xml.
* man/passwd.1.xml: Split some paragraphs.
* man/passwd.1.xml: Recommend other encryption methods than DES.
* libmisc/copydir.c: Added assert to help splint.
* libmisc/copydir.c: Free allocated structures in cas of failure.
* libmisc/copydir.c: Avoid implicit conversion of pointers to
booleans.
* libmisc/copydir.c: Use buffers of size PATH_MAX instead of 1024
for filenames.
* libmisc/copydir.c: Use fchmod and fchown to change the mode of
the opened file.
* libmisc/copydir.c: Indicate the mode to open(), even if we chmod
later.
* src/useradd.c: Replace peror by an strerror and avoid an
intermediate buffer.
* src/useradd.c: Save errno between the failure and the report by
perror/strerror.
* src/useradd.c: Prefer xmalloc to malloc.
limit with getlog() / getulong(). This also means, in case of
non-PAM enabled systems, that the umask specified on the GECOS
fields should start with a 0 if specified in octal. (it used to be
force to octal). Do the appropriate cast and range checking.
lib/pwio.c, lib/pwio.h, lib/shadowio.c, lib/shadowio.h: Added
splint annotations. The *_locate() and *_next() functions
currently return an observer. As the structure are often modified
by the caller, it could maybe be changed to exposed later. (and
non-const).
* src/newgrp.c: audit_buf is only used in newgrp. Make it static.
* src/newgrp.c: Ignore the return value of fputs().
* src/newgrp.c: Use exit(EXIT_FAILURE) instead of exit(1).
* libmisc/setupenv.c: Added brackets and parenthesis.
* libmisc/setupenv.c: Ignore the return value of fclose (file
opened read-only)
* libmisc/setupenv.c: Ignore the return value of puts().
* libmisc/setupenv.c:Avoid implicit conversion of pointers to
booleans.
* libmisc/age.c: Added brackets and parenthesis.
* libmisc/age.c: Ignore the return value of fclose (file opened
read-only)
* libmisc/age.c: Ignore puts() return value.
* libmisc/age.c: Use exit(EXIT_FAILURE) instead of exit(1).
* libmisc/age.c: Avoid assignments in comparisons.
* src/lastlog.c: Added splint annotations.
* src/lastlog.c: Avoid global pwent.
* src/lastlog.c: Cast ID to ulongs and use ulong formats for IDs.
* src/lastlog.c: Avoid assignment in comparisons.
* src/lastlog.c: Ignore fclose() return value since the file is
only opened for reading.
used_gids.
* libmisc/find_new_gid.c: Use getdef_ulong and cast to git_t to
get GID values.
* libmisc/find_new_gid.c: Use UL as a prefix for ulong values.
* libmisc/find_new_uid.c: Likewise.
default to 32.
* libmisc/chkname.c: Use USER_NAME_MAX_LENGTH.
* src/login.c: Use USER_NAME_MAX_LENGTH instead of the default 32.
username also needs to be bigger than USER_NAME_MAX_LENGTH because
it has to be nul-terminated.
change the terminal configuration. setup_tty() is just a best
effort configuration of the terminal.
* src/login.c: Ignore failures when setting the terminal
configuration.
* src/login.c: Fail if the ERASECHAR or KILLCHAR configurations
are not compatible with a cc_t type.
of global utent/utxent variables. Only reuse the ut_id and maybe
the ut_host fields from utmp.
* lib/prototypes.h, libmisc/utmp.c: Removed checkutmp(),
setutmp(), setutmpx().
* lib/prototypes.h, libmisc/utmp.c: Added get_current_utmp(),
prepare_utmp(), prepare_utmpx(), setutmp(), setutmpx().
* libmisc/utmp.c (is_my_tty): Only compare the name of the utmp
line with ttyname(). (No stat of the two terminals to compare the
devices).
* libmisc/utmp.c: Use getaddrinfo() to get the address of the
host.
* configure.in: Check for getaddrinfo().
* configure.in: Use AC_CHECK_MEMBERS to check for the existence of
fields in the utmp/utmpx structures.
* configure.in: Reject systems with utmpx support but no ut_id
field in utmp. This could be fixed later if needed.
* src/login.c: Use the new utmp functions. This also simplifies
the failtmp() handling.
* src/login.c: passwd_free() renamed to pw_free() and
shadow_free() renamed to spw_free()
* lib/shadowio.c: Use spw_free() for shadow_free().
* lib/groupmem.c: Added gr_free().
* lib/groupio.c: Use gr_free() for group_free().
* lib/pwmem.c: Include define.h before prototypes.h
* lib/pwmem.c: Added pw_free().
* lib/pwio.c: Use pw_free() for passwd_free().
* lib/sgroupio.c: Added sgr_free().
* lib/sgroupio.c: Use sgr_free() for gshadow_free().
* lib/prototypes.h: Added gr_free(), pw_free(), sgr_free(),
spw_free().
is always coming from xgetpwnam. There is no need to copy pwd to
pwent, this was not a good idea anyway as the strings from pwd
were not duplicated.
* src/login.c: Always free the pwd and spwd structure when we
retrieve a new one. This will clear the password of the previous
user from the memory.
* src/login.c: user_passwd is used to keep point to the password
of the user being authenticated.
* src/login.c: (non PAM) Fail if the user's entry cannot be found
after the user updated her password (if expire() requested an
update).
* src/login.c: If the user does not exist on the system, there is
no need to build a pwd structure (with shell).
username as first parameter of failtmp to avoid issues with
non-null terminated ut_user, unavailability of ut_user, incomplete
username (that should not happen currently).
username as first parameter of failtmp to avoid issues with
non-null terminated ut_user, unavailability of ut_user, incomplete
username (that should not happen currently).
* groupmems has a testsuite now
* long user names are tested for gpasswd
* groups and id arenot receiving updates
* tests with UID/GID sets to (*id_t)-1 already exist.
terminal termio flags. Reset echoprt, noflsh, tostop. This
behavior seems to have change by mistake in earlier releases
(4.0.8, for no obvious reason).
SELinux user for user's login.
* NEWS, src/usermod.c, man/usermod.8.xml: Likewise.
* libmisc/system.c, libmisc/Makefile.am, lib/prototypes.h: Added
safe_system(). Used to run semanage.
* lib/prototypes.h, libmisc/copydir.c: Make a
selinux_file_context() an extern function.
* libmisc/copydir.c: Reset SELinux to create files with default
contexts at the end of copy_tree().
* NEWS, src/userdel.c: Delete the SELinux user mapping for user's
login.
never set ifndef USE_PAM. Change the prototype of setup_uid_gid()
when USE_PAM is not defined. This permits to remove add_groups
from PAM builds. setup_uid_gid is already subject to
HAVE_INITGROUPS.
ifndef USE_PAM.
* lib/prototypes.h: Remove the declaration of add_cons_grps(). The
function does not exist.
* libmisc/age.c (setup_uid_gid): is_console is never set ifndef
USE_PAM. Change the prototype of setup_uid_gid() when USE_PAM is
not defined. This permits to remove add_groups from PAM builds.
setup_uid_gid is already subject to HAVE_INITGROUPS.
* libmisc/pwd2spwd.c (pwd_to_spwd): pwd_to_spwd() is not used in
PAM builds.
src/passwd.c, src/pwck.c, src/pwconv.c, src/useradd.c,
src/usermod.c: On Jan 01, 1970, do not set the sp_lstchg field to
0 (which means that the password shall be changed during the next
login), but use -1 (password aging disabled).
* src/passwd.c: Do not check sp_min if sp_lstchg is null or -1.
value should be -1 (no aging) rather than 0 (password must be
changed).
* src/chage.c: For password expiration and inactivity, indicate
that the password must be changed when sp_lstchg is null rather
than indicating that expiration and inactivity are not enabled.
database before calling spw_remove().
* NEWS, src/userdel.c: When the user's group is removed, make sure
the group is in the gshadow database before calling sgr_remove().
* src/userdel.c: Improve warning's wording.
expansion. Make sure config.xml does not exist when the POT file
is created in order to keep the configurations in the POT file
* man/generate_translations.mak: make sure config.xml does not
exist neither when the translated XML is generated. Add the
missing %config; (strip out by xml2po). and make sure config.xml
is present when the translated manpage is generated.
* man/generate_mans.mak: config.xml is needed for the generation
of manpages (already in the .deps for the English manpages, but
needed for the translations).
* man/Makefile.am: Added missing CREATE_HOME.xml.
as newgrp.
* man/login.defs.5.xml: vipw does not use any variable.
* man/login.defs.5.xml: In PAM enabled configurations, login still
uses some login.defs variables.
users with -u.
* src/faillog.c: Do not call print_one() for users which do not
exist.
* src/faillog.c: Make sure the user's entry is not outside the
faillog file and initialize the faillog structure in that case.
* src/faillog.c: Move print_one() closer to print().
* src/faillog.c: reset(), setmax(), set_locktime() can also change
entries of user which do not exist.
* src/faillog.c: reset(), setmax() and set_locktime() shall not
create entries for users which have no entries if the value has to
be set to 0.
* src/faillog.c: reset(), setmax() and set_locktime(): better
handling of users whose entry is outside the faillog file.
* src/faillog.c: Improved option handling. Options can now be
specified in any order.
* src/faillog.c: Improved warnings when options are not
compatible or when the faillog cannot be open with the right mode.
* src/faillog.c: Only fstat the faillog file once.
* man/faillog.8.xml: Improved documentation.
compiled when disabled at configuration time.
* src/chgpasswd.c: Make sure the SHA related variables is not
compiled when disabled at configuration time.
* src/chgpasswd.c: Fix the test for getlong() failure.
clash with the structure.
* src/lastlog.c: check the offset in print_one() so that it is
used for the display of one entry or a set of entries.
* src/lastlog.c: Do not loop over the whole user database when -u
is used with a single user.
* src/lastlog.c: Check the size of the lastlog file so that we
can identify failures to read.
when the changes are committed to the system. Do not log failure
for on-memory changes to audit or syslog. Make sure failures and
inconsistencies will be reported in case of unexpected failures
(e.g. malloc failures). Only specify an audit message if it is not
implicitly implied by the type argument. Removed fail_exit
(replaced by atexit(do_cleanups)). Log failures in case of
permission denied.
messages not related to an account.
* lib/prototypes.h, libmisc/cleanup.c, libmisc/cleanup_group.c,
libmisc/cleanup_user.c, libmisc/Makefile.am: Added stack of
cleanup functions to be executed on exit.
* NEWS, src/groupadd.c, src/groupdel.c, src/groupmod.c: Only
report success to audit and syslog when the changes are committed
to the system. Do not log failure for on-memory changes to audit
or syslog. Make sure failures and inconsistencies will be reported
in case of unexpected failures (e.g. malloc failures). Only
specify an audit message if it is not implicitly implied by the
type argument. Removed fail_exit (replaced by atexit(do_cleanups)).
configure option. The configure behavior encoded is:
<no option> -> default of 16 (like today);
--with-group-name-max-length -> default of 16;
--without-group-name-max-length -> no max length;
--with-group-name-max-length=n > max is set to n.
utmp.c to chowntty.c. checkutmp() now only uses an existing utmp
entry if the pid matches and ut_line matches with the current tty.
This fixes a possible DOS when entries can be forged in the utmp
file.
* libmisc/chowntty.c, src/login.c, lib/prototypes.h: Remove the
tty argument from chown_tty. chown_tty always changes stdin and
does not need this argument anymore.
man/grpck.8.xml: Sorted SEE ALSO references.
* man/gshadow.5.xml: Added reference to grpck(8) and grpconv(8).
* man/pwck.8.xml: Added reference to grpck(8).
* man/shadow.5.xml: Added reference to pwck(8).
* man/passwd.5.xml: Added reference to pwck(8).
* man/grpck.8.xml: Added reference to pwck(8).
parameter of -f. Fix the getopt optstring, remove the parsing of
username in the -f processing block, and remove unnecessary checks
(username cannot be parsed twice anymore), better documentation of
the synopsis.
caller, not the user login tries to authenticate.
* src/login.c: Use pwd->pw_name instead of pwd->pw_uid. This might
be more precise (name must be unique, uid might not be).
* src/pwck.c: Only unlock files if they were locked before (e.g.
not in read-only mode).
* src/pwck.c: Quote the username in error messages (harmonization
with other messages).
* libmisc/find_new_gid.c: Fixed typo (s/grp->gr_gid/group_id/).
* libmisc/find_new_gid.c: Likewise.
to create a home directory for new users.
* src/useradd.c, man/useradd.8.xml: New -M/--no-create-home option
and CREATE_HOME usage. System accounts are not impacted by
CREATE_HOME.
* man/useradd.8.xml: Indicate that a new group is created by
default.
* src/useradd.c: Removed TODO item (moved to the TODO file).
Thanks to Peter Vrabec.
* NEWS, libmisc/find_new_gid.c, libmisc/find_new_uid.c: Build an
index of used IDs to avoid a database request for each id in the
allowed range (when the highest allowed ID is already used).
This speedups the addition of users or groups when the highest
allowed ID is already used. The additional memory usage of the
tools should be acceptable when UID_MAX/SYS_UID_MAX are set to a
reasonable number.
* src/userdel.c, src/newusers.c, src/chpasswd.c, src/chfn.c,
src/groupmems.c, src/usermod.c, src/groupdel.c, src/chgpasswd.c,
src/useradd.c, src/groupmod.c, src/groupadd.c, src/chage.c,
src/chsh.c: If the username cannot be determined, report it as
such (not a PAM authentication failure).
--disable-account-tools-setuid options. This permits to disable
the PAM authentication of the caller for chage, chgpasswd,
chpasswd, groupadd, groupdel, groupmod, newusers, useradd,
userdel, and usermod. This authentication is not necessary when
these tools are not installed setuid root.
enable/disable the usage of PAM to authenticate the callers of
account management tools: chage, chgpasswd, chpasswd, groupadd,
groupdel, groupmod, useradd, userdel, usermod.
* src/Makefile.am: Do not link the above tools with libpam if
account-tools-setuid is disabled.
* src/userdel.c, src/newusers.c, src/chpasswd.c, src/usermod.c,
src/groupdel.c, src/chgpasswd.c, src/useradd.c, src/groupmod.c,
src/groupadd.c, src/chage.c: Implement ACCT_TOOLS_SETUID
(--enable-account-tools-setuid).
* etc/pam.d/Makefile.am: Install the pam service file for the
above tools only when needed.
* src/useradd.c, src/userdel.c, src/usermod.c: It is no more
needed to initialize retval to PAM_SUCCESS.
undefining some configuration macros when the file is included
multiple times.
* libmisc/xgetXXbyYY.c, libmisc/xgetpwnam.c, libmisc/xgetpwuid.c,
libmisc/xgetgrgid.c, libmisc/xgetgrnam.c, libmisc/xgetspnam.c:
Include <config.h> from teh compiled C file, not the included
getXXbyYY.c.
src/groupmems.c, src/usermod.c, src/groupdel.c, src/chgpasswd.c,
src/useradd.c, src/groupmod.c, src/groupadd.c, src/chage.c,
src/chsh.c: Simplify the PAM error handling. Do not keep the pamh
handle, but terminate the PAM transaction as soon as possible if
there are no PAM session opened.
src/newusers.c, src/pwconv.c, src/chpasswd.c, src/logoutd.c,
src/chfn.c, src/groupmems.c, src/usermod.c, src/pwunconv.c,
src/expiry.c, src/groupdel.c, src/chgpasswd.c, src/useradd.c,
src/su.c, src/groupmod.c, src/passwd.c, src/pwck.c,
src/groupadd.c, src/chage.c, src/login.c, src/grpconv.c,
src/groups.c, src/grpunconv.c, src/chsh.c: Prog is now global (not
static to the file) so that it can be used by the helper functions
of libmisc.
* lib/prototypes.h: Added extern char *Prog.
* libmisc/find_new_gid.c, libmisc/find_new_uid.c: Indicate the
program name with the warning.
shadow_audit_result for the result argument of audit_logger().
This permits stronger type checking and a better readability of
the results (SHADOW_AUDIT_FAILURE/SHADOW_AUDIT_SUCCESS constants).
* src/groupadd.c, src/groupdel.c, src/useradd.c, src/userdel.c:
Use the SHADOW_AUDIT_FAILURE/SHADOW_AUDIT_SUCCESS results instead
of 0 or 1 in audit_logger().
shadow_audit_result for the result argument of audit_logger().
This permits stronger type checking and a better readability of
the results (SHADOW_AUDIT_FAILURE/SHADOW_AUDIT_SUCCESS constants).
* src/groupadd.c, src/groupdel.c, src/useradd.c, src/userdel.c:
Use the SHADOW_AUDIT_FAILURE/SHADOW_AUDIT_SUCCESS results instead
of 0 or 1 in audit_logger().
audit.
* src/userdel.c: Log successful removal of home directory to audit
only in case of success.
* src/userdel.c: Move the audit log of failure to remove the home
directory before the call to function that may exit.
* src/userdel.c: Document that errors is only used to count errors
during the removal of the home directory.
changes have started to be reported to syslog.
* src/userdel.c: Fix some result parameters sent to
audit_logger().
* NEWS: Following changes from a patch contributed by Steve Grubb
<sgrubb@redhat.com>
* src/groupadd.c: Log to audit with type AUDIT_ADD_GROUP instead
of AUDIT_USER_CHAUTHTOK.
* src/groupdel.c: Log to audit with type AUDIT_DEL_GROUP instead
of AUDIT_USER_CHAUTHTOK.
* src/useradd.c: Log to audit with type AUDIT_ADD_USER /
AUDIT_ADD_GROUP / AUDIT_USYS_CONFIG instead of
AUDIT_USER_CHAUTHTOK.
* src/useradd.c: Add missing logs to audit.
* src/userdel.c: Log to audit with type AUDIT_DEL_USER /
AUDIT_DEL_GROUP instead of AUDIT_USER_CHAUTHTOK.
* src/userdel.c: Add missing logs to audit.
const. Duplicate this structure before working on it.
* src/groupmems.c: Do not fail and do not display warnings if a
close failure happens with the --list option. (Files are opened
read-only).
prototypes of the static functions.
* src/grpconv.c: Fail if unexpected parameters are provided.
* src/grpconv.c: Indicate that argc is not used in the no
SHADOWGRP version.
Include <sys/types.h> before <pwd.h> and <grp.h>. It is necessary
for the definition of uid_t and gid_t.
* lib/pwmem.c: do not include <pwd.h>, "pwio.h" is sufficient
here.
* configure.in, libmisc/copydir.c: futimes() and lutimes() are not
standard. Check if they are implemented before using them. Do not
set the time of links if lutimes() does not exist, and use
utimes() as a replacement for futimes().
the list, not adduser. This fixes a segmentation fault for every
call of groupmems -d.
* libmisc/list.c: Add assertions to help identifying these issues.
* libmisc/list.c: Avoid implicit conversion of pointers to
booleans.
* src/userdel.c: Add log to syslog when the mail file could not be
removed.
* src/userdel.c: Give more context an error message (merge with
perror()).
* src/usermod.c: Harmonize some error messages.
* src/chage.c, src/chfn.c, src/chgpasswd.c, src/chpasswd.c,
src/chsh.c, src/gpasswd.c, src/groupadd.c, src/groupmems.c,
src/groupmod.c, src/grpck.c, src/grpconv.c, src/grpunconv.c,
src/passwd.c, src/pwck.c, src/pwconv.c, src/pwunconv.c,
src/useradd.c, src/userdel.c, src/usermod.c: Harmonize the error
message sent to stderr in case of *_update () failure.
* src/chage.c, src/chsh.c, src/groupadd.c, src/passwd.c: Do not
log to syslog when pw_update() or spw_update() fail.
* src/newusers.c: Do not log specific error message to stderr when
sgr_update() fails.
* src/pwconv.c: Remove duplicated definition of Prog.
src/newgrp.c, src/passwd.c, src/su.c: Use the same stderr and
syslog warnings when the username cannot be determined.
* src/newgrp.c: Reuse the same stderr message for groups which do
not exist in the system.
* src/grpconv.c, src/grpunconv.c, src/pwconv.c, src/pwunconv.c,
src/vipw.c: Open syslog with the right identification name.
* src/vipw.c: Log unlock errors to syslog.
* src/vipw.c: Log edits to syslog.
* src/chage.c, src/chfn.c, src/chsh.c, src/gpasswd.c,
src/groupadd.c, src/groupdel.c, src/groupmod.c, src/grpconv.c,
src/grpunconv.c, src/passwd.c, src/pwck.c, src/pwunconv.c,
src/useradd.c, src/usermod.c: Harmonize the syslog levels. Failure
to close or unlock are errors. Failure to open files are warnings.
support for long options --add (-a), --delete (-d),
--remove-password (-r), --restrict (-R), --administrators (-A),
and --members (-M)
* man/gpasswd.1.xml: Document the new long options.
* src/gpasswd.c: The sgrp structure is only used if SHADOWGRP is
defined.
files. Report errors to stderr and syslog, but continue.
* src/grpconv.c: Check failure to unlock files. Report errors to
stderr and syslog, but continue.
src/groupadd.c, src/groupdel.c, src/groupmems.c, src/groupmod.c,
src/grpconv.c, src/grpunconv.c, src/newusers.c, src/pwconv.c,
src/pwunconv.c, src/useradd.c, src/userdel.c: Harmonize the name
of the variables keeping the lock status, to match the shadow
library prefixes.
src/gpasswd.c, src/groupadd.c, src/groupdel.c, src/groupmems.c,
src/groupmod.c, src/grpck.c, src/grpconv.c, src/grpunconv.c,
src/newusers.c, src/passwd.c, src/pwck.c, src/pwconv.c,
src/pwunconv.c, src/useradd.c, src/userdel.c, src/usermod.c: In
case of a lock failure, indicate to the user that she can try
again later. Do not log to syslog.
the --lock an --unlock options do not lock or unlock the user
account anymore. They only lock or unlock the user's password.
* man/passwd.1.xml: Document above change. Document how an account
can be locked and what a password lock means.
* src/pwck.c: Report failure to unlock files to stderr and
syslog.
* src/pwck.c: Report failure to sort to stderr, and exit with
E_CANTSORT.
* man/pwck.8.xml: Document return code 6 (E_CANTSORT).
* src/groupmems.c: members() renamed display_members() to
avoid name clash with its members argument.
* src/groupmems.c: Report failure to unlock to syslog.
* src/groupmems.c: Harmonize error messages.
* src/groupmems.c: Report failures to write the new group file to
syslog (gr_close() failure).
* src/groupmems.c: Don't use fail_exit for non-failure exit.
* src/chsh.c: Use fail_exit() instead of exit(), this avoid
calling closelog() every times.
* src/chsh.c: Ignore the return value or pam_end().
* src/chsh.c: Simplify the PAM error handling.
* src/chsh.c: Report failure to unlock files to stderr and
syslog.
* src/chpasswd.c: Added support for syslog.
* src/chpasswd.c: Report failure to unlock files to stderr and
syslog.
* src/chpasswd.c: Simplify the PAM error handling.
* src/chpasswd.c: Report failure during *_close() to syslog.
* src/chpasswd.c: Ignore the return value or pam_end().
* src/chgpasswd.c: Added support for syslog.
* src/chgpasswd.c: Report failure to unlock files to stderr and
syslog.
* src/chgpasswd.c: Simplify the PAM error handling.
* src/chgpasswd.c: Report failure during *_close() to syslog.
* src/chgpasswd.c: Ignore the return value or pam_end().
gshadow to stderr.
* src/userdel.c: Fail in case of failure during the write of a
user or group database. Report errors to syslog.
* src/userdel.c: Do not unlock non locked files.
* src/userdel.c: Report failure to unlock the passwd or shadow
file to stderr and syslog.
file to stderr and syslog.
* src/newusers.c: In case of error when files are open or closed,
indicate the failing file.
* src/newusers.c: Do not try to unlock the files manually since
this is done in fail_exit.
locked. Report failures to unlock to stderr and syslog.
* src/chfn.c: Is case of failure, use fail_exit() rather than
exit().
* src/chfn.c: Ignore the return value of pam_end().
lib/sgroupio.c, lib/sgroupio.h, lib/shadowio.c, lib/shadowio.h:
Added *_dbname() functions to retrieve the name of the databases.
* lib/groupio.c, lib/groupio.h, lib/pwio.c, lib/pwio.h,
lib/sgroupio.c, lib/sgroupio.h, lib/shadowio.c, lib/shadowio.h:
*_name() functions renamed *setname().
* src/grpck.c, src/pwck.c: Likewise.
* lib/groupio.h, lib/pwio.h, lib/sgroupio.h, lib/shadowio.h: Added
the name of the arguments to the prototypes.
* src/chage, src/chfn.c, src/chgpasswd.c, src/chpasswd.c,
src/chsh.c, src/gpasswd.c, src/groupadd.c, src/groupdel.c,
src/groupmod.c, src/grpck.c, src/grpconv.c, src/grpunconv.c,
src/newusers.c, src/passwd.c, src/pwck.c, src/pwconv.c,
src/pwunconv.c, src/useradd.c, src/userdel.c, src/usermod.c:
Harmonize the erro & syslog messages in case of failure of the
*_lock(), *_open(), *_close(), *_unlock(), *_remove() functions.
* src/chgpasswd.c, src/chpasswd.c, src/usermod.c: Avoid
capitalized messages.
* src/chpasswd.c, src/useradd.c, src/usermod.c: Harmonize messages
in case of inexistent entries.
* src/usermod.c: Harmonize messages in case of already existing
entries.
* src/newusers.c, src/useradd.c: Simplify PAM error handling.
* src/useradd.c: Report failures to unlock files (stderr, syslog,
and audit). But do not fail (continue).
* src/useradd.c (open_files): Do not report to syslog & audit
failures to lock or open the databases. This might be harmless,
and the logs were not already informed that a change was
requested.
* src/usermod.c: It's not the account which is unlocked, but its
password.
group or gshadow files were previously locked.
* src/groupadd.c: Make sure failures are reported to syslog/audit
after the change is mentioned.
* src/groupmod.c: Add logging to syslog & audit on lock/unlock
failures.
* src/groupmod.c: Make sure issues are reported to syslog or audit
after the change is mentioned.
* src/groupdel.c: Only call gr_unlock() and sgr_unlock() in the
group or gshadow files were previously locked.
* src/groupdel.c: Simplify the handling of PAM errors.
src/groupmod.c, src/grpck.c, src/login.c, src/logoutd.c,
src/newgrp.c, src/newusers.c, src/passwd.c, src/pwck.c,
src/suauth.c, src/useradd.c, src/userdel.c, src/usermod.c,
src/vipw.c: Complete the switch from the `' quotation style to ''.
Do it also in SYSLOG messages. Quote some parameters. All this
permits to merge some messages.
* src/groupmems.c: Added Prog global variable to indicate the name
of the program in error messages.
2008-07-22 Lukáš Kuklínek <lkukline@redhat.com>
* NEWS, src/groupmems.c: Check if the user added to group actually
exist. RedHat bug #455603
* NEWS, src/groupmems.c: Check if the group exists in the group
local database (/etc/group). RedHat bug #456088
effects. This avoid checking if the user exists in the local passwd
file if not necessary, and thus allow to add LDAP users to local
groups. (The user is already checked against the system
configuration with getpwnam()). Thanks to Dan Kopecek.
update_faillog(). Report errors (but don't fail) if the file
exist, but open(), lseek(), read(), write(), or close() fails.
* src/usermod.c: Add brackets and parenthesis.
* src/usermod.c: Ignore the return value of pam_end() before
exiting.
* src/usermod.c: Ignore the return value of strftime(),
snprintf(), and puts().
* src/usermod.c: Check the return value of gmtime() and asctime(),
and output the raw time_t on failures.
booleans.
* libmisc/env.c: Add brackets and parenthesis.
* libmisc/env.c: Ignore the return value of puts() and fputs().
* libmisc/env.c: Avoid multi-statements lines.
booleans.
* libmisc/utmp.c: Add brackets and parenthesis.
* libmisc/utmp.c: Exit with the EXIT_FAILURE status code in case
of failure.
* libmisc/utmp.c: Avoid assignments in comparisons.
* lib/prototypes.h, libmisc/utmp.c: Change setutmp()'s prototype
to return an int because pututline() and pututxline() may fail.
* libmisc/limits.c: Avoid implicit conversion of pointers /
integers to booleans.
* libmisc/limits.c: Ignore the return value of umask(). We will
never return to the original umask.
* libmisc/limits.c: Avoid multi-statements lines.
* libmisc/limits.c: Added default to a switch(). Report invalid
limit strings to syslog.
* libmisc/limits.c: Ignore the return value of fclose().
/etc/limits is open read-only.
* libmisc/limits.c: Ignore the return value of fputs() and
sleep().
* libmisc/limits.c: Check the return value of nice() and
set_filesize_limit(), and report errors to syslog.
* libmisc/ulimit.c, lib/prototypes.h: Return failures of
set_filesize_limit(). Change the prototype to return an int
instead of void.
libmisc/find_new_uid.c, libmisc/Makefile.am, lib/prototypes.h:
Split find_new_ids.c into find_new_gid.c and find_new_uid.c to
ease the description of login.defs variables in the different
tools.
booleans.
* lib/defines.h: Ignore return values of setlocale() except the
first call.
* lib/defines.h: Fix a splint observer warning by using an
intermediate variable (old_locale).
reading or writing at an unspecified location. Log to syslog in
case of failure when reading a faillog entry or writing in
faillog or btmp.
* libmisc/failure.c: Check if the file exist before opening it.
* libmisc/failure.c: Log failures of open() and close() when
necessary.
* src/lastlog.c: umin and umax do not need to be signed long. Use
an unsigned long which might be needed to parse a GID or UID. Add
the has_umin and has_umax to replace the -1 values.
* src/lastlog.c: Cast dates to time_t.
* src/lastlog.c: Prefix lastlog errors with "lastlog: ".
value of spwd.sp_flag.
* lib/shadow.c: Add brackets.
* lib/shadow.c: Avoid implicit conversion of pointers to
booleans.
* lib/shadow.c: The size argument of fgets is an int, not a
size_t.
* lib/gshadow.c: Remove __setsgNIS() -never used).
* lib/gshadow.c: Avoid multi-statements lines.
* lib/gshadow.c: Avoid assignments in comparisons.
* lib/gshadow.c: ptr[nelem] is a string. Initialize it to NULL
instead of '\0'.
* lib/gshadow.c: Add brackets and parenthesis.
* lib/gshadow.c: The size argument of strncpy is a size_t and the
size argument of fgets is an int.
* lib/getdef.c: Added TODOs because of lack of error checking.
* lib/getdef.c: The size argument of fgets is an int, not a
size_t.
* lib/getdef.c: Avoid multi-statements lines.
* src/login.c: Use a %lu format and cast group and user IDs to
unsigned long integers.
* src/login.c: Ignore return value of setlocale(),
bindtextdomain(), and textdomain().
prototypes.
* src/chage.c: The ID argument of audit_logger is an unsigned
int. Use AUDIT_NO_ID instead of -1.
* src/chage.c: print_date() received a time_t.
* src/chage.c: Use SHADOW_SP_FLAG_UNSET for the initial
value of spwent.sp_flag.
unsigned long integers.
* src/pwck.c: Cast number of days to a long integer.
* src/pwck.c: Use the SCALE macro instead of (24L * 3600L)
for the values to be set in /etc/shadow.
* src/pwck.c: Use SHADOW_SP_FLAG_UNSET for the initial
value of spent.sp_flag.
int. Use AUDIT_NO_ID instead of -1.
* src/groupmod.c: Use a %lu format and cast group and user IDs to
unsigned long integers.
* src/groupmod.c: Cast the parsed GID/UID to a gid_t/uid_t.
int. Use AUDIT_NO_ID instead of -1.
* src/useradd.c: Cast the parsed GID/UID to a gid_t/uid_t.
* src/useradd.c: The size argument of fgets is an int, not a
size_t.
* src/useradd.c: Cast number of days to a long integer.
* src/useradd.c: Use SHADOW_SP_FLAG_UNSET for the initial
value of spent.sp_flag.
* src/useradd.c: Use a %lu format and cast group and user IDs to
unsigned long integers.
* src/newusers.c: Comment why we use both getgrgid() and
gr_locate_gid().
* src/newusers.c: Cast the parsed GID/UID to a gid_t/uid_t.
* src/newusers.c: Cast the number of days to a long integer.
* src/newusers.c: Use SHADOW_SP_FLAG_UNSET for the initial
value of spent.sp_flag.
* src/newusers.c: The size argument of fgets is an int, not a
size_t.
long integer.
* src/usermod.c: Cast UIDs and GIDs to uid_t and gid_t after
checking the ranges.
* src/usermod.c: The ID argument of audit_logger is an unsigned
int.
* src/usermod.c: read() returns a ssize_t.
* src/usermod.c: Cast the return value of malloc and make sure it
receives a size_t.
int.
* src/gpasswd.c: Ignore the return value of signal(). The signal
handlers are only changed for the last steps of gpasswd, and there
is no need to restore them.
unsigned long integers.
* src/newgrp.c: The ID argument of audit_logger is an unsigned
int.
* src/newgrp.c: Ignore the return value of signal() (the signal
handlers are assumed to be the default one and are restored
later).
* src/newgrp.c: Do not checl if a pid_t is < 0, check if equal
to (pid_t)-1.
prototypes.
* libmisc/copydir.c: Use a size_t for the length of strings.
* libmisc/copydir.c: Cast the return value of malloc().
* libmisc/copydir.c: The size argument of write() is a size_t.
might be defined in this header file.
* lib/defines.h: Prefer checking if HAVE_LOCALE_H or ENABLE_NLS
are defined (rather than set to non 0).
* lib/defines.h: The dummy implementations of bindtextdomain() and
textdomain() should return a pointer. Return NULL instead of
/* empty */
* lib/defines.h: Define SHADOW_SP_FLAG_UNSET, to be used for the
initialization of sp_flag field of the shadow structures.
* lib/defines.h: Define LC_ALL, needed even if HAVE_LOCALE_H is
not defined.
* libmisc/log.c: Add brackets and parenthesis.
* libmisc/log.c: read() returns a ssize_t (note size_t).
* libmisc/log.c: Avoid implicit conversion of pointers to
booleans.
* libmisc/log.c: Ignore return value of time() when use with a
non NULL argument.
* libmisc/age.c: Avoid implicit conversion of pointers to
booleans.
* libmisc/age.c: Avoid assignments in comparisons.
* libmisc/age.c: Ignore the return value of printf(), puts(),
fputs() and fflush().
* libmisc/age.c: Add brackets and parenthesis.
* libmisc/age.c: Cast the return value of time() to long and
converted to a date.
Propagate this time to the callers (the get_remote_string() and
do_rlogin() functions).
* libmisc/rlogin.c: Add brackets and parenthesis.
* libmisc/rlogin.c: Avoid multi-statements lines.
* libmisc/rlogin.c: Avoid assignments in comparisons.
* libmisc/rlogin.c: Avoid implicit conversion of pointers to
booleans.
* libmisc/failure.c: read() returns a ssize_t.
* libmisc/failure.c: Add brackets and parenthesis.
* libmisc/failure.c: Ignore return value of time() when use with a
non NULL argument.
libmisc/loginprompt.c, libmisc/ttytype.c, libmisc/tz.c,
src/login_nopam.c, src/chpasswd.c, src/chgpasswd.c, lib/port.c:
The size argument of fgets is an int, not a size_t.
* libmisc/loginprompt.c: Ignore the return value from signal()
when the signal handlers are restored.
* src/chpasswd.c: Cast the return value of time() to a long
integer.
* src/chpasswd.c: Use the SCALE macro instead of (24L * 3600L)
for the values to be set in /etc/shadow.
* src/su.c: Add brackets and parenthesis.
* src/su.c: Avoid implicit conversion of pointers / integers
/ chars to booleans.
* src/su.c: Ignore the return value of pam_end() before
exiting.
* src/su.c: Avoid assignments in comparisons.
* src/su.c: Avoid multi-statements lines.
* lib/nscd.c: Always warn when the nscd cache cannot be flushed.
* lib/nscd.c: Avoid assignments in comparisons.
* lib/nscd.c: Ignore the return value of fputs() when printing
errors.
* lib/port.c: Change isttytime() prototype to return a bool.
* lib/port.c: Ignore the return value of fclose () (file open
read-only).
* lib/port.c: Avoid multi-statements lines.
and printf().
* src/id.c: Ignore return value of setlocale(),
bindtextdomain(), and textdomain().
* src/id.c: Add brackets and parenthesis.
* src/id.c: Avoid implicit conversion of pointers / integers
to booleans.
* src/chsh.c: restricted_shell() renamed is_restricted_shell().
check_shell() renamed shell_is_listed().
* src/chsh.c: Ignore return value of setlocale(),
bindtextdomain(), and textdomain().
* src/chsh.c: Avoid implicit conversion of pointers / integers
to booleans.
* src/chsh.c: Ignore the return value of pam_end() before
exiting.
integers.
* src/grpunconv.c: Add brackets and parenthesis.
* src/grpunconv.c: Ignore return value of setlocale(),
bindtextdomain(), and textdomain().
* src/grpunconv.c: Avoid implicit conversion of pointers / integers
to booleans.
bindtextdomain(), and textdomain().
* src/sulogin.c: Avoid implicit conversion of pointers / integers
/ chars to booleans.
* src/sulogin.c: Avoid assignments in comparisons.
* src/sulogin.c: Ignore the return value of alarm().
* src/groups.c: Add brackets and parenthesis.
* src/groups.c: Avoid implicit conversion of pointers / integers
to booleans.
* src/groups.c: Avoid assignments in comparisons.
* src/groups.c: Ignore the return value of putchar(), printf()
* src/groups.c: Ignore return value of setlocale(),
bindtextdomain(), and textdomain().
* src/grpconv.c: Add brackets and parenthesis.
* src/grpconv.c: Ignore return value of setlocale(),
bindtextdomain(), and textdomain().
* src/grpconv.c: Avoid implicit conversion of pointers / integers
to booleans.
* src/faillog.c: Avoid implicit conversion of pointers / integers
/ chars to booleans.
* src/faillog.c: Ignore return value of setlocale(),
bindtextdomain(), and textdomain().
* src/faillog.c: Add brackets and parenthesis.
* src/login.c: Ignore the return value of pam_end() before
exiting.
* src/login.c: Use a bool when possible instead of int integers.
* src/login.c: Add brackets and parenthesis.
* src/login.c: Ignore the return values of fflush(), putchar(), puts().
* src/login.c: Ignore the return value of fclose() for read-only
files.
* src/login.c: Avoid assignments in comparisons.
* src/login.c: Ignore return value of setlocale(),
bindtextdomain(), and textdomain().
integers.
* src/groupadd.c: Add brackets and parenthesis.
* src/groupadd.c: Avoid implicit conversion of pointers / integers
/ chars to booleans.
* src/groupadd.c: Ignore return value of setlocale(),
bindtextdomain(), and textdomain().
* src/groupadd.c: Ignore the return value of pam_end() before
exiting.
* src/passwd.c: Avoid assignments in comparisons.
* src/passwd.c: Add brackets and parenthesis.
* src/passwd.c: Avoid implicit conversion of pointers / integers /
chars to booleans.
* src/passwd.c: Move the "context_t c" declaration at the
beginning check_selinux_access.
* src/passwd.c: Ignore return value of setlocale(),
bindtextdomain(), and textdomain().
integers.
* src/groupmod.c: Avoid assignments in comparisons.
* src/groupmod.c: Add brackets and parenthesis.
* src/groupmod.c: Avoid implicit conversion of pointers / integers
/ chars to booleans.
* src/groupmod.c: Use a %lu format to print GIDs, and cast the GID
to (unsigned long int).
* src/groupmod.c: Ignore return value of setlocale(),
bindtextdomain(), and textdomain().
* src/groupmod.c: Ignore the return value of pam_end() before
exiting.
* src/useradd.c: Avoid implicit conversion of pointers / integers
/ chars to booleans.
* src/useradd.c: Add brackets and parenthesis.
* src/useradd.c: Avoid assignments in comparisons.
* src/useradd.c: Ignore the return value of fclose() for read-only
files.
* src/useradd.c: Ignore the return value of fflush() before
closing the files.
* src/useradd.c: Avoid multi-statements lines.
* src/useradd.c: Ignore return value of setlocale(),
bindtextdomain(), and textdomain().
* src/useradd.c: Ignore the return value of pam_end() before
exiting.
* src/vipw.c: Ignore the return value of umask(), when the mask is
set again to the old value.
* src/vipw.c: Avoid implicit conversion of pointers / integers to
booleans.
* src/vipw.c: Add brackets and parenthesis.
* src/vipw.c: Avoid assignments in comparisons.
* src/vipw.c: Ignore return value of setlocale(),
bindtextdomain(), and textdomain().
* src/vipw.c: Add missing termination of the longopts parameter
for getopt_long().
integers.
* src/chgpasswd.c: Ignore return value of setlocale(),
bindtextdomain(), and textdomain().
* src/chgpasswd.c: Avoid implicit conversion of integers to
booleans.
integers.
* src/groupdel.c: Avoid implicit conversion of pointers / integers to
booleans.
* src/groupdel.c: Avoid assignments in comparisons.
* src/groupdel.c: Ignore the return value of pam_end() before
exiting.
* src/groupdel.c: Ignore return value of setlocale(),
bindtextdomain(), and textdomain().
bindtextdomain(), and textdomain().
* src/expiry.c: Add brackets and parenthesis.
* src/expiry.c: Avoid assignments in comparisons.
* src/expiry.c: Avoid implicit conversion of pointers to booleans.
* src/usermod.c: Add brackets and parenthesis.
* src/usermod.c: Avoid implicit conversion of pointers / integers
/ chars to booleans.
* src/usermod.c: Avoid assignments in comparisons.
* src/usermod.c: Ignore return value of setlocale(),
bindtextdomain(), and textdomain().
* src/usermod.c: Ignore the return value of pam_end() before
exiting.
long_options before the blocks of code.
* src/groupmems.c: Ignore return value of setlocale(),
bindtextdomain(), and textdomain().
* src/groupmems.c: Ignore the return value of pam_end() before
exiting.
* src/chfn.c: Avoid implicit conversion of integers / chars to
booleans.
* src/chfn.c: Ignore return value of setlocale(),
bindtextdomain(), and textdomain().
* src/logoutd.c: Avoid implicit conversion of pointers / integers
/ chars to booleans.
* src/logoutd.c: Ignore return value of setlocale(),
bindtextdomain(), and textdomain().
* src/logoutd.c: Add brackets and parenthesis.
integers.
* src/chpasswd.c: Avoid implicit conversion of pointers / integers
/ chars to booleans.
* src/chpasswd.c: Ignore return value of setlocale(),
bindtextdomain(), and textdomain().
* src/pwconv.c: Add brackets and parenthesis.
* src/pwconv.c: Ignore return value of setlocale(),
bindtextdomain(), and textdomain().
* src/pwconv.c: Avoid implicit conversion of pointers / integers /
chars to booleans.
* src/newusers.c: Use a bool when possible instead of int
integers.
* src/newusers.c: Avoid implicit conversion of pointers / integers
/ chars to booleans.
* src/newusers.c: Ignore the return value of pam_end() before
exiting.
* src/newusers.c: Ignore return value of setlocale(),
bindtextdomain(), and textdomain().
* src/newusers.c: Avoid multi-statements lines.
* src/newusers.c: Add brackets and parenthesis.
* src/gpasswd.c: Avoid implicit conversion of pointers / integers
/ chars to booleans.
* src/gpasswd.c: Ignore the return value of putchar() and fflush()
before exiting.
* src/gpasswd.c: check_list() renamed is_valid_user_list(), and
return a bool.
* src/gpasswd.c: Ignore return value of setlocale(),
bindtextdomain(), and textdomain().
* src/lastlog.c: Avoid implicit conversion of pointers / integers
/ chars to booleans.
* src/lastlog.c: Add brackets and parenthesis.
* src/lastlog.c: Ignore return value of setlocale(),
bindtextdomain(), and textdomain().
deleted_user_group, was_member, was_admin, and the
options' flags.
* src/userdel.c: Change path_prefix() prototype to return a bool.
* src/userdel.c: Ignore return value of setlocale(),
bindtextdomain(), and textdomain().
* src/userdel.c: Ignore the return value from pam_end() since we
are exiting anyway just afterwards.
* src/userdel.c: Avoid implicit conversion of pointers /
integers / chars to booleans.
* src/userdel.c: Add brackets and parenthesis.
* src/userdel.c: Avoid assignments in comparisons.
* src/userdel.c: Do not ignore the return value of the *_unlock()
functions.
booleans true and false instead. Change the prototypes of
list_match(), user_match(), from_match(), and string_match()
accordingly. Also use booleans internally.
* src/login_nopam.c: Add brackets and parenthesis.
* src/login_nopam.c: Avoid implicit conversion of pointers /
integers / chars to booleans.
* src/login_nopam.c: Avoid assignments in comparisons.
The default parameter should also be a bool.
* libmisc/console.c: Add brackets and parenthesis.
* libmisc/console.c: Avoid assignments in comparisons.
* libmisc/console.c: Change console() prototype to return a bool.
* lib/sgroupio.c: Add brackets.
* lib/sgroupio.c: Avoid implicit conversion of pointers / integers to booleans.
* lib/sgroupio.c: The changed, isopen, locked, and readonly fields of the db are booleans.
* lib/sgroupio.h, lib/sgroupio.c: sgr_file_present returns a bool.
* lib/sgroupio.h: Add protection against multiple inclusions.
* lib/sgroupio.c: Add brackets.
* lib/sgroupio.c: Avoid implicit conversion of pointers / integers to booleans.
* lib/sgroupio.c: The changed, isopen, locked, and readonly fields of the db are booleans.
* lib/sgroupio.h, lib/sgroupio.c: sgr_file_present returns a bool.
* lib/sgroupio.h: Add protection against multiple inclusions.
* lib/port.c: Avoid multiple statement on the same line.
* lib/port.c: Add brackets and parenthesis.
* lib/port.c: Avoid assignments in comparisons.
* lib/port.c: Fix typo comparision -> comparison (in comment).
* lib/commonio.h: Include defines.h to get the definition of bool.
* lib/commonio.h: commonio_present returns a bool
* lib/commonio.c: Implement above changes.
* lib/commonio.c: add argument names in prototypes.
* lib/commonio.c: name_is_nis returns a bool.
* lib/commonio.c: nscd_need_reload is a bool.
* lib/commonio.c: Improve types (use size_t / pid_t when needed instead of int).
* lib/commonio.c: Avoid assignments in comparisons.
* lib/commonio.c: Add brackets and parenthesis.
* lib/commonio.c: Avoid implicit conversion of pointers / integers to booleans
* lib/commonio.c: The return values of utime is not checked on purpose.
* libmisc/ttytype.c: Avoid assignments in comparisons.
* libmisc/ttytype.c: Add brackets and parenthesis.
* libmisc/ttytype.c: The return values of fclose is not checked on purpose.
* libmisc/loginprompt.c: Add brackets.
* libmisc/loginprompt.c: Avoid assignments in comparisons.
* libmisc/loginprompt.c: The return values of fclose and fflush are not checked on purpose.
* Avoid assignments in comparisons.
* (hushed) Change type of found to bool.
* Add brackets.
* Always check if the user or the shell is in
the file. Do not check the first character of the line first. This
is simpler and match better with the HUSHLOGIN_FILE documentation.
check_user_name) renamed to is_valid_user_name (resp.
is_valid_group_name). is_valid_user_name and is_valid_group_name
return a bool.
* src/grpck.c, src/newusers.c, src/usermod.c, src/useradd.c,
src/groupmod.c, src/pwck.c, src/groupadd.c: Use is_valid_user_name
and is_valid_group_name, following above change.
* libmisc/chkname.c: Avoid implicit conversion of chars to
booleans. Add brackets and parenthesis.
* Remove leading and trailing whitespace. This also
* makes it possible to change the field to empty, by
* entering a space. --marekm
*/
while(newf<cp&&isspace(cp[-1])){
cp--;
}
*cp='\0';
cp=newf;
while(isspace(*cp)){
cp++;
}
strcpy(buf,cp);
}
}
Some files were not shown because too many files have changed in this diff
Show More
Reference in New Issue
Block a user
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
