All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 00/11] Sign Xilinx ZynqMP SPL/FSBL boot images using binman
@ 2023-07-06  8:38 lukas.funke-oss
  2023-07-06  8:38 ` [PATCH v2 01/11] binman: elf: Check for ELF_TOOLS availability and remove extra semicolon lukas.funke-oss
                   ` (11 more replies)
  0 siblings, 12 replies; 19+ messages in thread
From: lukas.funke-oss @ 2023-07-06  8:38 UTC (permalink / raw
  To: u-boot
  Cc: Simon Glass, Quentin Schulz, Lukas Funke, Alper Nebi Yasak,
	Michal Simek, Stefan Herbrechtsmeier

From: Lukas Funke <lukas.funke@weidmueller.com>


This series adds two etypes to create a verified boot chain for
Xilinx ZynqMP devices. The first etype 'xilinx_fsbl_auth' is used to
create a bootable, signed image for ZynqMP boards using the Xilinx
Bootgen tool. The second etype 'u_boot_spl_pubkey_dtb' is used to add
a '/signature' node to the SPL. The public key in the signature is read
from a certificate file and added using the 'fdt_add_pubkey' tool. The
series also contains the corresponding btool for calling 'bootgen' and
'fdt_add_pubkey'

The following block shows an example on how to use this functionality:

    spl {
        filename = "boot.signed.bin";

        xilinx_fsbl_auth {
            psk-filename = "psk0.pem";
            ssk-filename = "ssk0.pem";
            auth-params = "ppk_select=0", "spk_id=0x00000000";

            u_boot_spl_nodtb {
            };
            u_boot_spl_pubkey_dtb {
                algo = "sha384,rsa4096";
                required = "conf";
                key-name = "dev";
            };
        };
    };


Changes in v2:
- Changed u_boot_spl_pubkey_dtb to u-boot-spl-pubkey-dtb
- Improved rst/python documentation
- Changed u_boot_spl_pubkey_dtb to u-boot-spl-pubkey-dtb in example
- Pass additional 'keysrc_enc' parameter to Bootgen
- Added more information and terms to documentation
- Fixed typo in dts name
- Add 'keysrc-enc' property to pass down to Bootgen
- Improved documentation
- Use predictable output names for intermediated results

Lukas Funke (11):
  binman: elf: Check for ELF_TOOLS availability and remove extra
    semicolon
  binman: Don't decompress data while signing
  binman: blob_dtb: Add fake_size argument to ObtainContents()
  binman: doc: Add documentation for fdt_add_pubkey bintool
  binman: ftest: Add test for u_boot_spl_pubkey_dtb
  binman: btool: Add fdt_add_pubkey as btool
  binman: etype: Add u_boot_spl_pubkey_dtb etype
  binman: doc: Add documentation for Xilinx Bootgen bintool
  binman: btool: Add Xilinx Bootgen btool
  binman: ftest: Add test for xilinx_fsbl_auth etype
  binman: etype: Add xilinx_fsbl_auth etype

 tools/binman/bintools.rst                   |  22 ++
 tools/binman/btool/bootgen.py               | 136 +++++++++++++
 tools/binman/btool/fdt_add_pubkey.py        |  67 ++++++
 tools/binman/control.py                     |   2 +-
 tools/binman/elf.py                         |  10 +-
 tools/binman/entries.rst                    |  92 +++++++++
 tools/binman/etype/blob_dtb.py              |   2 +-
 tools/binman/etype/u_boot_spl_pubkey_dtb.py | 109 ++++++++++
 tools/binman/etype/xilinx_fsbl_auth.py      | 213 ++++++++++++++++++++
 tools/binman/ftest.py                       |  42 +++-
 tools/binman/test/280_xilinx_fsbl_auth.dts  |  23 +++
 tools/binman/test/281_spl_pubkey_dtb.dts    |  16 ++
 12 files changed, 727 insertions(+), 7 deletions(-)
 create mode 100644 tools/binman/btool/bootgen.py
 create mode 100644 tools/binman/btool/fdt_add_pubkey.py
 create mode 100644 tools/binman/etype/u_boot_spl_pubkey_dtb.py
 create mode 100644 tools/binman/etype/xilinx_fsbl_auth.py
 create mode 100644 tools/binman/test/280_xilinx_fsbl_auth.dts
 create mode 100644 tools/binman/test/281_spl_pubkey_dtb.dts

-- 
2.30.2


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v2 01/11] binman: elf: Check for ELF_TOOLS availability and remove extra semicolon
  2023-07-06  8:38 [PATCH v2 00/11] Sign Xilinx ZynqMP SPL/FSBL boot images using binman lukas.funke-oss
@ 2023-07-06  8:38 ` lukas.funke-oss
  2023-07-07 17:35   ` Simon Glass
  2023-07-06  8:38 ` [PATCH v2 02/11] binman: Don't decompress data while signing lukas.funke-oss
                   ` (10 subsequent siblings)
  11 siblings, 1 reply; 19+ messages in thread
From: lukas.funke-oss @ 2023-07-06  8:38 UTC (permalink / raw
  To: u-boot; +Cc: Simon Glass, Quentin Schulz, Lukas Funke, Alper Nebi Yasak

From: Lukas Funke <lukas.funke@weidmueller.com>

Check if elf tools are available when running DecodeElf(). Also
remove superfuous semicolon at line ending.

Signed-off-by: Lukas Funke <lukas.funke@weidmueller.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 tools/binman/elf.py | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/tools/binman/elf.py b/tools/binman/elf.py
index 5816284c32..a53f4b9c4f 100644
--- a/tools/binman/elf.py
+++ b/tools/binman/elf.py
@@ -438,13 +438,15 @@ def DecodeElf(data, location):
     Returns:
         ElfInfo object containing information about the decoded ELF file
     """
+    if not ELF_TOOLS:
+        raise ValueError("Python: No module named 'elftools'")
     file_size = len(data)
     with io.BytesIO(data) as fd:
         elf = ELFFile(fd)
-        data_start = 0xffffffff;
-        data_end = 0;
-        mem_end = 0;
-        virt_to_phys = 0;
+        data_start = 0xffffffff
+        data_end = 0
+        mem_end = 0
+        virt_to_phys = 0
 
         for i in range(elf.num_segments()):
             segment = elf.get_segment(i)
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 02/11] binman: Don't decompress data while signing
  2023-07-06  8:38 [PATCH v2 00/11] Sign Xilinx ZynqMP SPL/FSBL boot images using binman lukas.funke-oss
  2023-07-06  8:38 ` [PATCH v2 01/11] binman: elf: Check for ELF_TOOLS availability and remove extra semicolon lukas.funke-oss
@ 2023-07-06  8:38 ` lukas.funke-oss
  2023-07-06  8:38 ` [PATCH v2 03/11] binman: blob_dtb: Add fake_size argument to ObtainContents() lukas.funke-oss
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 19+ messages in thread
From: lukas.funke-oss @ 2023-07-06  8:38 UTC (permalink / raw
  To: u-boot; +Cc: Simon Glass, Quentin Schulz, Lukas Funke, Alper Nebi Yasak

From: Lukas Funke <lukas.funke@weidmueller.com>

While signing a fit compressed data (i.e. 'blob-ext') is decompressed,
but never compressed again. When compressed data was wrapped in a
section, decompression leads to an error because the outer section had
the original compressed size but the inner entry has the
uncompressed size now.

While singing there is no reason to decompress data. Thus, decompression
should be disabled.

Furthermore, bintools should be collected before loading the data. This
way bintools are available if processing is required on a node.

Signed-off-by: Lukas Funke <lukas.funke@weidmueller.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 tools/binman/control.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/binman/control.py b/tools/binman/control.py
index 68597c4e77..affc33ff3d 100644
--- a/tools/binman/control.py
+++ b/tools/binman/control.py
@@ -306,8 +306,8 @@ def BeforeReplace(image, allow_resize):
         image: Image to prepare
     """
     state.PrepareFromLoadedData(image)
-    image.LoadData()
     image.CollectBintools()
+    image.LoadData(decomp=False)
 
     # If repacking, drop the old offset/size values except for the original
     # ones, so we are only left with the constraints.
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 03/11] binman: blob_dtb: Add fake_size argument to ObtainContents()
  2023-07-06  8:38 [PATCH v2 00/11] Sign Xilinx ZynqMP SPL/FSBL boot images using binman lukas.funke-oss
  2023-07-06  8:38 ` [PATCH v2 01/11] binman: elf: Check for ELF_TOOLS availability and remove extra semicolon lukas.funke-oss
  2023-07-06  8:38 ` [PATCH v2 02/11] binman: Don't decompress data while signing lukas.funke-oss
@ 2023-07-06  8:38 ` lukas.funke-oss
  2023-07-06  8:38 ` [PATCH v2 04/11] binman: doc: Add documentation for fdt_add_pubkey bintool lukas.funke-oss
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 19+ messages in thread
From: lukas.funke-oss @ 2023-07-06  8:38 UTC (permalink / raw
  To: u-boot
  Cc: Simon Glass, Quentin Schulz, Lukas Funke, Alper Nebi Yasak,
	Stefan Herbrechtsmeier

From: Lukas Funke <lukas.funke@weidmueller.com>

The method 'connect_contents_to_file()' calls ObtainsContents() with
'fake_size' argument. Without providing the argument in the blob_dtb
we are not able to call this method without error.

Signed-off-by: Lukas Funke <lukas.funke@weidmueller.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 tools/binman/etype/blob_dtb.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/binman/etype/blob_dtb.py b/tools/binman/etype/blob_dtb.py
index 6a3fbc4775..d543de9f75 100644
--- a/tools/binman/etype/blob_dtb.py
+++ b/tools/binman/etype/blob_dtb.py
@@ -38,7 +38,7 @@ class Entry_blob_dtb(Entry_blob):
             self.Raise("Invalid prepend in '%s': '%s'" %
                        (self._node.name, self.prepend))
 
-    def ObtainContents(self):
+    def ObtainContents(self, fake_size=0):
         """Get the device-tree from the list held by the 'state' module"""
         self._filename = self.GetDefaultFilename()
         self._pathname, _ = state.GetFdtContents(self.GetFdtEtype())
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 04/11] binman: doc: Add documentation for fdt_add_pubkey bintool
  2023-07-06  8:38 [PATCH v2 00/11] Sign Xilinx ZynqMP SPL/FSBL boot images using binman lukas.funke-oss
                   ` (2 preceding siblings ...)
  2023-07-06  8:38 ` [PATCH v2 03/11] binman: blob_dtb: Add fake_size argument to ObtainContents() lukas.funke-oss
@ 2023-07-06  8:38 ` lukas.funke-oss
  2023-07-06  8:38 ` [PATCH v2 05/11] binman: ftest: Add test for u_boot_spl_pubkey_dtb lukas.funke-oss
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 19+ messages in thread
From: lukas.funke-oss @ 2023-07-06  8:38 UTC (permalink / raw
  To: u-boot; +Cc: Simon Glass, Quentin Schulz, Lukas Funke, Alper Nebi Yasak

From: Lukas Funke <lukas.funke@weidmueller.com>

Add documentation for btool which calls 'fdt_add_pubkey'

Signed-off-by: Lukas Funke <lukas.funke@weidmueller.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 tools/binman/bintools.rst | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/tools/binman/bintools.rst b/tools/binman/bintools.rst
index c30e7eb9ff..88221adbe1 100644
--- a/tools/binman/bintools.rst
+++ b/tools/binman/bintools.rst
@@ -183,3 +183,13 @@ Documentation is available via::
 
 
 
+Bintool: fdt_add_pubkey: Add public key to device tree
+---------------------------------------------
+
+This bintool supports running `fdt_add_pubkey` in order to add a public
+key coming from a certificate to a device-tree.
+
+Normally signing is done using `mkimage` in context of `binman sign`. However,
+in this process the public key is not added to the stage before u-boot proper.
+Using `fdt_add_pubkey` the key can be injected to the SPL independent of
+`mkimage`
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 05/11] binman: ftest: Add test for u_boot_spl_pubkey_dtb
  2023-07-06  8:38 [PATCH v2 00/11] Sign Xilinx ZynqMP SPL/FSBL boot images using binman lukas.funke-oss
                   ` (3 preceding siblings ...)
  2023-07-06  8:38 ` [PATCH v2 04/11] binman: doc: Add documentation for fdt_add_pubkey bintool lukas.funke-oss
