diff mbox series

certs: Add EFI_CERT_X509_GUID support for dbx entries

Message ID 2660556.1610545213@warthog.procyon.org.uk
State Accepted
Commit 56c5812623f95313f6a46fbf0beee7fa17c68bbf
Headers show
Series certs: Add EFI_CERT_X509_GUID support for dbx entries | expand

Commit Message

David Howells Jan. 13, 2021, 1:40 p.m. UTC
Hi Linus,

Are you willing to take this between merge windows - or does it need to wait
for the next merge window?  It's not technically a bug fix to the kernel, but
it does have a CVE attached to it.

Note that I've also updated Jarkko's address in his Reviewed-by since his
Intel address no longer works.

David
---
commit b5f71d4461d6d09463b2ce8bc4fc150ea1c385c0
Author: Eric Snowberg <eric.snowberg@oracle.com>
Date:   Tue Sep 15 20:49:27 2020 -0400

    certs: Add EFI_CERT_X509_GUID support for dbx entries
    
    This fixes CVE-2020-26541.
    
    The Secure Boot Forbidden Signature Database, dbx, contains a list of now
    revoked signatures and keys previously approved to boot with UEFI Secure
    Boot enabled.  The dbx is capable of containing any number of
    EFI_CERT_X509_SHA256_GUID, EFI_CERT_SHA256_GUID, and EFI_CERT_X509_GUID
    entries.
    
    Currently when EFI_CERT_X509_GUID are contained in the dbx, the entries are
    skipped.
    
    Add support for EFI_CERT_X509_GUID dbx entries. When a EFI_CERT_X509_GUID
    is found, it is added as an asymmetrical key to the .blacklist keyring.
    Anytime the .platform keyring is used, the keys in the .blacklist keyring
    are referenced, if a matching key is found, the key will be rejected.
    
    [DH: I've changed the names of the new functions with Eric's approval]
    
    Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
    Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
    Signed-off-by: David Howells <dhowells@redhat.com>

Comments

David Howells Jan. 13, 2021, 7:23 p.m. UTC | #1
David Howells <dhowells@redhat.com> wrote:

>     This fixes CVE-2020-26541.

Note that I added the CVE number, not Eric.

David
James Bottomley Jan. 13, 2021, 11:56 p.m. UTC | #2
On Wed, 2021-01-13 at 13:40 +0000, David Howells wrote:
> Hi Linus,
> 
> Are you willing to take this between merge windows - or does it need
> to wait for the next merge window?  It's not technically a bug fix to
> the kernel, but it does have a CVE attached to it.
> 
> Note that I've also updated Jarkko's address in his Reviewed-by since
> his Intel address no longer works.

Sorry, late to the party.

I suppose I lost the argument that we shouldn't really be trusting any
certs from db when shim is in operation because they're all EFI binary
signing ones and will usually simply be the microsoft certificate and
possibly an OEM platform one and we're usually pivoting the root of
trust to the certificates in the MokList.

However, if we are going to do this, we should also be blacklisting the
certificates in MokListX which the OS sees through MokListXRT.  Since
MokListX is an essential piece of our revocation infrastructure it
should have been mentioned in the CVE but wasn't for some reason.

James
diff mbox series

Patch

diff --git a/certs/blacklist.c b/certs/blacklist.c
index 6514f9ebc943..a7f021878a4b 100644
--- a/certs/blacklist.c
+++ b/certs/blacklist.c
@@ -100,6 +100,38 @@  int mark_hash_blacklisted(const char *hash)
 	return 0;
 }
 
