On Thu, 2021-08-12 at 16:36 -0600, Eric Snowberg wrote:
> > On Aug 12, 2021, at 3:31 PM, Mimi Zohar <zohar@xxxxxxxxxxxxx> wrote:
> >
> > On Wed, 2021-08-11 at 22:18 -0400, Eric Snowberg wrote:
> >> Many UEFI Linux distributions boot using shim. The UEFI shim provides
> >> what is called Machine Owner Keys (MOK). Shim uses both the UEFI Secure
> >> Boot DB and MOK keys to validate the next step in the boot chain. The
> >> MOK facility can be used to import user generated keys. These keys can
> >> be used to sign an end-users development kernel build. When Linux
> >> boots, both UEFI Secure Boot DB and MOK keys get loaded in the Linux
> >> .platform keyring.
> >>
> >> Add a new Linux keyring called .mok. This keyring shall contain just
> >> MOK keys and not the remaining keys in the platform keyring. This new
> >> .mok keyring will be used in follow on patches. Unlike keys in the
> >> platform keyring, keys contained in the .mok keyring will be trusted
> >> within the kernel if the end-user has chosen to do so.
> >>
> >> Signed-off-by: Eric Snowberg <eric.snowberg@xxxxxxxxxx>
> >> ---
> >> v1: Initial version
> >> v2: Removed destory keyring code
> >> v3: Unmodified from v2
> >> ---
> >> security/integrity/Makefile | 3 ++-
> >> security/integrity/digsig.c | 1 +
> >> security/integrity/integrity.h | 3 ++-
> >> .../integrity/platform_certs/mok_keyring.c | 21 +++++++++++++++++++
> >> 4 files changed, 26 insertions(+), 2 deletions(-)
> >> create mode 100644 security/integrity/platform_certs/mok_keyring.c
> >>
> >> diff --git a/security/integrity/Makefile b/security/integrity/Makefile
> >> index 7ee39d66cf16..8e2e98cba1f6 100644
> >> --- a/security/integrity/Makefile
> >> +++ b/security/integrity/Makefile
> >> @@ -9,7 +9,8 @@ integrity-y := iint.o
> >> integrity-$(CONFIG_INTEGRITY_AUDIT) += integrity_audit.o
> >> integrity-$(CONFIG_INTEGRITY_SIGNATURE) += digsig.o
> >> integrity-$(CONFIG_INTEGRITY_ASYMMETRIC_KEYS) += digsig_asymmetric.o
> >> -integrity-$(CONFIG_INTEGRITY_PLATFORM_KEYRING) += platform_certs/platform_keyring.o
> >> +integrity-$(CONFIG_INTEGRITY_PLATFORM_KEYRING) += platform_certs/platform_keyring.o \
> >> + platform_certs/mok_keyring.o
> >> integrity-$(CONFIG_LOAD_UEFI_KEYS) += platform_certs/efi_parser.o \
> >> platform_certs/load_uefi.o \
> >> platform_certs/keyring_handler.o
> >> diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
> >> index 3b06a01bd0fd..e07334504ef1 100644
> >> --- a/security/integrity/digsig.c
> >> +++ b/security/integrity/digsig.c
> >> @@ -30,6 +30,7 @@ static const char * const keyring_name[INTEGRITY_KEYRING_MAX] = {
> >> ".ima",
> >> #endif
> >> ".platform",
> >> + ".mok",
> >> };
> >>
> >> #ifdef CONFIG_IMA_KEYRINGS_PERMIT_SIGNED_BY_BUILTIN_OR_SECONDARY
> >> diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
> >> index 547425c20e11..e0e17ccba2e6 100644
> >> --- a/security/integrity/integrity.h
> >> +++ b/security/integrity/integrity.h
> >> @@ -151,7 +151,8 @@ int integrity_kernel_read(struct file *file, loff_t offset,
> >> #define INTEGRITY_KEYRING_EVM 0
> >> #define INTEGRITY_KEYRING_IMA 1
> >> #define INTEGRITY_KEYRING_PLATFORM 2
> >> -#define INTEGRITY_KEYRING_MAX 3
> >> +#define INTEGRITY_KEYRING_MOK 3
> >> +#define INTEGRITY_KEYRING_MAX 4
> >>
> >> extern struct dentry *integrity_dir;
> >>
> >> diff --git a/security/integrity/platform_certs/mok_keyring.c b/security/integrity/platform_certs/mok_keyring.c
> >> new file mode 100644
> >> index 000000000000..b1ee45b77731
> >> --- /dev/null
> >> +++ b/security/integrity/platform_certs/mok_keyring.c
> >> @@ -0,0 +1,21 @@
> >> +// SPDX-License-Identifier: GPL-2.0
> >> +/*
> >> + * MOK keyring routines.
> >> + *
> >> + * Copyright (c) 2021, Oracle and/or its affiliates.
> >> + */
> >> +
> >> +#include "../integrity.h"
> >> +
> >> +static __init int mok_keyring_init(void)
> >> +{
> >> + int rc;
> >> +
> >> + rc = integrity_init_keyring(INTEGRITY_KEYRING_MOK);
> >> + if (rc)
> >> + return rc;
> >> +
> >> + pr_notice("MOK Keyring initialized\n");
> >> + return 0;
> >> +}
> >> +device_initcall(mok_keyring_init);
> >
> > The ordering of the patches in this patch set is not quite
> > right.
>
> I will work on reordering the patches in the next round.
>
> > Please first introduce the new keyring with the new Kconfig,
> > new restriction, and loading the keys onto the new keyring. Introduce
> > the builitin_secondary_and_ca_trusted restriction and linking the new
> > keyring to the secondary keyring. Only after everything is in place,
> > define and use the UEFI mok variable(s).
> >
> > Originally, I asked you to "Separate each **logical change** into a
> > separate patch." After re-ordering the patches, see if merging some of
> > them together now makes sense.
>
> I’ll see if merging some of them together makes sense.
>
> With the new Kconfig option, should the default be 'y' or ’n' when the secondary
> is defined? Thanks.
It definitely needs to be opt in. Please make it 'n'.
Mimi
