[PATCH v4 07/11] binman: openssl: Add boot and load extensions to x509 cert

[Simon Glass]

Hi Beleswar,

On 2026-04-25T03:37:39, Padhi, Beleswar <b-padhi at ti.com> wrote:
> binman: openssl: Add boot and load extensions to x509 cert
>
> The boot and load extensions in the x509 certificate are required for
> requesting the secure entity (TIFS) to boot a core. These fields are
> defined in the binman node for each core that must be booted by TIFS
> and must be included when generating the signed certificate.
>
> Add support to parse the boot and load extension properties from the
> binman node and populate them into the certificate. If any of the
> mandatory properties for an extension are missing, that respective
> extension section is NOT added to the certificate.
>
> Signed-off-by: Beleswar Padhi <b-padhi at ti.com>
>
> tools/binman/btool/openssl.py   | 49 +++++++++++++++++++++++++++++++++++++----
>  tools/binman/etype/ti_secure.py | 18 +++++++++++++++
>  tools/binman/etype/x509_cert.py |  4 +++-
>  3 files changed, 66 insertions(+), 5 deletions(-)

> diff --git a/tools/binman/btool/openssl.py b/tools/binman/btool/openssl.py
> @@ -101,12 +102,52 @@ imageSize              = INTEGER:{len(indata)}
> +        if boot_ext_data is not None:
> +            boot_ext = f'''
> +[ sysfw_boot_seq ]
> +bootCore = INTEGER:{boot_ext_data['proc_id']}
> +bootCoreOpts_set = INTEGER:{boot_ext_data['flags_set']}
> +bootCoreOpts_clr = INTEGER:{boot_ext_data['flags_clr']}
> +resetVec = FORMAT:HEX,OCT:{boot_ext_data['reset_vector']:08x}

Does this section actually make it into the certificate? The test in
patch 8 only checks the file is non-empty, so it would not catch this.

> diff --git a/tools/binman/btool/openssl.py b/tools/binman/btool/openssl.py
> @@ -101,12 +102,52 @@ imageSize              = INTEGER:{len(indata)}
> +rsvd1 = INTEGER:0x00
> +rsdv2 = INTEGER:0x00
> +rsdv3 = INTEGER:0x00

rsdv2 / rsdv3 should be rsvd2 / rsvd3 to match rsvd1 above.

> diff --git a/tools/binman/etype/ti_secure.py b/tools/binman/etype/ti_secure.py
> @@ -116,6 +116,7 @@ class Entry_ti_secure(Entry_x509_cert):
>          if auth_in_place:
>              self.firewall_cert_data['auth_in_place'] = auth_in_place
>              self.ReadFirewallNode()
> +        self.ReadLoadableCoreNode()

The new properties (proc_id, flags_set, flags_clr, reset_vector,
dest_addr, auth_type) are not described in the Entry_ti_secure
docstring. Please document them alongside auth-in-place, including
whether each group is optional and what the values mean.

> diff --git a/tools/binman/etype/ti_secure.py b/tools/binman/etype/ti_secure.py
> @@ -126,6 +127,23 @@ class Entry_ti_secure(Entry_x509_cert):
> +    def ReadLoadableCoreNode(self):
> +        boot_ext_props = ['proc_id', 'flags_set', 'flags_clr', 'reset_vector']
> +        load_ext_props = ['dest_addr', 'auth_type']
> +
> +        self.boot_ext = self.ReadDictFromList(boot_ext_props)
> +        self.load_ext = self.ReadDictFromList(load_ext_props)
> +
> +    def ReadDictFromList(self, props):
> +        props_dict = dict.fromkeys(props)
> +        for prop in props:
> +            val = fdt_util.GetInt(self._node, prop)
> +            if val is None:
> +                return None
> +            else:
> +                props_dict[prop] = val
> +        return props_dict

A few things:

- ReadDictFromList() is a misleading name as it reads from a node,
using a list of property names, into a dict. How about ReadIntProps()
?
- Drop the else after 'return None' — just fall through, that's the
usual Python style.
- Silently returning None on the first missing prop means a
partially-specified set (e.g. user provides proc_id and flags_set but
forgets reset_vector) is treated identically to "no boot extension
wanted". That is going to be very hard to debug. Best to raise an
explicit error listing the missing properties (similar to what
Firewall.ensure_props() does) when some but not all of the group are
present. Then you can test it too.

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

Regards,
Simon