Re: [PATCH v4 2/2] integrity: support including firmware ".platform" keys at build time

From: Mimi Zohar <zohar@linux.ibm.com>
To: Nayna Jain <nayna@linux.ibm.com>,
	linux-integrity@vger.kernel.org, keyrings@vger.kernel.org
Cc: dhowells@redhat.com, jarkko@kernel.org,
	linux-security-module@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	Dimitri John Ledkov <dimitri.ledkov@canonical.com>,
	Seth Forshee <seth@forshee.me>
Subject: Re: [PATCH v4 2/2] integrity: support including firmware ".platform" keys at build time
Date: Mon, 22 Nov 2021 10:36:41 -0500	[thread overview]
Message-ID: <9b414c36080138ac2862b9d6c24dfd98b7ccf974.camel@linux.ibm.com> (raw)
In-Reply-To: <20211111002057.123741-3-nayna@linux.ibm.com>

On Wed, 2021-11-10 at 19:20 -0500, Nayna Jain wrote:

> diff --git a/security/integrity/Kconfig b/security/integrity/Kconfig
> index 71f0177e8716..b2009b792882 100644
> --- a/security/integrity/Kconfig
> +++ b/security/integrity/Kconfig
> @@ -62,6 +62,16 @@ config INTEGRITY_PLATFORM_KEYRING
>           provided by the platform for verifying the kexec'ed kerned image
>           and, possibly, the initramfs signature.
>  
> +config INTEGRITY_PLATFORM_BUILTIN_KEYS

With the ".builtin_trusted_keys" keyring, using the word "BUILTIN" here
could be confusing.  Instead, similar to SYSTEM_TRUSTED_KEYRING and
SYSTEM_TRUSTED_KEYS, this config should be named
INTEGRITY_PLATFORM_KEYS.

> +        string "Builtin X.509 keys for .platform keyring"
> +        depends on KEYS
> +        depends on ASYMMETRIC_KEY_TYPE
> +        depends on INTEGRITY_PLATFORM_KEYRING
> +        help
> +          If set, this option should be the filename of a PEM-formatted file
> +          containing X.509 certificates to be loaded onto the ".platform"
> +          keyring.
> +

> 
> @@ -37,6 +41,28 @@ void __init add_to_platform_keyring(const char *source, const void *data,
>  		pr_info("Error adding keys to platform keyring %s\n", source);
>  }
>  
> +static __init int load_builtin_platform_cert(void)

Similarly, this should be named load_platform_cert_list() or
load_platform_certificate_list().

> +{
> +	const u8 *p;
> +	unsigned long size;
> +	int rc;
> +	struct key *keyring;
> +
> +	p = platform_certificate_list;
> +	size = platform_certificate_list_size;
> +
> +	keyring = integrity_keyring_from_id(INTEGRITY_KEYRING_PLATFORM);
> +	if (IS_ERR(keyring))
> +		return PTR_ERR(keyring);
> +
> +	rc = load_certificate_list(p, size, keyring);
> +	if (rc)
> +		pr_info("Error adding keys to platform keyring %d\n", rc);
> +
> +	return rc;
> +}
> +late_initcall(load_builtin_platform_cert);
> +
>  /*
>   * Create the trusted keyrings.
>   */