linux-integrity.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/5] Enable root to update the blacklist keyring
@ 2020-12-11 19:03 Mickaël Salaün
  2020-12-11 19:03 ` [PATCH v2 1/5] certs: Make blacklist_vet_description() more strict Mickaël Salaün
                   ` (5 more replies)
  0 siblings, 6 replies; 9+ messages in thread
From: Mickaël Salaün @ 2020-12-11 19:03 UTC (permalink / raw)
  To: David Howells, David Woodhouse, Jarkko Sakkinen
  Cc: Mickaël Salaün, David S . Miller, Herbert Xu,
	James Morris, Mickaël Salaün, Mimi Zohar,
	Serge E . Hallyn, keyrings, linux-crypto, linux-integrity,
	linux-kernel, linux-security-module

Hi,

This second patch series includes some minor fixes and remove the 4 fix
patches picked by David Howells.  This patch series can then be applied
on top of
https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git/log/?h=keys-fixes

The goal of these patches is to add a new configuration option to enable
the root user to load signed keys in the blacklist keyring.  This
keyring is useful to "untrust" certificates or files.  Enabling to
safely update this keyring without recompiling the kernel makes it more
usable.

Previous patch series:
https://lore.kernel.org/lkml/20201120180426.922572-1-mic@digikod.net/

Regards,

Mickaël Salaün (5):
  certs: Make blacklist_vet_description() more strict
  certs: Factor out the blacklist hash creation
  certs: Check that builtin blacklist hashes are valid
  certs: Allow root user to append signed hashes to the blacklist
    keyring
  tools/certs: Add print-cert-tbs-hash.sh

 MAINTAINERS                                   |   2 +
 certs/.gitignore                              |   1 +
 certs/Kconfig                                 |  10 +
 certs/Makefile                                |  15 +-
 certs/blacklist.c                             | 202 ++++++++++++++----
 crypto/asymmetric_keys/x509_public_key.c      |   3 +-
 include/keys/system_keyring.h                 |  14 +-
 scripts/check-blacklist-hashes.awk            |  37 ++++
 .../platform_certs/keyring_handler.c          |  26 +--
 tools/certs/print-cert-tbs-hash.sh            |  91 ++++++++
 10 files changed, 326 insertions(+), 75 deletions(-)
 create mode 100755 scripts/check-blacklist-hashes.awk
 create mode 100755 tools/certs/print-cert-tbs-hash.sh


base-commit: 1b91ea77dfeb2c5924ab940f2e43177c78a37d8f
-- 
2.29.2


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2 1/5] certs: Make blacklist_vet_description() more strict
  2020-12-11 19:03 [PATCH v2 0/5] Enable root to update the blacklist keyring Mickaël Salaün
@ 2020-12-11 19:03 ` Mickaël Salaün
  2020-12-11 19:03 ` [PATCH v2 2/5] certs: Factor out the blacklist hash creation Mickaël Salaün
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Mickaël Salaün @ 2020-12-11 19:03 UTC (permalink / raw)
  To: David Howells, David Woodhouse, Jarkko Sakkinen
  Cc: Mickaël Salaün, David S . Miller, Herbert Xu,
	James Morris, Mickaël Salaün, Mimi Zohar,
	Serge E . Hallyn, keyrings, linux-crypto, linux-integrity,
	linux-kernel, linux-security-module

From: Mickaël Salaün <mic@linux.microsoft.com>

Before exposing this new key type to user space, make sure that only
meaningful blacklisted hashes are accepted.  This is also checked for
builtin blacklisted hashes, but a following commit make sure that the
user will notice (at built time) and will fix the configuration if it
already included errors.

Check that a blacklist key description starts with a valid prefix and
then a valid hexadecimal string.

Cc: David Howells <dhowells@redhat.com>
Cc: David Woodhouse <dwmw2@infradead.org>
Signed-off-by: Mickaël Salaün <mic@linux.microsoft.com>
---

Changes since v1:
* Return ENOPKG (instead of EINVAL) when a hash is greater than the
  maximum currently known hash (suggested by David Howells).
---
 certs/blacklist.c | 46 ++++++++++++++++++++++++++++++++++++----------
 1 file changed, 36 insertions(+), 10 deletions(-)

diff --git a/certs/blacklist.c b/certs/blacklist.c
index bffe4c6f4a9e..9ea7d4eb6bfb 100644
--- a/certs/blacklist.c
+++ b/certs/blacklist.c
@@ -18,6 +18,16 @@
 #include <keys/system_keyring.h>
 #include "blacklist.h"
 
