| Message ID | 20210114151909.2344974-9-mic@digikod.net |
|---|---|
| State | New |
| Headers | show |
| Series | Enable root to update the blacklist keyring | expand |
On Thu, Jan 14, 2021 at 04:19:07PM +0100, Mickaël Salaün wrote: > From: Mickaël Salaün <mic@linux.microsoft.com> > > Add and use a check-blacklist-hashes.awk script to make sure that the > builtin blacklist hashes will be approved by the run time blacklist > description checks. This is useful to debug invalid hash formats, and > it make sure that previous hashes which could have been loaded in the > kernel (but ignored) are now noticed and deal with by the user. > > Cc: David Howells <dhowells@redhat.com> > Cc: David Woodhouse <dwmw2@infradead.org> > Signed-off-by: Mickaël Salaün <mic@linux.microsoft.com> > Acked-by: Jarkko Sakkinen <jarkko@kernel.org> I get this with a self-signed cert: certs/Makefile:18: *** target pattern contains no '%'. Stop. CONFIG_SYSTEM_BLACKLIST_HASH_LIST="tbs:8eed1340eef37c1dc84d996406ad05c7dbb3eade19132d688408ca2f63904869" I used the script in 10/10 to test this, which is another reamark: the patches are in invalid order, as you need to apply 10/10 before you can test 8/10. /Jarkko
On 20/01/2021 06:19, Jarkko Sakkinen wrote: > On Thu, Jan 14, 2021 at 04:19:07PM +0100, Mickaël Salaün wrote: >> From: Mickaël Salaün <mic@linux.microsoft.com> >> >> Add and use a check-blacklist-hashes.awk script to make sure that the >> builtin blacklist hashes will be approved by the run time blacklist >> description checks. This is useful to debug invalid hash formats, and >> it make sure that previous hashes which could have been loaded in the >> kernel (but ignored) are now noticed and deal with by the user. >> >> Cc: David Howells <dhowells@redhat.com> >> Cc: David Woodhouse <dwmw2@infradead.org> >> Signed-off-by: Mickaël Salaün <mic@linux.microsoft.com> >> Acked-by: Jarkko Sakkinen <jarkko@kernel.org> > > I get this with a self-signed cert: > > certs/Makefile:18: *** target pattern contains no '%'. Stop. > > CONFIG_SYSTEM_BLACKLIST_HASH_LIST="tbs:8eed1340eef37c1dc84d996406ad05c7dbb3eade19132d688408ca2f63904869" As said in the Kconfig documentation for CONFIG_SYSTEM_BLACKLIST_HASH_LIST, you need to provide a file with the list, not to set the string directly in the configuration variable. This patch series didn't change this behavior. The same kind of macros are used for CONFIG_MODULE_SIG_KEY. > > I used the script in 10/10 to test this, which is another > reamark: the patches are in invalid order, as you need to > apply 10/10 before you can test 8/10. I'll move patch 10/10 earlier but this kind of formatting was already required (but silently ignored) for this option to be really taken into account. Only the kernel code was available to understand how to effectively create such hash. > > /Jarkko >
On Wed, Jan 20, 2021 at 12:57:55PM +0100, Mickaël Salaün wrote: > > On 20/01/2021 06:19, Jarkko Sakkinen wrote: > > On Thu, Jan 14, 2021 at 04:19:07PM +0100, Mickaël Salaün wrote: > >> From: Mickaël Salaün <mic@linux.microsoft.com> > >> > >> Add and use a check-blacklist-hashes.awk script to make sure that the > >> builtin blacklist hashes will be approved by the run time blacklist > >> description checks. This is useful to debug invalid hash formats, and > >> it make sure that previous hashes which could have been loaded in the > >> kernel (but ignored) are now noticed and deal with by the user. > >> > >> Cc: David Howells <dhowells@redhat.com> > >> Cc: David Woodhouse <dwmw2@infradead.org> > >> Signed-off-by: Mickaël Salaün <mic@linux.microsoft.com> > >> Acked-by: Jarkko Sakkinen <jarkko@kernel.org> > > > > I get this with a self-signed cert: > > > > certs/Makefile:18: *** target pattern contains no '%'. Stop. > > > > CONFIG_SYSTEM_BLACKLIST_HASH_LIST="tbs:8eed1340eef37c1dc84d996406ad05c7dbb3eade19132d688408ca2f63904869" > > As said in the Kconfig documentation for > CONFIG_SYSTEM_BLACKLIST_HASH_LIST, you need to provide a file with the > list, not to set the string directly in the configuration variable. This > patch series didn't change this behavior. The same kind of macros are > used for CONFIG_MODULE_SIG_KEY. OK, the documentation just states that: "Hashes to be preloaded into the system blacklist keyring" No mention about a file. I'd add a patch to update this documentation. > > > > > I used the script in 10/10 to test this, which is another > > reamark: the patches are in invalid order, as you need to > > apply 10/10 before you can test 8/10. > > I'll move patch 10/10 earlier but this kind of formatting was already > required (but silently ignored) for this option to be really taken into > account. Only the kernel code was available to understand how to > effectively create such hash. Great, thanks. > > > > /Jarkko > > /Jarkko
On 21/01/2021 00:53, Jarkko Sakkinen wrote: > On Wed, Jan 20, 2021 at 12:57:55PM +0100, Mickaël Salaün wrote: >> >> On 20/01/2021 06:19, Jarkko Sakkinen wrote: >>> On Thu, Jan 14, 2021 at 04:19:07PM +0100, Mickaël Salaün wrote: >>>> From: Mickaël Salaün <mic@linux.microsoft.com> >>>> >>>> Add and use a check-blacklist-hashes.awk script to make sure that the >>>> builtin blacklist hashes will be approved by the run time blacklist >>>> description checks. This is useful to debug invalid hash formats, and >>>> it make sure that previous hashes which could have been loaded in the >>>> kernel (but ignored) are now noticed and deal with by the user. >>>> >>>> Cc: David Howells <dhowells@redhat.com> >>>> Cc: David Woodhouse <dwmw2@infradead.org> >>>> Signed-off-by: Mickaël Salaün <mic@linux.microsoft.com> >>>> Acked-by: Jarkko Sakkinen <jarkko@kernel.org> >>> >>> I get this with a self-signed cert: >>> >>> certs/Makefile:18: *** target pattern contains no '%'. Stop. >>> >>> CONFIG_SYSTEM_BLACKLIST_HASH_LIST="tbs:8eed1340eef37c1dc84d996406ad05c7dbb3eade19132d688408ca2f63904869" >> >> As said in the Kconfig documentation for >> CONFIG_SYSTEM_BLACKLIST_HASH_LIST, you need to provide a file with the >> list, not to set the string directly in the configuration variable. This >> patch series didn't change this behavior. The same kind of macros are >> used for CONFIG_MODULE_SIG_KEY. > > OK, the documentation just states that: > > "Hashes to be preloaded into the system blacklist keyring" > > No mention about a file. I'd add a patch to update this documentation. I was referring to the full description: config SYSTEM_BLACKLIST_HASH_LIST string "Hashes to be preloaded into the system blacklist keyring" depends on SYSTEM_BLACKLIST_KEYRING 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. …but the short description doesn't mention filename. > >> >>> >>> I used the script in 10/10 to test this, which is another >>> reamark: the patches are in invalid order, as you need to >>> apply 10/10 before you can test 8/10. >> >> I'll move patch 10/10 earlier but this kind of formatting was already >> required (but silently ignored) for this option to be really taken into >> account. Only the kernel code was available to understand how to >> effectively create such hash. > > Great, thanks. > > >>> >>> /Jarkko >>> > > > /Jarkko >
On Thu, Jan 21, 2021 at 10:18:20AM +0100, Mickaël Salaün wrote: > > On 21/01/2021 00:53, Jarkko Sakkinen wrote: > > On Wed, Jan 20, 2021 at 12:57:55PM +0100, Mickaël Salaün wrote: > >> > >> On 20/01/2021 06:19, Jarkko Sakkinen wrote: > >>> On Thu, Jan 14, 2021 at 04:19:07PM +0100, Mickaël Salaün wrote: > >>>> From: Mickaël Salaün <mic@linux.microsoft.com> > >>>> > >>>> Add and use a check-blacklist-hashes.awk script to make sure that the > >>>> builtin blacklist hashes will be approved by the run time blacklist > >>>> description checks. This is useful to debug invalid hash formats, and > >>>> it make sure that previous hashes which could have been loaded in the > >>>> kernel (but ignored) are now noticed and deal with by the user. > >>>> > >>>> Cc: David Howells <dhowells@redhat.com> > >>>> Cc: David Woodhouse <dwmw2@infradead.org> > >>>> Signed-off-by: Mickaël Salaün <mic@linux.microsoft.com> > >>>> Acked-by: Jarkko Sakkinen <jarkko@kernel.org> > >>> > >>> I get this with a self-signed cert: > >>> > >>> certs/Makefile:18: *** target pattern contains no '%'. Stop. > >>> > >>> CONFIG_SYSTEM_BLACKLIST_HASH_LIST="tbs:8eed1340eef37c1dc84d996406ad05c7dbb3eade19132d688408ca2f63904869" > >> > >> As said in the Kconfig documentation for > >> CONFIG_SYSTEM_BLACKLIST_HASH_LIST, you need to provide a file with the > >> list, not to set the string directly in the configuration variable. This > >> patch series didn't change this behavior. The same kind of macros are > >> used for CONFIG_MODULE_SIG_KEY. > > > > OK, the documentation just states that: > > > > "Hashes to be preloaded into the system blacklist keyring" > > > > No mention about a file. I'd add a patch to update this documentation. > > I was referring to the full description: > > config SYSTEM_BLACKLIST_HASH_LIST > string "Hashes to be preloaded into the system blacklist keyring" > depends on SYSTEM_BLACKLIST_KEYRING > 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. > > …but the short description doesn't mention filename. Aah. Anyway, I'll test the next version. Now all should be clear how to approach that. Thanks. /Jarkko
diff --git a/MAINTAINERS b/MAINTAINERS index cc1e6a5ee6e6..bda31ccbfad0 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -4120,6 +4120,7 @@ L: keyrings@vger.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 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/Makefile b/certs/Makefile index f4c25b67aad9..eb45407ff282 100644 --- a/certs/Makefile +++ b/certs/Makefile @@ -6,7 +6,20 @@ obj-$(CONFIG_SYSTEM_TRUSTED_KEYRING) += system_keyring.o system_certificates.o obj-$(CONFIG_SYSTEM_BLACKLIST_KEYRING) += blacklist.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 + +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 @@ -29,7 +42,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 +clean-files := x509_certificate_list .x509.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@linux.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; + } +}