Re: [PATCH 0/7] Add CA enforcement keyring restrictions

[Mimi Zohar]

On Wed, 2022-04-06 at 22:53 +0000, Eric Snowberg wrote:
> 
> > On Apr 6, 2022, at 2:45 PM, Mimi Zohar <zohar@xxxxxxxxxxxxx> wrote:
> > 
> > Hi Eric,
> > 
> > On Tue, 2022-04-05 at 21:53 -0400, Eric Snowberg wrote:
> >> A key added to the ima keyring must be signed by a key contained within 
> >> either the builtin trusted or secondary trusted keyrings. Currently, there are 
> >> CA restrictions described in IMA_KEYRINGS_PERMIT_SIGNED_BY_BUILTIN_OR_SECONDARY,
> >> but these restrictions are not enforced within code. Therefore, keys within 
> >> either the builtin or secondary may not be a CA and could be used to
> >> vouch for an ima key.
> >> 
> >> The machine keyring can not be used as another trust anchor for adding keys 
> >> to the ima keyring, since CA enforcement does not currently exist [1]. This 
> >> would expand the current integrity gap.
> >> 
> >> Introduce a new root of trust key flag to close this integrity gap for
> >> all keyrings.  The first key type to use this is X.509.  When a X.509 
> >> certificate is self signed, contains kernCertSign Key Usage and contains 
> >> the CA bit, the new flag is set.  Introduce new keyring restrictions 
> >> that not only validates a key is signed by a key contained within the 
> >> keyring, but also validates the key has the new root of trust key flag 
> >> set.  Use this new restriction for keys added to the ima keyring.  Now 
> >> that we have CA enforcement, allow the machine keyring to be used as another 
> >> trust anchor for the ima keyring.
> >> 
> >> To recap, all keys that previously loaded into the builtin, secondary or
> >> machine keyring will still load after applying this series.  Keys
> >> contained within these keyrings may carry the root of trust flag. The
> >> ima keyring will use the new root of trust restriction to validate
> >> CA enforcement. Other keyrings that require a root of trust could also 
> >> use this in the future.
> > 
> > Your initial patch set indicated that you were addressing Linus'
> > request to allow end-users the ability "to add their own keys and sign
> > modules they trust".  However, from the design of the previous patch
> > set and now this one, everything indicates a lot more is going on than
> > just allowing end-users to add their own keys.  There would be no
> > reason for loading all the MOK keys, rather than just the CA keys, onto
> > the "machine" keyring.  Please provide the motivation for this design.
> 
> The motivation is to satisfy both Linus and your requests. Linus requested 
> the ability to allow users to add their own keys and sign modules they trust.  
> A code signing CA certificate does not require kernCertSign in the usage. Adding 
> this as a requirement for kernel modules would be a regression (or a bug).

Of course a code signing CA certificate should not also be a
certificate signing key (keyCertSign).  Remember the
"builtin_trusted_keys" and "secondary_trusted_keys" keyrings are
special.  Their root of trust is based on a secure boot signature chain
of trust up to and including a signed kernel image.  The "machine"
keyring is totally different in this regard.  Establishing a new root
of trust is really difficult.  Requiring a root-CA to have key
certifcate signing usage is a level of indirection, which I would
consider a small price to pay for being able to establish a, hopefully
safe or at least safer, new root of trust for trusting "end-user" keys.

> 
> This series addresses your request to only trust validly signed CA certs. 
> As you pointed out in the Kconfig help for 
> IMA_KEYRINGS_PERMIT_SIGNED_BY_BUILTIN_OR_SECONDARY:
> 
> help
>   Keys may be added to the IMA or IMA blacklist keyrings, if the
>   key is validly signed by a CA cert in the system built-in or
>   secondary trusted keyrings.
> 
>   Intermediate keys between those the kernel has compiled in and the 
>   IMA keys to be added may be added to the system secondary keyring,
>   provided they are validly signed by a key already resident in the
>   built-in or secondary trusted keyrings.
> 
> requires keys to be “validly” signed by a CA cert. Later the definition of a 
> validly signed CA cert was defined as: self signed, contains kernCertSign 
> key usage and contains the CA bit. While this help file states the CA restriction, 
> nothing in code enforces it.  One can place any type of self signed cert in either 
> keyring and ima will use it.  The motivation is for all keys added to the ima 
> keyring to abide by the restriction defined in the Kconfig help.  With this series 
> this can be accomplished without introducing a regression on keys placed in 
> any of the system keyrings.
> 
> > Please note that Patch 6/7 permits intermediary CA keys, without any
> > mention of it in the cover letter.  Please include this in the
> > motivation for this design.
> 
> Ok, I’ll add that in the next round.

Your cover letter should say that this patch series enables
verification of 3rd party modules.

thanks,

Mimi