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.
Serge Hallyn [Sun, 31 Jan 2021 23:38:20 +0000 (17:38 -0600)]
subids: support nsswitch
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.
Serge Hallyn [Mon, 1 Feb 2021 04:44:09 +0000 (22:44 -0600)]
try again to fix libmisc sharing problem
Issue #297 reported seeing
*** Warning: Linking the shared library libsubid.la against the
*** static library ../libmisc/libmisc.a is not portable!
which commit b5fb1b38eea2fb0489ed088c82daf6700e72363e 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.
ikerexxe [Tue, 2 Feb 2021 10:27:17 +0000 (11:27 +0100)]
man: usermod clarification
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.
ikerexxe [Tue, 2 Feb 2021 10:21:12 +0000 (11:21 +0100)]
man: shadow clarification
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.
ikerexxe [Tue, 2 Feb 2021 09:24:32 +0000 (10:24 +0100)]
man: chage clarification
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.
ikerexxe [Tue, 27 Oct 2020 10:35:53 +0000 (11:35 +0100)]
useradd: free grp to avoid leak
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| /*
Karel Zak [Thu, 8 Oct 2020 10:17:30 +0000 (12:17 +0200)]
getdefs: add foreign (non-shadow-utils) items
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>
Vito Caputo [Sun, 10 May 2020 00:43:14 +0000 (17:43 -0700)]
su.c: s/doshell/do_interactive_shell/
Mechanical rename distinguishing this variable from intended changes
supporting executing commands without using an interpretive shell
(i.e. no '/bin/sh -c').
Vito Caputo [Sat, 9 May 2020 23:06:27 +0000 (16:06 -0700)]
su.c: replace getopt with ad-hoc flag processing
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:
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.
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":
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
projects / thirdparty / shadow.git / log