+/*
+ * According to crypto/asymmetric_keys/x509_cert_parser.c:x509_note_pkey_algo(),
+ * the size of the currently longest supported hash algorithm is 512 bits,
+ * which translates into 128 hex characters.
+ */
+#define MAX_HASH_LEN	128
+
+static const char tbs_prefix[] = "tbs";
+static const char bin_prefix[] = "bin";
+
 static struct key *blacklist_keyring;
 
 /*
@@ -26,24 +36,40 @@ static struct key *blacklist_keyring;
  */
 static int blacklist_vet_description(const char *desc)
 {
-	int n = 0;
-
-	if (*desc == ':')
-		return -EINVAL;
-	for (; *desc; desc++)
-		if (*desc == ':')
-			goto found_colon;
+	int i, prefix_len, tbs_step = 0, bin_step = 0;
+
+	/* The following algorithm only works if prefix lenghts match. */
+	BUILD_BUG_ON(sizeof(tbs_prefix) != sizeof(bin_prefix));
+	prefix_len = sizeof(tbs_prefix) - 1;
+	for (i = 0; *desc; desc++, i++) {
+		if (*desc == ':') {
+			if (tbs_step == prefix_len)
+				goto found_colon;
+			if (bin_step == prefix_len)
+				goto found_colon;
+			return -EINVAL;
+		}
+		if (i >= prefix_len)
+			return -EINVAL;
+		if (*desc == tbs_prefix[i])
+			tbs_step++;
+		if (*desc == bin_prefix[i])
+			bin_step++;
+	}
 	return -EINVAL;
 
 found_colon:
 	desc++;
-	for (; *desc; desc++) {
+	for (i = 0; *desc && i < MAX_HASH_LEN; desc++, i++) {
 		if (!isxdigit(*desc) || isupper(*desc))
 			return -EINVAL;
-		n++;
 	}
+	if (*desc)
+		/* The hash is greater than MAX_HASH_LEN. */
+		return -ENOPKG;
 
-	if (n == 0 || n & 1)
+	/* Checks for an even number of hexadecimal characters. */
+	if (i == 0 || i & 1)
 		return -EINVAL;
 	return 0;
 }
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 2/5] certs: Factor out the blacklist hash creation
  2020-12-11 19:03 [PATCH v2 0/5] Enable root to update the blacklist keyring Mickaël Salaün
  2020-12-11 19:03 ` [PATCH v2 1/5] certs: Make blacklist_vet_description() more strict Mickaël Salaün
@ 2020-12-11 19:03 ` Mickaël Salaün
  2020-12-11 19:03 ` [PATCH v2 3/5] certs: Check that builtin blacklist hashes are valid Mickaël Salaün
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Mickaël Salaün @ 2020-12-11 19:03 UTC (permalink / raw)
  To: David Howells, David Woodhouse, Jarkko Sakkinen
  Cc: Mickaël Salaün, David S . Miller, Herbert Xu,
	James Morris, Mickaël Salaün, Mimi Zohar,
	Serge E . Hallyn, keyrings, linux-crypto, linux-integrity,
	linux-kernel, linux-security-module

From: Mickaël Salaün <mic@linux.microsoft.com>

Factor out the blacklist hash creation with the get_raw_hash() helper.
This also centralize the "tbs" and "bin" prefixes and make them private,
which help to manage them consistently.

Cc: David Howells <dhowells@redhat.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: Jarkko Sakkinen <jarkko@kernel.org>
Signed-off-by: Mickaël Salaün <mic@linux.microsoft.com>
---
 certs/blacklist.c                             | 73 ++++++++++++++-----
 crypto/asymmetric_keys/x509_public_key.c      |  3 +-
 include/keys/system_keyring.h                 | 14 +++-
 .../platform_certs/keyring_handler.c          | 26 +------
 4 files changed, 70 insertions(+), 46 deletions(-)

diff --git a/certs/blacklist.c b/certs/blacklist.c
index 9ea7d4eb6bfb..18653c1db6bd 100644
--- a/certs/blacklist.c
+++ b/certs/blacklist.c
@@ -103,11 +103,43 @@ static struct key_type key_type_blacklist = {
 	.describe		= blacklist_describe,
 };
 
+static char *get_raw_hash(const u8 *hash, size_t hash_len,
+		enum blacklist_hash_type hash_type)
+{
+	size_t type_len;
+	const char *type_prefix;
+	char *buffer, *p;
+
+	switch (hash_type) {
+	case BLACKLIST_HASH_X509_TBS:
+		type_len = sizeof(tbs_prefix) - 1;
+		type_prefix = tbs_prefix;
+		break;
+	case BLACKLIST_HASH_BINARY:
+		type_len = sizeof(bin_prefix) - 1;
+		type_prefix = bin_prefix;
+		break;
+	default:
+		WARN_ON_ONCE(1);
+		return ERR_PTR(-EINVAL);
+	}
+	buffer = kmalloc(type_len + 1 + hash_len * 2 + 1, GFP_KERNEL);
+	if (!buffer)
+		return ERR_PTR(-ENOMEM);
+	p = memcpy(buffer, type_prefix, type_len);
+	p += type_len;
+	*p++ = ':';
+	bin2hex(p, hash, hash_len);
+	p += hash_len * 2;
+	*p = '\0';
+	return buffer;
+}
+
 /**
- * mark_hash_blacklisted - Add a hash to the system blacklist
+ * mark_raw_hash_blacklisted - Add a hash to the system blacklist
  * @hash: The hash as a hex string with a type prefix (eg. "tbs:23aa429783")
  */
-int mark_hash_blacklisted(const char *hash)
+static int mark_raw_hash_blacklisted(const char *hash)
 {
 	key_ref_t key;
 
@@ -127,29 +159,33 @@ int mark_hash_blacklisted(const char *hash)
 	return 0;
 }
 
+int mark_hash_blacklisted(const u8 *hash, size_t hash_len,
+		enum blacklist_hash_type hash_type)
+{
+	const char *buffer;
+
+	buffer = get_raw_hash(hash, hash_len, hash_type);
+	if (IS_ERR(buffer))
+		return PTR_ERR(buffer);
+	kfree(buffer);
+	return 0;
+}
 /**
  * is_hash_blacklisted - Determine if a hash is blacklisted
  * @hash: The hash to be checked as a binary blob
  * @hash_len: The length of the binary hash
- * @type: Type of hash
+ * @hash_type: Type of hash
  */
-int is_hash_blacklisted(const u8 *hash, size_t hash_len, const char *type)
+int is_hash_blacklisted(const u8 *hash, size_t hash_len,
+		enum blacklist_hash_type hash_type)
 {
 	key_ref_t kref;
-	size_t type_len = strlen(type);
-	char *buffer, *p;
+	const char *buffer;
 	int ret = 0;
 
-	buffer = kmalloc(type_len + 1 + hash_len * 2 + 1, GFP_KERNEL);
-	if (!buffer)
-		return -ENOMEM;
-	p = memcpy(buffer, type, type_len);
-	p += type_len;
-	*p++ = ':';
-	bin2hex(p, hash, hash_len);
-	p += hash_len * 2;
-	*p = 0;
-
+	buffer = get_raw_hash(hash, hash_len, hash_type);
+	if (IS_ERR(buffer))
+		return PTR_ERR(buffer);
 	kref = keyring_search(make_key_ref(blacklist_keyring, true),
 			      &key_type_blacklist, buffer, false);
 	if (!IS_ERR(kref)) {
@@ -164,7 +200,8 @@ EXPORT_SYMBOL_GPL(is_hash_blacklisted);
 
 int is_binary_blacklisted(const u8 *hash, size_t hash_len)
 {
-	if (is_hash_blacklisted(hash, hash_len, "bin") == -EKEYREJECTED)
+	if (is_hash_blacklisted(hash, hash_len, BLACKLIST_HASH_BINARY) ==
+			-EKEYREJECTED)
 		return -EPERM;
 
 	return 0;
@@ -194,7 +231,7 @@ static int __init blacklist_init(void)
 		panic("Can't allocate system blacklist keyring\n");
 
 	for (bl = blacklist_hashes; *bl; bl++)
-		if (mark_hash_blacklisted(*bl) < 0)
+		if (mark_raw_hash_blacklisted(*bl) < 0)
 			pr_err("- blacklisting failed\n");
 	return 0;
 }
diff --git a/crypto/asymmetric_keys/x509_public_key.c b/crypto/asymmetric_keys/x509_public_key.c
index ae450eb8be14..3b7dba5e4cd9 100644
--- a/crypto/asymmetric_keys/x509_public_key.c
+++ b/crypto/asymmetric_keys/x509_public_key.c
@@ -81,7 +81,8 @@ int x509_get_sig_params(struct x509_certificate *cert)
 	if (ret < 0)
 		goto error_2;
 
-	ret = is_hash_blacklisted(sig->digest, sig->digest_size, "tbs");
+	ret = is_hash_blacklisted(sig->digest, sig->digest_size,
+				  BLACKLIST_HASH_X509_TBS);
 	if (ret == -EKEYREJECTED) {
 		pr_err("Cert %*phN is blacklisted\n",
 		       sig->digest_size, sig->digest);
diff --git a/include/keys/system_keyring.h b/include/keys/system_keyring.h
index fb8b07daa9d1..b184d8743e23 100644
--- a/include/keys/system_keyring.h
+++ b/include/keys/system_keyring.h
@@ -10,6 +10,13 @@
 
 #include <linux/key.h>
 
+enum blacklist_hash_type {
+	/* TBSCertificate hash */
+	BLACKLIST_HASH_X509_TBS = 1,
+	/* Raw data hash */
+	BLACKLIST_HASH_BINARY = 2,
+};
+
 #ifdef CONFIG_SYSTEM_TRUSTED_KEYRING
 
 extern int restrict_link_by_builtin_trusted(struct key *keyring,
@@ -32,13 +39,14 @@ extern int restrict_link_by_builtin_and_secondary_trusted(
 #endif
 
 #ifdef CONFIG_SYSTEM_BLACKLIST_KEYRING
-extern int mark_hash_blacklisted(const char *hash);
+extern int mark_hash_blacklisted(const u8 *hash, size_t hash_len,
+			       enum blacklist_hash_type hash_type);
 extern int is_hash_blacklisted(const u8 *hash, size_t hash_len,
-			       const char *type);
+			       enum blacklist_hash_type hash_type);
 extern int is_binary_blacklisted(const u8 *hash, size_t hash_len);
 #else
 static inline int is_hash_blacklisted(const u8 *hash, size_t hash_len,
-				      const char *type)
+				      enum blacklist_hash_type hash_type)
 {
 	return 0;
 }
diff --git a/security/integrity/platform_certs/keyring_handler.c b/security/integrity/platform_certs/keyring_handler.c
index c5ba695c10e3..4a78fa1fde53 100644
--- a/security/integrity/platform_certs/keyring_handler.c
+++ b/security/integrity/platform_certs/keyring_handler.c
@@ -15,35 +15,13 @@ 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;
 
-/*
- * 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);
+	mark_hash_blacklisted(data, len, BLACKLIST_HASH_X509_TBS);
 }
 
 /*
@@ -52,7 +30,7 @@ static __init void uefi_blacklist_x509_tbs(const char *source,
 static __init void uefi_blacklist_binary(const char *source,
 					 const void *data, size_t len)
 {
-	uefi_blacklist_hash(source, data, len, "bin:", 4);
+	mark_hash_blacklisted(data, len, BLACKLIST_HASH_BINARY);
 }
 
 /*
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 3/5] certs: Check that builtin blacklist hashes are valid
  2020-12-11 19:03 [PATCH v2 0/5] Enable root to update the blacklist keyring Mickaël Salaün
  2020-12-11 19:03 ` [PATCH v2 1/5] certs: Make blacklist_vet_description() more strict Mickaël Salaün
  2020-12-11 19:03 ` [PATCH v2 2/5] certs: Factor out the blacklist hash creation Mickaël Salaün
@ 2020-12-11 19:03 ` Mickaël Salaün
  2020-12-11 19:03 ` [PATCH v2 4/5] certs: Allow root user to append signed hashes to the blacklist keyring Mickaël Salaün
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Mickaël Salaün @ 2020-12-11 19:03 UTC (permalink / raw)
  To: David Howells, David Woodhouse, Jarkko Sakkinen
  Cc: Mickaël Salaün, David S . Miller, Herbert Xu,
	James Morris, Mickaël Salaün, Mimi Zohar,
	Serge E . Hallyn, keyrings, linux-crypto, linux-integrity,
	linux-kernel, linux-security-module

From: Mickaël Salaün <mic@linux.microsoft.com>

Add and use a check-blacklist-hashes.awk script to make sure that the
builtin blacklist hashes will be approved by the run time blacklist
description checks.  This is useful to debug invalid hash formats, and
it make sure that previous hashes which could have been loaded in the
kernel (but ignored) are now noticed and deal with by the user.

Cc: David Howells <dhowells@redhat.com>
Cc: David Woodhouse <dwmw2@infradead.org>
Signed-off-by: Mickaël Salaün <mic@linux.microsoft.com>
---

Changes since v1:
* Prefix script path with $(scrtree)/ (suggested by David Howells).
* Fix hexadecimal number check.
---
 MAINTAINERS                        |  1 +
 certs/.gitignore                   |  1 +
 certs/Makefile                     | 15 +++++++++++-
 scripts/check-blacklist-hashes.awk | 37 ++++++++++++++++++++++++++++++
 4 files changed, 53 insertions(+), 1 deletion(-)
 create mode 100755 scripts/check-blacklist-hashes.awk

diff --git a/MAINTAINERS b/MAINTAINERS
index a008b70f3c16..e9d4453caf5d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4080,6 +4080,7 @@ L:	keyrings@vger.kernel.org
 S:	Maintained
 F:	Documentation/admin-guide/module-signing.rst
 F:	certs/
+F:	scripts/check-blacklist-hashes.awk
 F:	scripts/extract-cert.c
 F:	scripts/sign-file.c
 
diff --git a/certs/.gitignore b/certs/.gitignore
index 2a2483990686..42cc2ac24b93 100644
--- a/certs/.gitignore
+++ b/certs/.gitignore
@@ -1,2 +1,3 @@
 # SPDX-License-Identifier: GPL-2.0-only
+blacklist_hashes_checked
 x509_certificate_list
diff --git a/certs/Makefile b/certs/Makefile
index f4c25b67aad9..eb45407ff282 100644
--- a/certs/Makefile
+++ b/certs/Makefile
@@ -6,7 +6,20 @@
 obj-$(CONFIG_SYSTEM_TRUSTED_KEYRING) += system_keyring.o system_certificates.o
 obj-$(CONFIG_SYSTEM_BLACKLIST_KEYRING) += blacklist.o
 ifneq ($(CONFIG_SYSTEM_BLACKLIST_HASH_LIST),"")
+
+quiet_cmd_check_blacklist_hashes = CHECK   $(patsubst "%",%,$(2))
+      cmd_check_blacklist_hashes = $(AWK) -f $(srctree)/scripts/check-blacklist-hashes.awk $(2); touch $@
+
+$(eval $(call config_filename,SYSTEM_BLACKLIST_HASH_LIST))
+
+$(obj)/blacklist_hashes.o: $(obj)/blacklist_hashes_checked
+
+targets += blacklist_hashes_checked
+$(obj)/blacklist_hashes_checked: $(SYSTEM_BLACKLIST_HASH_LIST_SRCPREFIX)$(SYSTEM_BLACKLIST_HASH_LIST_FILENAME) scripts/check-blacklist-hashes.awk FORCE
+	$(call if_changed,check_blacklist_hashes,$(SYSTEM_BLACKLIST_HASH_LIST_SRCPREFIX)$(CONFIG_SYSTEM_BLACKLIST_HASH_LIST))
+
 obj-$(CONFIG_SYSTEM_BLACKLIST_KEYRING) += blacklist_hashes.o
+
 else
 obj-$(CONFIG_SYSTEM_BLACKLIST_KEYRING) += blacklist_nohashes.o
 endif
@@ -29,7 +42,7 @@ $(obj)/x509_certificate_list: scripts/extract-cert $(SYSTEM_TRUSTED_KEYS_SRCPREF
 	$(call if_changed,extract_certs,$(SYSTEM_TRUSTED_KEYS_SRCPREFIX)$(CONFIG_SYSTEM_TRUSTED_KEYS))
 endif # CONFIG_SYSTEM_TRUSTED_KEYRING
 
-clean-files := x509_certificate_list .x509.list
+clean-files := x509_certificate_list .x509.list blacklist_hashes_checked
 
 ifeq ($(CONFIG_MODULE_SIG),y)
 ###############################################################################
diff --git a/scripts/check-blacklist-hashes.awk b/scripts/check-blacklist-hashes.awk
new file mode 100755
index 000000000000..107c1d3204d4
--- /dev/null
+++ b/scripts/check-blacklist-hashes.awk
@@ -0,0 +1,37 @@
+#!/usr/bin/awk -f
+# SPDX-License-Identifier: GPL-2.0
+#
+# Copyright © 2020, Microsoft Corporation. All rights reserved.
+#
+# Author: Mickaël Salaün <mic@linux.microsoft.com>
+#
+# Check that a CONFIG_SYSTEM_BLACKLIST_HASH_LIST file contains a valid array of
+# hash strings.  Such string must start with a prefix ("tbs" or "bin"), then a
+# colon (":"), and finally an even number of hexadecimal lowercase characters
+# (up to 128).
+
+BEGIN {
+	RS = ","
+}
+{
+	if (!match($0, "^[ \t\n\r]*\"([^\"]*)\"[ \t\n\r]*$", part1)) {
+		print "Not a string (item " NR "):", $0;
+		exit 1;
+	}
+	if (!match(part1[1], "^(tbs|bin):(.*)$", part2)) {
+		print "Unknown prefix (item " NR "):", part1[1];
+		exit 1;
+	}
+	if (!match(part2[2], "^([0-9a-f]+)$", part3)) {
+		print "Not a lowercase hexadecimal string (item " NR "):", part2[2];
+		exit 1;
+	}
+	if (length(part3[1]) > 128) {
+		print "Hash string too long (item " NR "):", part3[1];
+		exit 1;
+	}
+	if (length(part3[1]) % 2 == 1) {
+		print "Not an even number of hexadecimal characters (item " NR "):", part3[1];
+		exit 1;
+	}
+}
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 4/5] certs: Allow root user to append signed hashes to the blacklist keyring
  2020-12-11 19:03 [PATCH v2 0/5] Enable root to update the blacklist keyring Mickaël Salaün
                   ` (2 preceding siblings ...)
  2020-12-11 19:03 ` [PATCH v2 3/5] certs: Check that builtin blacklist hashes are valid Mickaël Salaün
@ 2020-12-11 19:03 ` Mickaël Salaün
  2020-12-11 19:03 ` [PATCH v2 5/5] tools/certs: Add print-cert-tbs-hash.sh Mickaël Salaün
  2021-01-05 10:12 ` [PATCH v2 0/5] Enable root to update the blacklist keyring Mickaël Salaün
  5 siblings, 0 replies; 9+ messages in thread
