[PATCH 0/3] Implement signing FIT images during image build

[PATCH 0/3] Implement signing FIT images during image build

[al.kochet]

From: Alexander Kochetkov <al.kochet@gmail.com>

Hello!

I've done verified boot on Radxa Rock 3A. I've embedded public key in U-Boot
SPL and signed FIT image configuration. All the work was done during U-Boot
image build. For some use cases building and signing images in one go will be
much simple, than building unsigned images and signing later. For example
SPL-image for rk3568 called idbloader.img consist of TPL, U-boot SPL and
U-boot SPL DTB with public key. So in order to assemble signed idbloader.img
lately we have to keep all the intermediate files used during build.

To embed public key, I've replaced u-boot-spl node with blob-ext and generated
u-boot-spl-with-pubkey-dtb blob using u-boot-spl-pubkey-dtb entry.

To sign FIT image I've used newly implemented fit property 'fit,sign'.

I haven't sign FIT image nodes, because I had realized that signing
configuration is safe and sufficient for verified boot. But I doubt.
So I've left that signing scheme in the test.

What do you think, is using signed configuration and signed images at the same
time is much safer or doesn't provide any benefits?

Now I thinking about implementing configuration option, something like
FIT_SIGNATURE_KEYDIR. The value of the option will be passed to binman
using -I.

Alsi I want to embed another public key in the configuration DTB, so
it will be used to verify kernel FIT. But I couldn't figure out how to
do it using binman.

&binman {
    u-boot-spl-with-pubkey-dtb {
        filename = "u-boot-spl-with-pubkey-dtb.bin";

        u-boot-spl-nodtb {
        };

        u-boot-spl-pubkey-dtb {
            algo = "sha256,rsa2048";
            required = "conf";
            key-name-hint = "uboot-spl";
        };
    };

    simple-bin {
        ...
        mkimage {
            ...

#ifdef CONFIG_ROCKCHIP_EXTERNAL_TPL
            rockchip-tpl {
            };
#elif defined(CONFIG_TPL)
            u-boot-tpl {
            };
#endif
            blob-ext {
                filename = "u-boot-spl-with-pubkey-dtb.bin";
            };
        };

        fit: fit {
            ...
            fit,sign;
            ...

            configurations {
                default = "@config-DEFAULT-SEQ";
                @config-SEQ {
                    ...
#ifdef CONFIG_SPL_FIT_SIGNATURE
                    signature {
                        algo = "sha256,rsa2048";
                        key-name-hint = "uboot-spl";
                        sign-images = "firmware", "loadables", "fdt";
                    };
#endif
                };
            };
        };
    };
}


Alexander Kochetkov (3):
  binman: fix passing loadables to mkimage on first run
  image-host: fix 'unknown error' error message
  binman: implement signing FIT images during image build

 tools/binman/btool/mkimage.py           |  5 +-
 tools/binman/entries.rst                |  7 ++
 tools/binman/etype/fit.py               | 57 +++++++++++++-
 tools/binman/ftest.py                   | 95 ++++++++++++++++++++++++
 tools/binman/test/326_fit_signature.dts | 98 +++++++++++++++++++++++++
 tools/binman/test/326_rsa2048.key       | 28 +++++++
 tools/binman/test/327_fit_signature.dts | 98 +++++++++++++++++++++++++
 tools/binman/test/328_fit_signature.dts | 61 +++++++++++++++
 tools/image-host.c                      |  2 +-
 9 files changed, 446 insertions(+), 5 deletions(-)
 create mode 100644 tools/binman/test/326_fit_signature.dts
 create mode 100644 tools/binman/test/326_rsa2048.key
 create mode 100644 tools/binman/test/327_fit_signature.dts
 create mode 100644 tools/binman/test/328_fit_signature.dts

-- 
2.17.1

[PATCH 1/3] binman: fix passing loadables to mkimage on first run

[al.kochet]

From: Alexander Kochetkov <al.kochet@gmail.com>

FIT use mkimage from BuildSectionData() to build FIT entry contents.
BuildSectionData() get called several times during building FIT image.

Currently when fit inserts loadables, it use self._loadables property that
contain loadables computed during previuos BuildSectionData() invocation.
So for the first run it use empty list and pass no loadables to mkimage.

That makes problem for adding signature to FIT image because mkimage
fails to add signature and aborts building FIT if no loadables provided.

The patch fixes described behaviour in a way that BuildSectionData() uses
recently calculated loadables value, not previosly calculated.