@ 2023-07-06  8:38 ` lukas.funke-oss
  2023-07-06  8:38 ` [PATCH v2 06/11] binman: btool: Add fdt_add_pubkey as btool lukas.funke-oss
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 19+ messages in thread
From: lukas.funke-oss @ 2023-07-06  8:38 UTC (permalink / raw
  To: u-boot; +Cc: Simon Glass, Quentin Schulz, Lukas Funke, Alper Nebi Yasak

From: Lukas Funke <lukas.funke@weidmueller.com>

Add test for u_boot_spl_pubkey_dtb. The test adds a public key to the
dtb and checks if the required nodes will be added to the images dtb.

Signed-off-by: Lukas Funke <lukas.funke@weidmueller.com>
Reviewed-by: Simon Glass <sjg@chromium.org>

---

Changes in v2:
- Changed u_boot_spl_pubkey_dtb to u-boot-spl-pubkey-dtb

 tools/binman/ftest.py                    | 32 ++++++++++++++++++++++++
 tools/binman/test/281_spl_pubkey_dtb.dts | 16 ++++++++++++
 2 files changed, 48 insertions(+)
 create mode 100644 tools/binman/test/281_spl_pubkey_dtb.dts

diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index 43b4f850a6..0ee0ce1155 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -638,6 +638,16 @@ class TestFunctional(unittest.TestCase):
         TestFunctional._MakeInputFile('vpl/u-boot-vpl',
             tools.read_file(cls.ElfTestFile(src_fname)))
 
+    @classmethod
+    def _SetupPmuFwlElf(cls, src_fname='bss_data'):
+        """Set up an ELF file with a '_dt_ucode_base_size' symbol
+
+        Args:
+            Filename of ELF file to use as VPL
+        """
+        TestFunctional._MakeInputFile('pmu-firmware.elf',
+            tools.read_file(cls.ElfTestFile(src_fname)))
+
     @classmethod
     def _SetupDescriptor(cls):
         with open(cls.TestFile('descriptor.bin'), 'rb') as fd:
@@ -6677,5 +6687,27 @@ fdt         fdtmap                Extract the devicetree blob from the fdtmap
         self.assertIn("Node '/fit': Missing tool: 'mkimage'", str(e.exception))
 
 
+    def testSplPubkeyDtb(self):
+         """Test u_boot_spl_pubkey_dtb etype"""
+         data = tools.read_file(self.TestFile("key.pem"))
+         self._MakeInputFile("key.crt", data)
+         self._DoReadFileRealDtb('281_spl_pubkey_dtb.dts')
+         image = control.images['image']
+         entries = image.GetEntries()
+         dtb_entry = entries['u-boot-spl-pubkey-dtb']
+         dtb_data = dtb_entry.GetData()
+         dtb = fdt.Fdt.FromData(dtb_data)
+         dtb.Scan()
+
+         signature_node = dtb.GetNode('/signature')
+         self.assertIsNotNone(signature_node)
+         key_node = signature_node.FindNode("key-key")
+         self.assertIsNotNone(key_node)
+         self.assertEqual(fdt_util.GetString(key_node, "required"),
+                          "conf")
+         self.assertEqual(fdt_util.GetString(key_node, "algo"),
+                          "sha384,rsa4096")
+         self.assertEqual(fdt_util.GetString(key_node, "key-name-hint"),
+                          "key")
 if __name__ == "__main__":
     unittest.main()
diff --git a/tools/binman/test/281_spl_pubkey_dtb.dts b/tools/binman/test/281_spl_pubkey_dtb.dts
new file mode 100644
index 0000000000..f845db66f5
--- /dev/null
+++ b/tools/binman/test/281_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 = "key";
+		};
+	};
+};
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 06/11] binman: btool: Add fdt_add_pubkey as btool
  2023-07-06  8:38 [PATCH v2 00/11] Sign Xilinx ZynqMP SPL/FSBL boot images using binman lukas.funke-oss
                   ` (4 preceding siblings ...)
  2023-07-06  8:38 ` [PATCH v2 05/11] binman: ftest: Add test for u_boot_spl_pubkey_dtb lukas.funke-oss
@ 2023-07-06  8:38 ` lukas.funke-oss
  2023-07-06  8:38 ` [PATCH v2 07/11] binman: etype: Add u_boot_spl_pubkey_dtb etype lukas.funke-oss
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 19+ messages in thread
From: lukas.funke-oss @ 2023-07-06  8:38 UTC (permalink / raw
  To: u-boot; +Cc: Simon Glass, Quentin Schulz, Lukas Funke, Alper Nebi Yasak

From: Lukas Funke <lukas.funke@weidmueller.com>

Add btool which calls 'fdt_add_pubkey'

Signed-off-by: Lukas Funke <lukas.funke@weidmueller.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 tools/binman/btool/fdt_add_pubkey.py | 67 ++++++++++++++++++++++++++++
 1 file changed, 67 insertions(+)
 create mode 100644 tools/binman/btool/fdt_add_pubkey.py

diff --git a/tools/binman/btool/fdt_add_pubkey.py b/tools/binman/btool/fdt_add_pubkey.py
new file mode 100644
index 0000000000..a50774200c
--- /dev/null
+++ b/tools/binman/btool/fdt_add_pubkey.py
@@ -0,0 +1,67 @@
+# SPDX-License-Identifier: GPL-2.0+
+# Copyright (C) 2023 Weidmüller Interface GmbH & Co. KG
+# Lukas Funke <lukas.funke@weidmueller.com>
+#
+"""Bintool implementation for fdt_add_pubkey"""
+
+from binman import bintool
+
+class Bintoolfdt_add_pubkey(bintool.Bintool):
+    """Add public key to control dtb (spl or u-boot proper)
+
+    This bintool supports running `fdt_add_pubkey`.
+
+    Normally mkimage adds signature information to the control dtb. However
+    binman images are built independent from each other. Thus it is required
+    to add the public key separately from mkimage.
+    """
+    def __init__(self, name):
+        super().__init__(name, 'Generate image for U-Boot')
+
+    # pylint: disable=R0913
+    def run(self, input_fname, keydir, keyname, required, algo):
+        """Run fdt_add_pubkey
+
+        Args:
+            input_fname (str): dtb file to sign
+            keydir (str): Directory with public key. Optional parameter,
+                default value: '.' (current directory)
+            keyname (str): Public key name. Optional parameter,
+                default value: key
+            required (str): If present this indicates that the key must be
+                verified for the image / configuration to be considered valid.
+            algo (str): Cryptographic algorithm. Optional parameter,
+                default value: sha1,rsa2048
+        """
+        args = []
+        if algo:
+            args += ['-a', algo]
+        if keydir:
+            args += ['-k', keydir]
+        if keyname:
+            args += ['-n', keyname]
+        if required:
+            args += ['-r', required]
+
+        args += [ input_fname ]
+
+        return self.run_cmd(*args)
+
+    def fetch(self, method):
+        """Fetch handler for fdt_add_pubkey
+
+        This installs fdt_add_pubkey using the apt utility.
+
+        Args:
+            method (FETCH_...): Method to use
+
+        Returns:
+            True if the file was fetched and now installed, None if a method
+            other than FETCH_BIN was requested
+
+        Raises:
+            Valuerror: Fetching could not be completed
+        """
+        if method != bintool.FETCH_BIN:
+            return None
+        return self.apt_install('u-boot-tools')
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 07/11] binman: etype: Add u_boot_spl_pubkey_dtb etype
  2023-07-06  8:38 [PATCH v2 00/11] Sign Xilinx ZynqMP SPL/FSBL boot images using binman lukas.funke-oss
                   ` (5 preceding siblings ...)
  2023-07-06  8:38 ` [PATCH v2 06/11] binman: btool: Add fdt_add_pubkey as btool lukas.funke-oss
