[PATCH v11 0/4] integrity: support including firmware ".platform" keys at build time

[PATCH v11 0/4] integrity: support including firmware ".platform" keys at build time

[Nayna Jain]

Some firmware support secure boot by embedding static keys to verify the
Linux kernel during boot. However, these firmware do not expose an
interface for the kernel to load firmware keys onto the ".platform"
keyring, preventing the kernel from verifying the kexec kernel image
signature.

This patchset exports load_certificate_list() and defines a new function
load_builtin_platform_cert() to load compiled in certificates onto the
".platform" keyring.

Changelog:
v11:
* Added a new patch to conditionally build extract-cert if
PLATFORM_KEYRING is enabled.

v10:
* Fixed the externs warning for Patch 3.

v9:
* Rebased on Jarkko's repo - 
git://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git

v8:
* Includes Jarkko's feedback on patch description and removed Reported-by
for Patch 1.

v7:
* Incldues Jarkko's feedback on patch description for Patch 1 and 3.

v6:
* Includes Jarkko's feedback:
 * Split Patch 2 into two.
 * Update Patch description.

v5:
* Renamed load_builtin_platform_cert() to load_platform_certificate_list()
and config INTEGRITY_PLATFORM_BUILTIN_KEYS to INTEGRITY_PLATFORM_KEYS, as
suggested by Mimi Zohar.

v4:
* Split into two patches as per Mimi Zohar and Dimitri John Ledkov
recommendation.

v3:
* Included Jarkko's feedback
 ** updated patch description to include approach.
 ** removed extern for function declaration in the .h file.
* Included load_certificate_list() within #ifdef CONFIG_KEYS condition.

v2:
* Fixed the error reported by kernel test robot
* Updated patch description based on Jarkko's feedback.

Nayna Jain (4):
  certs: export load_certificate_list() to be used outside certs/
  integrity: make integrity_keyring_from_id() non-static
  certs: conditionally build extract-cert if platform keyring is enabled
  integrity: support including firmware ".platform" keys at build time

 certs/Makefile                                |  9 ++++++--
 certs/blacklist.c                             |  1 -
 certs/common.c                                |  2 +-
 certs/common.h                                |  9 --------
 certs/system_keyring.c                        |  1 -
 include/keys/system_keyring.h                 |  6 +++++
 security/integrity/Kconfig                    | 10 ++++++++
 security/integrity/Makefile                   | 15 +++++++++++-
 security/integrity/digsig.c                   |  2 +-
 security/integrity/integrity.h                |  9 ++++++++
 .../integrity/platform_certs/platform_cert.S  | 23 +++++++++++++++++++
 .../platform_certs/platform_keyring.c         | 23 +++++++++++++++++++
 12 files changed, 94 insertions(+), 16 deletions(-)
 delete mode 100644 certs/common.h
 create mode 100644 security/integrity/platform_certs/platform_cert.S


base-commit: fb5abce6b2bb5cb3d628aaa63fa821da8c4600f9
-- 
2.27.0

[PATCH v11 1/4] certs: export load_certificate_list() to be used outside certs/

[Nayna Jain]

load_certificate_list() parses certificates embedded in the kernel
image to load them onto the keyring.

Commit "2565ca7f5ec1 (certs: Move load_system_certificate_list to a common
function)" made load_certificate_list() a common function in the certs/
directory. Export load_certificate_list() outside certs/ to be used by
load_platform_certificate_list() for loading compiled in platform keys
onto the .platform keyring at boot time.

Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
---
 certs/Makefile                | 5 +++--
 certs/blacklist.c             | 1 -
 certs/common.c                | 2 +-
 certs/common.h                | 9 ---------
 certs/system_keyring.c        | 1 -
 include/keys/system_keyring.h | 6 ++++++
 6 files changed, 10 insertions(+), 14 deletions(-)
 delete mode 100644 certs/common.h

diff --git a/certs/Makefile b/certs/Makefile
index 3ea7fe60823f..b92b6ff339d5 100644
--- a/certs/Makefile
+++ b/certs/Makefile
@@ -3,8 +3,9 @@
 # Makefile for the linux kernel signature checking certificates.
 #
 
-obj-$(CONFIG_SYSTEM_TRUSTED_KEYRING) += system_keyring.o system_certificates.o common.o
-obj-$(CONFIG_SYSTEM_BLACKLIST_KEYRING) += blacklist.o common.o
+obj-$(CONFIG_KEYS) += common.o
+obj-$(CONFIG_SYSTEM_TRUSTED_KEYRING) += system_keyring.o system_certificates.o
+obj-$(CONFIG_SYSTEM_BLACKLIST_KEYRING) += blacklist.o
 obj-$(CONFIG_SYSTEM_REVOCATION_LIST) += revocation_certificates.o
 ifneq ($(CONFIG_SYSTEM_BLACKLIST_HASH_LIST),)
 obj-$(CONFIG_SYSTEM_BLACKLIST_KEYRING) += blacklist_hashes.o
