2022-01-05 23:52:28

by Eric Snowberg

[permalink] [raw]
Subject: [PATCH v9 5/8] KEYS: Introduce link restriction for machine keys

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 <[email protected]>
Signed-off-by: Eric Snowberg <[email protected]>
---
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



2022-01-08 22:25:43

by Jarkko Sakkinen

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

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 <[email protected]>
> Signed-off-by: Eric Snowberg <[email protected]>
> ---
> 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 <[email protected]>

/Jarkko

2022-01-09 22:42:54

by Mimi Zohar

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

On Wed, 2022-01-05 at 18:50 -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.

As suggested the Kconfig in "[PATCH v9 2/8] integrity: Introduce a
Linux keyring called machine" only loads the platform keys onto the
.machine keyring, when
IMA_KEYRINGS_PERMIT_SIGNED_BY_BUILTIN_OR_SECONDARY is not enabled. The
last sentence needs to be updated to reflect v9.

thanks,

Mimi


2022-01-10 23:36:49

by Eric Snowberg

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



> On Jan 9, 2022, at 3:42 PM, Mimi Zohar <[email protected]> wrote:
>
> On Wed, 2022-01-05 at 18:50 -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.
>
> As suggested the Kconfig in "[PATCH v9 2/8] integrity: Introduce a
> Linux keyring called machine" only loads the platform keys onto the
> .machine keyring, when
> IMA_KEYRINGS_PERMIT_SIGNED_BY_BUILTIN_OR_SECONDARY is not enabled. The
> last sentence needs to be updated to reflect v9.

I missed that when I dropped IMA support. I will remove the ima reference in the next
round. Thanks.