2022-01-05 23:53:04

by Eric Snowberg

[permalink] [raw]
Subject: [PATCH v9 0/8] Enroll kernel keys thru MOK

Back in 2013 Linus requested a feature to allow end-users to have the
ability "to add their own keys and sign modules they trust". This was
his *second* order outlined here [1]. There have been many attempts
over the years to solve this problem, all have been rejected. Many
of the failed attempts loaded all preboot firmware keys into the kernel,
including the Secure Boot keys. Many distributions carry one of these
rejected attempts [2], [3], [4]. This series tries to solve this problem
with a solution that takes into account all the problems brought up in
the previous attempts.

On UEFI based systems, this series introduces a new Linux kernel keyring
containing the Machine Owner Keys (MOK) called machine. It also defines
a new MOK variable in shim. This variable allows the end-user to decide
if they want to load MOK keys into the machine keyring.

By default, nothing changes; MOK keys are not loaded into the machine
keyring. They are only loaded after the end-user makes the decision
themselves. The end-user would set this through mokutil using a new
--trust-mok option [5]. This would work similar to how the kernel uses
MOK variables to enable/disable signature validation as well as use/ignore
the db. Any kernel operation that uses either the builtin or secondary
trusted keys as a trust source shall also reference the new machine
keyring as a trust source.

Secure Boot keys will never be loaded into the machine keyring. They
will always be loaded into the platform keyring. If an end-user wanted
to load one, they would need to enroll it into the MOK.

Unlike previous versions of this patch set, IMA support has been removed
to simplify the series. After acceptance, a follow-on series will add IMA
support.

Steps required by the end user:

Sign kernel module with user created key:
$ /usr/src/kernels/$(uname -r)/scripts/sign-file sha512 \
machine_signing_key.priv machine_signing_key.x509 my_module.ko

Import the key into the MOK
$ mokutil --import machine_signing_key.x509

Setup the kernel to load MOK keys into the .machine keyring
$ mokutil --trust-mok

Then reboot, the MokManager will load and ask if you want to trust the
MOK key and enroll the MOK into the MOKList. Afterwards the signed kernel
module will load.

I have included a link to the mokutil [5] changes I have made to support
this new functionality. The shim changes have now been accepted
upstream [6].

Upstream shim is located here [7], the build instructions are here [8].
TLDR:

$ git clone --recurse-submodules https://github.com/rhboot/shim
$ cd shim
$ make

After building shim, move shimx64.efi and mmx64.efi to the vendor or
distribution specific directory on your EFI System Partition (assuming
you are building on x86). The instructions above are the minimal
steps needed to build shim to test this feature. It is assumed
Secure Boot shall not be enabled for this testing. To do testing
with Secure Boot enabled, all steps in the build instructions [8]
must be followed.

Instructions for building mokutil (including the new changes):

$ git clone -b mokvars-v3 https://github.com/esnowberg/mokutil.git
$ cd mokutil/
$ ./autogen.sh
$ make

[1] https://marc.info/?l=linux-kernel&m=136185386310140&w=2
[2] https://lore.kernel.org/lkml/[email protected]/
[3] https://lore.kernel.org/lkml/[email protected]/
[4] https://lore.kernel.org/linux-integrity/1e41f22b1f11784f1e943f32bf62034d4e054cdb.camel@HansenPartnership.com/
[5] https://github.com/esnowberg/mokutil/tree/mokvars-v3
[6] https://github.com/rhboot/shim/commit/4e513405b4f1641710115780d19dcec130c5208f
[7] https://github.com/rhboot/shim
[8] https://github.com/rhboot/shim/blob/main/BUILDING

Eric Snowberg (8):
integrity: Fix warning about missing prototypes
integrity: Introduce a Linux keyring called machine
integrity: add new keyring handler for mok keys
KEYS: store reference to machine keyring
KEYS: Introduce link restriction for machine keys
efi/mokvar: move up init order
integrity: Trust MOK keys if MokListTrustedRT found
integrity: Only use machine keyring when uefi_check_trust_mok_keys is
true

certs/system_keyring.c | 44 ++++++++++-
drivers/firmware/efi/mokvar-table.c | 2 +-
include/keys/system_keyring.h | 14 ++++
security/integrity/Kconfig | 13 ++++
security/integrity/Makefile | 1 +
security/integrity/digsig.c | 15 +++-
security/integrity/integrity.h | 17 +++-
.../platform_certs/keyring_handler.c | 18 ++++-
.../platform_certs/keyring_handler.h | 5 ++
security/integrity/platform_certs/load_uefi.c | 4 +-
.../platform_certs/machine_keyring.c | 77 +++++++++++++++++++
11 files changed, 202 insertions(+), 8 deletions(-)
create mode 100644 security/integrity/platform_certs/machine_keyring.c