@ 2023-07-06  8:38 ` lukas.funke-oss
  2023-07-06  8:38 ` [PATCH v2 08/11] binman: doc: Add documentation for Xilinx Bootgen bintool lukas.funke-oss
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 19+ messages in thread
From: lukas.funke-oss @ 2023-07-06  8:38 UTC (permalink / raw
  To: u-boot; +Cc: Simon Glass, Quentin Schulz, Lukas Funke, Alper Nebi Yasak

From: Lukas Funke <lukas.funke@weidmueller.com>

This adds a new etype 'u_boot_spl_pubkey_dtb'. The etype adds the public
key from a certificate to the dtb. This creates a '/signature' node which
is turn contains the fields which make up the public key. Usually this
is done by 'mkimage -K'. However, 'binman sign' does not add the public
key to the SPL. This is why the pubkey is added using this etype.

The etype calls the underlying 'fdt_add_pubkey' tool.

Signed-off-by: Lukas Funke <lukas.funke@weidmueller.com>
Reviewed-by: Simon Glass <sjg@chromium.org>

---

Changes in v2:
- Improved rst/python documentation
- Changed u_boot_spl_pubkey_dtb to u-boot-spl-pubkey-dtb in example

 tools/binman/entries.rst                    |  39 +++++++
 tools/binman/etype/u_boot_spl_pubkey_dtb.py | 109 ++++++++++++++++++++
 2 files changed, 148 insertions(+)
 create mode 100644 tools/binman/etype/u_boot_spl_pubkey_dtb.py

diff --git a/tools/binman/entries.rst b/tools/binman/entries.rst
index b71af801fd..c3c5bda881 100644
--- a/tools/binman/entries.rst
+++ b/tools/binman/entries.rst
@@ -1912,6 +1912,45 @@ binman uses that to look up symbols to write into the SPL binary.
 
 
 
+.. _etype_u_boot_spl_pubkey_dtb:
+
+Entry: u-boot-spl-pubkey-dtb: U-Boot SPL device tree including public key
+-------------------------------------------------------------------------
+
+Properties / Entry arguments:
+    - key-name: Public key name without extension (e.g. .crt). Default is
+                determined by underlying bintool (fdt_add_pubkey),
+                usually 'key'
+    - algo: (Optional) Algorithm used for signing. Default is determined by
+            underlying bintool (fdt_add_pubkey), usually 'sha1,rsa2048'
+    - required: (Optional) If present this indicates that the key must be
+                verified for the image / configuration to be
+                considered valid
+
+The following example shows an image containing an SPL which
+is packed together with the dtb. Binman will add a signature
+node to the dtb.
+
+Example node::
+
+    image {
+    ...
+        spl {
+            filename = "spl.bin"
+
+            u-boot-spl-nodtb {
+            };
+            u-boot-spl-pubkey-dtb {
+                algo = "sha384,rsa4096";
+                required = "conf";
+                key-name = "dev";
+            };
+        };
+    ...
+    }
+
+
+
 .. _etype_u_boot_spl_with_ucode_ptr:
 
 Entry: u-boot-spl-with-ucode-ptr: U-Boot SPL with embedded microcode pointer
diff --git a/tools/binman/etype/u_boot_spl_pubkey_dtb.py b/tools/binman/etype/u_boot_spl_pubkey_dtb.py
new file mode 100644
index 0000000000..e043001b11
--- /dev/null
+++ b/tools/binman/etype/u_boot_spl_pubkey_dtb.py
@@ -0,0 +1,109 @@
+# SPDX-License-Identifier: GPL-2.0+
+# Copyright (c) 2023 Weidmueller GmbH
+# Written by Lukas Funke <lukas.funke@weidmueller.com>
+#
+# Entry-type module for 'u-boot-spl-pubkey.dtb'
+#
+
+import tempfile
+import os
+
+from binman.etype.blob_dtb import Entry_blob_dtb
+
+from dtoc import fdt_util
+
+from u_boot_pylib import tools
+
+# pylint: disable=C0103
+class Entry_u_boot_spl_pubkey_dtb(Entry_blob_dtb):
+    """U-Boot SPL device tree including public key
+
+    Properties / Entry arguments:
+        - key-name: Public key name without extension (e.g. .crt). Default is
+                    determined by underlying bintool (fdt_add_pubkey),
+                    usually 'key'
+        - algo: (Optional) Algorithm used for signing. Default is determined by
+                underlying bintool (fdt_add_pubkey), usually 'sha1,rsa2048'
+        - required: (Optional) If present this indicates that the key must be
+                    verified for the image / configuration to be
+                    considered valid
+
+    The following example shows an image containing an SPL which
+    is packed together with the dtb. Binman will add a signature
+    node to the dtb.
+
+    Example node::
+
+        image {
+        ...
+            spl {
+                filename = "spl.bin"
+
+                u-boot-spl-nodtb {
+                };
+                u-boot-spl-pubkey-dtb {
+                    algo = "sha384,rsa4096";
+                    required = "conf";
+                    key-name = "dev";
+                };
+            };
+        ...
+        }
+    """
+
+    def __init__(self, section, etype, node):
+        # Put this here to allow entry-docs and help to work without libfdt
+        global state
+        from binman import state
+
+        super().__init__(section, etype, node)
+        self.required_props = ['key-name']
+        self.fdt_add_pubkey = None
+        self._algo = fdt_util.GetString(self._node, 'algo')
+        self._required = fdt_util.GetString(self._node, 'required')
+        self._keyname = fdt_util.GetString(self._node, 'key-name')
+
+    def ObtainContents(self, fake_size=0):
+        """ Add public key to SPL dtb
+
+            Add public key which is pointed out by
+            'key-name' to node 'signature' in the spl-dtb
+
+            This is equivalent to the '-K' option of 'mkimage'
+
+        Args:
+            fake_size (int): unused
+        """
+
+        # We don't pass fake_size upwards because this is currently
+        # not supported by the blob type
+        super().ObtainContents()
+
+        with tempfile.NamedTemporaryFile(prefix=os.path.basename(
+                                         self.GetFdtEtype()),
+                                         dir=tools.get_output_dir())\
+                                              as pubkey_tdb:
+            tools.write_file(pubkey_tdb.name, self.GetData())
+            keyname = tools.get_input_filename(self._keyname + ".crt")
+            self.fdt_add_pubkey.run(pubkey_tdb.name,
+                                    os.path.dirname(keyname),
+                                    self._keyname,
+                                    self._required, self._algo)
+            dtb = tools.read_file(pubkey_tdb.name)
+            self.SetContents(dtb)
+            state.UpdateFdtContents(self.GetFdtEtype(), dtb)
+
+        return True
+
+    # pylint: disable=R0201,C0116
+    def GetDefaultFilename(self):
+        return 'spl/u-boot-spl-pubkey.dtb'
+
+    # pylint: disable=R0201,C0116
+    def GetFdtEtype(self):
+        return 'u-boot-spl-dtb'
+
+    # pylint: disable=R0201,C0116
+    def AddBintools(self, btools):
+        super().AddBintools(btools)
+        self.fdt_add_pubkey = self.AddBintool(btools, 'fdt_add_pubkey')
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 08/11] binman: doc: Add documentation for Xilinx Bootgen bintool
  2023-07-06  8:38 [PATCH v2 00/11] Sign Xilinx ZynqMP SPL/FSBL boot images using binman lukas.funke-oss
                   ` (6 preceding siblings ...)
  2023-07-06  8:38 ` [PATCH v2 07/11] binman: etype: Add u_boot_spl_pubkey_dtb etype lukas.funke-oss
@ 2023-07-06  8:38 ` lukas.funke-oss
  2023-07-06  8:38 ` [PATCH v2 09/11] binman: btool: Add Xilinx Bootgen btool lukas.funke-oss
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 19+ messages in thread
From: lukas.funke-oss @ 2023-07-06  8:38 UTC (permalink / raw
  To: u-boot; +Cc: Simon Glass, Quentin Schulz, Lukas Funke, Alper Nebi Yasak

From: Lukas Funke <lukas.funke@weidmueller.com>

Add documentation for the 'bootgen' bintool

Signed-off-by: Lukas Funke <lukas.funke@weidmueller.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 tools/binman/bintools.rst | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/tools/binman/bintools.rst b/tools/binman/bintools.rst
index 88221adbe1..c8d69f7177 100644
--- a/tools/binman/bintools.rst
+++ b/tools/binman/bintools.rst
@@ -193,3 +193,15 @@ Normally signing is done using `mkimage` in context of `binman sign`. However,
 in this process the public key is not added to the stage before u-boot proper.
 Using `fdt_add_pubkey` the key can be injected to the SPL independent of
 `mkimage`
+
+
+
+Bintool: bootgen: Sign ZynqMP FSBL image
+---------------------------------------------
+
+This bintool supports running `bootgen` in order to sign a SPL for ZynqMP
+devices.
+
+The bintool automatically creates an appropriate input image file (.bif) for
+bootgen based on the passed arguments. The output is a bootable,
+authenticated `boot.bin` file.
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 09/11] binman: btool: Add Xilinx Bootgen btool
  2023-07-06  8:38 [PATCH v2 00/11] Sign Xilinx ZynqMP SPL/FSBL boot images using binman lukas.funke-oss
                   ` (7 preceding siblings ...)
  2023-07-06  8:38 ` [PATCH v2 08/11] binman: doc: Add documentation for Xilinx Bootgen bintool lukas.funke-oss
@ 2023-07-06  8:38 ` lukas.funke-oss
  2023-07-07 17:35   ` Simon Glass
  2023-07-06  8:38 ` [PATCH v2 10/11] binman: ftest: Add test for xilinx_fsbl_auth etype lukas.funke-oss
                   ` (2 subsequent siblings)
  11 siblings, 1 reply; 19+ messages in thread
From: lukas.funke-oss @ 2023-07-06  8:38 UTC (permalink / raw
  To: u-boot; +Cc: Simon Glass, Quentin Schulz, Lukas Funke, Alper Nebi Yasak

From: Lukas Funke <lukas.funke@weidmueller.com>

Add the Xilinx Bootgen as bintool. Xilinx Bootgen is used to create
bootable SPL (FSBL in Xilinx terms) images for Zynq/ZynqMP devices. The
btool creates a signed version of the SPL. Additionally to signing the
key source for the decryption engine can be passend to the boot image.

Signed-off-by: Lukas Funke <lukas.funke@weidmueller.com>

---

Changes in v2:
- Pass additional 'keysrc_enc' parameter to Bootgen
- Added more information and terms to documentation

 tools/binman/btool/bootgen.py | 136 ++++++++++++++++++++++++++++++++++
 1 file changed, 136 insertions(+)
 create mode 100644 tools/binman/btool/bootgen.py

diff --git a/tools/binman/btool/bootgen.py b/tools/binman/btool/bootgen.py
new file mode 100644
index 0000000000..a30268c964
--- /dev/null
+++ b/tools/binman/btool/bootgen.py
@@ -0,0 +1,136 @@
+# SPDX-License-Identifier: GPL-2.0+
+# Copyright (C) 2023 Weidmüller Interface GmbH & Co. KG
+# Lukas Funke <lukas.funke@weidmueller.com>
+#
+"""Bintool implementation for bootgen
+
+bootgen allows creating bootable SPL for Zynq(MP)
+
+Documentation is available via::
+https://www.xilinx.com/support/documents/sw_manuals/xilinx2022_1/ug1283-bootgen-user-guide.pdf
+
+Source code is available at:
+
+https://github.com/Xilinx/bootgen
+
+"""
+import tempfile
+
+from binman import bintool
+from u_boot_pylib import tools
+
+# pylint: disable=C0103
+class Bintoolbootgen(bintool.Bintool):
+    """Generate bootable fsbl image for zynq/zynqmp
+
+    This bintools supports running Xilinx "bootgen" in order
+    to generate a bootable, authenticated image form an SPL.
+
+    """
+    def __init__(self, name):
+        super().__init__(name, 'Xilinx Bootgen',
+                         version_regex=r'^\*\*\*\*\*\* Xilinx Bootgen',
+                         version_args='')
+
+    # pylint: disable=R0913
+    def sign(self, arch, spl_elf_fname, pmufw_elf_fname,
+             psk_fname, ssk_fname, fsbl_config, auth_params, keysrc_enc,
+             output_fname):
+        """ Sign SPL elf file and bundle it PMU firmware into an image
+
+        The method bundels the SPL together with a 'Platform Management Unit'
+        (PMU)[1] firmware into a single bootable image. The image in turn is
+        signed with the provided 'secondary secret key' (ssk), which in turn is
+        signed with the 'primary secret key' (ppk). In order to verify the
+        authenticity of the ppk, it's hash has to be fused into the device
+        itself.
+
+        In Xilinx terms the SPL is usually called 'FSBL'
+        (First Stage Boot Loder). The jobs of the SPL and the FSBL are mostly
+        the same: load bitstream, bootstrap u-boot.
+
+        Args:
+            arch (str): Xilinx SoC architecture. Currently only 'zynqmp' is
+                supported.
+            spl_elf_fname (str): Filename of SPL ELF file. The filename must end
+                with '.elf' in order for bootgen to recognized it as an ELF
+                file. Otherwise the start address field is missinterpreted.
+            pmufw_elf_fname (str): Filename PMU ELF firmware.
+            psk_fname (str): Filename of the primary secret key (psk). The psk
+                is a .pem file which holds the RSA private key used for signing
+                the secondardy secret key.
+            ssk_fname (str): Filename of the secondary secret key. The ssk
+                is a .pem file which holds the RSA private key used for signing
+                the aktual boot firmware.
+            fsbl_config (str): FSBL config options. A string list of fsbl config
+                options. Valid values according to [2] are:
+                "bh_auth_enable": Boot Header Authentication Enable: RSA
+                    authentication of the bootimage is done
+                    excluding the verification of PPK hash and SPK ID. This is
+                    useful for debugging before bricking a device.
+                "auth_only": Boot image is only RSA signed. FSBL should not be
+                    decrypted. See the
+                    Zynq UltraScale+ Device Technical Reference Manual (UG1085)
+                    for more information.
+                There are more options which relate to PUF (physical unclonable
+                functions). Please refer to Xilinx manuals for fruther info.
+            auth_params (str): Authentication parameter. A semicolon separated
+                list of authentication parameters. Valid values according to [3]
+                are:
+                "ppk_select=<0|1>" - Select which ppk to use
+                "spk_id=<32-bit spk id>" - Specifies which SPK can be
+                    used or revoked, default is 0x0
+                "spk_select=<spk-efuse/user-efuse>" - To differentiate spk and
+                    user efuses.
+                "auth_header" - To authenticate headers when no partition
+                    is authenticated.
+            keysrc_enc (str): This specifies the Key source for encryption.
+                Valid values according to [3] are:
+                "bbram_red_key" - RED key stored in BBRAM
+                "efuse_red_key" - RED key stored in efuse
+                "efuse_gry_key" - Grey (Obfuscated) Key stored in eFUSE.
+                "bh_gry_key" - Grey (Obfuscated) Key stored in boot header.
+                "bh_blk_key" - Black Key stored in boot header.
+                "efuse_blk_key" - Black Key stored in eFUSE.
+                "kup_key" - User Key.
+
+            output_fname (str): Filename where bootgen should write the result
+
+        [1] https://xilinx-wiki.atlassian.net/wiki/spaces/A/pages/18841724/PMU+Firmware
+        [2] https://docs.xilinx.com/r/en-US/ug1283-bootgen-user-guide/fsbl_config
+        [3] https://docs.xilinx.com/r/en-US/ug1283-bootgen-user-guide/auth_params
+        [4] https://docs.xilinx.com/r/en-US/ug1283-bootgen-user-guide/keysrc_encryption
+        """
+
+        _fsbl_config = f"[fsbl_config] {fsbl_config}" if fsbl_config else ""
+        _auth_params = f"[auth_params] {auth_params}" if auth_params else ""
+        _keysrc_enc  = f"[keysrc_encryption] {keysrc_enc}" if keysrc_enc else ""
+
+        bif_template = f"""u_boot_spl_aes_rsa: {{
+            [pskfile] {psk_fname}
+            [sskfile] {ssk_fname}
+            {_keysrc_enc}
+            {_fsbl_config}
+            {_auth_params}
+            [ bootloader,
+              authentication = rsa,
+              destination_cpu=a53-0] {spl_elf_fname}
+            [pmufw_image] {pmufw_elf_fname}
+        }}"""
+        args = ["-arch", arch]
+
+        bif_fname = tools.get_output_filename('bootgen-in.sign.bif')
+        tools.write_file(bif_fname, bif_template, False)
+        args += ["-image", bif_fname, '-w', '-o', output_fname]
+        self.run_cmd(*args)
+
+    def fetch(self, method):
+        """Fetch bootgen from git"""
+        if method != bintool.FETCH_BUILD:
+            return None
+
+        result = self.build_from_git(
+            'https://github.com/Xilinx/bootgen',
+            'all',
+            'build/bootgen/bootgen')
+        return result
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 10/11] binman: ftest: Add test for xilinx_fsbl_auth etype
  2023-07-06  8:38 [PATCH v2 00/11] Sign Xilinx ZynqMP SPL/FSBL boot images using binman lukas.funke-oss
                   ` (8 preceding siblings ...)
  2023-07-06  8:38 ` [PATCH v2 09/11] binman: btool: Add Xilinx Bootgen btool lukas.funke-oss
