linux-kernel - Re: [PATCH v3 01/14] integrity: Introduce a Linux keyring for the Machine Owner Key (MOK)

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Thu, 12 Aug 2021 20:35:41 -0400
From:   Mimi Zohar <zohar@...ux.ibm.com>
To:     Eric Snowberg <eric.snowberg@...cle.com>
Cc:     keyrings@...r.kernel.org,
        linux-integrity <linux-integrity@...r.kernel.org>,
        David Howells <dhowells@...hat.com>,
        David Woodhouse <dwmw2@...radead.org>,
        Herbert Xu <herbert@...dor.apana.org.au>,
        "David S . Miller" <davem@...emloft.net>,
        Jarkko Sakkinen <jarkko@...nel.org>,
        James Morris <jmorris@...ei.org>,
        "Serge E . Hallyn" <serge@...lyn.com>, keescook@...omium.org,
        gregkh@...uxfoundation.org, torvalds@...ux-foundation.org,
        scott.branden@...adcom.com, weiyongjun1@...wei.com,
        nayna@...ux.ibm.com, ebiggers@...gle.com, ardb@...nel.org,
        Lakshmi Ramasubramanian <nramas@...ux.microsoft.com>,
        lszubowi@...hat.com, linux-kernel@...r.kernel.org,
        linux-crypto@...r.kernel.org,
        linux-security-module@...r.kernel.org,
        James Bottomley <James.Bottomley@...senPartnership.com>,
        pjones@...hat.com,
        "konrad.wilk@...cle.com" <konrad.wilk@...cle.com>
Subject: Re: [PATCH v3 01/14] integrity: Introduce a Linux keyring for the
 Machine Owner Key (MOK)

On Thu, 2021-08-12 at 16:36 -0600, Eric Snowberg wrote:
> > On Aug 12, 2021, at 3:31 PM, Mimi Zohar <zohar@...ux.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@...cle.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