Hi Simon,

On 28/02/23 06:05, Simon Glass wrote:
Hi Neha,

On Fri, 24 Feb 2023 at 05:03, Neha Malcom Francis <n-fran...@ti.com> wrote:

core-secdev-k3 is the TI security development package provided for K3
platform devices. This tool helps sign bootloader images with the x509
ceritificate header.

Signed-off-by: Neha Malcom Francis <n-fran...@ti.com>
---
This patch depends on 
https://patchwork.ozlabs.org/project/uboot/patch/20230224115101.563729-1-n-fran...@ti.com/
and on ongoing development in 
https://git.ti.com/cgit/security-development-tools/core-secdev-k3

  tools/binman/btool/tisecuretool.py            |  72 +++++++++++
  tools/binman/etype/ti_secure.py               | 114 ++++++++++++++++++
  tools/binman/ftest.py                         |  36 ++++++
  tools/binman/test/278_ti_secure_rom.dts       |  11 ++
  tools/binman/test/279_ti_secure.dts           |  11 ++
  .../binman/test/280_ti_secure_nofilename.dts  |  10 ++
  tools/binman/test/281_ti_secure_combined.dts  |  12 ++
  7 files changed, 266 insertions(+)
  create mode 100644 tools/binman/btool/tisecuretool.py
  create mode 100644 tools/binman/etype/ti_secure.py
  create mode 100644 tools/binman/test/278_ti_secure_rom.dts
  create mode 100644 tools/binman/test/279_ti_secure.dts
  create mode 100644 tools/binman/test/280_ti_secure_nofilename.dts
  create mode 100644 tools/binman/test/281_ti_secure_combined.dts

Now that I see what you are doing, this it not quite the right way.

See this hack-up of how you can call the openssl thing. Basically you
should not have a shell script in the way, but instead make your
bintool do it.

https://github.com/sjg20/u-boot/commit/03c0d74f81106570b18d8e4fe7a3355bfeb0d5da#r100378804

I suppose we can have an openssl bintool that others build on top of?

Regards,
Simon



That is possible, maybe ti-secure extends from x509_cert etype so as to support the TI specific certificate extensions. I'll start working on this.

However the patches I've sent support external production flow where signing need not necessarily be carried out from U-Boot. An external repo that acts as what is core-secdev-k3 here, would be the repo responsible for signing.

Could we find a way to combine both so as to support production flow mandating an external agency, as well as a completely within U-Boot flow using bintool? i.e. we modify ti-secure etype to be able to support both external git repo/executable script signing as well as default signing using openssl bintool.


