| Message ID | 20211124204714.82514-3-nayna@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | integrity: support including firmware ".platform" keys at build time | expand |
On Wed, 2021-11-24 at 15:47 -0500, Nayna Jain wrote: > A new function load_builtin_platform_cert() is defined to load compiled > in certificates onto the ".platform" keyring. > > Signed-off-by: Nayna Jain <nayna@linux.ibm.com> Thanks, Nayna. Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
On Wed, 2021-11-24 at 15:47 -0500, Nayna Jain wrote: > A new function load_builtin_platform_cert() is defined to load compiled > in certificates onto the ".platform" keyring. > > Signed-off-by: Nayna Jain <nayna@linux.ibm.com> This is way too non-verbose. It does not *really* describe what the patch does, e.g. it is completely any description that a kconfig option is added. You should describe all things that are going to happen as a consequence of applying this patch. It's not like the commit is just adding a function, and that's the whole story. Nit: always use imperative form: "Add a new function...". > --- > security/integrity/Kconfig | 10 +++++++ > security/integrity/Makefile | 17 +++++++++++- > security/integrity/digsig.c | 2 +- > security/integrity/integrity.h | 6 +++++ > .../integrity/platform_certs/platform_cert.S | 23 ++++++++++++++++ > .../platform_certs/platform_keyring.c | 26 +++++++++++++++++++ > 6 files changed, 82 insertions(+), 2 deletions(-) > 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/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) This export should be split into separate patch. > { > 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 > 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. > */ /Jarkko
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/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 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. */
A new function load_builtin_platform_cert() is defined to load compiled in certificates onto the ".platform" keyring. Signed-off-by: Nayna Jain <nayna@linux.ibm.com> --- security/integrity/Kconfig | 10 +++++++ security/integrity/Makefile | 17 +++++++++++- security/integrity/digsig.c | 2 +- security/integrity/integrity.h | 6 +++++ .../integrity/platform_certs/platform_cert.S | 23 ++++++++++++++++ .../platform_certs/platform_keyring.c | 26 +++++++++++++++++++ 6 files changed, 82 insertions(+), 2 deletions(-) create mode 100644 security/integrity/platform_certs/platform_cert.S