diff --git a/certs/blacklist.c b/certs/blacklist.c
index c9a435b15af4..b95e9b19c42f 100644
--- a/certs/blacklist.c
+++ b/certs/blacklist.c
@@ -17,7 +17,6 @@
 #include <linux/uidgid.h>
 #include <keys/system_keyring.h>
 #include "blacklist.h"
-#include "common.h"
 
 static struct key *blacklist_keyring;
 
diff --git a/certs/common.c b/certs/common.c
index 16a220887a53..41f763415a00 100644
--- a/certs/common.c
+++ b/certs/common.c
@@ -2,7 +2,7 @@
 
 #include <linux/kernel.h>
 #include <linux/key.h>
-#include "common.h"
+#include <keys/system_keyring.h>
 
 int load_certificate_list(const u8 cert_list[],
 			  const unsigned long list_size,
diff --git a/certs/common.h b/certs/common.h
deleted file mode 100644
index abdb5795936b..000000000000
--- a/certs/common.h
+++ /dev/null
@@ -1,9 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-or-later */
-
-#ifndef _CERT_COMMON_H
-#define _CERT_COMMON_H
-
-int load_certificate_list(const u8 cert_list[], const unsigned long list_size,
-			  const struct key *keyring);
-
-#endif
diff --git a/certs/system_keyring.c b/certs/system_keyring.c
index 05b66ce9d1c9..2ae1b2e34375 100644
--- a/certs/system_keyring.c
+++ b/certs/system_keyring.c
@@ -16,7 +16,6 @@
 #include <keys/asymmetric-type.h>
 #include <keys/system_keyring.h>
 #include <crypto/pkcs7.h>
-#include "common.h"
 
 static struct key *builtin_trusted_keys;
 #ifdef CONFIG_SECONDARY_TRUSTED_KEYRING
diff --git a/include/keys/system_keyring.h b/include/keys/system_keyring.h
index 2419a735420f..35babdc45689 100644
--- a/include/keys/system_keyring.h
+++ b/include/keys/system_keyring.h
@@ -10,6 +10,12 @@
 
 #include <linux/key.h>
 
+#ifdef CONFIG_KEYS
+int load_certificate_list(const u8 cert_list[],
+			  const unsigned long list_size,
+			  const struct key *keyring);
+#endif
+
 #ifdef CONFIG_SYSTEM_TRUSTED_KEYRING
 
 extern int restrict_link_by_builtin_trusted(struct key *keyring,
-- 
2.27.0

[PATCH v11 2/4] integrity: make integrity_keyring_from_id() non-static

[Nayna Jain]

Make integrity_keyring_from_id() non-static so that it is accessible
by other files in security/integrity.

Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
---
 security/integrity/digsig.c    | 2 +-
 security/integrity/integrity.h | 6 ++++++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
index c8c8a4a4e7a0..9c3165c07935 100644
--- a/security/integrity/digsig.c
+++ b/security/integrity/digsig.c
@@ -39,7 +39,7 @@ static const char * const keyring_name[INTEGRITY_KEYRING_MAX] = {
 #define restrict_link_to_ima restrict_link_by_builtin_trusted
 #endif
 
-static struct key *integrity_keyring_from_id(const unsigned int id)
+struct key *integrity_keyring_from_id(const unsigned int id)
 {
 	if (id >= INTEGRITY_KEYRING_MAX)
 		return ERR_PTR(-EINVAL);
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index 2e214c761158..76e9a9515f99 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -168,6 +168,7 @@ int __init integrity_init_keyring(const unsigned int id);
 int __init integrity_load_x509(const unsigned int id, const char *path);
 int __init integrity_load_cert(const unsigned int id, const char *source,
 			       const void *data, size_t len, key_perm_t perm);
+struct key *integrity_keyring_from_id(const unsigned int id);
 #else
 
 static inline int integrity_digsig_verify(const unsigned int id,
@@ -195,6 +196,11 @@ static inline int __init integrity_load_cert(const unsigned int id,
 {
 	return 0;
 }
+
+static inline struct key *integrity_keyring_from_id(const unsigned int id)
+{
+	return ERR_PTR(-EOPNOTSUPP);
+}
 #endif /* CONFIG_INTEGRITY_SIGNATURE */
 
 #ifdef CONFIG_INTEGRITY_ASYMMETRIC_KEYS
-- 
2.27.0

[PATCH v11 3/4] certs: conditionally build extract-cert if platform keyring is enabled

[Nayna Jain]

extract-cert is used outside certs/ by INTEGRITY_PLATFORM_KEYRING.
Also build extract-cert if INTEGRITY_PLATFORM_KEYRING is enabled.

Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
---
 certs/Makefile | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/certs/Makefile b/certs/Makefile
index b92b6ff339d5..dfb48e043cfe 100644
--- a/certs/Makefile
+++ b/certs/Makefile
@@ -88,7 +88,11 @@ $(obj)/x509_revocation_list: $(CONFIG_SYSTEM_REVOCATION_KEYS) $(obj)/extract-cer
 
 targets += x509_revocation_list
 
+ifeq ($(CONFIG_INTEGRITY_PLATFORM_KEYRING),y)
+hostprogs-always-y := extract-cert
+else
 hostprogs := extract-cert
+endif
 
 HOSTCFLAGS_extract-cert.o = $(shell pkg-config --cflags libcrypto 2> /dev/null)
 HOSTLDLIBS_extract-cert = $(shell pkg-config --libs libcrypto 2> /dev/null || echo -lcrypto)
-- 
2.27.0

[PATCH v11 4/4] integrity: support including firmware ".platform" keys at build time

[Nayna Jain]

Allow firmware keys to be embedded in the Linux kernel and loaded onto
the ".platform" keyring on boot.

The firmware keys can be specified in a file as a list of PEM encoded
certificates using new config INTEGRITY_PLATFORM_KEYS. The certificates
are embedded in the image by converting the PEM-formatted certificates
into DER(binary) and generating
security/integrity/platform_certs/platform_certificate_list file at
build time. On boot, the embedded certs from the image are loaded onto
the ".platform" keyring at late_initcall(), ensuring the platform keyring
exists before loading the keys.

Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
---
 security/integrity/Kconfig                    | 10 ++++++++
 security/integrity/Makefile                   | 15 +++++++++++-
 security/integrity/integrity.h                |  3 +++
 .../integrity/platform_certs/platform_cert.S  | 23 +++++++++++++++++++
 .../platform_certs/platform_keyring.c         | 23 +++++++++++++++++++
 5 files changed, 73 insertions(+), 1 deletion(-)
 create mode 100644 security/integrity/platform_certs/platform_cert.S

diff --git a/security/integrity/Kconfig b/security/integrity/Kconfig
index 599429f99f99..77b2c22c0e1b 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_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.
+
 config INTEGRITY_MACHINE_KEYRING
 	bool "Provide a keyring to which Machine Owner Keys may be added"
 	depends on SECONDARY_TRUSTED_KEYRING
diff --git a/security/integrity/Makefile b/security/integrity/Makefile
index d0ffe37dc1d6..65bd93301a3a 100644
--- a/security/integrity/Makefile
+++ b/security/integrity/Makefile
@@ -3,13 +3,17 @@
 # Makefile for caching inode integrity data (iint)
 #
 
+quiet_cmd_extract_certs  = CERT  $@
+      cmd_extract_certs  = certs/extract-cert $(2) $@
+
 obj-$(CONFIG_INTEGRITY) += integrity.o
 
 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/platform_cert.o
 integrity-$(CONFIG_INTEGRITY_MACHINE_KEYRING) += platform_certs/machine_keyring.o
 integrity-$(CONFIG_LOAD_UEFI_KEYS) += platform_certs/efi_parser.o \
 				      platform_certs/load_uefi.o \
@@ -20,3 +24,12 @@ integrity-$(CONFIG_LOAD_PPC_KEYS) += platform_certs/efi_parser.o \
                                      platform_certs/keyring_handler.o
 obj-$(CONFIG_IMA)			+= ima/
 obj-$(CONFIG_EVM)			+= evm/
+
+$(obj)/platform_certs/platform_cert.o: $(obj)/platform_certs/platform_certificate_list
+
+targets += platform_certificate_list
+
+$(obj)/platform_certs/platform_certificate_list: $(CONFIG_INTEGRITY_PLATFORM_KEYS) certs/extract-cert FORCE
+	$(call if_changed,extract_certs,$(if $(CONFIG_INTEGRITY_PLATFORM_KEYS),$<,""))
+
+clean-files := platform_certs/platform_certificate_list
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index 76e9a9515f99..219da29fecf7 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -282,6 +282,9 @@ integrity_audit_log_start(struct audit_context *ctx, gfp_t gfp_mask, int type)
 #endif
 
 #ifdef CONFIG_INTEGRITY_PLATFORM_KEYRING
+extern __initconst const u8 platform_certificate_list[];
+extern __initconst const unsigned long platform_certificate_list_size;
+
 void __init add_to_platform_keyring(const char *source, const void *data,
 				    size_t len);
 #else
diff --git a/security/integrity/platform_certs/platform_cert.S b/security/integrity/platform_certs/platform_cert.S
new file mode 100644
index 000000000000..20bccce5dc5a
--- /dev/null
+++ b/security/integrity/platform_certs/platform_cert.S
@@ -0,0 +1,23 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#include <linux/export.h>
+#include <linux/init.h>
+
+	__INITRODATA
+
+	.align 8
+#ifdef CONFIG_INTEGRITY_PLATFORM_KEYRING
+	.globl platform_certificate_list
+platform_certificate_list:
+__cert_list_start:
+	.incbin "security/integrity/platform_certs/platform_certificate_list"
+__cert_list_end:
+#endif
+
+	.align 8
+	.globl platform_certificate_list_size
+platform_certificate_list_size:
+#ifdef CONFIG_64BIT
+	.quad __cert_list_end - __cert_list_start
+#else
+	.long __cert_list_end - __cert_list_start
+#endif
diff --git a/security/integrity/platform_certs/platform_keyring.c b/security/integrity/platform_certs/platform_keyring.c
index bcafd7387729..c2368912fd1b 100644
--- a/security/integrity/platform_certs/platform_keyring.c
+++ b/security/integrity/platform_certs/platform_keyring.c
@@ -12,6 +12,7 @@
 #include <linux/cred.h>
 #include <linux/err.h>
 #include <linux/slab.h>
+#include <keys/system_keyring.h>
 #include "../integrity.h"
 
 /**
@@ -37,6 +38,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_platform_certificate_list(void)
+{
+	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_platform_certificate_list);
+
 /*
  * Create the trusted keyrings.
  */
-- 
2.27.0

Re: [PATCH v11 3/4] certs: conditionally build extract-cert if platform keyring is enabled

[Masahiro Yamada]

On Fri, Mar 11, 2022 at 6:45 AM Nayna Jain <nayna@linux.ibm.com> wrote:
>
> extract-cert is used outside certs/ by INTEGRITY_PLATFORM_KEYRING.
> Also build extract-cert if INTEGRITY_PLATFORM_KEYRING is enabled.

If really so, extract-cert should go back to scripts/ again.
(i.e. revert 340a02535ee785c64c62a9c45706597a0139e972)



>
> Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
> ---
>  certs/Makefile | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/certs/Makefile b/certs/Makefile
> index b92b6ff339d5..dfb48e043cfe 100644
> --- a/certs/Makefile
> +++ b/certs/Makefile
> @@ -88,7 +88,11 @@ $(obj)/x509_revocation_list: $(CONFIG_SYSTEM_REVOCATION_KEYS) $(obj)/extract-cer
>
>  targets += x509_revocation_list
>
> +ifeq ($(CONFIG_INTEGRITY_PLATFORM_KEYRING),y)
> +hostprogs-always-y := extract-cert
> +else
>  hostprogs := extract-cert
> +endif
>
>  HOSTCFLAGS_extract-cert.o = $(shell pkg-config --cflags libcrypto 2> /dev/null)
>  HOSTLDLIBS_extract-cert = $(shell pkg-config --libs libcrypto 2> /dev/null || echo -lcrypto)
> --
> 2.27.0
>


--
Best Regards
Masahiro Yamada

Re: [PATCH v11 0/4] integrity: support including firmware ".platform" keys at build time

[Nageswara Sastry]

On 11/03/22 3:14 am, Nayna Jain wrote:
> Some firmware support secure boot by embedding static keys to verify the
> Linux kernel during boot. However, these firmware do not expose an
> interface for the kernel to load firmware keys onto the ".platform"
> keyring, preventing the kernel from verifying the kexec kernel image
> signature.
> 
> This patchset exports load_certificate_list() and defines a new function
> load_builtin_platform_cert() to load compiled in certificates onto the
> ".platform" keyring.
> 
> Changelog:
> v11:
> * Added a new patch to conditionally build extract-cert if
> PLATFORM_KEYRING is enabled.
> 

Tested the following four patches with and with out setting 
CONFIG_INTEGRITY_PLATFORM_KEYS

Tested-by: Nageswara R Sastry <rnsastry@linux.ibm.com>


1. With set CONFIG_INTEGRITY_PLATFORM_KEYS

# grep pem .config
CONFIG_INTEGRITY_PLATFORM_KEYS="certs/kernel.pem"
CONFIG_MODULE_SIG_KEY="certs/signing_key.pem"
CONFIG_SYSTEM_TRUSTED_KEYS="certs/rhel.pem"

# grep 
"CONFIG_INTEGRITY_PLATFORM_KEYS\|INTEGRITY_PLATFORM_KEYRING\|SYSTEM_REVOCATION_LIST" 
.config
CONFIG_INTEGRITY_PLATFORM_KEYRING=y
CONFIG_INTEGRITY_PLATFORM_KEYS="certs/kernel.pem"
# CONFIG_SYSTEM_REVOCATION_LIST is not set

# cat /proc/keys | grep platform
0e60c88d I------     1 perm 1f0b0000     0     0 keyring   .platform: 1

# keyctl show %keyring:.platform
Keyring
  241223821 ---lswrv      0     0  keyring: .platform
  308815460 ---lswrv      0     0   \_ asymmetric: IBM Corporation: 
Guest Secure Boot Imprint Kernel Signing Key: 
a0cf9069c30875320cb10a77325d4fa7012f8d12


2. With out set CONFIG_INTEGRITY_PLATFORM_KEYS

# grep pem .config
CONFIG_MODULE_SIG_KEY="certs/signing_key.pem"
CONFIG_SYSTEM_TRUSTED_KEYS="certs/rhel.pem"

# grep 
"CONFIG_INTEGRITY_PLATFORM_KEYS\|INTEGRITY_PLATFORM_KEYRING\|SYSTEM_REVOCATION_LIST" 
.config
CONFIG_INTEGRITY_PLATFORM_KEYRING=y
CONFIG_INTEGRITY_PLATFORM_KEYS=""
# CONFIG_SYSTEM_REVOCATION_LIST is not set

# cat /proc/keys | grep platform
12a5f301 I------     1 perm 1f0b0000     0     0 keyring   .platform: empty

# keyctl show %keyring:.platform
Keyring
  312865537 ---lswrv      0     0  keyring: .platform


> 
> 
> base-commit: fb5abce6b2bb5cb3d628aaa63fa821da8c4600f9

-- 
Thanks and Regards
R.Nageswara Sastry

Re: [PATCH v11 0/4] integrity: support including firmware ".platform" keys at build time

[Jarkko Sakkinen]

On Thu, 2022-03-10 at 16:44 -0500, Nayna Jain wrote:
> Some firmware support secure boot by embedding static keys to verify the
> Linux kernel during boot. However, these firmware do not expose an
> interface for the kernel to load firmware keys onto the ".platform"
> keyring, preventing the kernel from verifying the kexec kernel image
> signature.
> 
> This patchset exports load_certificate_list() and defines a new function
> load_builtin_platform_cert() to load compiled in certificates onto the
> ".platform" keyring.
> 
> Changelog:
> v11:
> * Added a new patch to conditionally build extract-cert if
> PLATFORM_KEYRING is enabled.
> 
> v10:
> * Fixed the externs warning for Patch 3.
> 
> v9:
> * Rebased on Jarkko's repo - 
> git://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git
> 
> v8:
> * Includes Jarkko's feedback on patch description and removed Reported-by
> for Patch 1.
> 
> v7:
> * Incldues Jarkko's feedback on patch description for Patch 1 and 3.
> 
> v6:
> * Includes Jarkko's feedback:
>  * Split Patch 2 into two.
>  * Update Patch description.
> 
> v5:
> * Renamed load_builtin_platform_cert() to load_platform_certificate_list()
> and config INTEGRITY_PLATFORM_BUILTIN_KEYS to INTEGRITY_PLATFORM_KEYS, as
> suggested by Mimi Zohar.
> 
> v4:
> * Split into two patches as per Mimi Zohar and Dimitri John Ledkov
> recommendation.
> 
> v3:
> * Included Jarkko's feedback
>  ** updated patch description to include approach.
>  ** removed extern for function declaration in the .h file.
> * Included load_certificate_list() within #ifdef CONFIG_KEYS condition.
> 
> v2:
> * Fixed the error reported by kernel test robot
> * Updated patch description based on Jarkko's feedback.
> 
> Nayna Jain (4):
>   certs: export load_certificate_list() to be used outside certs/
>   integrity: make integrity_keyring_from_id() non-static
>   certs: conditionally build extract-cert if platform keyring is enabled
>   integrity: support including firmware ".platform" keys at build time
> 
>  certs/Makefile                                |  9 ++++++--
>  certs/blacklist.c                             |  1 -
>  certs/common.c                                |  2 +-
>  certs/common.h                                |  9 --------
>  certs/system_keyring.c                        |  1 -
>  include/keys/system_keyring.h                 |  6 +++++
>  security/integrity/Kconfig                    | 10 ++++++++
>  security/integrity/Makefile                   | 15 +++++++++++-
>  security/integrity/digsig.c                   |  2 +-
>  security/integrity/integrity.h                |  9 ++++++++
>  .../integrity/platform_certs/platform_cert.S  | 23 +++++++++++++++++++
>  .../platform_certs/platform_keyring.c         | 23 +++++++++++++++++++
>  12 files changed, 94 insertions(+), 16 deletions(-)
>  delete mode 100644 certs/common.h
>  create mode 100644 security/integrity/platform_certs/platform_cert.S
> 
> 
> base-commit: fb5abce6b2bb5cb3d628aaa63fa821da8c4600f9

I tuned tags. Nayna, Mimi is this how it should be:

commit f38ebfc7469c4924087dab2203479ee9bb6911ac (HEAD -> master, origin/next, origin/master, origin/HEAD, next)
Author: Nayna Jain <nayna@linux.ibm.com>
Date:   Thu Mar 10 16:44:50 2022 -0500

    integrity: support including firmware ".platform" keys at build time
    
    Allow firmware keys to be embedded in the Linux kernel and loaded onto
    the ".platform" keyring on boot.
    
    The firmware keys can be specified in a file as a list of PEM encoded
    certificates using new config INTEGRITY_PLATFORM_KEYS. The certificates
    are embedded in the image by converting the PEM-formatted certificates
    into DER(binary) and generating
    security/integrity/platform_certs/platform_certificate_list file at
    build time. On boot, the embedded certs from the image are loaded onto
    the ".platform" keyring at late_initcall(), ensuring the platform keyring
    exists before loading the keys.
    
    Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
    Tested-by: Mimi Zohar <zohar@linux.ibm.com>
    Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
    Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>

commit 233bb42918e71f143e6f31d99763348768eb1619
Author: Nayna Jain <nayna@linux.ibm.com>
Date:   Thu Mar 10 16:44:49 2022 -0500

    certs: conditionally build extract-cert if platform keyring is enabled
    
    extract-cert is used outside certs/ by INTEGRITY_PLATFORM_KEYRING.
    Also build extract-cert if INTEGRITY_PLATFORM_KEYRING is enabled.
    
    Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
    Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
    Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>

commit 5e1a41f1c2ee838af6ac39d1cdd4ed0f509186a1
Author: Nayna Jain <nayna@linux.ibm.com>
Date:   Thu Mar 10 16:44:48 2022 -0500

    integrity: make integrity_keyring_from_id() non-static
    
    Make integrity_keyring_from_id() non-static so that it is accessible
    by other files in security/integrity.
    
    Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
    Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
    Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
    Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>

commit 05489d362bde0f25c4e921d15cad4492d22147ae
Author: Nayna Jain <nayna@linux.ibm.com>
Date:   Thu Mar 10 16:44:47 2022 -0500

    certs: export load_certificate_list() to be used outside certs/
    
    load_certificate_list() parses certificates embedded in the kernel
    image to load them onto the keyring.
    
    Commit "2565ca7f5ec1 (certs: Move load_system_certificate_list to a common
    function)" made load_certificate_list() a common function in the certs/
    directory. Export load_certificate_list() outside certs/ to be used by
    load_platform_certificate_list() for loading compiled in platform keys
    onto the .platform keyring at boot time.
    
    Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
    Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
    Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
    Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>

BR, Jarkko

Re: [PATCH v11 0/4] integrity: support including firmware ".platform" keys at build time

[Jarkko Sakkinen]

On Fri, 2022-03-11 at 10:11 +0530, Nageswara Sastry wrote:
> 
> 
> On 11/03/22 3:14 am, Nayna Jain wrote:
> > Some firmware support secure boot by embedding static keys to verify the
> > Linux kernel during boot. However, these firmware do not expose an
> > interface for the kernel to load firmware keys onto the ".platform"
> > keyring, preventing the kernel from verifying the kexec kernel image
> > signature.
> > 
> > This patchset exports load_certificate_list() and defines a new function
> > load_builtin_platform_cert() to load compiled in certificates onto the
> > ".platform" keyring.
> > 
> > Changelog:
> > v11:
> > * Added a new patch to conditionally build extract-cert if
> > PLATFORM_KEYRING is enabled.
> > 
> 
> Tested the following four patches with and with out setting 
> CONFIG_INTEGRITY_PLATFORM_KEYS
> 
> Tested-by: Nageswara R Sastry <rnsastry@linux.ibm.com>

OK, I added it:

git://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git

BR, Jarkko

Re: [PATCH v11 0/4] integrity: support including firmware ".platform" keys at build time

[Nayna]

On 3/11/22 11:42, Jarkko Sakkinen wrote:
> On Fri, 2022-03-11 at 10:11 +0530, Nageswara Sastry wrote:
>>
>> On 11/03/22 3:14 am, Nayna Jain wrote:
>>> Some firmware support secure boot by embedding static keys to verify the
>>> Linux kernel during boot. However, these firmware do not expose an
>>> interface for the kernel to load firmware keys onto the ".platform"
>>> keyring, preventing the kernel from verifying the kexec kernel image
>>> signature.
>>>
>>> This patchset exports load_certificate_list() and defines a new function
>>> load_builtin_platform_cert() to load compiled in certificates onto the
>>> ".platform" keyring.
>>>
>>> Changelog:
>>> v11:
>>> * Added a new patch to conditionally build extract-cert if
>>> PLATFORM_KEYRING is enabled.
>>>
>> Tested the following four patches with and with out setting
>> CONFIG_INTEGRITY_PLATFORM_KEYS
>>
>> Tested-by: Nageswara R Sastry <rnsastry@linux.ibm.com>
> OK, I added it:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git

Thanks Jarkko. Masahiro Yamada would prefer to revert the original 
commit 340a02535ee785c64c62a9c45706597a0139e972 i.e. move extract-cert 
back to the scripts/ directory.

I am just posting v12 which includes Masahiro feedback. Nageswara has 
already tested v12 version as well.

I am fine either way 1.) Adding v11 and then separately handling of 
reverting of the commit or 2.) Adding v12 version which includes the 
revert. I leave the decision on you as to which one to upstream.

Thanks & Regards,

     - Nayna

Re: [PATCH v11 0/4] integrity: support including firmware ".platform" keys at build time

[Mimi Zohar]

On Fri, 2022-03-11 at 18:39 +0200, Jarkko Sakkinen wrote:
> On Thu, 2022-03-10 at 16:44 -0500, Nayna Jain wrote:
> > Some firmware support secure boot by embedding static keys to verify the
> > Linux kernel during boot. However, these firmware do not expose an
> > interface for the kernel to load firmware keys onto the ".platform"
> > keyring, preventing the kernel from verifying the kexec kernel image
> > signature.
> > 
> > This patchset exports load_certificate_list() and defines a new function
> > load_builtin_platform_cert() to load compiled in certificates onto the
> > ".platform" keyring.
> > 
> > Changelog:
> > v11:
> > * Added a new patch to conditionally build extract-cert if
> > PLATFORM_KEYRING is enabled.
> > 
> > v10:
> > * Fixed the externs warning for Patch 3.
> > 
> > v9:
> > * Rebased on Jarkko's repo - 
> > git://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git
> > 
> > v8:
> > * Includes Jarkko's feedback on patch description and removed Reported-by
> > for Patch 1.
> > 
> > v7:
> > * Incldues Jarkko's feedback on patch description for Patch 1 and 3.
> > 
> > v6:
> > * Includes Jarkko's feedback:
> >  * Split Patch 2 into two.
> >  * Update Patch description.
> > 
> > v5:
> > * Renamed load_builtin_platform_cert() to load_platform_certificate_list()
> > and config INTEGRITY_PLATFORM_BUILTIN_KEYS to INTEGRITY_PLATFORM_KEYS, as
> > suggested by Mimi Zohar.
> > 
> > v4:
> > * Split into two patches as per Mimi Zohar and Dimitri John Ledkov
> > recommendation.
> > 
> > v3:
> > * Included Jarkko's feedback
> >  ** updated patch description to include approach.
> >  ** removed extern for function declaration in the .h file.
> > * Included load_certificate_list() within #ifdef CONFIG_KEYS condition.
> > 
> > v2:
> > * Fixed the error reported by kernel test robot
> > * Updated patch description based on Jarkko's feedback.
> > 
> > Nayna Jain (4):
> >   certs: export load_certificate_list() to be used outside certs/
> >   integrity: make integrity_keyring_from_id() non-static
> >   certs: conditionally build extract-cert if platform keyring is enabled
> >   integrity: support including firmware ".platform" keys at build time
> > 
> >  certs/Makefile                                |  9 ++++++--
> >  certs/blacklist.c                             |  1 -
> >  certs/common.c                                |  2 +-
> >  certs/common.h                                |  9 --------
> >  certs/system_keyring.c                        |  1 -
> >  include/keys/system_keyring.h                 |  6 +++++
> >  security/integrity/Kconfig                    | 10 ++++++++
> >  security/integrity/Makefile                   | 15 +++++++++++-
> >  security/integrity/digsig.c                   |  2 +-
> >  security/integrity/integrity.h                |  9 ++++++++
> >  .../integrity/platform_certs/platform_cert.S  | 23 +++++++++++++++++++
> >  .../platform_certs/platform_keyring.c         | 23 +++++++++++++++++++
> >  12 files changed, 94 insertions(+), 16 deletions(-)
> >  delete mode 100644 certs/common.h
> >  create mode 100644 security/integrity/platform_certs/platform_cert.S
> > 
> > 
> > base-commit: fb5abce6b2bb5cb3d628aaa63fa821da8c4600f9
> 
> I tuned tags. Nayna, Mimi is this how it should be:

Either v11 or v12 is fine by me, except Masahiro Yamada requested to
move extract_certs back to the scripts/ directory.  v12 moves
extract_certs back, as requested.  Also Nageswara Sastry tested both
v11 and v12 versions.

thanks,

Mimi

Re: [PATCH v11 0/4] integrity: support including firmware ".platform" keys at build time

[Jarkko Sakkinen]

On Fri, Mar 11, 2022 at 04:03:12PM -0500, Nayna wrote:
> 
> On 3/11/22 11:42, Jarkko Sakkinen wrote:
> > On Fri, 2022-03-11 at 10:11 +0530, Nageswara Sastry wrote:
> > > 
> > > On 11/03/22 3:14 am, Nayna Jain wrote:
> > > > Some firmware support secure boot by embedding static keys to verify the
> > > > Linux kernel during boot. However, these firmware do not expose an
> > > > interface for the kernel to load firmware keys onto the ".platform"
> > > > keyring, preventing the kernel from verifying the kexec kernel image
> > > > signature.
> > > > 
> > > > This patchset exports load_certificate_list() and defines a new function
> > > > load_builtin_platform_cert() to load compiled in certificates onto the
> > > > ".platform" keyring.
> > > > 
> > > > Changelog:
> > > > v11:
> > > > * Added a new patch to conditionally build extract-cert if
> > > > PLATFORM_KEYRING is enabled.
> > > > 
> > > Tested the following four patches with and with out setting
> > > CONFIG_INTEGRITY_PLATFORM_KEYS
> > > 
> > > Tested-by: Nageswara R Sastry <rnsastry@linux.ibm.com>
> > OK, I added it:
> > 
> > git://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git
> 
> Thanks Jarkko. Masahiro Yamada would prefer to revert the original commit
> 340a02535ee785c64c62a9c45706597a0139e972 i.e. move extract-cert back to the
> scripts/ directory.
> 
> I am just posting v12 which includes Masahiro feedback. Nageswara has
> already tested v12 version as well.
> 
> I am fine either way 1.) Adding v11 and then separately handling of
> reverting of the commit or 2.) Adding v12 version which includes the revert.
> I leave the decision on you as to which one to upstream.
> 
> Thanks & Regards,
> 
>     - Nayna
> 

I already sent PR for v5.18. Too many late changes to include this, which
means that v12 is the way to go.

BR, Jarkko

Re: [PATCH v11 0/4] integrity: support including firmware ".platform" keys at build time

[Nayna]

On 3/17/22 03:38, Jarkko Sakkinen wrote:
> On Fri, Mar 11, 2022 at 04:03:12PM -0500, Nayna wrote:
>> On 3/11/22 11:42, Jarkko Sakkinen wrote:
>>> ".platform" keyring.
>>>>> Changelog:
>>>>> v11:
>>>>> * Added a new patch to conditionally build extract-cert if
>>>>> PLATFORM_KEYRING is enabled.
>>>>>
>>>> Tested the following four patches with and with out setting
>>>> CONFIG_INTEGRITY_PLATFORM_KEYS
>>>>
>>>> Tested-by: Nageswara R Sastry <rnsastry@linux.ibm.com>
>>> OK, I added it:
>>>
>>> git://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git
>> Thanks Jarkko. Masahiro Yamada would prefer to revert the original commit
>> 340a02535ee785c64c62a9c45706597a0139e972 i.e. move extract-cert back to the
>> scripts/ directory.
>>
>> I am just posting v12 which includes Masahiro feedback. Nageswara has
>> already tested v12 version as well.
>>
>> I am fine either way 1.) Adding v11 and then separately handling of
>> reverting of the commit or 2.) Adding v12 version which includes the revert.
>> I leave the decision on you as to which one to upstream.
>>
>> Thanks & Regards,
>>
>>      - Nayna
>>
> I already sent PR for v5.18. Too many late changes to include this, which
> means that v12 is the way to go.