@ 2023-07-06  8:38 ` lukas.funke-oss
  2023-07-07 17:35   ` Simon Glass
  2023-07-06  8:38 ` [PATCH v2 11/11] binman: etype: Add " lukas.funke-oss
  2023-07-07 17:35 ` [PATCH v2 00/11] Sign Xilinx ZynqMP SPL/FSBL boot images using binman Simon Glass
  11 siblings, 1 reply; 19+ messages in thread
From: lukas.funke-oss @ 2023-07-06  8:38 UTC (permalink / raw
  To: u-boot
  Cc: Simon Glass, Quentin Schulz, Lukas Funke, Alper Nebi Yasak,
	Michal Simek

From: Lukas Funke <lukas.funke@weidmueller.com>

Add test for the 'xilinx_fsbl_auth' etype

Signed-off-by: Lukas Funke <lukas.funke@weidmueller.com>

---

Changes in v2:
- Fixed typo in dts name

 tools/binman/ftest.py                      |  8 ++++++++
 tools/binman/test/280_xilinx_fsbl_auth.dts | 23 ++++++++++++++++++++++
 2 files changed, 31 insertions(+)
 create mode 100644 tools/binman/test/280_xilinx_fsbl_auth.dts

diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index 0ee0ce1155..27a8a37864 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -6686,6 +6686,14 @@ fdt         fdtmap                Extract the devicetree blob from the fdtmap
                                 ['fit'])
         self.assertIn("Node '/fit': Missing tool: 'mkimage'", str(e.exception))
 
+    def testXilinxFsblAuth(self):
+        """Test xilinx_fsbl_auth etype"""
+        data = tools.read_file(self.TestFile("key.key"))
+        self._MakeInputFile("psk.pem", data)
+        self._MakeInputFile("ssk.pem", data)
+        self._SetupPmuFwlElf()
+        self._SetupSplElf()
+        self._DoReadFileRealDtb('280_xilinx_fsbl_auth.dts')
 
     def testSplPubkeyDtb(self):
          """Test u_boot_spl_pubkey_dtb etype"""
diff --git a/tools/binman/test/280_xilinx_fsbl_auth.dts b/tools/binman/test/280_xilinx_fsbl_auth.dts
new file mode 100644
index 0000000000..a37c5aa072
--- /dev/null
+++ b/tools/binman/test/280_xilinx_fsbl_auth.dts
@@ -0,0 +1,23 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		xilinx-fsbl-auth {
+			filename = "boot.bin";
+
+			psk-filename = "psk.pem";
+			ssk-filename = "ssk.pem";
+			auth-params = "ppk_select=0", "spk_id=0x00000000";
+
+			u-boot-spl-nodtb {
+			};
+			u-boot-spl-dtb {
+			};
+		};
+	};
+};
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 11/11] binman: etype: Add xilinx_fsbl_auth etype
  2023-07-06  8:38 [PATCH v2 00/11] Sign Xilinx ZynqMP SPL/FSBL boot images using binman lukas.funke-oss
                   ` (9 preceding siblings ...)
  2023-07-06  8:38 ` [PATCH v2 10/11] binman: ftest: Add test for xilinx_fsbl_auth etype lukas.funke-oss
@ 2023-07-06  8:38 ` lukas.funke-oss
  2023-07-07 17:35   ` Simon Glass
  2023-07-07 17:35 ` [PATCH v2 00/11] Sign Xilinx ZynqMP SPL/FSBL boot images using binman Simon Glass
  11 siblings, 1 reply; 19+ messages in thread
From: lukas.funke-oss @ 2023-07-06  8:38 UTC (permalink / raw
  To: u-boot
  Cc: Simon Glass, Quentin Schulz, Lukas Funke, Alper Nebi Yasak,
	Michal Simek

From: Lukas Funke <lukas.funke@weidmueller.com>

This adds a new etype 'xilinx_fsbl_auth'. Using this etype it is possible
to created an authenticated SPL (FSBL in Xilinx terms) for ZynqMP boards.

The etype uses Xilinx Bootgen tools in order to transform the SPL into
a bootable image and sign the image with a given primary and seconrady
public key. For more information to signing the FSBL please refer to the
Xilinx Bootgen documentation.

Here is an example of the etype in use:

    spl {
        filename = "boot.signed.bin";

        xilinx_fsbl_auth {
            psk-filename = "psk0.pem";
            ssk-filename = "ssk0.pem";
            auth-params = "ppk_select=0", "spk_id=0x00000000";

            u_boot_spl_nodtb {
            };
            u_boot_spl_dtb {
            };
        };
    };

For this to work the hash of the primary public key has to be fused
into the ZynqMP device and authentication (RSA_EN) has to be set.

For testing purposes: if ppk hash check should be skipped one can add
the property 'fsbl_config = "bh_auth_enable";' to the etype. However,
this should only be used for testing(!).

Signed-off-by: Lukas Funke <lukas.funke@weidmueller.com>

---

Changes in v2:
- Add 'keysrc-enc' property to pass down to Bootgen
- Improved documentation
- Use predictable output names for intermediated results

 tools/binman/entries.rst               |  53 ++++++
 tools/binman/etype/xilinx_fsbl_auth.py | 213 +++++++++++++++++++++++++
 2 files changed, 266 insertions(+)
 create mode 100644 tools/binman/etype/xilinx_fsbl_auth.py

diff --git a/tools/binman/entries.rst b/tools/binman/entries.rst
index c3c5bda881..98ec3c82a5 100644
--- a/tools/binman/entries.rst
+++ b/tools/binman/entries.rst
@@ -2462,3 +2462,56 @@ may be used instead.
 
 
 
+.. _etype_xilinx_fsbl_auth:
+
+Entry: xilinx-fsbl-auth: Authenticated SPL for booting Xilinx ZynqMP devices
+----------------------------------------------------------------------------
+
+Properties / Entry arguments:
+    - auth-params: (Optional) Authentication parameters passed to bootgen
+    - fsbl-config: (Optional) FSBL parameters passed to bootgen
+    - keysrc-enc: (Optional) Key source when using decryption engine
+    - pmufw-filename: Filename of PMU firmware. Default: pmu-firmware.elf
+    - psk-filename: Filename of primary public key
+    - ssk-filename: Filename of secondary public key
+
+The following example builds an authenticated boot image. The fuses of
+the primary public key (ppk) should be fused together with the RSA_EN flag.
+
+Example node::
+
+    spl {
+        filename = "boot.signed.bin";
+
+        xilinx-fsbl-auth {
+            psk-filename = "psk0.pem";
+            ssk-filename = "ssk0.pem";
+            auth-params = "ppk_select=0", "spk_id=0x00000000";
+
+            u-boot-spl-nodtb {
+            };
+            u-boot-spl-pubkey-dtb {
+                algo = "sha384,rsa4096";
+                required = "conf";
+                key-name = "dev";
+            };
+        };
+    };
+
+For testing purposes, e.g. if no RSA_EN should be fused, one could add
+the "bh_auth_enable" flag in the fsbl-config field. This will skip the
+verification of the ppk fuses and boot the image, even if ppk hash is
+invalid.
+
+Example node::
+
+    xilinx-fsbl-auth {
+        psk-filename = "psk0.pem";
+        ssk-filename = "ssk0.pem";
+        ...
+        fsbl-config = "bh_auth_enable";
+        ...
+    };
+
+
+
diff --git a/tools/binman/etype/xilinx_fsbl_auth.py b/tools/binman/etype/xilinx_fsbl_auth.py
new file mode 100644
index 0000000000..72794ad2bc
--- /dev/null
+++ b/tools/binman/etype/xilinx_fsbl_auth.py
@@ -0,0 +1,213 @@
+# SPDX-License-Identifier: GPL-2.0+
+# Copyright (c) 2023 Weidmueller GmbH
+# Written by Lukas Funke <lukas.funke@weidmueller.com>
+#
+# Entry-type module for signed ZynqMP boot images (boot.bin)
+#
+
+import tempfile
+
+from collections import OrderedDict
+
+from binman import elf
+from binman.entry import Entry
+
+from dtoc import fdt_util
+
+from u_boot_pylib import tools
+from u_boot_pylib import command
+
+# pylint: disable=C0103
+class Entry_xilinx_fsbl_auth(Entry):
+    """Authenticated SPL for booting Xilinx ZynqMP devices
+
+    Properties / Entry arguments:
+        - auth-params: (Optional) Authentication parameters passed to bootgen
+        - fsbl-config: (Optional) FSBL parameters passed to bootgen
+        - keysrc-enc: (Optional) Key source when using decryption engine
+        - pmufw-filename: Filename of PMU firmware. Default: pmu-firmware.elf
+        - psk-filename: Filename of primary public key
+        - ssk-filename: Filename of secondary public key
+
+    The following example builds an authenticated boot image. The fuses of
+    the primary public key (ppk) should be fused together with the RSA_EN flag.
+
+    Example node::
+
+        spl {
+            filename = "boot.signed.bin";
+
+            xilinx-fsbl-auth {
+                psk-filename = "psk0.pem";
+                ssk-filename = "ssk0.pem";
+                auth-params = "ppk_select=0", "spk_id=0x00000000";
+
+                u-boot-spl-nodtb {
+                };
+                u-boot-spl-pubkey-dtb {
+                    algo = "sha384,rsa4096";
+                    required = "conf";
+                    key-name = "dev";
+                };
+            };
+        };
+
+    For testing purposes, e.g. if no RSA_EN should be fused, one could add
+    the "bh_auth_enable" flag in the fsbl-config field. This will skip the
+    verification of the ppk fuses and boot the image, even if ppk hash is
+    invalid.
+
+    Example node::
+
+        xilinx-fsbl-auth {
+            psk-filename = "psk0.pem";
+            ssk-filename = "ssk0.pem";
+            ...
+            fsbl-config = "bh_auth_enable";
+            ...
+        };
+    """
+    def __init__(self, section, etype, node):
+        super().__init__(section, etype, node)
+        self._auth_params = None
+        self._entries = OrderedDict()
+        self._filename = None
+        self._fsbl_config = None
+        self._keysrc_enc = None
+        self._pmufw_filename = None
+        self._psk_filename = None
+        self._ssk_filename = None
+        self.align_default = None
+        self.bootgen = None
+        self.required_props = ['psk-filename', 'ssk-filename']
+
+    def ReadNode(self):
+        """Read properties from the xilinx_fsbl_auth node"""
+        super().ReadNode()
+        self._auth_params = fdt_util.GetStringList(self._node,
+                                                   'auth-params')
+        self._filename = fdt_util.GetString(self._node, 'filename')
+        self._fsbl_config = fdt_util.GetStringList(self._node,
+                                                   'fsbl-config')
+        self._keysrc_enc = fdt_util.GetString(self._node,
+                                                   'keysrc-enc')
+        self._pmufw_filename = fdt_util.GetString(self._node,
+                                                  'pmufw-filename',
+                                                  'pmu-firmware.elf')
+        self._psk_filename = fdt_util.GetString(self._node, 'psk-filename',
+                                            'psk.pem')
+        self._ssk_filename = fdt_util.GetString(self._node, 'ssk-filename',
+                                            'ssk.pem')
+        self.ReadEntries()
+
+    def ReadEntries(self):
+        """Read the subnodes to find out what should go in this image"""
+        for node in self._node.subnodes:
+            entry = Entry.Create(self, node)
+            entry.ReadNode()
+            self._entries[entry.name] = entry
+
+    @classmethod
+    def __ToElf(self, data, output_fname):
+        """ Convert SPL object file to bootable ELF file.
+
+        Args:
+            data (bytearray): u-boot-spl-nodtb + u-boot-spl-pubkey-dtb obj file
+                                data
+            output_fname (str): Filename of converted FSBL ELF file
+        """
+        platform_elfflags = []
+
+        gcc, args = tools.get_target_compile_tool('cc')
+        args += ['-dumpmachine']
+        stdout = command.output(gcc, *args)
+        # split target machine triplet (arch, vendor, os)
+        arch, _, _ = stdout.split('-')
+
+        if arch == 'aarch64':
+            platform_elfflags = ["-B", "aarch64", "-O", "elf64-littleaarch64"]
+        elif arch == 'x86_64':
+            # amd64 support makes no sense for the target platform, but we
+            # include it here to enable testing on hosts
+            platform_elfflags = ["-B", "i386", "-O", "elf64-x86-64"]
+
+        spl_text_base = hex(elf.GetSymbolAddress(
+            tools.get_input_filename('spl/u-boot-spl'), ".text"))
+
+        # Obj file to swap data and text section (rename-section)
+        with tempfile.NamedTemporaryFile(prefix="u-boot-spl-pubkey-",
+                                    suffix=".o.tmp",
+                                    dir=tools.get_output_dir())\
+                                    as tmp_obj:
+            input_objcopy_fname = tmp_obj.name
+            # Align packed content to 4 byte boundary
+            pad = bytearray(tools.align(len(data), 4) - len(data))
+            tools.write_file(input_objcopy_fname, data + pad)
+            # Final output elf file which contains a valid start address
+            with tempfile.NamedTemporaryFile(prefix="u-boot-spl-pubkey-elf-",
+                                            suffix=".o.tmp",
+                                            dir=tools.get_output_dir())\
+                                                as tmp_elf_obj:
+                input_ld_fname = tmp_elf_obj.name
+                objcopy, args = tools.get_target_compile_tool('objcopy')
+                args += ["--rename-section", ".data=.text",
+                        "-I", "binary"]
+                args += platform_elfflags
+                args += [input_objcopy_fname, input_ld_fname]
+                command.run(objcopy, *args)
+
+                ld, args = tools.get_target_compile_tool('ld')
+                args += [input_ld_fname, '-o', output_fname,
+                         "--defsym", f"_start={spl_text_base}",
+                         "-Ttext", spl_text_base]
+                command.run(ld, *args)
+
+    def ObtainContents(self, skip_entry=None, fake_size=0):
+        """ Pack node content, and create bootable, signed ZynqMP boot image
+
+        The method collects the content of this node (usually SPL + dtb) and
+        converts them to an ELF file. The ELF file is passed to the
+        Xilinx bootgen tool which packs the SPL ELF file together with
+        Platform Management Unit (PMU) firmware into a bootable image
+        for ZynqMP devices. The image is signed within this step.
+
+        The result is a bootable, authenticated SPL image for Xilinx ZynqMP
+        devices.
+
+        """
+        bootbin_fname = self._filename if self._filename else \
+                            tools.get_output_filename(
+                            f'boot.{self.GetUniqueName()}.bin')
+
+        pmufw_elf_fname = tools.get_input_filename(self._pmufw_filename)
+        psk_fname = tools.get_input_filename(self._psk_filename)
+        ssk_fname = tools.get_input_filename(self._ssk_filename)
+        fsbl_config = ";".join(self._fsbl_config) if self._fsbl_config else None
+        auth_params = ";".join(self._auth_params) if self._auth_params else None
+
+        spl_elf_fname = tools.get_output_filename('u-boot-spl-pubkey.dtb.elf')
+
+        # Collect node contents. This is usually the SPL concatenated
+        # with the SPL dtb (device tree).
+        data, _, _ = self.collect_contents_to_file(
+                                self._entries.values(), 'spl')
+
+        # We need to convert to node content (see above) into an ELF
+        # file in order to be processed by bootgen.
+        self.__ToElf(bytearray(data), spl_elf_fname)
+
+        # Call Bootgen in order to sign the SPL
+        self.bootgen.sign('zynqmp', spl_elf_fname, pmufw_elf_fname,
+                        psk_fname, ssk_fname, fsbl_config,
+                        auth_params, self._keysrc_enc, bootbin_fname)
+
+        self.SetContents(tools.read_file(bootbin_fname))
+
+        return True
+
+    # pylint: disable=C0116
+    def AddBintools(self, btools):
+        super().AddBintools(btools)
+        for entry in self._entries.values():
+            entry.AddBintools(btools)
+        self.bootgen = self.AddBintool(btools, 'bootgen')
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 01/11] binman: elf: Check for ELF_TOOLS availability and remove extra semicolon
  2023-07-06  8:38 ` [PATCH v2 01/11] binman: elf: Check for ELF_TOOLS availability and remove extra semicolon lukas.funke-oss
