Re: [PATCH 4/7] KEYS: Introduce a builtin root of trust key flag

[Mimi Zohar]

On Fri, 2022-04-08 at 17:34 +0000, Eric Snowberg wrote:
> 
> > On Apr 8, 2022, at 10:55 AM, Mimi Zohar <zohar@xxxxxxxxxxxxx> wrote:
> > 
> > On Fri, 2022-04-08 at 15:27 +0000, Eric Snowberg wrote:
> >> 
> >>> On Apr 8, 2022, at 8:40 AM, Mimi Zohar <zohar@xxxxxxxxxxxxx> wrote:
> >>> 
> >>> On Tue, 2022-04-05 at 21:53 -0400, Eric Snowberg wrote:
> >>>> 
> >>>> The first type of key to use this is X.509.  When a X.509 certificate
> >>>> is self signed, has the kernCertSign Key Usage set and contains the
> >>>> CA bit set this new flag is set.
> >>>> 
> >>>> Signed-off-by: Eric Snowberg <eric.snowberg@xxxxxxxxxx>
> >>>> 
> >>>> diff --git a/include/linux/key.h b/include/linux/key.h
> >>>> index 7febc4881363..97f6a1f86a27 100644
> >>>> --- a/include/linux/key.h
> >>>> +++ b/include/linux/key.h
> >>>> @@ -230,6 +230,7 @@ struct key {
> >>>> #define KEY_FLAG_ROOT_CAN_INVAL	7	/* set if key can be invalidated by root without permission */
> >>>> #define KEY_FLAG_KEEP		8	/* set if key should not be removed */
> >>>> #define KEY_FLAG_UID_KEYRING	9	/* set if key is a user or user session keyring */
> >>>> +#define KEY_FLAG_BUILTIN_ROT	10	/* set if key is a builtin Root of Trust key */
> >>>> 
> >>>> 	/* the key type and key description string
> >>>> 	 * - the desc is used to match a key against search criteria
> >>>> @@ -290,6 +291,7 @@ extern struct key *key_alloc(struct key_type *type,
> >>>> #define KEY_ALLOC_BYPASS_RESTRICTION	0x0008	/* Override the check on restricted keyrings */
> >>>> #define KEY_ALLOC_UID_KEYRING		0x0010	/* allocating a user or user session keyring */
> >>>> #define KEY_ALLOC_SET_KEEP		0x0020	/* Set the KEEP flag on the key/keyring */
> >>>> +#define KEY_ALLOC_BUILT_IN_ROT		0x0040  /* Add builtin root of trust key */
> >>> 
> >>> Since the concept of root of trust is not generic, but limited to
> >>> specific keyrings, the root CA certificate signing keys on the
> >>> "machine" keyring need to be identified.  Similar to the
> >>> KEY_ALLOC_BUILT_IN/KEY_FLAG_BUILTIN, new flags
> >>> KEY_ALLOC_MACHINE/KEY_FLAG_MACHINE should be defined instead.
> >> 
> >> I’m open to renaming these, however this name change seems confusing to me.  
> >> This flag gets set when the X.509 certificate contains the three CA requirements 
> >> identified above.  The remaining keys in the machine keyring can be used for 
> >> anything else.
> > 
> > Renaming the flag to KEY_ALLOC_MACHINE/KEY_FLAG_MACHINE differentiates
> > between the "builtin" keys from the "machine" keys.  The trust models
> > are very different.
> 
> Isn’t the trust model the same for machine and secondary keys?  Both are supplied by 
> the end-user. That is why I’m confused by naming something _MACHINE when it applies 
> to more than one keyring.

True both are supplied by the end-user, but the trust models are
different.  In one case the certificates are coming indirectly from
firmware, while in the other case the certificates would be limited to
certificates signed by the initial firmware certificates.  Loading only
root-CA signing key certificates onto the "machine" keyring highlights
and enforces the different types of trust.

> 
> >> Plus this flag can be set for keys loaded into the secondary trusted 
> >> keyring (6th patch in the series).  When an intermediate CA gets loaded into the 
> >> secondary, the flag is set as well.
> > 
> > Please include a full explanation with the motivation in the patch
> > description as to why support for intermediary CAs is required for the
> > "end-user" use case.
> 
> Ok, I can add it.  I thought this was an expectation, based on the help section of
> IMA_KEYRINGS_PERMIT_SIGNED_BY_BUILTIN_OR_SECONDARY:
> 
> " 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."

This paragraph refers to keys on the "builtin_trusted_keys" keyring. 
The concept would need to be expanded to include keys on the "machine"
keyring.   Since support for intermediary CA keys isn't required for
the simple "end-user" use case, the motivation  needs to be provided.

thanks,

Mimi