[PATCH v2 1/3] certs: define a trusted platform keyring

[PATCH v2 1/3] certs: define a trusted platform keyring

[Nayna Jain]

The kernel can be supplied in SEEPROM or lockable flash memory in embedded
devices. Some devices may not support secure boot, but the kernel is
trusted because the image is stored in protected memory. That kernel may
need to kexec additional kernels, it may be used as a bootloader, for
example, or it may need to kexec a crashdump kernel. In such cases, it may
want to verify the signature of the next kernel.

The kernel, however, cannot directly verify platform keys, and an
administrator may therefore not want to trust them for arbitrary usage.
In order to differentiate platform keys from other keys and provide the
necessary separation of trust, the kernel needs an additional keyring to
store platform keys.

This patch implements a built-in list of certificates that are loaded onto
the trusted platform keyring named ".platform_keys" to facilitate signature
verification during kexec. Because the platform keyring are builtin, it
cannot be updated from userspace.

This keyring can be enabled by setting CONFIG_PLATFORM_KEYRING. The
platform certificate can be provided using CONFIG_PLATFORM_TRUSTED_KEYS.

Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
---
Changelog:

v2:

* Include David Howell's feedback:
 * Fix the indentation
* Fix the patch description per line length as suggested by Mimi

 certs/Kconfig               | 17 ++++++++++++++
 certs/Makefile              | 13 +++++++++++
 certs/system_certificates.S | 20 +++++++++++++++++
 certs/system_keyring.c      | 55 ++++++++++++++++++++++++++++++++++++++-------
 4 files changed, 97 insertions(+), 8 deletions(-)

diff --git a/certs/Kconfig b/certs/Kconfig
index 5f7663df6e8e..608a4358a25e 100644
--- a/certs/Kconfig
+++ b/certs/Kconfig
@@ -83,4 +83,21 @@ config SYSTEM_BLACKLIST_HASH_LIST
 	  wrapper to incorporate the list into the kernel.  Each <hash> should
 	  be a string of hex digits.
 
+config PLATFORM_KEYRING
+        bool "Provide keyring for platform trusted keys"
+        depends on KEYS
+        depends on ASYMMETRIC_KEY_TYPE
+        help
+	  Provide a separate, distinct keyring for platform trusted keys, which
+	  the kernel automatically populates during initialization from values
+	  embedded during build, used for verifying the kexec'ed kernel image
+	  and, possibly, the initramfs signature.
+
+config PLATFORM_TRUSTED_KEYS
+	string "Platform/Firmware trusted X.509 certs."
+	depends on PLATFORM_KEYRING
+	help
+	  Provide the filename of a PEM-formatted file containing the platform
+	  trusted X.509 certificates to be loaded in the platform keyring.
+
 endmenu
diff --git a/certs/Makefile b/certs/Makefile
index 5d0999b9e21b..680903725031 100644
--- a/certs/Makefile
+++ b/certs/Makefile
@@ -104,3 +104,16 @@ targets += signing_key.x509
 $(obj)/signing_key.x509: scripts/extract-cert $(X509_DEP) FORCE
 	$(call if_changed,extract_certs,$(MODULE_SIG_KEY_SRCPREFIX)$(CONFIG_MODULE_SIG_KEY))
 endif # CONFIG_MODULE_SIG
+
+
+ifeq ($(CONFIG_PLATFORM_KEYRING),y)
+
+$(eval $(call config_filename,PLATFORM_TRUSTED_KEYS))
+
+# GCC doesn't include .incbin files in -MD generated dependencies (PR#66871)
+$(obj)/system_certificates.o: $(obj)/platform_certificate_list
+
+targets += platform_certificate_list
+$(obj)/platform_certificate_list: scripts/extract-cert $(PLATFORM_TRUSTED_KEYS_FILENAME) FORCE
+	$(call if_changed,extract_certs,$(CONFIG_PLATFORM_TRUSTED_KEYS))
+endif # CONFIG_PLATFORM_KEYRING
diff --git a/certs/system_certificates.S b/certs/system_certificates.S
index 3918ff7235ed..b0eb448ee617 100644
--- a/certs/system_certificates.S
+++ b/certs/system_certificates.S
@@ -14,6 +14,15 @@ __cert_list_start:
 	.incbin "certs/x509_certificate_list"
 __cert_list_end:
 
+#ifdef CONFIG_PLATFORM_KEYRING
+	.align 8
+	.globl VMLINUX_SYMBOL(platform_certificate_list)
+VMLINUX_SYMBOL(platform_certificate_list):
+__platform_cert_list_start:
+	.incbin "certs/platform_certificate_list"
+__platform_cert_list_end:
+#endif /* CONFIG_PLATFORM_KEYRING */
+
 #ifdef CONFIG_SYSTEM_EXTRA_CERTIFICATE
 	.globl VMLINUX_SYMBOL(system_extra_cert)
 	.size system_extra_cert, CONFIG_SYSTEM_EXTRA_CERTIFICATE_SIZE