From: Mickaël Salaün @ 2020-12-11 19:03 UTC (permalink / raw)
  To: David Howells, David Woodhouse, Jarkko Sakkinen
  Cc: Mickaël Salaün, David S . Miller, Herbert Xu,
	James Morris, Mickaël Salaün, Mimi Zohar,
	Serge E . Hallyn, keyrings, linux-crypto, linux-integrity,
	linux-kernel, linux-security-module

From: Mickaël Salaün <mic@linux.microsoft.com>

Add a kernel option SYSTEM_BLACKLIST_AUTH_UPDATE to enable the root user
to dynamically add new keys to the blacklist keyring.  This enables to
invalidate new certificates, either from being loaded in a keyring, or
from being trusted in a PKCS#7 certificate chain.  This also enables to
add new file hashes to be denied by the integrity infrastructure.

Being able to untrust a certificate which could have normaly been
trusted is a sensitive operation.  This is why adding new hashes to the
blacklist keyring is only allowed when these hashes are signed and
vouched by the builtin trusted keyring.  A blacklist hash is stored as a
key description.  The PKCS#7 signature of this description must be
provided as the key payload.

Marking a certificate as untrusted should be enforced while the system
is running.  It is then forbiden to remove such blacklist keys.

Update blacklist keyring and blacklist key access rights:
* allows the root user to search for a specific blacklisted hash, which
  make sense because the descriptions are already viewable;
