Received-SPF: pass (google.com: domain of linux-crypto-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Message-ID: <34b12d8d47564a182f0a29a9592e203b17ccdd69.camel@linux.ibm.com>
Subject: Re: [PATCH v3 01/14] integrity: Introduce a Linux keyring for the
 Machine Owner Key (MOK)
From:   Mimi Zohar <zohar@linux.ibm.com>
To:     Eric Snowberg <eric.snowberg@oracle.com>, keyrings@vger.kernel.org,
        linux-integrity@vger.kernel.org, dhowells@redhat.com,
        dwmw2@infradead.org, herbert@gondor.apana.org.au,
        davem@davemloft.net, jarkko@kernel.org, jmorris@namei.org,
        serge@hallyn.com
Cc:     keescook@chromium.org, gregkh@linuxfoundation.org,
        torvalds@linux-foundation.org, scott.branden@broadcom.com,
        weiyongjun1@huawei.com, nayna@linux.ibm.com, ebiggers@google.com,
        ardb@kernel.org, nramas@linux.microsoft.com, lszubowi@redhat.com,
        linux-kernel@vger.kernel.org, linux-crypto@vger.kernel.org,
        linux-security-module@vger.kernel.org,
        James.Bottomley@HansenPartnership.com, pjones@redhat.com,
        glin@suse.com, konrad.wilk@oracle.com
Date:   Thu, 12 Aug 2021 17:31:44 -0400
In-Reply-To: <20210812021855.3083178-2-eric.snowberg@oracle.com>
References: <20210812021855.3083178-1-eric.snowberg@oracle.com>
         <20210812021855.3083178-2-eric.snowberg@oracle.com>
Content-Type: text/plain; charset="ISO-8859-15"
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
Precedence: bulk

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