@@ -35,3 +44,14 @@ VMLINUX_SYMBOL(system_certificate_list_size):
 #else
 	.long __cert_list_end - __cert_list_start
 #endif
+
+#ifdef CONFIG_PLATFORM_KEYRING
+	.align 8
+	.globl VMLINUX_SYMBOL(platform_certificate_list_size)
+VMLINUX_SYMBOL(platform_certificate_list_size):
+#ifdef CONFIG_64BIT
+	.quad __platform_cert_list_end - __platform_cert_list_start
+#else
+	.long __platform_cert_list_end - __platform_cert_list_start
+#endif
+#endif /* CONFIG_PLATFORM_KEYRING */
diff --git a/certs/system_keyring.c b/certs/system_keyring.c
index 6251d1b27f0c..594b4986a081 100644
--- a/certs/system_keyring.c
+++ b/certs/system_keyring.c
@@ -19,14 +19,22 @@
 #include <keys/system_keyring.h>
 #include <crypto/pkcs7.h>
 
+#define BUILTIN_TRUSTED_KEYRING	0
+#define PLATFORM_KEYRING	1
+
 static struct key *builtin_trusted_keys;
 #ifdef CONFIG_SECONDARY_TRUSTED_KEYRING
 static struct key *secondary_trusted_keys;
 #endif
+#ifdef CONFIG_PLATFORM_KEYRING
+static struct key *platform_keys __ro_after_init;
+#endif
 
 extern __initconst const u8 system_certificate_list[];
 extern __initconst const unsigned long system_certificate_list_size;
 
+extern __initconst const u8 platform_certificate_list[];
+extern __initconst const unsigned long platform_certificate_list_size;
 /**
  * restrict_link_to_builtin_trusted - Restrict keyring addition by built in CA
  *
@@ -123,6 +131,18 @@ static __init int system_trusted_keyring_init(void)
 		panic("Can't link trusted keyrings\n");
 #endif
 
+#ifdef CONFIG_PLATFORM_KEYRING
+	platform_keys =
+		keyring_alloc(".platform_keys",
+			      KUIDT_INIT(0), KGIDT_INIT(0), current_cred(),
+			      ((KEY_POS_ALL & ~KEY_POS_SETATTR) |
+			      KEY_USR_VIEW | KEY_USR_READ | KEY_USR_SEARCH),
+			      KEY_ALLOC_NOT_IN_QUOTA,
+			      NULL, NULL);
+	if (IS_ERR(platform_keys))
+		panic("Can't allocate platform keyring\n");
+#endif
+
 	return 0;
 }
 
@@ -132,18 +152,19 @@ static __init int system_trusted_keyring_init(void)
 device_initcall(system_trusted_keyring_init);
 
 /*
- * Load the compiled-in list of X.509 certificates.
+ * Load the certificates to the keyring.
  */
