Re: [PATCH] X.509: Partially revert patch to add validation against IMA MOK keyring

[Mimi Zohar]

On Tue, 2016-01-12 at 13:55 +0000, David Howells wrote:
> Mimi Zohar <zohar@xxxxxxxxxxxxxxxxxx> wrote:
> 
> > >  (1) public_key_restrict_link() restricts to asymmetric keys that are
> > >      signed by a CA in the specified keyring.  It returns -ENOKEY if no
> > >      matching key is found rather than -EKEYREJECTED, however, so you can
> > >      call it several times for different keyrings.  -EKEYREJECTED is only
> > >      returned if a signature check fails.  This is used by the following
> > >      two functions.
> > 
> > Ok, so it is restricted it to CAs.  Combining it with an option to limit
> > it to the builtin CA keys based on the builtin flag would be nice.
> 
> Is there a point to the builtin flag if .system_keyring is closed?  Currently
> all keys that go into .system_keyring are marked BUILTIN.

It becomes a problem if the existing system keyring semantics of not
being writable change or if non builtin keys are added to the system
keyring.

Discussion continued below.

> But, yes, the restriction can include only using built in CAs.

> > >  (1) .system_keyring uses restrict_link_by_system_trusted() - though it
> > >      lacks any sort of write permission, so it's currently moot.  It could
> > >      just as well be replaced with a function that just returns -EPERM.
> > 
> > Why not retain the current semantics of the system keyring of not being
> > writable and create a new keyring for new feature(s)?
> 
> I think that the problem we have is that it can be argued either way.  You
> would rather create a new keyring to hold additional keys, whereas I would
> prefer to use the keyring we already have.
> 
> Do you have a technical reason why we can't just open the system keyring?
> It's not precisely a new feature, but rather an extension to an existing one
> that's been under consideration for a while.

There are two main concerns:
- Different keys are trusted for different things (eg. firmware,
modules, regular files) and at different levels of the OS.  I guess this
could be addressed, as you've previously suggested, by identifiers.

- The other issue is transitive trust (eg. A trusts B.  B trusts C.
Permit anything signed by C).  In some use case scenarios this is
desired, in others it is not.  We would need the option to limit trust
to just the builtin keys to support both use cases.

Even with the identifier support and ability to limit transitive trust,
I doubt it is as safe as limiting the system keyring to just the builtin
keys.   Refer to Petko's post.

> > The name "restrict_link_by_ima_mok()" doesn't reflect that it is either
> > the system keyring or the IMA MOK keyring.
> 
> How about restrict_link_by_ima_trusted()?

Good.  restrict_link_by_ima_trusted would only check the IMA MOK keyring
if it was configured.

Mimi