diff --git a/tools/binman/btool/tisecuretool.py 
b/tools/binman/btool/tisecuretool.py
new file mode 100644
index 0000000000..5102bb1f7d
--- /dev/null
+++ b/tools/binman/btool/tisecuretool.py
@@ -0,0 +1,72 @@
+# SPDX-License-Identifier: GPL-2.0+
+# Copyright (c) 2022 Texas Instruments Incorporated - https://www.ti.com/
+# Written by Neha Malcom Francis <n-fran...@ti.com>
+#
+"""Bintool implementation for TI security development tools
+
+tisecuretool helps add x509 certification for bootloader images for K3 
platform devices
+
+Source code:
+https://git.ti.com/cgit/security-development-tools/core-secdev-k3/""";
+
+import os
+
+from binman import bintool
+from patman import tout
+from patman import tools
+
+class Bintooltisecuretool(bintool.Bintool):
+    """Signing tool for TI bootloaders"""
+    name = 'tisecuretool'
+    def __init__(self, name):
+        super().__init__(name, 'TI secure tool')
+
+    def sign_binary_secure(self, fname, out_fname):
+        """Create a signed binary
+
+        Args:
+            fname (str): Filename to sign
+            out_fname (str): Output filename
+
+        Returns:
+            str: Tool output
+            or None
+        """
+        tool_path = self.get_path()
+        script_path = os.path.join(tool_path, 'scripts/secure-binary-image.sh')
+        args = [
+            'sh',
+            script_path,
+            fname,
+            out_fname
+            ]
+        output = self.run_cmd(*args, add_name=False)
+        return output
+
+    def sign_binary_rom(self, args):
+        """Create a signed binary that is booted by ROM
+
+        Args:
+            fname (str): Filename to sign
+            out_fname (str): Output filename"""
+        tool_path = self.get_path()
+        script_path = os.path.join(tool_path, 
'scripts/secure-rom-boot-image.sh')
+        #args.insert(0, script_path)
+        args.insert(0, script_path)
+        output = self.run_cmd(*args, add_name=False)
+        return output
+
+    def fetch(self, method):
+        """Fetch handler for TI secure tool
+
+        This builds the tool from source
+
+        Returns:
+            True if the file was fetched, None if a method other than 
FETCH_SOURCE
+            was requested
+        """
+        if method != bintool.FETCH_SOURCE:
+            return None
+        result = self.fetch_from_git(
+            'git://git.ti.com/security-development-tools/core-secdev-k3.git', 
'tisecuretool')
+        return result
diff --git a/tools/binman/etype/ti_secure.py b/tools/binman/etype/ti_secure.py
new file mode 100644
index 0000000000..26f81ff8e8
--- /dev/null
+++ b/tools/binman/etype/ti_secure.py
@@ -0,0 +1,114 @@
+# SPDX-License-Identifier: GPL-2.0+
+# Copyright (c) 2022 Texas Instruments Incorporated - https://www.ti.com/
+# Written by Neha Malcom Francis <n-fran...@ti.com>
+#
+# Entry-type module for signed binaries for TI K3 platform
+#
+
+from binman.etype.blob import Entry_blob
+from binman import bintool
+
+from dtoc import fdt_util
+from patman import terminal
+from patman import tools
+from patman import tout
+
+class Entry_ti_secure(Entry_blob):
+    """An entry which contains a signed x509 binary for signing TI
+    General Purpose as well as High-Security devices.
+
+    Properties / Entry arguments:
+       - filename: filename of binary file to be secured
+
+    Output files:
+        - filename_x509 - output file generated by secure x509 signing script 
(which
+            used as entry contents)
+    """
+    def __init__(self, section, etype, node):
+        super().__init__(section, etype, node)
+        self.filename = fdt_util.GetString(self._node, 'filename')
+        self.core = fdt_util.GetString(self._node, 'core', 'secure')
+        self.load_addr = fdt_util.GetInt(self._node, 'load', 0x41c00000)
+        self.sw_rev = fdt_util.GetInt(self._node, 'sw-rev', 1)
+        self.sysfw_cert = fdt_util.GetBool(self._node, 'sysfw-cert', False)
+        self.secure = fdt_util.GetBool(self._node, 'secure', False)
+        self.combined = fdt_util.GetBool(self._node, 'combined', False)
+        self.split_dm = fdt_util.GetBool(self._node, 'split-dm', False)
+        self.sysfw_filename = fdt_util.GetString(self._node, 'sysfw-filename')
+        self.sysfw_load_addr = fdt_util.GetInt(self._node, 'sysfw-load', 
0x40000)
+        self.sysfw_data_filename = fdt_util.GetString(self._node, 
'sysfw-data-filename')
+        self.sysfw_data_load_addr = fdt_util.GetInt(self._node, 
'sysfw-data-load', 0x7f000)
+        self.sysfw_inner_cert = fdt_util.GetString(self._node, 'sysfw-inner-cert', 
"")
+        self.dm_data_filename = fdt_util.GetString(self._node, 
'dm-data-filename')
+        self.dm_data_load_addr = fdt_util.GetInt(self._node, 'dm-data-load', 
0x41c80000)
+        self.sysfw_inner_cert_filename = fdt_util.GetString(self._node, 
'sysfw-inner-cert-filename')
+        self.toolpresent = False
+        if not self.filename:
+            self.Raise("ti_secure must have a 'filename' property")
+
+    def _SignBinarySecure(self):
+        infilename = self.filename
+        outfilename = infilename + '_signed'
+
+        data = self.tisecuretool.sign_binary_secure(infilename, outfilename)
+
+        if data is None:
+            # Bintool is missing, create a zeroed binary
+            self.record_missing_bintool(self.tisecuretool)
+            return tools.get_bytes(0, 1024)
+
+        return tools.read_file(outfilename)
+
+    def _SignBinaryROM(self):
+        input_fname = self.filename
+        output_fname = input_fname + "_x509"
+        if self.combined:
+            args = [
+                '-b', input_fname,
+                '-l', hex(self.load_addr),
+                '-s', self.sysfw_filename,
+                '-m', hex(self.sysfw_load_addr),
+                '-a', self.sysfw_inner_cert,
+                '-d', self.sysfw_data_filename,
+                '-n', hex(self.sysfw_data_load_addr),
+                '-r', str(self.sw_rev),
+                '-o', output_fname,
+            ]
+            if self.split_dm:
+                args.extend(['-t', self.dm_data_filename, '-y', 
hex(self.dm_data_load_addr)])
+        else:
+            args = [
+                '-a', self.core,
+                '-b', input_fname,
+                '-o', output_fname,
+                '-l', hex(self.load_addr),
+                '-r', str(self.sw_rev),
+
+            ]
+            if self.sysfw_cert == True:
+                args.insert(0, '-u')
+
+        out = self.tisecuretool.sign_binary_rom(args)
+
+        if out is None:
+            # Bintool is missing, create a zeroed binary
+            self.record_missing_bintool(self.tisecuretool)
+            return tools.get_bytes(0, 1024)
+
+        return tools.read_file(output_fname)
+
+    def ProcessContents(self):
+        if self.secure:
+            data = self._SignBinarySecure()
+        else:
+            data = self._SignBinaryROM()
+        return self.ProcessContentsUpdate(data)
+
+    def AddBintools(self, btools):
+        #super().AddBintools(btools)
+        col = terminal.Color()
+        self.tisecuretool = self.AddBintool(btools, 'tisecuretool')
+        out = self.tisecuretool.fetch_tool(bintool.FETCH_SOURCE, col, True)
+        if out == bintool.FAIL:
+            # Bintool is missing, record missing
+            self.record_missing_bintool(self.tisecuretool)
\ No newline at end of file
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index bf902341c5..72cce0ef02 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -97,6 +97,7 @@ PRE_LOAD_MAGIC        = b'UBSH'
  PRE_LOAD_VERSION      = 0x11223344.to_bytes(4, 'big')
  PRE_LOAD_HDR_SIZE     = 0x00001000.to_bytes(4, 'big')
  TI_BOARD_CONFIG_DATA  = b'\x00\x01'
