[PATCH 0/7] add platform/firmware keys support for kernel verification by IMA

[PATCH 0/7] add platform/firmware keys support for kernel verification by IMA

[Nayna Jain]

On secure boot enabled systems, a verified kernel may need to kexec
additional kernels. For example, it may be used as a bootloader needing
to kexec a target kernel or it may need to kexec a crashdump kernel.
In such cases, it may want to verify the signature of the next kernel
image.

It is possible that the new kernel image is signed with third party keys
which are stored as platform or firmware keys in the 'db' variable. The
kernel, however, can not directly verify these 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/firmware keys.

The secure boot key database is expected to store the keys as EFI
Signature List(ESL). The patch set uses David Howells and Josh Boyer's
patch to access and parse the ESL to extract the certificates and load
them onto the platform keyring.

The last patch in this patch set adds support for IMA-appraisal to
verify the kexec'ed kernel image based on keys stored in the platform
keyring.

Changelog:

v0:
- The original patches loaded the certificates onto the secondary
  trusted keyring. This patch set defines a new keyring named
  ".platform" and adds the certificates to this new keyring  
- removed CONFIG EFI_SIGNATURE_LIST_PARSER and LOAD_UEFI_KEYS
- moved files from certs/ to security/integrity/platform_certs/

Dave Howells (2):
  efi: Add EFI signature data types
  efi: Add an EFI signature blob parser

Josh Boyer (2):
  efi: Import certificates from UEFI Secure Boot
  efi: Allow the "db" UEFI variable to be suppressed

Nayna Jain (3):
  integrity: define a trusted platform keyring
  integrity: load certs to the platform keyring
  ima: support platform keyring for kernel appraisal

 include/linux/efi.h                                |  34 ++++
 security/integrity/Kconfig                         |  11 ++
 security/integrity/Makefile                        |   5 +
 security/integrity/digsig.c                        | 115 ++++++++----
 security/integrity/ima/ima_appraise.c              |  11 +-
 security/integrity/integrity.h                     |  23 ++-
 security/integrity/platform_certs/efi_parser.c     | 112 ++++++++++++
 security/integrity/platform_certs/load_uefi.c      | 192 +++++++++++++++++++++
 .../integrity/platform_certs/platform_keyring.c    |  62 +++++++
 9 files changed, 527 insertions(+), 38 deletions(-)
 create mode 100644 security/integrity/platform_certs/efi_parser.c
 create mode 100644 security/integrity/platform_certs/load_uefi.c
 create mode 100644 security/integrity/platform_certs/platform_keyring.c

-- 
2.13.6

[PATCH 1/7] integrity: Define a trusted platform keyring

[Nayna Jain]

On secure boot enabled systems, a verified kernel may need to kexec
additional kernels. For example, it may be used as a bootloader needing 
to kexec a target kernel or it may need to kexec a crashdump kernel. In
such cases, it may want to verify the signature of the next kernel
image.

It is further possible that the kernel image is signed with third party
keys which are stored as platform or firmware keys in the 'db' variable.
The kernel, however, can not directly verify these 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 creates the new keyring called ".platform" to isolate keys
provided by platform from keys by kernel. These keys are used to
facilitate signature verification during kexec. Since the scope of this
keyring is only the platform/firmware keys, it cannot be updated from
userspace.

This keyring can be enabled by setting CONFIG_INTEGRITY_PLATFORM_KEYRING.

Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
---
 security/integrity/Kconfig                         | 11 +++++
 security/integrity/Makefile                        |  1 +
 security/integrity/digsig.c                        | 48 +++++++++++++++-------
 security/integrity/integrity.h                     |  3 +-
 .../integrity/platform_certs/platform_keyring.c    | 39 ++++++++++++++++++
 5 files changed, 87 insertions(+), 15 deletions(-)
 create mode 100644 security/integrity/platform_certs/platform_keyring.c

diff --git a/security/integrity/Kconfig b/security/integrity/Kconfig
index da9565891738..4b4d2aeef539 100644
--- a/security/integrity/Kconfig
+++ b/security/integrity/Kconfig
@@ -51,6 +51,17 @@ config INTEGRITY_TRUSTED_KEYRING
 	   .evm keyrings be signed by a key on the system trusted
 	   keyring.
 
+config INTEGRITY_PLATFORM_KEYRING
+        bool "Provide keyring for platform/firmware trusted keys"
+        depends on INTEGRITY_ASYMMETRIC_KEYS
+        depends on SYSTEM_BLACKLIST_KEYRING
+        depends on EFI
+        help
+         Provide a separate, distinct keyring for platform trusted keys, which
+         the kernel automatically populates during initialization from values
+         provided by the platform for verifying the kexec'ed kerned image
+         and, possibly, the initramfs signature.
+
 config INTEGRITY_AUDIT
 	bool "Enables integrity auditing support "
 	depends on AUDIT
diff --git a/security/integrity/Makefile b/security/integrity/Makefile
index 04d6e462b079..046ffc1bb42d 100644
--- a/security/integrity/Makefile
+++ b/security/integrity/Makefile
@@ -9,6 +9,7 @@ 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
 
 subdir-$(CONFIG_IMA)			+= ima
 obj-$(CONFIG_IMA)			+= ima/
diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
index 5eacba858e4b..fef2a858300c 100644
--- a/security/integrity/digsig.c
+++ b/security/integrity/digsig.c
@@ -35,6 +35,7 @@ static const char * const keyring_name[INTEGRITY_KEYRING_MAX] = {
 	".ima",
 #endif
 	"_module",
+	".platform",
 };
 
 #ifdef CONFIG_IMA_KEYRINGS_PERMIT_SIGNED_BY_BUILTIN_OR_SECONDARY
@@ -73,12 +74,39 @@ int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen,
 	return -EOPNOTSUPP;
 }
 
