lkml.org 
[lkml]   [2020]   [Sep]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [RFC PATCH v8 1/3] fs: Introduce AT_INTERPRETED flag for faccessat2(2)
    From
    Date

    On 08/09/2020 15:42, Stephen Smalley wrote:
    > On Tue, Sep 8, 2020 at 9:29 AM Mimi Zohar <zohar@linux.ibm.com> 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@gmail.com> wrote:
    >>>>
    >>>> On Tue, Sep 8, 2020 at 8:43 AM Mickaël Salaün <mic@digikod.net> 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.
    >

    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().

    \
     
     \ /
      Last update: 2020-09-08 18:30    [W:4.482 / U:0.004 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site