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: Wed, 2 Jun 2021 11:13:27 -0400	[thread overview]
Message-ID: <CAHC9VhTuPnPs1wMTmoGUZ4fvyy-es9QJpE7O_yTs2JKos4fgbw@mail.gmail.com> (raw)
In-Reply-To: <3ca181e3-df32-9ae0-12c6-efb899b7ce7a@iogearbox.net>

On Wed, Jun 2, 2021 at 8:40 AM Daniel Borkmann <daniel@iogearbox.net> wrote:
> On 6/1/21 10:47 PM, Paul Moore wrote:
> > The thing I'm worried about would be the case where a LSM policy
> > change requires that an existing BPF program be removed or disabled.
> > I'm guessing based on the refcounting that there is not presently a
> > clean way to remove a BPF program from the system, but is this
> > something we could resolve?  If we can't safely remove a BPF program
> > from the system, can we replace/swap it with an empty/NULL BPF
> > program?
>
> Removing progs would somehow mean destroying those references from an
> async event and then /safely/ guaranteeing that nothing is accessing
> them anymore. But then if policy changes once more where they would
> be allowed again we would need to revert back to the original state,
> which brings us to your replace/swap question with an empty/null prog.
> It's not feasible either, because there are different BPF program types
> and they can have different return code semantics that lead to subsequent
> actions. If we were to replace them with an empty/NULL program, then
> essentially this will get us into an undefined system state given it's
> unclear what should be a default policy for each program type, etc.
> Just to pick one simple example, outside of tracing, that comes to mind:
> say, you attached a program with tc to a given device ingress hook. That
> program implements firewalling functionality, and potentially deep down
> in that program there is functionality to record/sample packets along
> with some meta data. Part of what is exported to the ring buffer to the
> user space reader may be a struct net_device field that is otherwise not
> available (or at least not yet), hence it's probe-read with mentioned
> helpers. If you were now to change the SELinux policy for that tc loader
> application, and therefore replace/swap the progs in the kernel that were
> loaded with it (given tc's lockdown policy was recorded in their sec blob)
> with an empty/NULL program, then either you say allow-all or drop-all,
> but either way, you break the firewalling functionality completely by
> locking yourself out of the machine or letting everything through. There
> is no sane way where we could reason about the context/internals of a
> given program where it would be safe to replace with a simple empty/NULL
> prog.

Help me out here, is your answer that the access check can only be
done at BPF program load time?  That isn't really a solution from a
SELinux perspective as far as I'm concerned.

I understand the ideas I've tossed out aren't practical from a BPF
perspective, but it would be nice if we could find something that does
work.  Surely you BPF folks can think of some way to provide a
runtime, not load time, check?

-- 
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: Wed, 2 Jun 2021 11:13:27 -0400	[thread overview]
Message-ID: <CAHC9VhTuPnPs1wMTmoGUZ4fvyy-es9QJpE7O_yTs2JKos4fgbw@mail.gmail.com> (raw)
In-Reply-To: <3ca181e3-df32-9ae0-12c6-efb899b7ce7a@iogearbox.net>

On Wed, Jun 2, 2021 at 8:40 AM Daniel Borkmann <daniel@iogearbox.net> wrote:
> On 6/1/21 10:47 PM, Paul Moore wrote:
> > The thing I'm worried about would be the case where a LSM policy
> > change requires that an existing BPF program be removed or disabled.
> > I'm guessing based on the refcounting that there is not presently a
> > clean way to remove a BPF program from the system, but is this
> > something we could resolve?  If we can't safely remove a BPF program
> > from the system, can we replace/swap it with an empty/NULL BPF
> > program?
>
> Removing progs would somehow mean destroying those references from an
> async event and then /safely/ guaranteeing that nothing is accessing
> them anymore. But then if policy changes once more where they would
> be allowed again we would need to revert back to the original state,
> which brings us to your replace/swap question with an empty/null prog.
> It's not feasible either, because there are different BPF program types
> and they can have different return code semantics that lead to subsequent
> actions. If we were to replace them with an empty/NULL program, then
> essentially this will get us into an undefined system state given it's
> unclear what should be a default policy for each program type, etc.
> Just to pick one simple example, outside of tracing, that comes to mind:
> say, you attached a program with tc to a given device ingress hook. That
> program implements firewalling functionality, and potentially deep down
> in that program there is functionality to record/sample packets along
> with some meta data. Part of what is exported to the ring buffer to the
> user space reader may be a struct net_device field that is otherwise not
> available (or at least not yet), hence it's probe-read with mentioned
> helpers. If you were now to change the SELinux policy for that tc loader
> application, and therefore replace/swap the progs in the kernel that were
> loaded with it (given tc's lockdown policy was recorded in their sec blob)
> with an empty/NULL program, then either you say allow-all or drop-all,
> but either way, you break the firewalling functionality completely by
> locking yourself out of the machine or letting everything through. There
> is no sane way where we could reason about the context/internals of a
> given program where it would be safe to replace with a simple empty/NULL
> prog.

Help me out here, is your answer that the access check can only be
done at BPF program load time?  That isn't really a solution from a
SELinux perspective as far as I'm concerned.

I understand the ideas I've tossed out aren't practical from a BPF
perspective, but it would be nice if we could find something that does
work.  Surely you BPF folks can think of some way to provide a
runtime, not load time, check?

-- 
paul moore
www.paul-moore.com

  reply	other threads:[~2021-06-02 15:14 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
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 [this message]
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=CAHC9VhTuPnPs1wMTmoGUZ4fvyy-es9QJpE7O_yTs2JKos4fgbw@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.