-int __init integrity_init_keyring(const unsigned int id)
+static int __integrity_init_keyring(const unsigned int id, key_perm_t perm,
+				    struct key_restriction *restriction)
 {
 	const struct cred *cred = current_cred();
+	int err = 0;
+
+	keyring[id] = keyring_alloc(keyring_name[id], KUIDT_INIT(0),
+				    KGIDT_INIT(0), cred, perm,
+				    KEY_ALLOC_NOT_IN_QUOTA,
+				    restriction, NULL);
+	if (IS_ERR(keyring[id])) {
+		err = PTR_ERR(keyring[id]);
+		pr_info("Can't allocate %s keyring (%d)\n",
+			keyring_name[id], err);
+		keyring[id] = NULL;
+	}
+
+	return err;
+}
+
+int __init integrity_init_keyring(const unsigned int id)
+{
 	struct key_restriction *restriction;
+	key_perm_t perm;
 	int err = 0;
 
+	if (id == INTEGRITY_KEYRING_PLATFORM) {
+		restriction = NULL;
+		perm = (KEY_POS_ALL & ~KEY_POS_SETATTR) | KEY_USR_VIEW
+			| KEY_USR_READ | KEY_USR_SEARCH;
+		goto out;
+	}
+
 	if (!IS_ENABLED(CONFIG_INTEGRITY_TRUSTED_KEYRING))
 		return 0;
 
@@ -87,20 +115,12 @@ int __init integrity_init_keyring(const unsigned int id)
 		return -ENOMEM;
 
 	restriction->check = restrict_link_to_ima;
+	perm = (KEY_POS_ALL & ~KEY_POS_SETATTR) | KEY_USR_VIEW | KEY_USR_READ
+		| KEY_USR_WRITE | KEY_USR_SEARCH;
+
+out:
+	err = __integrity_init_keyring(id, perm, restriction);
 
-	keyring[id] = keyring_alloc(keyring_name[id], KUIDT_INIT(0),
-				    KGIDT_INIT(0), cred,
-				    ((KEY_POS_ALL & ~KEY_POS_SETATTR) |
-				     KEY_USR_VIEW | KEY_USR_READ |
-				     KEY_USR_WRITE | KEY_USR_SEARCH),
-				    KEY_ALLOC_NOT_IN_QUOTA,
-				    restriction, NULL);
-	if (IS_ERR(keyring[id])) {
-		err = PTR_ERR(keyring[id]);
-		pr_info("Can't allocate %s keyring (%d)\n",
-			keyring_name[id], err);
-		keyring[id] = NULL;
-	}
 	return err;
 }
 
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index e60473b13a8d..c2332a44799e 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -142,7 +142,8 @@ 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
 
 extern struct dentry *integrity_dir;
 
diff --git a/security/integrity/platform_certs/platform_keyring.c b/security/integrity/platform_certs/platform_keyring.c
new file mode 100644
index 000000000000..dfc206bbe2ff
--- /dev/null
+++ b/security/integrity/platform_certs/platform_keyring.c
@@ -0,0 +1,39 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Platform keyring for firmware/platform keys
+ *
+ * Copyright (C) 2012 Red Hat, Inc. All Rights Reserved.
+ * Written by David Howells (dhowells@redhat.com)
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public Licence
+ * as published by the Free Software Foundation; either version
+ * 2 of the Licence, or (at your option) any later version.
+ */
+
+#include <linux/export.h>
+#include <linux/kernel.h>
+#include <linux/sched.h>
+#include <linux/cred.h>
+#include <linux/err.h>
+#include <linux/slab.h>
+#include "../integrity.h"
+
+/*
+ * Create the trusted keyrings.
+ */
+static __init int platform_keyring_init(void)
+{
+	int rc;
+
+	rc = integrity_init_keyring(INTEGRITY_KEYRING_PLATFORM);
+	if (rc)
+		return rc;
+
+	pr_notice("Platform Keyring initialized\n");
+	return 0;
+}
+
+/*
+ * Must be initialised before we try and load the keys into the keyring.
+ */
+device_initcall(platform_keyring_init);
-- 
2.13.6

[PATCH 2/7] integrity: Load certs to the platform keyring

[Nayna Jain]

The patch refactors integrity_load_x509(), making it a wrapper for a new
function named integrity_add_key(). This patch also defines a new
function named integrity_load_cert() for loading the platform keys.

Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
---
 security/integrity/digsig.c                        | 71 ++++++++++++++--------
 security/integrity/integrity.h                     | 20 ++++++
 .../integrity/platform_certs/platform_keyring.c    | 23 +++++++
 3 files changed, 90 insertions(+), 24 deletions(-)

diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
index fef2a858300c..ab30c451a19b 100644
--- a/security/integrity/digsig.c
+++ b/security/integrity/digsig.c
@@ -82,8 +82,7 @@ static int __integrity_init_keyring(const unsigned int id, key_perm_t perm,
 
 	keyring[id] = keyring_alloc(keyring_name[id], KUIDT_INIT(0),
 				    KGIDT_INIT(0), cred, perm,
-				    KEY_ALLOC_NOT_IN_QUOTA,
-				    restriction, NULL);
+				    KEY_ALLOC_NOT_IN_QUOTA, restriction, NULL);
 	if (IS_ERR(keyring[id])) {
 		err = PTR_ERR(keyring[id]);
 		pr_info("Can't allocate %s keyring (%d)\n",
@@ -124,16 +123,38 @@ int __init integrity_init_keyring(const unsigned int id)
 	return err;
 }
 
-int __init integrity_load_x509(const unsigned int id, const char *path)
+int __init integrity_add_key(const unsigned int id, const void *data,
+			     off_t size, key_perm_t perm)
 {
 	key_ref_t key;
-	void *data;
-	loff_t size;
-	int rc;
+	int rc = 0;
 
 	if (!keyring[id])
 		return -EINVAL;
 
+	key = key_create_or_update(make_key_ref(keyring[id], 1), "asymmetric",
+				   NULL, data, size, perm,
+				   KEY_ALLOC_NOT_IN_QUOTA);
+	if (IS_ERR(key)) {
+		rc = PTR_ERR(key);
+		pr_err("Problem loading X.509 certificate %d\n", rc);
+	} else {
+		pr_notice("Loaded X.509 cert '%s'\n",
+			  key_ref_to_ptr(key)->description);
+		key_ref_put(key);
+	}
+
+	return rc;
+
+}
+
+int __init integrity_load_x509(const unsigned int id, const char *path)
+{
+	void *data;
+	loff_t size;
+	int rc;
+	key_perm_t perm;
+
 	rc = kernel_read_file_from_path(path, &data, &size, 0,
 					READING_X509_CERTIFICATE);
 	if (rc < 0) {
@@ -141,23 +162,25 @@ int __init integrity_load_x509(const unsigned int id, const char *path)
 		return rc;
 	}
 
-	key = key_create_or_update(make_key_ref(keyring[id], 1),
-				   "asymmetric",
-				   NULL,
-				   data,
-				   size,
-				   ((KEY_POS_ALL & ~KEY_POS_SETATTR) |
-				    KEY_USR_VIEW | KEY_USR_READ),
-				   KEY_ALLOC_NOT_IN_QUOTA);
-	if (IS_ERR(key)) {
-		rc = PTR_ERR(key);
-		pr_err("Problem loading X.509 certificate (%d): %s\n",
-		       rc, path);
-	} else {
-		pr_notice("Loaded X.509 cert '%s': %s\n",
-			  key_ref_to_ptr(key)->description, path);
-		key_ref_put(key);
-	}
+	perm = (KEY_POS_ALL & ~KEY_POS_SETATTR) | KEY_USR_VIEW | KEY_USR_READ;
+
+	pr_info("Loading X.509 certificate: %s\n", path);
+	rc = integrity_add_key(id, (const void *)data, size, perm);
+
 	vfree(data);
-	return 0;
+	return rc;
+}
+
+int __init integrity_load_cert(const unsigned int id, const char *source,
+			       const void *data, size_t len, key_perm_t perm)
+{
+	int rc;
+
+	if (!data)
+		return -EINVAL;
+
+	pr_info("Loading X.509 certificate: %s\n", source);
+	rc = integrity_add_key(id, data, len, perm);
+
+	return rc;
 }
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index c2332a44799e..3517d2852a07 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -154,6 +154,8 @@ int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen,
 
 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);
 #else
 
 static inline int integrity_digsig_verify(const unsigned int id,
@@ -167,6 +169,14 @@ static inline int integrity_init_keyring(const unsigned int id)
 {
 	return 0;
 }
+
+static inline int __init integrity_load_cert(const unsigned int id,
+					     const char *source,
+					     const void *data, size_t len,
+					     key_perm_t perm)
+{
+	return 0;
+}
 #endif /* CONFIG_INTEGRITY_SIGNATURE */
 
 #ifdef CONFIG_INTEGRITY_ASYMMETRIC_KEYS
@@ -223,3 +233,13 @@ integrity_audit_log_start(struct audit_context *ctx, gfp_t gfp_mask, int type)
 }
 
 #endif
