New LSM hooks

[Paul Moore]

On Tue, Feb 5, 2019 at 3:04 PM Casey Schaufler <casey at schaufler-ca.com> wrote:
> On 2/5/2019 10:15 AM, Paul Moore wrote:
> > On Tue, Feb 5, 2019 at 12:40 PM Casey Schaufler <casey at schaufler-ca.com> wrote:
> >> Full disclosure: This is all about me and my interests.
> >>
> >> Developers propose new LSM hooks for many reasons. Often,
> >> it's in support of an exotic feature such as Infiniband.
> >> Sometimes it's because someone got clever when they optimized
> >> an otherwise mundane feature and bypassed the usual hooks,
> >> as is being proposed for kernfs. Usually there is a particular
> >> security module (i.e. SELinux) that is targeted for the hook.
> >> In almost all cases a hook is provided for that LSM, but not
> >> for any other. In many cases the developers don't even include
> >> the LSM email list in the discussions. LSM maintainers find
> >> out about the new hooks after the fact.
> >
> > I think it is each LSM maintainer's job to ensure that patches which
> > affect the LSM hooks, either through modification or addition, are
> > CC'd to the LSM list for discussion.  I've tried to be good about
> > this, but I'm sure I've missed some over the years.
>
> It's often not the LSM maintainer, but the developer of
> the feature that introduces the hook. That's the case that
> has been a problem.

I believe that will always be a problem, no matter what we do.  The
point I was trying to make was that everyone, especially the
maintainers, need to watch for this when patches are posted and make
sure the patch author posts to the LSM list in addition to any of the
relevant LSM specific lists.

> >> Prior to 2008, when there was only one LSM, this made perfect
> >> sense. Since then, it's been a regular practice justified by
> >> the notion that it's the LSM maintainer's option to use the
> >> hook or not. That also makes sense in cases where the use is
> >> strictly optional, as it is in binder. It does not make sense,
> >> however, when a core system facility like kernfs is involved.
> >
> > The term "optional" is tricky here.  In all the cases I can think of
> > where a LSM hook has been added to an existing bit of kernel
> > functionality, the hook has almost always been optional if for no
> > other reason than it didn't exist in the previous kernel.  I suppose
> > one could argue that there is a functional disparity between LSMs if
> > one LSM implements the hook and the others do not, but that is a
> > different issue.
>
> Agreed that there's ambiguity about what should be done for
> an "optional" feature. If RedHat adds support for a feature
> they have every reason to make sure it works correctly with
> SELinux and no incentive to implement AppArmor hooks.

I want to be very clear on this point, I don't care who submits the
patches, or who may pay them to do it.  From an upstream perspective,
patches are patches.  The individual patch submitter may not see
things quite this same way, but it is everyone's responsibility to
make sure that we do the Right Thing, regardless of who may be paying
us for our work.

Earlier in the thread I advocated for each of the LSM maintainers to
work with patchset authors who introduce new LSM hooks but don't
provide an implementation for their LSM.  I think it is that
cooperation and discussion which is important.

> ... But is overlayfs an "optional" feature?

I'm not using overlayfs on this Linux system right now, so I would
have to say "yes".  I know it will shock many who read this, but you
can still run a Linux system these days without containers.

> If the container developers
> who wanted overlayfs only cared about an SELinux environment
> does that excuse them from introducing a feature that didn't
> work with Smack?

There is a big difference between "doesn't work" and "not yet
implemented".  I'm not sure where things stand with respect to
overlayfs support in the various LSMs, but have you or any of the
other LSM maintainers tried working with Vivek to add support wherever
it may be missing?  He's a nice guy, you should send him mail.

