lkml.org 
[lkml]   [2021]   [May]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH] lockdown,selinux: fix bogus SELinux lockdown permission checks
    On Wed, May 12, 2021 at 6:18 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
    > On 5/12/2021 6:21 AM, Ondrej Mosnacek wrote:
    > > On Sat, May 8, 2021 at 12:17 AM Casey Schaufler <casey@schaufler-ca.com> wrote:
    > >> On 5/7/2021 4:40 AM, Ondrej Mosnacek wrote:
    > >>> Commit 59438b46471a ("security,lockdown,selinux: implement SELinux
    > >>> lockdown") added an implementation of the locked_down LSM hook to
    > >>> SELinux, with the aim to restrict which domains are allowed to perform
    > >>> operations that would breach lockdown.
    > >>>
    > >>> However, in several places the security_locked_down() hook is called in
    > >>> situations where the current task isn't doing any action that would
    > >>> directly breach lockdown, leading to SELinux checks that are basically
    > >>> bogus.
    > >>>
    > >>> Since in most of these situations converting the callers such that
    > >>> security_locked_down() is called in a context where the current task
    > >>> would be meaningful for SELinux is impossible or very non-trivial (and
    > >>> could lead to TOCTOU issues for the classic Lockdown LSM
    > >>> implementation), fix this by adding a separate hook
    > >>> security_locked_down_globally()
    > >> This is a poor solution to the stated problem. Rather than adding
    > >> a new hook you should add the task as a parameter to the existing hook
    > >> and let the security modules do as they will based on its value.
    > >> If the caller does not have an appropriate task it should pass NULL.
    > >> The lockdown LSM can ignore the task value and SELinux can make its
    > >> own decision based on the task value passed.
    > > The problem with that approach is that all callers would then need to
    > > be updated and I intended to keep the patch small as I'd like it to go
    > > to stable kernels as well.
    > >
    > > But it does seem to be a better long-term solution - would it work for
    > > you (and whichever maintainer would be taking the patch(es)) if I just
    > > added another patch that refactors it to use the task parameter?
    >
    > I can't figure out what you're suggesting. Are you saying that you
    > want to add a new hook *and* add the task parameter?

    No, just to keep this patch as-is (and let it go to stable in this
    form) and post another (non-stable) patch on top of it that undoes the
    new hook and re-implements the fix using your suggestion. (Yeah, it'll
    look weird, but I'm not sure how better to handle such situation - I'm
    open to doing it whatever different way the maintainers prefer.)

    --
    Ondrej Mosnacek
    Software Engineer, Linux Security - SELinux kernel
    Red Hat, Inc.

    \
     
     \ /
      Last update: 2021-05-12 23:02    [W:3.320 / U:0.792 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site