Signed-off-by: Alexander Kochetkov <al.kochet@gmail.com>
---
 tools/binman/etype/fit.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/binman/etype/fit.py b/tools/binman/etype/fit.py
index 2c14b15b03..e96222f4b6 100644
--- a/tools/binman/etype/fit.py
+++ b/tools/binman/etype/fit.py
@@ -542,8 +542,8 @@ class Entry_fit(Entry_section):
             """
             val = fdt_util.GetStringList(node, 'fit,firmware')
             if val is None:
-                return None, self._loadables
-            valid_entries = list(self._loadables)
+                return None, loadables
+            valid_entries = list(loadables)
             for name, entry in self.GetEntries().items():
                 missing = []
                 entry.CheckMissing(missing)
@@ -558,7 +558,7 @@ class Entry_fit(Entry_section):
                         firmware = name
                     elif name not in result:
                         result.append(name)
-            for name in self._loadables:
+            for name in loadables:
                 if name != firmware and name not in result:
                     result.append(name)
             return firmware, result
-- 
2.17.1

[PATCH 2/3] image-host: fix 'unknown error' error message

[al.kochet]

From: Alexander Kochetkov <al.kochet@gmail.com>

Fix error message like this:
Can't add verification data for node 'fdt-1' (<unknown error>)

We get unknown error because we decode error as fdt error
but actually it is system error.

Signed-off-by: Alexander Kochetkov <al.kochet@gmail.com>
---
 tools/image-host.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/image-host.c b/tools/image-host.c
index 7bfc0cb6b1..ac14d9aa86 100644
--- a/tools/image-host.c
+++ b/tools/image-host.c
@@ -1333,7 +1333,7 @@ int fit_add_verification_data(const char *keydir, const char *keyfile,
 		if (ret) {
 			fprintf(stderr, "Can't add verification data for node '%s' (%s)\n",
 				fdt_get_name(fit, noffset, NULL),
-				fdt_strerror(ret));
+				strerror(-ret));
 			return ret;
 		}
 	}
-- 
2.17.1

[PATCH 3/3] binman: implement signing FIT images during image build

[al.kochet]

From: Alexander Kochetkov <al.kochet@gmail.com>

The patch implement new property 'fit,sign' that can be declared
at the top-level 'fit' node. If that option is declared, fit tryies
to detect private keys directory among binman include directories.
That directory than passed to mkimage using '-k' flag and that enable
signing of FIT.

Signed-off-by: Alexander Kochetkov <al.kochet@gmail.com>
---
 tools/binman/btool/mkimage.py           |  5 +-
 tools/binman/entries.rst                |  7 ++
 tools/binman/etype/fit.py               | 51 +++++++++++++
 tools/binman/ftest.py                   | 95 ++++++++++++++++++++++++
 tools/binman/test/326_fit_signature.dts | 98 +++++++++++++++++++++++++
 tools/binman/test/326_rsa2048.key       | 28 +++++++
 tools/binman/test/327_fit_signature.dts | 98 +++++++++++++++++++++++++
 tools/binman/test/328_fit_signature.dts | 61 +++++++++++++++
 8 files changed, 442 insertions(+), 1 deletion(-)
 create mode 100644 tools/binman/test/326_fit_signature.dts
 create mode 100644 tools/binman/test/326_rsa2048.key
 create mode 100644 tools/binman/test/327_fit_signature.dts
 create mode 100644 tools/binman/test/328_fit_signature.dts

diff --git a/tools/binman/btool/mkimage.py b/tools/binman/btool/mkimage.py
index 39a4c8c143..78d3301bc1 100644
--- a/tools/binman/btool/mkimage.py
+++ b/tools/binman/btool/mkimage.py
@@ -22,7 +22,7 @@ class Bintoolmkimage(bintool.Bintool):
 
     # pylint: disable=R0913
     def run(self, reset_timestamp=False, output_fname=None, external=False,
-            pad=None, align=None):
+            pad=None, align=None, priv_keys_dir=None):
         """Run mkimage
 
         Args:
@@ -34,6 +34,7 @@ class Bintoolmkimage(bintool.Bintool):
                 other things to be easily added later, if required, such as
                 signatures
             align: Bytes to use for alignment of the FIT and its external data
