On Fri May 31, 2024 at 3:39 AM EEST, Eric Snowberg wrote:
> Introduce a new function to allow a keyring to link to a key contained
> within one of the system keyrings (builtin, secondary, or platform).
"Introduce system_key_link(), a new function..."
I hate when the exact thing added is not immediately transparent from
the commit message ;-) Helps a lot when bisecting for instance.
> Depending on how the kernel is built, if the machine keyring is
> available, it will be checked as well, since it is linked to the secondary
> keyring. If the asymmetric key id matches a key within one of these
> system keyrings, the matching key is linked into the passed in
> keyring.
>
> Signed-off-by: Eric Snowberg <eric.snowberg@xxxxxxxxxx>
> ---
> certs/system_keyring.c | 31 +++++++++++++++++++++++++++++++
> include/keys/system_keyring.h | 7 ++++++-
> 2 files changed, 37 insertions(+), 1 deletion(-)
>
> diff --git a/certs/system_keyring.c b/certs/system_keyring.c
> index 9de610bf1f4b..94e47b6b3333 100644
> --- a/certs/system_keyring.c
> +++ b/certs/system_keyring.c
> @@ -426,3 +426,34 @@ void __init set_platform_trusted_keys(struct key *keyring)
> platform_trusted_keys = keyring;
> }
> #endif
> +
> +/**
> + * system_key_link - Link to a system key
"system_key_link() - Link to a system key"
> + * @keyring: The keyring to link into
> + * @id: The asymmetric key id to look for in the system keyring
> + */
Really could use some overview keyrings traversed just as a reminder.
> +int system_key_link(struct key *keyring, struct asymmetric_key_id *id)
> +{
> + struct key *system_keyring;
> + struct key *key;
> +
> +#ifdef CONFIG_SECONDARY_TRUSTED_KEYRING
> + system_keyring = secondary_trusted_keys;
> +#else
> + system_keyring = builtin_trusted_keys;
> +#endif
Why not simply make secondary_trusted_keys in the first place be alias
to builtin_trusted_keys when it is not enabled?
> +
> + key = find_asymmetric_key(system_keyring, id, NULL, NULL, false);
> + if (!IS_ERR(key))
> + goto found;
> +
> + key = find_asymmetric_key(platform_trusted_keys, id, NULL, NULL, false);
> + if (!IS_ERR(key))
> + goto found;
> +
> + return -ENOKEY;
> +
> +found:
"link:"?
Then you could see already from goto statement what will happen next
(your call anyway).
> + key_link(keyring, key);
> + return 0;
> +}
> diff --git a/include/keys/system_keyring.h b/include/keys/system_keyring.h
> index 8365adf842ef..b47ac8e2001a 100644
> --- a/include/keys/system_keyring.h
> +++ b/include/keys/system_keyring.h
> @@ -9,6 +9,7 @@
> #define _KEYS_SYSTEM_KEYRING_H
>
> #include <linux/key.h>
> +struct asymmetric_key_id;
>
> enum blacklist_hash_type {
> /* TBSCertificate hash */
> @@ -28,7 +29,7 @@ int restrict_link_by_digsig_builtin(struct key *dest_keyring,
> const union key_payload *payload,
> struct key *restriction_key);
> extern __init int load_module_cert(struct key *keyring);
> -
> +extern int system_key_link(struct key *keyring, struct asymmetric_key_id *id);
> #else
> #define restrict_link_by_builtin_trusted restrict_link_reject
> #define restrict_link_by_digsig_builtin restrict_link_reject
> @@ -38,6 +39,10 @@ static inline __init int load_module_cert(struct key *keyring)
> return 0;
> }
>
> +static inline int system_key_link(struct key *keyring, struct asymmetric_key_id *id)
> +{
> + return 0;
> +}
> #endif
>
> #ifdef CONFIG_SECONDARY_TRUSTED_KEYRING
BR, Jarkko
