From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-3.7 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7A8D7C433E4 for ; Thu, 14 May 2020 19:16:26 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 62E04206D8 for ; Thu, 14 May 2020 19:16:26 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727839AbgENTQV (ORCPT ); Thu, 14 May 2020 15:16:21 -0400 Received: from smtp-bc0c.mail.infomaniak.ch ([45.157.188.12]:54185 "EHLO smtp-bc0c.mail.infomaniak.ch" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727772AbgENTQT (ORCPT ); Thu, 14 May 2020 15:16:19 -0400 Received: from smtp-2-0001.mail.infomaniak.ch (unknown [10.5.36.108]) by smtp-3-3000.mail.infomaniak.ch (Postfix) with ESMTPS id 49NLqj360nzlhjYr; Thu, 14 May 2020 21:16:17 +0200 (CEST) Received: from ns3096276.ip-94-23-54.eu (unknown [94.23.54.103]) by smtp-2-0001.mail.infomaniak.ch (Postfix) with ESMTPA id 49NLqf39VKzljTrt; Thu, 14 May 2020 21:16:14 +0200 (CEST) Subject: Re: [PATCH v5 3/6] fs: Enable to enforce noexec mounts or file exec through O_MAYEXEC To: Stephen Smalley , Kees Cook , John Johansen , Kentaro Takeda , Tetsuo Handa Cc: linux-kernel , Aleksa Sarai , Alexei Starovoitov , Al Viro , Andy Lutomirski , Christian Heimes , Daniel Borkmann , Deven Bowers , Eric Chiang , Florian Weimer , James Morris , Jan Kara , Jann Horn , Jonathan Corbet , Lakshmi Ramasubramanian , Matthew Garrett , Matthew Wilcox , Michael Kerrisk , =?UTF-8?Q?Micka=c3=abl_Sala=c3=bcn?= , Mimi Zohar , =?UTF-8?Q?Philippe_Tr=c3=a9buchet?= , Scott Shell , Sean Christopherson , Shuah Khan , Steve Dower , Steve Grubb , Thibaut Sautereau , Vincent Strubel , kernel-hardening@lists.openwall.com, linux-api@vger.kernel.org, linux-integrity@vger.kernel.org, LSM List , Linux FS Devel References: <20200505153156.925111-1-mic@digikod.net> <20200505153156.925111-4-mic@digikod.net> <202005131525.D08BFB3@keescook> <202005132002.91B8B63@keescook> <202005140830.2475344F86@keescook> From: =?UTF-8?Q?Micka=c3=abl_Sala=c3=bcn?= Message-ID: Date: Thu, 14 May 2020 21:16:13 +0200 User-Agent: MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Antivirus: Dr.Web (R) for Unix mail servers drweb plugin ver.6.0.2.8 X-Antivirus-Code: 0x100000 Sender: linux-integrity-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-integrity@vger.kernel.org On 14/05/2020 18:10, Stephen Smalley wrote: > On Thu, May 14, 2020 at 11:45 AM Kees Cook wrote: >> >> On Thu, May 14, 2020 at 08:22:01AM -0400, Stephen Smalley wrote: >>> On Wed, May 13, 2020 at 11:05 PM Kees Cook wrote: >>>> >>>> On Wed, May 13, 2020 at 04:27:39PM -0700, Kees Cook wrote: >>>>> Like, couldn't just the entire thing just be: >>>>> >>>>> diff --git a/fs/namei.c b/fs/namei.c >>>>> index a320371899cf..0ab18e19f5da 100644 >>>>> --- a/fs/namei.c >>>>> +++ b/fs/namei.c >>>>> @@ -2849,6 +2849,13 @@ static int may_open(const struct path *path, int acc_mode, int flag) >>>>> break; >>>>> } >>>>> >>>>> + if (unlikely(mask & MAY_OPENEXEC)) { >>>>> + if (sysctl_omayexec_enforce & OMAYEXEC_ENFORCE_MOUNT && >>>>> + path_noexec(path)) >>>>> + return -EACCES; >>>>> + if (sysctl_omayexec_enforce & OMAYEXEC_ENFORCE_FILE) >>>>> + acc_mode |= MAY_EXEC; >>>>> + } >>>>> error = inode_permission(inode, MAY_OPEN | acc_mode); >>>>> if (error) >>>>> return error; >>>>> >>>> >>>> FYI, I've confirmed this now. Effectively with patch 2 dropped, patch 3 >>>> reduced to this plus the Kconfig and sysctl changes, the self tests >>>> pass. >>>> >>>> I think this makes things much cleaner and correct. >>> >>> I think that covers inode-based security modules but not path-based >>> ones (they don't implement the inode_permission hook). For those, I >>> would tentatively guess that we need to make sure FMODE_EXEC is set on >>> the open file and then they need to check for that in their file_open >>> hooks. >> >> I kept confusing myself about what order things happened in, so I made >> these handy notes about the call graph: >> >> openat2(dfd, char * filename, open_how) >> do_filp_open(dfd, filename, open_flags) >> path_openat(nameidata, open_flags, flags) >> do_open(nameidata, file, open_flags) >> may_open(path, acc_mode, open_flag) >> inode_permission(inode, MAY_OPEN | acc_mode) >> security_inode_permission(inode, acc_mode) >> vfs_open(path, file) >> do_dentry_open(file, path->dentry->d_inode, open) >> if (unlikely(f->f_flags & FMODE_EXEC && !S_ISREG(inode->i_mode))) ... >> security_file_open(f) >> open() >> >> So, it looks like adding FMODE_EXEC into f_flags in do_open() is needed in >> addition to injecting MAY_EXEC into acc_mode in do_open()? Hmmm > > Just do both in build_open_flags() and be done with it? Looks like he > was already setting FMODE_EXEC in patch 1 so we just need to teach > AppArmor/TOMOYO to check for it and perform file execute checking in > that case if !current->in_execve? I can postpone the file permission check for another series to make this one simpler (i.e. mount noexec only). Because it depends on the sysctl setting, it is OK to add this check later, if needed. In the meantime, AppArmor and Tomoyo could be getting ready for this.