Merge patch series "binman: properly error out if path provided to key-name-hint in signature nodes"

Quentin Schulz <foss+uboot@0leil.net> says:

I misunderstood the documentation and put the signing key in a keys/
directory while setting key-name-hint property in the signature node and
u-boot-spl-pubkey-dtb to a path.

mkimage doesn't fail if it cannot find the public key when signing a
FIT but returns something on stderr to notify the user it couldn't find
the key. The issue is that bintool currently discards stderr if the
command successfully returns, so the FIT is not signed AND the user
isn't made aware of it unless the image is manually inspected.

mkimage does fail when trying to insert a public key in a DTB if it
isn't found but we can have a better error message.

Link: https://lore.kernel.org/r/20250418-binman-pubkey-dir-v2-0-b6b90a765ffe@cherry.de

5 files changed, 144 insertions, 0 deletions

diff --git a/tools/binman/etype/fit.py b/tools/binman/etype/fit.py
index ed3cac4ee7e..db40479d30e 100644
--- a/tools/binman/etype/fit.py
+++ b/tools/binman/etype/fit.py
@@ -557,12 +557,15 @@ 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:
                 if (subnode.name.startswith('signature') or
                     subnode.name.startswith('cipher')):
                     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/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 3d556535e07..8225216fbec 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -7310,6 +7310,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"""
@@ -7983,6 +7990,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";
+					};
+				};
+			};
+		};
+	};
+};
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";
+		};
+	};
+};