Johannes Segitz [Tue, 26 Sep 2023 13:14:14 +0000 (15:14 +0200)]
useradd: Set proper SELinux labels for def_usrtemplate
Fixes: 74c17c716 ("Add support for skeleton files from /usr/etc/skel") Signed-off-by: Johannes Segitz <jsegitz@suse.com>
Cherry-picked-from: 48aa12af31c0b72872b411857d03a518a4200a3d
Link: <https://github.com/shadow-maint/shadow/pull/812> Reviewed-by: Michael Vetter <jubalh@iodoru.org> Signed-off-by: Alejandro Colomar <alx@kernel.org>
lib, libmisc: Move source files to lib (where their headers were)
Scripted change:
$ find lib/ -type f \
| grep '\.h$' \
| sed 's,lib/,libmisc/,' \
| sed 's,\.h$,.c,' \
| xargs find 2>/dev/null \
| xargs mv -t lib/;
Plus updating the Makefiles.
Closes: <https://github.com/shadow-maint/shadow/issues/791> Closes: <https://bugs.gentoo.org/912446>
Link: <https://github.com/shadow-maint/shadow/issues/763#issuecomment-1664383425>
Link: <https://github.com/shadow-maint/shadow/pull/776>
Link: <https://github.com/shadow-maint/shadow/commit/d0518cc250afeaceb772a7f50a900cfc9b3ab937> Reported-by: Christian Bricart <christian@bricart.de> Reported-by: Robert Marmorstein <robert@marmorstein.org> Cc: Sam James <sam@gentoo.org>
[ jubalh tested the openSUSE package ] Tested-by: Michael Vetter <jubalh@iodoru.org> Acked-by: Michael Vetter <jubalh@iodoru.org>
[ Robert F. tested the Gentoo package ] Tested-by: Robert Förster <Dessa@gmake.de> Cc: David Seifert <soap@gentoo.org> Signed-off-by: Alejandro Colomar <alx@kernel.org>
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.
Create new configuration option `enable-logind` to select which session
support functionality to build, logind or utmp. By default the option is
logind.
shadow userdel: add the adaptation to the busybox ps in 01-kill_user_procs.sh
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'
Michael Vetter [Wed, 26 Jul 2023 08:13:53 +0000 (10:13 +0200)]
chsh: warn if root sets a shell not listed in /etc/shells
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
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:
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'?
Jeffrey Bencteux [Wed, 21 Jun 2023 13:12:43 +0000 (15:12 +0200)]
chgpasswd: fix segfault in command-line options
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:
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: 45c6603cc86c ("[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.
- Rewrite while loop calling strtok(3) as a for loop. This allows
doing more simplification inside the loop (see next commit).
- 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.
- Merge nested conditionals into a single if, to reduce indentation.
- Indent (1 SP) nested preprocessor conditionals.
- Reduce use of whitespace and newlines while unindenting.
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.
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: 2a61122b5e8f ("Unoptimize the higher part of the domain of csrand_uniform()") Signed-off-by: Alejandro Colomar <alx@kernel.org>
getline(3) might have succeeded in a previous iteration, in which case
p points to an offset that is not valid. Make p NULL at the end of the
loop, to make sure it doesn't hold old stuff.
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:
ch(g)passwd: Check selinux permissions upon startup
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>
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').