Message-ID: <9b414c36080138ac2862b9d6c24dfd98b7ccf974.camel@linux.ibm.com>
Subject: 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>
Date:   Mon, 22 Nov 2021 10:36:41 -0500
In-Reply-To: <20211111002057.123741-3-nayna@linux.ibm.com>
References: <20211111002057.123741-1-nayna@linux.ibm.com>
         <20211111002057.123741-3-nayna@linux.ibm.com>
Content-Type: text/plain; charset="ISO-8859-15"
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
Precedence: bulk

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.
>   */