[PATCH 1/3] certs: blacklist: define skid: entries and add is_key_blacklisted()

[Mimi Zohar]

A "tbs:" blacklist entry is only compared against certificates being
added to a keyring or presented in a signature chain, so it does not
stop a key that is already loaded from being used as a trust anchor.

Define a new "skid:<hex>" blacklist entry, holding a Subject Key
Identifier, and add is_key_blacklisted(), which looks up the SKID of an
asymmetric key in the system blacklist.  Entries are added like "tbs:"
and "bin:" entries and are subject to CONFIG_SYSTEM_BLACKLIST_AUTH_UPDATE.

blacklist_vet_description() now uses a prefix table, since the prefixes
no longer all have the same length.  "tbs:" and "bin:" entries behave
as before.

For example, to add an entry for a certificate (the signed string must
not include a trailing newline):

  skid=$(openssl x509 -in cert.pem -noout -ext subjectKeyIdentifier |
         sed -n '2s/[ :]//gp' | tr A-F a-f)
  printf 'skid:%s' "$skid" > skid.txt
  openssl smime -sign -binary -noattr -in skid.txt -signer signer.crt \
          -inkey signer.key -outform DER -out skid.p7s
  keyctl padd blacklist "skid:$skid" %:.blacklist < skid.p7s

The SKID is also the hex string in the description of the key on a
keyring, and its last four bytes are the key ID in the security.ima
xattr, so an entry is easy to correlate with the key it revokes:

  # keyctl show %:.ima | grep 073d
  927946571 --als--v  0  0   \_ asymmetric: test signing key: b7809c3372a07fe87248dcec7ff047a40fb3073d
  # keyctl show %:.blacklist | grep skid
  1014808518 ----s--v 0  0   \_ blacklist: skid:b7809c3372a07fe87248dcec7ff047a40fb3073d

Signed-off-by: Mimi Zohar <zohar@xxxxxxxxxxxxx>
---
Changelog v1:
- Addressed Sashiko issues: drop Kconfig change, linking issues when
blacklist support configured, but without CONFIG_ASYMMETRIC_KEY_TYPE
configured.

 certs/blacklist.c             | 83 ++++++++++++++++++++++++-----------
 include/keys/system_keyring.h | 12 +++++
 2 files changed, 70 insertions(+), 25 deletions(-)

diff --git a/certs/blacklist.c b/certs/blacklist.c
index fa561c12eabf..9158bd9f9fc9 100644
--- a/certs/blacklist.c
+++ b/certs/blacklist.c
@@ -32,6 +32,7 @@

 static const char tbs_prefix[] = "tbs";
 static const char bin_prefix[] = "bin";
+static const char skid_prefix[] = "skid";

 static struct key *blacklist_keyring;

@@ -40,36 +41,30 @@ extern __initconst const u8 revocation_certificate_list[];
 extern __initconst const unsigned long revocation_certificate_list_size;
 #endif