+            priv_keys_dir: Path to directory containing private keys
             version: True to get the mkimage version
         """
         args = []
@@ -45,6 +46,8 @@ class Bintoolmkimage(bintool.Bintool):
             args += ['-B', f'{align:x}']
         if reset_timestamp:
             args.append('-t')
+        if priv_keys_dir:
+            args += ['-k', f'{priv_keys_dir}']
         if output_fname:
             args += ['-F', output_fname]
         return self.run_cmd(*args)
diff --git a/tools/binman/entries.rst b/tools/binman/entries.rst
index 254afe7607..9151332c1e 100644
--- a/tools/binman/entries.rst
+++ b/tools/binman/entries.rst
@@ -815,6 +815,13 @@ The top-level 'fit' node supports the following special properties:
 
             fit,fdt-list-val = "dtb1", "dtb2";
 
+    fit,sign
+        Enable signing FIT images via mkimage as described in
+        verified-boot.rst. If the property is found, the private keys path is
+        detected among binman include directories and passed to mkimage via
+        -k flag. All the keys required for signing FIT must be available at
+        time of signing and must be located in single include directory.
+
 Substitutions
 ~~~~~~~~~~~~~
 
diff --git a/tools/binman/etype/fit.py b/tools/binman/etype/fit.py
index e96222f4b6..30d8532626 100644
--- a/tools/binman/etype/fit.py
+++ b/tools/binman/etype/fit.py
@@ -6,6 +6,7 @@
 """Entry-type module for producing a FIT"""
 
 import libfdt
+import os
 
 from binman.entry import Entry, EntryArg
 from binman.etype.section import Entry_section
@@ -87,6 +88,14 @@ class Entry_fit(Entry_section):
 
                 fit,fdt-list-val = "dtb1", "dtb2";
 
+        fit,sign
+            Enable signing FIT images via mkimage as described in
+            verified-boot.rst. If the property is found, the private keys path
+            is detected among binman include directories and passed to mkimage
+            via  -k flag. All the keys required for signing FIT must be
+            available at time of signing and must be located in single include
+            directory.
+
     Substitutions
     ~~~~~~~~~~~~~
 
@@ -355,6 +364,7 @@ class Entry_fit(Entry_section):
         self.mkimage = None
         self._priv_entries = {}
         self._loadables = []
+        self._fit_sign = None
 
     def ReadNode(self):
         super().ReadNode()
@@ -430,6 +440,45 @@ class Entry_fit(Entry_section):
         # are removed from self._entries later.
         self._priv_entries = dict(self._entries)
 
+    def _get_priv_keys_dir(self, data):
+        """Detect private keys path among binman include directories
+
+        Args:
+            data: FIT image in binary format
+
+        Returns:
+            str: Single path containing all private keys found or None
+
+        Raises:
+            ValueError: Filename 'rsa2048.key' not found in input path
+            ValueError: Multiple key paths found
+        """
+        def _find_keys_dir(node):
+            for subnode in node.subnodes:
+                if subnode.name.startswith('signature'):
+                    if subnode.props.get('key-name-hint') is None:
+                        continue
+                    hint = subnode.props['key-name-hint'].value
+                    name = tools.get_input_filename(f"{hint}.key")
+                    path = os.path.dirname(name)
+                    if path not in paths:
+                        paths.append(path)
+                else:
+                    _find_keys_dir(subnode)
+            return None
+
+        fdt = Fdt.FromData(data)
+        fdt.Scan()
+
+        paths = []
+
+        _find_keys_dir(fdt.GetRoot())
+
+        if len(paths) > 1:
+            self.Raise("multiple key paths found (%s)" % ",".join(paths))
+
+        return paths[0] if len(paths) else None
+
     def BuildSectionData(self, required):
         """Build FIT entry contents
 
@@ -460,6 +509,8 @@ class Entry_fit(Entry_section):
         align = self._fit_props.get('fit,align')
         if align is not None:
             args.update({'align': fdt_util.fdt32_to_cpu(align.value)})
+        if self._fit_props.get('fit,sign') is not None:
+            args.update({'priv_keys_dir': self._get_priv_keys_dir(data)})
         if self.mkimage.run(reset_timestamp=True, output_fname=output_fname,
                             **args) is None:
             if not self.GetAllowMissing():
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index 8a44bc051b..22c3da5962 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -6836,6 +6836,101 @@ fdt         fdtmap                Extract the devicetree blob from the fdtmap
                                 ['fit'])
         self.assertIn("Node '/fit': Missing tool: 'mkimage'", str(e.exception))
 
