LSM stacking in next for 6.1?

[Paul Moore]

On Thu, Sep 8, 2022 at 2:05 PM Casey Schaufler <casey at schaufler-ca.com> wrote:
> On 9/7/2022 8:57 PM, Paul Moore wrote:
> > On Wed, Sep 7, 2022 at 7:53 PM Casey Schaufler <casey at schaufler-ca.com> wrote:
> >> On 9/7/2022 4:27 PM, Paul Moore wrote:

...

> >>>   The ease-of-use quality is a bit subjective, but it does need
> >>> another interface to use properly and it requires string parsing which
> >>> history has shown to be an issue time and time again (although it is
> >>> relatively simple here).
> >> There was a lot of discussion regarding that. My proposed
> >> apparmor="unconfined",smack="User" format was panned for those same reasons.
> >> The nil byte format has been used elsewhere and suggested for that reason.
> > Based on what I recall from those discussions, it was my impression
> > the nil byte label delimiter was suggested simply because no one was
> > entertaining the idea of using something other than the existing
> > procfs interface.  It is my opinion that we've taken that interface
> > about as far as it can go, and while it needs to stay intact for
> > compatibility reasons, the LSM stacking functionality should move to a
> > different API that is better suited for it.
>
> It's going to raise its ugly head again with SO_PEERCONTEXT for the
> SELinux+Smack case. But we can cross that bridge when we come to it.

There are also problems with IP_PASSSEC/SCM_SECURITY that we've never
fully resolved (and have gotten a bit lucky over the years); it very
well could be time to add support for IP_SECURITY as the multi-LSM
replacement for SCM_SECURITY.  We could leverage the same LSM context
structures as in the other multi-LSM interfaces.  Existing
applications could continue to use SCM_SECURITY; in fact I believe we
could have both SCM_SECURITY and IP_SECURITY in the same message for
maximum compatibility.

https://github.com/SELinuxProject/selinux-kernel/issues/24

For SO_PEERSEC, we should probably just introduce SO_PEERSEC2 or
similar, using the same multi-LSM context structures as the other
interfaces.

> > In case it helps spur your imagination, here is a revised strawman:
> >
> > /**
> >  * struct lsm_ctx - LSM context
> >  * @id: the LSM id number, see LSM_ID_XXX
>
> A LSM ID hard coded in a kernel header makes it harder to develop new
> security modules.

There is so much precedence for defining a token scalar value to
represent a "thing" that I don't know where to begin.  Look at IANA,
there are entire organizations that exist to map "things" to numbers.

If you're objecting to assigning LSMs fixed integer numbers you've got
to give me some very explicit reasons (complete with examples) as to
why that would be a mistake.

> The security module can't be self contained. I say
> that, but I acknowledge that I've done the same kind of thing with the
> definition of the struct lsmblob. That isn't part of an external API
> however.

I'm not following you here.  See my comment above about better examples.

> It may also interfere with Tetsuo's long standing request that
> we don't do things that prevent the possibility of loadable security
> modules at some point in the future.

I already replied to Tetsuo's email, and while this particular point
about LSM ID numbers wasn't directly addressed, my response there
seems to apply equally well here: it's not so much about loadable
LSMs, it's about out-of-tree LSMs.  These are two very different
things, with different solutions.

> On the other hand, there's no great way to include two variable length
> strings in a structure like this. So unless we adopt something as ugly
> as the nil byte scheme this is supposed to replace I expect we're stuck
> with an LSM ID.

I don't like making general comments, but when in doubt, consider me
not-a-fan of string-based identifiers in APIs.  Give me token scalar
values instead.

> >  * @flags: LSM specific flags, zero if unused
>
> For an API shouldn't this be a specific size? u32? I'm not really
> up to date on the guidance regarding which it should be.

Enh, sure, whatever.  You'll remember my initial comment about not
being a syscall stylist; if the discussion has moved to seriously
discussing the syscall prototypes we should likely start a new thread
and bring in the syscall folks ... I vaguely remember there was a
mailing list for syscalls and API changes ...

> I will head in this direction. A couple questions:
>
> Would we want lsm_prev_ctx() as well as lsm_current_ctx(),

I'm not sure I'm following your thinking, what would lsm_prev_ctx() do?

> or should we use the lsm_ctx->flags to identify the provided
> context? If we did that we should have an lsm_ctx() system call
> that returns the current, prev, and whatever other security
> module specific attributes might be associated with the process,
> each identified in the flags field. While the "current" context
> is usually what we're after, there may be cases where the other
> attributes are desired.

I don't understand what you mean by "prev"{ious} attributes.  I'm
thinking of lsm_current_ctx() as a multi-LSM replacement for
/proc/self/attr/current.  If, for example, we wanted something for
/proc/self/attr/exec I imagine we would create lsm_current_exec(), or
something similarly named, with a similar prototype.

Or perhaps we try to stick a bit closer to the procfs naming and go
with lsm_self_cur(...) and lsm_self_exec(...).  All things to discuss.

-- 
paul-moore.com