From 2ddc47f9aa0581a907dd07cb4468ef6d4f3519dc Mon Sep 17 00:00:00 2001 From: Quentin Schulz Date: Fri, 18 Apr 2025 13:26:07 +0200 Subject: binman: etype: fit: raise ValueError if key-name-hint is a path mkimage doesn't fail if it cannot find the public key but it prints to stderr. Considering that btool.run() discards stderr, it means binman happily returns an unsigned FIT and doesn't tell you something went wrong. Binman will actually find the file if there's a path in the key-name-hint property but the current logic expects key-name-hint to be a filename and thus returns the dirname of the found path for the key, but with the original key-name-hint appended. This means we can have the following: - key-name-hint = "keys/dev" - name = "/home/qschulz/work/upstream/u-boot/keys/" so we pass /home/qschulz/work/upstream/u-boot/keys/ to the -k option of mkimage but the FIT still contains "keys/dev" in key-name-hint which means mkimage will try to find the key at /home/qschulz/work/upstream/u-boot/keys/keys/, which doesn't exist. Let's assume paths are simply not supported (it is named key-name-hint and not key-path-hint after all) and raise an error if the property contains a path so that the build fails and not quietly. Fixes: 133c000ca334 ("binman: implement signing FIT images during image build") Signed-off-by: Quentin Schulz Reviewed-by: Simon Glass --- tools/binman/etype/fit.py | 3 + tools/binman/ftest.py | 18 ++++ .../test/347_key_name_hint_dir_fit_signature.dts | 98 ++++++++++++++++++++++ 3 files changed, 119 insertions(+) create mode 100644 tools/binman/test/347_key_name_hint_dir_fit_signature.dts (limited to 'tools') diff --git a/tools/binman/etype/fit.py b/tools/binman/etype/fit.py index 803fb66ea83..284b19c4b88 100644 --- a/tools/binman/etype/fit.py +++ b/tools/binman/etype/fit.py @@ -557,6 +557,7 @@ class Entry_fit(Entry_section): Raises: ValueError: Filename 'rsa2048.key' not found in input path ValueError: Multiple key paths found + ValueError: 'dir/rsa2048' is a path not a filename """ def _find_keys_dir(node): for subnode in node.subnodes: @@ -565,6 +566,8 @@ class Entry_fit(Entry_section): if subnode.props.get('key-name-hint') is None: continue hint = subnode.props['key-name-hint'].value + if '/' in hint: + self.Raise(f"'{hint}' is a path not a filename") name = tools.get_input_filename( f"{hint}.key" if subnode.name.startswith('signature') else f"{hint}.bin") diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 948fcc02259..5ea15b36a71 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -7940,6 +7940,24 @@ fdt fdtmap Extract the devicetree blob from the fdtmap entry_args=entry_args, extra_indirs=[test_subdir])[0] + def testKeyNameHintIsPathSimpleFit(self): + """Test that binman errors out on key-name-hint being a path""" + if not elf.ELF_TOOLS: + self.skipTest('Python elftools not available') + entry_args = { + 'of-list': 'test-fdt1', + 'default-dt': 'test-fdt1', + 'atf-bl31-path': 'bl31.elf', + } + test_subdir = os.path.join(self._indir, TEST_FDT_SUBDIR) + with self.assertRaises(ValueError) as e: + self._DoReadFileDtb( + '347_key_name_hint_dir_fit_signature.dts', + entry_args=entry_args, + extra_indirs=[test_subdir]) + self.assertIn( + 'Node \'/binman/fit\': \'keys/rsa2048\' is a path not a filename', + str(e.exception)) def testSimpleFitEncryptedData(self): """Test an image with a FIT containing data to be encrypted""" diff --git a/tools/binman/test/347_key_name_hint_dir_fit_signature.dts b/tools/binman/test/347_key_name_hint_dir_fit_signature.dts new file mode 100644 index 00000000000..96e2126dadb --- /dev/null +++ b/tools/binman/test/347_key_name_hint_dir_fit_signature.dts @@ -0,0 +1,98 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + fit { + description = "test desc"; + #address-cells = <1>; + fit,fdt-list = "of-list"; + fit,sign; + + images { + u-boot { + description = "test u-boot"; + type = "standalone"; + arch = "arm64"; + os = "u-boot"; + compression = "none"; + load = <0x00000000>; + entry = <0x00000000>; + + u-boot-nodtb { + }; + + hash { + algo = "sha256"; + }; + + signature { + algo = "sha256,rsa2048"; + key-name-hint = "keys/rsa2048"; + }; + }; + @atf-SEQ { + fit,operation = "split-elf"; + description = "test tf-a"; + type = "firmware"; + arch = "arm64"; + os = "arm-trusted-firmware"; + compression = "none"; + fit,load; + fit,entry; + fit,data; + + atf-bl31 { + }; + + hash { + algo = "sha256"; + }; + + signature { + algo = "sha256,rsa2048"; + key-name-hint = "keys/rsa2048"; + }; + }; + @fdt-SEQ { + description = "test fdt"; + type = "flat_dt"; + compression = "none"; + + hash { + algo = "sha256"; + }; + + signature { + algo = "sha256,rsa2048"; + key-name-hint = "keys/rsa2048"; + }; + }; + }; + + configurations { + default = "@conf-uboot-DEFAULT-SEQ"; + @conf-uboot-SEQ { + description = "uboot config"; + fdt = "fdt-SEQ"; + fit,firmware = "u-boot"; + fit,loadables; + + hash { + algo = "sha256"; + }; + + signature { + algo = "sha256,rsa2048"; + key-name-hint = "keys/rsa2048"; + sign-images = "firmware", "loadables", "fdt"; + }; + }; + }; + }; + }; +}; -- cgit v1.3.1 From 4e7e0ebcf59794b97542bb6b3d1e0a0fefa145f2 Mon Sep 17 00:00:00 2001 From: Quentin Schulz Date: Fri, 18 Apr 2025 13:26:08 +0200 Subject: binman: etype: u_boot_spl_pubkey_dtb: provide more explicit error for key-name-hint with path key-name-hint property in u-boot-spl-pubkey-dtb binman entry may contain a path instead of a filename due to user mistake. Because we currently assume it is a filename instead of a path, binman will find the full path to the key based on that path, and return the dirname of the full path but keeps the path in key-name-hint instead of stripping the directories from it. This means mkimage will fail with the following error message if we have key-name-hint set to keys/dev: binman: Error 1 running 'fdt_add_pubkey -a sha256,rsa2048 -k /home/qschulz/work/upstream/u-boot/keys -n keys/dev -r conf /home/qschulz/work/upstream/u-boot/build/ringneck/u-boot-spl-dtbdhsfx3mf': Couldn't open RSA certificate: '/home/qschulz/work/upstream/u-boot/keys/keys/dev.crt': No such file or directory Let's make it a bit more obvious what the error is by erroring out in binman if a path is provided in key-name-hint (it is named key-name-hint and not key-path-hint after all). Fixes: 5609843b57a4 ("binman: etype: Add u-boot-spl-pubkey-dtb etype") Signed-off-by: Quentin Schulz Reviewed-by: Simon Glass --- tools/binman/etype/u_boot_spl_pubkey_dtb.py | 2 ++ tools/binman/ftest.py | 7 +++++++ .../binman/test/348_key_name_hint_dir_spl_pubkey_dtb.dts | 16 ++++++++++++++++ 3 files changed, 25 insertions(+) create mode 100644 tools/binman/test/348_key_name_hint_dir_spl_pubkey_dtb.dts (limited to 'tools') diff --git a/tools/binman/etype/u_boot_spl_pubkey_dtb.py b/tools/binman/etype/u_boot_spl_pubkey_dtb.py index cb196061de2..3061c4bcdc4 100644 --- a/tools/binman/etype/u_boot_spl_pubkey_dtb.py +++ b/tools/binman/etype/u_boot_spl_pubkey_dtb.py @@ -87,6 +87,8 @@ class Entry_u_boot_spl_pubkey_dtb(Entry_blob_dtb): dir=tools.get_output_dir())\ as pubkey_tdb: tools.write_file(pubkey_tdb.name, self.GetData()) + if '/' in self._key_name_hint: + self.Raise(f"'{self._key_name_hint}' is a path not a filename") keyname = tools.get_input_filename(self._key_name_hint + ".crt") self.fdt_add_pubkey.run(pubkey_tdb.name, os.path.dirname(keyname), diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 5ea15b36a71..1b68f4b51cf 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -7274,6 +7274,13 @@ fdt fdtmap Extract the devicetree blob from the fdtmap tools.to_bytes(''.join(node.props['key'].value))) self.assertNotIn('key-source', node.props) + def testKeyNameHintIsPathSplPubkeyDtb(self): + """Test that binman errors out on key-name-hint being a path""" + with self.assertRaises(ValueError) as e: + self._DoReadFile('348_key_name_hint_dir_spl_pubkey_dtb.dts') + self.assertIn( + 'Node \'/binman/u-boot-spl-pubkey-dtb\': \'keys/key\' is a path not a filename', + str(e.exception)) def testSplPubkeyDtb(self): """Test u_boot_spl_pubkey_dtb etype""" diff --git a/tools/binman/test/348_key_name_hint_dir_spl_pubkey_dtb.dts b/tools/binman/test/348_key_name_hint_dir_spl_pubkey_dtb.dts new file mode 100644 index 00000000000..85ebd58b6c0 --- /dev/null +++ b/tools/binman/test/348_key_name_hint_dir_spl_pubkey_dtb.dts @@ -0,0 +1,16 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + u-boot-spl-pubkey-dtb { + algo = "sha384,rsa4096"; + required = "conf"; + key-name-hint = "keys/key"; + }; + }; +}; -- cgit v1.3.1