+    def testFitSignSimple(self):
+        """Test that image with FIT and signature nodes can be signed"""
+        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',
+        }
+        data = tools.read_file(self.TestFile("326_rsa2048.key"))
+        self._MakeInputFile("keys/rsa2048.key", data)
+
+        test_subdir = os.path.join(self._indir, TEST_FDT_SUBDIR)
+        keys_subdir = os.path.join(self._indir, "keys")
+        data = self._DoReadFileDtb(
+            '326_fit_signature.dts',
+            entry_args=entry_args,
+            extra_indirs=[test_subdir, keys_subdir])[0]
+
+        dtb = fdt.Fdt.FromData(data)
+        dtb.Scan()
+
+        conf = dtb.GetNode('/configurations/conf-uboot-1')
+        self.assertIsNotNone(conf)
+        signature = conf.FindNode('signature')
+        self.assertIsNotNone(signature)
+        self.assertIsNotNone(signature.props.get('value'))
+
+        images = dtb.GetNode('/images')
+        self.assertIsNotNone(images)
+        for subnode in images.subnodes:
+            signature = subnode.FindNode('signature')
+            self.assertIsNotNone(signature)
+            self.assertIsNotNone(signature.props.get('value'))
+
+    def testFitSignKeyNotFound(self):
+        """Test that missing keys raise an error"""
+        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(
+                '326_fit_signature.dts',
+                entry_args=entry_args,
+                extra_indirs=[test_subdir])[0]
+        self.assertIn(
+            'Filename \'rsa2048.key\' not found in input path',
+            str(e.exception))
+
+    def testFitSignMultipleKeyPaths(self):
+        """Test that keys found in multiple paths raise an error"""
+        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',
+        }
+        data = tools.read_file(self.TestFile("326_rsa2048.key"))
+        self._MakeInputFile("keys1/rsa2048.key", data)
+        data = tools.read_file(self.TestFile("326_rsa2048.key"))
+        self._MakeInputFile("keys2/conf-rsa2048.key", data)
+
+        test_subdir = os.path.join(self._indir, TEST_FDT_SUBDIR)
+        keys_subdir1 = os.path.join(self._indir, "keys1")
+        keys_subdir2 = os.path.join(self._indir, "keys2")
+        with self.assertRaises(ValueError) as e:
+            self._DoReadFileDtb(
+                '327_fit_signature.dts',
+                entry_args=entry_args,
+                extra_indirs=[test_subdir, keys_subdir1, keys_subdir2])[0]
+        self.assertIn(
+            'Node \'/binman/fit\': multiple key paths found',
+            str(e.exception))
+
+    def testFitSignNoSingatureNodes(self):
+        """Test that fit,sign doens't raise error if no signature nodes found"""
+        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)
+        self._DoReadFileDtb(
+            '328_fit_signature.dts',
+            entry_args=entry_args,
+            extra_indirs=[test_subdir])[0]
+
     def testSymbolNoWrite(self):
         """Test disabling of symbol writing"""
         self._SetupSplElf()
