This third patch series includes back three fix patches taken from the first
series (and cherry-picked from David Howells's tree [1]), and one cosmetic fix
from Alex Shi which helps avoid future conflicts. I also added some Acked-by
and improved comments. As requested, this series is based on v5.11-rc3.
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.
[1] https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git/log/?h=keys-fixes
Previous patch series:
https://lore.kernel.org/lkml/[email protected]/
Regards,
Alex Shi (1):
certs/blacklist: fix kernel doc interface issue
David Howells (1):
certs: Fix blacklist flag type confusion
Mickaël Salaün (8):
certs: Fix blacklisted hexadecimal hash string check
PKCS#7: Fix missing include
certs: Replace K{U,G}IDT_INIT() with GLOBAL_ROOT_{U,G}ID
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 | 217 ++++++++++++++----
certs/system_keyring.c | 5 +-
crypto/asymmetric_keys/x509_public_key.c | 3 +-
include/keys/system_keyring.h | 14 +-
include/linux/key.h | 1 +
include/linux/verification.h | 2 +
scripts/check-blacklist-hashes.awk | 37 +++
security/integrity/ima/ima_mok.c | 4 +-
.../platform_certs/keyring_handler.c | 26 +--
security/keys/key.c | 2 +
tools/certs/print-cert-tbs-hash.sh | 91 ++++++++
15 files changed, 345 insertions(+), 85 deletions(-)
create mode 100755 scripts/check-blacklist-hashes.awk
create mode 100755 tools/certs/print-cert-tbs-hash.sh
base-commit: 7c53f6b671f4aba70ff15e1b05148b10d58c2837
--
2.30.0
From: Mickaël Salaün <[email protected]>
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 <[email protected]>
Cc: David Woodhouse <[email protected]>
Signed-off-by: Mickaël Salaün <[email protected]>
Acked-by: Jarkko Sakkinen <[email protected]>
---
Changes since v2:
* Fix typo in blacklist_vet_description() comment, spotted by Tyler
Hicks.
* Add Jarkko's Acked-by.
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..334ab7b964bc 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 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;
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.30.0
From: Mickaël Salaün <[email protected]>
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 <[email protected]>
Cc: David S. Miller <[email protected]>
Cc: David Woodhouse <[email protected]>
Cc: Herbert Xu <[email protected]>
Signed-off-by: Mickaël Salaün <[email protected]>
Acked-by: Jarkko Sakkinen <[email protected]>
---
Changes since v2:
* Add Jarkko's Acked-by.
---
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 334ab7b964bc..1e63971bea94 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.30.0
From: Mickaël Salaün <[email protected]>
Add missing linux/types.h for size_t.
[DH: Changed from stddef.h]
Signed-off-by: Mickaël Salaün <[email protected]>
Signed-off-by: David Howells <[email protected]>
Reviewed-by: Ben Boeckel <[email protected]>
---
Changes since v2:
* Cherry-pick v1 patch from
https://lore.kernel.org/lkml/[email protected]/
to rebase on v5.11-rc3.
---
include/linux/verification.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/include/linux/verification.h b/include/linux/verification.h
index 911ab7c2b1ab..a655923335ae 100644
--- a/include/linux/verification.h
+++ b/include/linux/verification.h
@@ -8,6 +8,8 @@
#ifndef _LINUX_VERIFICATION_H
#define _LINUX_VERIFICATION_H
+#include <linux/types.h>
+
/*
* Indicate that both builtin trusted keys and secondary trusted keys
* should be used.
--
2.30.0
From: Mickaël Salaün <[email protected]>
When looking for a blacklisted hash, bin2hex() is used to transform a
binary hash to an ascii (lowercase) hexadecimal string. This string is
then search for in the description of the keys from the blacklist
keyring. When adding a key to the blacklist keyring,
blacklist_vet_description() checks the hash prefix and the hexadecimal
string, but not that this string is lowercase. It is then valid to set
hashes with uppercase hexadecimal, which will be silently ignored by the
kernel.
Add an additional check to blacklist_vet_description() to check that
hexadecimal strings are in lowercase.
Cc: David Woodhouse <[email protected]>
Signed-off-by: Mickaël Salaün <[email protected]>
Signed-off-by: David Howells <[email protected]>
Reviewed-by: Ben Boeckel <[email protected]>
---
Changes since v2:
* Cherry-pick v1 patch from
https://lore.kernel.org/lkml/[email protected]/
to rebase on v5.11-rc3.
* Rearrange Cc order.
---
certs/blacklist.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/certs/blacklist.c b/certs/blacklist.c
index 2719fb2fbc1c..a888b934a1cd 100644
--- a/certs/blacklist.c
+++ b/certs/blacklist.c
@@ -37,7 +37,7 @@ static int blacklist_vet_description(const char *desc)
found_colon:
desc++;
for (; *desc; desc++) {
- if (!isxdigit(*desc))
+ if (!isxdigit(*desc) || isupper(*desc))
return -EINVAL;
n++;
}
--
2.30.0
From: Mickaël Salaün <[email protected]>
Align with the new macros and add appropriate include files.
Cc: David Woodhouse <[email protected]>
Signed-off-by: Mickaël Salaün <[email protected]>
Signed-off-by: David Howells <[email protected]>
---
Changes since v2:
* Cherry-pick v1 patch from
https://lore.kernel.org/lkml/[email protected]/
to rebase on v5.11-rc3.
---
certs/blacklist.c | 4 ++--
certs/system_keyring.c | 5 +++--
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/certs/blacklist.c b/certs/blacklist.c
index 029471947838..bffe4c6f4a9e 100644
--- a/certs/blacklist.c
+++ b/certs/blacklist.c
@@ -14,6 +14,7 @@
#include <linux/ctype.h>
#include <linux/err.h>
#include <linux/seq_file.h>
+#include <linux/uidgid.h>
#include <keys/system_keyring.h>
#include "blacklist.h"
@@ -156,8 +157,7 @@ static int __init blacklist_init(void)
blacklist_keyring =
keyring_alloc(".blacklist",
- KUIDT_INIT(0), KGIDT_INIT(0),
- current_cred(),
+ GLOBAL_ROOT_UID, GLOBAL_ROOT_GID, current_cred(),
(KEY_POS_ALL & ~KEY_POS_SETATTR) |
KEY_USR_VIEW | KEY_USR_READ |
KEY_USR_SEARCH,
diff --git a/certs/system_keyring.c b/certs/system_keyring.c
index 798291177186..4b693da488f1 100644
--- a/certs/system_keyring.c
+++ b/certs/system_keyring.c
@@ -11,6 +11,7 @@
#include <linux/cred.h>
#include <linux/err.h>
#include <linux/slab.h>
+#include <linux/uidgid.h>
#include <linux/verification.h>
#include <keys/asymmetric-type.h>
#include <keys/system_keyring.h>
@@ -98,7 +99,7 @@ static __init int system_trusted_keyring_init(void)
builtin_trusted_keys =
keyring_alloc(".builtin_trusted_keys",
- KUIDT_INIT(0), KGIDT_INIT(0), current_cred(),
+ 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,
@@ -109,7 +110,7 @@ static __init int system_trusted_keyring_init(void)
#ifdef CONFIG_SECONDARY_TRUSTED_KEYRING
secondary_trusted_keys =
keyring_alloc(".secondary_trusted_keys",
- KUIDT_INIT(0), KGIDT_INIT(0), current_cred(),
+ GLOBAL_ROOT_UID, GLOBAL_ROOT_GID, current_cred(),
((KEY_POS_ALL & ~KEY_POS_SETATTR) |
KEY_USR_VIEW | KEY_USR_READ | KEY_USR_SEARCH |
KEY_USR_WRITE),
--
2.30.0
From: Alex Shi <[email protected]>
certs/blacklist.c:84: warning: Function parameter or member 'hash' not
described in 'mark_hash_blacklisted'
Cc: David Woodhouse <[email protected]>
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Alex Shi <[email protected]>
Signed-off-by: David Howells <[email protected]>
Reviewed-by: Ben Boeckel <[email protected]>
Signed-off-by: Mickaël Salaün <[email protected]>
---
Changes since v2:
* Cherry-pick patch from
https://lore.kernel.org/lkml/[email protected]/
to avoid future merge conflicts.
* Rearrange Signed-off-by and Reviewed-by order.
---
certs/blacklist.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/certs/blacklist.c b/certs/blacklist.c
index 6514f9ebc943..2719fb2fbc1c 100644
--- a/certs/blacklist.c
+++ b/certs/blacklist.c
@@ -78,7 +78,7 @@ static struct key_type key_type_blacklist = {
/**
* mark_hash_blacklisted - Add a hash to the system blacklist
- * @hash - The hash as a hex string with a type prefix (eg. "tbs:23aa429783")
+ * @hash: The hash as a hex string with a type prefix (eg. "tbs:23aa429783")
*/
int mark_hash_blacklisted(const char *hash)
{
--
2.30.0
From: Mickaël Salaün <[email protected]>
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 <[email protected]>
Cc: David Woodhouse <[email protected]>
Signed-off-by: Mickaël Salaün <[email protected]>
---
Changes since v2:
* Add comment for blacklist_key_instantiate().
---
certs/Kconfig | 10 ++++++
certs/blacklist.c | 90 +++++++++++++++++++++++++++++++++++++----------
2 files changed, 81 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 1e63971bea94..07c592ae5307 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,51 @@ 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
+ /*
+ * It should not be possible to come here because the keyring doesn't
+ * have KEY_USR_WRITE and the only other way to call this function is
+ * for builtin hashes.
+ */
+ 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 +133,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 +183,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 +242,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.30.0
From: Mickaël Salaün <[email protected]>
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 <[email protected]>
Cc: David Woodhouse <[email protected]>
Signed-off-by: Mickaël Salaün <[email protected]>
---
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 bda31ccbfad0..c7bad1354531 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4123,6 +4123,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 <[email protected]>
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 <[email protected]>
+#
+# 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.30.0
Hi Micka?l,
On Thu, 2021-01-14 at 16:19 +0100, Micka?l Sala?n wrote:
> From: Micka?l Sala?n <[email protected]>
>
> 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.
The design looks good. I'm hoping to review/test at least this patch
next week.
thanks,
Mimi
On Thu, Jan 14, 2021 at 04:19:00PM +0100, Micka?l Sala?n wrote:
> From: Alex Shi <[email protected]>
>
> certs/blacklist.c:84: warning: Function parameter or member 'hash' not
> described in 'mark_hash_blacklisted'
>
> Cc: David Woodhouse <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Alex Shi <[email protected]>
> Signed-off-by: David Howells <[email protected]>
> Reviewed-by: Ben Boeckel <[email protected]>
> Signed-off-by: Micka?l Sala?n <[email protected]>
Reviewed-by: Jarkko Sakkinen <[email protected]>
/Jarkko
On Thu, Jan 14, 2021 at 04:19:01PM +0100, Micka?l Sala?n wrote:
> From: Micka?l Sala?n <[email protected]>
>
> When looking for a blacklisted hash, bin2hex() is used to transform a
> binary hash to an ascii (lowercase) hexadecimal string. This string is
> then search for in the description of the keys from the blacklist
> keyring. When adding a key to the blacklist keyring,
> blacklist_vet_description() checks the hash prefix and the hexadecimal
> string, but not that this string is lowercase. It is then valid to set
> hashes with uppercase hexadecimal, which will be silently ignored by the
> kernel.
>
> Add an additional check to blacklist_vet_description() to check that
> hexadecimal strings are in lowercase.
>
> Cc: David Woodhouse <[email protected]>
> Signed-off-by: Micka?l Sala?n <[email protected]>
> Signed-off-by: David Howells <[email protected]>
> Reviewed-by: Ben Boeckel <[email protected]>
> ---
>
> Changes since v2:
> * Cherry-pick v1 patch from
> https://lore.kernel.org/lkml/[email protected]/
> to rebase on v5.11-rc3.
> * Rearrange Cc order.
> ---
> certs/blacklist.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/certs/blacklist.c b/certs/blacklist.c
> index 2719fb2fbc1c..a888b934a1cd 100644
> --- a/certs/blacklist.c
> +++ b/certs/blacklist.c
> @@ -37,7 +37,7 @@ static int blacklist_vet_description(const char *desc)
> found_colon:
> desc++;
> for (; *desc; desc++) {
> - if (!isxdigit(*desc))
> + if (!isxdigit(*desc) || isupper(*desc))
> return -EINVAL;
> n++;
> }
> --
> 2.30.0
>
Shouldn't this rather convert the upper case to lower case? I don't like
the ABI break that this causes.
/Jarkko
On Thu, Jan 14, 2021 at 04:19:05PM +0100, Micka?l Sala?n wrote:
> From: Micka?l Sala?n <[email protected]>
>
> 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 <[email protected]>
> Cc: David Woodhouse <[email protected]>
> Signed-off-by: Micka?l Sala?n <[email protected]>
> Acked-by: Jarkko Sakkinen <[email protected]>
In this I'm not as worried about ABI, i.e. you don't have any reason
supply any other data, which doesn't follow these ruels, whereas there
could very well be a script that does format hex "incorrectly".
/Jarkko
On Thu, Jan 14, 2021 at 04:19:04PM +0100, Micka?l Sala?n wrote:
> From: Micka?l Sala?n <[email protected]>
>
> Align with the new macros and add appropriate include files.
>
> Cc: David Woodhouse <[email protected]>
> Signed-off-by: Micka?l Sala?n <[email protected]>
> Signed-off-by: David Howells <[email protected]>
The commit message makes no sense. What you new macros?
/Jarkko
On Wed, Jan 20, 2021 at 12:12:50PM +0100, Micka?l Sala?n wrote:
>
> On 20/01/2021 04:43, Jarkko Sakkinen wrote:
> > On Thu, Jan 14, 2021 at 04:19:01PM +0100, Micka?l Sala?n wrote:
> >> From: Micka?l Sala?n <[email protected]>
> >>
> >> When looking for a blacklisted hash, bin2hex() is used to transform a
> >> binary hash to an ascii (lowercase) hexadecimal string. This string is
> >> then search for in the description of the keys from the blacklist
> >> keyring. When adding a key to the blacklist keyring,
> >> blacklist_vet_description() checks the hash prefix and the hexadecimal
> >> string, but not that this string is lowercase. It is then valid to set
> >> hashes with uppercase hexadecimal, which will be silently ignored by the
> >> kernel.
> >>
> >> Add an additional check to blacklist_vet_description() to check that
> >> hexadecimal strings are in lowercase.
> >>
> >> Cc: David Woodhouse <[email protected]>
> >> Signed-off-by: Micka?l Sala?n <[email protected]>
> >> Signed-off-by: David Howells <[email protected]>
> >> Reviewed-by: Ben Boeckel <[email protected]>
> >> ---
> >>
> >> Changes since v2:
> >> * Cherry-pick v1 patch from
> >> https://lore.kernel.org/lkml/[email protected]/
> >> to rebase on v5.11-rc3.
> >> * Rearrange Cc order.
> >> ---
> >> certs/blacklist.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/certs/blacklist.c b/certs/blacklist.c
> >> index 2719fb2fbc1c..a888b934a1cd 100644
> >> --- a/certs/blacklist.c
> >> +++ b/certs/blacklist.c
> >> @@ -37,7 +37,7 @@ static int blacklist_vet_description(const char *desc)
> >> found_colon:
> >> desc++;
> >> for (; *desc; desc++) {
> >> - if (!isxdigit(*desc))
> >> + if (!isxdigit(*desc) || isupper(*desc))
> >> return -EINVAL;
> >> n++;
> >> }
> >> --
> >> 2.30.0
> >>
> >
> > Shouldn't this rather convert the upper case to lower case? I don't like
> > the ABI break that this causes.
>
> It doesn't break the ABI because keys loaded in the blacklist keyring
> can only happen with builtin hashes. Moreover these builtin hashes will
> be checked by patch 10/10 at build time.
Right the patches are just out of order then.
/Jarkko
>
> This patch is also important to remove a false sense of security and
> warns about mis-blacklisted certificates or binaries:
> https://lore.kernel.org/lkml/[email protected]/
>
> Hot-patching keys doesn't seem a good idea, especially when these keys
> are signed. Moreover, it would bring additional complexity and will
> require to change the core of the key management.
>
> >
> > /Jarkko
> >
>