2021-02-10 12:09:03

by Mickaël Salaün

[permalink] [raw]
Subject: [PATCH v6 0/5] Enable root to update the blacklist keyring

This new patch series is a rebase on David Howells's keys-misc branch.
This mainly fixes UEFI DBX and the new Eric Snowberg's feature to import
asymmetric keys to the blacklist keyring.
I successfully tested this patch series with the 186 entries from
https://uefi.org/sites/default/files/resources/dbxupdate_x64.bin (184
binary hashes and 2 certificates).

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.

This can be applied on top of David Howells's keys-next branch:
https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git/log/?h=keys-next
Git commits can be found in https://github.com/l0kod/linux branch
dyn-auth-blacklist-v6 commit fcf976b74ffcd4551683e6b70dbf5fb102cf9906 .

Previous patch series:
https://lore.kernel.org/lkml/[email protected]/

Regards,

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

MAINTAINERS | 2 +
certs/.gitignore | 1 +
certs/Kconfig | 17 +-
certs/Makefile | 17 +-
certs/blacklist.c | 218 ++++++++++++++----
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, 346 insertions(+), 80 deletions(-)
create mode 100755 scripts/check-blacklist-hashes.awk
create mode 100755 tools/certs/print-cert-tbs-hash.sh


base-commit: 5bcd72358a7d7794ade0452ed12919b8c4d6ffc7
--
2.30.0


2021-02-10 12:09:03

by Mickaël Salaün

[permalink] [raw]
Subject: [PATCH v6 3/5] certs: Make blacklist_vet_description() more strict

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]>
Cc: Eric Snowberg <[email protected]>
Signed-off-by: Mickaël Salaün <[email protected]>
Reviewed-by: Jarkko Sakkinen <[email protected]>
---

Changes since v5:
* Add Reviewed-by Jarkko.

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 f467b10030fb..069d1dd0fa05 100644
--- a/certs/blacklist.c
+++ b/certs/blacklist.c
@@ -19,6 +19,16 @@
#include "blacklist.h"
#include "common.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;

extern __initconst const u8 revocation_certificate_list[];
@@ -30,24 +40,40 @@ extern __initconst const unsigned long revocation_certificate_list_size;
*/
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

2021-02-21 11:18:15

by Mickaël Salaün

[permalink] [raw]
Subject: Re: [PATCH v6 0/5] Enable root to update the blacklist keyring

David, Eric, what is the status of this patch series?

On 10/02/2021 13:04, Mickaël Salaün wrote:
> This new patch series is a rebase on David Howells's keys-misc branch.
> This mainly fixes UEFI DBX and the new Eric Snowberg's feature to import
> asymmetric keys to the blacklist keyring.
> I successfully tested this patch series with the 186 entries from
> https://uefi.org/sites/default/files/resources/dbxupdate_x64.bin (184
> binary hashes and 2 certificates).
>
> 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.
>
> This can be applied on top of David Howells's keys-next branch:
> https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git/log/?h=keys-next
> Git commits can be found in https://github.com/l0kod/linux branch
> dyn-auth-blacklist-v6 commit fcf976b74ffcd4551683e6b70dbf5fb102cf9906 .
>
> Previous patch series:
> https://lore.kernel.org/lkml/[email protected]/
>
> Regards,
>
> Mickaël Salaün (5):
> tools/certs: Add print-cert-tbs-hash.sh
> certs: Check that builtin blacklist hashes are valid
> certs: Make blacklist_vet_description() more strict
> certs: Factor out the blacklist hash creation
> certs: Allow root user to append signed hashes to the blacklist
> keyring
>
> MAINTAINERS | 2 +
> certs/.gitignore | 1 +
> certs/Kconfig | 17 +-
> certs/Makefile | 17 +-
> certs/blacklist.c | 218 ++++++++++++++----
> 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, 346 insertions(+), 80 deletions(-)
> create mode 100755 scripts/check-blacklist-hashes.awk
> create mode 100755 tools/certs/print-cert-tbs-hash.sh
>
>
> base-commit: 5bcd72358a7d7794ade0452ed12919b8c4d6ffc7
>