diff --git a/tools/binman/test/326_fit_signature.dts b/tools/binman/test/326_fit_signature.dts
new file mode 100644
index 0000000000..9dce62e52d
--- /dev/null
+++ b/tools/binman/test/326_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 = "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 = "rsa2048";
+					};
+				};
+				@fdt-SEQ {
+					description = "test fdt";
+					type = "flat_dt";
+					compression = "none";
+
+					hash {
+						algo = "sha256";
+					};
+
+					signature {
+						algo = "sha256,rsa2048";
+						key-name-hint = "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 = "rsa2048";
+						sign-images = "firmware", "loadables", "fdt";
+					};
+				};
+			};
+		};
+	};
+};
diff --git a/tools/binman/test/326_rsa2048.key b/tools/binman/test/326_rsa2048.key
new file mode 100644
index 0000000000..e74b20cf39
--- /dev/null
+++ b/tools/binman/test/326_rsa2048.key
@@ -0,0 +1,28 @@
+-----BEGIN PRIVATE KEY-----
+MIIEvwIBADANBgkqhkiG9w0BAQEFAASCBKkwggSlAgEAAoIBAQDVUiT2JAF8Ajcx
+3XTB5qdGxuPMVFcXKJH+4L66oSt4YUBGi1bClo80U2azu08BTzk2Jzv6hez/mvzL
+hBvL3WnPwMl5vdOxb1kvUQyKLSw2bkM8VB0X1jGsKsKjzArg/aI8RknfiaSc5jua
+2lqwUFwv2RMF8jvIMN/1GnTLdECeMFVgVFSFkzIocISAHGPoGUOxTf8xK7o0x4RX
+NzB+95RtIqTQ5Az/KPVCOcQR5ETrUBXHF1I0rYjJjHHO4dUxxfDqFabt60EzQ/R2
+oZu58C4y0TrRI98g4hVPBYapildWjaNQm1Exa4ZaSDVl01OXsFW9Dm80PqfW4tTH
+Cm4nuCq5AgMBAAECggEBAIoG5b2SHJfFwzrzpQmVmeTU6i6a3+MvMBAwEZkmkb8J
+hhJfNFsiGjTsRgbDiuI5BbbBejCmmWvmN+3jZCzr7fwsLPEl36TufFF+atO5WOM7
+Qyv07QIwaOGSpXBgpSVhV6kSfdgy8p1G54hSAt4UkSGwnnt5ei8VWMP6Q1oltW3k
+f9DQ/ar4UEVa4jlJU3xqchcUTiKBKSH6pMC/Fqlq8x5JTLmk1Yb6C2UNcgJYez1u
+sHkdCA0FG3rFPrpFoQ1LUjMj1uEYNAxM3jOxE7Uvmk4yo9WpQDY7cRb2+Th9YY8a
+IKQ2s81Yg2TmkGzr8f5nrZz3WbAmQhQgsKbwlo6snjUCgYEA7kBOt0JlU7bJTfOr
+9s51g2VUfIH9lDS2Eh8MY+Bt6Y0Kdw/UK4HR8ZlN/nn0bHuHkc12K8lXEsQpgIEW
+DaqHytZJHqFs2egzKu/IvQYZ2WXEMj47LZQxEDHO9gtjE+5qCW9yJGqxW9BJKPVD
+F4spus4NqC+yD5OHM+6ESUtL/wMCgYEA5TZj6OHmECeh3efrwHqjDcjrqQbOTozU
+KPCNCY3Pv4Cg4xas/L93TE2CY6HJTr6mwEMUM+M4Ujjj15VCmSDQ/YYcGau1jo+f
+XdphOEENrPwoe9ATWIyBpT/wDrEz3L6JbE9dWMYY8vKYESt3qhVqDlbpmnYl8Jm+
+O3r5Cy2NlJMCgYEAyqzsCZuy5QcesnByvm8dqpxdxdkzJYu9wyakfKZj+gUgfO57
+OFOkjFk07yFB27MuPctCFredmfpDr+ygHRoPkG7AHw2Fss2EEaeP5bU18ilPQMqN
+vxVMs5EblVVUgJUVoVcsC2yz2f4S7oPOAk5BPoehOIzydauznWrvIAas7I8CgYBr
+CFHxLoNq6cbZQ3JACERZrIf2/vmZjoOHtoR1gKYRK7R1NmKDB7lihRMtCSBix/4/
+61Lkw+bJ5kzmn4lgzgUpTdWTWy5FquVlQxOA3EfRjlItNsXB5KKpksi7Y53vJ34u
+eIUDbkW6NPQzmFOhtaw3k/gzq5Yd2v0M82iWAqiJRwKBgQCl2+e2cjISK31QhKTC
+puhwQ0/YuC3zlwMXQgB3nPw8b9RlaDTMrRBCIUFIrrX11tHswGWpyVsxW2AvZ3Zm
+jsWpwGkUdpRdXJBhSaisV/PA+x3kYhpibzEI8FrzhU69zNROCb8CTkN4WcdBdq6J
+PUh/jRtKoE79qrlnIlNvFoz2gQ==
+-----END PRIVATE KEY-----
diff --git a/tools/binman/test/327_fit_signature.dts b/tools/binman/test/327_fit_signature.dts
new file mode 100644
index 0000000000..77bec8df1e
--- /dev/null
+++ b/tools/binman/test/327_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 = "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 = "rsa2048";
+					};
+				};
+				@fdt-SEQ {
+					description = "test fdt";
+					type = "flat_dt";
+					compression = "none";
+
+					hash {
+						algo = "sha256";
+					};
+
+					signature {
+						algo = "sha256,rsa2048";
+						key-name-hint = "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 = "conf-rsa2048";
+						sign-images = "firmware", "loadables", "fdt";
+					};
+				};
+			};
+		};
+	};
+};
diff --git a/tools/binman/test/328_fit_signature.dts b/tools/binman/test/328_fit_signature.dts
new file mode 100644
index 0000000000..267105d0f6
--- /dev/null
+++ b/tools/binman/test/328_fit_signature.dts
@@ -0,0 +1,61 @@
+// 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 {
+					};
+				};
+				@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 {
+					};
+				};
+				@fdt-SEQ {
+					description = "test fdt";
+					type = "flat_dt";
+					compression = "none";
+				};
+			};
+
+			configurations {
+				default = "@conf-uboot-DEFAULT-SEQ";
+				@conf-uboot-SEQ {
+					description = "uboot config";
+					fdt = "fdt-SEQ";
+					fit,firmware = "u-boot";
+					fit,loadables;
+				};
+			};
+		};
+	};
+};
-- 
2.17.1