-static __init int load_system_certificate_list(void)
+static __init int load_certificate_list(const u8 *p, unsigned long size,
+		struct key *keyring)
 {
 	key_ref_t key;
-	const u8 *p, *end;
+	const u8 *end;
 	size_t plen;
 
-	pr_notice("Loading compiled-in X.509 certificates\n");
+	pr_notice("Loading compiled-in X.509 certificates to %s\n",
+			keyring->description);
 
-	p = system_certificate_list;
-	end = p + system_certificate_list_size;
+	end = p + size;
 	while (p < end) {
 		/* Each cert begins with an ASN.1 SEQUENCE tag and must be more
 		 * than 256 bytes in size.
@@ -158,7 +179,7 @@ static __init int load_system_certificate_list(void)
 		if (plen > end - p)
 			goto dodgy_cert;
 
-		key = key_create_or_update(make_key_ref(builtin_trusted_keys, 1),
+		key = key_create_or_update(make_key_ref(keyring, 1),
 					   "asymmetric",
 					   NULL,
 					   p,
@@ -185,7 +206,25 @@ static __init int load_system_certificate_list(void)
 	pr_err("Problem parsing in-kernel X.509 certificate list\n");
 	return 0;
 }
-late_initcall(load_system_certificate_list);
+
+/*
+ * Load the compiled-in list of system and platform X.509 certificates.
+ */
+static __init int load_compiled_certificate_list(void)
+{
+	/* Loading certs in builtin keyring */
+	load_certificate_list(system_certificate_list,
+			system_certificate_list_size, builtin_trusted_keys);
+
+#ifdef CONFIG_PLATFORM_KEYRING
+	/* Loading certs in platform keyring */
+	load_certificate_list(platform_certificate_list,
+			platform_certificate_list_size, platform_keys);
+#endif
+
+	return 0;
+}
+late_initcall(load_compiled_certificate_list);
 
 #ifdef CONFIG_SYSTEM_DATA_VERIFICATION
 
-- 
2.13.6

[PATCH v2 2/3] keys: export find_keyring_by_name()

[Nayna Jain]

This patch exports the function find_keyring_by_name() to be used by
other subsystems.

Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
---
Changelog:

v2:
* Fix the patch description per line length as suggested by Mimi

 include/linux/key.h      | 2 ++
 security/keys/internal.h | 2 --
 security/keys/keyring.c  | 1 +
 3 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/include/linux/key.h b/include/linux/key.h
index e58ee10f6e58..c8d332d4103c 100644
--- a/include/linux/key.h
+++ b/include/linux/key.h
@@ -346,6 +346,8 @@ static inline key_serial_t key_serial(const struct key *key)
 
 extern void key_set_timeout(struct key *, unsigned);
 
+extern struct key *find_keyring_by_name(const char *name, bool uid_keyring);
+
 /*
  * The permissions required on a key that we're looking up.
  */
diff --git a/security/keys/internal.h b/security/keys/internal.h
index 9f8208dc0e55..8aa8d347a1ab 100644
--- a/security/keys/internal.h
+++ b/security/keys/internal.h
@@ -141,8 +141,6 @@ extern key_ref_t keyring_search_aux(key_ref_t keyring_ref,
 extern key_ref_t search_my_process_keyrings(struct keyring_search_context *ctx);
 extern key_ref_t search_process_keyrings(struct keyring_search_context *ctx);
 
-extern struct key *find_keyring_by_name(const char *name, bool uid_keyring);
-
 extern int install_user_keyrings(void);
 extern int install_thread_keyring_to_cred(struct cred *);
 extern int install_process_keyring_to_cred(struct cred *);
diff --git a/security/keys/keyring.c b/security/keys/keyring.c
index 41bcf57e96f2..4b9c3f1166d1 100644
--- a/security/keys/keyring.c
+++ b/security/keys/keyring.c
@@ -1152,6 +1152,7 @@ struct key *find_keyring_by_name(const char *name, bool uid_keyring)
 	read_unlock(&keyring_name_lock);
 	return keyring;
 }
+EXPORT_SYMBOL(find_keyring_by_name);
 
 static int keyring_detect_cycle_iterator(const void *object,
 					 void *iterator_data)
-- 
2.13.6

[PATCH v2 3/3] ima: support platform keyring for kernel appraisal

[Nayna Jain]

Distros may sign the kernel images and, possibly, the initramfs with
platform trusted keys. On secure boot enabled systems or embedded devices,
these signatures are to be validated using keys on the platform keyring.

This patch enables IMA-appraisal to access the platform keyring, based on a
new Kconfig option "IMA_USE_PLATFORM_KEYRING".

Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
---
Changelog:

v2:
* Rename integrity_load_keyring() to integrity_find_keyring()
* Fix the patch description per line length as suggested by Mimi

 security/integrity/digsig.c           | 15 +++++++++++++++
 security/integrity/ima/Kconfig        | 10 ++++++++++
 security/integrity/ima/ima_appraise.c | 22 +++++++++++++++++-----
 security/integrity/ima/ima_init.c     |  4 ++++
 security/integrity/integrity.h        | 17 ++++++++++++++++-
 5 files changed, 62 insertions(+), 6 deletions(-)

diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
index 6f9e4ce568cd..cfeb977bced9 100644
--- a/security/integrity/digsig.c
+++ b/security/integrity/digsig.c
@@ -34,6 +34,8 @@ static const char *keyring_name[INTEGRITY_KEYRING_MAX] = {
 	".ima",
 #endif
 	"_module",
+	".platform_keys",
+
 };
 
 #ifdef CONFIG_INTEGRITY_TRUSTED_KEYRING
@@ -78,6 +80,19 @@ int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen,
 	return -EOPNOTSUPP;
 }
 
+#ifdef CONFIG_IMA_USE_PLATFORM_KEYRING
+int __init integrity_find_keyring(const unsigned int id)
+{
+
+	keyring[id] = find_keyring_by_name(keyring_name[id], 0);
+	if (IS_ERR(keyring[id]))
+		if (PTR_ERR(keyring[id]) != -ENOKEY)
+			return PTR_ERR(keyring[id]);
+	return 0;
+
+}
+#endif
+
 int __init integrity_init_keyring(const unsigned int id)
 {
 	const struct cred *cred = current_cred();
diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
index 35ef69312811..2e89d4f8a364 100644
--- a/security/integrity/ima/Kconfig
+++ b/security/integrity/ima/Kconfig
@@ -227,3 +227,13 @@ config IMA_APPRAISE_SIGNED_INIT
 	default n
 	help
 	   This option requires user-space init to be signed.
+
+config IMA_USE_PLATFORM_KEYRING
+       bool "IMA uses keys from Platform Keyring for verification"
+       depends on PLATFORM_KEYRING
+       depends on IMA_APPRAISE
+       depends on INTEGRITY_ASYMMETRIC_KEYS
+       default n
+       help
+	  This option enables IMA appraisal to look for the platform
+	  trusted keys in .platform_keys keyring.
diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index f2803a40ff82..5fec29f40595 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -276,13 +276,25 @@ int ima_appraise_measurement(enum ima_hooks func,
 					     (const char *)xattr_value, rc,
 					     iint->ima_hash->digest,
 					     iint->ima_hash->length);
-		if (rc == -EOPNOTSUPP) {
-			status = INTEGRITY_UNKNOWN;
-		} else if (rc) {
+		if (rc) {
+			if (rc == -EOPNOTSUPP) {
+				status = INTEGRITY_UNKNOWN;
+				break;
+			}
+			if (func == KEXEC_KERNEL_CHECK) {
+				rc = integrity_digsig_verify(
+						INTEGRITY_KEYRING_PLATFORM,
+						(const char *)xattr_value,
+						xattr_len,
+						iint->ima_hash->digest,
+						iint->ima_hash->length);
+				if (!rc) {
+					status = INTEGRITY_PASS;
+					break;
+				}
+			}
 			cause = "invalid-signature";
 			status = INTEGRITY_FAIL;
-		} else {
-			status = INTEGRITY_PASS;
 		}
 		break;
 	default:
diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c
index 29b72cd2502e..5778647c6bc4 100644
--- a/security/integrity/ima/ima_init.c
+++ b/security/integrity/ima/ima_init.c
@@ -122,6 +122,10 @@ int __init ima_init(void)
 	if (rc)
 		return rc;
 
+	rc = integrity_find_keyring(INTEGRITY_KEYRING_PLATFORM);
+	if (rc)
+		pr_info("Platform keyring is not found. (rc=%d)\n", rc);
+
 	rc = ima_init_crypto();
 	if (rc)
 		return rc;
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index 50a8e3365df7..3d3b7171ead2 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -136,13 +136,23 @@ int integrity_kernel_read(struct file *file, loff_t offset,
 #define INTEGRITY_KEYRING_EVM		0
 #define INTEGRITY_KEYRING_IMA		1
 #define INTEGRITY_KEYRING_MODULE	2
-#define INTEGRITY_KEYRING_MAX		3
+#define INTEGRITY_KEYRING_PLATFORM	3
+#define INTEGRITY_KEYRING_MAX		4
 
 #ifdef CONFIG_INTEGRITY_SIGNATURE
 
 int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen,
 			    const char *digest, int digestlen);
 
+#ifdef CONFIG_IMA_USE_PLATFORM_KEYRING
+int __init integrity_find_keyring(const unsigned int id);
+#else
+static inline int __init integrity_find_keyring(const unsigned int id)
+{
+	return 0;
+}
+#endif
+
 int __init integrity_init_keyring(const unsigned int id);
 int __init integrity_load_x509(const unsigned int id, const char *path);
 #else
@@ -154,6 +164,11 @@ static inline int integrity_digsig_verify(const unsigned int id,
 	return -EOPNOTSUPP;
 }
 
+static inline int __init integrity_find_keyring(const unsigned int id)
+{
+	return 0;
+}
+
 static inline int integrity_init_keyring(const unsigned int id)
 {
 	return 0;
-- 
2.13.6

Re: [PATCH v2 3/3] ima: support platform keyring for kernel appraisal

[Mimi Zohar]

On Fri, 2018-03-09 at 21:08 +0530, Nayna Jain wrote:
> Distros may sign the kernel images and, possibly, the initramfs with
> platform trusted keys. On secure boot enabled systems or embedded devices,
> these signatures are to be validated using keys on the platform keyring.
> 
> This patch enables IMA-appraisal to access the platform keyring, based on a
> new Kconfig option "IMA_USE_PLATFORM_KEYRING".
> 
> Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>

Thanks, Nayna!

Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>


> ---
> Changelog:
> 
> v2:
> * Rename integrity_load_keyring() to integrity_find_keyring()
> * Fix the patch description per line length as suggested by Mimi
> 
>  security/integrity/digsig.c           | 15 +++++++++++++++
>  security/integrity/ima/Kconfig        | 10 ++++++++++
>  security/integrity/ima/ima_appraise.c | 22 +++++++++++++++++-----
>  security/integrity/ima/ima_init.c     |  4 ++++
>  security/integrity/integrity.h        | 17 ++++++++++++++++-
>  5 files changed, 62 insertions(+), 6 deletions(-)
> 
> diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
> index 6f9e4ce568cd..cfeb977bced9 100644
> --- a/security/integrity/digsig.c
> +++ b/security/integrity/digsig.c
> @@ -34,6 +34,8 @@ static const char *keyring_name[INTEGRITY_KEYRING_MAX] = {
>  	".ima",
>  #endif
>  	"_module",
> +	".platform_keys",
> +
>  };
> 
>  #ifdef CONFIG_INTEGRITY_TRUSTED_KEYRING
> @@ -78,6 +80,19 @@ int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen,
>  	return -EOPNOTSUPP;
>  }
> 
> +#ifdef CONFIG_IMA_USE_PLATFORM_KEYRING
> +int __init integrity_find_keyring(const unsigned int id)
> +{
> +
> +	keyring[id] = find_keyring_by_name(keyring_name[id], 0);
> +	if (IS_ERR(keyring[id]))
> +		if (PTR_ERR(keyring[id]) != -ENOKEY)
> +			return PTR_ERR(keyring[id]);
> +	return 0;
> +
> +}
> +#endif
> +
>  int __init integrity_init_keyring(const unsigned int id)
>  {
>  	const struct cred *cred = current_cred();
> diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
> index 35ef69312811..2e89d4f8a364 100644
> --- a/security/integrity/ima/Kconfig
> +++ b/security/integrity/ima/Kconfig
> @@ -227,3 +227,13 @@ config IMA_APPRAISE_SIGNED_INIT
>  	default n
>  	help
>  	   This option requires user-space init to be signed.
> +
> +config IMA_USE_PLATFORM_KEYRING
> +       bool "IMA uses keys from Platform Keyring for verification"
> +       depends on PLATFORM_KEYRING
> +       depends on IMA_APPRAISE
> +       depends on INTEGRITY_ASYMMETRIC_KEYS
> +       default n
> +       help
> +	  This option enables IMA appraisal to look for the platform
> +	  trusted keys in .platform_keys keyring.
> diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
> index f2803a40ff82..5fec29f40595 100644
> --- a/security/integrity/ima/ima_appraise.c
> +++ b/security/integrity/ima/ima_appraise.c
> @@ -276,13 +276,25 @@ int ima_appraise_measurement(enum ima_hooks func,
>  					     (const char *)xattr_value, rc,
>  					     iint->ima_hash->digest,
>  					     iint->ima_hash->length);
> -		if (rc == -EOPNOTSUPP) {
> -			status = INTEGRITY_UNKNOWN;
> -		} else if (rc) {
> +		if (rc) {
> +			if (rc == -EOPNOTSUPP) {
> +				status = INTEGRITY_UNKNOWN;
> +				break;
> +			}
> +			if (func == KEXEC_KERNEL_CHECK) {
> +				rc = integrity_digsig_verify(
> +						INTEGRITY_KEYRING_PLATFORM,
> +						(const char *)xattr_value,
> +						xattr_len,
> +						iint->ima_hash->digest,
> +						iint->ima_hash->length);
> +				if (!rc) {
> +					status = INTEGRITY_PASS;
> +					break;
> +				}
> +			}
>  			cause = "invalid-signature";
>  			status = INTEGRITY_FAIL;
> -		} else {
> -			status = INTEGRITY_PASS;
>  		}
>  		break;
>  	default:
> diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c
> index 29b72cd2502e..5778647c6bc4 100644
> --- a/security/integrity/ima/ima_init.c
> +++ b/security/integrity/ima/ima_init.c
> @@ -122,6 +122,10 @@ int __init ima_init(void)
>  	if (rc)
>  		return rc;
> 
> +	rc = integrity_find_keyring(INTEGRITY_KEYRING_PLATFORM);
> +	if (rc)
> +		pr_info("Platform keyring is not found. (rc=%d)\n", rc);
> +
>  	rc = ima_init_crypto();
>  	if (rc)
>  		return rc;
> diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
> index 50a8e3365df7..3d3b7171ead2 100644
> --- a/security/integrity/integrity.h
> +++ b/security/integrity/integrity.h
> @@ -136,13 +136,23 @@ int integrity_kernel_read(struct file *file, loff_t offset,
>  #define INTEGRITY_KEYRING_EVM		0
>  #define INTEGRITY_KEYRING_IMA		1
>  #define INTEGRITY_KEYRING_MODULE	2
> -#define INTEGRITY_KEYRING_MAX		3
> +#define INTEGRITY_KEYRING_PLATFORM	3
> +#define INTEGRITY_KEYRING_MAX		4
> 
>  #ifdef CONFIG_INTEGRITY_SIGNATURE
> 
>  int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen,
>  			    const char *digest, int digestlen);
> 
> +#ifdef CONFIG_IMA_USE_PLATFORM_KEYRING
> +int __init integrity_find_keyring(const unsigned int id);
> +#else
> +static inline int __init integrity_find_keyring(const unsigned int id)
> +{
> +	return 0;
> +}
> +#endif
> +
>  int __init integrity_init_keyring(const unsigned int id);
>  int __init integrity_load_x509(const unsigned int id, const char *path);
>  #else
> @@ -154,6 +164,11 @@ static inline int integrity_digsig_verify(const unsigned int id,
>  	return -EOPNOTSUPP;
>  }
> 
> +static inline int __init integrity_find_keyring(const unsigned int id)
> +{
> +	return 0;
> +}
> +
>  static inline int integrity_init_keyring(const unsigned int id)
>  {
>  	return 0;

Re: [PATCH v2 1/3] certs: define a trusted platform keyring

[Mimi Zohar]

On Fri, 2018-03-09 at 21:08 +0530, Nayna Jain wrote:
> The kernel can be supplied in SEEPROM or lockable flash memory in embedded
> devices. Some devices may not support secure boot, but the kernel is
> trusted because the image is stored in protected memory. That kernel may
> need to kexec additional kernels, it may be used as a bootloader, for
> example, or it may need to kexec a crashdump kernel. In such cases, it may
> want to verify the signature of the next kernel.
> 
> The kernel, however, cannot directly verify platform keys, and an
> administrator may therefore not want to trust them for arbitrary usage.
> In order to differentiate platform keys from other keys and provide the
> necessary separation of trust, the kernel needs an additional keyring to
> store platform keys.
> 
> This patch implements a built-in list of certificates that are loaded onto
> the trusted platform keyring named ".platform_keys" to facilitate signature
> verification during kexec. Because the platform keyring are builtin, it
> cannot be updated from userspace.
> 
> This keyring can be enabled by setting CONFIG_PLATFORM_KEYRING. The
> platform certificate can be provided using CONFIG_PLATFORM_TRUSTED_KEYS.
> 
> Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>

Please add my Reviewed-by: Mimi Zohar <zohar@linux.vnet.ibm.com> on
this and 2/3.

Mimi

> ---
> Changelog:
> 
> v2:
> 
> * Include David Howell's feedback:
>  * Fix the indentation
> * Fix the patch description per line length as suggested by Mimi
> 
>  certs/Kconfig               | 17 ++++++++++++++
>  certs/Makefile              | 13 +++++++++++
>  certs/system_certificates.S | 20 +++++++++++++++++
>  certs/system_keyring.c      | 55 ++++++++++++++++++++++++++++++++++++++-------
>  4 files changed, 97 insertions(+), 8 deletions(-)
> 
> diff --git a/certs/Kconfig b/certs/Kconfig
> index 5f7663df6e8e..608a4358a25e 100644
> --- a/certs/Kconfig
> +++ b/certs/Kconfig
> @@ -83,4 +83,21 @@ config SYSTEM_BLACKLIST_HASH_LIST
>  	  wrapper to incorporate the list into the kernel.  Each <hash> should
>  	  be a string of hex digits.
> 
> +config PLATFORM_KEYRING
> +        bool "Provide keyring for platform trusted keys"
> +        depends on KEYS
> +        depends on ASYMMETRIC_KEY_TYPE
> +        help
> +	  Provide a separate, distinct keyring for platform trusted keys, which
> +	  the kernel automatically populates during initialization from values
> +	  embedded during build, used for verifying the kexec'ed kernel image
> +	  and, possibly, the initramfs signature.
> +
> +config PLATFORM_TRUSTED_KEYS
> +	string "Platform/Firmware trusted X.509 certs."
> +	depends on PLATFORM_KEYRING
> +	help
> +	  Provide the filename of a PEM-formatted file containing the platform
> +	  trusted X.509 certificates to be loaded in the platform keyring.
> +
>  endmenu
> diff --git a/certs/Makefile b/certs/Makefile
> index 5d0999b9e21b..680903725031 100644
> --- a/certs/Makefile
> +++ b/certs/Makefile
> @@ -104,3 +104,16 @@ targets += signing_key.x509
>  $(obj)/signing_key.x509: scripts/extract-cert $(X509_DEP) FORCE
>  	$(call if_changed,extract_certs,$(MODULE_SIG_KEY_SRCPREFIX)$(CONFIG_MODULE_SIG_KEY))
>  endif # CONFIG_MODULE_SIG
> +
> +
> +ifeq ($(CONFIG_PLATFORM_KEYRING),y)
> +
> +$(eval $(call config_filename,PLATFORM_TRUSTED_KEYS))
> +
> +# GCC doesn't include .incbin files in -MD generated dependencies (PR#66871)
> +$(obj)/system_certificates.o: $(obj)/platform_certificate_list
> +
> +targets += platform_certificate_list
> +$(obj)/platform_certificate_list: scripts/extract-cert $(PLATFORM_TRUSTED_KEYS_FILENAME) FORCE
> +	$(call if_changed,extract_certs,$(CONFIG_PLATFORM_TRUSTED_KEYS))
> +endif # CONFIG_PLATFORM_KEYRING
> diff --git a/certs/system_certificates.S b/certs/system_certificates.S
> index 3918ff7235ed..b0eb448ee617 100644
> --- a/certs/system_certificates.S
> +++ b/certs/system_certificates.S
> @@ -14,6 +14,15 @@ __cert_list_start:
>  	.incbin "certs/x509_certificate_list"
>  __cert_list_end:
> 
> +#ifdef CONFIG_PLATFORM_KEYRING
> +	.align 8
> +	.globl VMLINUX_SYMBOL(platform_certificate_list)
> +VMLINUX_SYMBOL(platform_certificate_list):
> +__platform_cert_list_start:
> +	.incbin "certs/platform_certificate_list"
> +__platform_cert_list_end:
> +#endif /* CONFIG_PLATFORM_KEYRING */
> +
>  #ifdef CONFIG_SYSTEM_EXTRA_CERTIFICATE
>  	.globl VMLINUX_SYMBOL(system_extra_cert)
>  	.size system_extra_cert, CONFIG_SYSTEM_EXTRA_CERTIFICATE_SIZE
> @@ -35,3 +44,14 @@ VMLINUX_SYMBOL(system_certificate_list_size):
>  #else
>  	.long __cert_list_end - __cert_list_start
>  #endif
> +
> +#ifdef CONFIG_PLATFORM_KEYRING
> +	.align 8
> +	.globl VMLINUX_SYMBOL(platform_certificate_list_size)
> +VMLINUX_SYMBOL(platform_certificate_list_size):
> +#ifdef CONFIG_64BIT
> +	.quad __platform_cert_list_end - __platform_cert_list_start
> +#else
> +	.long __platform_cert_list_end - __platform_cert_list_start
> +#endif
> +#endif /* CONFIG_PLATFORM_KEYRING */
> diff --git a/certs/system_keyring.c b/certs/system_keyring.c
> index 6251d1b27f0c..594b4986a081 100644
> --- a/certs/system_keyring.c
> +++ b/certs/system_keyring.c
> @@ -19,14 +19,22 @@
>  #include <keys/system_keyring.h>
>  #include <crypto/pkcs7.h>
> 
> +#define BUILTIN_TRUSTED_KEYRING	0
> +#define PLATFORM_KEYRING	1
> +
>  static struct key *builtin_trusted_keys;
>  #ifdef CONFIG_SECONDARY_TRUSTED_KEYRING
>  static struct key *secondary_trusted_keys;
>  #endif
> +#ifdef CONFIG_PLATFORM_KEYRING
> +static struct key *platform_keys __ro_after_init;
> +#endif
> 
>  extern __initconst const u8 system_certificate_list[];
>  extern __initconst const unsigned long system_certificate_list_size;
> 
> +extern __initconst const u8 platform_certificate_list[];
> +extern __initconst const unsigned long platform_certificate_list_size;
>  /**
>   * restrict_link_to_builtin_trusted - Restrict keyring addition by built in CA
>   *
> @@ -123,6 +131,18 @@ static __init int system_trusted_keyring_init(void)
>  		panic("Can't link trusted keyrings\n");
>  #endif
> 
> +#ifdef CONFIG_PLATFORM_KEYRING
> +	platform_keys =
> +		keyring_alloc(".platform_keys",
> +			      KUIDT_INIT(0), KGIDT_INIT(0), current_cred(),
> +			      ((KEY_POS_ALL & ~KEY_POS_SETATTR) |
> +			      KEY_USR_VIEW | KEY_USR_READ | KEY_USR_SEARCH),
> +			      KEY_ALLOC_NOT_IN_QUOTA,
> +			      NULL, NULL);
> +	if (IS_ERR(platform_keys))
> +		panic("Can't allocate platform keyring\n");
> +#endif
> +
>  	return 0;
>  }
> 
> @@ -132,18 +152,19 @@ static __init int system_trusted_keyring_init(void)
>  device_initcall(system_trusted_keyring_init);
> 
>  /*
> - * Load the compiled-in list of X.509 certificates.
> + * Load the certificates to the keyring.
>   */
> -static __init int load_system_certificate_list(void)
> +static __init int load_certificate_list(const u8 *p, unsigned long size,
> +		struct key *keyring)
>  {
>  	key_ref_t key;
> -	const u8 *p, *end;
> +	const u8 *end;
>  	size_t plen;
> 
> -	pr_notice("Loading compiled-in X.509 certificates\n");
> +	pr_notice("Loading compiled-in X.509 certificates to %s\n",
> +			keyring->description);
> 
> -	p = system_certificate_list;
> -	end = p + system_certificate_list_size;
> +	end = p + size;
>  	while (p < end) {
>  		/* Each cert begins with an ASN.1 SEQUENCE tag and must be more
>  		 * than 256 bytes in size.
> @@ -158,7 +179,7 @@ static __init int load_system_certificate_list(void)
>  		if (plen > end - p)
>  			goto dodgy_cert;
> 
> -		key = key_create_or_update(make_key_ref(builtin_trusted_keys, 1),
> +		key = key_create_or_update(make_key_ref(keyring, 1),
>  					   "asymmetric",
>  					   NULL,
>  					   p,
> @@ -185,7 +206,25 @@ static __init int load_system_certificate_list(void)
>  	pr_err("Problem parsing in-kernel X.509 certificate list\n");
>  	return 0;
>  }
> -late_initcall(load_system_certificate_list);
> +
> +/*
> + * Load the compiled-in list of system and platform X.509 certificates.
> + */
> +static __init int load_compiled_certificate_list(void)
> +{
> +	/* Loading certs in builtin keyring */
> +	load_certificate_list(system_certificate_list,
> +			system_certificate_list_size, builtin_trusted_keys);
> +
> +#ifdef CONFIG_PLATFORM_KEYRING
> +	/* Loading certs in platform keyring */
> +	load_certificate_list(platform_certificate_list,
> +			platform_certificate_list_size, platform_keys);
> +#endif
> +
> +	return 0;
> +}
> +late_initcall(load_compiled_certificate_list);
> 
>  #ifdef CONFIG_SYSTEM_DATA_VERIFICATION
> 

Re: [PATCH v2 2/3] keys: export find_keyring_by_name()

[Mimi Zohar]

Hi Nayna,

On Fri, 2018-03-09 at 21:08 +0530, Nayna Jain wrote:
> This patch exports the function find_keyring_by_name() to be used by
> other subsystems.

Looking this patch over again, I realize that exported functions must
be prefixed with the subsystem name.  I'm also a bit concerned with
exporting find_keyring_by_name().  David, any comments?

Perhaps it would be better if IMA creates the .platform keyring so
that it has access to the keyring id instead.

Mimi
  
> 
> Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
> ---
> Changelog:
> 
> v2:
> * Fix the patch description per line length as suggested by Mimi
> 
>  include/linux/key.h      | 2 ++
>  security/keys/internal.h | 2 --
>  security/keys/keyring.c  | 1 +
>  3 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/key.h b/include/linux/key.h
> index e58ee10f6e58..c8d332d4103c 100644
> --- a/include/linux/key.h
> +++ b/include/linux/key.h
> @@ -346,6 +346,8 @@ static inline key_serial_t key_serial(const struct key *key)
> 
>  extern void key_set_timeout(struct key *, unsigned);
> 
> +extern struct key *find_keyring_by_name(const char *name, bool uid_keyring);
> +
>  /*
>   * The permissions required on a key that we're looking up.
>   */
> diff --git a/security/keys/internal.h b/security/keys/internal.h
> index 9f8208dc0e55..8aa8d347a1ab 100644
> --- a/security/keys/internal.h
> +++ b/security/keys/internal.h
> @@ -141,8 +141,6 @@ extern key_ref_t keyring_search_aux(key_ref_t keyring_ref,
>  extern key_ref_t search_my_process_keyrings(struct keyring_search_context *ctx);
>  extern key_ref_t search_process_keyrings(struct keyring_search_context *ctx);
> 
> -extern struct key *find_keyring_by_name(const char *name, bool uid_keyring);
> -
>  extern int install_user_keyrings(void);
>  extern int install_thread_keyring_to_cred(struct cred *);
>  extern int install_process_keyring_to_cred(struct cred *);
> diff --git a/security/keys/keyring.c b/security/keys/keyring.c
> index 41bcf57e96f2..4b9c3f1166d1 100644
> --- a/security/keys/keyring.c
> +++ b/security/keys/keyring.c
> @@ -1152,6 +1152,7 @@ struct key *find_keyring_by_name(const char *name, bool uid_keyring)
>  	read_unlock(&keyring_name_lock);
>  	return keyring;
>  }
> +EXPORT_SYMBOL(find_keyring_by_name);
> 
>  static int keyring_detect_cycle_iterator(const void *object,
>  					 void *iterator_data)