[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <YE0KMGjI+L6elHF2@kernel.org>
Date: Sat, 13 Mar 2021 20:53:36 +0200
From: Jarkko Sakkinen <jarkko@...nel.org>
To: Mickaël Salaün <mic@...ikod.net>
Cc: David Howells <dhowells@...hat.com>,
David Woodhouse <dwmw2@...radead.org>,
"David S . Miller" <davem@...emloft.net>,
Eric Snowberg <eric.snowberg@...cle.com>,
Herbert Xu <herbert@...dor.apana.org.au>,
James Morris <jmorris@...ei.org>,
Mickaël Salaün <mic@...ux.microsoft.com>,
Mimi Zohar <zohar@...ux.ibm.com>,
"Serge E . Hallyn" <serge@...lyn.com>,
Tyler Hicks <tyhicks@...ux.microsoft.com>,
keyrings@...r.kernel.org, linux-crypto@...r.kernel.org,
linux-integrity@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-security-module@...r.kernel.org
Subject: Re: [PATCH v7 2/5] certs: Check that builtin blacklist hashes are
valid
On Fri, Mar 12, 2021 at 06:12:29PM +0100, Mickaël Salaün wrote:
> From: Mickaël Salaün <mic@...ux.microsoft.com>
>
> Add and use a check-blacklist-hashes.awk script to make sure that the
> builtin blacklist hashes set with CONFIG_SYSTEM_BLACKLIST_HASH_LIST will
> effectively be taken into account as blacklisted hashes. This is useful
> to debug invalid hash formats, and it make sure that previous hashes
> which could have been loaded in the kernel, but silently ignored, are
> now noticed and deal with by the user at kernel build time.
>
> This also prevent stricter blacklist key description checking (provided
> by following commits) to failed for builtin hashes.
>
> Update CONFIG_SYSTEM_BLACKLIST_HASH_LIST help to explain the content of
> a hash string and how to generate certificate ones.
>
> Cc: David Howells <dhowells@...hat.com>
> Cc: David Woodhouse <dwmw2@...radead.org>
> Cc: Eric Snowberg <eric.snowberg@...cle.com>
> Cc: Jarkko Sakkinen <jarkko@...nel.org>
> Signed-off-by: Mickaël Salaün <mic@...ux.microsoft.com>
> Link: https://lore.kernel.org/r/20210312171232.2681989-3-mic@digikod.net
Reviewed-by: Jarkko Sakkinen <jarkko@...nel.org>
/Jarkko
> ---
>
> Changes since v5:
> * Rebase on keys-next and fix conflict as previously done by David
> Howells.
> * Enable to use a file path relative to the kernel source directory.
> This align with the handling of CONFIG_SYSTEM_TRUSTED_KEYS,
> CONFIG_MODULE_SIG_KEY and CONFIG_SYSTEM_REVOCATION_KEYS.
>
> Changes since v3:
> * Improve commit description.
> * Update CONFIG_SYSTEM_BLACKLIST_HASH_LIST help.
> * Remove Acked-by Jarkko Sakkinen because of the above changes.
>
> Changes since v2:
> * Add Jarkko's Acked-by.
>
> Changes since v1:
> * Prefix script path with $(scrtree)/ (suggested by David Howells).
> * Fix hexadecimal number check.
> ---
> MAINTAINERS | 1 +
> certs/.gitignore | 1 +
> certs/Kconfig | 7 ++++--
> certs/Makefile | 17 +++++++++++++-
> scripts/check-blacklist-hashes.awk | 37 ++++++++++++++++++++++++++++++
> 5 files changed, 60 insertions(+), 3 deletions(-)
> create mode 100755 scripts/check-blacklist-hashes.awk
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 773a362e807f..a18fd3d283c6 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -4118,6 +4118,7 @@ L: keyrings@...r.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
> F: tools/certs/
> 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/Kconfig b/certs/Kconfig
> index ab88d2a7f3c7..cf3740c1b22b 100644
> --- a/certs/Kconfig
> +++ b/certs/Kconfig
> @@ -80,8 +80,11 @@ config SYSTEM_BLACKLIST_HASH_LIST
> help
> If set, this option should be the filename of a list of hashes in the
> form "<hash>", "<hash>", ... . This will be included into a C
> - wrapper to incorporate the list into the kernel. Each <hash> should
> - be a string of hex digits.
> + wrapper to incorporate the list into the kernel. Each <hash> must be a
> + string starting with a prefix ("tbs" or "bin"), then a colon (":"), and
> + finally an even number of hexadecimal lowercase characters (up to 128).
> + Certificate hashes can be generated with
> + tools/certs/print-cert-tbs-hash.sh .
>
> config SYSTEM_REVOCATION_LIST
> bool "Provide system-wide ring of revocation certificates"
> diff --git a/certs/Makefile b/certs/Makefile
> index b6db52ebf0be..61e82b8eacd2 100644
> --- a/certs/Makefile
> +++ b/certs/Makefile
> @@ -7,7 +7,22 @@ obj-$(CONFIG_SYSTEM_TRUSTED_KEYRING) += system_keyring.o system_certificates.o c
> obj-$(CONFIG_SYSTEM_BLACKLIST_KEYRING) += blacklist.o common.o
> obj-$(CONFIG_SYSTEM_REVOCATION_LIST) += revocation_certificates.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
> +
> +CFLAGS_blacklist_hashes.o += -I$(srctree)
> +
> +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
> @@ -30,7 +45,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 x509_revocation_list
> +clean-files := x509_certificate_list .x509.list x509_revocation_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@...ux.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.30.2
>
>
Powered by blists - more mailing lists