LKML Archive on lore.kernel.org
 help / color / Atom feed
From: "Mickaël Salaün" <mic@digikod.net>
To: Stephen Smalley <stephen.smalley.work@gmail.com>,
	Kees Cook <keescook@chromium.org>,
	John Johansen <john.johansen@canonical.com>,
	Kentaro Takeda <takedakn@nttdata.co.jp>,
	Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Cc: linux-kernel <linux-kernel@vger.kernel.org>,
	"Aleksa Sarai" <cyphar@cyphar.com>,
	"Alexei Starovoitov" <ast@kernel.org>,
	"Al Viro" <viro@zeniv.linux.org.uk>,
	"Andy Lutomirski" <luto@kernel.org>,
	"Christian Heimes" <christian@python.org>,
	"Daniel Borkmann" <daniel@iogearbox.net>,
	"Deven Bowers" <deven.desai@linux.microsoft.com>,
	"Eric Chiang" <ericchiang@google.com>,
	"Florian Weimer" <fweimer@redhat.com>,
	"James Morris" <jmorris@namei.org>, "Jan Kara" <jack@suse.cz>,
	"Jann Horn" <jannh@google.com>,
	"Jonathan Corbet" <corbet@lwn.net>,
	"Lakshmi Ramasubramanian" <nramas@linux.microsoft.com>,
	"Matthew Garrett" <mjg59@google.com>,
	"Matthew Wilcox" <willy@infradead.org>,
	"Michael Kerrisk" <mtk.manpages@gmail.com>,
	"Mickaël Salaün" <mickael.salaun@ssi.gouv.fr>,
	"Mimi Zohar" <zohar@linux.ibm.com>,
	"Philippe Trébuchet" <philippe.trebuchet@ssi.gouv.fr>,
	"Scott Shell" <scottsh@microsoft.com>,
	"Sean Christopherson" <sean.j.christopherson@intel.com>,
	"Shuah Khan" <shuah@kernel.org>,
	"Steve Dower" <steve.dower@python.org>,
	"Steve Grubb" <sgrubb@redhat.com>,
	"Thibaut Sautereau" <thibaut.sautereau@ssi.gouv.fr>,
	"Vincent Strubel" <vincent.strubel@ssi.gouv.fr>,
	kernel-hardening@lists.openwall.com, linux-api@vger.kernel.org,
	linux-integrity@vger.kernel.org,
	"LSM List" <linux-security-module@vger.kernel.org>,
	"Linux FS Devel" <linux-fsdevel@vger.kernel.org>
Subject: Re: [PATCH v5 3/6] fs: Enable to enforce noexec mounts or file exec through O_MAYEXEC
Date: Thu, 14 May 2020 21:16:13 +0200
Message-ID: <b740d658-a2da-5773-7a10-59a0ca52ac6b@digikod.net> (raw)
In-Reply-To: <CAEjxPJ4R_juwvRbKiCg5OGuhAi1ZuVytK4fKCDT_kT6VKc8iRg@mail.gmail.com>


On 14/05/2020 18:10, Stephen Smalley wrote:
> On Thu, May 14, 2020 at 11:45 AM Kees Cook <keescook@chromium.org> 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 <keescook@chromium.org> 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.

  reply index