+
+#ifdef CONFIG_INTEGRITY_PLATFORM_KEYRING
+void __init add_to_platform_keyring(const char *source, const void *data,
+				    size_t len);
+#else
+static inline void __init add_to_platform_keyring(const char *source,
+						  const void *data, size_t len)
+{
+}
+#endif
diff --git a/security/integrity/platform_certs/platform_keyring.c b/security/integrity/platform_certs/platform_keyring.c
index dfc206bbe2ff..dc49b3b02697 100644
--- a/security/integrity/platform_certs/platform_keyring.c
+++ b/security/integrity/platform_certs/platform_keyring.c
@@ -18,6 +18,29 @@
 #include <linux/slab.h>
 #include "../integrity.h"
 
+/**
+ * add_to_platform_keyring - Add to platform keyring without validation.
+ * @source: Source of key
+ * @data: The blob holding the key
+ * @len: The length of the data blob
+ *
+ * Add a key to the platform keyring without checking its trust chain.  This
+ * is available only during kernel initialisation.
+ */
+void __init add_to_platform_keyring(const char *source, const void *data,
+				    size_t len)
+{
+	key_perm_t perm;
+	int rc;
+
+	perm = (KEY_POS_ALL & ~KEY_POS_SETATTR) | KEY_USR_VIEW;
+
+	rc = integrity_load_cert(INTEGRITY_KEYRING_PLATFORM, source, data, len,
+				 perm);
+	if (rc)
+		pr_info("Error adding keys to platform keyring %s\n", source);
+}
+
 /*
  * Create the trusted keyrings.
  */
-- 
2.13.6

[PATCH 3/7] efi: Add EFI signature data types

[Nayna Jain]

From: Dave Howells <dhowells@redhat.com>

Add the data types that are used for containing hashes, keys and
certificates for cryptographic verification along with their corresponding
type GUIDs.

Signed-off-by: David Howells <dhowells@redhat.com>
Acked-by: Nayna Jain <nayna@linux.ibm.com>
---
Changelog:

v0: 
- No changes

 include/linux/efi.h | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/include/linux/efi.h b/include/linux/efi.h
index 401e4b254e30..99cba6fe1234 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -663,6 +663,10 @@ void efi_native_runtime_setup(void);
 #define EFI_IMAGE_SECURITY_DATABASE_GUID	EFI_GUID(0xd719b2cb, 0x3d3a, 0x4596,  0xa3, 0xbc, 0xda, 0xd0, 0x0e, 0x67, 0x65, 0x6f)
 #define EFI_SHIM_LOCK_GUID			EFI_GUID(0x605dab50, 0xe046, 0x4300,  0xab, 0xb6, 0x3d, 0xd8, 0x10, 0xdd, 0x8b, 0x23)
 
+#define EFI_CERT_SHA256_GUID			EFI_GUID(0xc1c41626, 0x504c, 0x4092, 0xac, 0xa9, 0x41, 0xf9, 0x36, 0x93, 0x43, 0x28)
+#define EFI_CERT_X509_GUID			EFI_GUID(0xa5c059a1, 0x94e4, 0x4aa7, 0x87, 0xb5, 0xab, 0x15, 0x5c, 0x2b, 0xf0, 0x72)
+#define EFI_CERT_X509_SHA256_GUID		EFI_GUID(0x3bd2a492, 0x96c0, 0x4079, 0xb4, 0x20, 0xfc, 0xf9, 0x8e, 0xf1, 0x03, 0xed)
+
 /*
  * This GUID is used to pass to the kernel proper the struct screen_info
  * structure that was populated by the stub based on the GOP protocol instance
@@ -933,6 +937,27 @@ typedef struct {
 	efi_memory_desc_t entry[0];
 } efi_memory_attributes_table_t;
 
+typedef struct  {
+	efi_guid_t signature_owner;
+	u8 signature_data[];
+} efi_signature_data_t;
+
+typedef struct {
+	efi_guid_t signature_type;
+	u32 signature_list_size;
+	u32 signature_header_size;
+	u32 signature_size;
+	u8 signature_header[];
+	/* efi_signature_data_t signatures[][] */
+} efi_signature_list_t;
+
+typedef u8 efi_sha256_hash_t[32];
+
+typedef struct {
+	efi_sha256_hash_t to_be_signed_hash;
+	efi_time_t time_of_revocation;
+} efi_cert_x509_sha256_t;
+
 /*
  * All runtime access to EFI goes through this structure:
  */
-- 
2.13.6

[PATCH 4/7] efi: Add an EFI signature blob parser

[Nayna Jain]

From: Dave Howells <dhowells@redhat.com>

Add a function to parse an EFI signature blob looking for elements of
interest. A list is made up of a series of sublists, where all the
elements in a sublist are of the same type, but sublists can be of
different types.