@ 2023-07-07 17:35   ` Simon Glass
  2023-07-10 13:49     ` Lukas Funke
  0 siblings, 1 reply; 19+ messages in thread
From: Simon Glass @ 2023-07-07 17:35 UTC (permalink / raw
  To: lukas.funke-oss; +Cc: u-boot, Quentin Schulz, Lukas Funke, Alper Nebi Yasak

Hi Lukas,

On Thu, 6 Jul 2023 at 09:38, <lukas.funke-oss@weidmueller.com> wrote:
>
> From: Lukas Funke <lukas.funke@weidmueller.com>
>
> Check if elf tools are available when running DecodeElf(). Also
> remove superfuous semicolon at line ending.
>
> Signed-off-by: Lukas Funke <lukas.funke@weidmueller.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> ---
>
> (no changes since v1)
>
>  tools/binman/elf.py | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/tools/binman/elf.py b/tools/binman/elf.py
> index 5816284c32..a53f4b9c4f 100644
> --- a/tools/binman/elf.py
> +++ b/tools/binman/elf.py
> @@ -438,13 +438,15 @@ def DecodeElf(data, location):
>      Returns:
>          ElfInfo object containing information about the decoded ELF file
>      """
> +    if not ELF_TOOLS:
> +        raise ValueError("Python: No module named 'elftools'")

Actually this is missing test coverage. See testEmbedFail() for an
example of how to add it for this function.

Use 'binman test -T' to see test coverage.

>      file_size = len(data)
>      with io.BytesIO(data) as fd:
>          elf = ELFFile(fd)
> -        data_start = 0xffffffff;
> -        data_end = 0;
> -        mem_end = 0;
> -        virt_to_phys = 0;
> +        data_start = 0xffffffff
> +        data_end = 0
> +        mem_end = 0
> +        virt_to_phys = 0
>
>          for i in range(elf.num_segments()):
>              segment = elf.get_segment(i)
> --
> 2.30.2
>

Regards,
Simon

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 10/11] binman: ftest: Add test for xilinx_fsbl_auth etype
  2023-07-06  8:38 ` [PATCH v2 10/11] binman: ftest: Add test for xilinx_fsbl_auth etype lukas.funke-oss
@ 2023-07-07 17:35   ` Simon Glass
  0 siblings, 0 replies; 19+ messages in thread
From: Simon Glass @ 2023-07-07 17:35 UTC (permalink / raw
  To: lukas.funke-oss
  Cc: u-boot, Quentin Schulz, Lukas Funke, Alper Nebi Yasak,
	Michal Simek

Hi Lukas,

On Thu, 6 Jul 2023 at 09:38, <lukas.funke-oss@weidmueller.com> wrote:
>
> From: Lukas Funke <lukas.funke@weidmueller.com>
>
> Add test for the 'xilinx_fsbl_auth' etype
>
> Signed-off-by: Lukas Funke <lukas.funke@weidmueller.com>
>
> ---
>
> Changes in v2:
> - Fixed typo in dts name
>
>  tools/binman/ftest.py                      |  8 ++++++++
>  tools/binman/test/280_xilinx_fsbl_auth.dts | 23 ++++++++++++++++++++++
>  2 files changed, 31 insertions(+)
>  create mode 100644 tools/binman/test/280_xilinx_fsbl_auth.dts
>
> diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
> index 0ee0ce1155..27a8a37864 100644
> --- a/tools/binman/ftest.py
> +++ b/tools/binman/ftest.py
> @@ -6686,6 +6686,14 @@ fdt         fdtmap                Extract the devicetree blob from the fdtmap
>                                  ['fit'])
>          self.assertIn("Node '/fit': Missing tool: 'mkimage'", str(e.exception))
>
> +    def testXilinxFsblAuth(self):
> +        """Test xilinx_fsbl_auth etype"""
> +        data = tools.read_file(self.TestFile("key.key"))
> +        self._MakeInputFile("psk.pem", data)
> +        self._MakeInputFile("ssk.pem", data)
> +        self._SetupPmuFwlElf()
> +        self._SetupSplElf()
> +        self._DoReadFileRealDtb('280_xilinx_fsbl_auth.dts')

Can you check something in the image to see it is there?

>
>      def testSplPubkeyDtb(self):
>           """Test u_boot_spl_pubkey_dtb etype"""
> diff --git a/tools/binman/test/280_xilinx_fsbl_auth.dts b/tools/binman/test/280_xilinx_fsbl_auth.dts
> new file mode 100644
> index 0000000000..a37c5aa072
> --- /dev/null
> +++ b/tools/binman/test/280_xilinx_fsbl_auth.dts
> @@ -0,0 +1,23 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +
> +/dts-v1/;
> +
> +/ {
> +       #address-cells = <1>;
> +       #size-cells = <1>;
> +
> +       binman {
> +               xilinx-fsbl-auth {
> +                       filename = "boot.bin";
> +
> +                       psk-filename = "psk.pem";
> +                       ssk-filename = "ssk.pem";
> +                       auth-params = "ppk_select=0", "spk_id=0x00000000";
> +
> +                       u-boot-spl-nodtb {
> +                       };
> +                       u-boot-spl-dtb {
> +                       };
> +               };
> +       };
> +};
> --
> 2.30.2
>

Regards,
Simon

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 09/11] binman: btool: Add Xilinx Bootgen btool
  2023-07-06  8:38 ` [PATCH v2 09/11] binman: btool: Add Xilinx Bootgen btool lukas.funke-oss
@ 2023-07-07 17:35   ` Simon Glass
  0 siblings, 0 replies; 19+ messages in thread
From: Simon Glass @ 2023-07-07 17:35 UTC (permalink / raw
  To: lukas.funke-oss; +Cc: u-boot, Quentin Schulz, Lukas Funke, Alper Nebi Yasak

Hi Lukas,

On Thu, 6 Jul 2023 at 09:38, <lukas.funke-oss@weidmueller.com> wrote:
>
> From: Lukas Funke <lukas.funke@weidmueller.com>
>
> Add the Xilinx Bootgen as bintool. Xilinx Bootgen is used to create
> bootable SPL (FSBL in Xilinx terms) images for Zynq/ZynqMP devices. The
> btool creates a signed version of the SPL. Additionally to signing the
> key source for the decryption engine can be passend to the boot image.
>
> Signed-off-by: Lukas Funke <lukas.funke@weidmueller.com>
>
> ---
>
> Changes in v2:
> - Pass additional 'keysrc_enc' parameter to Bootgen
> - Added more information and terms to documentation
>
>  tools/binman/btool/bootgen.py | 136 ++++++++++++++++++++++++++++++++++
>  1 file changed, 136 insertions(+)
>  create mode 100644 tools/binman/btool/bootgen.py
>

This does not work for me:

binman tool -f missing
Fetching tools:      bootgen bzip2 cbfstool fdt_add_pubkey fiptool
futility gzip ifwitool lz4 lzma_alone lzop mkimage openssl xz zstd
Fetch: bootgen
- trying method: binary download
- trying method: build from source
- clone git repo 'https://github.com/Xilinx/bootgen' to '/tmp/binmanf.qq6wo7tw'
- build target 'all'
- File '/tmp/binmanf.qq6wo7tw/build/bootgen/bootgen' was not produced
- failed to fetch with all methods

Otherwise it looks good.

> diff --git a/tools/binman/btool/bootgen.py b/tools/binman/btool/bootgen.py
> new file mode 100644
> index 0000000000..a30268c964
> --- /dev/null
> +++ b/tools/binman/btool/bootgen.py
> @@ -0,0 +1,136 @@
> +# SPDX-License-Identifier: GPL-2.0+
> +# Copyright (C) 2023 Weidmüller Interface GmbH & Co. KG
> +# Lukas Funke <lukas.funke@weidmueller.com>
> +#
> +"""Bintool implementation for bootgen
> +
> +bootgen allows creating bootable SPL for Zynq(MP)
> +
> +Documentation is available via::
> +https://www.xilinx.com/support/documents/sw_manuals/xilinx2022_1/ug1283-bootgen-user-guide.pdf
> +
> +Source code is available at:
> +
> +https://github.com/Xilinx/bootgen
> +
> +"""
> +import tempfile
> +
> +from binman import bintool
> +from u_boot_pylib import tools
> +
> +# pylint: disable=C0103
> +class Bintoolbootgen(bintool.Bintool):
> +    """Generate bootable fsbl image for zynq/zynqmp
> +
> +    This bintools supports running Xilinx "bootgen" in order
> +    to generate a bootable, authenticated image form an SPL.
> +
> +    """
> +    def __init__(self, name):
> +        super().__init__(name, 'Xilinx Bootgen',
> +                         version_regex=r'^\*\*\*\*\*\* Xilinx Bootgen',
> +                         version_args='')
> +
> +    # pylint: disable=R0913
> +    def sign(self, arch, spl_elf_fname, pmufw_elf_fname,
> +             psk_fname, ssk_fname, fsbl_config, auth_params, keysrc_enc,
> +             output_fname):
> +        """ Sign SPL elf file and bundle it PMU firmware into an image
> +
> +        The method bundels the SPL together with a 'Platform Management Unit'
> +        (PMU)[1] firmware into a single bootable image. The image in turn is
> +        signed with the provided 'secondary secret key' (ssk), which in turn is
> +        signed with the 'primary secret key' (ppk). In order to verify the
> +        authenticity of the ppk, it's hash has to be fused into the device
> +        itself.
> +
> +        In Xilinx terms the SPL is usually called 'FSBL'
> +        (First Stage Boot Loder). The jobs of the SPL and the FSBL are mostly
> +        the same: load bitstream, bootstrap u-boot.
> +
> +        Args:
> +            arch (str): Xilinx SoC architecture. Currently only 'zynqmp' is
> +                supported.
> +            spl_elf_fname (str): Filename of SPL ELF file. The filename must end
> +                with '.elf' in order for bootgen to recognized it as an ELF
> +                file. Otherwise the start address field is missinterpreted.
> +            pmufw_elf_fname (str): Filename PMU ELF firmware.
> +            psk_fname (str): Filename of the primary secret key (psk). The psk
> +                is a .pem file which holds the RSA private key used for signing
> +                the secondardy secret key.
> +            ssk_fname (str): Filename of the secondary secret key. The ssk
> +                is a .pem file which holds the RSA private key used for signing
> +                the aktual boot firmware.
> +            fsbl_config (str): FSBL config options. A string list of fsbl config
> +                options. Valid values according to [2] are:
> +                "bh_auth_enable": Boot Header Authentication Enable: RSA
> +                    authentication of the bootimage is done
> +                    excluding the verification of PPK hash and SPK ID. This is
> +                    useful for debugging before bricking a device.
> +                "auth_only": Boot image is only RSA signed. FSBL should not be
> +                    decrypted. See the
> +                    Zynq UltraScale+ Device Technical Reference Manual (UG1085)
> +                    for more information.
> +                There are more options which relate to PUF (physical unclonable
> +                functions). Please refer to Xilinx manuals for fruther info.
> +            auth_params (str): Authentication parameter. A semicolon separated
> +                list of authentication parameters. Valid values according to [3]
> +                are:
> +                "ppk_select=<0|1>" - Select which ppk to use
> +                "spk_id=<32-bit spk id>" - Specifies which SPK can be
> +                    used or revoked, default is 0x0
> +                "spk_select=<spk-efuse/user-efuse>" - To differentiate spk and
> +                    user efuses.
> +                "auth_header" - To authenticate headers when no partition
> +                    is authenticated.
> +            keysrc_enc (str): This specifies the Key source for encryption.
> +                Valid values according to [3] are:
> +                "bbram_red_key" - RED key stored in BBRAM
> +                "efuse_red_key" - RED key stored in efuse
> +                "efuse_gry_key" - Grey (Obfuscated) Key stored in eFUSE.
> +                "bh_gry_key" - Grey (Obfuscated) Key stored in boot header.
> +                "bh_blk_key" - Black Key stored in boot header.
> +                "efuse_blk_key" - Black Key stored in eFUSE.
> +                "kup_key" - User Key.
> +
> +            output_fname (str): Filename where bootgen should write the result
> +
> +        [1] https://xilinx-wiki.atlassian.net/wiki/spaces/A/pages/18841724/PMU+Firmware
> +        [2] https://docs.xilinx.com/r/en-US/ug1283-bootgen-user-guide/fsbl_config
> +        [3] https://docs.xilinx.com/r/en-US/ug1283-bootgen-user-guide/auth_params
> +        [4] https://docs.xilinx.com/r/en-US/ug1283-bootgen-user-guide/keysrc_encryption
> +        """
> +
> +        _fsbl_config = f"[fsbl_config] {fsbl_config}" if fsbl_config else ""
> +        _auth_params = f"[auth_params] {auth_params}" if auth_params else ""
> +        _keysrc_enc  = f"[keysrc_encryption] {keysrc_enc}" if keysrc_enc else ""
> +
> +        bif_template = f"""u_boot_spl_aes_rsa: {{
> +            [pskfile] {psk_fname}
> +            [sskfile] {ssk_fname}
> +            {_keysrc_enc}
> +            {_fsbl_config}
> +            {_auth_params}
> +            [ bootloader,
> +              authentication = rsa,
> +              destination_cpu=a53-0] {spl_elf_fname}
> +            [pmufw_image] {pmufw_elf_fname}
> +        }}"""
> +        args = ["-arch", arch]
> +
> +        bif_fname = tools.get_output_filename('bootgen-in.sign.bif')
> +        tools.write_file(bif_fname, bif_template, False)
> +        args += ["-image", bif_fname, '-w', '-o', output_fname]
> +        self.run_cmd(*args)
> +
> +    def fetch(self, method):
> +        """Fetch bootgen from git"""
> +        if method != bintool.FETCH_BUILD:
> +            return None
> +
> +        result = self.build_from_git(
> +            'https://github.com/Xilinx/bootgen',
> +            'all',
> +            'build/bootgen/bootgen')
> +        return result
> --
> 2.30.2
>

Regards,
Simon

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 11/11] binman: etype: Add xilinx_fsbl_auth etype
  2023-07-06  8:38 ` [PATCH v2 11/11] binman: etype: Add " lukas.funke-oss
@ 2023-07-07 17:35   ` Simon Glass
  0 siblings, 0 replies; 19+ messages in thread
From: Simon Glass @ 2023-07-07 17:35 UTC (permalink / raw
  To: lukas.funke-oss
  Cc: u-boot, Quentin Schulz, Lukas Funke, Alper Nebi Yasak,
	Michal Simek

Hi Lukas,

On Thu, 6 Jul 2023 at 09:38, <lukas.funke-oss@weidmueller.com> wrote:
>
> From: Lukas Funke <lukas.funke@weidmueller.com>
>
> This adds a new etype 'xilinx_fsbl_auth'. Using this etype it is possible
> to created an authenticated SPL (FSBL in Xilinx terms) for ZynqMP boards.
>
> The etype uses Xilinx Bootgen tools in order to transform the SPL into
> a bootable image and sign the image with a given primary and seconrady
> public key. For more information to signing the FSBL please refer to the
> Xilinx Bootgen documentation.
>
> Here is an example of the etype in use:
>
>     spl {
>         filename = "boot.signed.bin";
>
>         xilinx_fsbl_auth {
>             psk-filename = "psk0.pem";
>             ssk-filename = "ssk0.pem";
>             auth-params = "ppk_select=0", "spk_id=0x00000000";
>
>             u_boot_spl_nodtb {
>             };
>             u_boot_spl_dtb {
>             };
>         };
>     };
>
> For this to work the hash of the primary public key has to be fused
> into the ZynqMP device and authentication (RSA_EN) has to be set.
>
> For testing purposes: if ppk hash check should be skipped one can add
> the property 'fsbl_config = "bh_auth_enable";' to the etype. However,
> this should only be used for testing(!).
>
> Signed-off-by: Lukas Funke <lukas.funke@weidmueller.com>
>
> ---
>
> Changes in v2:
> - Add 'keysrc-enc' property to pass down to Bootgen
> - Improved documentation
> - Use predictable output names for intermediated results
>
>  tools/binman/entries.rst               |  53 ++++++
>  tools/binman/etype/xilinx_fsbl_auth.py | 213 +++++++++++++++++++++++++
>  2 files changed, 266 insertions(+)
>  create mode 100644 tools/binman/etype/xilinx_fsbl_auth.py
>
> diff --git a/tools/binman/entries.rst b/tools/binman/entries.rst
> index c3c5bda881..98ec3c82a5 100644
> --- a/tools/binman/entries.rst
> +++ b/tools/binman/entries.rst
> @@ -2462,3 +2462,56 @@ may be used instead.
>
>
>
> +.. _etype_xilinx_fsbl_auth:
> +
> +Entry: xilinx-fsbl-auth: Authenticated SPL for booting Xilinx ZynqMP devices
> +----------------------------------------------------------------------------
> +
> +Properties / Entry arguments:
> +    - auth-params: (Optional) Authentication parameters passed to bootgen
> +    - fsbl-config: (Optional) FSBL parameters passed to bootgen
> +    - keysrc-enc: (Optional) Key source when using decryption engine
> +    - pmufw-filename: Filename of PMU firmware. Default: pmu-firmware.elf
> +    - psk-filename: Filename of primary public key
> +    - ssk-filename: Filename of secondary public key
> +
> +The following example builds an authenticated boot image. The fuses of
> +the primary public key (ppk) should be fused together with the RSA_EN flag.
> +
> +Example node::
> +
> +    spl {
> +        filename = "boot.signed.bin";
> +
> +        xilinx-fsbl-auth {
> +            psk-filename = "psk0.pem";
> +            ssk-filename = "ssk0.pem";
> +            auth-params = "ppk_select=0", "spk_id=0x00000000";
> +
> +            u-boot-spl-nodtb {
> +            };
> +            u-boot-spl-pubkey-dtb {
> +                algo = "sha384,rsa4096";
> +                required = "conf";
> +                key-name = "dev";
> +            };
> +        };
> +    };
> +
> +For testing purposes, e.g. if no RSA_EN should be fused, one could add
> +the "bh_auth_enable" flag in the fsbl-config field. This will skip the
> +verification of the ppk fuses and boot the image, even if ppk hash is
> +invalid.
> +
> +Example node::
> +
> +    xilinx-fsbl-auth {
> +        psk-filename = "psk0.pem";
> +        ssk-filename = "ssk0.pem";
> +        ...
> +        fsbl-config = "bh_auth_enable";
> +        ...
> +    };
> +
> +
> +
> diff --git a/tools/binman/etype/xilinx_fsbl_auth.py b/tools/binman/etype/xilinx_fsbl_auth.py
> new file mode 100644
> index 0000000000..72794ad2bc
> --- /dev/null
> +++ b/tools/binman/etype/xilinx_fsbl_auth.py
> @@ -0,0 +1,213 @@
> +# SPDX-License-Identifier: GPL-2.0+
> +# Copyright (c) 2023 Weidmueller GmbH
> +# Written by Lukas Funke <lukas.funke@weidmueller.com>
> +#
> +# Entry-type module for signed ZynqMP boot images (boot.bin)
> +#
> +
> +import tempfile
> +
> +from collections import OrderedDict
> +
> +from binman import elf
> +from binman.entry import Entry
> +
> +from dtoc import fdt_util
> +
> +from u_boot_pylib import tools
> +from u_boot_pylib import command
> +
> +# pylint: disable=C0103
> +class Entry_xilinx_fsbl_auth(Entry):
> +    """Authenticated SPL for booting Xilinx ZynqMP devices
> +
> +    Properties / Entry arguments:
> +        - auth-params: (Optional) Authentication parameters passed to bootgen
> +        - fsbl-config: (Optional) FSBL parameters passed to bootgen
> +        - keysrc-enc: (Optional) Key source when using decryption engine
> +        - pmufw-filename: Filename of PMU firmware. Default: pmu-firmware.elf
> +        - psk-filename: Filename of primary public key
> +        - ssk-filename: Filename of secondary public key

Can you link to some docs that explains what all of these really are,
e.g. format of files.

> +
> +    The following example builds an authenticated boot image. The fuses of
> +    the primary public key (ppk) should be fused together with the RSA_EN flag.
> +
> +    Example node::
> +
> +        spl {
> +            filename = "boot.signed.bin";
> +
> +            xilinx-fsbl-auth {
> +                psk-filename = "psk0.pem";
> +                ssk-filename = "ssk0.pem";
> +                auth-params = "ppk_select=0", "spk_id=0x00000000";
> +
> +                u-boot-spl-nodtb {
> +                };
> +                u-boot-spl-pubkey-dtb {
> +                    algo = "sha384,rsa4096";
> +                    required = "conf";
> +                    key-name = "dev";
> +                };
> +            };
> +        };
> +
> +    For testing purposes, e.g. if no RSA_EN should be fused, one could add
> +    the "bh_auth_enable" flag in the fsbl-config field. This will skip the
> +    verification of the ppk fuses and boot the image, even if ppk hash is
> +    invalid.
> +
> +    Example node::
> +
> +        xilinx-fsbl-auth {
> +            psk-filename = "psk0.pem";
> +            ssk-filename = "ssk0.pem";
> +            ...
> +            fsbl-config = "bh_auth_enable";
> +            ...
> +        };
> +    """
> +    def __init__(self, section, etype, node):
> +        super().__init__(section, etype, node)
> +        self._auth_params = None
> +        self._entries = OrderedDict()
> +        self._filename = None
> +        self._fsbl_config = None
> +        self._keysrc_enc = None
> +        self._pmufw_filename = None
> +        self._psk_filename = None
> +        self._ssk_filename = None
> +        self.align_default = None
> +        self.bootgen = None
> +        self.required_props = ['psk-filename', 'ssk-filename']
> +
> +    def ReadNode(self):
> +        """Read properties from the xilinx_fsbl_auth node"""
> +        super().ReadNode()
> +        self._auth_params = fdt_util.GetStringList(self._node,
> +                                                   'auth-params')
> +        self._filename = fdt_util.GetString(self._node, 'filename')
> +        self._fsbl_config = fdt_util.GetStringList(self._node,
> +                                                   'fsbl-config')
> +        self._keysrc_enc = fdt_util.GetString(self._node,
> +                                                   'keysrc-enc')
> +        self._pmufw_filename = fdt_util.GetString(self._node,
> +                                                  'pmufw-filename',
> +                                                  'pmu-firmware.elf')
> +        self._psk_filename = fdt_util.GetString(self._node, 'psk-filename',
> +                                            'psk.pem')
> +        self._ssk_filename = fdt_util.GetString(self._node, 'ssk-filename',
> +                                            'ssk.pem')
> +        self.ReadEntries()
> +
> +    def ReadEntries(self):
> +        """Read the subnodes to find out what should go in this image"""
> +        for node in self._node.subnodes:
> +            entry = Entry.Create(self, node)
> +            entry.ReadNode()
> +            self._entries[entry.name] = entry

This looks like a section, so I suspect your class should be a
subclass of entry_Section. Then you can drop this function since it is
already there.

> +
> +    @classmethod
> +    def __ToElf(self, data, output_fname):

Single _

> +        """ Convert SPL object file to bootable ELF file.

Drop space before C

> +
> +        Args:
> +            data (bytearray): u-boot-spl-nodtb + u-boot-spl-pubkey-dtb obj file
> +                                data
> +            output_fname (str): Filename of converted FSBL ELF file
> +        """
> +        platform_elfflags = []
> +
> +        gcc, args = tools.get_target_compile_tool('cc')
> +        args += ['-dumpmachine']
> +        stdout = command.output(gcc, *args)
> +        # split target machine triplet (arch, vendor, os)
> +        arch, _, _ = stdout.split('-')
> +
> +        if arch == 'aarch64':
> +            platform_elfflags = ["-B", "aarch64", "-O", "elf64-littleaarch64"]
> +        elif arch == 'x86_64':
> +            # amd64 support makes no sense for the target platform, but we
> +            # include it here to enable testing on hosts
> +            platform_elfflags = ["-B", "i386", "-O", "elf64-x86-64"]
> +
> +        spl_text_base = hex(elf.GetSymbolAddress(
> +            tools.get_input_filename('spl/u-boot-spl'), ".text"))
> +
> +        # Obj file to swap data and text section (rename-section)
> +        with tempfile.NamedTemporaryFile(prefix="u-boot-spl-pubkey-",
> +                                    suffix=".o.tmp",
> +                                    dir=tools.get_output_dir())\
> +                                    as tmp_obj:
> +            input_objcopy_fname = tmp_obj.name
> +            # Align packed content to 4 byte boundary
> +            pad = bytearray(tools.align(len(data), 4) - len(data))
> +            tools.write_file(input_objcopy_fname, data + pad)
> +            # Final output elf file which contains a valid start address
> +            with tempfile.NamedTemporaryFile(prefix="u-boot-spl-pubkey-elf-",
> +                                            suffix=".o.tmp",
> +                                            dir=tools.get_output_dir())\
> +                                                as tmp_elf_obj:
> +                input_ld_fname = tmp_elf_obj.name
> +                objcopy, args = tools.get_target_compile_tool('objcopy')
> +                args += ["--rename-section", ".data=.text",
> +                        "-I", "binary"]
> +                args += platform_elfflags
> +                args += [input_objcopy_fname, input_ld_fname]
> +                command.run(objcopy, *args)
> +
> +                ld, args = tools.get_target_compile_tool('ld')
> +                args += [input_ld_fname, '-o', output_fname,
> +                         "--defsym", f"_start={spl_text_base}",
> +                         "-Ttext", spl_text_base]
> +                command.run(ld, *args)
> +
> +    def ObtainContents(self, skip_entry=None, fake_size=0):
> +        """ Pack node content, and create bootable, signed ZynqMP boot image

Drop space before first P

> +
> +        The method collects the content of this node (usually SPL + dtb) and
> +        converts them to an ELF file. The ELF file is passed to the
> +        Xilinx bootgen tool which packs the SPL ELF file together with
> +        Platform Management Unit (PMU) firmware into a bootable image
> +        for ZynqMP devices. The image is signed within this step.
> +
> +        The result is a bootable, authenticated SPL image for Xilinx ZynqMP
> +        devices.
> +
> +        """
> +        bootbin_fname = self._filename if self._filename else \
> +                            tools.get_output_filename(
> +                            f'boot.{self.GetUniqueName()}.bin')
> +
> +        pmufw_elf_fname = tools.get_input_filename(self._pmufw_filename)
> +        psk_fname = tools.get_input_filename(self._psk_filename)
> +        ssk_fname = tools.get_input_filename(self._ssk_filename)
> +        fsbl_config = ";".join(self._fsbl_config) if self._fsbl_config else None
> +        auth_params = ";".join(self._auth_params) if self._auth_params else None
> +
> +        spl_elf_fname = tools.get_output_filename('u-boot-spl-pubkey.dtb.elf')
> +
> +        # Collect node contents. This is usually the SPL concatenated
> +        # with the SPL dtb (device tree).
> +        data, _, _ = self.collect_contents_to_file(
> +                                self._entries.values(), 'spl')

data = self.collect_contents_to_file(...)[0]

> +
> +        # We need to convert to node content (see above) into an ELF
> +        # file in order to be processed by bootgen.
> +        self.__ToElf(bytearray(data), spl_elf_fname)
> +
> +        # Call Bootgen in order to sign the SPL
> +        self.bootgen.sign('zynqmp', spl_elf_fname, pmufw_elf_fname,
> +                        psk_fname, ssk_fname, fsbl_config,
> +                        auth_params, self._keysrc_enc, bootbin_fname)
> +
> +        self.SetContents(tools.read_file(bootbin_fname))
> +
> +        return True

If you convert this class to a sectoin then should you implement
BuildSectionContents() instead of ObtainContents()

> +
> +    # pylint: disable=C0116
> +    def AddBintools(self, btools):
> +        super().AddBintools(btools)
> +        for entry in self._entries.values():
> +            entry.AddBintools(btools)
> +        self.bootgen = self.AddBintool(btools, 'bootgen')
> --
> 2.30.2
>

I notice that test coverage is not 100% with these patches. Please use
'binman test -T' to see what is missing and add more test cases to the
bottom of ftest.py

Regards,
Simon

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 00/11] Sign Xilinx ZynqMP SPL/FSBL boot images using binman
  2023-07-06  8:38 [PATCH v2 00/11] Sign Xilinx ZynqMP SPL/FSBL boot images using binman lukas.funke-oss
                   ` (10 preceding siblings ...)
  2023-07-06  8:38 ` [PATCH v2 11/11] binman: etype: Add " lukas.funke-oss
@ 2023-07-07 17:35 ` Simon Glass
  11 siblings, 0 replies; 19+ messages in thread
From: Simon Glass @ 2023-07-07 17:35 UTC (permalink / raw
  To: lukas.funke-oss
  Cc: u-boot, Quentin Schulz, Lukas Funke, Alper Nebi Yasak,
	Michal Simek, Stefan Herbrechtsmeier

Hi Lukas,

On Thu, 6 Jul 2023 at 09:38, <lukas.funke-oss@weidmueller.com> wrote:
>
> From: Lukas Funke <lukas.funke@weidmueller.com>
>
>
> This series adds two etypes to create a verified boot chain for
> Xilinx ZynqMP devices. The first etype 'xilinx_fsbl_auth' is used to
> create a bootable, signed image for ZynqMP boards using the Xilinx
> Bootgen tool. The second etype 'u_boot_spl_pubkey_dtb' is used to add
> a '/signature' node to the SPL. The public key in the signature is read
> from a certificate file and added using the 'fdt_add_pubkey' tool. The
> series also contains the corresponding btool for calling 'bootgen' and
> 'fdt_add_pubkey'
>
> The following block shows an example on how to use this functionality:
>
>     spl {
>         filename = "boot.signed.bin";
>
>         xilinx_fsbl_auth {
>             psk-filename = "psk0.pem";
>             ssk-filename = "ssk0.pem";
>             auth-params = "ppk_select=0", "spk_id=0x00000000";
>
>             u_boot_spl_nodtb {
>             };
>             u_boot_spl_pubkey_dtb {
>                 algo = "sha384,rsa4096";
>                 required = "conf";
>                 key-name = "dev";
>             };
>         };
>     };
>
>
> Changes in v2:
> - Changed u_boot_spl_pubkey_dtb to u-boot-spl-pubkey-dtb
> - Improved rst/python documentation
> - Changed u_boot_spl_pubkey_dtb to u-boot-spl-pubkey-dtb in example
> - Pass additional 'keysrc_enc' parameter to Bootgen
> - Added more information and terms to documentation
> - Fixed typo in dts name
> - Add 'keysrc-enc' property to pass down to Bootgen
> - Improved documentation
> - Use predictable output names for intermediated results
>
> Lukas Funke (11):
>   binman: elf: Check for ELF_TOOLS availability and remove extra
>     semicolon
>   binman: Don't decompress data while signing
>   binman: blob_dtb: Add fake_size argument to ObtainContents()
>   binman: doc: Add documentation for fdt_add_pubkey bintool
>   binman: ftest: Add test for u_boot_spl_pubkey_dtb
>   binman: btool: Add fdt_add_pubkey as btool
>   binman: etype: Add u_boot_spl_pubkey_dtb etype
>   binman: doc: Add documentation for Xilinx Bootgen bintool
>   binman: btool: Add Xilinx Bootgen btool
>   binman: ftest: Add test for xilinx_fsbl_auth etype
>   binman: etype: Add xilinx_fsbl_auth etype
>
>  tools/binman/bintools.rst                   |  22 ++
>  tools/binman/btool/bootgen.py               | 136 +++++++++++++
>  tools/binman/btool/fdt_add_pubkey.py        |  67 ++++++
>  tools/binman/control.py                     |   2 +-
>  tools/binman/elf.py                         |  10 +-
>  tools/binman/entries.rst                    |  92 +++++++++
>  tools/binman/etype/blob_dtb.py              |   2 +-
>  tools/binman/etype/u_boot_spl_pubkey_dtb.py | 109 ++++++++++
>  tools/binman/etype/xilinx_fsbl_auth.py      | 213 ++++++++++++++++++++
>  tools/binman/ftest.py                       |  42 +++-
>  tools/binman/test/280_xilinx_fsbl_auth.dts  |  23 +++
>  tools/binman/test/281_spl_pubkey_dtb.dts    |  16 ++
>  12 files changed, 727 insertions(+), 7 deletions(-)
>  create mode 100644 tools/binman/btool/bootgen.py
>  create mode 100644 tools/binman/btool/fdt_add_pubkey.py
>  create mode 100644 tools/binman/etype/u_boot_spl_pubkey_dtb.py
>  create mode 100644 tools/binman/etype/xilinx_fsbl_auth.py
>  create mode 100644 tools/binman/test/280_xilinx_fsbl_auth.dts
>  create mode 100644 tools/binman/test/281_spl_pubkey_dtb.dts
>
> --
> 2.30.2
>

This looks pretty good to me. I've made comments on individual
patches. Please do make sure that the docs are enough to understand
the feature - e.g. describing each field. You may need to link to some
'forever' docs somewhere too.

Regards,
Simon

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 01/11] binman: elf: Check for ELF_TOOLS availability and remove extra semicolon
  2023-07-07 17:35   ` Simon Glass
@ 2023-07-10 13:49     ` Lukas Funke
  2023-07-10 21:38       ` Simon Glass
  0 siblings, 1 reply; 19+ messages in thread
From: Lukas Funke @ 2023-07-10 13:49 UTC (permalink / raw
  To: Simon Glass; +Cc: u-boot, Quentin Schulz, Lukas Funke, Alper Nebi Yasak

Hi Simon,

On 07.07.2023 19:35, Simon Glass wrote:
> Hi Lukas,
> 
> On Thu, 6 Jul 2023 at 09:38, <lukas.funke-oss@weidmueller.com> wrote:
>>
>> From: Lukas Funke <lukas.funke@weidmueller.com>
>>
>> Check if elf tools are available when running DecodeElf(). Also
>> remove superfuous semicolon at line ending.
>>
>> Signed-off-by: Lukas Funke <lukas.funke@weidmueller.com>
>> Reviewed-by: Simon Glass <sjg@chromium.org>
>> ---
>>
>> (no changes since v1)
>>
>>   tools/binman/elf.py | 10 ++++++----
>>   1 file changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/tools/binman/elf.py b/tools/binman/elf.py
>> index 5816284c32..a53f4b9c4f 100644
>> --- a/tools/binman/elf.py
>> +++ b/tools/binman/elf.py
>> @@ -438,13 +438,15 @@ def DecodeElf(data, location):
>>       Returns:
>>           ElfInfo object containing information about the decoded ELF file
>>       """
>> +    if not ELF_TOOLS:
>> +        raise ValueError("Python: No module named 'elftools'")
> 
> Actually this is missing test coverage. See testEmbedFail() for an
> example of how to add it for this function.
> 
> Use 'binman test -T' to see test coverage.

I've extended the test in order to cover the other functions. But 
'LookupAndWriteSymbols' failed due to different error message:
"Cannot write symbols to an ELF file without Python elftools" instead of 
"Python: No module named 'elftools'"

Would your prefer to adapt the error message in 'LookupAndWriteSymbols' 
or add another 'assertIn' to cover the additional message? IMHO the 
error messages should be the same.

> 
>>       file_size = len(data)
>>       with io.BytesIO(data) as fd:
>>           elf = ELFFile(fd)
>> -        data_start = 0xffffffff;
>> -        data_end = 0;
>> -        mem_end = 0;
>> -        virt_to_phys = 0;
>> +        data_start = 0xffffffff
>> +        data_end = 0
>> +        mem_end = 0
>> +        virt_to_phys = 0
>>
>>           for i in range(elf.num_segments()):
>>               segment = elf.get_segment(i)
>> --
>> 2.30.2
>>
> 
> Regards,
> Simon


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 01/11] binman: elf: Check for ELF_TOOLS availability and remove extra semicolon
  2023-07-10 13:49     ` Lukas Funke
@ 2023-07-10 21:38       ` Simon Glass
  0 siblings, 0 replies; 19+ messages in thread