* forbids key update;
* restricts kernel rights on the blacklist keyring to align with the
  root user rights.

See the help in tools/certs/print-cert-tbs-hash.sh provided by a
following commit.

Cc: David Howells <dhowells@redhat.com>
Cc: David Woodhouse <dwmw2@infradead.org>
Signed-off-by: Mickaël Salaün <mic@linux.microsoft.com>
---
 certs/Kconfig     | 10 ++++++
 certs/blacklist.c | 85 ++++++++++++++++++++++++++++++++++++-----------
 2 files changed, 76 insertions(+), 19 deletions(-)

diff --git a/certs/Kconfig b/certs/Kconfig
index c94e93d8bccf..35fe9989e7b9 100644
--- a/certs/Kconfig
+++ b/certs/Kconfig
@@ -83,4 +83,14 @@ config SYSTEM_BLACKLIST_HASH_LIST
 	  wrapper to incorporate the list into the kernel.  Each <hash> should
 	  be a string of hex digits.
 
+config SYSTEM_BLACKLIST_AUTH_UPDATE
+	bool "Allow root to add signed blacklist keys"
+	depends on SYSTEM_BLACKLIST_KEYRING
+	depends on SYSTEM_DATA_VERIFICATION
+	help
+	  If set, provide the ability to load new blacklist keys at run time if
+	  they are signed and vouched by a certificate from the builtin trusted
+	  keyring.  The PKCS#7 signature of the description is set in the key
+	  payload.  Blacklist keys cannot be removed.
+
 endmenu
