[PATCH 03/21] tools: binman: add ti-secure entry type
Simon Glass
sjg at chromium.org
Fri Jan 20 20:46:07 CET 2023
Hi Neha,
\
On Fri, 20 Jan 2023 at 03:19, Neha Malcom Francis <n-francis at ti.com> wrote:
>
> This entry type is used to create a secured binary
> for use with K3 High Security (HS) devices.
>
> This allows us to no longer depend on k3_fit_atf.sh for
> A53 SPL and u-boot image generation even for HS devices.
>
> We still depend on the availability of an external
> tool provided by the TI_SECURE_DEV_PKG environment
> variable to secure the binaries.
>
> Signed-off-by: Roger Quadros <rogerq at kernel.org>
> [n-francis at ti.com: enabled signing for all K3 boot binaries for all
> different boot flows]
> Signed-off-by: Neha Malcom Francis <n-francis at ti.com>
> ---
> Makefile | 1 +
> tools/binman/entries.rst | 15 ++++
> tools/binman/etype/ti_secure.py | 133 ++++++++++++++++++++++++++++++++
> tools/binman/ftest.py | 8 ++
> 4 files changed, 157 insertions(+)
> create mode 100644 tools/binman/etype/ti_secure.py
>
Sorry, I have rather a lot of comments, so will do a round of just the
major ones for this version.
> diff --git a/Makefile b/Makefile
> index eb354c045c..c568a6e59a 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1329,6 +1329,7 @@ cmd_binman = $(srctree)/tools/binman/binman $(if $(BINMAN_DEBUG),-D) \
> $(foreach f,$(BINMAN_INDIRS),-I $(f)) \
> -a atf-bl31-path=${BL31} \
> -a tee-os-path=${TEE} \
> + -a ti-secure-dev-pkg-path=${TI_SECURE_DEV_PKG} \
Cam we use ti-secure-path ? The 'dev' and 'pkg' seem to be noise.
> -a opensbi-path=${OPENSBI} \
> -a default-dt=$(default_dt) \
> -a scp-path=$(SCP) \
> diff --git a/tools/binman/entries.rst b/tools/binman/entries.rst
> index 2b32c131ed..bf363434a2 100644
> --- a/tools/binman/entries.rst
> +++ b/tools/binman/entries.rst
> @@ -2361,3 +2361,18 @@ may be used instead.
>
>
>
> +Entry: ti-secure: Entry containing a Secured binary blob
> +--------------------------------------------------------
> +
> +Properties / Entry arguments:
> + - filename: Filename of file to sign and read into entry
> +
> +Texas Instruments High-Security (HS) devices need secure binaries to be
> +provided. This entry uses an external tool to append a x509 certificate
> +to the file provided in the filename property and places it in the entry.
> +
> +The path for the external tool is fetched from TI_SECURE_DEV_PKG
> +environment variable.
> +
> +
> +
> diff --git a/tools/binman/etype/ti_secure.py b/tools/binman/etype/ti_secure.py
> new file mode 100644
> index 0000000000..5447bb61df
> --- /dev/null
> +++ b/tools/binman/etype/ti_secure.py
> @@ -0,0 +1,133 @@
> +# SPDX-License-Identifier: GPL-2.0+
> +# Copyright (c) 2022 Texas Instruments Incorporated - https://www.ti.com/
> +#
> +
> +# Support for signed binaries for TI K3 platform
> +
> +from collections import OrderedDict
> +import os
> +
> +from binman.entry import Entry, EntryArg
> +
> +from dtoc import fdt_util
> +from patman import tools
> +
> +class Entry_ti_secure(Entry):
So shouldn't this be Entry_blob ?
> + """An entry which contains a signed x509 binary for signing TI
> + General Purpose as well as High-Security devices.
First line of comment must be the summary. Then add a blank line and
the extra detail. See how this is done elsewhere
> +
> + Properties / Entry arguments:
> + - filename: filename of binary file to be secured
Please describe all the properties
> +
> + 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.key = fdt_util.GetString(self._node, 'key', "")
If key is '' does it work?
Please use single quotes consistently except for function comments.
> + self.core = fdt_util.GetInt(self._node, 'core', 16)
> + self.load_addr = fdt_util.GetInt(self._node, 'load', 0x41c00000)
> + self.sw_rev = fdt_util.GetInt(self._node, 'sw-rev')
> + self.cert3 = fdt_util.GetBool(self._node, 'sysfw-cert', False)
try to use the same name for the property as the DT one if you can
> + 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')
> + 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')
> + 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')
> + self.sysfw_inner_cert_filename = fdt_util.GetString(self._node, 'sysfw-inner-cert-filename')
> + self.sysfw_inner_cert_load_addr = fdt_util.GetInt(self._node, 'sysfw-inner-cert-load')
> + self.toolpresent = False
> + if not self.filename:
> + self.Raise("ti_secure must have a 'filename' property")
This is handled by putting something like this in __init__
self.required_props = ['filename']
> + self.toolspath, = self.GetEntryArgsOrProps(
> + [EntryArg('ti-secure-dev-pkg-path', str)])
> + if not self.toolspath:
> + print("WARNING: TI_SECURE_DEV_PKG environment " \
> + "variable must be defined for TI GP and HS devices! " +
> + self.filename + " was NOT signed!")
> + return
This is handled by the bintool being 'missing'.
See how the other bintools are implemented. Create a bintool for your tool.
> +
> + if self.cert3 == True:
> + self.tool = self.toolspath + "/scripts/gen_x509_cert3.sh"
> + self.core = "m3"
> + elif self.secure == True:
> + self.tool = self.toolspath + "/scripts/secure-binary-image.sh"
> + elif self.combined:
> + self.tool = self.toolspath + "/scripts/gen_x509_combined_cert.sh"
> + else:
> + self.tool = self.toolspath + "/scripts/gen_x509_cert.sh"
These need to be bintools too, or perhaps other entry types. I'm not sure...
> + self.toolpresent = os.path.exists(self.tool)
> + if not self.toolpresent:
> + print(self.tool + " not found. " +
> + self.filename + " was NOT signed! ")
> +
> + if self.key == "" and not self.secure:
> + self.key = self.toolspath + "/keys/ti-degenerate-key.pem"
> + self.keypresent = os.path.exists(self.key)
> + if not self.keypresent:
> + print(self.key + " not found. " +
> + self.filename + " was NOT signed! ")
> + else:
> + print("Signing " + self.filename + " with degenerate RSA key...")
> + else:
> + self.key = self.toolspath + self.key
> + print("Signing " + self.filename + " with " + self.key)
You can't use 'print' in binman. You can use things like tout.info()
to emit info.
> +
> + if self.sw_rev is None and not self.secure:
> + self.sw_revfile = self.toolspath + "/keys/swrv.txt"
> + with open(self.sw_revfile) as f:
> + self.sw_rev = int(f.read())
> + self.swrevpresent = os.path.exists(self.sw_rev)
> + if not self.swrevpresent:
> + print(self.sw_rev + " not found. " +
> + "Software revision file not found. Default may not work on HS hardware.")
> + self.sw_rev = 1
> +
> + def ObtainContents(self):
> + input_fname = self.filename
> + output_fname = input_fname + "_x509"
> + if self.secure:
> + args = [
> + input_fname, output_fname,
> + ]
> + elif self.combined:
> + args = [
> + '-b', input_fname,
> + '-l', hex(self.load_addr),
> + '-s', self.sysfw_filename,
> + '-m', hex(self.sysfw_load_addr),
> + '-c', self.sysfw_inner_cert,
> + '-d', self.sysfw_data_filename,
> + '-n', hex(self.sysfw_data_load_addr),
> + '-k', self.key,
> + '-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 = [
> + '-c', str(self.core),
> + '-b', input_fname,
> + '-o', output_fname,
> + '-l', hex(self.load_addr),
> + '-r', str(self.sw_rev),
> + '-k', self.key,
> + ]
> + if self.cert3 == True:
nit: use:
if self.cert3:
> + args.insert(0, '-d')
> + if self.toolpresent:
> + stdout = tools.run(self.tool, *args)
> + else:
> + stdout = tools.run('cp', *args)
> + print(output_fname + ' not signed!')
> +
> + self.SetContents(tools.read_file(output_fname))
> + return True
> diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
> index be0aea49ce..aaa2c610b0 100644
> --- a/tools/binman/ftest.py
> +++ b/tools/binman/ftest.py
> @@ -93,6 +93,7 @@ SCP_DATA = b'scp'
> TEST_FDT1_DATA = b'fdt1'
> TEST_FDT2_DATA = b'test-fdt2'
> ENV_DATA = b'var1=1\nvar2="2"'
> +TI_UNSECURE_DATA = b'this is some unsecure data'
> PRE_LOAD_MAGIC = b'UBSH'
> PRE_LOAD_VERSION = 0x11223344.to_bytes(4, 'big')
> PRE_LOAD_HDR_SIZE = 0x00001000.to_bytes(4, 'big')
> @@ -213,6 +214,7 @@ class TestFunctional(unittest.TestCase):
> TEST_FDT2_DATA)
>
> TestFunctional._MakeInputFile('env.txt', ENV_DATA)
> + TestFunctional._MakeInputFile('ti_unsecure.bin', TI_UNSECURE_DATA)
>
> # ELF file with two sections in different parts of memory, used for both
> # ATF and OP_TEE
> @@ -5545,6 +5547,12 @@ fdt fdtmap Extract the devicetree blob from the fdtmap
> err)
>
>
> + def testPackTisecure(self):
> + """Test that an image with a TI secured binary can be created"""
> + data = self._DoReadFile('187_ti_secure.dts')
> + securedata = tools.ReadFile('ti_unsecure.bin_HS')
> + self.assertEquals(data, securedata)
> +
The test needs to cover the tool being missing.
> def testFitSplitElfMissing(self):
> """Test an split-elf FIT with a missing ELF file"""
> if not elf.ELF_TOOLS:
> --
> 2.34.1
>
Regards,
Simon
More information about the U-Boot
mailing list