For each sublist encountered, the function pointed to by the
get_handler_for_guid argument is called with the type specifier GUID and
returns either a pointer to a function to handle elements of that type or
NULL if the type is not of interest.

If the sublist is of interest, each element is passed to the handler
function in turn.

Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
---
Changelog:

v0:
- removed the CONFIG EFI_SIGNATURE_LIST_PARSER
- moved efi_parser.c from certs to security/integrity/platform_certs
  directory

 include/linux/efi.h                            |   9 ++
 security/integrity/Makefile                    |   3 +-
 security/integrity/platform_certs/efi_parser.c | 112 +++++++++++++++++++++++++
 3 files changed, 123 insertions(+), 1 deletion(-)
 create mode 100644 security/integrity/platform_certs/efi_parser.c

diff --git a/include/linux/efi.h b/include/linux/efi.h
index 99cba6fe1234..2016145e2d6d 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -1138,6 +1138,15 @@ extern int efi_memattr_apply_permissions(struct mm_struct *mm,
 char * __init efi_md_typeattr_format(char *buf, size_t size,
 				     const efi_memory_desc_t *md);
 
+
+typedef void (*efi_element_handler_t)(const char *source,
+				      const void *element_data,
+				      size_t element_size);
+extern int __init parse_efi_signature_list(
+	const char *source,
+	const void *data, size_t size,
+	efi_element_handler_t (*get_handler_for_guid)(const efi_guid_t *));
+
 /**
  * efi_range_is_wc - check the WC bit on an address range
  * @start: starting kvirt address
diff --git a/security/integrity/Makefile b/security/integrity/Makefile
index 046ffc1bb42d..6ee9058866cd 100644
--- a/security/integrity/Makefile
+++ b/security/integrity/Makefile
@@ -9,7 +9,8 @@ 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/efi_parser.o
 
 subdir-$(CONFIG_IMA)			+= ima
 obj-$(CONFIG_IMA)			+= ima/
diff --git a/security/integrity/platform_certs/efi_parser.c b/security/integrity/platform_certs/efi_parser.c
new file mode 100644
index 000000000000..4e396f98f5c7
--- /dev/null
+++ b/security/integrity/platform_certs/efi_parser.c
@@ -0,0 +1,112 @@
+/* EFI signature/key/certificate list parser
+ *
+ * Copyright (C) 2012, 2016 Red Hat, Inc. All Rights Reserved.
+ * Written by David Howells (dhowells@redhat.com)
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public Licence
+ * as published by the Free Software Foundation; either version
+ * 2 of the Licence, or (at your option) any later version.
+ */
+
+#define pr_fmt(fmt) "EFI: "fmt
+#include <linux/module.h>
+#include <linux/printk.h>
+#include <linux/err.h>
+#include <linux/efi.h>
+
+/**
+ * parse_efi_signature_list - Parse an EFI signature list for certificates
+ * @source: The source of the key
+ * @data: The data blob to parse
+ * @size: The size of the data blob
+ * @get_handler_for_guid: Get the handler func for the sig type (or NULL)
+ *
+ * Parse an EFI signature list looking for elements of interest.  A list is
+ * made up of a series of sublists, where all the elements in a sublist are of
+ * the same type, but sublists can be of different types.
+ *
+ * For each sublist encountered, the @get_handler_for_guid function is called
+ * with the type specifier GUID and returns either a pointer to a function to
+ * handle elements of that type or NULL if the type is not of interest.
+ *
+ * If the sublist is of interest, each element is passed to the handler
+ * function in turn.
+ *
+ * Error EBADMSG is returned if the list doesn't parse correctly and 0 is
+ * returned if the list was parsed correctly.  No error can be returned from
+ * the @get_handler_for_guid function or the element handler function it
+ * returns.
+ */
+int __init parse_efi_signature_list(
+	const char *source,
+	const void *data, size_t size,
+	efi_element_handler_t (*get_handler_for_guid)(const efi_guid_t *))
+{
+	efi_element_handler_t handler;
+	unsigned offs = 0;
+
+	pr_devel("-->%s(,%zu)\n", __func__, size);
+
+	while (size > 0) {
+		const efi_signature_data_t *elem;
+		efi_signature_list_t list;
+		size_t lsize, esize, hsize, elsize;
+
+		if (size < sizeof(list))
+			return -EBADMSG;
+
+		memcpy(&list, data, sizeof(list));
+		pr_devel("LIST[%04x] guid=%pUl ls=%x hs=%x ss=%x\n",
+			 offs,
+			 list.signature_type.b, list.signature_list_size,
+			 list.signature_header_size, list.signature_size);
+
+		lsize = list.signature_list_size;
+		hsize = list.signature_header_size;
+		esize = list.signature_size;
+		elsize = lsize - sizeof(list) - hsize;
+
+		if (lsize > size) {
+			pr_devel("<--%s() = -EBADMSG [overrun @%x]\n",
+				 __func__, offs);
+			return -EBADMSG;
+		}
+
+		if (lsize < sizeof(list) ||
+		    lsize - sizeof(list) < hsize ||
+		    esize < sizeof(*elem) ||
+		    elsize < esize ||
+		    elsize % esize != 0) {
+			pr_devel("- bad size combo @%x\n", offs);
+			return -EBADMSG;
+		}
+
+		handler = get_handler_for_guid(&list.signature_type);
+		if (!handler) {
+			data += lsize;
+			size -= lsize;
+			offs += lsize;
+			continue;
+		}
+
+		data += sizeof(list) + hsize;
+		size -= sizeof(list) + hsize;
+		offs += sizeof(list) + hsize;
+
+		for (; elsize > 0; elsize -= esize) {
+			elem = data;
+
+			pr_devel("ELEM[%04x]\n", offs);
+			handler(source,
+				&elem->signature_data,
+				esize - sizeof(*elem));
+
+			data += esize;
+			size -= esize;
+			offs += esize;
+		}
+	}
+
+	return 0;
+}
-- 
2.13.6

[PATCH 5/7] efi: Import certificates from UEFI Secure Boot

[Nayna Jain]

From: Josh Boyer <jwboyer@fedoraproject.org>

New Patch Description:
======================

Secure Boot stores a list of allowed certificates in the 'db' variable.
This patch imports those certificates into the platform keyring. The shim
UEFI bootloader has a similar certificate list stored in the 'MokListRT'
variable. We import those as well.

Secure Boot also maintains a list of disallowed certificates in the 'dbx'
variable. We load those certificates into the system blacklist keyring
and forbid any kernel signed with those from loading.

Original Patch Description:
============================

Secure Boot stores a list of allowed certificates in the 'db' variable.
This imports those certificates into the system trusted keyring.  This
allows for a third party signing certificate to be used in conjunction
with signed modules. By importing the public certificate into the 'db'
variable, a user can allow a module signed with that certificate to
load. The shim UEFI bootloader has a similar certificate list stored
in the 'MokListRT' variable. We import those as well.

Secure Boot also maintains a list of disallowed certificates in the 'dbx'
variable. We load those certificates into the newly introduced system
blacklist keyring and forbid any module signed with those from loading and
forbid the use within the kernel of any key with a matching hash.

This facility is enabled by setting CONFIG_LOAD_UEFI_KEYS.

Signed-off-by: Josh Boyer <jwboyer@fedoraproject.org>
Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
---
Changelog:

v0:
- This patch replaces the loading of certificates onto the secondary
  keyring with platform keyring
- removed the CONFIG LOAD_UEFI_KEYS
- moved the file load_uefi.o from certs to
  security/integrity/platform_certs

 security/integrity/Makefile                   |   5 +-
 security/integrity/platform_certs/load_uefi.c | 168 ++++++++++++++++++++++++++
 2 files changed, 172 insertions(+), 1 deletion(-)
 create mode 100644 security/integrity/platform_certs/load_uefi.c

diff --git a/security/integrity/Makefile b/security/integrity/Makefile
index 6ee9058866cd..86df9aba8c0f 100644
--- a/security/integrity/Makefile
+++ b/security/integrity/Makefile
@@ -10,7 +10,10 @@ 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 \
-						  platform_certs/efi_parser.o
+						  platform_certs/efi_parser.o \
+						  platform_certs/load_uefi.o
+obj-$(CONFIG_LOAD_UEFI_KEYS) += platform_certs/load_uefi.o
+$(obj)/load_uefi.o: KBUILD_CFLAGS += -fshort-wchar
 
 subdir-$(CONFIG_IMA)			+= ima
 obj-$(CONFIG_IMA)			+= ima/
diff --git a/security/integrity/platform_certs/load_uefi.c b/security/integrity/platform_certs/load_uefi.c
new file mode 100644
index 000000000000..dbccb45147ef
--- /dev/null
+++ b/security/integrity/platform_certs/load_uefi.c
@@ -0,0 +1,168 @@
+#include <linux/kernel.h>
+#include <linux/sched.h>
+#include <linux/cred.h>
+#include <linux/err.h>
+#include <linux/efi.h>
+#include <linux/slab.h>
+#include <keys/asymmetric-type.h>
+#include <keys/system_keyring.h>
+#include "../integrity.h"
+
+static __initdata efi_guid_t efi_cert_x509_guid = EFI_CERT_X509_GUID;
+static __initdata efi_guid_t efi_cert_x509_sha256_guid = EFI_CERT_X509_SHA256_GUID;
+static __initdata efi_guid_t efi_cert_sha256_guid = EFI_CERT_SHA256_GUID;
+
+/*
+ * Get a certificate list blob from the named EFI variable.
+ */
+static __init void *get_cert_list(efi_char16_t *name, efi_guid_t *guid,
+				  unsigned long *size)
+{
+	efi_status_t status;
+	unsigned long lsize = 4;
+	unsigned long tmpdb[4];
+	void *db;
+
+	status = efi.get_variable(name, guid, NULL, &lsize, &tmpdb);
+	if (status != EFI_BUFFER_TOO_SMALL) {
+		pr_err("Couldn't get size: 0x%lx\n", status);
+		return NULL;
+	}
+
+	db = kmalloc(lsize, GFP_KERNEL);
+	if (!db) {
+		pr_err("Couldn't allocate memory for uefi cert list\n");
+		return NULL;
+	}
+
+	status = efi.get_variable(name, guid, NULL, &lsize, db);
+	if (status != EFI_SUCCESS) {
+		kfree(db);
+		pr_err("Error reading db var: 0x%lx\n", status);
+		return NULL;
+	}
+
+	*size = lsize;
+	return db;
+}
+
+/*
+ * Blacklist an X509 TBS hash.
+ */
+static __init void uefi_blacklist_x509_tbs(const char *source,
+					   const void *data, size_t len)
+{
+	char *hash, *p;
+
+	hash = kmalloc(4 + len * 2 + 1, GFP_KERNEL);
+	if (!hash)
+		return;
+	p = memcpy(hash, "tbs:", 4);
+	p += 4;
+	bin2hex(p, data, len);
+	p += len * 2;
+	*p = 0;
+
+	mark_hash_blacklisted(hash);
+	kfree(hash);
+}
+
+/*
+ * Blacklist the hash of an executable.
+ */
+static __init void uefi_blacklist_binary(const char *source,
+					 const void *data, size_t len)
+{
+	char *hash, *p;
+
+	hash = kmalloc(4 + len * 2 + 1, GFP_KERNEL);
+	if (!hash)
+		return;
+	p = memcpy(hash, "bin:", 4);
+	p += 4;
+	bin2hex(p, data, len);
+	p += len * 2;
+	*p = 0;
+
+	mark_hash_blacklisted(hash);
+	kfree(hash);
+}
+
+/*
+ * Return the appropriate handler for particular signature list types found in
+ * the UEFI db and MokListRT tables.
+ */
+static __init efi_element_handler_t get_handler_for_db(const efi_guid_t *sig_type)
+{
+	if (efi_guidcmp(*sig_type, efi_cert_x509_guid) == 0)
+		return add_to_platform_keyring;
+	return 0;
+}
+
+/*
+ * Return the appropriate handler for particular signature list types found in
+ * the UEFI dbx and MokListXRT tables.
+ */
+static __init efi_element_handler_t get_handler_for_dbx(const efi_guid_t *sig_type)
+{
+	if (efi_guidcmp(*sig_type, efi_cert_x509_sha256_guid) == 0)
+		return uefi_blacklist_x509_tbs;
+	if (efi_guidcmp(*sig_type, efi_cert_sha256_guid) == 0)
+		return uefi_blacklist_binary;
+	return 0;
+}
+
+/*
+ * Load the certs contained in the UEFI databases
+ */
+static int __init load_uefi_certs(void)
+{
+	efi_guid_t secure_var = EFI_IMAGE_SECURITY_DATABASE_GUID;
+	efi_guid_t mok_var = EFI_SHIM_LOCK_GUID;
+	void *db = NULL, *dbx = NULL, *mok = NULL;
+	unsigned long dbsize = 0, dbxsize = 0, moksize = 0;
+	int rc = 0;
+
+	if (!efi.get_variable)
+		return false;
+
+	/* Get db, MokListRT, and dbx.  They might not exist, so it isn't
+	 * an error if we can't get them.
+	 */
+	db = get_cert_list(L"db", &secure_var, &dbsize);
+	if (!db) {
+		pr_err("Couldn't get UEFI db list\n");
+	} else {
+		rc = parse_efi_signature_list("UEFI:db",
+					      db, dbsize, get_handler_for_db);
+		if (rc)
+			pr_err("Couldn't parse db signatures: %d\n", rc);
+		kfree(db);
+	}
+
+	mok = get_cert_list(L"MokListRT", &mok_var, &moksize);
+	if (!mok) {
+		pr_info("Couldn't get UEFI MokListRT\n");
+	} else {
+		rc = parse_efi_signature_list("UEFI:MokListRT",
+					      mok, moksize, get_handler_for_db);
+		if (rc)
+			pr_err("Couldn't parse MokListRT signatures: %d\n", rc);
+		kfree(mok);
+	}
+
+	dbx = get_cert_list(L"dbx", &secure_var, &dbxsize);
+	if (!dbx) {
+		pr_info("Couldn't get UEFI dbx list\n");
+	} else {
+		rc = parse_efi_signature_list("UEFI:dbx",
+					      dbx, dbxsize,
+					      get_handler_for_dbx);
+		if (rc)
+			pr_err("Couldn't parse dbx signatures: %d\n", rc);
+		kfree(dbx);
+	}
+
+	return rc;
+}
+late_initcall(load_uefi_certs);
-- 
2.13.6

[PATCH 6/7] efi: Allow the "db" UEFI variable to be suppressed

[Nayna Jain]

From: Josh Boyer <jwboyer@fedoraproject.org>

If a user tells shim to not use the certs/hashes in the UEFI db variable
for verification purposes, shim will set a UEFI variable called
MokIgnoreDB. Have the uefi import code look for this and ignore the db
variable if it is found.

Signed-off-by: Josh Boyer <jwboyer@fedoraproject.org>
Signed-off-by: David Howells <dhowells@redhat.com>
Acked-by: Nayna Jain <nayna@linux.ibm.com>
---
Changelog:

v0:
- No changes

 security/integrity/platform_certs/load_uefi.c | 44 +++++++++++++++++++++------
 1 file changed, 34 insertions(+), 10 deletions(-)

diff --git a/security/integrity/platform_certs/load_uefi.c b/security/integrity/platform_certs/load_uefi.c
index dbccb45147ef..978c8d0dc151 100644
--- a/security/integrity/platform_certs/load_uefi.c
+++ b/security/integrity/platform_certs/load_uefi.c
@@ -13,6 +13,26 @@ static __initdata efi_guid_t efi_cert_x509_sha256_guid = EFI_CERT_X509_SHA256_GU
 static __initdata efi_guid_t efi_cert_sha256_guid = EFI_CERT_SHA256_GUID;
 
 /*
+ * Look to see if a UEFI variable called MokIgnoreDB exists and return true if
+ * it does.
+ *
+ * This UEFI variable is set by the shim if a user tells the shim to not use
+ * the certs/hashes in the UEFI db variable for verification purposes.  If it
+ * is set, we should ignore the db variable also and the true return indicates
+ * this.
+ */
+static __init bool uefi_check_ignore_db(void)
+{
+	efi_status_t status;
+	unsigned int db = 0;
+	unsigned long size = sizeof(db);
+	efi_guid_t guid = EFI_SHIM_LOCK_GUID;
+
+	status = efi.get_variable(L"MokIgnoreDB", &guid, NULL, &size, &db);
+	return status == EFI_SUCCESS;
+}
+
+/*
  * Get a certificate list blob from the named EFI variable.
  */
 static __init void *get_cert_list(efi_char16_t *name, efi_guid_t *guid,
@@ -113,7 +133,9 @@ static __init efi_element_handler_t get_handler_for_dbx(const efi_guid_t *sig_ty
 }
 
 /*
- * Load the certs contained in the UEFI databases
+ * Load the certs contained in the UEFI databases into the secondary trusted
+ * keyring and the UEFI blacklisted X.509 cert SHA256 hashes into the blacklist
+ * keyring.
  */
 static int __init load_uefi_certs(void)
 {
@@ -129,15 +151,17 @@ static int __init load_uefi_certs(void)
 	/* Get db, MokListRT, and dbx.  They might not exist, so it isn't
 	 * an error if we can't get them.
 	 */
-	db = get_cert_list(L"db", &secure_var, &dbsize);
-	if (!db) {
-		pr_err("Couldn't get UEFI db list\n");
-	} else {
-		rc = parse_efi_signature_list("UEFI:db",
-					      db, dbsize, get_handler_for_db);
-		if (rc)
-			pr_err("Couldn't parse db signatures: %d\n", rc);
-		kfree(db);
+	if (!uefi_check_ignore_db()) {
+		db = get_cert_list(L"db", &secure_var, &dbsize);
+		if (!db) {
+			pr_err("MODSIGN: Couldn't get UEFI db list\n");
+		} else {
+			rc = parse_efi_signature_list("UEFI:db",
+					db, dbsize, get_handler_for_db);
+			if (rc)
+				pr_err("Couldn't parse db signatures: %d\n", rc);
+			kfree(db);
+		}
 	}
 
 	mok = get_cert_list(L"MokListRT", &mok_var, &moksize);
-- 
2.13.6

[PATCH 7/7] ima: Support platform keyring for kernel appraisal

[Nayna Jain]

On secure boot enabled systems, the bootloader verifies the kernel
image and possibly the initramfs signatures based on a set of keys. A
soft reboot(kexec) of the system, with the same kernel image and
initramfs, requires access to the original keys to verify the
signatures.

This patch allows IMA-appraisal access to those original keys, now
loaded on the platform keyring, needed for verifying the kernel image
and initramfs signatures.

Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
---
 security/integrity/ima/ima_appraise.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index deec1804a00a..9c13585e7d3e 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -294,7 +294,16 @@ int ima_appraise_measurement(enum ima_hooks func,
 					     iint->ima_hash->length);
 		if (rc == -EOPNOTSUPP) {
 			status = INTEGRITY_UNKNOWN;
-		} else if (rc) {
+			break;
+		}
+		if (rc && 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) {
 			cause = "invalid-signature";
 			status = INTEGRITY_FAIL;
 		} else {
-- 
2.13.6

Re: [PATCH 5/7] efi: Import certificates from UEFI Secure Boot

[Mimi Zohar]

On Sun, 2018-11-25 at 20:44 +0530, Nayna Jain wrote:
> From: Josh Boyer <jwboyer@fedoraproject.org>
> 
> New Patch Description:
> ======================
> 
> Secure Boot stores a list of allowed certificates in the 'db' variable.
> This patch imports those certificates into the platform keyring. The shim
> UEFI bootloader has a similar certificate list stored in the 'MokListRT'
> variable. We import those as well.
> 
> Secure Boot also maintains a list of disallowed certificates in the 'dbx'
> variable. We load those certificates into the system blacklist keyring
> and forbid any kernel signed with those from loading.
> 
> Original Patch Description:
> ============================
> 
> Secure Boot stores a list of allowed certificates in the 'db' variable.
> This imports those certificates into the system trusted keyring.  This
> allows for a third party signing certificate to be used in conjunction
> with signed modules. By importing the public certificate into the 'db'
> variable, a user can allow a module signed with that certificate to
> load. The shim UEFI bootloader has a similar certificate list stored
> in the 'MokListRT' variable. We import those as well.
> 
> Secure Boot also maintains a list of disallowed certificates in the 'dbx'
> variable. We load those certificates into the newly introduced system
> blacklist keyring and forbid any module signed with those from loading and
> forbid the use within the kernel of any key with a matching hash.
> 
> This facility is enabled by setting CONFIG_LOAD_UEFI_KEYS.

There are quite a few checkpatch.pl warnings that need to be
addressed, including the missing SPDX license.

Mimi

Re: [PATCH 4/7] efi: Add an EFI signature blob parser

[Mimi Zohar]

On Sun, 2018-11-25 at 20:44 +0530, Nayna Jain wrote:
> From: Dave Howells <dhowells@redhat.com>
> 
> Add a function to parse an EFI signature blob looking for elements of
> interest. A list is made up of a series of sublists, where all the
> elements in a sublist are of the same type, but sublists can be of
> different types.
> 
> For each sublist encountered, the function pointed to by the
> get_handler_for_guid argument is called with the type specifier GUID and
> returns either a pointer to a function to handle elements of that type or
> NULL if the type is not of interest.
> 
> If the sublist is of interest, each element is passed to the handler
> function in turn.

There are a few checkpatch.pl warnings that need to be
addressed, including the missing SPDX license.

Mimi

> 
> Signed-off-by: David Howells <dhowells@redhat.com>
> Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
> ---
> Changelog:
> 
> v0:
> - removed the CONFIG EFI_SIGNATURE_LIST_PARSER
> - moved efi_parser.c from certs to security/integrity/platform_certs
>   directory
> 
>  include/linux/efi.h                            |   9 ++
>  security/integrity/Makefile                    |   3 +-
>  security/integrity/platform_certs/efi_parser.c | 112 +++++++++++++++++++++++++
>  3 files changed, 123 insertions(+), 1 deletion(-)
>  create mode 100644 security/integrity/platform_certs/efi_parser.c
> 
> diff --git a/include/linux/efi.h b/include/linux/efi.h
> index 99cba6fe1234..2016145e2d6d 100644
> --- a/include/linux/efi.h
> +++ b/include/linux/efi.h
> @@ -1138,6 +1138,15 @@ extern int efi_memattr_apply_permissions(struct mm_struct *mm,
>  char * __init efi_md_typeattr_format(char *buf, size_t size,
>  				     const efi_memory_desc_t *md);
>  
> +
> +typedef void (*efi_element_handler_t)(const char *source,
> +				      const void *element_data,
> +				      size_t element_size);
> +extern int __init parse_efi_signature_list(
> +	const char *source,
> +	const void *data, size_t size,
> +	efi_element_handler_t (*get_handler_for_guid)(const efi_guid_t *));
> +
>  /**
>   * efi_range_is_wc - check the WC bit on an address range
>   * @start: starting kvirt address
> diff --git a/security/integrity/Makefile b/security/integrity/Makefile
> index 046ffc1bb42d..6ee9058866cd 100644
> --- a/security/integrity/Makefile
> +++ b/security/integrity/Makefile
> @@ -9,7 +9,8 @@ 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/efi_parser.o
>  
>  subdir-$(CONFIG_IMA)			+= ima
>  obj-$(CONFIG_IMA)			+= ima/
> diff --git a/security/integrity/platform_certs/efi_parser.c b/security/integrity/platform_certs/efi_parser.c
> new file mode 100644
> index 000000000000..4e396f98f5c7
> --- /dev/null
> +++ b/security/integrity/platform_certs/efi_parser.c
> @@ -0,0 +1,112 @@
> +/* EFI signature/key/certificate list parser
> + *
> + * Copyright (C) 2012, 2016 Red Hat, Inc. All Rights Reserved.
> + * Written by David Howells (dhowells@redhat.com)
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public Licence
> + * as published by the Free Software Foundation; either version
> + * 2 of the Licence, or (at your option) any later version.
> + */
> +
> +#define pr_fmt(fmt) "EFI: "fmt
> +#include <linux/module.h>
> +#include <linux/printk.h>
> +#include <linux/err.h>
> +#include <linux/efi.h>
> +
> +/**
> + * parse_efi_signature_list - Parse an EFI signature list for certificates
> + * @source: The source of the key
> + * @data: The data blob to parse
> + * @size: The size of the data blob
> + * @get_handler_for_guid: Get the handler func for the sig type (or NULL)
> + *
> + * Parse an EFI signature list looking for elements of interest.  A list is
> + * made up of a series of sublists, where all the elements in a sublist are of
> + * the same type, but sublists can be of different types.
> + *
> + * For each sublist encountered, the @get_handler_for_guid function is called
> + * with the type specifier GUID and returns either a pointer to a function to
> + * handle elements of that type or NULL if the type is not of interest.
> + *
> + * If the sublist is of interest, each element is passed to the handler
> + * function in turn.
> + *
> + * Error EBADMSG is returned if the list doesn't parse correctly and 0 is
> + * returned if the list was parsed correctly.  No error can be returned from
> + * the @get_handler_for_guid function or the element handler function it
> + * returns.
> + */
> +int __init parse_efi_signature_list(
> +	const char *source,
> +	const void *data, size_t size,
> +	efi_element_handler_t (*get_handler_for_guid)(const efi_guid_t *))
> +{
> +	efi_element_handler_t handler;
> +	unsigned offs = 0;
> +
> +	pr_devel("-->%s(,%zu)\n", __func__, size);
> +
> +	while (size > 0) {
> +		const efi_signature_data_t *elem;
> +		efi_signature_list_t list;
> +		size_t lsize, esize, hsize, elsize;
> +
> +		if (size < sizeof(list))
> +			return -EBADMSG;
> +
> +		memcpy(&list, data, sizeof(list));
> +		pr_devel("LIST[%04x] guid=%pUl ls=%x hs=%x ss=%x\n",
> +			 offs,
> +			 list.signature_type.b, list.signature_list_size,
> +			 list.signature_header_size, list.signature_size);
> +
> +		lsize = list.signature_list_size;
> +		hsize = list.signature_header_size;
> +		esize = list.signature_size;
> +		elsize = lsize - sizeof(list) - hsize;
> +
> +		if (lsize > size) {
> +			pr_devel("<--%s() = -EBADMSG [overrun @%x]\n",
> +				 __func__, offs);
> +			return -EBADMSG;
> +		}
> +
> +		if (lsize < sizeof(list) ||
> +		    lsize - sizeof(list) < hsize ||
> +		    esize < sizeof(*elem) ||
> +		    elsize < esize ||
> +		    elsize % esize != 0) {
> +			pr_devel("- bad size combo @%x\n", offs);
> +			return -EBADMSG;
> +		}
> +
> +		handler = get_handler_for_guid(&list.signature_type);
> +		if (!handler) {
> +			data += lsize;
> +			size -= lsize;
> +			offs += lsize;
> +			continue;
> +		}
> +
> +		data += sizeof(list) + hsize;
> +		size -= sizeof(list) + hsize;
> +		offs += sizeof(list) + hsize;
> +
> +		for (; elsize > 0; elsize -= esize) {
> +			elem = data;
> +
> +			pr_devel("ELEM[%04x]\n", offs);
> +			handler(source,
> +				&elem->signature_data,
> +				esize - sizeof(*elem));
> +
> +			data += esize;
> +			size -= esize;
> +			offs += esize;
> +		}
> +	}
> +
> +	return 0;
> +}

Re: [PATCH 0/7] add platform/firmware keys support for kernel verification by IMA

[Mimi Zohar]

Hi Nayna,

On Sun, 2018-11-25 at 20:44 +0530, Nayna Jain wrote:
> On secure boot enabled systems, a verified kernel may need to kexec
> additional kernels. For example, it may be used as a bootloader needing
> to kexec a target kernel or it may need to kexec a crashdump kernel.
> In such cases, it may want to verify the signature of the next kernel
> image.
> 
> It is possible that the new kernel image is signed with third party keys
> which are stored as platform or firmware keys in the 'db' variable. The
> kernel, however, can not directly verify these 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/firmware keys.
> 
> The secure boot key database is expected to store the keys as EFI
> Signature List(ESL). The patch set uses David Howells and Josh Boyer's
> patch to access and parse the ESL to extract the certificates and load
> them onto the platform keyring.
> 
> The last patch in this patch set adds support for IMA-appraisal to
> verify the kexec'ed kernel image based on keys stored in the platform
> keyring.
> 
> Changelog:
> 
> v0:
> - The original patches loaded the certificates onto the secondary
>   trusted keyring. This patch set defines a new keyring named
>   ".platform" and adds the certificates to this new keyring  
> - removed CONFIG EFI_SIGNATURE_LIST_PARSER and LOAD_UEFI_KEYS
> - moved files from certs/ to security/integrity/platform_certs/

This patch set is looking really good!  There are a couple of
checkpatch.pl warnings that need to be addressed before these patches
can be upstreamed.  I'd also like to see some Reviews/Acks for them as
well.

For the time being these patches are queued in the #next-integrity-
queued branch.

https://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.
git/

thanks!

Mimi

> 
> Dave Howells (2):
>   efi: Add EFI signature data types
>   efi: Add an EFI signature blob parser
> 
> Josh Boyer (2):
>   efi: Import certificates from UEFI Secure Boot
>   efi: Allow the "db" UEFI variable to be suppressed
> 
> Nayna Jain (3):
>   integrity: define a trusted platform keyring
>   integrity: load certs to the platform keyring
>   ima: support platform keyring for kernel appraisal
> 
>  include/linux/efi.h                                |  34 ++++
>  security/integrity/Kconfig                         |  11 ++
>  security/integrity/Makefile                        |   5 +
>  security/integrity/digsig.c                        | 115 ++++++++----
>  security/integrity/ima/ima_appraise.c              |  11 +-
>  security/integrity/integrity.h                     |  23 ++-
>  security/integrity/platform_certs/efi_parser.c     | 112 ++++++++++++
>  security/integrity/platform_certs/load_uefi.c      | 192 +++++++++++++++++++++
>  .../integrity/platform_certs/platform_keyring.c    |  62 +++++++
>  9 files changed, 527 insertions(+), 38 deletions(-)
>  create mode 100644 security/integrity/platform_certs/efi_parser.c
>  create mode 100644 security/integrity/platform_certs/load_uefi.c
>  create mode 100644 security/integrity/platform_certs/platform_keyring.c
> 

Re: [PATCH 7/7] ima: Support platform keyring for kernel appraisal

[Serge E. Hallyn]

On Sun, Nov 25, 2018 at 08:45:00PM +0530, Nayna Jain wrote:
> On secure boot enabled systems, the bootloader verifies the kernel
> image and possibly the initramfs signatures based on a set of keys. A
> soft reboot(kexec) of the system, with the same kernel image and
> initramfs, requires access to the original keys to verify the
> signatures.
> 
> This patch allows IMA-appraisal access to those original keys, now
> loaded on the platform keyring, needed for verifying the kernel image
> and initramfs signatures.
> 
> Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
> Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>

The overall set seems sensible to me, and I see no errors here,

Acked-by: Serge Hallyn <serge@hallyn.com>

I do think that replacing the 'rc' with xattr_len in the previous line might
help future readers save a few cycles.

> ---
>  security/integrity/ima/ima_appraise.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
> index deec1804a00a..9c13585e7d3e 100644
> --- a/security/integrity/ima/ima_appraise.c
> +++ b/security/integrity/ima/ima_appraise.c
> @@ -294,7 +294,16 @@ int ima_appraise_measurement(enum ima_hooks func,
>  					     iint->ima_hash->length);
>  		if (rc == -EOPNOTSUPP) {
>  			status = INTEGRITY_UNKNOWN;
> -		} else if (rc) {
> +			break;
> +		}
> +		if (rc && 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) {
>  			cause = "invalid-signature";
>  			status = INTEGRITY_FAIL;
>  		} else {
> -- 
> 2.13.6
>