base-commit: c9e6606c7fe92b50a02ce51dda82586ebdf99b48
--
2.18.4



2022-01-05 23:53:17

by Eric Snowberg

[permalink] [raw]
Subject: [PATCH v9 3/8] integrity: add new keyring handler for mok keys

Currently both Secure Boot DB and Machine Owner Keys (MOK) go through
the same keyring handler (get_handler_for_db). With the addition of the
new machine keyring, the end-user may choose to trust MOK keys.

Introduce a new keyring handler specific for MOK keys. If MOK keys are
trusted by the end-user, use the new keyring handler instead.

Signed-off-by: Eric Snowberg <[email protected]>
Reviewed-by: Mimi Zohar <[email protected]>
---
v1: Initial version
v3: Only change the keyring handler if the secondary is enabled
v4: Removed trust_moklist check
v5: Rename to machine keyring
v7: Unmodified from v5
v8: Code unmodified from v7 added Mimi's Reviewed-by
v9: Unmodified from v8
---
.../integrity/platform_certs/keyring_handler.c | 17 ++++++++++++++++-
.../integrity/platform_certs/keyring_handler.h | 5 +++++
security/integrity/platform_certs/load_uefi.c | 4 ++--
3 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/security/integrity/platform_certs/keyring_handler.c b/security/integrity/platform_certs/keyring_handler.c
index e9791be98fd9..4872850d081f 100644
--- a/security/integrity/platform_certs/keyring_handler.c
+++ b/security/integrity/platform_certs/keyring_handler.c
@@ -67,7 +67,7 @@ static __init void uefi_revocation_list_x509(const char *source,

/*
* Return the appropriate handler for particular signature list types found in
- * the UEFI db and MokListRT tables.
+ * the UEFI db tables.
*/
__init efi_element_handler_t get_handler_for_db(const efi_guid_t *sig_type)
{
@@ -76,6 +76,21 @@ __init efi_element_handler_t get_handler_for_db(const efi_guid_t *sig_type)
return 0;
}

+/*
+ * Return the appropriate handler for particular signature list types found in
+ * the MokListRT tables.
+ */
+__init efi_element_handler_t get_handler_for_mok(const efi_guid_t *sig_type)
+{
+ if (efi_guidcmp(*sig_type, efi_cert_x509_guid) == 0) {
+ if (IS_ENABLED(CONFIG_INTEGRITY_MACHINE_KEYRING))
+ return add_to_machine_keyring;
+ else
+ return add_to_platform_keyring;
+ }
+ return 0;
+}
+
/*
* Return the appropriate handler for particular signature list types found in
* the UEFI dbx and MokListXRT tables.
diff --git a/security/integrity/platform_certs/keyring_handler.h b/security/integrity/platform_certs/keyring_handler.h
index 2462bfa08fe3..284558f30411 100644
--- a/security/integrity/platform_certs/keyring_handler.h
+++ b/security/integrity/platform_certs/keyring_handler.h
@@ -24,6 +24,11 @@ void blacklist_binary(const char *source, const void *data, size_t len);
*/
efi_element_handler_t get_handler_for_db(const efi_guid_t *sig_type);

+/*
+ * Return the handler for particular signature list types found in the mok.
+ */
+efi_element_handler_t get_handler_for_mok(const efi_guid_t *sig_type);
+
/*
* Return the handler for particular signature list types found in the dbx.
*/
diff --git a/security/integrity/platform_certs/load_uefi.c b/security/integrity/platform_certs/load_uefi.c
index f290f78c3f30..c1bfd1cd7cc3 100644
--- a/security/integrity/platform_certs/load_uefi.c
+++ b/security/integrity/platform_certs/load_uefi.c
@@ -94,7 +94,7 @@ static int __init load_moklist_certs(void)
rc = parse_efi_signature_list("UEFI:MokListRT (MOKvar table)",
mokvar_entry->data,
mokvar_entry->data_size,
- get_handler_for_db);
+ get_handler_for_mok);
/* All done if that worked. */
if (!rc)
return rc;
@@ -109,7 +109,7 @@ static int __init load_moklist_certs(void)
mok = get_cert_list(L"MokListRT", &mok_var, &moksize, &status);
if (mok) {
rc = parse_efi_signature_list("UEFI:MokListRT",
- mok, moksize, get_handler_for_db);
+ mok, moksize, get_handler_for_mok);
kfree(mok);
if (rc)
pr_err("Couldn't parse MokListRT signatures: %d\n", rc);
--
2.18.4


2022-01-08 22:21:46

by Jarkko Sakkinen

[permalink] [raw]
Subject: Re: [PATCH v9 3/8] integrity: add new keyring handler for mok keys