+int add_key_to_revocation_list(const char *data, size_t size)
+{
+	key_ref_t key;
+
+	key = key_create_or_update(make_key_ref(blacklist_keyring, true),
+				   "asymmetric",
+				   NULL,
+				   data,
+				   size,
+				   ((KEY_POS_ALL & ~KEY_POS_SETATTR) | KEY_USR_VIEW),
+				   KEY_ALLOC_NOT_IN_QUOTA | KEY_ALLOC_BUILT_IN);
+
+	if (IS_ERR(key)) {
+		pr_err("Problem with revocation key (%ld)\n", PTR_ERR(key));
+		return PTR_ERR(key);
+	}
+
+	return 0;
+}
+
+int is_key_on_revocation_list(struct pkcs7_message *pkcs7)
+{
+	int ret;
+
+	ret = validate_trust(pkcs7, blacklist_keyring);
+
+	if (ret == 0)
+		return -EKEYREJECTED;
+
+	return -ENOKEY;
+}
+
 /**
  * is_hash_blacklisted - Determine if a hash is blacklisted
  * @hash: The hash to be checked as a binary blob
diff --git a/certs/blacklist.h b/certs/blacklist.h
index 1efd6fa0dc60..420bb7c86e07 100644
--- a/certs/blacklist.h
+++ b/certs/blacklist.h
@@ -1,3 +1,15 @@ 
 #include <linux/kernel.h>
+#include <linux/errno.h>
+#include <crypto/pkcs7.h>
 
 extern const char __initconst *const blacklist_hashes[];
+
+#ifdef CONFIG_INTEGRITY_PLATFORM_KEYRING
+#define validate_trust pkcs7_validate_trust
+#else
+static inline int validate_trust(struct pkcs7_message *pkcs7,
+				 struct key *trust_keyring)
+{
+	return -ENOKEY;
+}
+#endif
diff --git a/certs/system_keyring.c b/certs/system_keyring.c
index 798291177186..cc165b359ea3 100644
--- a/certs/system_keyring.c
+++ b/certs/system_keyring.c
@@ -241,6 +241,12 @@  int verify_pkcs7_message_sig(const void *data, size_t len,
 			pr_devel("PKCS#7 platform keyring is not available\n");
 			goto error;
 		}
+
+		ret = is_key_on_revocation_list(pkcs7);
+		if (ret != -ENOKEY) {
+			pr_devel("PKCS#7 platform key is on revocation list\n");
+			goto error;
+		}
 	}
 	ret = pkcs7_validate_trust(pkcs7, trusted_keys);
 	if (ret < 0) {
diff --git a/include/keys/system_keyring.h b/include/keys/system_keyring.h
index fb8b07daa9d1..61f98739e8b1 100644
--- a/include/keys/system_keyring.h
+++ b/include/keys/system_keyring.h
@@ -31,11 +31,14 @@  extern int restrict_link_by_builtin_and_secondary_trusted(
 #define restrict_link_by_builtin_and_secondary_trusted restrict_link_by_builtin_trusted
 #endif
 
+extern struct pkcs7_message *pkcs7;
 #ifdef CONFIG_SYSTEM_BLACKLIST_KEYRING
 extern int mark_hash_blacklisted(const char *hash);
+extern int add_key_to_revocation_list(const char *data, size_t size);
 extern int is_hash_blacklisted(const u8 *hash, size_t hash_len,
 			       const char *type);
 extern int is_binary_blacklisted(const u8 *hash, size_t hash_len);
+extern int is_key_on_revocation_list(struct pkcs7_message *pkcs7);
 #else
 static inline int is_hash_blacklisted(const u8 *hash, size_t hash_len,
 				      const char *type)
@@ -47,6 +50,14 @@  static inline int is_binary_blacklisted(const u8 *hash, size_t hash_len)
 {
 	return 0;
 }
+static inline int add_key_to_revocation_list(const char *data, size_t size)
+{
+	return 0;
+}
+static inline int is_key_on_revocation_list(struct pkcs7_message *pkcs7)
+{
+	return -ENOKEY;
+}
 #endif
 
 #ifdef CONFIG_IMA_BLACKLIST_KEYRING
diff --git a/security/integrity/platform_certs/keyring_handler.c b/security/integrity/platform_certs/keyring_handler.c
index c5ba695c10e3..5604bd57c990 100644
--- a/security/integrity/platform_certs/keyring_handler.c
+++ b/security/integrity/platform_certs/keyring_handler.c
@@ -55,6 +55,15 @@  static __init void uefi_blacklist_binary(const char *source,
 	uefi_blacklist_hash(source, data, len, "bin:", 4);
 }
 
+/*
+ * Add an X509 cert to the revocation list.
+ */
+static __init void uefi_revocation_list_x509(const char *source,
+					     const void *data, size_t len)
+{
+	add_key_to_revocation_list(data, len);
+}
+
 /*
  * Return the appropriate handler for particular signature list types found in
  * the UEFI db and MokListRT tables.
@@ -76,5 +85,7 @@  __init efi_element_handler_t get_handler_for_dbx(const efi_guid_t *sig_type)
 		return uefi_blacklist_x509_tbs;
 	if (efi_guidcmp(*sig_type, efi_cert_sha256_guid) == 0)
 		return uefi_blacklist_binary;
+	if (efi_guidcmp(*sig_type, efi_cert_x509_guid) == 0)
+		return uefi_revocation_list_x509;
 	return 0;
 }