Re: [PATCH v9 5/8] KEYS: Introduce link restriction for machine keys

[Jarkko Sakkinen]

On Wed, Jan 05, 2022 at 06:50:09PM -0500, Eric Snowberg wrote:
> Introduce a new link restriction that includes the trusted builtin,
> secondary and machine keys. The restriction is based on the key to be
> added being vouched for by a key in any of these three keyrings.
> 
> With the introduction of the machine keyring, the end-user may choose to
> trust Machine Owner Keys (MOK) within the kernel. If they have chosen to
> trust them, the .machine keyring will contain these keys.  If not, the
> machine keyring will always be empty.  Update the restriction check to
> allow the secondary trusted keyring and ima keyring to also trust
> machine keys.
> 
> Allow the .machine keyring to be linked to the secondary_trusted_keys.
> After the link is created, keys contained in the .machine keyring will
> automatically be searched when searching secondary_trusted_keys.
> 
> Suggested-by: Mimi Zohar <zohar@xxxxxxxxxxxxx>
> Signed-off-by: Eric Snowberg <eric.snowberg@xxxxxxxxxx>
> ---
> v3: Initial version
> v4: moved code under CONFIG_INTEGRITY_MOK_KEYRING
> v5: Rename to machine keyring
> v6: Change subject name (suggested by Mimi)
>     Rename restrict_link_by_builtin_secondary_and_ca_trusted
>       to restrict_link_by_builtin_secondary_and_machine (suggested by
>       Mimi)
> v7: Unmodified from v6
> v8: Add missing parameter definitions (suggested by Mimi)
> v9: Combine with "change link restriction to trust the machine keyring"
>       patch
> ---
>  certs/system_keyring.c        | 35 ++++++++++++++++++++++++++++++++++-
>  include/keys/system_keyring.h |  6 ++++++
>  2 files changed, 40 insertions(+), 1 deletion(-)
> 
> diff --git a/certs/system_keyring.c b/certs/system_keyring.c
> index 08ea542c8096..05b66ce9d1c9 100644
> --- a/certs/system_keyring.c
> +++ b/certs/system_keyring.c
> @@ -89,7 +89,10 @@ static __init struct key_restriction *get_builtin_and_secondary_restriction(void
>  	if (!restriction)
>  		panic("Can't allocate secondary trusted keyring restriction\n");
>  
> -	restriction->check = restrict_link_by_builtin_and_secondary_trusted;
> +	if (IS_ENABLED(CONFIG_INTEGRITY_MACHINE_KEYRING))
> +		restriction->check = restrict_link_by_builtin_secondary_and_machine;
> +	else
> +		restriction->check = restrict_link_by_builtin_and_secondary_trusted;
>  
>  	return restriction;
>  }
> @@ -98,6 +101,36 @@ static __init struct key_restriction *get_builtin_and_secondary_restriction(void
>  void __init set_machine_trusted_keys(struct key *keyring)
>  {
>  	machine_trusted_keys = keyring;
> +
> +	if (key_link(secondary_trusted_keys, machine_trusted_keys) < 0)
> +		panic("Can't link (machine) trusted keyrings\n");
> +}
> +
> +/**
> + * restrict_link_by_builtin_secondary_and_machine - Restrict keyring addition.
> + * @dest_keyring: Keyring being linked to.
> + * @type: The type of key being added.
> + * @payload: The payload of the new key.
> + * @restrict_key: A ring of keys that can be used to vouch for the new cert.
> + *
> + * Restrict the addition of keys into a keyring based on the key-to-be-added
> + * being vouched for by a key in either the built-in, the secondary, or
> + * the machine keyrings.
> + */
> +int restrict_link_by_builtin_secondary_and_machine(
> +	struct key *dest_keyring,
> +	const struct key_type *type,
> +	const union key_payload *payload,
> +	struct key *restrict_key)
> +{
> +	if (machine_trusted_keys && type == &key_type_keyring &&
> +	    dest_keyring == secondary_trusted_keys &&
> +	    payload == &machine_trusted_keys->payload)
> +		/* Allow the machine keyring to be added to the secondary */
> +		return 0;
> +
> +	return restrict_link_by_builtin_and_secondary_trusted(dest_keyring, type,
> +							      payload, restrict_key);
>  }
>  #endif
>  
> diff --git a/include/keys/system_keyring.h b/include/keys/system_keyring.h
> index 98c9b10cdc17..2419a735420f 100644
> --- a/include/keys/system_keyring.h
> +++ b/include/keys/system_keyring.h
> @@ -39,8 +39,14 @@ extern int restrict_link_by_builtin_and_secondary_trusted(
>  #endif
>  
>  #ifdef CONFIG_INTEGRITY_MACHINE_KEYRING
> +extern int restrict_link_by_builtin_secondary_and_machine(
> +	struct key *dest_keyring,
> +	const struct key_type *type,
> +	const union key_payload *payload,
> +	struct key *restrict_key);
>  extern void __init set_machine_trusted_keys(struct key *keyring);
>  #else
> +#define restrict_link_by_builtin_secondary_and_machine restrict_link_by_builtin_trusted
>  static inline void __init set_machine_trusted_keys(struct key *keyring)
>  {
>  }
> -- 
> 2.18.4
> 

Reviewed-by: Jarkko Sakkinen <jarkko@xxxxxxxxxx>

/Jarkko