Assuming v12 looks good, could you please queue it now ?

Thanks & Regards,

     - Nayna

Re: [PATCH v11 0/4] integrity: support including firmware ".platform" keys at build time

[Jarkko Sakkinen]

On Fri, Mar 18, 2022 at 05:25:07PM -0400, Nayna wrote:
> 
> On 3/17/22 03:38, Jarkko Sakkinen wrote:
> > On Fri, Mar 11, 2022 at 04:03:12PM -0500, Nayna wrote:
> > > On 3/11/22 11:42, Jarkko Sakkinen wrote:
> > > > ".platform" keyring.
> > > > > > Changelog:
> > > > > > v11:
> > > > > > * Added a new patch to conditionally build extract-cert if
> > > > > > PLATFORM_KEYRING is enabled.
> > > > > > 
> > > > > Tested the following four patches with and with out setting
> > > > > CONFIG_INTEGRITY_PLATFORM_KEYS
> > > > > 
> > > > > Tested-by: Nageswara R Sastry <rnsastry@linux.ibm.com>
> > > > OK, I added it:
> > > > 
> > > > git://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git
> > > Thanks Jarkko. Masahiro Yamada would prefer to revert the original commit
> > > 340a02535ee785c64c62a9c45706597a0139e972 i.e. move extract-cert back to the
> > > scripts/ directory.
> > > 
> > > I am just posting v12 which includes Masahiro feedback. Nageswara has
> > > already tested v12 version as well.
> > > 
> > > I am fine either way 1.) Adding v11 and then separately handling of
> > > reverting of the commit or 2.) Adding v12 version which includes the revert.
> > > I leave the decision on you as to which one to upstream.
> > > 
> > > Thanks & Regards,
> > > 
> > >      - Nayna
> > > 
> > I already sent PR for v5.18. Too many late changes to include this, which
> > means that v12 is the way to go.
> 
> Assuming v12 looks good, could you please queue it now ?
> 
> Thanks & Regards,
> 
>     - Nayna
> 

Unfortunately, I've already sent my v5.18 PR over a week ago. I can put it
to my queue but I think it is lacking some of the tested by tags, doesn't
it?

BR, Jarkko