Bug 885288 - seunshare, etc should set no_new_privs
Summary: seunshare, etc should set no_new_privs
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: policycoreutils
Version: rawhide
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Miroslav Grepl
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On: 1035427
Blocks: 1095222
TreeView+ depends on / blocked
 
Reported: 2012-12-08 07:42 UTC by Andy Lutomirski
Modified: 2014-05-14 19:43 UTC (History)
10 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
: 1095222 (view as bug list)
Environment:
Last Closed: 2014-05-14 19:43:17 UTC
Type: Bug


Attachments (Terms of Use)
Example of why this is bad (472 bytes, text/plain)
2014-04-23 22:01 UTC, Andy Lutomirski
no flags Details

Comment 2 Steve Grubb 2012-12-10 19:02:08 UTC
Not sure how successful anything can be. For example, let's make captest suid root:

# ls -l /bin/captest
-rwsr-xr-x. 1 root root 13624 Nov 30 10:07 /bin/captest

Now, running it via seunshare as unprivileged user:

$ seunshare -h /home/sgrubb/test /usr/bin/captest 
User  credentials uid:4325 euid:0 suid:0
Group credentials gid:4325 egid:4325 sgid:4325
Note: app has mismatching credentials!!
Current capabilities: none
securebits flags: NOROOT, NOROOT_LOCKED, NO_SETUID_FIXUP, NO_SETUID_FIXUP_LOCKED
Attempting direct access to shadow...FAILED (Permission denied)
Attempting to access shadow by child process...FAILED
Attempting to regain root...FAILED
Child User  credentials uid:4325 euid:0 suid:0
Child Group credentials gid:4325 egid:4325 sgid:4325
Note: app has mismatching credentials!!
Child capabilities: none
Child securebits flags: NOROOT, NOROOT_LOCKED, NO_SETUID_FIXUP, NO_SETUID_FIXUP_LOCKED

I also made a change to captest to try to read /etc/passwd- which is 0600 and it fails also. Still thinking about it...but I thought I'd go ahead and report these results.

Comment 4 Fedora End Of Life 2013-07-03 22:17:29 UTC
This message is a reminder that Fedora 17 is nearing its end of life.
Approximately 4 (four) weeks from now Fedora will stop maintaining
and issuing updates for Fedora 17. It is Fedora's policy to close all
bug reports from releases that are no longer maintained. At that time
this bug will be closed as WONTFIX if it remains open with a Fedora 
'version' of '17'.

Package Maintainer: If you wish for this bug to remain open because you
plan to fix it in a currently maintained version, simply change the 'version' 
to a later Fedora version prior to Fedora 17's end of life.

Bug Reporter:  Thank you for reporting this issue and we are sorry that 
we may not be able to fix it before Fedora 17 is end of life. If you 
would still like  to see this bug fixed and are able to reproduce it 
against a later version  of Fedora, you are encouraged  change the 
'version' to a later Fedora version prior to Fedora 17's end of life.

Although we aim to fix as many bugs as possible during every release's 
lifetime, sometimes those efforts are overtaken by events. Often a 
more recent Fedora release includes newer upstream software that fixes 
bugs or makes them obsolete.

Comment 5 Andy Lutomirski 2013-07-03 22:30:48 UTC
This bug has nothing to do with any particular Fedora version, so I'm bumping it.

Comment 6 Daniel Walsh 2013-08-28 17:18:35 UTC
Andy you never responded to Steve's Comments?

Comment 7 Andy Lutomirski 2013-08-28 17:26:21 UTC
Steve's comments?  The only comments I see are the original post, the end of life message, my version bump, your version bump, and "Andy you never responded to Steve's Comments?".

Comment 8 Steve Grubb 2013-08-28 18:07:07 UTC
Comment #2.

Comment 9 Andy Lutomirski 2013-08-28 18:20:48 UTC
I suspect that selinux is preventing that access.  You may be able to do other dangerous things by, say, creating a setuid file.

To be clear, I don't know of an actual exploit here.  But fixing this issue is a single line of code.

Comment 10 Daniel Walsh 2013-08-28 20:12:49 UTC
Steve is saying that this is blocked by capabilities, I believe not just SELinux.

Comment 11 Steve Grubb 2013-08-28 20:42:55 UTC
To be able to do anything with the suid, you would need CAP_SETUID. Its blocked. So, what we have left is euid and that is only used to determine access rights on shared resources such as message queues, shared memory, and semaphores. Those are rarely used.

Comment 12 Andy Lutomirski 2013-08-28 21:04:41 UTC
Huh?

If you're running with suid == 0, you cat setresuid(0, 0, 0) without caps.  You also picked a bad example with /etc/shadow -- its mode is 0000.  But anything that's owned by root and has mode 0600 (or 0400 for read) is fair game.

# [create setuid_bash, a setuid-root copy of bash]

$ mkdir tmp
$ seunshare -t tmp /bin/bash