> >> I get a double whammy on these new LSM hooks. I have to try
> >> to figure out if Smack cares, and if it does, whether to proposed
> >> hook will solve the problem for Smack. Because Smack uses
> >> xattrs and process attributes differently from SELinux there are
> >> often problems with hooks that are provided with only an SELinux
> >> implementation. I also have to work out how the new hook will
> >> work in the stacked security module case. There are some existing
> >> hooks that are a special challenge there, and when a new hook is
> >> proposed that does the same kind of things (e.g. use of secid,
> >> secctx or list of xattrs) without considering the consequences
> >> for other modules.
> >
> > Once again, I think it is important to CC the LSM list on these sorts
> > of patchsets, and to get the patch author to consider other LSMs, but
> > I think asking the original author to preemptively add support to each
> > LSM for every new addition is too high a bar.
>
> Two edged sword. If someone introduced an LSM that worked
> with btrfs but broke ext4 and xfs it wouldn't be likely to
> make it upstream. I wouldn't expect the developer who wants
> to introduce a new hook to provide all the LSM versions, but
> I would like that developer to work with the LSM maintainers
> before the patch is submitted.

With that last sentence above I think we're pretty much saying the
same thing, the only real difference is that I don't particularly care
when the collaboration takes place so long as it takes place.  In my
opinion it would be foolish to hold off committing working support in
one LSM because support for another remains a work in progress.  The
LSM interface is not a protected interface that needs to be preserved,
if it is broken or doesn't meet our needs, we change it.  We've been
doing this for years, and regardless of what may come from this I
expect existing and future hooks will continue to change over time.
Predicting the future is impossible, agreeing to work together to deal
with the future as it comes is much easier.

> >   I would encourage the
> > individual LSMs (both maintainers and contributors) to work with patch
> > authors in a constructive manner to add support for new hooks when
> > they make sense for the given LSM.
>
> Yes.
>
> >> What do I want, I hear you cry? I want some sanity in the way
> >> LSM hooks are introduced. I want some standards or conventions
> >> on what is appropriate to pass into and out of LSM hooks. I
> >> want push back on special purpose hooks that are required to
> >> fix the deficiencies of a filesystem or bizarre hardware
> >> implementation. I want to stop spending all my time trying to
> >> deal with new, crazy LSM hooks. There are enough old ones to
> >> keep me entertained, thank you very much.
> > Yeah, good luck with that.
>
> Hey, if you don't play, you can't win. ;)
>
> > I think trying to legislate this too much
> > is a sure path to failure.
>
> There's not doubt that "too much" legislation would be a problem.
> Right now, we have *no* guidance or guidelines.

Sigh.  Be very careful what you wish for, we've got some pretty good
convention and established practices here, I would hate for your quest
for regulation to break what we've got going.  Especially when people
will use that regulation as a substitute for discussion.

> > Our saving grace is that the LSM hook
> > boundary is not a kernel/userspace boundary so we are free to change
> > it as needed, we just need to make sure if we change an existing hook
> > we don't break any of the existing in-tree LSMs.  We've done this in
> > the past without too much problem, even other subsystems have done
> > this for us (without much notification to the LSMs) and it has
> > generally worked out okay.
>
> Sure, but it's a royal pain to deal with the wild inconsistency
> of interfaces we have today.

We are writing hooks that deal with the entire system, there is
*always* going to be inconsistency; just look at the subsystems.
Further, just like the rest of the kernel, the hooks evolve over time
and often that evolution happens at different paces because the
different pieces of the kernel move at different paces.

> I would like to discourage new
> hooks that do things like expose filesystem or network protocol
> implementation details to the LSM interface.

Until something comes along and we need to break that abstraction.
Once again, if we're going to mandate anything, let's mandate that we
need to discuss changes to the LSM hooks and leave it at that.

> > I'm all for increased collaboration between LSMs, e.g. requiring hooks
> > changes to CC the LSM list, but I think mandating anything beyond that
> > is a fool's errand.
>
> VFS manages this, and I wouldn't call Al a fool.

You sir, are no Al Viro. :)  (and for the record, none of us are IMHO)

While the VFS subsystem may be the closest analogy to the LSM
subsystem, I think the two subsystems are quite different.  If nothing
else, the other kernel developers see the need for a working
filesystem and are willing to make compromises to keep their
filesystem working.  I think we can all name at least one subsystem
that would very much like to see the LSMs go away, and in order to
keep things functioning we sometimes have to get "creative", and that
can mean breaking abstractions.

-- 
paul moore
www.paul-moore.com