On 08/09/2020 15:42, Stephen Smalley wrote:

On Tue, Sep 8, 2020 at 9:29 AM Mimi Zohar [off-list ref] wrote:

quoted

On Tue, 2020-09-08 at 08:52 -0400, Stephen Smalley wrote:

quoted

On Tue, Sep 8, 2020 at 8:50 AM Stephen Smalley
[off-list ref] wrote:

quoted

On Tue, Sep 8, 2020 at 8:43 AM Mickaël Salaün [off-list ref] wrote:

quoted

On 08/09/2020 14:28, Mimi Zohar wrote:

quoted

Hi Mickael,

On Tue, 2020-09-08 at 09:59 +0200, Mickaël Salaün wrote:

quoted

+                    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.

quoted

quoted

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.

We could create a new hook path_permission(struct path *path, int mask)
as a superset of inode_permission(). To be more convenient, his new hook
could then just call inode_permission() for every LSMs not implementing
path_permission().