[now running inside seunshare]
$ setpriv -d
uid: 1000
euid: 1000
gid: 1000
egid: 1000
Supplementary groups: 4,10,18,989,1000
no_new_privs: 0
Inheritable capabilities: [none]
Capability bounding set: chown,dac_override,dac_read_search,fowner,fsetid,kill,setgid,setuid,setpcap,linux_immutable,net_bind_service,net_broadcast,net_admin,net_raw,ipc_lock,ipc_owner,sys_module,sys_rawio,sys_chroot,sys_ptrace,sys_psacct,sys_admin,sys_boot,sys_nice,sys_resource,sys_time,sys_tty_config,mknod,lease,audit_write,audit_control,setfcap,mac_override,mac_admin,syslog,wake_alarm,block_suspend,compromise_kernel
Securebits: noroot,noroot_locked,no_setuid_fixup,no_setuid_fixup_locked
SELinux label: unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
$ ./setuid_bash -p
setuid_bash: no job control in this shell
# cat /etc/shadow
cat: /etc/shadow: Permission denied
# setpriv -d
uid: 1000
euid: 0
gid: 1000
egid: 1000
Supplementary groups: 4,10,18,989,1000
no_new_privs: 0
Inheritable capabilities: [none]
Capability bounding set: chown,dac_override,dac_read_search,fowner,fsetid,kill,setgid,setuid,setpcap,linux_immutable,net_bind_service,net_broadcast,net_admin,net_raw,ipc_lock,ipc_owner,sys_module,sys_rawio,sys_chroot,sys_ptrace,sys_psacct,sys_admin,sys_boot,sys_nice,sys_resource,sys_time,sys_tty_config,mknod,lease,audit_write,audit_control,setfcap,mac_override,mac_admin,syslog,wake_alarm,block_suspend,compromise_kernel
Securebits: noroot,noroot_locked,no_setuid_fixup,no_setuid_fixup_locked
SELinux label: unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
# cat /etc/ssh/ssh_host_rsa_key >/dev/null

Comment 13 Daniel Walsh 2013-09-04 19:28:05 UTC
Why do I need to block the executing of a setuid app from within my container environment by default?  Why can't I allow someone to setup a different user and /tmp but still be allowed to execute su or sudo?  Or any setuid app?

If I want to block that access I will use SELinux not make seunshare use of containers block it?

Comment 14 Andy Lutomirski 2013-09-04 19:38:34 UTC
seunshare is setuid root, so this kind of blocking needs to be unconditional, not just the default.  seunshare already uses securebits to mostly block setuid programs, but this isn't fully effective.

The risk is that an unprivileged user would exploit a combination of seunshare and some other setuid root binary to escalate their privilege.  Most setuid binaries are not written with the expectations of running with SECURE_NOROOT set or with various directories swapped out.

Comment 15 Florian Weimer 2013-09-05 09:55:35 UTC
I looked at this and here's what I found: The -t parameter does not actually specify the temporary directory directly.  Instead, it's a directory whose contents is used to populate a freshly created directory.  This new directory is owned by root and has the sticky bit set, and it is a subdirectory of the original /tmp, so all paths in it are guaranteed to be as stable as the original /tmp.  (I agree that without these permissions, we'd introduce security vulnerabilities in some SUID/SGID binaries.)

For users with have HOME=/, the bind mount for the home directory seems to fail silently and the old / is preserved inside the new namespace (on Fedora 19).  seunshare bails out after detecting that the new home directory is not the expected one, so no harm done.  I think an ownership check for the old home directory would make sense here, as an additional hardening measure.

Comment 16 Andy Lutomirski 2013-11-27 01:28:03 UTC
This is getting ridiculous.  There are at least two vulnerabilities in seunshare:

1. Any user can use seunshare to create a process with securebits set.  This is dangerous.

2. Any user can use seunshare to create a process that has a /tmp that's *OWNED BY THE UNTRUSTED USER*.  This is not even remotely safe.

Both of these problems are effectively mitigated by no_new_privs, which was designed for exactly this kind of shenanigans.

I'll go get a CVE number assigned.  Maybe that will encourage the two-line fix to be written.  It really is as simple as:

if (prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0) != 0)
  err(1, "PR_SET_NO_NEW_PRIVS");  /* or your favorite way to abort */

Comment 17 Andy Lutomirski 2013-11-27 01:34:15 UTC
Bah, I can't read.  The /tmp thing is odd but okay.

Comment 18 Daniel Walsh 2013-11-27 14:02:30 UTC
Andy do you agree this is not a problem, or do you still think the PR_SET_NO_NEW_PRIVS needs to be set?