+TIX509DATA            = b'letsgo'

  # Subdirectory of the input dir to use to put test FDTs
  TEST_FDT_SUBDIR       = 'fdts'
@@ -206,6 +207,7 @@ class TestFunctional(unittest.TestCase):
          TestFunctional._MakeInputFile('bl2u.bin', ATF_BL2U_DATA)
          TestFunctional._MakeInputFile('fw_dynamic.bin', OPENSBI_DATA)
          TestFunctional._MakeInputFile('scp.bin', SCP_DATA)
+        TestFunctional._MakeInputFile('to_sign.bin', TIX509DATA)

          # Add a few .dtb files for testing
          TestFunctional._MakeInputFile('%s/test-fdt1.dtb' % TEST_FDT_SUBDIR,
@@ -6398,5 +6400,39 @@ fdt         fdtmap                Extract the devicetree 
blob from the fdtmap
          configlen_noheader = TI_BOARD_CONFIG_DATA*4
          self.assertGreater(data, configlen_noheader)

+    def testTISecureROMImageMissingTool(self):
+        """Test that a TI secure signed ROM booted image can be generated although 
tisecuretool is missing"""
+        with test_util.capture_sys_output() as (_, stderr):
+            data = self._DoTestFile('278_ti_secure_rom.dts',
+                force_missing_bintools='tisecuretool')
+        err = stderr.getvalue()
+        self.assertRegex(err,
+                         "Image 'image'.*missing bintools.*: tisecuretool")
+
+    def testTISecureImageMissingTool(self):
+        """Test that a TI secure signed image can be generated although tisecuretool is 
missing"""
+        with test_util.capture_sys_output() as (_, stderr):
+            data = self._DoTestFile('279_ti_secure.dts',
+                force_missing_bintools='tisecuretool')
+        err = stderr.getvalue()
+        self.assertRegex(err,
+                         "Image 'image'.*missing bintools.*: tisecuretool")
+
+    def testTISecureNoFilename(self):
+        """Test that a missing 'filename' property is detected"""
+        with self.assertRaises(ValueError) as e:
+            self._DoTestFile('280_ti_secure_nofilename.dts')
+        self.assertIn("ti_secure must have a 'filename' property",
+                      str(e.exception))
+
+    def testTISecureImageCombinedMissingTool(self):
+        """Test that a TI secure signed image can be generated although tisecuretool is 
missing"""
+        with test_util.capture_sys_output() as (_, stderr):
+            data = self._DoTestFile('281_ti_secure_combined.dts',
+                force_missing_bintools='tisecuretool')
+        err = stderr.getvalue()
+        self.assertRegex(err,
+                         "Image 'image'.*missing bintools.*: tisecuretool")
+
  if __name__ == "__main__":
      unittest.main()
diff --git a/tools/binman/test/278_ti_secure_rom.dts 
b/tools/binman/test/278_ti_secure_rom.dts
new file mode 100644
index 0000000000..67a6a9de8d
--- /dev/null
+++ b/tools/binman/test/278_ti_secure_rom.dts
@@ -0,0 +1,11 @@
+// SPDX-License-Identifier: GPL-2.0+
+/dts-v1/;
+
+/ {
+       binman {
+                       ti-secure {
+                               filename = "to_sign.bin";
+                               sysfw-cert;
+                       };
+       };
+};
diff --git a/tools/binman/test/279_ti_secure.dts 
b/tools/binman/test/279_ti_secure.dts
new file mode 100644
index 0000000000..0e60984013
--- /dev/null
+++ b/tools/binman/test/279_ti_secure.dts
@@ -0,0 +1,11 @@
+// SPDX-License-Identifier: GPL-2.0+
+/dts-v1/;
+
+/ {
+       binman {
+                       ti-secure {
+                               filename = "to_sign.bin";
+                               secure;
+                       };
+       };
+};
diff --git a/tools/binman/test/280_ti_secure_nofilename.dts 
b/tools/binman/test/280_ti_secure_nofilename.dts
new file mode 100644
index 0000000000..928a50a499
--- /dev/null
+++ b/tools/binman/test/280_ti_secure_nofilename.dts
@@ -0,0 +1,10 @@
+// SPDX-License-Identifier: GPL-2.0+
+/dts-v1/;
+
+/ {
+       binman {
+                       ti-secure {
+                               secure;
+                       };
+       };
+};
diff --git a/tools/binman/test/281_ti_secure_combined.dts 
b/tools/binman/test/281_ti_secure_combined.dts
new file mode 100644
index 0000000000..aadb5a4306
--- /dev/null
+++ b/tools/binman/test/281_ti_secure_combined.dts
@@ -0,0 +1,12 @@
+// SPDX-License-Identifier: GPL-2.0+
+/dts-v1/;
+
+/ {
+       binman {
+                       ti-secure {
+                               filename = "to_sign.bin";
+                               combined;
+                               split-dm;
+                       };
+       };
+};
--
2.34.1


--
Thanking You
Neha Malcom Francis

Reply via email to