diff --git a/certs/blacklist.c b/certs/blacklist.c
index 18653c1db6bd..111d3bcea1e4 100644
--- a/certs/blacklist.c
+++ b/certs/blacklist.c
@@ -15,6 +15,7 @@
 #include <linux/err.h>
 #include <linux/seq_file.h>
 #include <linux/uidgid.h>
+#include <linux/verification.h>
 #include <keys/system_keyring.h>
 #include "blacklist.h"
 
@@ -25,6 +26,9 @@
  */
 #define MAX_HASH_LEN	128
 
+#define BLACKLIST_KEY_PERM (KEY_POS_SEARCH | KEY_POS_VIEW | \
+			    KEY_USR_SEARCH | KEY_USR_VIEW)
+
 static const char tbs_prefix[] = "tbs";
 static const char bin_prefix[] = "bin";
 
@@ -74,19 +78,46 @@ static int blacklist_vet_description(const char *desc)
 	return 0;
 }
 
-/*
- * The hash to be blacklisted is expected to be in the description.  There will
- * be no payload.
- */
-static int blacklist_preparse(struct key_preparsed_payload *prep)
+static int blacklist_key_instantiate(struct key *key,
+		struct key_preparsed_payload *prep)
 {
-	if (prep->datalen > 0)
-		return -EINVAL;
-	return 0;
+#ifdef CONFIG_SYSTEM_BLACKLIST_AUTH_UPDATE
+	int err;
+#endif
+
+	/* Sets safe default permissions for keys loaded by user space. */
+	key->perm = BLACKLIST_KEY_PERM;
+
+	/*
+	 * Skips the authentication step for builtin hashes, they are not
+	 * signed but still trusted.
+	 */
+	if (key->flags & (1 << KEY_FLAG_BUILTIN))
+		goto out;
+
+#ifdef CONFIG_SYSTEM_BLACKLIST_AUTH_UPDATE
+	/*
+	 * Verifies the description's PKCS#7 signature against the builtin
+	 * trusted keyring.
+	 */
+	err = verify_pkcs7_signature(key->description,
+			strlen(key->description), prep->data, prep->datalen,
+			NULL, VERIFYING_UNSPECIFIED_SIGNATURE, NULL, NULL);
+	if (err)
+		return err;
+#else
+	WARN_ON_ONCE(1);
+	return -EPERM;
+#endif
+
+out:
+	return generic_key_instantiate(key, prep);
 }
 
