On Tue, Sep 8, 2020 at 9:29 AM Mimi Zohar <zohar@xxxxxxxxxxxxx> wrote: > > On Tue, 2020-09-08 at 08:52 -0400, Stephen Smalley wrote: > > On Tue, Sep 8, 2020 at 8:50 AM Stephen Smalley > > <stephen.smalley.work@xxxxxxxxx> wrote: > > > > > > On Tue, Sep 8, 2020 at 8:43 AM Mickaël Salaün <mic@xxxxxxxxxxx> wrote: > > > > > > > > > > > > On 08/09/2020 14:28, Mimi Zohar wrote: > > > > > Hi Mickael, > > > > > > > > > > On Tue, 2020-09-08 at 09:59 +0200, Mickaël Salaün wrote: > > > > >> + mode |= MAY_INTERPRETED_EXEC; > > > > >> + /* > > > > >> + * For compatibility reasons, if the system-wide policy > > > > >> + * doesn't enforce file permission checks, then > > > > >> + * replaces the execute permission request with a read > > > > >> + * permission request. > > > > >> + */ > > > > >> + mode &= ~MAY_EXEC; > > > > >> + /* To be executed *by* user space, files must be readable. */ > > > > >> + mode |= MAY_READ; > > > > > > > > > > After this change, I'm wondering if it makes sense to add a call to > > > > > security_file_permission(). IMA doesn't currently define it, but > > > > > could. > > > > > > > > Yes, that's the idea. We could replace the following inode_permission() > > > > with file_permission(). I'm not sure how this will impact other LSMs though. > > I wasn't suggesting replacing the existing security_inode_permission > hook later, but adding a new security_file_permission hook here. > > > > > > > They are not equivalent at least as far as SELinux is concerned. > > > security_file_permission() was only to be used to revalidate > > > read/write permissions previously checked at file open to support > > > policy changes and file or process label changes. We'd have to modify > > > the SELinux hook if we wanted to have it check execute access even if > > > nothing has changed since open time. > > > > Also Smack doesn't appear to implement file_permission at all, so it > > would skip Smack checking. > > My question is whether adding a new security_file_permission call here > would break either SELinux or Apparmor? selinux_inode_permission() has special handling for MAY_ACCESS so we'd need to duplicate that into selinux_file_permission() -> selinux_revalidate_file_permission(). Also likely need to adjust selinux_file_permission() to explicitly check whether the mask includes any permissions not checked at open time. So some changes would be needed here. By default, it would be a no-op unless there was a policy reload or the file was relabeled between the open(2) and the faccessat(2) call.
