From patchwork Thu May 28 15:41:22 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Eric W. Biederman" X-Patchwork-Id: 11576187 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id D1E041391 for ; Thu, 28 May 2020 15:45:35 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id BCB25207BC for ; Thu, 28 May 2020 15:45:35 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2404587AbgE1Ppe (ORCPT ); Thu, 28 May 2020 11:45:34 -0400 Received: from out01.mta.xmission.com ([166.70.13.231]:58144 "EHLO out01.mta.xmission.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2404617AbgE1Pp2 (ORCPT ); Thu, 28 May 2020 11:45:28 -0400 Received: from in01.mta.xmission.com ([166.70.13.51]) by out01.mta.xmission.com with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1jeKic-0000P5-1Z; Thu, 28 May 2020 09:45:18 -0600 Received: from ip68-227-160-95.om.om.cox.net ([68.227.160.95] helo=x220.xmission.com) by in01.mta.xmission.com with esmtpsa (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.87) (envelope-from ) id 1jeKiZ-0006cv-J1; Thu, 28 May 2020 09:45:17 -0600 From: ebiederm@xmission.com (Eric W. Biederman) To: Cc: Linus Torvalds , Oleg Nesterov , Jann Horn , Kees Cook , Greg Ungerer , Rob Landley , Bernd Edlinger , , Al Viro , Alexey Dobriyan , Andrew Morton , Casey Schaufler , linux-security-module@vger.kernel.org, James Morris , "Serge E. Hallyn" , Andy Lutomirski References: <87h7wujhmz.fsf@x220.int.ebiederm.org> <87sgga6ze4.fsf@x220.int.ebiederm.org> <87v9l4zyla.fsf_-_@x220.int.ebiederm.org> <877dx822er.fsf_-_@x220.int.ebiederm.org> <87k10wysqz.fsf_-_@x220.int.ebiederm.org> Date: Thu, 28 May 2020 10:41:22 -0500 In-Reply-To: <87k10wysqz.fsf_-_@x220.int.ebiederm.org> (Eric W. Biederman's message of "Thu, 28 May 2020 10:38:28 -0500") Message-ID: <87eer4ysm5.fsf_-_@x220.int.ebiederm.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) MIME-Version: 1.0 X-XM-SPF: eid=1jeKiZ-0006cv-J1;;;mid=<87eer4ysm5.fsf_-_@x220.int.ebiederm.org>;;;hst=in01.mta.xmission.com;;;ip=68.227.160.95;;;frm=ebiederm@xmission.com;;;spf=neutral X-XM-AID: U2FsdGVkX18haYcbIQZKo6eWp1tkySil7Dv2wO8d9Cs= X-SA-Exim-Connect-IP: 68.227.160.95 X-SA-Exim-Mail-From: ebiederm@xmission.com X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on sa05.xmission.com X-Spam-Level: ** X-Spam-Status: No, score=2.5 required=8.0 tests=ALL_TRUSTED,BAYES_50, DCC_CHECK_NEGATIVE,LotsOfNums_01,T_TooManySym_01,T_TooManySym_02, T_TooManySym_03,XMNoVowels autolearn=disabled version=3.4.2 X-Spam-Report: * -1.0 ALL_TRUSTED Passed through trusted hosts only via SMTP * 0.8 BAYES_50 BODY: Bayes spam probability is 40 to 60% * [score: 0.5000] * 1.5 XMNoVowels Alpha-numberic number with no vowels * 1.2 LotsOfNums_01 BODY: Lots of long strings of numbers * -0.0 DCC_CHECK_NEGATIVE Not listed in DCC * [sa05 0; Body=1 Fuz1=1 Fuz2=1] * 0.0 T_TooManySym_03 6+ unique symbols in subject * 0.0 T_TooManySym_02 5+ unique symbols in subject * 0.0 T_TooManySym_01 4+ unique symbols in subject X-Spam-DCC: ; sa05 0; Body=1 Fuz1=1 Fuz2=1 X-Spam-Combo: **; X-Spam-Relay-Country: X-Spam-Timing: total 1330 ms - load_scoreonly_sql: 0.13 (0.0%), signal_user_changed: 14 (1.0%), b_tie_ro: 12 (0.9%), parse: 1.61 (0.1%), extract_message_metadata: 14 (1.1%), get_uri_detail_list: 3.1 (0.2%), tests_pri_-1000: 16 (1.2%), tests_pri_-950: 1.98 (0.1%), tests_pri_-900: 1.32 (0.1%), tests_pri_-90: 233 (17.6%), check_bayes: 232 (17.4%), b_tokenize: 23 (1.7%), b_tok_get_all: 12 (0.9%), b_comp_prob: 3.8 (0.3%), b_tok_touch_all: 188 (14.1%), b_finish: 1.18 (0.1%), tests_pri_0: 1026 (77.1%), check_dkim_signature: 1.20 (0.1%), check_dkim_adsp: 3.0 (0.2%), poll_dns_idle: 0.65 (0.0%), tests_pri_10: 2.4 (0.2%), tests_pri_500: 15 (1.1%), rewrite_mail: 0.00 (0.0%) Subject: [PATCH 01/11] exec: Reduce bprm->per_clear to a single bit X-Spam-Flag: No X-SA-Exim-Version: 4.2.1 (built Thu, 05 May 2016 13:38:54 -0600) X-SA-Exim-Scanned: Yes (on in01.mta.xmission.com) Sender: owner-linux-security-module@vger.kernel.org Precedence: bulk List-ID: The bprm->per_clear field only takes the values 0 and PER_CLEAR_ON_SETID. Reduce the field to a signle bit to make it clear that the only question is should the dangerous personality bits be cleared or not. Update the documentation of the security lsm hooks. Signed-off-by: "Eric W. Biederman" --- fs/exec.c | 7 ++++--- include/linux/binfmts.h | 4 +++- include/linux/lsm_hooks.h | 4 ++++ security/apparmor/domain.c | 2 +- security/commoncap.c | 2 +- security/selinux/hooks.c | 2 +- security/smack/smack_lsm.c | 2 +- 7 files changed, 15 insertions(+), 8 deletions(-) diff --git a/fs/exec.c b/fs/exec.c index c3c879a55d65..51fab62b9fca 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -1354,7 +1354,8 @@ int begin_new_exec(struct linux_binprm * bprm) me->flags &= ~(PF_RANDOMIZE | PF_FORKNOEXEC | PF_KTHREAD | PF_NOFREEZE | PF_NO_SETAFFINITY); flush_thread(); - me->personality &= ~bprm->per_clear; + if (bprm->per_clear) + me->personality &= ~PER_CLEAR_ON_SETID; /* * We have to apply CLOEXEC before we change whether the process is @@ -1628,12 +1629,12 @@ static void bprm_fill_uid(struct linux_binprm *bprm) return; if (mode & S_ISUID) { - bprm->per_clear |= PER_CLEAR_ON_SETID; + bprm->per_clear = 1; bprm->cred->euid = uid; } if ((mode & (S_ISGID | S_IXGRP)) == (S_ISGID | S_IXGRP)) { - bprm->per_clear |= PER_CLEAR_ON_SETID; + bprm->per_clear = 1; bprm->cred->egid = gid; } } diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h index 7fc05929c967..e7959a6a895a 100644 --- a/include/linux/binfmts.h +++ b/include/linux/binfmts.h @@ -26,6 +26,9 @@ struct linux_binprm { unsigned long p; /* current top of mem */ unsigned long argmin; /* rlimit marker for copy_strings() */ unsigned int + /* Should unsafe personality bits be cleared? */ + per_clear:1, + /* Should an execfd be passed to userspace? */ have_execfd:1, @@ -55,7 +58,6 @@ struct linux_binprm { struct file * file; struct cred *cred; /* new credentials */ int unsafe; /* how unsafe this exec is (mask of LSM_UNSAFE_*) */ - unsigned int per_clear; /* bits to clear in current->personality */ int argc, envc; const char * filename; /* Name of binary as seen by procps */ const char * interp; /* Name of the binary really executed. Most diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h index d618ecc4d660..0ca68ad53592 100644 --- a/include/linux/lsm_hooks.h +++ b/include/linux/lsm_hooks.h @@ -42,6 +42,8 @@ * (e.g. for transitions between security domains). * The hook must set @bprm->secureexec to 1 if AT_SECURE should be set to * request libc enable secure mode. + * The hook must set @bprm->per_clear to 1 if the dangerous personality + * bits must be cleared from current->personality. * @bprm contains the linux_binprm structure. * Return 0 if the hook is successful and permission is granted. * @bprm_repopulate_creds: @@ -55,6 +57,8 @@ * transitions between security domains). * The hook must set @bprm->active_secureexec to 1 if AT_SECURE should be set to * request libc enable secure mode. + * The hook must set @bprm->per_clear to 1 if the dangerous personality + * bits must be cleared from current->personality. * @bprm contains the linux_binprm structure. * Return 0 if the hook is successful and permission is granted. * @bprm_check_security: diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c index 0b870a647488..c6d00735a40a 100644 --- a/security/apparmor/domain.c +++ b/security/apparmor/domain.c @@ -962,7 +962,7 @@ int apparmor_bprm_creds_for_exec(struct linux_binprm *bprm) aa_label_printk(new, GFP_KERNEL); dbg_printk("\n"); } - bprm->per_clear |= PER_CLEAR_ON_SETID; + bprm->per_clear = 1; } aa_put_label(cred_label(bprm->cred)); /* transfer reference, released when cred is freed */ diff --git a/security/commoncap.c b/security/commoncap.c index 77b04cb6feac..48b556046483 100644 --- a/security/commoncap.c +++ b/security/commoncap.c @@ -826,7 +826,7 @@ int cap_bprm_repopulate_creds(struct linux_binprm *bprm) /* if we have fs caps, clear dangerous personality flags */ if (__cap_gained(permitted, new, old)) - bprm->per_clear |= PER_CLEAR_ON_SETID; + bprm->per_clear = 1; /* Don't let someone trace a set[ug]id/setpcap binary with the revised * credentials unless they have the appropriate permit. diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 718345dd76bb..6bea1b879fdb 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -2385,7 +2385,7 @@ static int selinux_bprm_creds_for_exec(struct linux_binprm *bprm) } /* Clear any possibly unsafe personality bits on exec: */ - bprm->per_clear |= PER_CLEAR_ON_SETID; + bprm->per_clear = 1; /* Enable secure mode for SIDs transitions unless the noatsecure permission is granted between diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index 0ac8f4518d07..a0d2fad27b33 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -933,7 +933,7 @@ static int smack_bprm_creds_for_exec(struct linux_binprm *bprm) return -EPERM; bsp->smk_task = isp->smk_task; - bprm->per_clear |= PER_CLEAR_ON_SETID; + bprm->per_clear = 1; /* Decide if this is a secure exec. */ if (bsp->smk_task != bsp->smk_forked) From patchwork Thu May 28 15:42:06 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Eric W. Biederman" X-Patchwork-Id: 11576197 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 1229A1391 for ; Thu, 28 May 2020 15:46:05 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 02F94207F5 for ; Thu, 28 May 2020 15:46:04 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2404643AbgE1PqC (ORCPT ); Thu, 28 May 2020 11:46:02 -0400 Received: from out03.mta.xmission.com ([166.70.13.233]:47054 "EHLO out03.mta.xmission.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2404511AbgE1PqB (ORCPT ); Thu, 28 May 2020 11:46:01 -0400 Received: from in01.mta.xmission.com ([166.70.13.51]) by out03.mta.xmission.com with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1jeKjH-0003Zc-Mm; Thu, 28 May 2020 09:45:59 -0600 Received: from ip68-227-160-95.om.om.cox.net ([68.227.160.95] helo=x220.xmission.com) by in01.mta.xmission.com with esmtpsa (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.87) (envelope-from ) id 1jeKjF-0006q8-Rr; Thu, 28 May 2020 09:45:59 -0600 From: ebiederm@xmission.com (Eric W. Biederman) To: Cc: Linus Torvalds , Oleg Nesterov , Jann Horn , Kees Cook , Greg Ungerer , Rob Landley , Bernd Edlinger , , Al Viro , Alexey Dobriyan , Andrew Morton , Casey Schaufler , linux-security-module@vger.kernel.org, James Morris , "Serge E. Hallyn" , Andy Lutomirski References: <87h7wujhmz.fsf@x220.int.ebiederm.org> <87sgga6ze4.fsf@x220.int.ebiederm.org> <87v9l4zyla.fsf_-_@x220.int.ebiederm.org> <877dx822er.fsf_-_@x220.int.ebiederm.org> <87k10wysqz.fsf_-_@x220.int.ebiederm.org> Date: Thu, 28 May 2020 10:42:06 -0500 In-Reply-To: <87k10wysqz.fsf_-_@x220.int.ebiederm.org> (Eric W. Biederman's message of "Thu, 28 May 2020 10:38:28 -0500") Message-ID: <878shcyskx.fsf_-_@x220.int.ebiederm.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) MIME-Version: 1.0 X-XM-SPF: eid=1jeKjF-0006q8-Rr;;;mid=<878shcyskx.fsf_-_@x220.int.ebiederm.org>;;;hst=in01.mta.xmission.com;;;ip=68.227.160.95;;;frm=ebiederm@xmission.com;;;spf=neutral X-XM-AID: U2FsdGVkX1+Bo5/YnCMJYnYnBAcEfoRXNYXUapdUns8= X-SA-Exim-Connect-IP: 68.227.160.95 X-SA-Exim-Mail-From: ebiederm@xmission.com X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on sa01.xmission.com X-Spam-Level: ** X-Spam-Status: No, score=2.0 required=8.0 tests=ALL_TRUSTED,BAYES_50, DCC_CHECK_NEGATIVE,NO_DNS_FOR_FROM,T_TooManySym_01,XMNoVowels, XMSubLong autolearn=disabled version=3.4.2 X-Spam-Virus: No X-Spam-Report: * -1.0 ALL_TRUSTED Passed through trusted hosts only via SMTP * 0.8 BAYES_50 BODY: Bayes spam probability is 40 to 60% * [score: 0.5000] * 1.5 XMNoVowels Alpha-numberic number with no vowels * 0.7 XMSubLong Long Subject * 0.0 NO_DNS_FOR_FROM DNS: Envelope sender has no MX or A DNS records * -0.0 DCC_CHECK_NEGATIVE Not listed in DCC * [sa01 0; Body=1 Fuz1=1 Fuz2=1] * 0.0 T_TooManySym_01 4+ unique symbols in subject X-Spam-DCC: ; sa01 0; Body=1 Fuz1=1 Fuz2=1 X-Spam-Combo: **; X-Spam-Relay-Country: X-Spam-Timing: total 1354 ms - load_scoreonly_sql: 0.04 (0.0%), signal_user_changed: 4.0 (0.3%), b_tie_ro: 2.7 (0.2%), parse: 1.78 (0.1%), extract_message_metadata: 15 (1.1%), get_uri_detail_list: 3.8 (0.3%), tests_pri_-1000: 4.3 (0.3%), tests_pri_-950: 1.49 (0.1%), tests_pri_-900: 1.17 (0.1%), tests_pri_-90: 103 (7.6%), check_bayes: 101 (7.5%), b_tokenize: 14 (1.0%), b_tok_get_all: 9 (0.7%), b_comp_prob: 2.3 (0.2%), b_tok_touch_all: 72 (5.3%), b_finish: 0.74 (0.1%), tests_pri_0: 1212 (89.5%), check_dkim_signature: 0.40 (0.0%), check_dkim_adsp: 862 (63.7%), poll_dns_idle: 858 (63.3%), tests_pri_10: 1.74 (0.1%), tests_pri_500: 5 (0.4%), rewrite_mail: 0.00 (0.0%) Subject: [PATCH 02/11] exec: Introduce active_per_clear the per file version of per_clear X-Spam-Flag: No X-SA-Exim-Version: 4.2.1 (built Thu, 05 May 2016 13:38:54 -0600) X-SA-Exim-Scanned: Yes (on in01.mta.xmission.com) Sender: owner-linux-security-module@vger.kernel.org Precedence: bulk List-ID: When the credentials have been recomputed per file the per_clear status has not been recomputed. Update the per file calcuations to recompute per_clear on a per file basis in a separate variable and to combine that variable into the final per_clear value. This makes which personality bits are clear not depend on the permissions of shell scripts with interpreters, but instead only on the final bprm->file that bprm_fill_uid and security_bprm_repopulate_creds are called upon. History Tree: git://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git Fixes: 1bb0fa189c6a ("[PATCH] NX: clean up legacy binary support") Signed-off-by: "Eric W. Biederman" --- fs/exec.c | 7 ++++--- include/linux/binfmts.h | 3 +++ include/linux/lsm_hooks.h | 2 +- security/commoncap.c | 2 +- 4 files changed, 9 insertions(+), 5 deletions(-) diff --git a/fs/exec.c b/fs/exec.c index 51fab62b9fca..221d12dcaa3e 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -1354,7 +1354,7 @@ int begin_new_exec(struct linux_binprm * bprm) me->flags &= ~(PF_RANDOMIZE | PF_FORKNOEXEC | PF_KTHREAD | PF_NOFREEZE | PF_NO_SETAFFINITY); flush_thread(); - if (bprm->per_clear) + if (bprm->per_clear || bprm->active_per_clear) me->personality &= ~PER_CLEAR_ON_SETID; /* @@ -1629,12 +1629,12 @@ static void bprm_fill_uid(struct linux_binprm *bprm) return; if (mode & S_ISUID) { - bprm->per_clear = 1; + bprm->active_per_clear = 1; bprm->cred->euid = uid; } if ((mode & (S_ISGID | S_IXGRP)) == (S_ISGID | S_IXGRP)) { - bprm->per_clear = 1; + bprm->active_per_clear = 1; bprm->cred->egid = gid; } } @@ -1655,6 +1655,7 @@ static int prepare_binprm(struct linux_binprm *bprm) /* Recompute parts of bprm->cred based on bprm->file */ bprm->active_secureexec = 0; + bprm->active_per_clear = 0; bprm_fill_uid(bprm); retval = security_bprm_repopulate_creds(bprm); if (retval) diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h index e7959a6a895a..89231a689957 100644 --- a/include/linux/binfmts.h +++ b/include/linux/binfmts.h @@ -26,6 +26,9 @@ struct linux_binprm { unsigned long p; /* current top of mem */ unsigned long argmin; /* rlimit marker for copy_strings() */ unsigned int + /* Does bprm->file warrant clearing personality bits? */ + active_per_clear:1, + /* Should unsafe personality bits be cleared? */ per_clear:1, diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h index 0ca68ad53592..62e60e55cb99 100644 --- a/include/linux/lsm_hooks.h +++ b/include/linux/lsm_hooks.h @@ -57,7 +57,7 @@ * transitions between security domains). * The hook must set @bprm->active_secureexec to 1 if AT_SECURE should be set to * request libc enable secure mode. - * The hook must set @bprm->per_clear to 1 if the dangerous personality + * The hook must set @bprm->active_per_clear to 1 if the dangerous personality * bits must be cleared from current->personality. * @bprm contains the linux_binprm structure. * Return 0 if the hook is successful and permission is granted. diff --git a/security/commoncap.c b/security/commoncap.c index 48b556046483..0b72d7bf23e1 100644 --- a/security/commoncap.c +++ b/security/commoncap.c @@ -826,7 +826,7 @@ int cap_bprm_repopulate_creds(struct linux_binprm *bprm) /* if we have fs caps, clear dangerous personality flags */ if (__cap_gained(permitted, new, old)) - bprm->per_clear = 1; + bprm->active_per_clear = 1; /* Don't let someone trace a set[ug]id/setpcap binary with the revised * credentials unless they have the appropriate permit. From patchwork Thu May 28 15:42:40 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Eric W. Biederman" X-Patchwork-Id: 11576203 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 9BA5A60D for ; Thu, 28 May 2020 15:46:37 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 84829207BC for ; Thu, 28 May 2020 15:46:37 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2404526AbgE1Pqg (ORCPT ); Thu, 28 May 2020 11:46:36 -0400 Received: from out03.mta.xmission.com ([166.70.13.233]:47282 "EHLO out03.mta.xmission.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2404511AbgE1Pqf (ORCPT ); Thu, 28 May 2020 11:46:35 -0400 Received: from in01.mta.xmission.com ([166.70.13.51]) by out03.mta.xmission.com with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1jeKjp-0003d4-LB; Thu, 28 May 2020 09:46:33 -0600 Received: from ip68-227-160-95.om.om.cox.net ([68.227.160.95] helo=x220.xmission.com) by in01.mta.xmission.com with esmtpsa (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.87) (envelope-from ) id 1jeKjo-0006wE-5f; Thu, 28 May 2020 09:46:33 -0600 From: ebiederm@xmission.com (Eric W. Biederman) To: Cc: Linus Torvalds , Oleg Nesterov , Jann Horn , Kees Cook , Greg Ungerer , Rob Landley , Bernd Edlinger , , Al Viro , Alexey Dobriyan , Andrew Morton , Casey Schaufler , linux-security-module@vger.kernel.org, James Morris , "Serge E. Hallyn" , Andy Lutomirski References: <87h7wujhmz.fsf@x220.int.ebiederm.org> <87sgga6ze4.fsf@x220.int.ebiederm.org> <87v9l4zyla.fsf_-_@x220.int.ebiederm.org> <877dx822er.fsf_-_@x220.int.ebiederm.org> <87k10wysqz.fsf_-_@x220.int.ebiederm.org> Date: Thu, 28 May 2020 10:42:40 -0500 In-Reply-To: <87k10wysqz.fsf_-_@x220.int.ebiederm.org> (Eric W. Biederman's message of "Thu, 28 May 2020 10:38:28 -0500") Message-ID: <87367kysjz.fsf_-_@x220.int.ebiederm.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) MIME-Version: 1.0 X-XM-SPF: eid=1jeKjo-0006wE-5f;;;mid=<87367kysjz.fsf_-_@x220.int.ebiederm.org>;;;hst=in01.mta.xmission.com;;;ip=68.227.160.95;;;frm=ebiederm@xmission.com;;;spf=neutral X-XM-AID: U2FsdGVkX19In91xGLctcIhWOZF7QnMbxz9oDXYbZF8= X-SA-Exim-Connect-IP: 68.227.160.95 X-SA-Exim-Mail-From: ebiederm@xmission.com X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on sa04.xmission.com X-Spam-Level: * X-Spam-Status: No, score=1.3 required=8.0 tests=ALL_TRUSTED,BAYES_50, DCC_CHECK_NEGATIVE,XMNoVowels autolearn=disabled version=3.4.2 X-Spam-Report: * -1.0 ALL_TRUSTED Passed through trusted hosts only via SMTP * 0.8 BAYES_50 BODY: Bayes spam probability is 40 to 60% * [score: 0.5000] * 1.5 XMNoVowels Alpha-numberic number with no vowels * -0.0 DCC_CHECK_NEGATIVE Not listed in DCC * [sa04 0; Body=1 Fuz1=1 Fuz2=1] X-Spam-DCC: ; sa04 0; Body=1 Fuz1=1 Fuz2=1 X-Spam-Combo: *; X-Spam-Relay-Country: X-Spam-Timing: total 1008 ms - load_scoreonly_sql: 0.50 (0.1%), signal_user_changed: 15 (1.5%), b_tie_ro: 12 (1.2%), parse: 2.6 (0.3%), extract_message_metadata: 30 (3.0%), get_uri_detail_list: 12 (1.2%), tests_pri_-1000: 19 (1.9%), tests_pri_-950: 1.72 (0.2%), tests_pri_-900: 1.41 (0.1%), tests_pri_-90: 127 (12.6%), check_bayes: 124 (12.3%), b_tokenize: 30 (3.0%), b_tok_get_all: 17 (1.7%), b_comp_prob: 5 (0.5%), b_tok_touch_all: 68 (6.7%), b_finish: 1.17 (0.1%), tests_pri_0: 790 (78.3%), check_dkim_signature: 1.01 (0.1%), check_dkim_adsp: 3.1 (0.3%), poll_dns_idle: 0.86 (0.1%), tests_pri_10: 4.4 (0.4%), tests_pri_500: 10 (1.0%), rewrite_mail: 0.00 (0.0%) Subject: [PATCH 03/11] exec: Compute file based creds only once X-Spam-Flag: No X-SA-Exim-Version: 4.2.1 (built Thu, 05 May 2016 13:38:54 -0600) X-SA-Exim-Scanned: Yes (on in01.mta.xmission.com) Sender: owner-linux-security-module@vger.kernel.org Precedence: bulk List-ID: Move the computation of creds from prepare_binfmt into begin_new_exec so that the creds can be computed only onc. I have looked through the kernel and verified none of the binfmts look at bprm->cred directly so computing the bprm->cred later should be safe. Rename preserve_creds to execfd_creds to make it clear that the creds should be derived from the executable file descriptor. Remove active_secureexec and active_per_clear and use secureexec and per_clear respectively. The active versions of these variables were only necessary to allow their values to be recomputed from scratch for each value of bprm->file. Remove the now unnecessary work from bprm_fill_uid to reset the bprm->cred->euid and bprm->cred->egid, and add a small comment about what bprm_fill_uid now does. Remove the now unnecessary work in cap_bprm_creds_from_file to reset the ambient capabilities, and add a small comment about what cap_bprm_creds_from_file does. Signed-off-by: "Eric W. Biederman" --- fs/binfmt_misc.c | 2 +- fs/exec.c | 65 +++++++++++++++++------------------ include/linux/binfmts.h | 12 ++----- include/linux/lsm_hook_defs.h | 2 +- include/linux/lsm_hooks.h | 19 +++++----- include/linux/security.h | 8 ++--- security/commoncap.c | 12 +++---- security/security.c | 4 +-- 8 files changed, 57 insertions(+), 67 deletions(-) diff --git a/fs/binfmt_misc.c b/fs/binfmt_misc.c index 53968ea07b57..bc5506619b7e 100644 --- a/fs/binfmt_misc.c +++ b/fs/binfmt_misc.c @@ -192,7 +192,7 @@ static int load_misc_binary(struct linux_binprm *bprm) bprm->interpreter = interp_file; if (fmt->flags & MISC_FMT_CREDENTIALS) - bprm->preserve_creds = 1; + bprm->execfd_creds = 1; retval = 0; ret: diff --git a/fs/exec.c b/fs/exec.c index 221d12dcaa3e..091ff6269610 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -72,6 +72,8 @@ #include +static int bprm_creds_from_file(struct linux_binprm *bprm); + int suid_dumpable = 0; static LIST_HEAD(formats); @@ -1304,6 +1306,11 @@ int begin_new_exec(struct linux_binprm * bprm) struct task_struct *me = current; int retval; + /* Once we are committed compute the creds */ + retval = bprm_creds_from_file(bprm); + if (retval) + return retval; + /* * Ensure all future errors are fatal. */ @@ -1354,7 +1361,7 @@ int begin_new_exec(struct linux_binprm * bprm) me->flags &= ~(PF_RANDOMIZE | PF_FORKNOEXEC | PF_KTHREAD | PF_NOFREEZE | PF_NO_SETAFFINITY); flush_thread(); - if (bprm->per_clear || bprm->active_per_clear) + if (bprm->per_clear) me->personality &= ~PER_CLEAR_ON_SETID; /* @@ -1365,13 +1372,6 @@ int begin_new_exec(struct linux_binprm * bprm) */ do_close_on_exec(me->files); - /* - * Once here, prepare_binrpm() will not be called any more, so - * the final state of setuid/setgid/fscaps can be merged into the - * secureexec flag. - */ - bprm->secureexec |= bprm->active_secureexec; - if (bprm->secureexec) { /* Make sure parent cannot signal privileged process. */ me->pdeath_signal = 0; @@ -1589,20 +1589,12 @@ static void check_unsafe_exec(struct linux_binprm *bprm) static void bprm_fill_uid(struct linux_binprm *bprm) { + /* Handle suid and sgid on files */ struct inode *inode; unsigned int mode; kuid_t uid; kgid_t gid; - /* - * Since this can be called multiple times (via prepare_binprm), - * we must clear any previous work done when setting set[ug]id - * bits from any earlier bprm->file uses (for example when run - * first for a setuid script then again for its interpreter). - */ - bprm->cred->euid = current_euid(); - bprm->cred->egid = current_egid(); - if (!mnt_may_suid(bprm->file->f_path.mnt)) return; @@ -1629,19 +1621,38 @@ static void bprm_fill_uid(struct linux_binprm *bprm) return; if (mode & S_ISUID) { - bprm->active_per_clear = 1; + bprm->per_clear = 1; bprm->cred->euid = uid; } if ((mode & (S_ISGID | S_IXGRP)) == (S_ISGID | S_IXGRP)) { - bprm->active_per_clear = 1; + bprm->per_clear = 1; bprm->cred->egid = gid; } } +/* + * Compute brpm->cred based upon the final binary. + */ +static int bprm_creds_from_file(struct linux_binprm *bprm) +{ + struct file *file = bprm->file; + int retval; + + /* Compute creds from the executable passed to userspace? */ + if (bprm->execfd_creds) + bprm->file = bprm->executable; + + bprm_fill_uid(bprm); + retval = security_bprm_creds_from_file(bprm); + bprm->file = file; + + return retval; +} + /* * Fill the binprm structure from the inode. - * Check permissions, then read the first BINPRM_BUF_SIZE bytes + * Read the first BINPRM_BUF_SIZE bytes * * This may be called multiple times for binary chains (scripts for example). */ @@ -1649,20 +1660,6 @@ static int prepare_binprm(struct linux_binprm *bprm) { loff_t pos = 0; - /* Can the interpreter get to the executable without races? */ - if (!bprm->preserve_creds) { - int retval; - - /* Recompute parts of bprm->cred based on bprm->file */ - bprm->active_secureexec = 0; - bprm->active_per_clear = 0; - bprm_fill_uid(bprm); - retval = security_bprm_repopulate_creds(bprm); - if (retval) - return retval; - } - bprm->preserve_creds = 0; - memset(bprm->buf, 0, BINPRM_BUF_SIZE); return kernel_read(bprm->file, bprm->buf, BINPRM_BUF_SIZE, &pos); } diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h index 89231a689957..39f6b5a7ace7 100644 --- a/include/linux/binfmts.h +++ b/include/linux/binfmts.h @@ -26,22 +26,14 @@ struct linux_binprm { unsigned long p; /* current top of mem */ unsigned long argmin; /* rlimit marker for copy_strings() */ unsigned int - /* Does bprm->file warrant clearing personality bits? */ - active_per_clear:1, - /* Should unsafe personality bits be cleared? */ per_clear:1, /* Should an execfd be passed to userspace? */ have_execfd:1, - /* It is safe to use the creds of a script (see binfmt_misc) */ - preserve_creds:1, - /* - * True if most recent call to security_bprm_set_creds - * resulted in elevated privileges. - */ - active_secureexec:1, + /* Use the creds of a script (see binfmt_misc) */ + execfd_creds:1, /* * Set by bprm_creds_for_exec hook to indicate a * privilege-gaining exec has happened. Used to set diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h index 1e295ba12c0d..36b07c1eb0f1 100644 --- a/include/linux/lsm_hook_defs.h +++ b/include/linux/lsm_hook_defs.h @@ -50,7 +50,7 @@ LSM_HOOK(int, 0, settime, const struct timespec64 *ts, const struct timezone *tz) LSM_HOOK(int, 0, vm_enough_memory, struct mm_struct *mm, long pages) LSM_HOOK(int, 0, bprm_creds_for_exec, struct linux_binprm *bprm) -LSM_HOOK(int, 0, bprm_repopulate_creds, struct linux_binprm *bprm) +LSM_HOOK(int, 0, bprm_creds_from_file, struct linux_binprm *bprm) LSM_HOOK(int, 0, bprm_check_security, struct linux_binprm *bprm) LSM_HOOK(void, LSM_RET_VOID, bprm_committing_creds, struct linux_binprm *bprm) LSM_HOOK(void, LSM_RET_VOID, bprm_committed_creds, struct linux_binprm *bprm) diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h index 62e60e55cb99..0aeaa3de69b2 100644 --- a/include/linux/lsm_hooks.h +++ b/include/linux/lsm_hooks.h @@ -46,18 +46,19 @@ * bits must be cleared from current->personality. * @bprm contains the linux_binprm structure. * Return 0 if the hook is successful and permission is granted. - * @bprm_repopulate_creds: - * Assuming that the relevant bits of @bprm->cred->security have been - * previously set, examine @bprm->file and regenerate them. This is - * so that the credentials derived from the interpreter the code is - * actually going to run are used rather than credentials derived - * from a script. This done because the interpreter binary needs to - * reopen script, and may end up opening something completely different. + * @bprm_creds_from_file: + * If @bprm->file is setpcap, suid, sgid or otherwise marked to + * change the privilege level upon exec update @bprm->cred to + * handle the marking on the file. This is called after finding + * the native code binary that will be executed. This ensures that + * the credentials will not be derived from a script that the binary + * will need to reopen, which when reopend may end up being a completely + * different file. * This hook may also optionally check permissions (e.g. for * transitions between security domains). - * The hook must set @bprm->active_secureexec to 1 if AT_SECURE should be set to + * The hook must set @bprm->secureexec to 1 if AT_SECURE should be set to * request libc enable secure mode. - * The hook must set @bprm->active_per_clear to 1 if the dangerous personality + * The hook must set @bprm->per_clear to 1 if the dangerous personality * bits must be cleared from current->personality. * @bprm contains the linux_binprm structure. * Return 0 if the hook is successful and permission is granted. diff --git a/include/linux/security.h b/include/linux/security.h index 6dcec9375e8f..df8ad2fb7374 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -140,7 +140,7 @@ extern int cap_capset(struct cred *new, const struct cred *old, const kernel_cap_t *effective, const kernel_cap_t *inheritable, const kernel_cap_t *permitted); -extern int cap_bprm_repopulate_creds(struct linux_binprm *bprm); +extern int cap_bprm_creds_from_file(struct linux_binprm *bprm); extern int cap_inode_setxattr(struct dentry *dentry, const char *name, const void *value, size_t size, int flags); extern int cap_inode_removexattr(struct dentry *dentry, const char *name); @@ -277,7 +277,7 @@ int security_syslog(int type); int security_settime64(const struct timespec64 *ts, const struct timezone *tz); int security_vm_enough_memory_mm(struct mm_struct *mm, long pages); int security_bprm_creds_for_exec(struct linux_binprm *bprm); -int security_bprm_repopulate_creds(struct linux_binprm *bprm); +int security_bprm_creds_from_file(struct linux_binprm *bprm); int security_bprm_check(struct linux_binprm *bprm); void security_bprm_committing_creds(struct linux_binprm *bprm); void security_bprm_committed_creds(struct linux_binprm *bprm); @@ -575,9 +575,9 @@ static inline int security_bprm_creds_for_exec(struct linux_binprm *bprm) return 0; } -static inline int security_bprm_repopulate_creds(struct linux_binprm *bprm) +static inline int security_bprm_creds_from_file(struct linux_binprm *bprm) { - return cap_bprm_repopulate_creds(bprm); + return cap_bprm_creds_from_file(bprm); } static inline int security_bprm_check(struct linux_binprm *bprm) diff --git a/security/commoncap.c b/security/commoncap.c index 0b72d7bf23e1..2bd1f24f3796 100644 --- a/security/commoncap.c +++ b/security/commoncap.c @@ -797,22 +797,22 @@ static inline bool nonroot_raised_pE(struct cred *new, const struct cred *old, } /** - * cap_bprm_repopulate_creds - Set up the proposed credentials for execve(). + * cap_bprm_creds_from_file - Set up the proposed credentials for execve(). * @bprm: The execution parameters, including the proposed creds * * Set up the proposed credentials for a new execution context being * constructed by execve(). The proposed creds in @bprm->cred is altered, * which won't take effect immediately. Returns 0 if successful, -ve on error. */ -int cap_bprm_repopulate_creds(struct linux_binprm *bprm) +int cap_bprm_creds_from_file(struct linux_binprm *bprm) { + /* Process setpcap binaries and capabilities for uid 0 */ const struct cred *old = current_cred(); struct cred *new = bprm->cred; bool effective = false, has_fcap = false, is_setid; int ret; kuid_t root_uid; - new->cap_ambient = old->cap_ambient; if (WARN_ON(!cap_ambient_invariant_ok(old))) return -EPERM; @@ -826,7 +826,7 @@ int cap_bprm_repopulate_creds(struct linux_binprm *bprm) /* if we have fs caps, clear dangerous personality flags */ if (__cap_gained(permitted, new, old)) - bprm->active_per_clear = 1; + bprm->per_clear = 1; /* Don't let someone trace a set[ug]id/setpcap binary with the revised * credentials unless they have the appropriate permit. @@ -889,7 +889,7 @@ int cap_bprm_repopulate_creds(struct linux_binprm *bprm) (!__is_real(root_uid, new) && (effective || __cap_grew(permitted, ambient, new)))) - bprm->active_secureexec = 1; + bprm->secureexec = 1; return 0; } @@ -1346,7 +1346,7 @@ static struct security_hook_list capability_hooks[] __lsm_ro_after_init = { LSM_HOOK_INIT(ptrace_traceme, cap_ptrace_traceme), LSM_HOOK_INIT(capget, cap_capget), LSM_HOOK_INIT(capset, cap_capset), - LSM_HOOK_INIT(bprm_repopulate_creds, cap_bprm_repopulate_creds), + LSM_HOOK_INIT(bprm_creds_from_file, cap_bprm_creds_from_file), LSM_HOOK_INIT(inode_need_killpriv, cap_inode_need_killpriv), LSM_HOOK_INIT(inode_killpriv, cap_inode_killpriv), LSM_HOOK_INIT(inode_getsecurity, cap_inode_getsecurity), diff --git a/security/security.c b/security/security.c index b890b7e2a765..0688359bf8f4 100644 --- a/security/security.c +++ b/security/security.c @@ -828,9 +828,9 @@ int security_bprm_creds_for_exec(struct linux_binprm *bprm) return call_int_hook(bprm_creds_for_exec, 0, bprm); } -int security_bprm_repopulate_creds(struct linux_binprm *bprm) +int security_bprm_creds_from_file(struct linux_binprm *bprm) { - return call_int_hook(bprm_repopulate_creds, 0, bprm); + return call_int_hook(bprm_creds_from_file, 0, bprm); } int security_bprm_check(struct linux_binprm *bprm) From patchwork Thu May 28 15:43:09 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Eric W. Biederman" X-Patchwork-Id: 11576217 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 4F68A1391 for ; Thu, 28 May 2020 15:47:39 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 3979F207F5 for ; Thu, 28 May 2020 15:47:39 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2404701AbgE1PrS (ORCPT ); Thu, 28 May 2020 11:47:18 -0400 Received: from out02.mta.xmission.com ([166.70.13.232]:50642 "EHLO out02.mta.xmission.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2404679AbgE1PrF (ORCPT ); Thu, 28 May 2020 11:47:05 -0400 Received: from in02.mta.xmission.com ([166.70.13.52]) by out02.mta.xmission.com with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1jeKkJ-0004Qw-2N; Thu, 28 May 2020 09:47:03 -0600 Received: from ip68-227-160-95.om.om.cox.net ([68.227.160.95] helo=x220.xmission.com) by in02.mta.xmission.com with esmtpsa (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.87) (envelope-from ) id 1jeKkH-00024D-UJ; Thu, 28 May 2020 09:47:02 -0600 From: ebiederm@xmission.com (Eric W. Biederman) To: Cc: Linus Torvalds , Oleg Nesterov , Jann Horn , Kees Cook , Greg Ungerer , Rob Landley , Bernd Edlinger , , Al Viro , Alexey Dobriyan , Andrew Morton , Casey Schaufler , linux-security-module@vger.kernel.org, James Morris , "Serge E. Hallyn" , Andy Lutomirski References: <87h7wujhmz.fsf@x220.int.ebiederm.org> <87sgga6ze4.fsf@x220.int.ebiederm.org> <87v9l4zyla.fsf_-_@x220.int.ebiederm.org> <877dx822er.fsf_-_@x220.int.ebiederm.org> <87k10wysqz.fsf_-_@x220.int.ebiederm.org> Date: Thu, 28 May 2020 10:43:09 -0500 In-Reply-To: <87k10wysqz.fsf_-_@x220.int.ebiederm.org> (Eric W. Biederman's message of "Thu, 28 May 2020 10:38:28 -0500") Message-ID: <87wo4wxdyq.fsf_-_@x220.int.ebiederm.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) MIME-Version: 1.0 X-XM-SPF: eid=1jeKkH-00024D-UJ;;;mid=<87wo4wxdyq.fsf_-_@x220.int.ebiederm.org>;;;hst=in02.mta.xmission.com;;;ip=68.227.160.95;;;frm=ebiederm@xmission.com;;;spf=neutral X-XM-AID: U2FsdGVkX1+4yct8JYeIUy8W9FZXXCz8gjMVFKhpUuQ= X-SA-Exim-Connect-IP: 68.227.160.95 X-SA-Exim-Mail-From: ebiederm@xmission.com X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on sa04.xmission.com X-Spam-Level: **** X-Spam-Status: No, score=4.3 required=8.0 tests=ALL_TRUSTED,BAYES_50, DCC_CHECK_NEGATIVE,T_TM2_M_HEADER_IN_MSG,T_TooManySym_01,XMNoVowels, XMSubLong,XMSubMetaSxObfu_03,XMSubMetaSx_00 autolearn=disabled version=3.4.2 X-Spam-Report: * -1.0 ALL_TRUSTED Passed through trusted hosts only via SMTP * 0.8 BAYES_50 BODY: Bayes spam probability is 40 to 60% * [score: 0.5000] * 0.7 XMSubLong Long Subject * 1.5 XMNoVowels Alpha-numberic number with no vowels * 0.0 T_TM2_M_HEADER_IN_MSG BODY: No description available. * -0.0 DCC_CHECK_NEGATIVE Not listed in DCC * [sa04 0; Body=1 Fuz1=1 Fuz2=1] * 1.2 XMSubMetaSxObfu_03 Obfuscated Sexy Noun-People * 1.0 XMSubMetaSx_00 1+ Sexy Words * 0.0 T_TooManySym_01 4+ unique symbols in subject X-Spam-DCC: ; sa04 0; Body=1 Fuz1=1 Fuz2=1 X-Spam-Combo: ****; X-Spam-Relay-Country: X-Spam-Timing: total 737 ms - load_scoreonly_sql: 0.17 (0.0%), signal_user_changed: 15 (2.1%), b_tie_ro: 13 (1.7%), parse: 2.9 (0.4%), extract_message_metadata: 26 (3.5%), get_uri_detail_list: 6 (0.8%), tests_pri_-1000: 20 (2.8%), tests_pri_-950: 1.82 (0.2%), tests_pri_-900: 1.40 (0.2%), tests_pri_-90: 200 (27.1%), check_bayes: 198 (26.8%), b_tokenize: 17 (2.3%), b_tok_get_all: 9 (1.3%), b_comp_prob: 3.7 (0.5%), b_tok_touch_all: 162 (22.0%), b_finish: 1.20 (0.2%), tests_pri_0: 449 (61.0%), check_dkim_signature: 1.07 (0.1%), check_dkim_adsp: 3.0 (0.4%), poll_dns_idle: 0.66 (0.1%), tests_pri_10: 2.2 (0.3%), tests_pri_500: 11 (1.5%), rewrite_mail: 0.00 (0.0%) Subject: [PATCH 04/11] exec: Move uid/gid handling from creds_from_file into bprm_fill_uid X-Spam-Flag: No X-SA-Exim-Version: 4.2.1 (built Thu, 05 May 2016 13:38:54 -0600) X-SA-Exim-Scanned: Yes (on in02.mta.xmission.com) Sender: owner-linux-security-module@vger.kernel.org Precedence: bulk List-ID: The logic in cap_bprm_creds_from_file is difficult to follow in part because it handles both uids/gids and capabilities. That difficulty in following the code has resulted in several small bugs. Move the handling of uids/gids into bprm_fill_uid to make the code clearer. A small bug is fixed where the ambient capabilities were unnecessarily cleared when the presence of a ptracer or a shared fs_struct resulted in the setuid or setgid not being honored. This bug was not possible to leave in place with the movement of the uids and gids handling out of cap_bprm_repopultate_creds. The rest of the bugs I have tried to make more apparent but left in tact when moving the code into bprm_fill_uid. Ref: ee67ae7ef6ff ("commoncap: Move cap_elevated calculation into bprm_set_creds") Fixes: 58319057b784 ("capabilities: ambient capabilities") Signed-off-by: "Eric W. Biederman" --- fs/exec.c | 49 ++++++++++++++++++++++++++++++++++++-------- security/commoncap.c | 25 +++++++--------------- 2 files changed, 48 insertions(+), 26 deletions(-) diff --git a/fs/exec.c b/fs/exec.c index 091ff6269610..956ee3a0d824 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -1590,21 +1590,23 @@ static void check_unsafe_exec(struct linux_binprm *bprm) static void bprm_fill_uid(struct linux_binprm *bprm) { /* Handle suid and sgid on files */ + struct cred *new = bprm->cred; struct inode *inode; unsigned int mode; + bool need_cap; kuid_t uid; kgid_t gid; if (!mnt_may_suid(bprm->file->f_path.mnt)) - return; + goto after_setid; if (task_no_new_privs(current)) - return; + goto after_setid; inode = bprm->file->f_path.dentry->d_inode; mode = READ_ONCE(inode->i_mode); if (!(mode & (S_ISUID|S_ISGID))) - return; + goto after_setid; /* Be careful if suid/sgid is set */ inode_lock(inode); @@ -1616,19 +1618,50 @@ static void bprm_fill_uid(struct linux_binprm *bprm) inode_unlock(inode); /* We ignore suid/sgid if there are no mappings for them in the ns */ - if (!kuid_has_mapping(bprm->cred->user_ns, uid) || - !kgid_has_mapping(bprm->cred->user_ns, gid)) - return; + if (!kuid_has_mapping(new->user_ns, uid) || + !kgid_has_mapping(new->user_ns, gid)) + goto after_setid; if (mode & S_ISUID) { bprm->per_clear = 1; - bprm->cred->euid = uid; + new->euid = uid; } if ((mode & (S_ISGID | S_IXGRP)) == (S_ISGID | S_IXGRP)) { bprm->per_clear = 1; - bprm->cred->egid = gid; + new->egid = gid; + } + +after_setid: + /* Will the new creds have multiple uids or gids? */ + if (!uid_eq(new->euid, new->uid) || !gid_eq(new->egid, new->gid)) { + bprm->secureexec = 1; + + /* + * Is the root directory and working directory shared or is + * the process traced and the tracing process does not have + * CAP_SYS_PTRACE? + * + * In either case it is not safe to change the euid or egid + * unless the current process has the appropriate cap and so + * chaning the euid or egid was already possible. + */ + need_cap = bprm->unsafe & LSM_UNSAFE_SHARE || + !ptracer_capable(current, new->user_ns); + if (need_cap && !uid_eq(new->euid, new->uid) && + (!ns_capable(new->user_ns, CAP_SETUID) || + (bprm->unsafe & LSM_UNSAFE_NO_NEW_PRIVS))) { + new->euid = new->uid; + } + if (need_cap && !gid_eq(new->egid, new->gid) && + (!ns_capable(new->user_ns, CAP_SETUID) || + (bprm->unsafe & LSM_UNSAFE_NO_NEW_PRIVS))) { + new->egid = new->gid; + } } + + new->suid = new->fsuid = new->euid; + new->sgid = new->fsgid = new->egid; } /* diff --git a/security/commoncap.c b/security/commoncap.c index 2bd1f24f3796..b39c7511862e 100644 --- a/security/commoncap.c +++ b/security/commoncap.c @@ -809,7 +809,7 @@ int cap_bprm_creds_from_file(struct linux_binprm *bprm) /* Process setpcap binaries and capabilities for uid 0 */ const struct cred *old = current_cred(); struct cred *new = bprm->cred; - bool effective = false, has_fcap = false, is_setid; + bool effective = false, has_fcap = false; int ret; kuid_t root_uid; @@ -828,31 +828,21 @@ int cap_bprm_creds_from_file(struct linux_binprm *bprm) if (__cap_gained(permitted, new, old)) bprm->per_clear = 1; - /* Don't let someone trace a set[ug]id/setpcap binary with the revised + /* Don't let someone trace a setpcap binary with the revised * credentials unless they have the appropriate permit. * * In addition, if NO_NEW_PRIVS, then ensure we get no new privs. */ - is_setid = __is_setuid(new, old) || __is_setgid(new, old); - - if ((is_setid || __cap_gained(permitted, new, old)) && + if (__cap_gained(permitted, new, old) && ((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) || - (bprm->unsafe & LSM_UNSAFE_NO_NEW_PRIVS)) { - new->euid = new->uid; - new->egid = new->gid; - } new->cap_permitted = cap_intersect(new->cap_permitted, old->cap_permitted); } - new->suid = new->fsuid = new->euid; - new->sgid = new->fsgid = new->egid; - /* File caps or setid cancels ambient. */ - if (has_fcap || is_setid) + if (has_fcap || __is_setuid(new, old) || __is_setgid(new, old)) cap_clear(new->cap_ambient); /* @@ -885,10 +875,9 @@ int cap_bprm_creds_from_file(struct linux_binprm *bprm) return -EPERM; /* Check for privilege-elevated exec. */ - if (is_setid || - (!__is_real(root_uid, new) && - (effective || - __cap_grew(permitted, ambient, new)))) + if (!__is_real(root_uid, new) && + (effective || + __cap_grew(permitted, ambient, new))) bprm->secureexec = 1; return 0; From patchwork Thu May 28 15:44:35 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Eric W. Biederman" X-Patchwork-Id: 11576231 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 944EE1391 for ; Thu, 28 May 2020 15:48:34 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 85565207F9 for ; Thu, 28 May 2020 15:48:34 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2404600AbgE1Psb (ORCPT ); Thu, 28 May 2020 11:48:31 -0400 Received: from out03.mta.xmission.com ([166.70.13.233]:47924 "EHLO out03.mta.xmission.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2404544AbgE1Ps3 (ORCPT ); Thu, 28 May 2020 11:48:29 -0400 Received: from in01.mta.xmission.com ([166.70.13.51]) by out03.mta.xmission.com with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1jeKlg-0003vA-65; Thu, 28 May 2020 09:48:28 -0600 Received: from ip68-227-160-95.om.om.cox.net ([68.227.160.95] helo=x220.xmission.com) by in01.mta.xmission.com with esmtpsa (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.87) (envelope-from ) id 1jeKlf-0007Eb-7U; Thu, 28 May 2020 09:48:28 -0600 From: ebiederm@xmission.com (Eric W. Biederman) To: Cc: Linus Torvalds , Oleg Nesterov , Jann Horn , Kees Cook , Greg Ungerer , Rob Landley , Bernd Edlinger , , Al Viro , Alexey Dobriyan , Andrew Morton , Casey Schaufler , linux-security-module@vger.kernel.org, James Morris , "Serge E. Hallyn" , Andy Lutomirski References: <87h7wujhmz.fsf@x220.int.ebiederm.org> <87sgga6ze4.fsf@x220.int.ebiederm.org> <87v9l4zyla.fsf_-_@x220.int.ebiederm.org> <877dx822er.fsf_-_@x220.int.ebiederm.org> <87k10wysqz.fsf_-_@x220.int.ebiederm.org> Date: Thu, 28 May 2020 10:44:35 -0500 In-Reply-To: <87k10wysqz.fsf_-_@x220.int.ebiederm.org> (Eric W. Biederman's message of "Thu, 28 May 2020 10:38:28 -0500") Message-ID: <87lflcxdwc.fsf_-_@x220.int.ebiederm.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) MIME-Version: 1.0 X-XM-SPF: eid=1jeKlf-0007Eb-7U;;;mid=<87lflcxdwc.fsf_-_@x220.int.ebiederm.org>;;;hst=in01.mta.xmission.com;;;ip=68.227.160.95;;;frm=ebiederm@xmission.com;;;spf=neutral X-XM-AID: U2FsdGVkX1/QbUmGNs6WJY8er87D7LRHfXhQCTcWzGM= X-SA-Exim-Connect-IP: 68.227.160.95 X-SA-Exim-Mail-From: ebiederm@xmission.com X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on sa05.xmission.com X-Spam-Level: **** X-Spam-Status: No, score=4.3 required=8.0 tests=ALL_TRUSTED,BAYES_50, DCC_CHECK_NEGATIVE,T_TM2_M_HEADER_IN_MSG,T_TooManySym_01,XMNoVowels, XMSubLong,XMSubMetaSxObfu_03,XMSubMetaSx_00 autolearn=disabled version=3.4.2 X-Spam-Report: * -1.0 ALL_TRUSTED Passed through trusted hosts only via SMTP * 0.8 BAYES_50 BODY: Bayes spam probability is 40 to 60% * [score: 0.5000] * 0.7 XMSubLong Long Subject * 1.5 XMNoVowels Alpha-numberic number with no vowels * 0.0 T_TM2_M_HEADER_IN_MSG BODY: No description available. * -0.0 DCC_CHECK_NEGATIVE Not listed in DCC * [sa05 0; Body=1 Fuz1=1 Fuz2=1] * 1.0 XMSubMetaSx_00 1+ Sexy Words * 1.2 XMSubMetaSxObfu_03 Obfuscated Sexy Noun-People * 0.0 T_TooManySym_01 4+ unique symbols in subject X-Spam-DCC: ; sa05 0; Body=1 Fuz1=1 Fuz2=1 X-Spam-Combo: ****; X-Spam-Relay-Country: X-Spam-Timing: total 506 ms - load_scoreonly_sql: 0.10 (0.0%), signal_user_changed: 12 (2.4%), b_tie_ro: 10 (1.9%), parse: 1.84 (0.4%), extract_message_metadata: 24 (4.6%), get_uri_detail_list: 2.0 (0.4%), tests_pri_-1000: 36 (7.1%), tests_pri_-950: 2.4 (0.5%), tests_pri_-900: 1.68 (0.3%), tests_pri_-90: 129 (25.6%), check_bayes: 126 (24.9%), b_tokenize: 25 (4.9%), b_tok_get_all: 12 (2.4%), b_comp_prob: 5 (1.0%), b_tok_touch_all: 77 (15.1%), b_finish: 2.6 (0.5%), tests_pri_0: 278 (55.0%), check_dkim_signature: 1.08 (0.2%), check_dkim_adsp: 4.3 (0.9%), poll_dns_idle: 0.56 (0.1%), tests_pri_10: 2.1 (0.4%), tests_pri_500: 12 (2.4%), rewrite_mail: 0.00 (0.0%) Subject: [PATCH 05/11] exec: In bprm_fill_uid use CAP_SETGID to see if a gid change is safe X-Spam-Flag: No X-SA-Exim-Version: 4.2.1 (built Thu, 05 May 2016 13:38:54 -0600) X-SA-Exim-Scanned: Yes (on in01.mta.xmission.com) Sender: owner-linux-security-module@vger.kernel.org Precedence: bulk List-ID: If the task has CAP_SETGID and a shared fs struct or is being ptraced than it is clear that nothing new is being introduced when the gid changes, and so it is safe to honor a setgid executable. However if all we know is that the task has CAP_SETUID things are less clear. This bug looks like it was introduced in v2.1.100 when !suser was replaced by !capable(CAP_SETUID). It appears to have been an oversight at that time. Fixing this 22 years later seems weird but even now it still looks worth fixing. As conceptually what is happening is testing to see if the process already had the potential to make a gid change or if the trancer needs permissions in addition to the permissions needed to trace the process to trace the process through a gid change. Fixes: v2.1.100 Signed-off-by: "Eric W. Biederman" --- fs/exec.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/exec.c b/fs/exec.c index 956ee3a0d824..bac8db14f30d 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -1654,7 +1654,7 @@ static void bprm_fill_uid(struct linux_binprm *bprm) new->euid = new->uid; } if (need_cap && !gid_eq(new->egid, new->gid) && - (!ns_capable(new->user_ns, CAP_SETUID) || + (!ns_capable(new->user_ns, CAP_SETGID) || (bprm->unsafe & LSM_UNSAFE_NO_NEW_PRIVS))) { new->egid = new->gid; } From patchwork Thu May 28 15:48:11 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Eric W. Biederman" X-Patchwork-Id: 11576237 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 19A7E13B4 for ; Thu, 28 May 2020 15:52:17 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 0CA5F207D3 for ; Thu, 28 May 2020 15:52:17 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2404620AbgE1PwL (ORCPT ); Thu, 28 May 2020 11:52:11 -0400 Received: from out03.mta.xmission.com ([166.70.13.233]:49102 "EHLO out03.mta.xmission.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2404518AbgE1PwK (ORCPT ); Thu, 28 May 2020 11:52:10 -0400 Received: from in01.mta.xmission.com ([166.70.13.51]) by out03.mta.xmission.com with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1jeKpA-0004Mo-Cr; Thu, 28 May 2020 09:52:04 -0600 Received: from ip68-227-160-95.om.om.cox.net ([68.227.160.95] helo=x220.xmission.com) by in01.mta.xmission.com with esmtpsa (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.87) (envelope-from ) id 1jeKp9-0007rJ-31; Thu, 28 May 2020 09:52:04 -0600 From: ebiederm@xmission.com (Eric W. Biederman) To: Cc: Linus Torvalds , Oleg Nesterov , Jann Horn , Kees Cook , Greg Ungerer , Rob Landley , Bernd Edlinger , , Al Viro , Alexey Dobriyan , Andrew Morton , Casey Schaufler , linux-security-module@vger.kernel.org, James Morris , "Serge E. Hallyn" , Andy Lutomirski References: <87h7wujhmz.fsf@x220.int.ebiederm.org> <87sgga6ze4.fsf@x220.int.ebiederm.org> <87v9l4zyla.fsf_-_@x220.int.ebiederm.org> <877dx822er.fsf_-_@x220.int.ebiederm.org> <87k10wysqz.fsf_-_@x220.int.ebiederm.org> Date: Thu, 28 May 2020 10:48:11 -0500 In-Reply-To: <87k10wysqz.fsf_-_@x220.int.ebiederm.org> (Eric W. Biederman's message of "Thu, 28 May 2020 10:38:28 -0500") Message-ID: <87ftbkxdqc.fsf_-_@x220.int.ebiederm.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) MIME-Version: 1.0 X-XM-SPF: eid=1jeKp9-0007rJ-31;;;mid=<87ftbkxdqc.fsf_-_@x220.int.ebiederm.org>;;;hst=in01.mta.xmission.com;;;ip=68.227.160.95;;;frm=ebiederm@xmission.com;;;spf=neutral X-XM-AID: U2FsdGVkX19acW7gYINIOd1UYM//vzVBWHHd36D56kw= X-SA-Exim-Connect-IP: 68.227.160.95 X-SA-Exim-Mail-From: ebiederm@xmission.com X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on sa01.xmission.com X-Spam-Level: **** X-Spam-Status: No, score=4.3 required=8.0 tests=ALL_TRUSTED,BAYES_50, DCC_CHECK_NEGATIVE,T_TM2_M_HEADER_IN_MSG,T_TooManySym_01,XMNoVowels, XMSubLong,XMSubMetaSxObfu_03,XMSubMetaSx_00 autolearn=disabled version=3.4.2 X-Spam-Virus: No X-Spam-Report: * -1.0 ALL_TRUSTED Passed through trusted hosts only via SMTP * 0.8 BAYES_50 BODY: Bayes spam probability is 40 to 60% * [score: 0.5000] * 1.5 XMNoVowels Alpha-numberic number with no vowels * 0.7 XMSubLong Long Subject * 0.0 T_TM2_M_HEADER_IN_MSG BODY: No description available. * -0.0 DCC_CHECK_NEGATIVE Not listed in DCC * [sa01 0; Body=1 Fuz1=1 Fuz2=1] * 1.0 XMSubMetaSx_00 1+ Sexy Words * 1.2 XMSubMetaSxObfu_03 Obfuscated Sexy Noun-People * 0.0 T_TooManySym_01 4+ unique symbols in subject X-Spam-DCC: ; sa01 0; Body=1 Fuz1=1 Fuz2=1 X-Spam-Combo: ****; X-Spam-Relay-Country: X-Spam-Timing: total 479 ms - load_scoreonly_sql: 0.04 (0.0%), signal_user_changed: 3.8 (0.8%), b_tie_ro: 2.6 (0.5%), parse: 0.77 (0.2%), extract_message_metadata: 9 (1.9%), get_uri_detail_list: 1.42 (0.3%), tests_pri_-1000: 11 (2.3%), tests_pri_-950: 1.00 (0.2%), tests_pri_-900: 0.81 (0.2%), tests_pri_-90: 119 (24.8%), check_bayes: 118 (24.6%), b_tokenize: 8 (1.6%), b_tok_get_all: 7 (1.5%), b_comp_prob: 1.67 (0.3%), b_tok_touch_all: 98 (20.5%), b_finish: 0.67 (0.1%), tests_pri_0: 323 (67.5%), check_dkim_signature: 0.41 (0.1%), check_dkim_adsp: 1.95 (0.4%), poll_dns_idle: 0.60 (0.1%), tests_pri_10: 1.72 (0.4%), tests_pri_500: 6 (1.2%), rewrite_mail: 0.00 (0.0%) Subject: [PATCH 06/11] exec: Don't set secureexec when the uid or gid changes are abandoned X-Spam-Flag: No X-SA-Exim-Version: 4.2.1 (built Thu, 05 May 2016 13:38:54 -0600) X-SA-Exim-Scanned: Yes (on in01.mta.xmission.com) Sender: owner-linux-security-module@vger.kernel.org Precedence: bulk List-ID: When the is_secureexec test was removed from cap_bprm_set_creds the test was modified so that it based the status of secureexec on a version of the euid and egid before ptrace and shared fs tests possibly reverted them. The effect of which is that secureexec continued to be set when the euid and egid change were abandoned because the executable was being ptraced to secureexec being set in that same situation. As far as I can tell it is just an oversight and very poor quality of implementation to set AT_SECURE when it is not ncessary. So improve the quality of the implementation by only setting secureexec when there will be multiple uids or gids in the final cred structure. Fixes: ee67ae7ef6ff ("commoncap: Move cap_elevated calculation into bprm_set_creds") Signed-off-by: "Eric W. Biederman" --- fs/exec.c | 48 +++++++++++++++++++++--------------------------- 1 file changed, 21 insertions(+), 27 deletions(-) diff --git a/fs/exec.c b/fs/exec.c index bac8db14f30d..123402f218fe 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -1622,44 +1622,38 @@ static void bprm_fill_uid(struct linux_binprm *bprm) !kgid_has_mapping(new->user_ns, gid)) goto after_setid; + /* + * Is the root directory and working directory shared or is + * the process traced and the tracing process does not have + * CAP_SYS_PTRACE? + * + * In either case it is not safe to change the euid or egid + * unless the current process has the appropriate cap and so + * chaning the euid or egid was already possible. + */ + need_cap = bprm->unsafe & LSM_UNSAFE_SHARE || + !ptracer_capable(current, new->user_ns); + if (mode & S_ISUID) { bprm->per_clear = 1; - new->euid = uid; + if (!need_cap || + (ns_capable(new->user_ns, CAP_SETUID) && + !(bprm->unsafe & LSM_UNSAFE_NO_NEW_PRIVS))) + new->euid = uid; } - if ((mode & (S_ISGID | S_IXGRP)) == (S_ISGID | S_IXGRP)) { bprm->per_clear = 1; - new->egid = gid; + if (!need_cap || + (ns_capable(new->user_ns, CAP_SETGID) && + !(bprm->unsafe & LSM_UNSAFE_NO_NEW_PRIVS))) + new->egid = gid; } after_setid: /* Will the new creds have multiple uids or gids? */ - if (!uid_eq(new->euid, new->uid) || !gid_eq(new->egid, new->gid)) { + if (!uid_eq(new->euid, new->uid) || !gid_eq(new->egid, new->gid)) bprm->secureexec = 1; - /* - * Is the root directory and working directory shared or is - * the process traced and the tracing process does not have - * CAP_SYS_PTRACE? - * - * In either case it is not safe to change the euid or egid - * unless the current process has the appropriate cap and so - * chaning the euid or egid was already possible. - */ - need_cap = bprm->unsafe & LSM_UNSAFE_SHARE || - !ptracer_capable(current, new->user_ns); - if (need_cap && !uid_eq(new->euid, new->uid) && - (!ns_capable(new->user_ns, CAP_SETUID) || - (bprm->unsafe & LSM_UNSAFE_NO_NEW_PRIVS))) { - new->euid = new->uid; - } - if (need_cap && !gid_eq(new->egid, new->gid) && - (!ns_capable(new->user_ns, CAP_SETGID) || - (bprm->unsafe & LSM_UNSAFE_NO_NEW_PRIVS))) { - new->egid = new->gid; - } - } - new->suid = new->fsuid = new->euid; new->sgid = new->fsgid = new->egid; } From patchwork Thu May 28 15:48:44 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Eric W. Biederman" X-Patchwork-Id: 11576239 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 4350C1391 for ; Thu, 28 May 2020 15:52:39 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 347D7207D3 for ; Thu, 28 May 2020 15:52:39 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2404603AbgE1Pwi (ORCPT ); Thu, 28 May 2020 11:52:38 -0400 Received: from out02.mta.xmission.com ([166.70.13.232]:52566 "EHLO out02.mta.xmission.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2404565AbgE1Pwi (ORCPT ); Thu, 28 May 2020 11:52:38 -0400 Received: from in02.mta.xmission.com ([166.70.13.52]) by out02.mta.xmission.com with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1jeKpg-0005EG-KT; Thu, 28 May 2020 09:52:36 -0600 Received: from ip68-227-160-95.om.om.cox.net ([68.227.160.95] helo=x220.xmission.com) by in02.mta.xmission.com with esmtpsa (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.87) (envelope-from ) id 1jeKpf-00033R-Pz; Thu, 28 May 2020 09:52:36 -0600 From: ebiederm@xmission.com (Eric W. Biederman) To: Cc: Linus Torvalds , Oleg Nesterov , Jann Horn , Kees Cook , Greg Ungerer , Rob Landley , Bernd Edlinger , , Al Viro , Alexey Dobriyan , Andrew Morton , Casey Schaufler , linux-security-module@vger.kernel.org, James Morris , "Serge E. Hallyn" , Andy Lutomirski References: <87h7wujhmz.fsf@x220.int.ebiederm.org> <87sgga6ze4.fsf@x220.int.ebiederm.org> <87v9l4zyla.fsf_-_@x220.int.ebiederm.org> <877dx822er.fsf_-_@x220.int.ebiederm.org> <87k10wysqz.fsf_-_@x220.int.ebiederm.org> Date: Thu, 28 May 2020 10:48:44 -0500 In-Reply-To: <87k10wysqz.fsf_-_@x220.int.ebiederm.org> (Eric W. Biederman's message of "Thu, 28 May 2020 10:38:28 -0500") Message-ID: <87a71sxdpf.fsf_-_@x220.int.ebiederm.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) MIME-Version: 1.0 X-XM-SPF: eid=1jeKpf-00033R-Pz;;;mid=<87a71sxdpf.fsf_-_@x220.int.ebiederm.org>;;;hst=in02.mta.xmission.com;;;ip=68.227.160.95;;;frm=ebiederm@xmission.com;;;spf=neutral X-XM-AID: U2FsdGVkX19L2GQX5s9eCjNWe4gHmrLWVMHrvsxnIhk= X-SA-Exim-Connect-IP: 68.227.160.95 X-SA-Exim-Mail-From: ebiederm@xmission.com X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on sa04.xmission.com X-Spam-Level: ** X-Spam-Status: No, score=2.0 required=8.0 tests=ALL_TRUSTED,BAYES_50, DCC_CHECK_NEGATIVE,T_TooManySym_01,T_TooManySym_02,XMNoVowels, XMSubLong autolearn=disabled version=3.4.2 X-Spam-Report: * -1.0 ALL_TRUSTED Passed through trusted hosts only via SMTP * 0.8 BAYES_50 BODY: Bayes spam probability is 40 to 60% * [score: 0.5000] * 0.7 XMSubLong Long Subject * 1.5 XMNoVowels Alpha-numberic number with no vowels * -0.0 DCC_CHECK_NEGATIVE Not listed in DCC * [sa04 0; Body=1 Fuz1=1 Fuz2=1] * 0.0 T_TooManySym_02 5+ unique symbols in subject * 0.0 T_TooManySym_01 4+ unique symbols in subject X-Spam-DCC: ; sa04 0; Body=1 Fuz1=1 Fuz2=1 X-Spam-Combo: **; X-Spam-Relay-Country: X-Spam-Timing: total 387 ms - load_scoreonly_sql: 0.07 (0.0%), signal_user_changed: 14 (3.5%), b_tie_ro: 12 (3.0%), parse: 1.36 (0.4%), extract_message_metadata: 16 (4.1%), get_uri_detail_list: 1.55 (0.4%), tests_pri_-1000: 19 (4.8%), tests_pri_-950: 1.59 (0.4%), tests_pri_-900: 1.32 (0.3%), tests_pri_-90: 120 (31.0%), check_bayes: 118 (30.6%), b_tokenize: 9 (2.3%), b_tok_get_all: 7 (1.7%), b_comp_prob: 2.2 (0.6%), b_tok_touch_all: 97 (25.1%), b_finish: 0.87 (0.2%), tests_pri_0: 200 (51.8%), check_dkim_signature: 0.52 (0.1%), check_dkim_adsp: 2.2 (0.6%), poll_dns_idle: 0.55 (0.1%), tests_pri_10: 2.1 (0.5%), tests_pri_500: 8 (2.0%), rewrite_mail: 0.00 (0.0%) Subject: [PATCH 07/11] exec: Set saved, fs, and effective ids together in bprm_fill_uid X-Spam-Flag: No X-SA-Exim-Version: 4.2.1 (built Thu, 05 May 2016 13:38:54 -0600) X-SA-Exim-Scanned: Yes (on in02.mta.xmission.com) Sender: owner-linux-security-module@vger.kernel.org Precedence: bulk List-ID: Now that there is only one place in bprm_fill_uid where the euid and the egid are set, move setting of the saved, and the fs ids to that place. This makes it clear that this is the only location in the function that changes these ids. Signed-off-by: "Eric W. Biederman" --- fs/exec.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/fs/exec.c b/fs/exec.c index 123402f218fe..8dd7254931dc 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -1639,23 +1639,20 @@ static void bprm_fill_uid(struct linux_binprm *bprm) if (!need_cap || (ns_capable(new->user_ns, CAP_SETUID) && !(bprm->unsafe & LSM_UNSAFE_NO_NEW_PRIVS))) - new->euid = uid; + new->suid = new->fsuid = new->euid = uid; } if ((mode & (S_ISGID | S_IXGRP)) == (S_ISGID | S_IXGRP)) { bprm->per_clear = 1; if (!need_cap || (ns_capable(new->user_ns, CAP_SETGID) && !(bprm->unsafe & LSM_UNSAFE_NO_NEW_PRIVS))) - new->egid = gid; + new->sgid = new->fsgid = new->egid = gid; } after_setid: /* Will the new creds have multiple uids or gids? */ if (!uid_eq(new->euid, new->uid) || !gid_eq(new->egid, new->gid)) bprm->secureexec = 1; - - new->suid = new->fsuid = new->euid; - new->sgid = new->fsgid = new->egid; } /* From patchwork Thu May 28 15:49:10 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Eric W. Biederman" X-Patchwork-Id: 11576243 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 24A101391 for ; Thu, 28 May 2020 15:53:12 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 167B62071A for ; Thu, 28 May 2020 15:53:12 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2404658AbgE1PxL (ORCPT ); Thu, 28 May 2020 11:53:11 -0400 Received: from out01.mta.xmission.com ([166.70.13.231]:33262 "EHLO out01.mta.xmission.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2404565AbgE1PxJ (ORCPT ); Thu, 28 May 2020 11:53:09 -0400 Received: from in01.mta.xmission.com ([166.70.13.51]) by out01.mta.xmission.com with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1jeKqC-0001W7-C9; Thu, 28 May 2020 09:53:08 -0600 Received: from ip68-227-160-95.om.om.cox.net ([68.227.160.95] helo=x220.xmission.com) by in01.mta.xmission.com with esmtpsa (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.87) (envelope-from ) id 1jeKq6-0007zz-5k; Thu, 28 May 2020 09:53:07 -0600 From: ebiederm@xmission.com (Eric W. Biederman) To: Cc: Linus Torvalds , Oleg Nesterov , Jann Horn , Kees Cook , Greg Ungerer , Rob Landley , Bernd Edlinger , , Al Viro , Alexey Dobriyan , Andrew Morton , Casey Schaufler , linux-security-module@vger.kernel.org, James Morris , "Serge E. Hallyn" , Andy Lutomirski References: <87h7wujhmz.fsf@x220.int.ebiederm.org> <87sgga6ze4.fsf@x220.int.ebiederm.org> <87v9l4zyla.fsf_-_@x220.int.ebiederm.org> <877dx822er.fsf_-_@x220.int.ebiederm.org> <87k10wysqz.fsf_-_@x220.int.ebiederm.org> Date: Thu, 28 May 2020 10:49:10 -0500 In-Reply-To: <87k10wysqz.fsf_-_@x220.int.ebiederm.org> (Eric W. Biederman's message of "Thu, 28 May 2020 10:38:28 -0500") Message-ID: <874ks0xdop.fsf_-_@x220.int.ebiederm.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) MIME-Version: 1.0 X-XM-SPF: eid=1jeKq6-0007zz-5k;;;mid=<874ks0xdop.fsf_-_@x220.int.ebiederm.org>;;;hst=in01.mta.xmission.com;;;ip=68.227.160.95;;;frm=ebiederm@xmission.com;;;spf=neutral X-XM-AID: U2FsdGVkX1/ELoUISo4voXjVc4cOnoi5+XqInd2HxUI= X-SA-Exim-Connect-IP: 68.227.160.95 X-SA-Exim-Mail-From: ebiederm@xmission.com X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on sa01.xmission.com X-Spam-Level: ** X-Spam-Status: No, score=2.0 required=8.0 tests=ALL_TRUSTED,BAYES_50, DCC_CHECK_NEGATIVE,T_TooManySym_01,XMNoVowels,XMSubLong autolearn=disabled version=3.4.2 X-Spam-Virus: No X-Spam-Report: * -1.0 ALL_TRUSTED Passed through trusted hosts only via SMTP * 0.8 BAYES_50 BODY: Bayes spam probability is 40 to 60% * [score: 0.5000] * 1.5 XMNoVowels Alpha-numberic number with no vowels * 0.7 XMSubLong Long Subject * -0.0 DCC_CHECK_NEGATIVE Not listed in DCC * [sa01 0; Body=1 Fuz1=1 Fuz2=1] * 0.0 T_TooManySym_01 4+ unique symbols in subject X-Spam-DCC: ; sa01 0; Body=1 Fuz1=1 Fuz2=1 X-Spam-Combo: **; X-Spam-Relay-Country: X-Spam-Timing: total 515 ms - load_scoreonly_sql: 0.05 (0.0%), signal_user_changed: 3.5 (0.7%), b_tie_ro: 2.3 (0.4%), parse: 1.20 (0.2%), extract_message_metadata: 15 (2.9%), get_uri_detail_list: 1.48 (0.3%), tests_pri_-1000: 17 (3.3%), tests_pri_-950: 1.47 (0.3%), tests_pri_-900: 1.21 (0.2%), tests_pri_-90: 134 (26.0%), check_bayes: 132 (25.7%), b_tokenize: 10 (1.8%), b_tok_get_all: 7 (1.3%), b_comp_prob: 2.2 (0.4%), b_tok_touch_all: 111 (21.5%), b_finish: 0.71 (0.1%), tests_pri_0: 328 (63.6%), check_dkim_signature: 0.40 (0.1%), check_dkim_adsp: 2.5 (0.5%), poll_dns_idle: 0.81 (0.2%), tests_pri_10: 2.6 (0.5%), tests_pri_500: 8 (1.5%), rewrite_mail: 0.00 (0.0%) Subject: [PATCH 08/11] exec: In bprm_fill_uid remove unnecessary no new privs check X-Spam-Flag: No X-SA-Exim-Version: 4.2.1 (built Thu, 05 May 2016 13:38:54 -0600) X-SA-Exim-Scanned: Yes (on in01.mta.xmission.com) Sender: owner-linux-security-module@vger.kernel.org Precedence: bulk List-ID: When the no new privs code was added[1], a test was added to cap_bprm_set_creds to ensure that the credential change were always reverted if no new privs was set. That test has been refactored into a test to not make the credential change in bprm_fill_uid when no new privs is set. Remove that unncessary test as it can now been seen by a quick inspection that execution can never make it to the test with no new privs set. The same change[1] also added a test that guaranteed the credentials would never change when no_new_privs was set, so the test I am removing was never necessary but historically that was far from obvious. [1]: 259e5e6c75a9 ("Add PR_{GET,SET}_NO_NEW_PRIVS to prevent execve from granting privs") Signed-off-by: "Eric W. Biederman" --- fs/exec.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/fs/exec.c b/fs/exec.c index 8dd7254931dc..af108ecf9632 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -1636,16 +1636,12 @@ static void bprm_fill_uid(struct linux_binprm *bprm) if (mode & S_ISUID) { bprm->per_clear = 1; - if (!need_cap || - (ns_capable(new->user_ns, CAP_SETUID) && - !(bprm->unsafe & LSM_UNSAFE_NO_NEW_PRIVS))) + if (!need_cap || ns_capable(new->user_ns, CAP_SETUID)) new->suid = new->fsuid = new->euid = uid; } if ((mode & (S_ISGID | S_IXGRP)) == (S_ISGID | S_IXGRP)) { bprm->per_clear = 1; - if (!need_cap || - (ns_capable(new->user_ns, CAP_SETGID) && - !(bprm->unsafe & LSM_UNSAFE_NO_NEW_PRIVS))) + if (!need_cap || ns_capable(new->user_ns, CAP_SETGID)) new->sgid = new->fsgid = new->egid = gid; } From patchwork Thu May 28 15:49:44 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Eric W. Biederman" X-Patchwork-Id: 11576249 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id E59D71391 for ; Thu, 28 May 2020 15:53:46 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id D72122071A for ; Thu, 28 May 2020 15:53:46 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2404687AbgE1Pxj (ORCPT ); Thu, 28 May 2020 11:53:39 -0400 Received: from out01.mta.xmission.com ([166.70.13.231]:33472 "EHLO out01.mta.xmission.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2404511AbgE1Pxh (ORCPT ); Thu, 28 May 2020 11:53:37 -0400 Received: from in01.mta.xmission.com ([166.70.13.51]) by out01.mta.xmission.com with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1jeKqe-0001Zi-I4; Thu, 28 May 2020 09:53:36 -0600 Received: from ip68-227-160-95.om.om.cox.net ([68.227.160.95] helo=x220.xmission.com) by in01.mta.xmission.com with esmtpsa (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.87) (envelope-from ) id 1jeKqd-00085a-M7; Thu, 28 May 2020 09:53:36 -0600 From: ebiederm@xmission.com (Eric W. Biederman) To: Cc: Linus Torvalds , Oleg Nesterov , Jann Horn , Kees Cook , Greg Ungerer , Rob Landley , Bernd Edlinger , , Al Viro , Alexey Dobriyan , Andrew Morton , Casey Schaufler , linux-security-module@vger.kernel.org, James Morris , "Serge E. Hallyn" , Andy Lutomirski References: <87h7wujhmz.fsf@x220.int.ebiederm.org> <87sgga6ze4.fsf@x220.int.ebiederm.org> <87v9l4zyla.fsf_-_@x220.int.ebiederm.org> <877dx822er.fsf_-_@x220.int.ebiederm.org> <87k10wysqz.fsf_-_@x220.int.ebiederm.org> Date: Thu, 28 May 2020 10:49:44 -0500 In-Reply-To: <87k10wysqz.fsf_-_@x220.int.ebiederm.org> (Eric W. Biederman's message of "Thu, 28 May 2020 10:38:28 -0500") Message-ID: <87y2pcvz3b.fsf_-_@x220.int.ebiederm.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) MIME-Version: 1.0 X-XM-SPF: eid=1jeKqd-00085a-M7;;;mid=<87y2pcvz3b.fsf_-_@x220.int.ebiederm.org>;;;hst=in01.mta.xmission.com;;;ip=68.227.160.95;;;frm=ebiederm@xmission.com;;;spf=neutral X-XM-AID: U2FsdGVkX1+Uao3hKNmbCnTKyFSqtDONOkhIC5W+4JQ= X-SA-Exim-Connect-IP: 68.227.160.95 X-SA-Exim-Mail-From: ebiederm@xmission.com X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on sa06.xmission.com X-Spam-Level: **** X-Spam-Status: No, score=4.3 required=8.0 tests=ALL_TRUSTED,BAYES_50, DCC_CHECK_NEGATIVE,T_TooManySym_01,XMNoVowels,XMSubLong, XMSubMetaSxObfu_03,XMSubMetaSx_00 autolearn=disabled version=3.4.2 X-Spam-Report: * -1.0 ALL_TRUSTED Passed through trusted hosts only via SMTP * 0.8 BAYES_50 BODY: Bayes spam probability is 40 to 60% * [score: 0.5000] * 1.5 XMNoVowels Alpha-numberic number with no vowels * 0.7 XMSubLong Long Subject * -0.0 DCC_CHECK_NEGATIVE Not listed in DCC * [sa06 0; Body=1 Fuz1=1 Fuz2=1] * 1.0 XMSubMetaSx_00 1+ Sexy Words * 0.0 T_TooManySym_01 4+ unique symbols in subject * 1.2 XMSubMetaSxObfu_03 Obfuscated Sexy Noun-People X-Spam-DCC: ; sa06 0; Body=1 Fuz1=1 Fuz2=1 X-Spam-Combo: ****; X-Spam-Relay-Country: X-Spam-Timing: total 385 ms - load_scoreonly_sql: 0.04 (0.0%), signal_user_changed: 10 (2.6%), b_tie_ro: 9 (2.3%), parse: 1.04 (0.3%), extract_message_metadata: 11 (2.8%), get_uri_detail_list: 1.44 (0.4%), tests_pri_-1000: 13 (3.4%), tests_pri_-950: 1.24 (0.3%), tests_pri_-900: 1.00 (0.3%), tests_pri_-90: 84 (21.9%), check_bayes: 83 (21.5%), b_tokenize: 8 (2.1%), b_tok_get_all: 8 (2.1%), b_comp_prob: 2.7 (0.7%), b_tok_touch_all: 60 (15.6%), b_finish: 0.94 (0.2%), tests_pri_0: 246 (64.0%), check_dkim_signature: 0.55 (0.1%), check_dkim_adsp: 2.5 (0.6%), poll_dns_idle: 0.48 (0.1%), tests_pri_10: 2.3 (0.6%), tests_pri_500: 12 (3.0%), rewrite_mail: 0.00 (0.0%) Subject: [PATCH 09/11] exec: In bprm_fill_uid only set per_clear when honoring suid or sgid X-Spam-Flag: No X-SA-Exim-Version: 4.2.1 (built Thu, 05 May 2016 13:38:54 -0600) X-SA-Exim-Scanned: Yes (on in01.mta.xmission.com) Sender: owner-linux-security-module@vger.kernel.org Precedence: bulk List-ID: It makes no sense to set active_per_clear when the kernel decides not to honor the executables setuid or or setgid bits. Instead set active_per_clear when the kernel actually decides to honor the suid or sgid permission bits of an executable. As far as I can tell this was the intended behavior but with the ptrace logic hiding out in security/commcap.c:cap_bprm_apply_creds I believe it was just overlooked that the setuid or setgid operation could be cancelled. History Tree: git://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git Fixes: 1bb0fa189c6a ("[PATCH] NX: clean up legacy binary support") Signed-off-by: "Eric W. Biederman" --- fs/exec.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/fs/exec.c b/fs/exec.c index af108ecf9632..347dade4bc54 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -1634,15 +1634,16 @@ static void bprm_fill_uid(struct linux_binprm *bprm) need_cap = bprm->unsafe & LSM_UNSAFE_SHARE || !ptracer_capable(current, new->user_ns); - if (mode & S_ISUID) { + if ((mode & S_ISUID) && + (!need_cap || ns_capable(new->user_ns, CAP_SETUID))) { bprm->per_clear = 1; - if (!need_cap || ns_capable(new->user_ns, CAP_SETUID)) - new->suid = new->fsuid = new->euid = uid; + new->suid = new->fsuid = new->euid = uid; } - if ((mode & (S_ISGID | S_IXGRP)) == (S_ISGID | S_IXGRP)) { + + if (((mode & (S_ISGID | S_IXGRP)) == (S_ISGID | S_IXGRP)) && + (!need_cap || ns_capable(new->user_ns, CAP_SETGID))) { bprm->per_clear = 1; - if (!need_cap || ns_capable(new->user_ns, CAP_SETGID)) - new->sgid = new->fsgid = new->egid = gid; + new->sgid = new->fsgid = new->egid = gid; } after_setid: From patchwork Thu May 28 15:50:09 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Eric W. Biederman" X-Patchwork-Id: 11576251 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 4CCB813B4 for ; Thu, 28 May 2020 15:54:09 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 3EFDE2071A for ; Thu, 28 May 2020 15:54:09 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2404743AbgE1PyI (ORCPT ); Thu, 28 May 2020 11:54:08 -0400 Received: from out02.mta.xmission.com ([166.70.13.232]:53032 "EHLO out02.mta.xmission.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2404511AbgE1PyD (ORCPT ); Thu, 28 May 2020 11:54:03 -0400 Received: from in02.mta.xmission.com ([166.70.13.52]) by out02.mta.xmission.com with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1jeKr3-0005Lz-CC; Thu, 28 May 2020 09:54:01 -0600 Received: from ip68-227-160-95.om.om.cox.net ([68.227.160.95] helo=x220.xmission.com) by in02.mta.xmission.com with esmtpsa (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.87) (envelope-from ) id 1jeKr2-0003Dg-EH; Thu, 28 May 2020 09:54:01 -0600 From: ebiederm@xmission.com (Eric W. Biederman) To: Cc: Linus Torvalds , Oleg Nesterov , Jann Horn , Kees Cook , Greg Ungerer , Rob Landley , Bernd Edlinger , , Al Viro , Alexey Dobriyan , Andrew Morton , Casey Schaufler , linux-security-module@vger.kernel.org, James Morris , "Serge E. Hallyn" , Andy Lutomirski References: <87h7wujhmz.fsf@x220.int.ebiederm.org> <87sgga6ze4.fsf@x220.int.ebiederm.org> <87v9l4zyla.fsf_-_@x220.int.ebiederm.org> <877dx822er.fsf_-_@x220.int.ebiederm.org> <87k10wysqz.fsf_-_@x220.int.ebiederm.org> Date: Thu, 28 May 2020 10:50:09 -0500 In-Reply-To: <87k10wysqz.fsf_-_@x220.int.ebiederm.org> (Eric W. Biederman's message of "Thu, 28 May 2020 10:38:28 -0500") Message-ID: <87sgfkvz2m.fsf_-_@x220.int.ebiederm.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) MIME-Version: 1.0 X-XM-SPF: eid=1jeKr2-0003Dg-EH;;;mid=<87sgfkvz2m.fsf_-_@x220.int.ebiederm.org>;;;hst=in02.mta.xmission.com;;;ip=68.227.160.95;;;frm=ebiederm@xmission.com;;;spf=neutral X-XM-AID: U2FsdGVkX1+noWZXFqvq+oA4X8eGsRDwP21udiMjpIw= X-SA-Exim-Connect-IP: 68.227.160.95 X-SA-Exim-Mail-From: ebiederm@xmission.com X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on sa02.xmission.com X-Spam-Level: ** X-Spam-Status: No, score=2.0 required=8.0 tests=ALL_TRUSTED,BAYES_50, DCC_CHECK_NEGATIVE,T_TooManySym_01,XMNoVowels,XMSubLong autolearn=disabled version=3.4.2 X-Spam-Virus: No X-Spam-Report: * -1.0 ALL_TRUSTED Passed through trusted hosts only via SMTP * 0.8 BAYES_50 BODY: Bayes spam probability is 40 to 60% * [score: 0.5000] * 1.5 XMNoVowels Alpha-numberic number with no vowels * 0.7 XMSubLong Long Subject * -0.0 DCC_CHECK_NEGATIVE Not listed in DCC * [sa02 0; Body=1 Fuz1=1 Fuz2=1] * 0.0 T_TooManySym_01 4+ unique symbols in subject X-Spam-DCC: ; sa02 0; Body=1 Fuz1=1 Fuz2=1 X-Spam-Combo: **; X-Spam-Relay-Country: X-Spam-Timing: total 530 ms - load_scoreonly_sql: 0.03 (0.0%), signal_user_changed: 4.4 (0.8%), b_tie_ro: 3.0 (0.6%), parse: 1.09 (0.2%), extract_message_metadata: 12 (2.2%), get_uri_detail_list: 1.92 (0.4%), tests_pri_-1000: 11 (2.1%), tests_pri_-950: 1.14 (0.2%), tests_pri_-900: 0.85 (0.2%), tests_pri_-90: 229 (43.1%), check_bayes: 227 (42.8%), b_tokenize: 7 (1.3%), b_tok_get_all: 7 (1.2%), b_comp_prob: 1.73 (0.3%), b_tok_touch_all: 209 (39.4%), b_finish: 0.83 (0.2%), tests_pri_0: 262 (49.3%), check_dkim_signature: 0.59 (0.1%), check_dkim_adsp: 2.1 (0.4%), poll_dns_idle: 0.80 (0.2%), tests_pri_10: 1.80 (0.3%), tests_pri_500: 6 (1.0%), rewrite_mail: 0.00 (0.0%) Subject: [PATCH 10/11] exec: In bprm_fill_uid set secureexec at same time as per_clear X-Spam-Flag: No X-SA-Exim-Version: 4.2.1 (built Thu, 05 May 2016 13:38:54 -0600) X-SA-Exim-Scanned: Yes (on in02.mta.xmission.com) Sender: owner-linux-security-module@vger.kernel.org Precedence: bulk List-ID: We currently have two different policies for setting per_clear and for setting secureexec. For per_clear the policy is if the setxid bits on a file are honored. For secureexec the policy is if the resulting credentials will have multiple uids or gids. Looking closely the policy for setting AT_SECURE and asking userspace not to trust our caller in all cases where we have multiple uids or gids does not make sense. In some of those cases it is the caller of exec that provides multiple uids and gids. The point of setting AT_SECURE is so that the called application or it's libraries can take defensive measures to guard against a lesser privileged program which calls it via exec. If all of your privilege comes from your caller there is no point in taking defensive measures, against them. Further the only way that libc or other userspace can know that the privilege came from the caller of exec and not from the exec being suid or sgid is by the kernel telling it. As userspace does not have enough information to distinguish between these two cases. So set secureexec when the exec itself results in multiple uids or gids, not when we happen to have mulitple ids because the binary was called that way. Signed-off-by: "Eric W. Biederman" --- fs/exec.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/fs/exec.c b/fs/exec.c index 347dade4bc54..fc4edc7517a6 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -1637,19 +1637,19 @@ static void bprm_fill_uid(struct linux_binprm *bprm) if ((mode & S_ISUID) && (!need_cap || ns_capable(new->user_ns, CAP_SETUID))) { bprm->per_clear = 1; + bprm->secureexec = 1; new->suid = new->fsuid = new->euid = uid; } if (((mode & (S_ISGID | S_IXGRP)) == (S_ISGID | S_IXGRP)) && (!need_cap || ns_capable(new->user_ns, CAP_SETGID))) { bprm->per_clear = 1; + bprm->secureexec = 1; new->sgid = new->fsgid = new->egid = gid; } after_setid: - /* Will the new creds have multiple uids or gids? */ - if (!uid_eq(new->euid, new->uid) || !gid_eq(new->egid, new->gid)) - bprm->secureexec = 1; + ; } /* From patchwork Thu May 28 15:50:36 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Eric W. Biederman" X-Patchwork-Id: 11576255 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id B04BD13B4 for ; Thu, 28 May 2020 15:54:36 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id A2A002071A for ; Thu, 28 May 2020 15:54:36 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2404778AbgE1Pye (ORCPT ); Thu, 28 May 2020 11:54:34 -0400 Received: from out03.mta.xmission.com ([166.70.13.233]:49760 "EHLO out03.mta.xmission.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2404774AbgE1Pyc (ORCPT ); Thu, 28 May 2020 11:54:32 -0400 Received: from in01.mta.xmission.com ([166.70.13.51]) by out03.mta.xmission.com with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1jeKrU-0004Y0-Lc; Thu, 28 May 2020 09:54:29 -0600 Received: from ip68-227-160-95.om.om.cox.net ([68.227.160.95] helo=x220.xmission.com) by in01.mta.xmission.com with esmtpsa (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.87) (envelope-from ) id 1jeKrT-0008Bp-Qq; Thu, 28 May 2020 09:54:28 -0600 From: ebiederm@xmission.com (Eric W. Biederman) To: Cc: Linus Torvalds , Oleg Nesterov , Jann Horn , Kees Cook , Greg Ungerer , Rob Landley , Bernd Edlinger , , Al Viro , Alexey Dobriyan , Andrew Morton , Casey Schaufler , linux-security-module@vger.kernel.org, James Morris , "Serge E. Hallyn" , Andy Lutomirski References: <87h7wujhmz.fsf@x220.int.ebiederm.org> <87sgga6ze4.fsf@x220.int.ebiederm.org> <87v9l4zyla.fsf_-_@x220.int.ebiederm.org> <877dx822er.fsf_-_@x220.int.ebiederm.org> <87k10wysqz.fsf_-_@x220.int.ebiederm.org> Date: Thu, 28 May 2020 10:50:36 -0500 In-Reply-To: <87k10wysqz.fsf_-_@x220.int.ebiederm.org> (Eric W. Biederman's message of "Thu, 28 May 2020 10:38:28 -0500") Message-ID: <87mu5svz1v.fsf_-_@x220.int.ebiederm.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) MIME-Version: 1.0 X-XM-SPF: eid=1jeKrT-0008Bp-Qq;;;mid=<87mu5svz1v.fsf_-_@x220.int.ebiederm.org>;;;hst=in01.mta.xmission.com;;;ip=68.227.160.95;;;frm=ebiederm@xmission.com;;;spf=neutral X-XM-AID: U2FsdGVkX1+MHtH6hLkB3oWNuRodn+5tMsim9ktuu6s= X-SA-Exim-Connect-IP: 68.227.160.95 X-SA-Exim-Mail-From: ebiederm@xmission.com X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on sa06.xmission.com X-Spam-Level: ** X-Spam-Status: No, score=2.0 required=8.0 tests=ALL_TRUSTED,BAYES_50, DCC_CHECK_NEGATIVE,T_TooManySym_01,XMNoVowels,XMSubLong autolearn=disabled version=3.4.2 X-Spam-Report: * -1.0 ALL_TRUSTED Passed through trusted hosts only via SMTP * 0.8 BAYES_50 BODY: Bayes spam probability is 40 to 60% * [score: 0.5000] * 1.5 XMNoVowels Alpha-numberic number with no vowels * 0.7 XMSubLong Long Subject * -0.0 DCC_CHECK_NEGATIVE Not listed in DCC * [sa06 0; Body=1 Fuz1=1 Fuz2=1] * 0.0 T_TooManySym_01 4+ unique symbols in subject X-Spam-DCC: ; sa06 0; Body=1 Fuz1=1 Fuz2=1 X-Spam-Combo: **; X-Spam-Relay-Country: X-Spam-Timing: total 408 ms - load_scoreonly_sql: 0.41 (0.1%), signal_user_changed: 14 (3.3%), b_tie_ro: 11 (2.7%), parse: 1.64 (0.4%), extract_message_metadata: 15 (3.6%), get_uri_detail_list: 1.60 (0.4%), tests_pri_-1000: 13 (3.2%), tests_pri_-950: 1.27 (0.3%), tests_pri_-900: 1.05 (0.3%), tests_pri_-90: 134 (32.9%), check_bayes: 132 (32.5%), b_tokenize: 7 (1.8%), b_tok_get_all: 7 (1.8%), b_comp_prob: 1.88 (0.5%), b_tok_touch_all: 113 (27.6%), b_finish: 0.89 (0.2%), tests_pri_0: 214 (52.5%), check_dkim_signature: 0.57 (0.1%), check_dkim_adsp: 2.1 (0.5%), poll_dns_idle: 0.57 (0.1%), tests_pri_10: 2.5 (0.6%), tests_pri_500: 8 (1.9%), rewrite_mail: 0.00 (0.0%) Subject: [PATCH 11/11] exec: Remove the label after_setid from bprm_fill_uid X-Spam-Flag: No X-SA-Exim-Version: 4.2.1 (built Thu, 05 May 2016 13:38:54 -0600) X-SA-Exim-Scanned: Yes (on in01.mta.xmission.com) Sender: owner-linux-security-module@vger.kernel.org Precedence: bulk List-ID: There is nothing past the label after_setid in bprm_fill_uid so replace code that jumps to it with return, and delete the label entirely. Signed-off-by: "Eric W. Biederman" --- fs/exec.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/fs/exec.c b/fs/exec.c index fc4edc7517a6..ccb552fcdcff 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -1598,15 +1598,15 @@ static void bprm_fill_uid(struct linux_binprm *bprm) kgid_t gid; if (!mnt_may_suid(bprm->file->f_path.mnt)) - goto after_setid; + return; if (task_no_new_privs(current)) - goto after_setid; + return; inode = bprm->file->f_path.dentry->d_inode; mode = READ_ONCE(inode->i_mode); if (!(mode & (S_ISUID|S_ISGID))) - goto after_setid; + return; /* Be careful if suid/sgid is set */ inode_lock(inode); @@ -1620,7 +1620,7 @@ static void bprm_fill_uid(struct linux_binprm *bprm) /* We ignore suid/sgid if there are no mappings for them in the ns */ if (!kuid_has_mapping(new->user_ns, uid) || !kgid_has_mapping(new->user_ns, gid)) - goto after_setid; + return; /* * Is the root directory and working directory shared or is @@ -1647,9 +1647,6 @@ static void bprm_fill_uid(struct linux_binprm *bprm) bprm->secureexec = 1; new->sgid = new->fsgid = new->egid = gid; } - -after_setid: - ; } /*