LinuxLists.cc - [PATCH v2 0/7] add platform/firmware keys support for kernel verification by IMA

[permalink] [raw]

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

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 <[email protected]>
Reviewed-by: Mimi Zohar <[email protected]>
Acked-by: Serge Hallyn <[email protected]>
---
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 b5b180ff1434..4ba167255225 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

2018-12-08 20:32:09

[permalink] [raw]

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

From: Dave Howells <[email protected]>

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 <[email protected]>
Signed-off-by: Nayna Jain <[email protected]>
Acked-by: Serge Hallyn <[email protected]>
---
Changelog:

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

v2:
- Fixed the checkpatch.pl warnings

include/linux/efi.h | 9 +++
security/integrity/Makefile | 3 +-
security/integrity/platform_certs/efi_parser.c | 108 +++++++++++++++++++++++++
3 files changed, 119 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 214516b29b36..c3206b134137 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -1141,6 +1141,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..18f01f36fe6a
--- /dev/null
+++ b/security/integrity/platform_certs/efi_parser.c
@@ -0,0 +1,108 @@
+// SPDX-License-Identifier: GPL-2.0+
+/* EFI signature/key/certificate list parser
+ *
+ * Copyright (C) 2012, 2016 Red Hat, Inc. All Rights Reserved.
+ * Written by David Howells ([email protected])
+ */
+
+#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 int 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

2018-12-08 20:32:15

[permalink] [raw]

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

From: Josh Boyer <[email protected]>

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 <[email protected]>
Signed-off-by: David Howells <[email protected]>
Signed-off-by: Nayna Jain <[email protected]>
Acked-by: Serge Hallyn <[email protected]>
---
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

v2:
- Fixed the checkpatch.pl warnings

security/integrity/Makefile | 5 +-
security/integrity/platform_certs/load_uefi.c | 171 ++++++++++++++++++++++++++
2 files changed, 175 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..acd9db90dde7
--- /dev/null
+++ b/security/integrity/platform_certs/load_uefi.c
@@ -0,0 +1,171 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#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 efi_guid_t efi_cert_x509_guid __initdata = EFI_CERT_X509_GUID;
+static efi_guid_t efi_cert_x509_sha256_guid __initdata =
+ EFI_CERT_X509_SHA256_GUID;
+static efi_guid_t efi_cert_sha256_guid __initdata = 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)
+ 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

2018-12-08 20:32:48

[permalink] [raw]

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

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 <[email protected]>
Reviewed-by: Mimi Zohar <[email protected]>
Acked-by: Serge Hallyn <[email protected]>
- replace 'rc' with 'xattr_len' when calling integrity_digsig_verify()
with INTEGRITY_KEYRING_IMA for readability
Suggested-by: Serge Hallyn <[email protected]>
---
Changelog:

v2:
- replace 'rc' with 'xattr_len' when calling integrity_digsig_verify()
with INTEGRITY_KEYRING_IMA for readability

