* [PATCH v6 1/3] certs: export load_certificate_list() to be used outside certs/
2021-12-23 1:39 [PATCH v6 0/3] integrity: support including firmware ".platform" keys at build time Nayna Jain
@ 2021-12-23 1:39 ` Nayna Jain
2021-12-29 1:29 ` Jarkko Sakkinen
2021-12-23 1:39 ` [PATCH v6 2/3] integrity: make integrity_keyring_from_id() non-static Nayna Jain
2021-12-23 1:39 ` [PATCH v6 3/3] integrity: support including firmware ".platform" keys at build time Nayna Jain
2 siblings, 1 reply; 6+ messages in thread
From: Nayna Jain @ 2021-12-23 1:39 UTC (permalink / raw)
To: linux-integrity, keyrings
Cc: dhowells, zohar, jarkko, linux-security-module, linux-kernel,
dimitri.ledkov, seth, Nayna Jain, kernel test robot
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. Now, export load_certificate_list() outside certs/ to be used
by other functions.
Reported-by: kernel test robot <lkp@intel.com> (auto build test ERROR)
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] 6+ messages in thread
* Re: [PATCH v6 1/3] certs: export load_certificate_list() to be used outside certs/
2021-12-23 1:39 ` [PATCH v6 1/3] certs: export load_certificate_list() to be used outside certs/ Nayna Jain
@ 2021-12-29 1:29 ` Jarkko Sakkinen
0 siblings, 0 replies; 6+ messages in thread
From: Jarkko Sakkinen @ 2021-12-29 1:29 UTC (permalink / raw)
To: Nayna Jain
Cc: linux-integrity, keyrings, dhowells, zohar, linux-security-module,
linux-kernel, dimitri.ledkov, seth, kernel test robot
On Wed, Dec 22, 2021 at 08:39:17PM -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. Now, export load_certificate_list() outside certs/ to be used
> by other functions.
It is exported to make callable by load_platform_certificate_list(). Making
anything callable by other functions by itself has no inherit value, and
does not serve as a stimulus to make such code change.
/Jarkko
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v6 2/3] integrity: make integrity_keyring_from_id() non-static
2021-12-23 1:39 [PATCH v6 0/3] integrity: support including firmware ".platform" keys at build time Nayna Jain
2021-12-23 1:39 ` [PATCH v6 1/3] certs: export load_certificate_list() to be used outside certs/ Nayna Jain
@ 2021-12-23 1:39 ` Nayna Jain
2021-12-23 1:39 ` [PATCH v6 3/3] integrity: support including firmware ".platform" keys at build time Nayna Jain
2 siblings, 0 replies; 6+ messages in thread
From: Nayna Jain @ 2021-12-23 1:39 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] 6+ messages in thread* [PATCH v6 3/3] integrity: support including firmware ".platform" keys at build time
2021-12-23 1:39 [PATCH v6 0/3] integrity: support including firmware ".platform" keys at build time Nayna Jain
2021-12-23 1:39 ` [PATCH v6 1/3] certs: export load_certificate_list() to be used outside certs/ Nayna Jain
2021-12-23 1:39 ` [PATCH v6 2/3] integrity: make integrity_keyring_from_id() non-static Nayna Jain
@ 2021-12-23 1:39 ` Nayna Jain
2021-12-29 1:33 ` Jarkko Sakkinen
2 siblings, 1 reply; 6+ messages in thread
From: Nayna Jain @ 2021-12-23 1:39 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.
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] 6+ messages in thread* Re: [PATCH v6 3/3] integrity: support including firmware ".platform" keys at build time
2021-12-23 1:39 ` [PATCH v6 3/3] integrity: support including firmware ".platform" keys at build time Nayna Jain
@ 2021-12-29 1:33 ` Jarkko Sakkinen
0 siblings, 0 replies; 6+ messages in thread
From: Jarkko Sakkinen @ 2021-12-29 1:33 UTC (permalink / raw)
To: Nayna Jain
Cc: linux-integrity, keyrings, dhowells, zohar, linux-security-module,
linux-kernel, dimitri.ledkov, seth
On Wed, Dec 22, 2021 at 08:39:19PM -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.
>
> 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);
Would not hurt to briefly explain the chosen initcall level in the commit
message.
> +
> /*
> * Create the trusted keyrings.
> */
> --
> 2.27.0
>
/Jarkko
^ permalink raw reply [flat|nested] 6+ messages in thread