All of lore.kernel.org
 help / color / mirror / Atom feed
From: Paul Moore <paul@paul-moore.com>
To: Daniel Borkmann <daniel@iogearbox.net>
Cc: Ondrej Mosnacek <omosnace@redhat.com>,
	linux-security-module@vger.kernel.org,
	James Morris <jmorris@namei.org>,
	Steven Rostedt <rostedt@goodmis.org>,
	Ingo Molnar <mingo@redhat.com>,
	Stephen Smalley <stephen.smalley.work@gmail.com>,
	selinux@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
	linux-fsdevel@vger.kernel.org, bpf@vger.kernel.org,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	Casey Schaufler <casey@schaufler-ca.com>,
	jolsa@redhat.com
Subject: Re: [PATCH v2] lockdown,selinux: avoid bogus SELinux lockdown permission checks
Date: Sat, 29 May 2021 14:48:33 -0400	[thread overview]
Message-ID: <CAHC9VhS1XRZjKcTFgH1+n5uA-CeT+9BeSP5jvT2+RE5ougLpUg@mail.gmail.com> (raw)
In-Reply-To: <c7c2d7e1-e253-dce0-d35c-392192e4926e@iogearbox.net>

On Fri, May 28, 2021 at 2:28 PM Daniel Borkmann <daniel@iogearbox.net> wrote:
> In the case of tracing, it's different. You install small programs that are
> triggered when certain events fire. Random example from bpftrace's README [0],
> you want to generate a histogram of syscall counts by program. One-liner is:
>
>    bpftrace -e 'tracepoint:raw_syscalls:sys_enter { @[comm] = count(); }'
>
> bpftrace then goes and generates a BPF prog from this internally. One way of
> doing it could be to call bpf_get_current_task() helper and then access
> current->comm via one of bpf_probe_read_kernel{,_str}() helpers ...

I think we can all agree that the BPF tracing is a bit chaotic in the
sense that the tracing programs can be executed in various
places/contexts and that presents some challenges with respect to
access control and auditing.  If you are following the io_uring stuff
that is going on now you can see a little of what is required to make
audit work properly in the various io_uring contexts and that is
relatively small compared to what is possible with BPF tracing.  Of
course this assumes I've managed to understand bpf tracing properly
this morning, and I very well may still be missing points and/or
confused about some of the important details.  Corrections are
welcome.

Daniel's patch side steps that worry by just doing the lockdown
permission check when the BPF program is loaded, but that isn't a
great solution if the policy changes afterward.  I was hoping there
might be some way to perform the permission check as needed, but the
more I look the more that appears to be difficult, if not impossible
(once again, corrections are welcome).

I'm now wondering if the right solution here is to make use of the LSM
notifier mechanism.  I'm not yet entirely sure if this would work from
a BPF perspective, but I could envision the BPF subsystem registering
a LSM notification callback via register_blocking_lsm_notifier(), see
if Infiniband code as an example, and then when the LSM(s) policy
changes the BPF subsystem would get a notification and it could
revalidate the existing BPF programs and take block/remove/whatever
the offending BPF programs.  This obviously requires a few things
which I'm not sure are easily done, or even possible:

1. Somehow the BPF programs would need to be "marked" at
load/verification time with respect to their lockdown requirements so
that decisions can be made later.  Perhaps a flag in bpf_prog_aux?

2. While it looks like it should be possible to iterate over all of
the loaded BPF programs in the LSM notifier callback via
idr_for_each(prog_idr, ...), it is not clear to me if it is possible
to safely remove, or somehow disable, BPF programs once they have been
loaded.  Hopefully the BPF folks can help answer that question.

3. Disabling of BPF programs might be preferable to removing them
entirely on LSM policy changes as it would be possible to make the
lockdown state less restrictive at a future point in time, allowing
for the BPF program to be executed again.  Once again, not sure if
this is even possible.

Related, the lockdown LSM should probably also grow LSM notifier
support similar to selinux_lsm_notifier_avc_callback(), for example
either lock_kernel_down() or lockdown_write() might want to do a
call_blocking_lsm_notifier(LSM_POLICY_CHANGE, NULL) call.

-- 
paul moore
www.paul-moore.com

