Commit Graph

2791 Commits

Author SHA1 Message Date
52c081b02c new{g,u}idmap: align setuid and fscaps behavior
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>
2018-10-28 01:27:48 +02:00
bb3f810611 Merge pull request #136 from giuseppe/fcap-newuidmap-newgidmap
newuidmap/newgidmap: install with file capabilities
2018-10-27 11:26:31 -05:00
d5255da20b Merge pull request #132 from giuseppe/no-cap-sys-admin
newuidmap/newgidmap: do not require CAP_SYS_ADMIN in the parent user namespace
2018-10-27 11:22:37 -05:00
70971457b7 newuidmap/newgidmap: install with file capabilities
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>
2018-10-24 23:10:59 +02:00
ff8b1ebafa Merge pull request #118 from AdelieLinux/utmpx-only-support
[WIP] Support systems that only have utmpx
2018-10-23 22:35:19 -05:00
83f1380600 Merge pull request #133 from t8m/trivial
Fix some issues found in Coverity scan.
2018-10-23 22:21:12 -05:00
1ecca8439d new[ug]idmap: not require CAP_SYS_ADMIN in the parent userNS
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>
2018-10-22 16:57:50 +02:00
58ee899dce Merge pull request #128 from jhrozek/sssd
Flush sssd caches in addition to nscd caches
2018-10-18 12:26:38 -07:00
10e388efc2 useradd: fix segfault trying to overwrite const data with mkstemp
Also fix memory leaks in error paths.
2018-10-12 10:14:02 +02:00
fb97da1ce1 Fix some issues found in Coverity scan. 2018-10-10 12:22:04 +02:00
4aaf05d72e Flush sssd caches in addition to nscd caches
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.
2018-09-13 14:20:02 +02:00
6bf2d74dfc Merge pull request #122 from ivladdalvi/nologin-uid
Log UID in nologin
2018-08-13 18:37:02 -05:00
4be18d3299 Log UID in nologin
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.
2018-08-13 16:46:04 +08:00
0d725c1e60 Merge pull request #116 from LionNatsu/master
po/zh_CN: update
2018-08-11 00:40:02 -05:00
3c69af9ed3 Merge pull request #119 from mvo5/su-l
su.c: run pam_getenvlist() after setup_env
2018-08-11 00:39:07 -05:00
89b96cb85c su.c: run pam_getenvlist() after setup_env
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
2018-06-25 16:00:21 +02:00
99dbd4b9ee Support systems that only have utmpx
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/
2018-06-24 00:13:12 -05:00
67ec1a5266 Merge pull request #117 from rindeal/ENABLE_SUBIDS
fix unguarded ENABLE_SUBIDS code
2018-06-19 08:17:57 -04:00
2fd5815546 fix unguarded ENABLE_SUBIDS code 2018-06-18 15:51:27 +02:00
cba31b52b3 po/zh_CN: update 2018-06-16 18:26:28 +08:00
eadcb472e1 Merge pull request #112 from jubalh/useradd-mkdirs
Create parent dirs for useradd -m
2018-05-23 09:57:40 -05:00
b3b6d9d77c Create parent dirs for useradd -m
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.
2018-05-15 17:30:34 +02:00
48dcf7852e usermod: prevent a segv
in the case where prefix does not exist.

Signed-off-by: Serge Hallyn <shallyn@cisco.com>
2018-05-08 21:37:55 -05:00
73a876a056 Fix usermod crash
Return newly allocated pointers when the caller will free them.

Closes #110
2018-05-08 21:17:46 -05:00
f50603a5fc release 4.6
Signed-off-by: Serge Hallyn <shallyn@cisco.com>
2018-04-29 11:41:41 -05:00
164dcfe65b Merge pull request #103 from HarmtH/be-predictable
su.c: be more predictable
2018-03-29 23:10:51 -07:00
fb356b1344 Merge pull request #21 from fariouche/master
Add --prefix argument
2018-03-29 22:36:28 -07:00
a3d91ae318 Merge pull request #102 from HarmtH/fix-dashdash-slurp
su.c: fix '--' slurping
2018-03-29 15:45:54 -07:00
65b4f58703 add --prefix option: some fixes + fixed pwd.lock file location 2018-03-28 21:14:12 +02:00
54551c7d6e Merge remote-tracking branch 'upstream/master' 2018-03-28 21:11:36 +02:00
d877e3fcac su.c: be more predictable
Always parse first non-option as username.
2018-03-27 00:57:21 +02:00
dbfe7dd42e su.c: fix '--' slurping
All arguments are already reordered and parsed by getopt_long since e663c69, so manual '--' slurping is wrong.

Closes #101
2018-03-26 22:37:56 +02:00
45b4187596 pwconv and grpconv: rewind after deleting an entry
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>
2018-03-25 09:18:22 -05:00
44c63795a7 userdel: fix wrong variable name in tcb case
Found in mandriva distro patch, and with a test build.

Signed-off-by: Serge Hallyn <shallyn@cisco.com>
2018-03-24 23:44:09 -05:00
36244ac1ff src/Makefile.am: tcb fixes from mandriva
1. suidubins -= was breaking build with WITH_TCB.
2. stick libtcb at end of ldlibs list.

Signed-off-by: Serge Hallyn <shallyn@cisco.com>
2018-03-24 23:41:23 -05:00
d3790feac0 pwck.c: do not pass O_CREAT
It causes a crash later when we try to close files.

Closes #96

Signed-off-by: Serge Hallyn <shallyn@cisco.com>
2018-03-24 20:29:48 -05:00
b63aca9a2c src/Makefile.am: drop duplicate inclusion of chage
Closes #80

Signed-off-by: Serge Hallyn <shallyn@cisco.com>
2018-03-24 16:27:20 -05:00
8f2f2a0d9d Merge pull request #98 from jsoref/spelling
Spelling
2018-03-24 15:54:51 -05:00
5d57ca10d9 Merge pull request #100 from akrosikam/patch-1
Complete translation to Norwegian bokmål
2018-03-24 15:54:22 -05:00
a5913d98e1 Complete translation to Norwegian bokmål
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.
2018-03-12 08:39:16 +01:00
5f3e3c2c62 Merge pull request #93 from rahul1809/master
Double freeing up pointers , Causing Segmentation fault
2018-02-19 14:45:13 -06:00
c53e4c1d77 Merge pull request #97 from cyphar/newgidmap-secure-setgroups
newgidmap: enforce setgroups=deny if self-mapping a group
2018-02-16 08:40:39 -06:00
6d8be68071 README: add Aleksa Sarai to author list
Signed-off-by: Aleksa Sarai <asarai@suse.de>
2018-02-16 17:56:36 +11:00
fb28c99b8a newgidmap: enforce setgroups=deny if self-mapping a group
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>
2018-02-16 17:56:35 +11:00
acaed3deab upstream merge 2018-01-23 23:10:19 +01:00
bb47fdf25e indentation fix 2018-01-22 17:07:27 +05:30
97bb5b2b6d added a check to avoid freeing null pointer 2018-01-22 17:05:52 +05:30
c0f0c67864 Merge pull request #92 from IronicBadger/master
Fixes mispelling of MAX_DAYS help text
2018-01-18 22:42:12 -06:00
e91b0f0517 Fixes mispelling of MAX_DAYS help text 2018-01-17 12:21:48 +00:00
3f1f999e2d Merge pull request #90 from t8m/userdel-chroot
Make userdel to work with -R.
2018-01-08 22:57:43 -06:00