Re: [PATCH v7 4/5] certs: Factor out the blacklist hash creation
On Fri, Mar 12, 2021 at 06:12:31PM +0100, Mickaël Salaün wrote:
> From: Mickaël Salaün
>
> 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
> Cc: David S. Miller
> Cc: David Woodhouse
> Cc: Eric Snowberg
> Cc: Herbert Xu
> Cc: Jarkko Sakkinen
> Signed-off-by: Mickaël Salaün
> Link: https://lore.kernel.org/r/20210312171232.2681989-5-...@digikod.net
Reviewed-by: Jarkko Sakkinen
/Jarkko
> ---
>
> Changes since v6:
> * Rebase on keys-cve-2020-26541-v3: commit ebd9c2ae369a ("integrity:
> Load mokx variables into the blacklist keyring").
>
> Changes since v5:
> * Rebase on keys-next and fix conflict as previously done by David
> Howells.
> * Fix missing part to effectively handle UEFI DBX blacklisting.
> * Remove Jarkko's Acked-by because of the above changes.
>
> Changes since v2:
> * Add Jarkko's Acked-by.
> ---
> certs/blacklist.c | 76 ++-
> crypto/asymmetric_keys/x509_public_key.c | 3 +-
> include/keys/system_keyring.h | 14 +++-
> .../platform_certs/keyring_handler.c | 26 +--
> 4 files changed, 73 insertions(+), 46 deletions(-)
>
> diff --git a/certs/blacklist.c b/certs/blacklist.c
> index 97a35cf9a62c..b254c87ceb3a 100644
> --- a/certs/blacklist.c
> +++ b/certs/blacklist.c
> @@ -109,11 +109,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;
>
> @@ -133,29 +165,36 @@ 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;
> + int err;
> +
> + buffer = get_raw_hash(hash, hash_len, hash_type);
> + if (IS_ERR(buffer))
> + return PTR_ERR(buffer);
> + err = mark_raw_hash_blacklisted(buffer);
> + kfree(buffer);
> + return err;
> +}
> +
> /**
> * 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),
> _type_blacklist, buffer, false);
> if (!IS_ERR(kref)) {
> @@ -170,7 +209,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;
> @@ -243,7 +283,7 @@ static int __init
[PATCH v7 4/5] certs: Factor out the blacklist hash creation
From: Mickaël Salaün
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
Cc: David S. Miller
Cc: David Woodhouse
Cc: Eric Snowberg
Cc: Herbert Xu
Cc: Jarkko Sakkinen
Signed-off-by: Mickaël Salaün
Link: https://lore.kernel.org/r/20210312171232.2681989-5-...@digikod.net
---
Changes since v6:
* Rebase on keys-cve-2020-26541-v3: commit ebd9c2ae369a ("integrity:
Load mokx variables into the blacklist keyring").
Changes since v5:
* Rebase on keys-next and fix conflict as previously done by David
Howells.
* Fix missing part to effectively handle UEFI DBX blacklisting.
* Remove Jarkko's Acked-by because of the above changes.
Changes since v2:
* Add Jarkko's Acked-by.
---
certs/blacklist.c | 76 ++-
crypto/asymmetric_keys/x509_public_key.c | 3 +-
include/keys/system_keyring.h | 14 +++-
.../platform_certs/keyring_handler.c | 26 +--
4 files changed, 73 insertions(+), 46 deletions(-)
diff --git a/certs/blacklist.c b/certs/blacklist.c
index 97a35cf9a62c..b254c87ceb3a 100644
--- a/certs/blacklist.c
+++ b/certs/blacklist.c
@@ -109,11 +109,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;
@@ -133,29 +165,36 @@ 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;
+ int err;
+
+ buffer = get_raw_hash(hash, hash_len, hash_type);
+ if (IS_ERR(buffer))
+ return PTR_ERR(buffer);
+ err = mark_raw_hash_blacklisted(buffer);
+ kfree(buffer);
+ return err;
+}
+
/**
* 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),
_type_blacklist, buffer, false);
if (!IS_ERR(kref)) {
@@ -170,7 +209,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;
@@ -243,7 +283,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)
