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

[Petko Manolov]

On 16-01-12 17:08:44, David Howells wrote:
> Petko Manolov <petkan@xxxxxxxxxxxx> wrote:
> 
> > There is no need for .ima_mok if here is .mok, which should be system wide
> > keyring.  I'm trying to say that once we have .mok (you're more than welcome
> > to suggest better name) we'll get rid of .ima_mok.
> 
> If we really must have separate keyrings - and I still don't see that it's
> necessary - then maybe:
>
> 	.builtin_trusted_keys (RO)
> 	.secondarily_trusted_keys (RW).

Yep, that's the basic idea.

> I'd prefer to avoid "mok" as that might be misconstrued in a UEFI system.

It is a good idea to avoid confusion.

> > I still see value in immutable system keyring.  Being able to reboot to a
> > known state is only one of the reasons.
> 
> I'm not sure what you mean.  Changes to .system_keyring would not be 
> persistent across reboot.

Among other things it is nice to have a keyring with a known good state without 
the need to reboot.  You (the root user and only under certain circumstances) 
should always be able to zap the secondary keyring, but not the .system.

> > The other is the ultimate trust one should have in .system...
> 
> Do you have a use case where you would use an immutable set of keys 
> exclusively?

Yep.  Certain government structures will not use the machine in multi tenant 
mode so they'll settle with the vendor keys.

> > Can't we incorporate this functionality in .blacklist and avoid rebooting.
> 
> I think you misunderstood.  Once you've discarded a builtin keyring you cannot 
> get it back without rebooting (unless you hold another trusted key that signed 
> it).  Once you blacklist a builtin keyring you cannot get it back without 
> rebooting (unless you can remove things from a blacklist).

I do not get that part about blacklisting an entire keyring.  I hope we will be 
able to blacklist particular key, not the whole keyring.

> Clearing .system_keyring would be equivalent to blacklisting all the keys held 
> therein.  However, I presume you would have it that you cannot add to 
> .blacklist unless your bundle of keys to be blacklisted is appropriately 
> signed.
> 
> > > And why can't .system be a dynamic keyring?
> > 
> > Because this makes me uneasy.  What are we saving?  A few pages of memory?..
> 
> A key struct and some associative array metadata plus the cost of looking up
> in a second keyring.

Hopefully something that will be done only when importing new trusted key.

> I'm not sure why it makes you any more uneasy than having .ima_mok at all. 
> However, if it makes you able to sleep at night (;-)), and you're willing to 
> accept modification of the trust model along the lines of the patchset I 
> posted (which will need a couple of alterations) and move the new trust 
> keyring and blacklist keyring to the core, then okay, we can do that.

I'll sleep much better once i grasp your entire patch-set, although it will 
require some lack of sleep on my part. ;-)

I am also afraid these "couple of alternations" should be communicated to all 
stakeholders (Mimi, for one), especially if we decide to make .ima_mok to be 
system wide .secondarily_trusted_keys.  I volunteer to be excluded from the name 
giving competition. :-)

It is also not clear to me how we'll move keys to the blacklist keyring.  They 
should obviously be signed by CA in primary/secondary trusted keyring(s), the 
permissions should be set correctly, but i somehow feel this is not enough.

Mark, would you please comment on the above?

> > I don't mind linking in general as long as the permission check is 
> > supplementary to the keys CA hierarchy verification.
> 
> Which it currently isn't really.  As things stand, the CA hierarchy 
> verification takes place once at key creation and is assumed applicable to all 
> trusted keyrings thereafter.  KEY_FLAG_TRUSTED_ONLY was only really supposed 
> to apply to the system keyring.

I am not opposed to everything what you suggest.  Since we did that work in 
parallel (your stuff and the IMA keyring additions) with no communication 
between us, we ended up with broken IMA model.  I see three possibilities:

  - dump the IMA changes for this release (not happy about it);

  - try to quickly adapt the IMA system to your changes (not sure if it can be 
    done easily and/or quickly) and do it properly for 4.6;

  - elevate .ima_mok/blacklist to system wide RW keyrings (we may miss the merge 
    window);



		Petko