Re: Preview of changes to the Security susbystem for 2.6.36

From: Valdis . Kletnieks
Date: Mon Aug 02 2010 - 14:52:30 EST


On Mon, 02 Aug 2010 09:59:36 PDT, Kees Cook said:

> > Al gave you some very clear advice how a the sticky check should be
>
> This is patently false. "Very clear advice" would have included actionable
> instructions. He (and everyone else) has ignored my requests for
> clarification[2]. If you see how the check should be implemented, please
> send a patch demonstrating how. I would greatly prefer having these
> protections in the VFS itself.

You're overlooking step zero of Al's advice: First, *think* about the issue
in a deep fashion, rather than a knee-jerk patch to fix one instance of
the problem.

> > done for symlinks (if we need it at all, which I tend to disagree with),
>
> I can see how one might disagree with it, but anyone who handles day-to-day
> security threats understands this protection to be a clear and obvious
> solution for the past decade.

The problem is that although your patch closes *one set* of symlink attacks
that has been traditionally a problem, it doesn't do a very good job of
creating a conceptual model and then *really* dealing with the issue. That's
the big distinction between SELinux, Tomoyo, Smack, and your proposal - they
form a *model* of what's important to protect, and what actions need to be
taken to *actually* protect them. They don't just apply one arbitrary rule
that closes some attacks - they make an honest effort to deal with all variants
of the attack, and other attacks that allow bypass, and so on.

The reason people are worried that this might grow into a "large" LSM is that
quite often, throwing in a bunch of ad-hoc rules may create *apparent*
security, but not provide any *real* security. You yourself admit that Yama
only closes one set of symlink attacks without addressing the general issue of
symlinks, hard links, TOCTOU races, and a lot of *other* similar "the file you
actually opened is not the one you intended to open" attacks. And the reason it
doesn't address the general issue is because it lacks a security model. And
the reason you're having so much trouble getting it into the tree is because if
you're going to apply this at either the VFS or LSM layers, you need to address
the *general* problem and not one ad-hoc variant of it.

And quite frankly, the idea of this morphing into a "large" LSM containing a
lot of ad-hoc rules scares most security people, because without a good
conceptual model, it's hard to define if the security is in fact working, or
what the problem is if it isn't working.

> > and the ptrace check completely breaks crash handlers that we have
> > in all kinds of applications. If you can get it into the main ptrace
>
> I've seen two so far. Both are addressed with a one line fix. And I would
> stress that no other existing subsystem in the kernel can provide the same
> level of control that my ptrace exception logic provides. SELinux cannot do
> this.

Quick question: Now is that "SELinux doesn't consider the added granularity
important and doesn't bother doing it", or "SELinux can't do it *currently*",
or "there are innate structural reasons why SELinux is by design unable to do
it"? Note that it's a big difference, and it's dangerous for your cause to
bring it up without understanding which it is, and why...

> This advice is precisely counter to prior advise. It would seem that no one
> knows how to handle these patches. I find it very simple; either:
> - let me put them in an LSM that people can choose to enable
> - help me get the patches into shape for the subsystems directly

You're still missing the point:

> [2] http://lkml.org/lkml/2010/6/4/44

You were told to go back and form an actual *security model*. What's important
to protect? What attacks can be made against it? What syscalls are included in
the forseeable attacks (hint - probably more than you think - if you're
mediating symlink access, a bit of thought will show symlinks aren't the only
problem you need to worry about to *actually* secure the resource).


Attachment: pgp00000.pgp
Description: PGP signature