-static void blacklist_free_preparse(struct key_preparsed_payload *prep)
+static int blacklist_key_update(struct key *key,
+		struct key_preparsed_payload *prep)
 {
+	return -EPERM;
 }
 
 static void blacklist_describe(const struct key *key, struct seq_file *m)
@@ -97,9 +128,8 @@ static void blacklist_describe(const struct key *key, struct seq_file *m)
 static struct key_type key_type_blacklist = {
 	.name			= "blacklist",
 	.vet_description	= blacklist_vet_description,
-	.preparse		= blacklist_preparse,
-	.free_preparse		= blacklist_free_preparse,
-	.instantiate		= generic_key_instantiate,
+	.instantiate		= blacklist_key_instantiate,
+	.update			= blacklist_key_update,
 	.describe		= blacklist_describe,
 };
 
@@ -148,8 +178,7 @@ static int mark_raw_hash_blacklisted(const char *hash)
 				   hash,
 				   NULL,
 				   0,
-				   ((KEY_POS_ALL & ~KEY_POS_SETATTR) |
-				    KEY_USR_VIEW),
+				   BLACKLIST_KEY_PERM,
 				   KEY_ALLOC_NOT_IN_QUOTA |
 				   KEY_ALLOC_BUILT_IN);
 	if (IS_ERR(key)) {
@@ -208,25 +237,43 @@ int is_binary_blacklisted(const u8 *hash, size_t hash_len)
 }
 EXPORT_SYMBOL_GPL(is_binary_blacklisted);
 
+static int restrict_link_for_blacklist(struct key *dest_keyring,
+		const struct key_type *type, const union key_payload *payload,
+		struct key *restrict_key)
+{
+	if (type != &key_type_blacklist)
+		return -EPERM;
+	return 0;
+}
+
 /*
  * Initialise the blacklist
  */
 static int __init blacklist_init(void)
 {
 	const char *const *bl;
+	struct key_restriction *restriction;
 
 	if (register_key_type(&key_type_blacklist) < 0)
 		panic("Can't allocate system blacklist key type\n");
 
+	restriction = kzalloc(sizeof(*restriction), GFP_KERNEL);
+	if (!restriction)
+		panic("Can't allocate blacklist keyring restriction\n");
+	restriction->check = restrict_link_for_blacklist;
+
 	blacklist_keyring =
 		keyring_alloc(".blacklist",
 			      GLOBAL_ROOT_UID, GLOBAL_ROOT_GID, current_cred(),
-			      (KEY_POS_ALL & ~KEY_POS_SETATTR) |
-			      KEY_USR_VIEW | KEY_USR_READ |
-			      KEY_USR_SEARCH,
-			      KEY_ALLOC_NOT_IN_QUOTA |
+			      KEY_POS_VIEW | KEY_POS_READ | KEY_POS_SEARCH |
+			      KEY_POS_WRITE |
+			      KEY_USR_VIEW | KEY_USR_READ | KEY_USR_SEARCH
+#ifdef CONFIG_SYSTEM_BLACKLIST_AUTH_UPDATE
+			      | KEY_USR_WRITE
+#endif
+			      , KEY_ALLOC_NOT_IN_QUOTA |
 			      KEY_ALLOC_SET_KEEP,
-			      NULL, NULL);
+			      restriction, NULL);
 	if (IS_ERR(blacklist_keyring))
 		panic("Can't allocate system blacklist keyring\n");
 
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 5/5] tools/certs: Add print-cert-tbs-hash.sh
  2020-12-11 19:03 [PATCH v2 0/5] Enable root to update the blacklist keyring Mickaël Salaün
                   ` (3 preceding siblings ...)
  2020-12-11 19:03 ` [PATCH v2 4/5] certs: Allow root user to append signed hashes to the blacklist keyring Mickaël Salaün
@ 2020-12-11 19:03 ` Mickaël Salaün
  2021-01-05 10:12 ` [PATCH v2 0/5] Enable root to update the blacklist keyring Mickaël Salaün
  5 siblings, 0 replies; 9+ messages in thread
From: Mickaël Salaün @ 2020-12-11 19:03 UTC (permalink / raw)
  To: David Howells, David Woodhouse, Jarkko Sakkinen
  Cc: Mickaël Salaün, David S . Miller, Herbert Xu,
	James Morris, Mickaël Salaün, Mimi Zohar,
	Serge E . Hallyn, keyrings, linux-crypto, linux-integrity,
	linux-kernel, linux-security-module

From: Mickaël Salaün <mic@linux.microsoft.com>

Add a new helper print-cert-tbs-hash.sh to generate a TBSCertificate
hash from a given certificate.  This is useful to generate a blacklist
key description used to forbid loading a specific certificate in a
keyring, or to invalidate a certificate provided by a PKCS#7 file.

Cc: David Howells <dhowells@redhat.com>
Cc: David Woodhouse <dwmw2@infradead.org>
Signed-off-by: Mickaël Salaün <mic@linux.microsoft.com>
---

Changes since v1:
* Fix typo.
* Use "if" block instead of "||" .
---
 MAINTAINERS                        |  1 +
 tools/certs/print-cert-tbs-hash.sh | 91 ++++++++++++++++++++++++++++++
 2 files changed, 92 insertions(+)
 create mode 100755 tools/certs/print-cert-tbs-hash.sh

diff --git a/MAINTAINERS b/MAINTAINERS
index e9d4453caf5d..666c99595b26 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4083,6 +4083,7 @@ F:	certs/
 F:	scripts/check-blacklist-hashes.awk
 F:	scripts/extract-cert.c
 F:	scripts/sign-file.c
+F:	tools/certs/
 
 CFAG12864B LCD DRIVER
 M:	Miguel Ojeda Sandonis <miguel.ojeda.sandonis@gmail.com>
