Bug 121073 - [kernel] [patch] run chroot as an unprivileged user
Summary: [kernel] [patch] run chroot as an unprivileged user
Status: Open
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: 8.0-CURRENT
Hardware: Any Any
: Normal Affects Only Me
Assignee: freebsd-bugs mailing list
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-02-25 09:50 UTC by Jille
Modified: 2018-05-21 03:51 UTC (History)
8 users (show)

See Also:


Attachments
chroot.patch (3.80 KB, patch)
2014-05-30 22:54 UTC, Nathan Whitehorn
no flags Details | Diff
Prevents escape from unprivileged chroot (6.35 KB, patch)
2014-06-08 22:47 UTC, Nathan Whitehorn
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jille 2008-02-25 09:50:03 UTC
	We (Ed and I) thought it should be possible to chroot as non-root,
	This should (hopefully) increase the security, because no setuid-root and privilege dropping after the chroot(2) call is longer needed.
	This is tested on 8.0-CURRENT, should work on 7.0-RC2 and I will backport it later to 6.3-RELEASE.

Fix: 

--- lib/libc/sys/chroot.2
+++ lib/libc/sys/chroot.2
@@ -61,7 +61,13 @@
 .Fn chroot
 has no effect on the process's current directory.
 .Pp
-This call is restricted to the super-user.
+By default, this call is restricted to the super-user. If
+.Ql kern.chroot_allow_unprivileged
+is set to a non-zero value, all users are capable of performing the
+.Fn chroot
+call. When called by an unprivileged user, the process and its children
+won't honor the setuid and setgid bits when performing an
+.Xr execve 2 .
 .Pp
 Depending on the setting of the
 .Ql kern.chroot_allow_open_directories
@@ -140,3 +146,8 @@
 open directories, or a MAC check), it is possible that this system
 call may return an error, with the working directory of the process
 left changed.