Re: [PATCH 0/3] Implement signing FIT images during image build

[Simon Glass]

Hi,

On Mon, 16 Sept 2024 at 02:24, <al.kochet@gmail.com> wrote:
>
> From: Alexander Kochetkov <al.kochet@gmail.com>
>
> Hello!
>
> I've done verified boot on Radxa Rock 3A. I've embedded public key in U-Boot
> SPL and signed FIT image configuration. All the work was done during U-Boot
> image build. For some use cases building and signing images in one go will be
> much simple, than building unsigned images and signing later. For example
> SPL-image for rk3568 called idbloader.img consist of TPL, U-boot SPL and
> U-boot SPL DTB with public key. So in order to assemble signed idbloader.img
> lately we have to keep all the intermediate files used during build.
>
> To embed public key, I've replaced u-boot-spl node with blob-ext and generated
> u-boot-spl-with-pubkey-dtb blob using u-boot-spl-pubkey-dtb entry.
>
> To sign FIT image I've used newly implemented fit property 'fit,sign'.
>
> I haven't sign FIT image nodes, because I had realized that signing
> configuration is safe and sufficient for verified boot. But I doubt.
> So I've left that signing scheme in the test.
>
> What do you think, is using signed configuration and signed images at the same
> time is much safer or doesn't provide any benefits?

So long as you have hashes on the images, then yes it is sufficient.

>
> Now I thinking about implementing configuration option, something like
> FIT_SIGNATURE_KEYDIR. The value of the option will be passed to binman
> using -I.

I suppose you saw entryargs - could that help?

>
> Alsi I want to embed another public key in the configuration DTB, so
> it will be used to verify kernel FIT. But I couldn't figure out how to
> do it using binman.

I'm not sure what you are trying to do here. The kernel image (or
really, its hash) would normally be verified using the same signature
as for everything else in the configuration.

>
> &binman {
>     u-boot-spl-with-pubkey-dtb {
>         filename = "u-boot-spl-with-pubkey-dtb.bin";
>
>         u-boot-spl-nodtb {
>         };
>
>         u-boot-spl-pubkey-dtb {
>             algo = "sha256,rsa2048";
>             required = "conf";
>             key-name-hint = "uboot-spl";
>         };
>     };
>
>     simple-bin {
>         ...
>         mkimage {
>             ...
>
> #ifdef CONFIG_ROCKCHIP_EXTERNAL_TPL
>             rockchip-tpl {
>             };
> #elif defined(CONFIG_TPL)
>             u-boot-tpl {
>             };
> #endif
>             blob-ext {
>                 filename = "u-boot-spl-with-pubkey-dtb.bin";
>             };
>         };
>
>         fit: fit {
>             ...
>             fit,sign;
>             ...
>
>             configurations {
>                 default = "@config-DEFAULT-SEQ";
>                 @config-SEQ {
>                     ...
> #ifdef CONFIG_SPL_FIT_SIGNATURE
>                     signature {
>                         algo = "sha256,rsa2048";
>                         key-name-hint = "uboot-spl";
>                         sign-images = "firmware", "loadables", "fdt";
>                     };
> #endif
>                 };
>             };
>         };
>     };
> }
>
>
> Alexander Kochetkov (3):
>   binman: fix passing loadables to mkimage on first run
>   image-host: fix 'unknown error' error message
>   binman: implement signing FIT images during image build
>
>  tools/binman/btool/mkimage.py           |  5 +-
>  tools/binman/entries.rst                |  7 ++
>  tools/binman/etype/fit.py               | 57 +++++++++++++-
>  tools/binman/ftest.py                   | 95 ++++++++++++++++++++++++
>  tools/binman/test/326_fit_signature.dts | 98 +++++++++++++++++++++++++
>  tools/binman/test/326_rsa2048.key       | 28 +++++++
>  tools/binman/test/327_fit_signature.dts | 98 +++++++++++++++++++++++++
>  tools/binman/test/328_fit_signature.dts | 61 +++++++++++++++
>  tools/image-host.c                      |  2 +-
>  9 files changed, 446 insertions(+), 5 deletions(-)
>  create mode 100644 tools/binman/test/326_fit_signature.dts
>  create mode 100644 tools/binman/test/326_rsa2048.key
>  create mode 100644 tools/binman/test/327_fit_signature.dts
>  create mode 100644 tools/binman/test/328_fit_signature.dts
>
> --
> 2.17.1
>