security/integrity/ima/ima_appraise.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index deec1804a00a..e8f520450895 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -289,12 +289,21 @@ int ima_appraise_measurement(enum ima_hooks func,
case EVM_IMA_XATTR_DIGSIG:
set_bit(IMA_DIGSIG, &iint->atomic_flags);
rc = integrity_digsig_verify(INTEGRITY_KEYRING_IMA,
- (const char *)xattr_value, rc,
+ (const char *)xattr_value,
+ xattr_len,
iint->ima_hash->digest,
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

2018-12-08 20:33:07

[permalink] [raw]

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

From: Dave Howells <[email protected]>

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 <[email protected]>
Acked-by: Nayna Jain <[email protected]>
Acked-by: Serge Hallyn <[email protected]>
---
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 845174e113ce..214516b29b36 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
@@ -934,6 +938,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

2018-12-08 20:33:29

[permalink] [raw]

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

From: Josh Boyer <[email protected]>

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 <[email protected]>
Signed-off-by: David Howells <[email protected]>
Acked-by: Nayna Jain <[email protected]>
Acked-by: Serge Hallyn <[email protected]>
---
Changelog:

v0:
- No changes

v2:
- Fixed the checkpatch.pl warnings

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

diff --git a/security/integrity/platform_certs/load_uefi.c b/security/integrity/platform_certs/load_uefi.c
index acd9db90dde7..8bd2e9b421e1 100644
--- a/security/integrity/platform_certs/load_uefi.c
+++ b/security/integrity/platform_certs/load_uefi.c
@@ -16,6 +16,26 @@ static efi_guid_t efi_cert_x509_sha256_guid __initdata =
static efi_guid_t efi_cert_sha256_guid __initdata = 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,
@@ -116,7 +136,9 @@ static __init efi_element_handler_t get_handler_for_dbx(const efi_guid_t *
}

/*
- * 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)
{
@@ -132,15 +154,18 @@ 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

2018-12-09 04:53:25

[permalink] [raw]

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

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 <[email protected]>
Reviewed-by: Mimi Zohar <[email protected]>
Acked-by: Serge Hallyn <[email protected]>
---
security/integrity/Kconfig | 11 +++++
security/integrity/Makefile | 1 +
security/integrity/digsig.c | 48 +++++++++++++++-------
security/integrity/integrity.h | 3 +-
.../integrity/platform_certs/platform_keyring.c | 35 ++++++++++++++++
5 files changed, 83 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..79f80af5b470
--- /dev/null
+++ b/security/integrity/platform_certs/platform_keyring.c
@@ -0,0 +1,35 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Platform keyring for firmware/platform keys
+ *
+ * Copyright IBM Corporation, 2018
+ * Author(s): Nayna Jain <[email protected]>
+ */
+
+#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

2018-12-09 18:42:21

by Mimi Zohar

[permalink] [raw]

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

Hi Nayna,

On Sun, 2018-12-09 at 01:56 +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.

Thanks! This patch set is now in the #next-integrity branch.

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

Mimi

2018-12-11 18:31:04

[permalink] [raw]

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

On Sun, 9 Dec 2018, 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 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 <[email protected]>
> Reviewed-by: Mimi Zohar <[email protected]>
> Acked-by: Serge Hallyn <[email protected]>

Reviewed-by: James Morris <[email protected]>

--
James Morris
<[email protected]>

2018-12-11 18:49:30

[permalink] [raw]

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

On Sun, 9 Dec 2018, Nayna Jain wrote:

> 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 <[email protected]>
> Reviewed-by: Mimi Zohar <[email protected]>
> Acked-by: Serge Hallyn <[email protected]>

Reviewed-by: James Morris <[email protected]>

--
James Morris
<[email protected]>

2018-12-11 18:51:47

[permalink] [raw]

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

On Sun, 9 Dec 2018, Nayna Jain wrote:

> From: Josh Boyer <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: David Howells <[email protected]>
> Acked-by: Nayna Jain <[email protected]>
> Acked-by: Serge Hallyn <[email protected]>

Reviewed-by: James Morris <[email protected]>

--
James Morris
<[email protected]>

2018-12-11 18:52:21

[permalink] [raw]

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

On Sun, 9 Dec 2018, Nayna Jain wrote:

> +/*
> + * 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);
> +}
>

These could be refactored into one function.

--
James Morris
<[email protected]>

2018-12-11 18:57:00

[permalink] [raw]

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

On Sun, 9 Dec 2018, 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 <[email protected]>
> Reviewed-by: Mimi Zohar <[email protected]>
> Acked-by: Serge Hallyn <[email protected]>
> - replace 'rc' with 'xattr_len' when calling integrity_digsig_verify()
> with INTEGRITY_KEYRING_IMA for readability
> Suggested-by: Serge Hallyn <[email protected]>

Reviewed-by: James Morris <[email protected]>

--
James Morris
<[email protected]>

2018-12-11 19:39:38

[permalink] [raw]

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

On Sun, 9 Dec 2018, Nayna Jain wrote:

> From: Dave Howells <[email protected]>
>
> 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 <[email protected]>
> Acked-by: Nayna Jain <[email protected]>
> Acked-by: Serge Hallyn <[email protected]>

Reviewed-by: James Morris <[email protected]>

--
James Morris
<[email protected]>

2018-12-12 17:38:09

[permalink] [raw]

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

On 12/12/2018 12:17 AM, James Morris wrote:
> On Sun, 9 Dec 2018, Nayna Jain wrote:
>
>> +/*
>> + * 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);
>> +}
>>
> These could be refactored into one function.
>
>

Thanks James for reviewing.

Yes, the code should be refactored. However, I think making it a single
function would require adding a new field to the function callback
definitions as well. Probably, a simpler approach would be to define a
common function uefi_blacklist_hash(...) which can then be used by the
two functions uefi_blacklist_x509_tbs(...) and
uefi_blacklist_binary(...). These two functions now act as wrapper
functions. Below is the example code:

+/*
+ * Blacklist a hash.
+ */
+static __init void uefi_blacklist_hash(const char *source, const void
*data,
+                                 size_t len, char *type, size_t type_len)
+{
+       char *hash, *p;
+
+       hash = kmalloc(type_len + len * 2 + 1, GFP_KERNEL);
+       if (!hash)
+               return;
+       p = memcpy(hash, type, type_len);
+       p += type_len;
+       bin2hex(p, data, len);
+       p += len * 2;
+       *p = 0;
+
+       mark_hash_blacklisted(hash);
+       kfree(hash);
+}
+
+/*
+ * Blacklist an X509 TBS hash.
+ */
+static __init void uefi_blacklist_x509_tbs(const char *source,
+                                          const void *data, size_t len)
+{
+       uefi_blacklist_hash(source, data, len, "tbs:" , 4);
+}
+
+/*
+ * Blacklist the hash of an executable.
+ */
+static __init void uefi_blacklist_binary(const char *source,
+                                        const void *data, size_t len)
+{
+       uefi_blacklist_hash(source, data, len, "bin:" , 4);
+}

Thanks & Regards,
   - Nayna

2018-12-12 18:18:06

[permalink] [raw]

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

Hello,

Nayna Jain <[email protected]> writes:

> 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 <[email protected]>
> Reviewed-by: Mimi Zohar <[email protected]>
> Acked-by: Serge Hallyn <[email protected]>
> - replace 'rc' with 'xattr_len' when calling integrity_digsig_verify()
> with INTEGRITY_KEYRING_IMA for readability
> Suggested-by: Serge Hallyn <[email protected]>
> ---
> Changelog:
>
> v2:
> - replace 'rc' with 'xattr_len' when calling integrity_digsig_verify()
> with INTEGRITY_KEYRING_IMA for readability
>
> security/integrity/ima/ima_appraise.c | 13 +++++++++++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
> index deec1804a00a..e8f520450895 100644
> --- a/security/integrity/ima/ima_appraise.c
> +++ b/security/integrity/ima/ima_appraise.c
> @@ -289,12 +289,21 @@ int ima_appraise_measurement(enum ima_hooks func,
> case EVM_IMA_XATTR_DIGSIG:
> set_bit(IMA_DIGSIG, &iint->atomic_flags);
> rc = integrity_digsig_verify(INTEGRITY_KEYRING_IMA,
> - (const char *)xattr_value, rc,
> + (const char *)xattr_value,
> + xattr_len,
> iint->ima_hash->digest,
> 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 CONFIG_INTEGRITY_PLATFORM_KEYRING=n the second call to
integrity_digsig_verify() above will always fail, and the audit message
of failed signature verifications for KEXEC_KERNEL will always log the
same rc value, which is whatever request_key() returns when asked to
look for an inexistent keyring.

Here is a patch which only performs the second try if the platform
keyring is enabled.

From d5fb94ab9eb13f6294f8dc44d1344cb85dfa41b8 Mon Sep 17 00:00:00 2001
From: Thiago Jung Bauermann <[email protected]>
Date: Wed, 12 Dec 2018 16:02:09 -0200
Subject: [PATCH] ima: Only use the platform keyring if it's enabled

Signed-off-by: Thiago Jung Bauermann <[email protected]>
---
security/integrity/ima/ima_appraise.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index e8f520450895..f6ac405daabb 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -297,7 +297,8 @@ int ima_appraise_measurement(enum ima_hooks func,
status = INTEGRITY_UNKNOWN;
break;
}
- if (rc && func == KEXEC_KERNEL_CHECK)
+ if (IS_ENABLED(CONFIG_INTEGRITY_PLATFORM_KEYRING) && rc &&
+ func == KEXEC_KERNEL_CHECK)
rc = integrity_digsig_verify(INTEGRITY_KEYRING_PLATFORM,
(const char *)xattr_value,
xattr_len,

2018-12-12 21:36:57

[permalink] [raw]

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

From: Josh Boyer <[email protected]>

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.

[[email protected]: dropped Josh's original patch description]
Signed-off-by: Josh Boyer <[email protected]>
Signed-off-by: David Howells <[email protected]>
Signed-off-by: Nayna Jain <[email protected]>
Acked-by: Serge Hallyn <[email protected]>
Signed-off-by: Mimi Zohar <[email protected]>
---
Changelog:

v2a:
- refactored uefi_blacklist_x509_tbs() and uefi_blacklist_binary()

v2:
- Fixed the checkpatch.pl warnings

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 | 169 ++++++++++++++++++++++++++
2 files changed, 173 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..8ceafa58d98c
--- /dev/null
+++ b/security/integrity/platform_certs/load_uefi.c
@@ -0,0 +1,169 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#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 efi_guid_t efi_cert_x509_guid __initdata = EFI_CERT_X509_GUID;
+static efi_guid_t efi_cert_x509_sha256_guid __initdata =
+ EFI_CERT_X509_SHA256_GUID;
+static efi_guid_t efi_cert_sha256_guid __initdata = 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)
+ 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 a hash.
+ */
+static __init void uefi_blacklist_hash(const char *source, const void *data,
+ size_t len, const char *type,
+ size_t type_len)
+{
+ char *hash, *p;
+
+ hash = kmalloc(type_len + len * 2 + 1, GFP_KERNEL);
+ if (!hash)
+ return;
+ p = memcpy(hash, type, type_len);
+ p += type_len;
+ bin2hex(p, data, len);
+ p += len * 2;
+ *p = 0;
+
+ mark_hash_blacklisted(hash);
+ kfree(hash);
+}
+
+/*
+ * Blacklist an X509 TBS hash.
+ */
+static __init void uefi_blacklist_x509_tbs(const char *source,
+ const void *data, size_t len)
+{
+ uefi_blacklist_hash(source, data, len, "tbs:", 4);
+}
+
+/*
+ * Blacklist the hash of an executable.
+ */
+static __init void uefi_blacklist_binary(const char *source,
+ const void *data, size_t len)
+{
+ uefi_blacklist_hash(source, data, len, "bin:", 4);
+}
+
+/*
+ * 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

2018-12-13 00:18:42

[permalink] [raw]

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

Nayna Jain <[email protected]> writes:

> 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 <[email protected]>
> Reviewed-by: Mimi Zohar <[email protected]>
> Acked-by: Serge Hallyn <[email protected]>
> ---
> security/integrity/Kconfig | 11 +++++
> security/integrity/Makefile | 1 +
> security/integrity/digsig.c | 48 +++++++++++++++-------
> security/integrity/integrity.h | 3 +-
> .../integrity/platform_certs/platform_keyring.c | 35 ++++++++++++++++
> 5 files changed, 83 insertions(+), 15 deletions(-)
> create mode 100644 security/integrity/platform_certs/platform_keyring.c

Reviewed-by: Thiago Jung Bauermann <[email protected]>

--
Thiago Jung Bauermann
IBM Linux Technology Center

2018-12-13 00:18:54

[permalink] [raw]

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

Nayna Jain <[email protected]> writes:

> 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 <[email protected]>
> Reviewed-by: Mimi Zohar <[email protected]>
> Acked-by: Serge Hallyn <[email protected]>
> ---
> security/integrity/digsig.c | 71 ++++++++++++++--------
> security/integrity/integrity.h | 20 ++++++
> .../integrity/platform_certs/platform_keyring.c | 23 +++++++
> 3 files changed, 90 insertions(+), 24 deletions(-)

Reviewed-by: Thiago Jung Bauermann <[email protected]>

--
Thiago Jung Bauermann
IBM Linux Technology Center

2018-12-13 00:21:20

by Mimi Zohar

[permalink] [raw]

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

On Wed, 2018-12-12 at 16:14 -0200, Thiago Jung Bauermann wrote:
[snip]

> Subject: [PATCH] ima: Only use the platform keyring if it's enabled
>
> Signed-off-by: Thiago Jung Bauermann <[email protected]>

Good catch! Thanks.

Mimi

> ---
> security/integrity/ima/ima_appraise.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
> index e8f520450895..f6ac405daabb 100644
> --- a/security/integrity/ima/ima_appraise.c
> +++ b/security/integrity/ima/ima_appraise.c
> @@ -297,7 +297,8 @@ int ima_appraise_measurement(enum ima_hooks func,
> status = INTEGRITY_UNKNOWN;
> break;
> }
> - if (rc && func == KEXEC_KERNEL_CHECK)
> + if (IS_ENABLED(CONFIG_INTEGRITY_PLATFORM_KEYRING) && rc &&
> + func == KEXEC_KERNEL_CHECK)
> rc = integrity_digsig_verify(INTEGRITY_KEYRING_PLATFORM,
> (const char *)xattr_value,
> xattr_len,

2018-12-13 00:22:17