WARNING: multiple messages have this Message-ID (diff)
From: Paul Moore <paul@paul-moore.com>
To: Daniel Borkmann <daniel@iogearbox.net>
Cc: jolsa@redhat.com, selinux@vger.kernel.org,
	netdev@vger.kernel.org,
	Stephen Smalley <stephen.smalley.work@gmail.com>,
	Ondrej Mosnacek <omosnace@redhat.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	James Morris <jmorris@namei.org>,
	Casey Schaufler <casey@schaufler-ca.com>,
	linux-security-module@vger.kernel.org,
	Ingo Molnar <mingo@redhat.com>,
	linux-fsdevel@vger.kernel.org, bpf@vger.kernel.org,
	linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] lockdown,selinux: avoid bogus SELinux lockdown permission checks
Date: Sat, 29 May 2021 14:48:33 -0400	[thread overview]
Message-ID: <CAHC9VhS1XRZjKcTFgH1+n5uA-CeT+9BeSP5jvT2+RE5ougLpUg@mail.gmail.com> (raw)
In-Reply-To: <c7c2d7e1-e253-dce0-d35c-392192e4926e@iogearbox.net>

On Fri, May 28, 2021 at 2:28 PM Daniel Borkmann <daniel@iogearbox.net> wrote:
> In the case of tracing, it's different. You install small programs that are
> triggered when certain events fire. Random example from bpftrace's README [0],
> you want to generate a histogram of syscall counts by program. One-liner is:
>
>    bpftrace -e 'tracepoint:raw_syscalls:sys_enter { @[comm] = count(); }'
>
> bpftrace then goes and generates a BPF prog from this internally. One way of
> doing it could be to call bpf_get_current_task() helper and then access
> current->comm via one of bpf_probe_read_kernel{,_str}() helpers ...

I think we can all agree that the BPF tracing is a bit chaotic in the
sense that the tracing programs can be executed in various
places/contexts and that presents some challenges with respect to
access control and auditing.  If you are following the io_uring stuff
that is going on now you can see a little of what is required to make
audit work properly in the various io_uring contexts and that is
relatively small compared to what is possible with BPF tracing.  Of
course this assumes I've managed to understand bpf tracing properly
this morning, and I very well may still be missing points and/or
confused about some of the important details.  Corrections are
welcome.

Daniel's patch side steps that worry by just doing the lockdown
permission check when the BPF program is loaded, but that isn't a
great solution if the policy changes afterward.  I was hoping there
might be some way to perform the permission check as needed, but the
more I look the more that appears to be difficult, if not impossible
(once again, corrections are welcome).

I'm now wondering if the right solution here is to make use of the LSM
notifier mechanism.  I'm not yet entirely sure if this would work from
a BPF perspective, but I could envision the BPF subsystem registering
a LSM notification callback via register_blocking_lsm_notifier(), see
if Infiniband code as an example, and then when the LSM(s) policy
changes the BPF subsystem would get a notification and it could
revalidate the existing BPF programs and take block/remove/whatever
the offending BPF programs.  This obviously requires a few things
which I'm not sure are easily done, or even possible:

1. Somehow the BPF programs would need to be "marked" at
load/verification time with respect to their lockdown requirements so
that decisions can be made later.  Perhaps a flag in bpf_prog_aux?

2. While it looks like it should be possible to iterate over all of
the loaded BPF programs in the LSM notifier callback via
idr_for_each(prog_idr, ...), it is not clear to me if it is possible
to safely remove, or somehow disable, BPF programs once they have been
loaded.  Hopefully the BPF folks can help answer that question.

3. Disabling of BPF programs might be preferable to removing them
entirely on LSM policy changes as it would be possible to make the
lockdown state less restrictive at a future point in time, allowing
for the BPF program to be executed again.  Once again, not sure if
this is even possible.

Related, the lockdown LSM should probably also grow LSM notifier
support similar to selinux_lsm_notifier_avc_callback(), for example
either lock_kernel_down() or lockdown_write() might want to do a
call_blocking_lsm_notifier(LSM_POLICY_CHANGE, NULL) call.

-- 
paul moore
www.paul-moore.com

  parent reply	other threads:[~2021-05-29 18:48 UTC|newest]

