[PATCH v3 1/6] binman: ti-secure: Add support for firewalling entities

Manorit Chawdhry m-chawdhry at ti.com
Tue Oct 10 06:59:53 CEST 2023


Hi Simon,

On 11:56-20231009, Simon Glass wrote:
> Hi Manorit,
> 
> On Mon, 9 Oct 2023 at 01:43, Manorit Chawdhry <m-chawdhry at ti.com> wrote:
> >
> > Hi Simon,
> >
> > On 17:10-20231007, Simon Glass wrote:
> > > Hi Manorit,
> > >
> > > On Wed, 4 Oct 2023 at 06:32, Manorit Chawdhry <m-chawdhry at ti.com> wrote:
> > > >
> > > > We can now firewall entities while loading them through our secure
> > > > entity TIFS, the required information should be present in the
> > > > certificate that is being parsed by TIFS.
> > > >
> > > > The following commit adds the support to enable the certificates to be
> > > > generated if the firewall configurations are present in the binman
> dtsi
> > > > nodes.
> > > >
> > > > Signed-off-by: Manorit Chawdhry <m-chawdhry at ti.com>
> > > > ---
> > > >  tools/binman/btool/openssl.py   | 16 +++++++-
> > > >  tools/binman/etype/ti_secure.py | 85
> +++++++++++++++++++++++++++++++++++++++++
> > > >  tools/binman/etype/x509_cert.py |  3 +-
> > > >  3 files changed, 101 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/tools/binman/btool/openssl.py
> b/tools/binman/btool/openssl.py
> > > > index aad3b61ae27c..dff439df211f 100644
> > > > --- a/tools/binman/btool/openssl.py
> > > > +++ b/tools/binman/btool/openssl.py
> > > > @@ -82,7 +82,7 @@ imageSize              = INTEGER:{len(indata)}
> > > >          return self.run_cmd(*args)
> > > >
> > > >      def x509_cert_sysfw(self, cert_fname, input_fname, key_fname,
> sw_rev,
> > > > -                  config_fname, req_dist_name_dict):
> > > > +                  config_fname, req_dist_name_dict,
> firewall_cert_data):
> > > >          """Create a certificate to be booted by system firmware
> > > >
> > > >          Args:
> > > > @@ -94,6 +94,13 @@ imageSize              = INTEGER:{len(indata)}
> > > >              req_dist_name_dict (dict): Dictionary containing
> key-value pairs of
> > > >              req_distinguished_name section extensions, must contain
> extensions for
> > > >              C, ST, L, O, OU, CN and emailAddress
> > > > +            firewall_cert_data (dict):
> > > > +              - auth_in_place (int): The Priv ID for copying as the
> > > > +                specific host in firewall protected region
> > > > +              - num_firewalls (int): The number of firewalls in the
> > > > +                extended certificate
> > > > +              - certificate (str): Extended firewall certificate with
> > > > +                the information for the firewall configurations.
> > > >
> > > >          Returns:
> > > >              str: Tool output
> > > > @@ -121,6 +128,7 @@ basicConstraints       = CA:true
> > > >  1.3.6.1.4.1.294.1.3    = ASN1:SEQUENCE:swrv
> > > >  1.3.6.1.4.1.294.1.34   = ASN1:SEQUENCE:sysfw_image_integrity
> > > >  1.3.6.1.4.1.294.1.35   = ASN1:SEQUENCE:sysfw_image_load
> > > > +1.3.6.1.4.1.294.1.37   = ASN1:SEQUENCE:firewall
> > > >
> > > >  [ swrv ]
> > > >  swrv = INTEGER:{sw_rev}
> > > > @@ -132,7 +140,11 @@ imageSize              = INTEGER:{len(indata)}
> > > >
> > > >  [ sysfw_image_load ]
> > > >  destAddr = FORMAT:HEX,OCT:00000000
> > > > -authInPlace = INTEGER:2
> > > > +authInPlace = INTEGER:{hex(firewall_cert_data['auth_in_place'])}
> > > > +
> > > > +[ firewall ]
> > > > +numFirewallRegions = INTEGER:{firewall_cert_data['num_firewalls']}
> > > > +{firewall_cert_data['certificate']}
> > >
> > > Do we want the text above if there is no firewall info?
> > >
> >
> > Yes, keeping numFirewallRegions = INTEGER:0 is valid. The other way would
> be
> > to remove OIDs and everything that we have kept in the certificate when
> > firewall nodes are not present but that seems a bit more difficult given
> > that we have an easy fix keeping the numFirewallRegions as 0.
> 
> OK
> 
> >
> > > >  ''', file=outf)
> > > >          args = ['req', '-new', '-x509', '-key', key_fname, '-nodes',
> > > >                  '-outform', 'DER', '-out', cert_fname, '-config',
> config_fname,
> > > > diff --git a/tools/binman/etype/ti_secure.py
> b/tools/binman/etype/ti_secure.py
> > > > index d939dce57139..a7409023fa55 100644
> > > > --- a/tools/binman/etype/ti_secure.py
> > > > +++ b/tools/binman/etype/ti_secure.py
> > > > @@ -7,9 +7,35 @@
> > > >
> > > >  from binman.entry import EntryArg
> > > >  from binman.etype.x509_cert import Entry_x509_cert
> > > > +from dataclasses import dataclass
> > > >
> > > >  from dtoc import fdt_util
> > > >
> > > > + at dataclass
> > > > +class Firewall():
> > > > +    id: int
> > > > +    region: int
> > > > +    control : int
> > > > +    permissions: list[hex]
> > > > +    start_address: str
> > > > +    end_address: str
> > > > +
> > > > +    def get_certificate(self) -> str:
> > > > +        unique_identifier = f"{self.id}{self.region}"
> > > > +        cert = f"""
> > > > +firewallID{unique_identifier} = INTEGER:{self.id}
> > > > +region{unique_identifier} = INTEGER:{self.region}
> > > > +control{unique_identifier} = INTEGER:{hex(self.control)}
> > > > +nPermissionRegs{unique_identifier} = INTEGER:{len(self.permissions)}
> > > > +"""
> > > > +        for index, permission in enumerate(self.permissions):
> > > > +            cert += f"""permissions{unique_identifier}{index} =
> INTEGER:{hex(permission)}
> > > > +"""
> > > > +        cert += f"""startAddress{unique_identifier} =
> FORMAT:HEX,OCT:{self.start_address:02x}
> > > > +endAddress{unique_identifier} = FORMAT:HEX,OCT:{self.end_address:02x}
> > > > +"""
> > > > +        return cert
> > > > +
> > > >  class Entry_ti_secure(Entry_x509_cert):
> > > >      """Entry containing a TI x509 certificate binary
> > > >
> > > > @@ -17,6 +43,11 @@ class Entry_ti_secure(Entry_x509_cert):
> > > >          - content: List of phandles to entries to sign
> > > >          - keyfile: Filename of file containing key to sign binary
> with
> > > >          - sha: Hash function to be used for signing
> > > > +        - auth_in_place: This is an integer field that contains two
> pieces
> > > > +          of information
> > > > +            Lower Byte - Remains 0x02 as per our use case
> > > > +            ( 0x02: Move the authenticated binary back to the header
> )
> > > > +            Upper Byte - The Host ID of the core owning the firewall
> > > >
> > > >      Output files:
> > > >          - input.<unique_name> - input file passed to openssl
> > > > @@ -25,6 +56,35 @@ class Entry_ti_secure(Entry_x509_cert):
> > > >          - cert.<unique_name> - output file generated by openssl
> (which is
> > > >            used as the entry contents)
> > > >
> > > > +    Depending on auth_in_place information in the inputs, we read the
> > > > +    firewall nodes that describe the configurations of firewall that
> TIFS
> > > > +    will be doing after reading the certificate.
> > > > +
> > > > +    The syntax of the firewall nodes are as such:
> > > > +
> > > > +    firewall-257-0 {
> > > > +        id = <257>;           /* The ID of the firewall being
> configured */
> > > > +        region = <0>;         /* Region number to configure */
> > > > +
> > > > +        control =             /* The control register */
> > > > +            <(FWCTRL_EN | FWCTRL_LOCK | FWCTRL_BG | FWCTRL_CACHE)>;
> > > > +
> > > > +        permissions =         /* The permission registers */
> > > > +            <((FWPRIVID_ALL << FWPRIVID_SHIFT) |
> > > > +                        FWPERM_SECURE_PRIV_RWCD |
> > > > +                        FWPERM_SECURE_USER_RWCD |
> > > > +                        FWPERM_NON_SECURE_PRIV_RWCD |
> > > > +                        FWPERM_NON_SECURE_USER_RWCD)>;
> > > > +
> > > > +        /* More defines can be found in k3-security.h */
> > > > +
> > > > +        start_address =        /* The Start Address of the firewall
> */
> > > > +            <0x0 0x0>;
> > > > +        end_address =          /* The End Address of the firewall */
> > > > +            <0xff 0xffffffff>;
> > > > +    };
> > > > +
> > > > +
> > > >      openssl signs the provided data, using the TI templated config
> file and
> > > >      writes the signature in this entry. This allows verification
> that the
> > > >      data is genuine.
> > > > @@ -32,11 +92,20 @@ class Entry_ti_secure(Entry_x509_cert):
> > > >      def __init__(self, section, etype, node):
> > > >          super().__init__(section, etype, node)
> > > >          self.openssl = None
> > > > +        self.firewall_cert_data: dict = {
> > > > +            'auth_in_place': 0x02,
> > > > +            'num_firewalls': 0,
> > > > +            'certificate': "",
> > > > +        }
> > > >
> > > >      def ReadNode(self):
> > > >          super().ReadNode()
> > > >          self.key_fname = self.GetEntryArgsOrProps([
> > > >              EntryArg('keyfile', str)], required=True)[0]
> > > > +        auth_in_place = fdt_util.GetInt(self._node, "auth_in_place")
> > >
> > > Please use hyphens in properties. If that is a bool, use GetBool()
> >
> > I will use hyphens, the property is supposed to be int. Maybe doesn't
> > convey what it really is by the name but hoping the documentation around
> > it makes it clear.
> 
> No, but you can't use a property name in the DT called 'auth_in_place'
> (please use single quotes BTW). It should have hyphens. Of course the
> Python var will have underscore. but not the property.
> 

