linux-kernel - Re: [PATCH bpf] security: Fix hook iteration for secid_to

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CACYkzJ5e_JOLS-gmNug6e4RJkSsv7sjMUfMWyfMCsQLSoxS8RQ@mail.gmail.com>
Date:   Fri, 19 Jun 2020 15:13:32 +0200
From:   KP Singh <kpsingh@...omium.org>
To:     Ondrej Mosnacek <omosnace@...hat.com>
Cc:     Linux kernel mailing list <linux-kernel@...r.kernel.org>,
        bpf <bpf@...r.kernel.org>,
        Linux Security Module list 
        <linux-security-module@...r.kernel.org>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        James Morris <jmorris@...ei.org>,
        Anders Roxell <anders.roxell@...aro.org>,
        Casey Schaufler <casey@...aufler-ca.com>,
        Peter Zijlstra <peterz@...radead.org>, jpoimboe@...hat.com
Subject: Re: [PATCH bpf] security: Fix hook iteration for secid_to_secctx

Hi,

On Fri, Jun 19, 2020 at 2:49 PM Ondrej Mosnacek <omosnace@...hat.com> wrote:
>
> On Wed, May 20, 2020 at 2:56 PM KP Singh <kpsingh@...omium.org> wrote:
> > From: KP Singh <kpsingh@...gle.com>
> >
> > secid_to_secctx is not stackable, and since the BPF LSM registers this
> > hook by default, the call_int_hook logic is not suitable which
> > "bails-on-fail" and casues issues when other LSMs register this hook and
> > eventually breaks Audit.
> >
> > In order to fix this, directly iterate over the security hooks instead
> > of using call_int_hook as suggested in:
> >
> > https: //lore.kernel.org/bpf/9d0eb6c6-803a-ff3a-5603-9ad6d9edfc00@...aufler-ca.com/#t
> >
> > Fixes: 98e828a0650f ("security: Refactor declaration of LSM hooks")
> > Fixes: 625236ba3832 ("security: Fix the default value of secid_to_secctx hook"
> > Reported-by: Alexei Starovoitov <ast@...nel.org>
> > Signed-off-by: KP Singh <kpsingh@...gle.com>
> [...]
>
> Sorry for being late to the party, but doesn't this (and the
> associated default return value patch) just paper over a bigger
> problem? What if I have only the BPF LSM enabled and I attach a BPF
> program to this hook that just returns 0? Doesn't that allow anything
> privileged enough to do this to force the kernel to try and send
> memory from uninitialized pointers to userspace and/or copy such
> memory around and/or free uninitialized pointers?
>
> Why on earth does the BPF LSM directly expose *all* of the hooks, even
> those that are not being used for any security decisions (and are
> "useful" in this context only for borking the kernel...)? Feel free to
> prove me wrong, but this lazy approach of "let's just take all the
> hooks as they are and stick BPF programs to them" doesn't seem like a

The plan was definitely to not hook everywhere but only call the hooks
that do have a BPF program registered. This was one of the versions
we proposed in the initial patches where the call to the BPF LSM was
guarded by a static key with it being enabled only when there's a
BPF program attached to the hook.

https://lore.kernel.org/bpf/20200220175250.10795-5-kpsingh@chromium.org/

However, this special-cased BPF in the LSM framework, and, was met
with opposition. Our plan is to still achieve this, but we want to do this
with DEFINE_STATIC_CALL patches:

https://lore.kernel.org/lkml/cover.1547073843.git.jpoimboe@redhat.com

Using these, only can we enable the call into the hook based on whether
a program is attached, we can also eliminate the indirect call overhead which
currently affects the "slow" way which was decided in the discussion:

https://lore.kernel.org/bpf/202002241136.C4F9F7DFF@keescook/

> good choice... IMHO you should either limit the set of hooks that can
> be attached to only those that aren't used to return back values via

I am not sure if limiting the hooks is required here once we have
the ability to call into BPF only when a program is attached. If the
the user provides a BPF program, deliberately returns 0 (or any
other value) then it is working as intended. Even if we limit this in the
bpf LSM, deliberate privileged users can still achieve this with
other means.

- KP

> pointers, or (if you really really need to do some state
> updates/logging in those hooks) use wrapper functions that will call
> the BPF progs via a simplified interface so that they cannot cause
> unsafe behavior.
>
> --
> Ondrej Mosnacek
> Software Engineer, Platform Security - SELinux kernel
> Red Hat, Inc.
>