+.Pp
+When a call to
+.Fn chroot
+fails when invoked by an unprivileged user, the process is not properly
+capable of executing setuid or setgid applications anymore.
--- sys/kern/kern_exec.c
+++ sys/kern/kern_exec.c
@@ -560,7 +560,7 @@
 
 	if (credential_changing &&
 	    (imgp->vp->v_mount->mnt_flag & MNT_NOSUID) == 0 &&
-	    (p->p_flag & P_TRACED) == 0) {
+	    (p->p_flag & (P_NOSUGID|P_TRACED)) == 0) {
 		/*
 		 * Turn off syscall tracing for set-id programs, except for
 		 * root.  Record any set-id flags first to make sure that
--- sys/kern/kern_fork.c
+++ sys/kern/kern_fork.c
@@ -584,7 +584,7 @@
 	 * Preserve some more flags in subprocess.  P_PROFIL has already
 	 * been preserved.
 	 */
-	p2->p_flag |= p1->p_flag & P_SUGID;
+	p2->p_flag |= p1->p_flag & P_INHERITED;
 	td2->td_pflags |= td->td_pflags & TDP_ALTSTACK;
 	SESS_LOCK(p1->p_session);
 	if (p1->p_session->s_ttyvp != NULL && p1->p_flag & P_CONTROLT)
--- sys/kern/vfs_syscalls.c
+++ sys/kern/vfs_syscalls.c
@@ -860,9 +860,12 @@
  */
 
 static int chroot_allow_open_directories = 1;
+static int chroot_allow_unprivileged = 0;
 
 SYSCTL_INT(_kern, OID_AUTO, chroot_allow_open_directories, CTLFLAG_RW,
      &chroot_allow_open_directories, 0, "");
+SYSCTL_INT(_kern, OID_AUTO, chroot_allow_unprivileged, CTLFLAG_RW,
+     &chroot_allow_unprivileged, 0, "");
 
 /*
  * Change notion of root (``/'') directory.
@@ -880,12 +883,27 @@
 	} */ *uap;
 {
 	int error;
+	struct proc *p;
 	struct nameidata nd;
 	int vfslocked;
 
 	error = priv_check(td, PRIV_VFS_CHROOT);
-	if (error)
-		return (error);
+	if (error) {
+		if (!chroot_allow_unprivileged)
+			return (error);
+
+		/*
+		 * Disallow this process and its children to use setuid
+		 * bits. Users could hardlink setuid applications into a
+		 * chroot which contains a fake C library to obtain
+		 * super-user privileges.
+		 */
+		p = td->td_proc;
+		PROC_LOCK(p);
+		p->p_flag |= P_NOSUGID;
+		PROC_UNLOCK(p);
+	}
+
 	NDINIT(&nd, LOOKUP, FOLLOW | LOCKLEAF | MPSAFE | AUDITVNODE1,
 	    UIO_USERSPACE, uap->path, td);
 	error = namei(&nd);
--- sys/sys/proc.h
+++ sys/sys/proc.h
@@ -643,7 +643,9 @@
 #define	P_INMEM		0x10000000 /* Loaded into memory. */
 #define	P_SWAPPINGOUT	0x20000000 /* Process is being swapped out. */
 #define	P_SWAPPINGIN	0x40000000 /* Process is being swapped in. */
+#define	P_NOSUGID	0x80000000 /* Ignore set[ug]id on exec. */
 
+#define	P_INHERITED	(P_SUGID|P_NOSUGID)
 #define	P_STOPPED	(P_STOPPED_SIG|P_STOPPED_SINGLE|P_STOPPED_TRACE)
 #define	P_SHOULDSTOP(p)	((p)->p_flag & P_STOPPED)
How-To-Repeat: 	sysctl kern.chroot_allow_unprivileged=1
	su nobody
	chroot / sh
	ping 127.0.0.1 # Should fail
Comment 1 Ed Schouten 2008-02-25 15:21:46 UTC
Hello,

Just wanted to add some info about what this patch does:

As far as I know, the only unsafe thing about chroot(2) is the fact that
you can trick set[ug]id applications to do unwanted things when
hardlinked into a new root directory, for example:

- The user could store a different C library inside the chroot to
  perform an execl("/bin/sh", ...).
- The user could just store his own passwd files, including database
  files, to make applications like su(8) work, without the proper
  privileges.

This patch adds a new flag called P_NOSUGID. When enabled, this process
will not honor the setuid and setgid flags anymore, just like MNT_NOSUID
and P_TRACED.

I have great confidence that this patch does not add any security holes,
but just to be sure, this patch adds a sysctl to disable this behaviour
by default.

-- 
 Ed Schouten <ed@fxq.nl>
 WWW: http://g-rave.nl/
Comment 2 Colin Percival freebsd_committer 2008-03-01 06:36:24 UTC
This is an interesting approach, and at first glance I can't see anything wrong
with it.  That said, it needs very careful consideration; so to avoid confusion
and make sure that this doesn't get committed before the right people have a
chance to consider it sufficiently:

         +----------------------------------------------------------+
         |UNDER NO CONDITIONS SHOULD THIS PATCH BE COMMITTED WITHOUT|
         |EXPLICIT APPROVAL FROM THE FREEBSD SECURITY OFFICER.      |
         +----------------------------------------------------------+

Colin Percival
Comment 3 Jille 2008-03-07 13:32:49 UTC
Hello,

This is the way I think about this; correct me when/where I'm wrong:

- When an user chroot()'s himself, there is no possibility to change 
user, because setuid is disabled, and this is the only way to switch user.
- This means no local exploits will be possible.
- Something that is possible, is programs use eg /etc/passwd, and think 
it is reliable. It is the chroot()'ing user's job to make sure this is 
reliable (no empty passwords, weak passwords, etc)
Example 'remote exploit':
userx chroot()'s an ftpd, because root doesn't want to run one, and he 
wants to ftp.
the ftpd checks the /etc/passwd in the chroot, and checks whether 
passwords match that file.
So if userx added users, or 'changed' passwords in the /etc/passwd the 
ftpd will think that are his own passwords.

This way a remote attacker can get unwanted access to the system.
I think this is userx' responsibility, he should know what chroot()'ing 
for (side)effects has.

- What if some setuid program does a chroot() ? (euid!=ruid)
1) userx runs a setuid-usery: usery might have coded an chroot() that 
chroots, and userx doesn't know that
  - this is userx' fault, he shouldn't run binaries owned by others
2) userx runs a setuid-usery binary: There is no way userx can 'insert' 
a chroot() (I assume), or modify the memory etc; otherwise even su is an 
enourmous security risk.

Am I making mistakes ? Missing things ?

-- Jille Timmermans
Comment 4 Nathan Whitehorn freebsd_committer 2014-05-30 22:54:07 UTC
This patch would be extremely useful for package building, as I think it 
would allow all operations to be done without root. It has gone stale 
since the bug was filed, so I've updated it and attached a new version.
-Nathan
Comment 5 Jilles Tjoelker freebsd_committer 2014-05-31 00:02:46 UTC
In FreeBSD PR kern/121073, you wrote:
> We (Ed and I) thought it should be possible to chroot as non-root,
> This should (hopefully) increase the security, because no setuid-root
> and privilege dropping after the chroot(2) call is longer needed.

This change may be useful for package building without root.

The disable setuid/setgid part looks similar to Linux's
prctl(PR_SET_NO_NEW_PRIVS). In Linux, this is a separate operation that
is a precondition for certain operations if unprivileged.

I found two possible security risks with this:

Firstly, unprivileged chroot might be used to break out of a chroot. For
example, a directory file descriptor may be put onto a unix socket (to
defeat kern.chroot_allow_open_directories), chroot to a subdirectory,
get the file descriptor back, fchdir and abuse "..". If this is the
first chroot, fd_jdir will stop it but other chroots (but not nested
jails) can be escaped from. This can be fixed by only allowing a first
or second chroot (fdp->fd_jdir == NULL || fdp->fd_jdir == fdp->fd_rdir).
Due to locking this check must be in change_root().

