linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v8 0/3] integrity: support including firmware ".platform" keys at build time
@ 2022-01-11 18:36 Nayna Jain
  2022-01-11 18:36 ` [PATCH v8 1/3] certs: export load_certificate_list() to be used outside certs/ Nayna Jain
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Nayna Jain @ 2022-01-11 18:36 UTC (permalink / raw)
  To: linux-integrity, keyrings
  Cc: dhowells, zohar, jarkko, linux-security-module, linux-kernel,
	dimitri.ledkov, seth, 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:

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 (3):
  certs: export load_certificate_list() to be used outside certs/
  integrity: make integrity_keyring_from_id() non-static
  integrity: support including firmware ".platform" keys at build time

 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 +++++
 security/integrity/Kconfig                    | 10 +++++++
 security/integrity/Makefile                   | 17 +++++++++++-
 security/integrity/digsig.c                   |  2 +-
 security/integrity/integrity.h                |  6 +++++
 .../integrity/platform_certs/platform_cert.S  | 23 ++++++++++++++++
 .../platform_certs/platform_keyring.c         | 26 +++++++++++++++++++
 12 files changed, 92 insertions(+), 16 deletions(-)
 delete mode 100644 certs/common.h
 create mode 100644 security/integrity/platform_certs/platform_cert.S

-- 
2.27.0

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v8 1/3] certs: export load_certificate_list() to be used outside certs/
  2022-01-11 18:36 [PATCH v8 0/3] integrity: support including firmware ".platform" keys at build time Nayna Jain
@ 2022-01-11 18:36 ` Nayna Jain
  2022-01-27 14:52   ` Mimi Zohar
  2022-01-11 18:36 ` [PATCH v8 2/3] integrity: make integrity_keyring_from_id() non-static Nayna Jain
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 8+ messages in thread
From: Nayna Jain @ 2022-01-11 18:36 UTC (permalink / raw)
  To: linux-integrity, keyrings
  Cc: dhowells, zohar, jarkko, linux-security-module, linux-kernel,
	dimitri.ledkov, seth, 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 279433783b10..6f26c93ff56b 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 692365dee2bd..d130d5a96e09 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 6acd3cf13a18..d3f914d9a632 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


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v8 2/3] integrity: make integrity_keyring_from_id() non-static
  2022-01-11 18:36 [PATCH v8 0/3] integrity: support including firmware ".platform" keys at build time Nayna Jain
  2022-01-11 18:36 ` [PATCH v8 1/3] certs: export load_certificate_list() to be used outside certs/ Nayna Jain
@ 2022-01-11 18:36 ` Nayna Jain
  2022-01-11 18:36 ` [PATCH v8 3/3] integrity: support including firmware ".platform" keys at build time Nayna Jain
  2022-02-20 23:02 ` [PATCH v8 0/3] " Jarkko Sakkinen
  3 siblings, 0 replies; 8+ messages in thread
From: Nayna Jain @ 2022-01-11 18:36 UTC (permalink / raw)
  To: linux-integrity, keyrings
  Cc: dhowells, zohar, jarkko, linux-security-module, linux-kernel,
	dimitri.ledkov, seth, 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 3b06a01bd0fd..0ea40ed8dfcb 100644
--- a/security/integrity/digsig.c
+++ b/security/integrity/digsig.c
@@ -38,7 +38,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 547425c20e11..feb84e1b1105 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -167,6 +167,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,
@@ -194,6 +195,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


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v8 3/3] integrity: support including firmware ".platform" keys at build time
  2022-01-11 18:36 [PATCH v8 0/3] integrity: support including firmware ".platform" keys at build time Nayna Jain
  2022-01-11 18:36 ` [PATCH v8 1/3] certs: export load_certificate_list() to be used outside certs/ Nayna Jain
  2022-01-11 18:36 ` [PATCH v8 2/3] integrity: make integrity_keyring_from_id() non-static Nayna Jain
@ 2022-01-11 18:36 ` Nayna Jain
  2022-02-20 23:02 ` [PATCH v8 0/3] " Jarkko Sakkinen
  3 siblings, 0 replies; 8+ messages in thread
From: Nayna Jain @ 2022-01-11 18:36 UTC (permalink / raw)
  To: linux-integrity, keyrings
  Cc: dhowells, zohar, jarkko, linux-security-module, linux-kernel,
	dimitri.ledkov, seth, 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                   | 17 +++++++++++-
 .../integrity/platform_certs/platform_cert.S  | 23 ++++++++++++++++
 .../platform_certs/platform_keyring.c         | 26 +++++++++++++++++++
 4 files changed, 75 insertions(+), 1 deletion(-)
 create mode 100644 security/integrity/platform_certs/platform_cert.S

diff --git a/security/integrity/Kconfig b/security/integrity/Kconfig
index 71f0177e8716..9fccf1368b8a 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 LOAD_UEFI_KEYS
        depends on INTEGRITY_PLATFORM_KEYRING
        depends on EFI