diff --git a/tools/certs/print-cert-tbs-hash.sh b/tools/certs/print-cert-tbs-hash.sh
new file mode 100755
index 000000000000..c93df5387ec9
--- /dev/null
+++ b/tools/certs/print-cert-tbs-hash.sh
@@ -0,0 +1,91 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+#
+# Copyright © 2020, Microsoft Corporation. All rights reserved.
+#
+# Author: Mickaël Salaün <mic@linux.microsoft.com>
+#
+# Compute and print the To Be Signed (TBS) hash of a certificate.  This is used
+# as description of keys in the blacklist keyring to identify certificates.
+# This output should be redirected, without newline, in a file (hash0.txt) and
+# signed to create a PKCS#7 file (hash0.p7s).  Both of these files can then be
+# loaded in the kernel with.
+#
+# Exemple on a workstation:
+# ./print-cert-tbs-hash.sh certificate-to-invalidate.pem > hash0.txt
+# openssl smime -sign -in hash0.txt -inkey builtin-private-key.pem \
+#               -signer builtin-certificate.pem -certfile certificate-chain.pem \
+#               -noattr -binary -outform DER -out hash0.p7s
+#
+# Exemple on a managed system:
+# keyctl padd blacklist "$(< hash0.txt)" %:.blacklist < hash0.p7s
+
+set -u -e -o pipefail
+
+CERT="${1:-}"
+BASENAME="$(basename -- "${BASH_SOURCE[0]}")"
+
+if [ $# -ne 1 ] || [ ! -f "${CERT}" ]; then
+	echo "usage: ${BASENAME} <certificate>" >&2
+	exit 1
+fi
+
+# Checks that it is indeed a certificate (PEM or DER encoded) and exclude the
+# optional PEM text header.
+if ! PEM="$(openssl x509 -inform DER -in "${CERT}" 2>/dev/null || openssl x509 -in "${CERT}")"; then
+	echo "ERROR: Failed to parse certificate" >&2
+	exit 1
+fi
+
+# TBSCertificate starts at the second entry.
+# Cf. https://tools.ietf.org/html/rfc3280#section-4.1
+#
+# Exemple of first lines printed by openssl asn1parse:
+#    0:d=0  hl=4 l= 763 cons: SEQUENCE
+#    4:d=1  hl=4 l= 483 cons: SEQUENCE
+#    8:d=2  hl=2 l=   3 cons: cont [ 0 ]
+#   10:d=3  hl=2 l=   1 prim: INTEGER           :02
+#   13:d=2  hl=2 l=  20 prim: INTEGER           :3CEB2CB8818D968AC00EEFE195F0DF9665328B7B
+#   35:d=2  hl=2 l=  13 cons: SEQUENCE
+#   37:d=3  hl=2 l=   9 prim: OBJECT            :sha256WithRSAEncryption
+RANGE_AND_DIGEST_RE='
+2s/^\s*\([0-9]\+\):d=\s*[0-9]\+\s\+hl=\s*[0-9]\+\s\+l=\s*\([0-9]\+\)\s\+cons:\s*SEQUENCE\s*$/\1 \2/p;
+7s/^\s*[0-9]\+:d=\s*[0-9]\+\s\+hl=\s*[0-9]\+\s\+l=\s*[0-9]\+\s\+prim:\s*OBJECT\s*:\(.*\)$/\1/p;
+'
+
+RANGE_AND_DIGEST=($(echo "${PEM}" | \
+	openssl asn1parse -in - | \
+	sed -n -e "${RANGE_AND_DIGEST_RE}"))
+
+if [ "${#RANGE_AND_DIGEST[@]}" != 3 ]; then
+	echo "ERROR: Failed to parse TBSCertificate." >&2
+	exit 1
+fi
+
+OFFSET="${RANGE_AND_DIGEST[0]}"
+END="$(( OFFSET + RANGE_AND_DIGEST[1] ))"
+DIGEST="${RANGE_AND_DIGEST[2]}"
+
+# The signature hash algorithm is used by Linux to blacklist certificates.
+# Cf. crypto/asymmetric_keys/x509_cert_parser.c:x509_note_pkey_algo()
+DIGEST_MATCH=""
+while read -r DIGEST_ITEM; do
+	if [ -z "${DIGEST_ITEM}" ]; then
+		break
+	fi
+	if echo "${DIGEST}" | grep -qiF "${DIGEST_ITEM}"; then
+		DIGEST_MATCH="${DIGEST_ITEM}"
+		break
+	fi
+done < <(openssl list -digest-commands | tr ' ' '\n' | sort -ur)
+
+if [ -z "${DIGEST_MATCH}" ]; then
+	echo "ERROR: Unknown digest algorithm: ${DIGEST}" >&2
+	exit 1
+fi
+
+echo "${PEM}" | \
+	openssl x509 -in - -outform DER | \
+	dd "bs=1" "skip=${OFFSET}" "count=${END}" "status=none" | \
+	openssl dgst "-${DIGEST_MATCH}" - | \
+	awk '{printf "tbs:" $2}'
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 0/5] Enable root to update the blacklist keyring
  2020-12-11 19:03 [PATCH v2 0/5] Enable root to update the blacklist keyring Mickaël Salaün
                   ` (4 preceding siblings ...)
  2020-12-11 19:03 ` [PATCH v2 5/5] tools/certs: Add print-cert-tbs-hash.sh Mickaël Salaün
@ 2021-01-05 10:12 ` Mickaël Salaün
  2021-01-10  4:57   ` Jarkko Sakkinen
  5 siblings, 1 reply; 9+ messages in thread
From: Mickaël Salaün @ 2021-01-05 10:12 UTC (permalink / raw)
  To: David Howells, David Woodhouse, Jarkko Sakkinen
  Cc: David S . Miller, Herbert Xu, James Morris,
	Mickaël Salaün, Mimi Zohar, Serge E . Hallyn, keyrings,
	linux-crypto, linux-integrity, linux-kernel,
	linux-security-module