Thread overview: 74+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-17  9:20 [PATCH v2] lockdown,selinux: avoid bogus SELinux lockdown permission checks Ondrej Mosnacek
2021-05-17  9:20 ` [PATCH v2] lockdown, selinux: " Ondrej Mosnacek
2021-05-17 11:00 ` [PATCH v2] lockdown,selinux: " Michael Ellerman
2021-05-17 11:00   ` Michael Ellerman
2021-05-26 11:44   ` Ondrej Mosnacek
2021-05-26 11:44     ` Ondrej Mosnacek
2021-05-27  4:28     ` James Morris
2021-05-27  4:28       ` James Morris
2021-05-27 14:18       ` Paul Moore
2021-05-27 14:18         ` Paul Moore
2021-05-28  1:37 ` Paul Moore
2021-05-28  1:37   ` Paul Moore
2021-05-28  7:09   ` Daniel Borkmann
2021-05-28  7:09     ` Daniel Borkmann
2021-05-28  9:53     ` Jiri Olsa
2021-05-28  9:53       ` Jiri Olsa
2021-05-28  9:56     ` Daniel Borkmann
2021-05-28  9:56       ` Daniel Borkmann
2021-05-28 10:16       ` Jiri Olsa
2021-05-28 10:16         ` Jiri Olsa
2021-05-28 11:47       ` Jiri Olsa
2021-05-28 11:47         ` Jiri Olsa
2021-05-28 11:54         ` Daniel Borkmann
2021-05-28 11:54           ` Daniel Borkmann
2021-05-28 13:42       ` Ondrej Mosnacek
2021-05-28 13:42         ` Ondrej Mosnacek
2021-05-28 14:20         ` Daniel Borkmann
2021-05-28 14:20           ` Daniel Borkmann
2021-05-28 15:54           ` Paul Moore
2021-05-28 15:54             ` Paul Moore
2021-05-28 15:47     ` Paul Moore
2021-05-28 15:47       ` Paul Moore
2021-05-28 18:10       ` Daniel Borkmann
2021-05-28 18:10         ` Daniel Borkmann
2021-05-28 22:52         ` Paul Moore
2021-05-28 22:52           ` Paul Moore
2021-05-29 18:48         ` Paul Moore [this message]
2021-05-29 18:48           ` Paul Moore
2021-05-31  8:24           ` Daniel Borkmann
2021-05-31  8:24             ` Daniel Borkmann
2021-06-01 20:47             ` Paul Moore
2021-06-01 20:47               ` Paul Moore
2021-06-02 12:40               ` Daniel Borkmann
2021-06-02 12:40                 ` Daniel Borkmann
2021-06-02 15:13                 ` Paul Moore
2021-06-02 15:13                   ` Paul Moore
2021-06-03 18:52                   ` Daniel Borkmann
2021-06-03 18:52                     ` Daniel Borkmann
2021-06-04  4:50                     ` Paul Moore
2021-06-04  4:50                       ` Paul Moore
2021-06-04 18:02                       ` Daniel Borkmann
2021-06-04 18:02                         ` Daniel Borkmann
2021-06-04 23:34                         ` Paul Moore
2021-06-04 23:34                           ` Paul Moore
2021-06-05  0:08                           ` Alexei Starovoitov
2021-06-05  0:08                             ` Alexei Starovoitov
2021-06-05 18:10                             ` Casey Schaufler
2021-06-05 18:10                               ` Casey Schaufler
2021-06-05 18:17                               ` Linus Torvalds
2021-06-05 18:17                                 ` Linus Torvalds
2021-06-06  2:11                                 ` Paul Moore
2021-06-06  2:11                                   ` Paul Moore
2021-06-06  1:30                             ` Paul Moore
2021-06-06  1:30                               ` Paul Moore
2021-06-02 13:39   ` Ondrej Mosnacek
2021-06-02 13:39     ` Ondrej Mosnacek
2021-06-03 17:46     ` Paul Moore
2021-06-03 17:46       ` Paul Moore
2021-06-08 11:01       ` Ondrej Mosnacek
2021-06-08 11:01         ` Ondrej Mosnacek
2021-06-09  2:40         ` Paul Moore
2021-06-09  2:40           ` Paul Moore
2021-05-28 13:58 ` Steven Rostedt
2021-05-28 13:58   ` Steven Rostedt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAHC9VhS1XRZjKcTFgH1+n5uA-CeT+9BeSP5jvT2+RE5ougLpUg@mail.gmail.com \
    --to=paul@paul-moore.com \
    --cc=bpf@vger.kernel.org \
    --cc=casey@schaufler-ca.com \
    --cc=daniel@iogearbox.net \
    --cc=jmorris@namei.org \
    --cc=jolsa@redhat.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=mingo@redhat.com \
    --cc=netdev@vger.kernel.org \
    --cc=omosnace@redhat.com \
    --cc=rostedt@goodmis.org \
    --cc=selinux@vger.kernel.org \
    --cc=stephen.smalley.work@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.