PR_SET_NO_NEW_PRIVS, unshare, and chroot

To make the no_new_privs discussion more concrete, here is an updated series that is actually useful. It adds PR_SET_NO_NEW_PRIVS with the same semantics as before (plus John Johansen's AppArmor fix and with improved bisectability). It then allows some unshare flags and chroot (sometimes) when no_new_privs is set.

The unprivileged chroot could be quite useful, even though it's rather constrained for now.

I think that blocking setresuid, setuid, and capset in no_new_privs mode will make this a little less useful. Comments are welcome.

For the git-inclined, this series is here:
https://git.kernel.org/?p=linux/kernel/git/luto/linux.git;a=shortlog;h=refs/heads/security/no_new_privs/patch_v2

Test it like this:

---- begin test case

#include <sys/prctl.h>
#include <stdio.h>
#include <unistd.h>
#include <errno.h>

#define PR_SET_NO_NEW_PRIVS 35
#define PR_GET_NO_NEW_PRIVS 36

int main()
{
int nnp = prctl(PR_GET_NO_NEW_PRIVS, 0, 0, 0, 0);

  if (nnp == -EINVAL) {
    printf("Failed!\n");
    return 1;

}

  printf("nnp was %d\n", nnp);

  if (prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0) != 0) {
    printf("Failed!\n");
    return 1;
  }

  nnp = prctl(PR_GET_NO_NEW_PRIVS, 0, 0, 0, 0);
  if (nnp == -EINVAL) {
    printf("Failed!\n");
    return 1;
  }

  printf("nnp is %d\n", nnp);

printf("here goes...\n");
execlp("bash", "bash", NULL);
printf("Failed to exec bash\n");
return 1;
}

---- end test case

Andy Lutomirski (3):
Add PR_{GET,SET}_NO_NEW_PRIVS to prevent execve from granting privs Allow unprivileged CLONE_NEWUTS and CLONE_NEWIPC with no_new_privs Allow unprivileged chroot when safe

John Johansen (1):
Fix apparmor for PR_{GET,SET}_NO_NEW_PRIVS

 fs/exec.c                  |   10 +++++++++-
 fs/open.c                  |   16 ++++++++++++++--
 include/linux/prctl.h      |   15 +++++++++++++++
 include/linux/sched.h      |    2 ++
 include/linux/security.h   |    1 +
 kernel/fork.c              |    2 ++
 kernel/nsproxy.c           |    8 +++++++-
 kernel/sys.c               |   10 ++++++++++
 security/apparmor/domain.c |   35 +++++++++++++++++++++++++++++++++++
 security/commoncap.c       |    7 +++++--
 security/selinux/hooks.c   |   10 +++++++++-
 11 files changed, 109 insertions(+), 7 deletions(-)

1.7.7.5

With this set, a lot of dangerous operations (chroot, unshare, etc) become a lot less dangerous because there is no possibility of subverting privileged binaries.

This patch completely breaks apparmor. Someone who understands (and uses) apparmor should fix it or at least give me a hint.

---
 fs/exec.c                  |   10 +++++++++-
 include/linux/prctl.h      |   15 +++++++++++++++
 include/linux/sched.h      |    2 ++
 include/linux/security.h   |    1 +
 kernel/fork.c              |    2 ++
 kernel/sys.c               |   10 ++++++++++
 security/apparmor/domain.c |    4 ++++
 security/commoncap.c       |    7 +++++--
 security/selinux/hooks.c   |   10 +++++++++-
 9 files changed, 57 insertions(+), 4 deletions(-)

index 25dcbe5..ca6a966 100644
@@ -1242,6 +1242,13 @@ int check_unsafe_exec(struct linux_binprm *bprm)
 			bprm->unsafe |= LSM_UNSAFE_PTRACE;
 	}
 
+	/*
+	 * This isn't strictly necessary, but it makes it harder for LSMs to
+	 * mess up.
+	 */
+	if (current->no_new_privs)
+		bprm->unsafe |= LSM_UNSAFE_NO_NEW_PRIVS;
+
 	n_fs = 1;
 	spin_lock(&p->fs->lock);
 	rcu_read_lock();

@@ -1285,7 +1292,8 @@ int prepare_binprm(struct linux_binprm *bprm)
 	bprm->cred->euid = current_euid();
 	bprm->cred->egid = current_egid();
 