Thread overview: 59+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-05 15:31 [PATCH v5 0/6] Add support for O_MAYEXEC Mickaël Salaün
2020-05-05 15:31 ` [PATCH v5 1/6] fs: Add support for an O_MAYEXEC flag on openat2(2) Mickaël Salaün
2020-05-12 21:05   ` Kees Cook
2020-05-12 21:40     ` Christian Heimes
2020-05-12 22:56       ` Kees Cook
2020-05-13 10:13     ` Mickaël Salaün
2020-05-05 15:31 ` [PATCH v5 2/6] fs: Add a MAY_EXECMOUNT flag to infer the noexec mount property Mickaël Salaün
2020-05-12 21:09   ` Kees Cook
2020-05-14  8:14     ` Lev R. Oshvang .
2020-05-14 15:48       ` Kees Cook
2020-05-17 16:57         ` Lev R. Oshvang .
2020-05-05 15:31 ` [PATCH v5 3/6] fs: Enable to enforce noexec mounts or file exec through O_MAYEXEC Mickaël Salaün
2020-05-05 15:44   ` Randy Dunlap
2020-05-05 16:55     ` Mickaël Salaün
2020-05-05 17:40       ` Randy Dunlap
2020-05-12 21:48   ` Kees Cook
2020-05-13 11:09     ` Mickaël Salaün
2020-05-13 15:37   ` Stephen Smalley
2020-05-13 23:27     ` Kees Cook
2020-05-14  3:05       ` Kees Cook
2020-05-14 10:12         ` David Laight
2020-05-14 12:22         ` Stephen Smalley
2020-05-14 14:41           ` Kees Cook
2020-05-14 15:52             ` Stephen Smalley
2020-05-14 15:45           ` Kees Cook
2020-05-14 16:10             ` Stephen Smalley
2020-05-14 19:16               ` Mickaël Salaün [this message]
2020-05-15  0:58                 ` Tetsuo Handa
2020-05-15  8:01                 ` How about just O_EXEC? (was Re: [PATCH v5 3/6] fs: Enable to enforce noexec mounts or file exec through O_MAYEXEC) Kees Cook
2020-05-15  8:43                   ` Florian Weimer
2020-05-15 14:37                     ` Kees Cook
2020-05-15 14:43                       ` Florian Weimer
2020-05-15 15:50                         ` Kees Cook
2020-05-18  7:26                           ` Florian Weimer
2020-05-19  2:23                           ` Aleksa Sarai
2020-05-19 10:13                             ` Mickaël Salaün
2020-05-15 11:04                   ` Mickaël Salaün
2020-05-15 15:46                     ` Kees Cook
2020-05-15 18:24                       ` Mickaël Salaün
2020-05-14 19:21       ` [PATCH v5 3/6] fs: Enable to enforce noexec mounts or file exec through O_MAYEXEC Mickaël Salaün
2020-05-05 15:31 ` [PATCH v5 4/6] selftest/openat2: Add tests for O_MAYEXEC enforcing Mickaël Salaün
2020-05-12 21:57   ` Kees Cook
2020-05-13 11:18     ` Mickaël Salaün
2020-05-05 15:31 ` [PATCH v5 5/6] doc: Add documentation for the fs.open_mayexec_enforce sysctl Mickaël Salaün
2020-05-12 22:00   ` Kees Cook
2020-05-13 11:20     ` Mickaël Salaün
2020-05-05 15:31 ` [PATCH v5 6/6] ima: add policy support for the new file open MAY_OPENEXEC flag Mickaël Salaün
2020-05-05 15:36 ` [PATCH v5 0/6] Add support for O_MAYEXEC Mickaël Salaün
2020-05-06 13:58   ` Lev R. Oshvang .
2020-05-06 15:41     ` Aleksa Sarai
2020-05-07  8:30     ` Mickaël Salaün
2020-05-07  8:05 ` David Laight
2020-05-07  8:36   ` Mickaël Salaün
2020-05-07  9:00     ` David Laight
2020-05-07  9:30       ` Mickaël Salaün
2020-05-07  9:44         ` David Laight
2020-05-07 13:38           ` Mickaël Salaün
2020-05-08  7:15             ` Lev R. Oshvang .
2020-05-08 14:01               ` Mimi Zohar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b740d658-a2da-5773-7a10-59a0ca52ac6b@digikod.net \
    --to=mic@digikod.net \
    --cc=ast@kernel.org \
    --cc=christian@python.org \
    --cc=corbet@lwn.net \
    --cc=cyphar@cyphar.com \
    --cc=daniel@iogearbox.net \
    --cc=deven.desai@linux.microsoft.com \
    --cc=ericchiang@google.com \
    --cc=fweimer@redhat.com \
    --cc=jack@suse.cz \
    --cc=jannh@google.com \
    --cc=jmorris@namei.org \
    --cc=john.johansen@canonical.com \
    --cc=keescook@chromium.org \
    --cc=kernel-hardening@lists.openwall.com \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-integrity@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=mickael.salaun@ssi.gouv.fr \
    --cc=mjg59@google.com \
    --cc=mtk.manpages@gmail.com \
    --cc=nramas@linux.microsoft.com \
    --cc=penguin-kernel@I-love.SAKURA.ne.jp \
    --cc=philippe.trebuchet@ssi.gouv.fr \
    --cc=scottsh@microsoft.com \
    --cc=sean.j.christopherson@intel.com \
    --cc=sgrubb@redhat.com \
    --cc=shuah@kernel.org \
    --cc=stephen.smalley.work@gmail.com \
    --cc=steve.dower@python.org \
    --cc=takedakn@nttdata.co.jp \
    --cc=thibaut.sautereau@ssi.gouv.fr \
    --cc=vincent.strubel@ssi.gouv.fr \
    --cc=viro@zeniv.linux.org.uk \
    --cc=willy@infradead.org \
    --cc=zohar@linux.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git
	git clone --mirror https://lore.kernel.org/lkml/8 lkml/git/8.git
	git clone --mirror https://lore.kernel.org/lkml/9 lkml/git/9.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org
	public-inbox-index lkml

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git