Regards,
Simon

Re: [PATCH 3/3] binman: implement signing FIT images during image build

[Simon Glass]

On Mon, 16 Sept 2024 at 02:25, <al.kochet@gmail.com> wrote:
>
> From: Alexander Kochetkov <al.kochet@gmail.com>
>
> The patch implement new property 'fit,sign' that can be declared
> at the top-level 'fit' node. If that option is declared, fit tryies
> to detect private keys directory among binman include directories.
> That directory than passed to mkimage using '-k' flag and that enable
> signing of FIT.
>
> Signed-off-by: Alexander Kochetkov <al.kochet@gmail.com>
> ---
>  tools/binman/btool/mkimage.py           |  5 +-
>  tools/binman/entries.rst                |  7 ++
>  tools/binman/etype/fit.py               | 51 +++++++++++++
>  tools/binman/ftest.py                   | 95 ++++++++++++++++++++++++
>  tools/binman/test/326_fit_signature.dts | 98 +++++++++++++++++++++++++
>  tools/binman/test/326_rsa2048.key       | 28 +++++++
>  tools/binman/test/327_fit_signature.dts | 98 +++++++++++++++++++++++++
>  tools/binman/test/328_fit_signature.dts | 61 +++++++++++++++
>  8 files changed, 442 insertions(+), 1 deletion(-)
>  create mode 100644 tools/binman/test/326_fit_signature.dts
>  create mode 100644 tools/binman/test/326_rsa2048.key
>  create mode 100644 tools/binman/test/327_fit_signature.dts
>  create mode 100644 tools/binman/test/328_fit_signature.dts
>

Reviewed-by: Simon Glass <sjg@chromium.org>

Nice

Re: [PATCH 2/3] image-host: fix 'unknown error' error message

[Simon Glass]

On Mon, 16 Sept 2024 at 02:25, <al.kochet@gmail.com> wrote:
>
> From: Alexander Kochetkov <al.kochet@gmail.com>
>
> Fix error message like this:
> Can't add verification data for node 'fdt-1' (<unknown error>)
>
> We get unknown error because we decode error as fdt error
> but actually it is system error.
>
> Signed-off-by: Alexander Kochetkov <al.kochet@gmail.com>
> ---
>  tools/image-host.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Simon Glass <sjg@chromium.org>


>
> diff --git a/tools/image-host.c b/tools/image-host.c
> index 7bfc0cb6b1..ac14d9aa86 100644
> --- a/tools/image-host.c
> +++ b/tools/image-host.c
> @@ -1333,7 +1333,7 @@ int fit_add_verification_data(const char *keydir, const char *keyfile,
>                 if (ret) {
>                         fprintf(stderr, "Can't add verification data for node '%s' (%s)\n",
>                                 fdt_get_name(fit, noffset, NULL),
> -                               fdt_strerror(ret));
> +                               strerror(-ret));
>                         return ret;
>                 }
>         }
> --
> 2.17.1
>

Re: [PATCH 1/3] binman: fix passing loadables to mkimage on first run

[Simon Glass]

On Mon, 16 Sept 2024 at 02:25, <al.kochet@gmail.com> wrote:
>
> From: Alexander Kochetkov <al.kochet@gmail.com>
>
> FIT use mkimage from BuildSectionData() to build FIT entry contents.
> BuildSectionData() get called several times during building FIT image.
>
> Currently when fit inserts loadables, it use self._loadables property that
> contain loadables computed during previuos BuildSectionData() invocation.
> So for the first run it use empty list and pass no loadables to mkimage.
>
> That makes problem for adding signature to FIT image because mkimage
> fails to add signature and aborts building FIT if no loadables provided.
>
> The patch fixes described behaviour in a way that BuildSectionData() uses
> recently calculated loadables value, not previosly calculated.
>
> Signed-off-by: Alexander Kochetkov <al.kochet@gmail.com>
> ---
>  tools/binman/etype/fit.py | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>