Secondly, a mac_vnode_execve_will_transition could lower as well as
increase privilege; it may be better to reject the exec entirely if a
MAC transition is denied.

-- 
Jilles Tjoelker
Comment 6 Nathan Whitehorn freebsd_committer 2014-06-08 22:47:23 UTC
Created attachment 143547 [details]
Prevents escape from unprivileged chroot

This fixes the issue of using this feature to escape from a chroot established with privileges after dropping them by the simple expedient of unconditionally preventing unprivileged chroot while already in a chroot.

The second issue raised (MAC transitions) I know nothing about and cannot address.
Comment 7 Jille 2014-06-08 23:36:45 UTC
I remember someone saying this could be exploited using rfork. I don't know why it's not listed in this bug.

IIRC the problem was that fd_rdir (root of the processes) was stored in proc->p_fd (struct filedesc) and the P_NOSUGID-flag in struct proc itself. One could use rfork to create a new process with the same descriptor table and call chroot in the child which would flag the child with P_NOSUGID but change to root for the parent as well. The parent doesn't get P_NOSUGID however and will be able to execve a setuid executable with a fake libc.
Comment 8 Robert Watson freebsd_committer 2014-06-16 12:19:41 UTC
A appreciate the desirability for the features implied by this change, but given the propensity for vulnerabilities relating to chroot() in the past, think we should take a very conservative approach to potentially adopting it.  There's a particular concern with how it interacts with non-UNIX-ID-based models -- e.g., MAC, Capsicum, Audit, Jail, as well as a future fine-grained privilege model.

Overall, I'd rate this proposed change as "extremely high risk; we will fix multiple vulnerabilities in it in the future," and so that cost would need to be carefully weighed against presumed benefit -- a fine-grained privilege model in which PRIV_CHROOT is delegable to only specific users or roles would help mitigate that risk.

I wonder if a more suitable name for the proposed P_NOSUGID would be P_NOCREDCHANGE, and I also wonder if it should be CR_NOCREDCHANGE.
Comment 9 Nathan Whitehorn freebsd_committer 2014-06-16 16:13:24 UTC
There are, I think, two potential security issues here:
1. Many pieces of software assume that if you chroot and drop privileges, no further chroot is possible.
2. There could be sneaky ways of obtaining privileges once no-new-privileges is set.

(1) is pretty straightforward since we can just disallow unprivileged chroot after any other chroot. (2) is the complex one. Are there others?

Some no-cred-change property for processes seems extremely useful from a security perspective and, if we have one we could trust, this patch becomes trivial. Would it make sense just to work on that first and come back to unprivileged chroot later?
Comment 10 Robert Watson freebsd_committer 2014-06-17 15:04:34 UTC
Just to follow up on Nathan and my conversation on IRC, things are made rather more complicated than one might hope by a gradual increase in the number of processes, over time, with credential changes. For example, Mac OS X's sandboxing system, based on our MAC Framework, frequently experiences domain transitions, and we could anticipate similar changes.  It sounds like there is a net agreement that adopting a nice model for finer-grained, role-based privileges (e.g., the Solaris model) would have significant benefit to reducing the exposure in the event something did go wrong with unprivileged chroot -- and solve a number of other problems (e.g., unprivileged DTrace, better privilege-set abstractions for Jail), and is a worthy cause on the path to this sort of change. However, unprivileged chroot() will remain a sticky problem as programs of necessity place enormous trust in the UNIX filesystem namespace -- especially when it comes to features such as runtime linking, configuration files, etc, so if there is any form of 'call-gate'-style privilege escalation (e.g., setuid, setgid, TE MAC transitioning binaries), we could get ourselves into trouble.
Comment 11 Eitan Adler freebsd_committer freebsd_triage 2018-05-20 23:52:39 UTC
For bugs matching the following conditions:
- Status == In Progress
- Assignee == "bugs@FreeBSD.org"
- Last Modified Year <= 2017

Do
- Set Status to "Open"
Comment 12 Julian Elischer freebsd_committer 2018-05-21 03:51:33 UTC
If the ability to do this operation (unpriv chroot) is inherited, and the ability to set that bit is only settable by root then a process can only do this if a root ancestor has said that security is being lowered by this family of processes. I would even go as far as saying secure level would disable it along with a "no return" policy. (by which I mean once it is set in a process and then used you cannot get that ability back ... full stop.)

This would allow the use of the functionality for "build machine" type situations where in reality it is root or trusted proxy doing the chroot. In addition it should be a one-shot.. you use it , you lose it.
With the advent of "everyone has there own computer" I am not sure how important it is to have "real users"  be able to do builds.