From 70169420f555210147f3cab74bb0f6debd488bdb Mon Sep 17 00:00:00 2001 From: "Eric W. Biederman" Date: Thu, 17 Nov 2016 01:38:35 -0600 Subject: exec: Don't reset euid and egid when the tracee has CAP_SETUID Don't reset euid and egid when the tracee has CAP_SETUID in it's user namespace. I punted on relaxing this permission check long ago but now that I have read this code closely it is clear it is safe to test against CAP_SETUID in the user namespace. Signed-off-by: "Eric W. Biederman" --- security/commoncap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'security') diff --git a/security/commoncap.c b/security/commoncap.c index 8df676fbd393..feb6044f701d 100644 --- a/security/commoncap.c +++ b/security/commoncap.c @@ -550,7 +550,7 @@ skip: !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 (!ns_capable(new->user_ns, CAP_SETUID) || (bprm->unsafe & LSM_UNSAFE_NO_NEW_PRIVS)) { new->euid = new->uid; new->egid = new->gid; -- cgit v1.2.3-59-g8ed1b From 20523132ec5d1b481e1d66557292ed3a3021e817 Mon Sep 17 00:00:00 2001 From: "Eric W. Biederman" Date: Mon, 23 Jan 2017 17:17:26 +1300 Subject: exec: Test the ptracer's saved cred to see if the tracee can gain caps Now that we have user namespaces and non-global capabilities verify the tracer has capabilities in the relevant user namespace instead of in the current_user_ns(). As the test for setting LSM_UNSAFE_PTRACE_CAP is currently ptracer_capable(p, current_user_ns()) and the new task credentials are in current_user_ns() this change does not have any user visible change and simply moves the test to where it is used, making the code easier to read. Signed-off-by: "Eric W. Biederman" --- security/commoncap.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'security') diff --git a/security/commoncap.c b/security/commoncap.c index feb6044f701d..cbb203c91406 100644 --- a/security/commoncap.c +++ b/security/commoncap.c @@ -548,7 +548,8 @@ skip: if ((is_setid || !cap_issubset(new->cap_permitted, old->cap_permitted)) && - bprm->unsafe & ~LSM_UNSAFE_PTRACE_CAP) { + ((bprm->unsafe & ~(LSM_UNSAFE_PTRACE | LSM_UNSAFE_PTRACE_CAP)) || + !ptracer_capable(current, new->user_ns))) { /* downgrade; they get no more than they had, and maybe less */ if (!ns_capable(new->user_ns, CAP_SETUID) || (bprm->unsafe & LSM_UNSAFE_NO_NEW_PRIVS)) { -- cgit v1.2.3-59-g8ed1b From 9227dd2a84a765fcfef1677ff17de0958b192eda Mon Sep 17 00:00:00 2001 From: "Eric W. Biederman" Date: Mon, 23 Jan 2017 17:26:31 +1300 Subject: exec: Remove LSM_UNSAFE_PTRACE_CAP With previous changes every location that tests for LSM_UNSAFE_PTRACE_CAP also tests for LSM_UNSAFE_PTRACE making the LSM_UNSAFE_PTRACE_CAP redundant, so remove it. Signed-off-by: "Eric W. Biederman" --- fs/exec.c | 8 ++------ include/linux/security.h | 3 +-- security/apparmor/domain.c | 2 +- security/commoncap.c | 2 +- security/selinux/hooks.c | 3 +-- security/smack/smack_lsm.c | 2 +- 6 files changed, 7 insertions(+), 13 deletions(-) (limited to 'security') diff --git a/fs/exec.c b/fs/exec.c index e57946610733..c195ebb8e2aa 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -1426,12 +1426,8 @@ static void check_unsafe_exec(struct linux_binprm *bprm) struct task_struct *p = current, *t; unsigned n_fs; - if (p->ptrace) { - if (ptracer_capable(p, current_user_ns())) - bprm->unsafe |= LSM_UNSAFE_PTRACE_CAP; - else - bprm->unsafe |= LSM_UNSAFE_PTRACE; - } + if (p->ptrace) + bprm->unsafe |= LSM_UNSAFE_PTRACE; /* * This isn't strictly necessary, but it makes it harder for LSMs to diff --git a/include/linux/security.h b/include/linux/security.h index c2125e9093e8..9d9ee90f1f35 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -140,8 +140,7 @@ struct request_sock; /* bprm->unsafe reasons */ #define LSM_UNSAFE_SHARE 1 #define LSM_UNSAFE_PTRACE 2 -#define LSM_UNSAFE_PTRACE_CAP 4 -#define LSM_UNSAFE_NO_NEW_PRIVS 8 +#define LSM_UNSAFE_NO_NEW_PRIVS 4 #ifdef CONFIG_MMU extern int mmap_min_addr_handler(struct ctl_table *table, int write, diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c index a4d90aa1045a..04185b7fd38a 100644 --- a/security/apparmor/domain.c +++ b/security/apparmor/domain.c @@ -469,7 +469,7 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm) ; } - if (bprm->unsafe & (LSM_UNSAFE_PTRACE | LSM_UNSAFE_PTRACE_CAP)) { + if (bprm->unsafe & LSM_UNSAFE_PTRACE) { error = may_change_ptraced_domain(new_profile); if (error) goto audit; diff --git a/security/commoncap.c b/security/commoncap.c index cbb203c91406..8ec6b7fe909e 100644 --- a/security/commoncap.c +++ b/security/commoncap.c @@ -548,7 +548,7 @@ skip: if ((is_setid || !cap_issubset(new->cap_permitted, old->cap_permitted)) && - ((bprm->unsafe & ~(LSM_UNSAFE_PTRACE | LSM_UNSAFE_PTRACE_CAP)) || + ((bprm->unsafe & ~LSM_UNSAFE_PTRACE) || !ptracer_capable(current, new->user_ns))) { /* downgrade; they get no more than they had, and maybe less */ if (!ns_capable(new->user_ns, CAP_SETUID) || diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index c7c6619431d5..cece6fe55f02 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -2404,8 +2404,7 @@ static int selinux_bprm_set_creds(struct linux_binprm *bprm) /* Make sure that anyone attempting to ptrace over a task that * changes its SID has the appropriate permit */ - if (bprm->unsafe & - (LSM_UNSAFE_PTRACE | LSM_UNSAFE_PTRACE_CAP)) { + if (bprm->unsafe & LSM_UNSAFE_PTRACE) { u32 ptsid = ptrace_parent_sid(current); if (ptsid != 0) { rc = avc_has_perm(ptsid, new_tsec->sid, diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index 94dc9d406ce3..bc2ff09f1494 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -934,7 +934,7 @@ static int smack_bprm_set_creds(struct linux_binprm *bprm) isp->smk_task != sbsp->smk_root) return 0; - if (bprm->unsafe & (LSM_UNSAFE_PTRACE | LSM_UNSAFE_PTRACE_CAP)) { + if (bprm->unsafe & LSM_UNSAFE_PTRACE) { struct task_struct *tracer; rc = 0; -- cgit v1.2.3-59-g8ed1b