| Message ID | 20210812021855.3083178-2-eric.snowberg@oracle.com |
|---|---|
| State | New |
| Headers | show |
| Series | Enroll kernel keys thru MOK | expand |
Hi Eric, 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@oracle.com> > --- > 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. 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. thanks, Mimi
> On Aug 12, 2021, at 12:58 PM, Jarkko Sakkinen <jarkko@kernel.org> wrote: > > On Wed, Aug 11, 2021 at 10:18:42PM -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 > > I would consider ".machine" instead. It holds MOK keys but is not a > MOK key. I’m open to renaming it to anything that you and the other maintainers feel would be appropriate. I just want to make sure there is an agreement on the new name before I make the change. Thanks.
On Thu, 2021-08-12 at 16:36 -0600, Eric Snowberg wrote: > > On Aug 12, 2021, at 3:31 PM, Mimi Zohar <zohar@linux.ibm.com> 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@oracle.com> > >> --- > >> 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
On 8/12/21 2:58 PM, Jarkko Sakkinen wrote: > On Wed, Aug 11, 2021 at 10:18:42PM -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 > I would consider ".machine" instead. It holds MOK keys but is not a > MOK key. I agree with changing the name. I believe the underlying source from where CA keys are loaded might vary based on the architecture (".mok" is UEFI specific.). The key part is that this new keyring should contain only CA keys which can be later used to vouch for user keys loaded onto IMA or secondary keyring at runtime. It would be good to have a "ca" in the name, like .xxxx-ca, where xxxx can be machine, owner, or system. I prefer .system-ca. Thanks & Regards, - Nayna
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);
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@oracle.com> --- 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