>
> diff --git a/tools/binman/etype/fit.py b/tools/binman/etype/fit.py
> index 2c14b15b03..e96222f4b6 100644
> --- a/tools/binman/etype/fit.py
> +++ b/tools/binman/etype/fit.py
> @@ -542,8 +542,8 @@ class Entry_fit(Entry_section):
>              """
>              val = fdt_util.GetStringList(node, 'fit,firmware')
>              if val is None:
> -                return None, self._loadables
> -            valid_entries = list(self._loadables)
> +                return None, loadables
> +            valid_entries = list(loadables)
>              for name, entry in self.GetEntries().items():
>                  missing = []
>                  entry.CheckMissing(missing)
> @@ -558,7 +558,7 @@ class Entry_fit(Entry_section):
>                          firmware = name
>                      elif name not in result:
>                          result.append(name)
> -            for name in self._loadables:
> +            for name in loadables:
>                  if name != firmware and name not in result:
>                      result.append(name)
>              return firmware, result
> --
> 2.17.1
>

Re: [PATCH 3/3] binman: implement signing FIT images during image build

[Simon Glass]

On Mon, 16 Sept 2024 at 02:25, <al.kochet@gmail.com> wrote:
>
> From: Alexander Kochetkov <al.kochet@gmail.com>
>
> The patch implement new property 'fit,sign' that can be declared
> at the top-level 'fit' node. If that option is declared, fit tryies
> to detect private keys directory among binman include directories.
> That directory than passed to mkimage using '-k' flag and that enable
> signing of FIT.
>
> Signed-off-by: Alexander Kochetkov <al.kochet@gmail.com>
> ---
>  tools/binman/btool/mkimage.py           |  5 +-
>  tools/binman/entries.rst                |  7 ++
>  tools/binman/etype/fit.py               | 51 +++++++++++++
>  tools/binman/ftest.py                   | 95 ++++++++++++++++++++++++
>  tools/binman/test/326_fit_signature.dts | 98 +++++++++++++++++++++++++
>  tools/binman/test/326_rsa2048.key       | 28 +++++++
>  tools/binman/test/327_fit_signature.dts | 98 +++++++++++++++++++++++++
>  tools/binman/test/328_fit_signature.dts | 61 +++++++++++++++
>  8 files changed, 442 insertions(+), 1 deletion(-)
>  create mode 100644 tools/binman/test/326_fit_signature.dts
>  create mode 100644 tools/binman/test/326_rsa2048.key
>  create mode 100644 tools/binman/test/327_fit_signature.dts
>  create mode 100644 tools/binman/test/328_fit_signature.dts
>

Reviewed-by: Simon Glass <sjg@chromium.org>

Nice

Applied to u-boot-dm, thanks!

Re: [PATCH 2/3] image-host: fix 'unknown error' error message

[Simon Glass]

On Mon, 16 Sept 2024 at 02:25, <al.kochet@gmail.com> wrote:
>
> From: Alexander Kochetkov <al.kochet@gmail.com>
>
> Fix error message like this:
> Can't add verification data for node 'fdt-1' (<unknown error>)
>
> We get unknown error because we decode error as fdt error
> but actually it is system error.
>
> Signed-off-by: Alexander Kochetkov <al.kochet@gmail.com>
> ---
>  tools/image-host.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Simon Glass <sjg@chromium.org>


>
Applied to u-boot-dm, thanks!

Re: [PATCH 1/3] binman: fix passing loadables to mkimage on first run

[Simon Glass]

On Mon, 16 Sept 2024 at 02:25, <al.kochet@gmail.com> wrote:
>
> From: Alexander Kochetkov <al.kochet@gmail.com>
>
> FIT use mkimage from BuildSectionData() to build FIT entry contents.
> BuildSectionData() get called several times during building FIT image.
>
> Currently when fit inserts loadables, it use self._loadables property that
> contain loadables computed during previuos BuildSectionData() invocation.
> So for the first run it use empty list and pass no loadables to mkimage.
>
> That makes problem for adding signature to FIT image because mkimage
> fails to add signature and aborts building FIT if no loadables provided.
>
> The patch fixes described behaviour in a way that BuildSectionData() uses
> recently calculated loadables value, not previosly calculated.
>
> Signed-off-by: Alexander Kochetkov <al.kochet@gmail.com>
> ---
>  tools/binman/etype/fit.py | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>


>
Applied to u-boot-dm, thanks!