2022-03-07 00:51:14

by Nayna Jain

[permalink] [raw]
Subject: [PATCH v10 0/3] integrity: support including firmware ".platform" keys at build time

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:
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 (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 | 9 ++++++++
.../integrity/platform_certs/platform_cert.S | 23 +++++++++++++++++++
.../platform_certs/platform_keyring.c | 23 +++++++++++++++++++
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


2022-03-07 03:37:30

by Nayna Jain

[permalink] [raw]
Subject: [PATCH v10 3/3] 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 <[email protected]>
Signed-off-by: Nayna Jain <[email protected]>
---
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

2022-03-07 08:51:17

by Nayna Jain

[permalink] [raw]
Subject: [PATCH v10 1/3] 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.

Reviewed-by: Mimi Zohar <[email protected]>
Signed-off-by: Nayna Jain <[email protected]>
---
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

2022-03-07 17:50:42

by Jarkko Sakkinen

[permalink] [raw]
Subject: Re: [PATCH v10 3/3] integrity: support including firmware ".platform" keys at build time

On Sun, Mar 06, 2022 at 03:51:00PM -0500, Nayna Jain wrote:
> 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 <[email protected]>
> Signed-off-by: Nayna Jain <[email protected]>
> ---
> 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
>

There's zero tested-by's for this, i.e. cannot be applied before someone
has tested this. Mimi, do not mean to be rude, but I don't frankly
understand why you ask to pick a patch set that is *untested*.

So I generated a self-signed certificate:

openssl req -x509 -out localhost.crt -keyout localhost.key \
-newkey rsa:2048 -nodes -sha256 \
-subj '/CN=localhost' -extensions EXT -config <( \
printf "[dn]\nCN=localhost\n[req]\ndistinguished_name = dn\n[EXT]\nsubjectAltName=DNS:localhost\nkeyUsage=digitalSignature\nextendedKeyUsage=serverAuth")

(by courtesy of letsencrypt: https://letsencrypt.org/docs/certificates-for-localhost/)

openssl x509 -in localhost.crt -out localhost.pem -outform PEM

And starting with tinyconfig I added minimal options to enable this
feature. The config is attached.

The end result is:

make[2]: *** No rule to make target 'certs/extract-cert', needed by 'security/integrity/platform_certs/platform_certificate_list'. Stop.
make[1]: *** [scripts/Makefile.build:550: security/integrity] Error 2
make: *** [Makefile:1831: security] Error 2

BR, Jarkko


Attachments:
(No filename) (7.68 kB)
.config (35.96 kB)
Download all attachments

2022-03-08 08:52:38

by Jarkko Sakkinen

[permalink] [raw]
Subject: Re: [PATCH v10 3/3] integrity: support including firmware ".platform" keys at build time

On Mon, Mar 07, 2022 at 02:48:52PM +0200, Jarkko Sakkinen wrote:
> On Sun, Mar 06, 2022 at 03:51:00PM -0500, Nayna Jain wrote:
> > 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 <[email protected]>
> > Signed-off-by: Nayna Jain <[email protected]>
> > ---
> > 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
> >
>
> There's zero tested-by's for this, i.e. cannot be applied before someone
> has tested this. Mimi, do not mean to be rude, but I don't frankly
> understand why you ask to pick a patch set that is *untested*.
>
> So I generated a self-signed certificate:
>
> openssl req -x509 -out localhost.crt -keyout localhost.key \
> -newkey rsa:2048 -nodes -sha256 \
> -subj '/CN=localhost' -extensions EXT -config <( \
> printf "[dn]\nCN=localhost\n[req]\ndistinguished_name = dn\n[EXT]\nsubjectAltName=DNS:localhost\nkeyUsage=digitalSignature\nextendedKeyUsage=serverAuth")
>
> (by courtesy of letsencrypt: https://letsencrypt.org/docs/certificates-for-localhost/)
>
> openssl x509 -in localhost.crt -out localhost.pem -outform PEM
>
> And starting with tinyconfig I added minimal options to enable this
> feature. The config is attached.
>
> The end result is:
>
> make[2]: *** No rule to make target 'certs/extract-cert', needed by 'security/integrity/platform_certs/platform_certificate_list'. Stop.
> make[1]: *** [scripts/Makefile.build:550: security/integrity] Error 2
> make: *** [Makefile:1831: security] Error 2
>
> BR, Jarkko

At least for the next PR, I'm not including this, sorry.

BR, Jarkko

2022-03-08 21:04:09

by Mimi Zohar

[permalink] [raw]
Subject: Re: [PATCH v10 3/3] integrity: support including firmware ".platform" keys at build time

[Cc'ing Masahiro Yamada]

On Mon, 2022-03-07 at 14:48 +0200, Jarkko Sakkinen wrote:
> On Sun, Mar 06, 2022 at 03:51:00PM -0500, Nayna Jain wrote:
> > 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 <[email protected]>
> > Signed-off-by: Nayna Jain <[email protected]>
> > ---
> > 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
> >
>
> There's zero tested-by's for this, i.e. cannot be applied before someone
> has tested this. Mimi, do not mean to be rude, but I don't frankly
> understand why you ask to pick a patch set that is *untested*.
> So I generated a self-signed certificate:
>
> openssl req -x509 -out localhost.crt -keyout localhost.key \
> -newkey rsa:2048 -nodes -sha256 \
> -subj '/CN=localhost' -extensions EXT -config <( \
> printf "[dn]\nCN=localhost\n[req]\ndistinguished_name = dn\n[EXT]\nsubjectAltName=DNS:localhost\nkeyUsage=digitalSignature\nextendedKeyUsage=serverAuth")
>
> (by courtesy of letsencrypt: https://letsencrypt.org/docs/certificates-for-localhost/)
>
> openssl x509 -in localhost.crt -out localhost.pem -outform PEM
>
> And starting with tinyconfig I added minimal options to enable this
> feature. The config is attached.
>
> The end result is:
>
> make[2]: *** No rule to make target 'certs/extract-cert', needed by 'security/integrity/platform_certs/platform_certificate_list'. Stop.
> make[1]: *** [scripts/Makefile.build:550: security/integrity] Error 2
> make: *** [Makefile:1831: security] Error 2

I've reviewed and tested this patch set each time it was posted last
fall/winter. Recent changes were limited to the cover letter and patch
description. Only recently was "extract_cert" moved to the certs/
directory and not built automatically. The commit message says the
move was because it wasn't being used outside the certs directory.
Refer to commit 340a02535ee7 ("certs: move scripts/extract-cert to
certs/").

Masahiro Yamada would you be ok with reverting the move?

thanks,

Mimi

2022-03-09 00:55:04

by Jarkko Sakkinen

[permalink] [raw]
Subject: Re: [PATCH v10 3/3] integrity: support including firmware ".platform" keys at build time

On Mon, Mar 07, 2022 at 05:03:09PM -0500, Mimi Zohar wrote:
> [Cc'ing Masahiro Yamada]
>
> On Mon, 2022-03-07 at 14:48 +0200, Jarkko Sakkinen wrote:
> > On Sun, Mar 06, 2022 at 03:51:00PM -0500, Nayna Jain wrote:
> > > 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 <[email protected]>
> > > Signed-off-by: Nayna Jain <[email protected]>
> > > ---
> > > 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
> > >
> >
> > There's zero tested-by's for this, i.e. cannot be applied before someone
> > has tested this. Mimi, do not mean to be rude, but I don't frankly
> > understand why you ask to pick a patch set that is *untested*.
> > So I generated a self-signed certificate:
> >
> > openssl req -x509 -out localhost.crt -keyout localhost.key \
> > -newkey rsa:2048 -nodes -sha256 \
> > -subj '/CN=localhost' -extensions EXT -config <( \
> > printf "[dn]\nCN=localhost\n[req]\ndistinguished_name = dn\n[EXT]\nsubjectAltName=DNS:localhost\nkeyUsage=digitalSignature\nextendedKeyUsage=serverAuth")
> >
> > (by courtesy of letsencrypt: https://letsencrypt.org/docs/certificates-for-localhost/)
> >
> > openssl x509 -in localhost.crt -out localhost.pem -outform PEM
> >
> > And starting with tinyconfig I added minimal options to enable this
> > feature. The config is attached.
> >
> > The end result is:
> >
> > make[2]: *** No rule to make target 'certs/extract-cert', needed by 'security/integrity/platform_certs/platform_certificate_list'. Stop.
> > make[1]: *** [scripts/Makefile.build:550: security/integrity] Error 2
> > make: *** [Makefile:1831: security] Error 2
>
> I've reviewed and tested this patch set each time it was posted last
> fall/winter. Recent changes were limited to the cover letter and patch
> description. Only recently was "extract_cert" moved to the certs/
> directory and not built automatically. The commit message says the
> move was because it wasn't being used outside the certs directory.
> Refer to commit 340a02535ee7 ("certs: move scripts/extract-cert to
> certs/").
>
> Masahiro Yamada would you be ok with reverting the move?
>
> thanks,
>
> Mimi

OK, so I can add your tested-by? It was missing, that's where the
concern came from.

BR, Jarkko