* [PATCH v9 1/3] certs: export load_certificate_list() to be used outside certs/
2022-03-04 17:54 [PATCH v9 0/3] integrity: support including firmware ".platform" keys at build time Nayna Jain
@ 2022-03-04 17:54 ` Nayna Jain
2022-03-04 17:54 ` [PATCH v9 2/3] integrity: make integrity_keyring_from_id() non-static Nayna Jain
` (3 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: Nayna Jain @ 2022-03-04 17:54 UTC (permalink / raw)
To: linux-integrity, keyrings
Cc: dhowells, zohar, jarkko, linux-security-module, linux-kernel,
dimitri.ledkov, seth, rnsastry, 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 102e9ec993c7..811706a0824a 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),)
quiet_cmd_check_blacklist_hashes = CHECK $(patsubst "%",%,$(2))
diff --git a/certs/blacklist.c b/certs/blacklist.c
index 486ce0dd8e9c..0d20264fa27c 100644
--- a/certs/blacklist.c
+++ b/certs/blacklist.c
@@ -18,7 +18,6 @@
#include <linux/verification.h>
#include <keys/system_keyring.h>
#include "blacklist.h"
-#include "common.h"
/*
* According to crypto/asymmetric_keys/x509_cert_parser.c:x509_note_pkey_algo(),
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 91e080efb918..69beb444464a 100644
--- a/include/keys/system_keyring.h
+++ b/include/keys/system_keyring.h
@@ -17,6 +17,12 @@ enum blacklist_hash_type {
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
+
#ifdef CONFIG_SYSTEM_TRUSTED_KEYRING
extern int restrict_link_by_builtin_trusted(struct key *keyring,
--
2.27.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v9 2/3] integrity: make integrity_keyring_from_id() non-static
2022-03-04 17:54 [PATCH v9 0/3] integrity: support including firmware ".platform" keys at build time Nayna Jain
2022-03-04 17:54 ` [PATCH v9 1/3] certs: export load_certificate_list() to be used outside certs/ Nayna Jain
@ 2022-03-04 17:54 ` Nayna Jain
2022-03-04 17:54 ` [PATCH v9 3/3] integrity: support including firmware ".platform" keys at build time Nayna Jain
` (2 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: Nayna Jain @ 2022-03-04 17:54 UTC (permalink / raw)
To: linux-integrity, keyrings
Cc: dhowells, zohar, jarkko, linux-security-module, linux-kernel,
dimitri.ledkov, seth, rnsastry, 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
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v9 3/3] integrity: support including firmware ".platform" keys at build time
2022-03-04 17:54 [PATCH v9 0/3] integrity: support including firmware ".platform" keys at build time Nayna Jain
2022-03-04 17:54 ` [PATCH v9 1/3] certs: export load_certificate_list() to be used outside certs/ Nayna Jain
2022-03-04 17:54 ` [PATCH v9 2/3] integrity: make integrity_keyring_from_id() non-static Nayna Jain
@ 2022-03-04 17:54 ` Nayna Jain
2022-03-04 23:45 ` [PATCH v9 0/3] " Jarkko Sakkinen
2022-03-05 5:37 ` Jarkko Sakkinen
4 siblings, 0 replies; 9+ messages in thread
From: Nayna Jain @ 2022-03-04 17:54 UTC (permalink / raw)
To: linux-integrity, keyrings
Cc: dhowells, zohar, jarkko, linux-security-module, linux-kernel,
dimitri.ledkov, seth, rnsastry, 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 ++++++++++-
.../integrity/platform_certs/platform_cert.S | 23 ++++++++++++++++
.../platform_certs/platform_keyring.c | 26 +++++++++++++++++++
4 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/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] 9+ messages in thread* Re: [PATCH v9 0/3] integrity: support including firmware ".platform" keys at build time
2022-03-04 17:54 [PATCH v9 0/3] integrity: support including firmware ".platform" keys at build time Nayna Jain
` (2 preceding siblings ...)
2022-03-04 17:54 ` [PATCH v9 3/3] integrity: support including firmware ".platform" keys at build time Nayna Jain
@ 2022-03-04 23:45 ` Jarkko Sakkinen
2022-03-05 5:37 ` Jarkko Sakkinen
4 siblings, 0 replies; 9+ messages in thread
From: Jarkko Sakkinen @ 2022-03-04 23:45 UTC (permalink / raw)
To: Nayna Jain, linux-integrity, keyrings
Cc: dhowells, zohar, linux-security-module, linux-kernel,
dimitri.ledkov, seth, rnsastry
On Fri, 2022-03-04 at 12:54 -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:
> v9:
> * Rebased on tpmdd master branch 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 (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 | 15 ++++++++++-
> 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, 90 insertions(+), 16 deletions(-)
> delete mode 100644 certs/common.h
> create mode 100644 security/integrity/platform_certs/platform_cert.S
>
>
> base-commit: c9e54f38976a1c0ec69c0a6208b3fd55fceb01d1
Thanks for the trouble! I'll pick these.
BR, Jarkko
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v9 0/3] integrity: support including firmware ".platform" keys at build time
2022-03-04 17:54 [PATCH v9 0/3] integrity: support including firmware ".platform" keys at build time Nayna Jain
` (3 preceding siblings ...)
2022-03-04 23:45 ` [PATCH v9 0/3] " Jarkko Sakkinen
@ 2022-03-05 5:37 ` Jarkko Sakkinen
2022-03-05 5:49 ` Jarkko Sakkinen
4 siblings, 1 reply; 9+ messages in thread
From: Jarkko Sakkinen @ 2022-03-05 5:37 UTC (permalink / raw)
To: Nayna Jain
Cc: linux-integrity, keyrings, dhowells, zohar, linux-security-module,
linux-kernel, dimitri.ledkov, seth, rnsastry
On Fri, Mar 04, 2022 at 12:54:00PM -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:
> v9:
> * Rebased on tpmdd master branch 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 (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 | 15 ++++++++++-
> 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, 90 insertions(+), 16 deletions(-)
> delete mode 100644 certs/common.h
> create mode 100644 security/integrity/platform_certs/platform_cert.S
>
>
> base-commit: c9e54f38976a1c0ec69c0a6208b3fd55fceb01d1
> --
> 2.27.0
Thank you, applied.
BR, Jarkko
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v9 0/3] integrity: support including firmware ".platform" keys at build time
2022-03-05 5:37 ` Jarkko Sakkinen
@ 2022-03-05 5:49 ` Jarkko Sakkinen
2022-03-06 1:32 ` Nayna
0 siblings, 1 reply; 9+ messages in thread
From: Jarkko Sakkinen @ 2022-03-05 5:49 UTC (permalink / raw)
To: Nayna Jain
Cc: linux-integrity, keyrings, dhowells, zohar, linux-security-module,
linux-kernel, dimitri.ledkov, seth, rnsastry
On Sat, Mar 05, 2022 at 07:37:18AM +0200, Jarkko Sakkinen wrote:
> On Fri, Mar 04, 2022 at 12:54:00PM -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:
> > v9:
> > * Rebased on tpmdd master branch 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 (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 | 15 ++++++++++-
> > 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, 90 insertions(+), 16 deletions(-)
> > delete mode 100644 certs/common.h
> > create mode 100644 security/integrity/platform_certs/platform_cert.S
> >
> >
> > base-commit: c9e54f38976a1c0ec69c0a6208b3fd55fceb01d1
> > --
> > 2.27.0
>
> Thank you, applied.
>
> BR, Jarkko
You need to fix this:
WARNING: externs should be avoided in .c files
#129: FILE: security/integrity/platform_certs/platform_keyring.c:19:
+extern __initconst const unsigned long platform_certificate_list_size;
I.e. remove extern's from font.
Please send one more version.
BR, Jarkko
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v9 0/3] integrity: support including firmware ".platform" keys at build time
2022-03-05 5:49 ` Jarkko Sakkinen
@ 2022-03-06 1:32 ` Nayna
2022-03-06 2:10 ` Jarkko Sakkinen
0 siblings, 1 reply; 9+ messages in thread
From: Nayna @ 2022-03-06 1:32 UTC (permalink / raw)
To: Jarkko Sakkinen, Nayna Jain
Cc: linux-integrity, keyrings, dhowells, zohar, linux-security-module,
linux-kernel, dimitri.ledkov, seth, rnsastry
On 3/5/22 00:49, Jarkko Sakkinen wrote:
> On Sat, Mar 05, 2022 at 07:37:18AM +0200, Jarkko Sakkinen wrote:
>> On Fri, Mar 04, 2022 at 12:54:00PM -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:
>>> v9:
>>> * Rebased on tpmdd master branch 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 (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 | 15 ++++++++++-
>>> 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, 90 insertions(+), 16 deletions(-)
>>> delete mode 100644 certs/common.h
>>> create mode 100644 security/integrity/platform_certs/platform_cert.S
>>>
>>>
>>> base-commit: c9e54f38976a1c0ec69c0a6208b3fd55fceb01d1
>>> --
>>> 2.27.0
>> Thank you, applied.
>>
>> BR, Jarkko
> You need to fix this:
>
> WARNING: externs should be avoided in .c files
> #129: FILE: security/integrity/platform_certs/platform_keyring.c:19:
> +extern __initconst const unsigned long platform_certificate_list_size;
Yes, because I followed the same convention as used for system keyring
certs. Following externs are defined in system_keyring.c for referencing
variables defined in .S file.
extern __initconst const u8 system_certificate_list[];
extern __initconst const unsigned long system_certificate_list_size;
extern __initconst const unsigned long module_cert_size;
Thanks & Regards,
- Nayna
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v9 0/3] integrity: support including firmware ".platform" keys at build time
2022-03-06 1:32 ` Nayna
@ 2022-03-06 2:10 ` Jarkko Sakkinen
0 siblings, 0 replies; 9+ messages in thread
From: Jarkko Sakkinen @ 2022-03-06 2:10 UTC (permalink / raw)
To: Nayna
Cc: Nayna Jain, linux-integrity, keyrings, dhowells, zohar,
linux-security-module, linux-kernel, dimitri.ledkov, seth,
rnsastry
On Sat, Mar 05, 2022 at 08:32:43PM -0500, Nayna wrote:
>
> On 3/5/22 00:49, Jarkko Sakkinen wrote:
> > On Sat, Mar 05, 2022 at 07:37:18AM +0200, Jarkko Sakkinen wrote:
> > > On Fri, Mar 04, 2022 at 12:54:00PM -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:
> > > > v9:
> > > > * Rebased on tpmdd master branch 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 (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 | 15 ++++++++++-
> > > > 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, 90 insertions(+), 16 deletions(-)
> > > > delete mode 100644 certs/common.h
> > > > create mode 100644 security/integrity/platform_certs/platform_cert.S
> > > >
> > > >
> > > > base-commit: c9e54f38976a1c0ec69c0a6208b3fd55fceb01d1
> > > > --
> > > > 2.27.0
> > > Thank you, applied.
> > >
> > > BR, Jarkko
> > You need to fix this:
> >
> > WARNING: externs should be avoided in .c files
> > #129: FILE: security/integrity/platform_certs/platform_keyring.c:19:
> > +extern __initconst const unsigned long platform_certificate_list_size;
>
> Yes, because I followed the same convention as used for system keyring
> certs. Following externs are defined in system_keyring.c for referencing
> variables defined in .S file.
>
> extern __initconst const u8 system_certificate_list[];
> extern __initconst const unsigned long system_certificate_list_size;
> extern __initconst const unsigned long module_cert_size;
So why don't you do as the warning says and place them to integrity.h
for instance?
BR, Jarkko
^ permalink raw reply [flat|nested] 9+ messages in thread