I think there has been some miscommunication but I believe we are both
in agreement for the change unless I understood something incorrectly,
would be sending a v4 with the change.

Thanks and Regards,
Manorit

> >
> > >
> > >
> > > > +        if auth_in_place:
> > > > +            self.firewall_cert_data['auth_in_place'] = auth_in_place
> > > > +            self.ReadFirewallNode()
> > > >          self.sha = fdt_util.GetInt(self._node, 'sha', 512)
> > > >          self.req_dist_name = {'C': 'US',
> > > >                  'ST': 'TX',
> > > > @@ -46,6 +115,22 @@ class Entry_ti_secure(Entry_x509_cert):
> > > >                  'CN': 'TI Support',
> > > >                  'emailAddress': 'support at ti.com'}
> > > >
> > > > +    def ReadFirewallNode(self):
> > > > +        self.firewall_cert_data['certificate'] = ""
> > > > +        self.firewall_cert_data['num_firewalls'] = 0
> > > > +        for node in self._node.subnodes:
> > > > +            if 'firewall' in node.name:
> > > > +                firewall = Firewall(
> > > > +                     fdt_util.GetInt(node, 'id'),
> > > > +                     fdt_util.GetInt(node, 'region'),
> > > > +                     fdt_util.GetInt(node, 'control'),
> > > > +                     fdt_util.GetPhandleList(node, 'permissions'),
> > > > +                     fdt_util.GetInt64(node, 'start_address'),
> > > > +                     fdt_util.GetInt64(node, 'end_address'),
> > >
> > > What happens if some of these properties are missing?
> >
> > Should be an error as the firewall would'nt be configurable without
> > these properties. Though am not sure why am not getting an error while
> > passing None to the dataclass, ideally it should..
> >
> > Regards,
> > Manorit
> > >
> > > > +                )
> > > > +                self.firewall_cert_data['num_firewalls'] += 1
> > > > +                self.firewall_cert_data['certificate'] +=
> firewall.get_certificate()
> > > > +
> > > >      def GetCertificate(self, required):
> > > >          """Get the contents of this entry
> > > >
> > > > diff --git a/tools/binman/etype/x509_cert.py
> b/tools/binman/etype/x509_cert.py
> > > > index d028cfe38cd9..9e1cf479023b 100644
> > > > --- a/tools/binman/etype/x509_cert.py
> > > > +++ b/tools/binman/etype/x509_cert.py
> > > > @@ -98,7 +98,8 @@ class Entry_x509_cert(Entry_collection):
> > > >                  key_fname=self.key_fname,
> > > >                  config_fname=config_fname,
> > > >                  sw_rev=self.sw_rev,
> > > > -                req_dist_name_dict=self.req_dist_name)
> > > > +                req_dist_name_dict=self.req_dist_name,
> > > > +                firewall_cert_data=self.firewall_cert_data)
> > > >          elif type == 'rom':
> > > >              stdout = self.openssl.x509_cert_rom(
> > > >                  cert_fname=output_fname,
> > > >
> > > > --
> > > > 2.41.0
> > > >
> > >
> > > Regards,
> > > Simon
> 
> Regards,
> Simon


More information about the U-Boot mailing list