Comment 19 Andy Lutomirski 2013-11-27 16:36:31 UTC
I haven't found any setuid binary on my Fedora box that's exploitable using seunshare.  That being said, there are probably people who install custom setuid binaries under the entirely reasonable assumption that no one can try to attack them by setting securebits.  (There's a reason that setting securebits requires CAP_SETPCAP.)

I assume that the intent of setting SECURE_NOROOT | SECURE_NO_SETUID_FIXUP being set in seunshare is to block setuid programs, it seems very worthwhile to set no_new_privs as well, at least on kernels that support that feature.

I just read the code, though, and maybe a better fix is to modify capng_lock to set no_new_privs.  I also requested a trac account so I can file a bug upstream, which might be a better place to discuss this.

Comment 20 Andy Lutomirski 2013-11-27 16:43:25 UTC
(I emailed Steve Grubb about this.)

Comment 21 Andy Lutomirski 2014-04-23 22:01:57 UTC
Created attachment 889104 [details]
Example of why this is bad

$ sudo chown root: sesploit; sudo chmod 4755 sesploit; seunshare -t . `realpath ./sesploit`
Dropped privs; real uid is 1000 and effective uid is 1000
It's baaaack!

Comment 22 Andy Lutomirski 2014-04-23 22:25:46 UTC
I think I've let this sit here for long enough.  Let's see if anyone comes up with an exploit:

http://seclists.org/fulldisclosure/2014/Apr/262

Comment 23 Steve Grubb 2014-04-24 00:54:44 UTC
For an exploit, why don't you edit /etc/passwd and change your acct number to 0?

Also, the setuid exploitable program should call setresuid() to clear the saved uid so that you don't have this problem.

Comment 24 Andy Lutomirski 2014-04-24 01:00:36 UTC
(In reply to Steve Grubb from comment #23)
> For an exploit, why don't you edit /etc/passwd and change your acct number
> to 0?

That's certainly one way to do it, assuming you can find a setuid program that can be exploited after it thinks it's dropped privileges.

> 
> Also, the setuid exploitable program should call setresuid() to clear the
> saved uid so that you don't have this problem.

If you want to change every single setuid program to use setresuid instead of setreuid, be my guest.  I suspect you'll find that basically nothing except the Chromium suid sandbox uses setresuid to drop privilege, though, since it isn't supposed to be necessary.

Comment 25 Steve Grubb 2014-04-24 02:05:55 UTC
Every setuid program that does not expect itself, a child process, or thread to need to regain the original uid should call setresuid. Perhaps a review is needed. But sandboxed or not with seunshare, the problem exists should an exploit be possible on setuid programs that mistakenly call setuid thinking it erases everything. The saved uid needs to be replaced if not needed.

Comment 26 Andy Lutomirski 2014-04-24 02:09:32 UTC
Alternatively, you could apply my patch and prevent this from ever being a problem.

Comment 27 Andy Lutomirski 2014-04-24 02:12:23 UTC
Let me clarify this.  In this context, seunshare *is not a sandbox*.  It is a tool that can be used by malicious people to do probe at new attack surfaces *that do not exist without seunshare*.

There's a very good reason that setting securebits requires privilege.  Merely having seunshare installed allows anyone to set those bits.

Comment 31 Steve Grubb 2014-04-30 13:51:59 UTC
libcap-ng-0.7.4-1 was pushed live in F20. Yum update should fix the system. However, I am still investigating if this is the ultimate and correct fix. During the investigation, it doesn't hurt anything to be there.

Comment 32 Alexandru Constantin Minoiu 2014-05-01 00:33:18 UTC
Hi,
There is an issue with seunshare after the latest update to libcap-ng-0.7.4-1 .
This breaks the sandbox program. Please see my comment : https://bugzilla.redhat.com/show_bug.cgi?id=1091761#c6

Thanks.

Comment 33 Pavel Kankovsky 2014-05-01 13:22:07 UTC
(In reply to Steve Grubb from comment #23)
> For an exploit, why don't you edit /etc/passwd and change your acct
> number to 0?

That--or drop a "cuckoo's egg" into one of the many directories
whose content is executed by system processes (and say hi to Clifford
Stoll).

(In reply to Steve Grubb from comment #25)
> Every setuid program that does not expect itself, a child process, or
> thread to need to regain the original uid should call setresuid.

For several decades of the history of unix-like operating systems,
"the appropriate privileges" of POSIX meant "euid 0". The belief that
setuid() is always irreversible when invoked by a setuid-root program
might be mistaken but this is no excuse for making things worse for
every legacy setuid-program program out there and even less excuse
to break things in a very subtle way.

I do not get why seunshare sets SECURE_NOROOT. It does not seem to
block capabilities granted via fcaps and the whole notion of
"unprivileged root" as implemented by SECURE_NOROOT is absurd.
It completely disregards the fact root owns and is allowed (by DAC;
SELinux et al. might disagree) to modify the majority of system
objects and subvert the system to regain most if not all missing
capabilities (see above). SECURE_NOROOT is Pandora's box and
seunshare let users open it.

Comment 35 Fedora Update System 2014-05-07 09:44:17 UTC
selinux-policy-3.12.1-161.fc20 has been submitted as an update for Fedora 20.
https://admin.fedoraproject.org/updates/selinux-policy-3.12.1-161.fc20

Comment 36 Lukas Vrabec 2014-05-07 10:10:54 UTC
Sorry, I put bad bugzilla ID to my update.


Note You need to log in before you can comment on or make changes to this bug.