Bug 121073

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 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

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 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 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 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.

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 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 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 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 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 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.

Comment 13 Ed Maste 2022-09-20 14:18:44 UTC

Implemented in:

commit a40cf4175c90142442d0c6515f6c83956336699b
Author: Edward Tomasz Napierala <trasz@FreeBSD.org>
Date:   Tue Jul 20 08:56:04 2021 +0000

    Implement unprivileged chroot
    
    This builds on recently introduced NO_NEW_PRIVS flag to implement
    unprivileged chroot, enabled by `security.bsd.unprivileged_chroot`.
    It allows non-root processes to chroot(2), provided they have the
    NO_NEW_PRIVS flag set.
    
    The chroot(8) utility gets a new flag, -n, which sets NO_NEW_PRIVS
    before chrooting.
    
    Reviewed By:    kib
    Sponsored By:   EPSRC
    Relnotes:       yes
    Differential Revision:  https://reviews.freebsd.org/D30130

Comment 14 Kubilay Kocak 2022-09-20 23:47:38 UTC

^Triage:

 - Correct resolutions (FIXED .. 'issue as reported', by commit, even if not by patch attached here)
 - Assign to committer that resolved
 - Add meta (review)
 - Set version to earliest (currently) supported branch/version

@trasz If this ended up being MFC'd to stable/13 (was probably current at the time?) or stable/12, please set mfc-stableX flags as appropriate