diff --git a/security/integrity/Makefile b/security/integrity/Makefile
index 7ee39d66cf16..46629f5ef4f6 100644
--- a/security/integrity/Makefile
+++ b/security/integrity/Makefile
@@ -3,13 +3,18 @@
 # Makefile for caching inode integrity data (iint)
 #
 
+quiet_cmd_extract_certs  = EXTRACT_CERTS   $(patsubst "%",%,$(2))
+      cmd_extract_certs  = scripts/extract-cert $(2) $@
+$(eval $(call config_filename,INTEGRITY_PLATFORM_KEYS))
+
 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_LOAD_UEFI_KEYS) += platform_certs/efi_parser.o \
 				      platform_certs/load_uefi.o \
 				      platform_certs/keyring_handler.o
@@ -19,3 +24,13 @@ 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: scripts/extract-cert $(INTEGRITY_PLATFORM_KEYS_FILENAME) FORCE
+	$(call if_changed,extract_certs,$(CONFIG_INTEGRITY_PLATFORM_KEYS))
+
+clean-files := platform_certs/platform_certificate_list
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..b45de142c5f5 100644
--- a/security/integrity/platform_certs/platform_keyring.c
+++ b/security/integrity/platform_certs/platform_keyring.c
@@ -12,8 +12,12 @@
 #include <linux/cred.h>
 #include <linux/err.h>
 #include <linux/slab.h>
+#include <keys/system_keyring.h>
 #include "../integrity.h"
 
