Hi Simon, On 11:56-20231009, Simon Glass wrote: > Hi Manorit, > > On Mon, 9 Oct 2023 at 01:43, Manorit Chawdhry <m-chawd...@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-chawd...@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-chawd...@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 > > > > > > > > +@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': 'supp...@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