-	if (!(bprm->file->f_path.mnt->mnt_flags & MNT_NOSUID)) {
+	if (!(bprm->file->f_path.mnt->mnt_flags & MNT_NOSUID) &&
+	    !current->no_new_privs) {
 		/* Set-uid? */
 		if (mode & S_ISUID) {
 			bprm->per_clear |= PER_CLEAR_ON_SETID;

index a3baeb2..7bafc98 100644
@@ -102,4 +102,19 @@
 
 #define PR_MCE_KILL_GET 34
 
+/*
+ * If no_new_privs is set, then operations that grant new privileges (i.e.
+ * execve) will either fail or not grant them.  This affects suid/sgid,
+ * file capabilities, and LSMs.
+ *
+ * Operations that merely manipulate or drop existing privileges (setresuid,
+ * capset, etc.) will still work.  Drop those privileges if you want them gone.
+ *
+ * Changing LSM security domain is considered a new privilege.  So, for example,
+ * asking selinux for a specific new context (e.g. with runcon) will result
+ * in execve returning -EPERM.
+ */
+#define PR_SET_NO_NEW_PRIVS 35
+#define PR_GET_NO_NEW_PRIVS 36
+
 #endif /* _LINUX_PRCTL_H */

index 41d0237..02b342c 100644
@@ -1300,6 +1300,8 @@ struct task_struct {
 				 * execve */
 	unsigned in_iowait:1;
 
+	/* task may not gain privileges */
+	unsigned no_new_privs:1;
 
 	/* Revert to default priority/policy when forking */
 	unsigned sched_reset_on_fork:1;

index ebd2a53..48294e0 100644
@@ -129,6 +129,7 @@ struct request_sock;
 #define LSM_UNSAFE_SHARE	1
 #define LSM_UNSAFE_PTRACE	2
 #define LSM_UNSAFE_PTRACE_CAP	4
+#define LSM_UNSAFE_NO_NEW_PRIVS	8
 
 #ifdef CONFIG_MMU
 /*

index 8e6b6f4..97cd34a 100644
@@ -1117,6 +1117,8 @@ static struct task_struct *copy_process(unsigned long clone_flags,
 	if (retval < 0)
 		goto bad_fork_free;
 
+	p->no_new_privs = current->no_new_privs;
+
 	/*
 	 * If multiple threads are within copy_process(), then this check
 	 * triggers too late. This doesn't hurt, the check is only there

index 1dbbe69..219b3dc 100644
@@ -1837,6 +1837,16 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3,
 			else
 				error = PR_MCE_KILL_DEFAULT;
 			break;
+		case PR_SET_NO_NEW_PRIVS:
+			if (arg2 != 1 || arg3 || arg4 || arg5)
+				return -EINVAL;
+
+			current->no_new_privs = 1;
+			break;
+		case PR_GET_NO_NEW_PRIVS:
+			if (arg2 || arg3 || arg4 || arg5)
+				return -EINVAL;
+			return current->no_new_privs ? 1 : 0;
 		default:
 			error = -EINVAL;
 			break;

index c1e18ba..a24ee44 100644
@@ -360,6 +360,10 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm)
 	if (bprm->cred_prepared)
 		return 0;
 
+	/* XXX: no_new_privs is not useable with AppArmor yet */
+	if (bprm->unsafe & LSM_UNSAFE_NO_NEW_PRIVS)
+		return -EPERM;
+
 	cxt = bprm->cred->security;
 	BUG_ON(!cxt);
 

index a93b3b7..b0e0f7b 100644
@@ -511,14 +511,17 @@ int cap_bprm_set_creds(struct linux_binprm *bprm)
 skip:
 
 	/* Don't let someone trace a set[ug]id/setpcap binary with the revised
-	 * credentials unless they have the appropriate permit
+	 * credentials unless they have the appropriate permit.
+	 *
+	 * In addition, if NO_NEW_PRIVS, then ensure we get no new privs.
 	 */
 	if ((new->euid != old->uid ||
 	     new->egid != old->gid ||
 	     !cap_issubset(new->cap_permitted, old->cap_permitted)) &&
 	    bprm->unsafe & ~LSM_UNSAFE_PTRACE_CAP) {
 		/* downgrade; they get no more than they had, and maybe less */
-		if (!capable(CAP_SETUID)) {
+		if (!capable(CAP_SETUID) ||
+		    (bprm->unsafe & LSM_UNSAFE_NO_NEW_PRIVS)) {
 			new->euid = new->uid;
 			new->egid = new->gid;
 		}

index 266a229..43eea9b 100644
@@ -2003,6 +2003,13 @@ static int selinux_bprm_set_creds(struct linux_binprm *bprm)
 		new_tsec->sid = old_tsec->exec_sid;
 		/* Reset exec SID on execve. */
 		new_tsec->exec_sid = 0;
+
+		/*
+		 * Minimize confusion: if no_new_privs and a transition is
+		 * explicitly requested, then fail the exec.
+		 */
+		if (bprm->unsafe & LSM_UNSAFE_NO_NEW_PRIVS)
+			return -EPERM;
 	} else {
 		/* Check for a default transition on this program. */
 		rc = security_transition_sid(old_tsec->sid, isec->sid,

@@ -2015,7 +2022,8 @@ static int selinux_bprm_set_creds(struct linux_binprm *bprm)
 	COMMON_AUDIT_DATA_INIT(&ad, PATH);
 	ad.u.path = bprm->file->f_path;
 
-	if (bprm->file->f_path.mnt->mnt_flags & MNT_NOSUID)
+	if ((bprm->file->f_path.mnt->mnt_flags & MNT_NOSUID) ||
+	    (bprm->unsafe & LSM_UNSAFE_NO_NEW_PRIVS))
 		new_tsec->sid = old_tsec->sid;
 
 	if (new_tsec->sid == old_tsec->sid) {

1.7.7.5

Yeah, sorry. Will fix in v3.

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

From: John Johansen <[email protected]>

---
 security/apparmor/domain.c |   39 +++++++++++++++++++++++++++++++++++----
 1 files changed, 35 insertions(+), 4 deletions(-)

index a24ee44..7316d77 100644
@@ -360,10 +360,6 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm)
 	if (bprm->cred_prepared)
 		return 0;
 
-	/* XXX: no_new_privs is not useable with AppArmor yet */
-	if (bprm->unsafe & LSM_UNSAFE_NO_NEW_PRIVS)
-		return -EPERM;
-
 	cxt = bprm->cred->security;
 	BUG_ON(!cxt);
 

@@ -399,6 +395,11 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm)
 			new_profile = find_attach(ns, &ns->base.profiles, name);
 		if (!new_profile)
 			goto cleanup;
+		/*
+		 * NOTE: Domain transitions from unconfined are allowed
+		 * even when no_new_privs is set because this aways results
+		 * in a further reduction of permissions.
+		 */
 		goto apply;
 	}
 

@@ -459,6 +460,16 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm)
 		/* fail exec */
 		error = -EACCES;
 
+	/*
+	 * Policy has specified a domain transition, if no_new_privs then
+	 * fail the exec.
+	 */
+	if (bprm->unsafe & LSM_UNSAFE_NO_NEW_PRIVS) {
+		aa_put_profile(new_profile);
+		error = -EPERM;
+		goto cleanup;
+	}
+
 	if (!new_profile)
 		goto audit;
 

@@ -613,6 +624,14 @@ int aa_change_hat(const char *hats[], int count, u64 token, bool permtest)
 	const char *target = NULL, *info = NULL;
 	int error = 0;
 
+	/*
+	 * Fail explicitly requested domain transitions if no_new_privs.
+	 * There is no exception for unconfined as change_hat is not
+	 * available.
+	 */
+	if (current->no_new_privs)
+		return -EPERM;
+
 	/* released below */
 	cred = get_current_cred();
 	cxt = cred->security;

@@ -754,6 +773,18 @@ int aa_change_profile(const char *ns_name, const char *hname, bool onexec,
 	cxt = cred->security;
 	profile = aa_cred_profile(cred);
 
+	/*
+	 * Fail explicitly requested domain transitions if no_new_privs
+	 * and not unconfined.
+	 * Domain transitions from unconfined are allowed even when
+	 * no_new_privs is set because this aways results in a reduction
+	 * of permissions.
+	 */
+	if (current->no_new_privs && !unconfined(profile)) {
+		put_cred(cred);
+		return -EPERM;
+	}
+
 	if (ns_name) {
 		/* released below */
 		ns = aa_find_namespace(profile->ns, ns_name);

1.7.7.5

They are normally disallowed because they could be used to subvert setuid programs. But if setuid is disabled, then they are safe.

---
 kernel/nsproxy.c |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

index 9aeab4b..471f4a3 100644
@@ -191,7 +191,13 @@ int unshare_nsproxy_namespaces(unsigned long unshare_flags,
 			       CLONE_NEWNET)))
 		return 0;
 
-	if (!capable(CAP_SYS_ADMIN))
+	/* We require either no_new_privs or CAP_SYS_ADMIN for all modes */
+	if (!current->no_new_privs && !capable(CAP_SYS_ADMIN))
+		return -EPERM;
+
+	/* NEWNS and NEWNET always require CAP_SYS_ADMIN. */
+	if ((unshare_flags & (CLONE_NEWNS | CLONE_NEWNET)) &&
+	    !capable(CAP_SYS_ADMIN))
 		return -EPERM;
 
 	*new_nsp = create_new_namespaces(unshare_flags, current,

1.7.7.5

Chroot can easily be used to subvert setuid programs. If no_new_privs, then setuid programs don't gain any privilege, so allow chroot.

Because chroot is an easy way to break out of chroot jail, CAP_SYS_ADMIN is still required if the caller is already chrooted.

---
 fs/open.c |   16 ++++++++++++++--
 1 files changed, 14 insertions(+), 2 deletions(-)

index f711921..80ca7e2 100644
@@ -422,6 +422,8 @@ SYSCALL_DEFINE1(chroot, const char __user *, filename)
 {
 	struct path path;
 	int error;
+	struct fs_struct *fs = current->fs;
+	bool is_chrooted;
 
 	error = user_path_dir(filename, &path);
 	if (error)

@@ -432,13 +434,23 @@ SYSCALL_DEFINE1(chroot, const char __user *, filename)
 		goto dput_and_out;
 
 	error = -EPERM;
-	if (!capable(CAP_SYS_CHROOT))
+	/*
+	 * Chroot is dangerous unless no_new_privs is set.  But we also
+	 * don't want to allow unprivileged users to break out of chroot
+	 * jail with another chroot call, so we require either CAP_SYS_CHROOT
+	 * unless we're not chrooted already and we have no_new_privs.
+	 */
+	is_chrooted = (fs->root.mnt->mnt_mountpoint !=
+		       fs->root.mnt->mnt_parent->mnt_root ||
+		       fs->root.dentry != fs->root.mnt->mnt_root);
+	if (!(current->no_new_privs && !is_chrooted) &&
+	    !capable(CAP_SYS_CHROOT))
 		goto dput_and_out;
 	error = security_path_chroot(&path);
 	if (error)
 		goto dput_and_out;
 
-	set_fs_root(current->fs, &path);
+	set_fs_root(fs, &path);
 	error = 0;
 dput_and_out:
 	path_put(&path);

1.7.7.5

So I think this whole chroot thing needs more people looking at it. I brought up chroot as an example, but there may be other reasons why you don't want user chrooting things than just the setuid confusion.

There's also the whole issue with doing things like local non-root bind mounts, which are arguably more useful than chroot, and which are disallowed for similar reasons. So I don't think chroot is all that special.

Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Agreed. There are plenty of security people cc'd. Thoughts (and attacks) are welcome!

They're almost certainly more useful. Binding the tree of your choice on top of / is a nice (and more secure) way to emulate chroot. The only downside I've thought of in five minutes is that it would prevent the administrator from blocking access to a directory by bind-mounting something on to of it -- an unprivileged non-recursive bind mount of the containing filesystem would get the hidden directory back. I'm not sure this is a real problem.

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

This part is pretty gross. It means it won't work for stuff like containers (systemd-nspawn etc.) and furthermore I have plans that involve running OS trees inside a chroot, and this would obviously not work for that.

Incidentally I ended up putting my setuid program here:
http://git.gnome.org/browse/linux-user-chroot/

Now unfortunately, even if we say that a new setuid program is the way to gain these privileges, you still can't nest it, because all of these things are predicated on disabling setuid programs. But it would at least not fail initially if your process was inside a chroot.

Agreed.

Unprivileged bind mounts would be a much better approach, but that would need some concept of an unprivileged user owning a namespace. Maybe the namespace id work would make this work.

--Andy

Indeed, I do run many of my machines inside a chroot. The real filesystem has:

/distro/that
/distro/newer
/distro/this

instead of partitions. I'm chroot'd and fully booted up in /distro/this, although I can see files in the others and I might run a few things from them as well.

This isn't "userland chroot", it's the top level of the process tree:
/distro/this/sbin/init. It would be a shame if it behaved differently just because "it's a chroot".

-- Jamie

I compiled it, booted it, and tested it. I based it off an oldish kernel, though, so I can rebase.

The first approach I tried was (from memory -- may not compile at all on any version) fs->root.mnt != fs->root.mnt->mnt_parent. That didn't work. The issue is that on dracut-based distros, AFAICT, the root (in the sense of the root of the tree of struct vfsmounts) is rootfs. The apparent root (the filesystem containing /, /usr, etc) is mounted on top of (rootfs)/. Dracut then does something with the effect of chroot("/"). So you end up with the vfsmount that contains "/" not being the actual root vfsmount. But there's nothing hidden by the chroot -- even if fs->root.mnt pointed at rootfs, "/" would still follow the mountpoint into the actual filesystem.

An different approach would be to have fs_struct keep track of a hard and a soft root. chroot would stay CAP_SYS_ADMIN only and change both roots. A new unprivileged_chroot would change only the soft root. follow_dotdot would check both, so unprivileged_chroot wouldn't be useful for breaking chroot. The big downside would be an extra branch on every follow_dotdot.

--Andy

That has nothing whatsoever to do with dracut. _Everything_ ends up that way; IOW, everything including init(8) runs chrooted into the final userland root. On any normal distro. Your test is complete BS - e.g. mount /dev/crap /mnt/blah
mount /dev/garbage /mnt/blah
chroot /mnt/blah
will *NOT* be chrooted per your definition.

Whoops. That needs fixing.

--Andy

Here's another reason to allow setuid/setresuid when no_new_privs is set:

I'd like a pam_no_new_privs module that lets me set no_new_privs for certain highly untrusted users. The way to do it is:

PR_SET_NO_NEW_PRIVS
setuid(uid)
execve(the user's shell)

If setuid were disallowed, it would have to be:

setuid(uid)
PR_SET_NO_NEW_PRIVS
execve(the user's shell)

The latter is racy: the user can log in once, then log in a second time and ptrace the login or sshd process between setuid and PR_SET_NO_NEW_PRIVS.

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

I think it'd be clearer to call it PR_SET_NOSUID - basically it should match the semantics for MS_NOSUID mounts, as if on every exec() thereafter the target binary was on a nosuid filesystem.

You might then change this flag to only take effect on a later exec(), which would solve your race condition for the hypothetical PAM module.

What's the rationale behind the unshare subset? Did you actually analyze any setuid binaries found on Debian/Fedora etc. and determined that e.g. CLONE_NEWNET was problematic for some reason?

I actually want CLONE_NEWNET for my build tool, so I can be sure the arbitrary code I'm executing as part of the build at least isn't downloading more new code.

The MS_NOSUID semantics are somewhat ridiculous for selinux, and I'd rather not make them match for no_new_privs. AppArmor completely ignores MS_NOSUID, so I think the rename would just cause more confusion.

That would just make it more complicated. The race is already solved in the current patch, anyway.

CLONE_NEWNET seems more likely to consume significant kernel resources than the others. I didn't have a great reason, though. Unsharing the filesystem namespace is possibly dangerous because it could prevent an unmount in the original namespace from taking effect everywhere.

Fair enough. I may add this in v3. seccomp is an even better solution, though :)

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

I don't see how they're ridiculous.

Note your patch for selinux does exactly the same thing in the NOSUID case and your NO_NEW_PRIVS flag. Right?

- if (bprm->file->f_path.mnt->mnt_flags & MNT_NOSUID)
+ if ((bprm->file->f_path.mnt->mnt_flags & MNT_NOSUID) ||
+ (bprm->unsafe & LSM_UNSAFE_NO_NEW_PRIVS))

                new_tsec->sid = old_tsec->sid;

Ugh...well, I guess if it doesn't store any security data associated with files, only with file names, then there's nothing for it to do. Like I said before though, I think SELinux is the only sane LSM.

This actually brings up something we need to think about - if we're heading towards being able to do bind mounts as non-root (which is necessary for me) we'd need limits on e.g. the number of mounts that can be made for a given uid/cgroup.

I have a picked-from-thin-air hardcoded limit of 50 in my setuid binary, but I just realized that that's 50*RLIMIT_NPROC which is kind of large...

Hmmm...hadn't considered that either. So the issue here is if a server admin has e.g. a NFS mount and my build tool makes a new copy of the mount namespace, a process may still have it busy when she goes to unmount it?

Yeah, definitely more flexible, though realistic use of seccomp depends on someone making a nice userspace tool to compile sets of syscalls like "no networking".

exec_sid is silently ignored. So runcon will not switch context but will still appear to succeed.

See several lines up.

Nope. It looks it up by file name or path, I think.

I think the fact that there is a bprm_set_creds hook at all is insane, but maybe that's just me. I think this is one of the things that Windows does far better than POSIX. On Windows, CreateProcess (the moral equivalent of execve) never gains privileges.

--Andy