On Wed, Jan 05, 2022 at 06:50:07PM -0500, Eric Snowberg wrote:
> Currently both Secure Boot DB and Machine Owner Keys (MOK) go through
> the same keyring handler (get_handler_for_db). With the addition of the
> new machine keyring, the end-user may choose to trust MOK keys.
>
> Introduce a new keyring handler specific for MOK keys. If MOK keys are
> trusted by the end-user, use the new keyring handler instead.
>
> Signed-off-by: Eric Snowberg <[email protected]>
> Reviewed-by: Mimi Zohar <[email protected]>
> ---
> v1: Initial version
> v3: Only change the keyring handler if the secondary is enabled
> v4: Removed trust_moklist check
> v5: Rename to machine keyring
> v7: Unmodified from v5
> v8: Code unmodified from v7 added Mimi's Reviewed-by
> v9: Unmodified from v8
> ---
> .../integrity/platform_certs/keyring_handler.c | 17 ++++++++++++++++-
> .../integrity/platform_certs/keyring_handler.h | 5 +++++
> security/integrity/platform_certs/load_uefi.c | 4 ++--
> 3 files changed, 23 insertions(+), 3 deletions(-)
>
> diff --git a/security/integrity/platform_certs/keyring_handler.c b/security/integrity/platform_certs/keyring_handler.c
> index e9791be98fd9..4872850d081f 100644
> --- a/security/integrity/platform_certs/keyring_handler.c
> +++ b/security/integrity/platform_certs/keyring_handler.c
> @@ -67,7 +67,7 @@ static __init void uefi_revocation_list_x509(const char *source,
>
> /*
> * Return the appropriate handler for particular signature list types found in
> - * the UEFI db and MokListRT tables.
> + * the UEFI db tables.
> */
> __init efi_element_handler_t get_handler_for_db(const efi_guid_t *sig_type)
> {
> @@ -76,6 +76,21 @@ __init efi_element_handler_t get_handler_for_db(const efi_guid_t *sig_type)
> return 0;
> }
>
> +/*
> + * Return the appropriate handler for particular signature list types found in
> + * the MokListRT tables.
> + */
> +__init efi_element_handler_t get_handler_for_mok(const efi_guid_t *sig_type)
> +{
> + if (efi_guidcmp(*sig_type, efi_cert_x509_guid) == 0) {
> + if (IS_ENABLED(CONFIG_INTEGRITY_MACHINE_KEYRING))
> + return add_to_machine_keyring;
> + else
> + return add_to_platform_keyring;
> + }
> + return 0;
> +}
> +
> /*
> * Return the appropriate handler for particular signature list types found in
> * the UEFI dbx and MokListXRT tables.
> diff --git a/security/integrity/platform_certs/keyring_handler.h b/security/integrity/platform_certs/keyring_handler.h
> index 2462bfa08fe3..284558f30411 100644
> --- a/security/integrity/platform_certs/keyring_handler.h
> +++ b/security/integrity/platform_certs/keyring_handler.h
> @@ -24,6 +24,11 @@ void blacklist_binary(const char *source, const void *data, size_t len);
> */
> efi_element_handler_t get_handler_for_db(const efi_guid_t *sig_type);
>
> +/*
> + * Return the handler for particular signature list types found in the mok.
> + */
> +efi_element_handler_t get_handler_for_mok(const efi_guid_t *sig_type);
> +
> /*
> * Return the handler for particular signature list types found in the dbx.
> */
> diff --git a/security/integrity/platform_certs/load_uefi.c b/security/integrity/platform_certs/load_uefi.c
> index f290f78c3f30..c1bfd1cd7cc3 100644
> --- a/security/integrity/platform_certs/load_uefi.c
> +++ b/security/integrity/platform_certs/load_uefi.c
> @@ -94,7 +94,7 @@ static int __init load_moklist_certs(void)
> rc = parse_efi_signature_list("UEFI:MokListRT (MOKvar table)",
> mokvar_entry->data,
> mokvar_entry->data_size,
> - get_handler_for_db);
> + get_handler_for_mok);
> /* All done if that worked. */
> if (!rc)
> return rc;
> @@ -109,7 +109,7 @@ static int __init load_moklist_certs(void)
> mok = get_cert_list(L"MokListRT", &mok_var, &moksize, &status);
> if (mok) {
> rc = parse_efi_signature_list("UEFI:MokListRT",
> - mok, moksize, get_handler_for_db);
> + mok, moksize, get_handler_for_mok);
> kfree(mok);
> if (rc)
> pr_err("Couldn't parse MokListRT signatures: %d\n", rc);
> --
> 2.18.4
>

Reviewed-by: Jarkko Sakkinen <[email protected]>

/Jarkko