+extern __initconst const u8 platform_certificate_list[];
+extern __initconst const unsigned long platform_certificate_list_size;
+
 /**
  * add_to_platform_keyring - Add to platform keyring without validation.
  * @source: Source of key
@@ -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_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


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v8 1/3] certs: export load_certificate_list() to be used outside certs/
  2022-01-11 18:36 ` [PATCH v8 1/3] certs: export load_certificate_list() to be used outside certs/ Nayna Jain
@ 2022-01-27 14:52   ` Mimi Zohar
  0 siblings, 0 replies; 8+ messages in thread
From: Mimi Zohar @ 2022-01-27 14:52 UTC (permalink / raw)
  To: Nayna Jain, linux-integrity, keyrings
  Cc: dhowells, jarkko, linux-security-module, linux-kernel,
	dimitri.ledkov, seth

Hi Jarkko,

On Tue, 2022-01-11 at 13:36 -0500, Nayna Jain wrote:
> 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>

I think Nayna addressed your concerns.  Did you want to upstream this
patch set or would you prefer I did?

thanks,

Mimi


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v8 0/3] integrity: support including firmware ".platform" keys at build time
  2022-01-11 18:36 [PATCH v8 0/3] integrity: support including firmware ".platform" keys at build time Nayna Jain
                   ` (2 preceding siblings ...)
  2022-01-11 18:36 ` [PATCH v8 3/3] integrity: support including firmware ".platform" keys at build time Nayna Jain
@ 2022-02-20 23:02 ` Jarkko Sakkinen
  2022-02-21 13:33   ` Nayna
  3 siblings, 1 reply; 8+ messages in thread
From: Jarkko Sakkinen @ 2022-02-20 23:02 UTC (permalink / raw)
  To: Nayna Jain
  Cc: linux-integrity, keyrings, dhowells, zohar,
	linux-security-module, linux-kernel, dimitri.ledkov, seth

On Tue, Jan 11, 2022 at 01:36:44PM -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:
> 
> 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 (3):
>   certs: export load_certificate_list() to be used outside certs/
>   integrity: make integrity_keyring_from_id() non-static
>   integrity: support including firmware ".platform" keys at build time
> 
>  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 +++++
>  security/integrity/Kconfig                    | 10 +++++++
>  security/integrity/Makefile                   | 17 +++++++++++-
>  security/integrity/digsig.c                   |  2 +-
>  security/integrity/integrity.h                |  6 +++++
>  .../integrity/platform_certs/platform_cert.S  | 23 ++++++++++++++++
>  .../platform_certs/platform_keyring.c         | 26 +++++++++++++++++++
>  12 files changed, 92 insertions(+), 16 deletions(-)
>  delete mode 100644 certs/common.h
>  create mode 100644 security/integrity/platform_certs/platform_cert.S
> 
> -- 
> 2.27.0

To sort out tree conflicts: what if I pick these patches? They look fine
to me now. I can try to fix the possible merge conflicts and you can check
them before I make a PR.

/Jarkko

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v8 0/3] integrity: support including firmware ".platform" keys at build time
  2022-02-20 23:02 ` [PATCH v8 0/3] " Jarkko Sakkinen
@ 2022-02-21 13:33   ` Nayna
  2022-03-02  4:21     ` Jarkko Sakkinen
  0 siblings, 1 reply; 8+ messages in thread
From: Nayna @ 2022-02-21 13:33 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: linux-integrity, keyrings, dhowells, zohar,
	linux-security-module, linux-kernel, dimitri.ledkov, seth,
	Nayna Jain


On 2/20/22 18:02, Jarkko Sakkinen wrote:
> On Tue, Jan 11, 2022 at 01:36:44PM -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:
>>
>> 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 (3):
>>    certs: export load_certificate_list() to be used outside certs/
>>    integrity: make integrity_keyring_from_id() non-static
>>    integrity: support including firmware ".platform" keys at build time
>>
>>   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 +++++
>>   security/integrity/Kconfig                    | 10 +++++++
>>   security/integrity/Makefile                   | 17 +++++++++++-
>>   security/integrity/digsig.c                   |  2 +-
>>   security/integrity/integrity.h                |  6 +++++
>>   .../integrity/platform_certs/platform_cert.S  | 23 ++++++++++++++++
>>   .../platform_certs/platform_keyring.c         | 26 +++++++++++++++++++
>>   12 files changed, 92 insertions(+), 16 deletions(-)
>>   delete mode 100644 certs/common.h
>>   create mode 100644 security/integrity/platform_certs/platform_cert.S
>>
>> -- 
>> 2.27.0
> To sort out tree conflicts: what if I pick these patches? They look fine
> to me now. I can try to fix the possible merge conflicts and you can check
> them before I make a PR.

Sounds good. Thanks !!

Thanks & Regards,

      - Nayna


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v8 0/3] integrity: support including firmware ".platform" keys at build time
  2022-02-21 13:33   ` Nayna
@ 2022-03-02  4:21     ` Jarkko Sakkinen
  0 siblings, 0 replies; 8+ messages in thread
From: Jarkko Sakkinen @ 2022-03-02  4:21 UTC (permalink / raw)
  To: Nayna
  Cc: linux-integrity, keyrings, dhowells, zohar,
	linux-security-module, linux-kernel, dimitri.ledkov, seth,
	Nayna Jain

On Mon, Feb 21, 2022 at 08:33:55AM -0500, Nayna wrote:
> 
> On 2/20/22 18:02, Jarkko Sakkinen wrote:
> > On Tue, Jan 11, 2022 at 01:36:44PM -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:
> > > 
> > > 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 (3):
> > >    certs: export load_certificate_list() to be used outside certs/
> > >    integrity: make integrity_keyring_from_id() non-static
> > >    integrity: support including firmware ".platform" keys at build time
> > > 
> > >   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 +++++
> > >   security/integrity/Kconfig                    | 10 +++++++
> > >   security/integrity/Makefile                   | 17 +++++++++++-
> > >   security/integrity/digsig.c                   |  2 +-
> > >   security/integrity/integrity.h                |  6 +++++
> > >   .../integrity/platform_certs/platform_cert.S  | 23 ++++++++++++++++
> > >   .../platform_certs/platform_keyring.c         | 26 +++++++++++++++++++
> > >   12 files changed, 92 insertions(+), 16 deletions(-)
> > >   delete mode 100644 certs/common.h
> > >   create mode 100644 security/integrity/platform_certs/platform_cert.S
> > > 
> > > -- 
> > > 2.27.0
> > To sort out tree conflicts: what if I pick these patches? They look fine
> > to me now. I can try to fix the possible merge conflicts and you can check
> > them before I make a PR.
> 
> Sounds good. Thanks !!
> 
> Thanks & Regards,
> 
>      - Nayna
> 

Sorry that this comes so later but would it possible for you to fix this
conflict against my tree and send v10 with just that change:

$ git diff
diff --cc include/keys/system_keyring.h
index 91e080efb918,d3f914d9a632..000000000000
--- a/include/keys/system_keyring.h
+++ b/include/keys/system_keyring.h
@@@ -10,12 -10,11 +10,20 @@@
  
  #include <linux/key.h>
  
++<<<<<<< HEAD
 +enum blacklist_hash_type {
 +      /* TBSCertificate hash */
 +      BLACKLIST_HASH_X509_TBS = 1,
 +      /* Raw data hash */
 +      BLACKLIST_HASH_BINARY = 2,
 +};
++=======
+ #ifdef CONFIG_KEYS
+ int load_certificate_list(const u8 cert_list[],
+                         const unsigned long list_size,
+                         const struct key *keyring);
+ #endif
++>>>>>>> certs: export load_certificate_list() to be used outside certs/
  
I somehow lost your response to the jungle and found it only now :-(

I'm still happy to include this to the PR.

BR, Jarkko

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2022-03-02  4:20 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-11 18:36 [PATCH v8 0/3] integrity: support including firmware ".platform" keys at build time Nayna Jain
2022-01-11 18:36 ` [PATCH v8 1/3] certs: export load_certificate_list() to be used outside certs/ Nayna Jain
2022-01-27 14:52   ` Mimi Zohar
2022-01-11 18:36 ` [PATCH v8 2/3] integrity: make integrity_keyring_from_id() non-static Nayna Jain
2022-01-11 18:36 ` [PATCH v8 3/3] integrity: support including firmware ".platform" keys at build time Nayna Jain
2022-02-20 23:02 ` [PATCH v8 0/3] " Jarkko Sakkinen
2022-02-21 13:33   ` Nayna
2022-03-02  4:21     ` Jarkko Sakkinen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).