+static bool blacklist_prefix_valid(const char *desc, size_t len)
+{
+	return (len == sizeof(tbs_prefix) - 1 &&
+		!strncmp(desc, tbs_prefix, len)) ||
+	       (len == sizeof(bin_prefix) - 1 &&
+		!strncmp(desc, bin_prefix, len)) ||
+	       (len == sizeof(skid_prefix) - 1 &&
+		!strncmp(desc, skid_prefix, len));
+}
+
 /*
- * The description must be a type prefix, a colon and then an even number of
- * hex digits.  The hash is kept in the description.
+ * The description must be a type prefix ("tbs", "bin" or "skid"), a colon and
+ * then an even number of lower-case hex digits.  The hash (or, for "skid", the
+ * Subject Key Identifier) is kept in the description.
  */
 static int blacklist_vet_description(const char *desc)
 {
-	int i, prefix_len, tbs_step = 0, bin_step = 0;
-
-	/* The following algorithm only works if prefix lengths 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;
+	const char *colon = strchr(desc, ':');
+	int i;

-found_colon:
-	desc++;
+	if (!colon || !blacklist_prefix_valid(desc, colon - desc))
+		return -EINVAL;
+
+	desc = colon + 1;
 	for (i = 0; *desc && i < MAX_HASH_LEN; desc++, i++) {
 		if (!isxdigit(*desc) || isupper(*desc))
 			return -EINVAL;
@@ -160,6 +155,10 @@ static char *get_raw_hash(const u8 *hash, size_t hash_len,
 		type_len = sizeof(bin_prefix) - 1;
 		type_prefix = bin_prefix;
 		break;
+	case BLACKLIST_HASH_SKID:
+		type_len = sizeof(skid_prefix) - 1;
+		type_prefix = skid_prefix;
+		break;
 	default:
 		WARN_ON_ONCE(1);
 		return ERR_PTR(-EINVAL);
@@ -244,6 +243,40 @@ int is_hash_blacklisted(const u8 *hash, size_t hash_len,
 }
 EXPORT_SYMBOL_GPL(is_hash_blacklisted);

+#ifdef CONFIG_ASYMMETRIC_KEY_TYPE
+/**
+ * is_key_blacklisted - Determine if a key is blacklisted by its SKID
+ * @key: The key to be checked, e.g. a trust anchor found in a keyring
+ *
+ * Unlike a "tbs:" entry, which is only compared against certificates being
+ * added or presented in a chain, this checks a key that is already loaded, so
+ * that keys that cannot be unlinked from their keyring can still be revoked.
+ * The identifier is taken from the key itself, not from the caller.
+ *
+ * Returns -EKEYREJECTED if the key's Subject Key Identifier is blacklisted,
+ * 0 if it is not (or the key has no SKID), or another negative error.
+ */
+int is_key_blacklisted(const struct key *key)
+{
+	const struct asymmetric_key_ids *kids;
+	const struct asymmetric_key_id *skid;
+
+	if (!key || key->type != &key_type_asymmetric)
+		return 0;
+
+	kids = asymmetric_key_ids(key);
+	if (!kids)
+		return 0;
+
+	skid = kids->id[1];
+	if (!skid || !skid->len)
+		return 0;
+
+	return is_hash_blacklisted(skid->data, skid->len, BLACKLIST_HASH_SKID);
+}
+EXPORT_SYMBOL_GPL(is_key_blacklisted);
+#endif
+
 int is_binary_blacklisted(const u8 *hash, size_t hash_len)
 {
 	if (is_hash_blacklisted(hash, hash_len, BLACKLIST_HASH_BINARY) ==
diff --git a/include/keys/system_keyring.h b/include/keys/system_keyring.h
index a6c2897bcc63..2ac1dae2063a 100644
--- a/include/keys/system_keyring.h
+++ b/include/keys/system_keyring.h
@@ -15,6 +15,8 @@ enum blacklist_hash_type {
 	BLACKLIST_HASH_X509_TBS = 1,
 	/* Raw data hash */
 	BLACKLIST_HASH_BINARY = 2,
+	/* Subject Key Identifier of a key (not a hash) */
+	BLACKLIST_HASH_SKID = 3,
 };

 #ifdef CONFIG_SYSTEM_TRUSTED_KEYRING
@@ -92,6 +94,16 @@ static inline int is_binary_blacklisted(const u8 *hash, size_t hash_len)
 }
 #endif

+#if defined(CONFIG_SYSTEM_BLACKLIST_KEYRING) && \
+    defined(CONFIG_ASYMMETRIC_KEY_TYPE)
+extern int is_key_blacklisted(const struct key *key);
+#else
+static inline int is_key_blacklisted(const struct key *key)
+{
+	return 0;
+}
+#endif
+
 struct pkcs7_message;
 #ifdef CONFIG_SYSTEM_REVOCATION_LIST
 extern int add_key_to_revocation_list(const char *data, size_t size);
--
2.56.0