From: Simon Glass @ 2023-07-10 21:38 UTC (permalink / raw
  To: Lukas Funke; +Cc: u-boot, Quentin Schulz, Lukas Funke, Alper Nebi Yasak

Hi Lukas,

On Mon, 10 Jul 2023 at 07:49, Lukas Funke
<lukas.funke-oss@weidmueller.com> wrote:
>
> Hi Simon,
>
> On 07.07.2023 19:35, Simon Glass wrote:
> > Hi Lukas,
> >
> > On Thu, 6 Jul 2023 at 09:38, <lukas.funke-oss@weidmueller.com> wrote:
> >>
> >> From: Lukas Funke <lukas.funke@weidmueller.com>
> >>
> >> Check if elf tools are available when running DecodeElf(). Also
> >> remove superfuous semicolon at line ending.
> >>
> >> Signed-off-by: Lukas Funke <lukas.funke@weidmueller.com>
> >> Reviewed-by: Simon Glass <sjg@chromium.org>
> >> ---
> >>
> >> (no changes since v1)
> >>
> >>   tools/binman/elf.py | 10 ++++++----
> >>   1 file changed, 6 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/tools/binman/elf.py b/tools/binman/elf.py
> >> index 5816284c32..a53f4b9c4f 100644
> >> --- a/tools/binman/elf.py
> >> +++ b/tools/binman/elf.py
> >> @@ -438,13 +438,15 @@ def DecodeElf(data, location):
> >>       Returns:
> >>           ElfInfo object containing information about the decoded ELF file
> >>       """
> >> +    if not ELF_TOOLS:
> >> +        raise ValueError("Python: No module named 'elftools'")
> >
> > Actually this is missing test coverage. See testEmbedFail() for an
> > example of how to add it for this function.
> >
> > Use 'binman test -T' to see test coverage.
>
> I've extended the test in order to cover the other functions. But
> 'LookupAndWriteSymbols' failed due to different error message:
> "Cannot write symbols to an ELF file without Python elftools" instead of
> "Python: No module named 'elftools'"
>
> Would your prefer to adapt the error message in 'LookupAndWriteSymbols'
> or add another 'assertIn' to cover the additional message? IMHO the
> error messages should be the same.

Yes that's fine, either way. So long as there is test coverage we are good.

Regards,
Simon

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2023-07-10 21:38 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-07-06  8:38 [PATCH v2 00/11] Sign Xilinx ZynqMP SPL/FSBL boot images using binman lukas.funke-oss
2023-07-06  8:38 ` [PATCH v2 01/11] binman: elf: Check for ELF_TOOLS availability and remove extra semicolon lukas.funke-oss
2023-07-07 17:35   ` Simon Glass
2023-07-10 13:49     ` Lukas Funke
2023-07-10 21:38       ` Simon Glass
2023-07-06  8:38 ` [PATCH v2 02/11] binman: Don't decompress data while signing lukas.funke-oss
2023-07-06  8:38 ` [PATCH v2 03/11] binman: blob_dtb: Add fake_size argument to ObtainContents() lukas.funke-oss
2023-07-06  8:38 ` [PATCH v2 04/11] binman: doc: Add documentation for fdt_add_pubkey bintool lukas.funke-oss
2023-07-06  8:38 ` [PATCH v2 05/11] binman: ftest: Add test for u_boot_spl_pubkey_dtb lukas.funke-oss
2023-07-06  8:38 ` [PATCH v2 06/11] binman: btool: Add fdt_add_pubkey as btool lukas.funke-oss
2023-07-06  8:38 ` [PATCH v2 07/11] binman: etype: Add u_boot_spl_pubkey_dtb etype lukas.funke-oss
2023-07-06  8:38 ` [PATCH v2 08/11] binman: doc: Add documentation for Xilinx Bootgen bintool lukas.funke-oss
2023-07-06  8:38 ` [PATCH v2 09/11] binman: btool: Add Xilinx Bootgen btool lukas.funke-oss
2023-07-07 17:35   ` Simon Glass
2023-07-06  8:38 ` [PATCH v2 10/11] binman: ftest: Add test for xilinx_fsbl_auth etype lukas.funke-oss
2023-07-07 17:35   ` Simon Glass
2023-07-06  8:38 ` [PATCH v2 11/11] binman: etype: Add " lukas.funke-oss
2023-07-07 17:35   ` Simon Glass
2023-07-07 17:35 ` [PATCH v2 00/11] Sign Xilinx ZynqMP SPL/FSBL boot images using binman Simon Glass

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.