Jarkko, David, what is the status of this patch series? Do you need help
to test it?

On 11/12/2020 20:03, Mickaël Salaün wrote:
> Hi,
> 
> This second patch series includes some minor fixes and remove the 4 fix
> patches picked by David Howells.  This patch series can then be applied
> on top of
> https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git/log/?h=keys-fixes
> 
> The goal of these patches is to add a new configuration option to enable
> the root user to load signed keys in the blacklist keyring.  This
> keyring is useful to "untrust" certificates or files.  Enabling to
> safely update this keyring without recompiling the kernel makes it more
> usable.
> 
> Previous patch series:
> https://lore.kernel.org/lkml/20201120180426.922572-1-mic@digikod.net/
> 
> Regards,
> 
> Mickaël Salaün (5):
>   certs: Make blacklist_vet_description() more strict
>   certs: Factor out the blacklist hash creation
>   certs: Check that builtin blacklist hashes are valid
>   certs: Allow root user to append signed hashes to the blacklist
>     keyring
>   tools/certs: Add print-cert-tbs-hash.sh
> 
>  MAINTAINERS                                   |   2 +
>  certs/.gitignore                              |   1 +
>  certs/Kconfig                                 |  10 +
>  certs/Makefile                                |  15 +-
>  certs/blacklist.c                             | 202 ++++++++++++++----
>  crypto/asymmetric_keys/x509_public_key.c      |   3 +-
>  include/keys/system_keyring.h                 |  14 +-
>  scripts/check-blacklist-hashes.awk            |  37 ++++
>  .../platform_certs/keyring_handler.c          |  26 +--
>  tools/certs/print-cert-tbs-hash.sh            |  91 ++++++++
>  10 files changed, 326 insertions(+), 75 deletions(-)
>  create mode 100755 scripts/check-blacklist-hashes.awk
>  create mode 100755 tools/certs/print-cert-tbs-hash.sh
> 
> 
> base-commit: 1b91ea77dfeb2c5924ab940f2e43177c78a37d8f
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 0/5] Enable root to update the blacklist keyring
  2021-01-05 10:12 ` [PATCH v2 0/5] Enable root to update the blacklist keyring Mickaël Salaün
@ 2021-01-10  4:57   ` Jarkko Sakkinen
  2021-01-14  4:15     ` Jarkko Sakkinen
  0 siblings, 1 reply; 9+ messages in thread
From: Jarkko Sakkinen @ 2021-01-10  4:57 UTC (permalink / raw)
  To: Mickaël Salaün
  Cc: David Howells, David Woodhouse, David S . Miller, Herbert Xu,
	James Morris, Mickaël Salaün, Mimi Zohar,
	Serge E . Hallyn, keyrings, linux-crypto, linux-integrity,
	linux-kernel, linux-security-module

On Tue, Jan 05, 2021 at 11:12:57AM +0100, Mickaël Salaün wrote:
> Jarkko, David, what is the status of this patch series? Do you need help
> to test it?

Hi, a leave/vacation and the holiday period badly mixed my schedules.

I'm testing this upcoming week.

/Jarkko

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 0/5] Enable root to update the blacklist keyring
  2021-01-10  4:57   ` Jarkko Sakkinen
@ 2021-01-14  4:15     ` Jarkko Sakkinen
  0 siblings, 0 replies; 9+ messages in thread
From: Jarkko Sakkinen @ 2021-01-14  4:15 UTC (permalink / raw)
  To: Mickaël Salaün
  Cc: David Howells, David Woodhouse, David S . Miller, Herbert Xu,
	James Morris, Mickaël Salaün, Mimi Zohar,
	Serge E . Hallyn, keyrings, linux-crypto, linux-integrity,
	linux-kernel, linux-security-module

On Sun, Jan 10, 2021 at 06:57:10AM +0200, Jarkko Sakkinen wrote:
> On Tue, Jan 05, 2021 at 11:12:57AM +0100, Mickaël Salaün wrote:
> > Jarkko, David, what is the status of this patch series? Do you need help
> > to test it?
> 
> Hi, a leave/vacation and the holiday period badly mixed my schedules.
> 
> I'm testing this upcoming week.
> 
> /Jarkko

❯ git-pw series apply 400795
Applying: certs: Make blacklist_vet_description() more strict
error: sha1 information is lacking or useless (certs/blacklist.c).
error: could not build fake ancestor
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 certs: Make blacklist_vet_description() more strict
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

Can you rebase to rc3 and resend? 

Also, please add this to the patches 1-3:

Acked-by: Jarkko Sakkinen <jarkko@kernel.org>

Also, 4-5 look good but I hold for testing before acking further.

Thanks, and apologies for such a long wait.

/Jarkko

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2021-01-14  4:16 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-11 19:03 [PATCH v2 0/5] Enable root to update the blacklist keyring Mickaël Salaün
2020-12-11 19:03 ` [PATCH v2 1/5] certs: Make blacklist_vet_description() more strict Mickaël Salaün
2020-12-11 19:03 ` [PATCH v2 2/5] certs: Factor out the blacklist hash creation Mickaël Salaün
2020-12-11 19:03 ` [PATCH v2 3/5] certs: Check that builtin blacklist hashes are valid Mickaël Salaün
2020-12-11 19:03 ` [PATCH v2 4/5] certs: Allow root user to append signed hashes to the blacklist keyring Mickaël Salaün
2020-12-11 19:03 ` [PATCH v2 5/5] tools/certs: Add print-cert-tbs-hash.sh Mickaël Salaün
2021-01-05 10:12 ` [PATCH v2 0/5] Enable root to update the blacklist keyring Mickaël Salaün
2021-01-10  4:57   ` Jarkko Sakkinen
2021-01-14  4:15     ` Jarkko Sakkinen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).