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: 11576189 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 A3FB460D for ; Thu, 28 May 2020 15:45:36 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 85EE5207BC for ; Thu, 28 May 2020 15:45:36 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2404623AbgE1Ppe (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: linux-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org 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: 11576199 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 759681391 for ; Thu, 28 May 2020 15:46:08 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 62319207F5 for ; Thu, 28 May 2020 15:46:08 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2404646AbgE1PqD (ORCPT ); Thu, 28 May 2020 11:46:03 -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: linux-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org 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: 11576205 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 82E4460D for ; Thu, 28 May 2020 15:46:43 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 68403207F9 for ; Thu, 28 May 2020 15:46:43 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2404579AbgE1Pqg (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: linux-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org 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: 11576213 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 E0FCA1391 for ; Thu, 28 May 2020 15:47:20 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id C969320897 for ; Thu, 28 May 2020 15:47:20 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2404685AbgE1PrS (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: linux-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org 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;