[PATCH v2 4/4] imx: hab: Use nxp_imx8mcst etype for i.MX8M flash.bin signing

Thu May 16 17:31:22 CEST 2024

On Wed, May 15, 2024 at 6:53 PM Marek Vasut <marex at denx.de> wrote:
>
> On 5/16/24 12:31 AM, Tim Harvey wrote:
>
> Hi,
>
> > (this is a resend... apologies if its a duplicate. I got some strange
> > bounce that mime types were included so I'm resending with the otuput
> > of strace cliped out)
> >
> > strace was a good idea and showed me what was going on.
> >
> > The previous documentation stated to pass your keys via env vars that
> > were full paths to key certificates. Using strace shows me that it
> > will use the directory the KEY certificate is in and try to open up
> > ../keys/*_usr_key.pem if the key path is specified. So apparently the
> > 'File' in the CST config file is used indirectly. Pointing to the
> > usr_key.pem isn't enough either by the way, it seems to need both of
> > these:
> >
> > so if I hack the path to my certs in like this it works:diff --git
> > a/tools/binman/etype/nxp_imx8mcst.py
> > b/tools/binman/etype/nxp_imx8mcst.py
> > index 132127ad4827..b432200960df 100644
> > --- a/tools/binman/etype/nxp_imx8mcst.py
> > +++ b/tools/binman/etype/nxp_imx8mcst.py
> > @@ -67,10 +67,11 @@ class Entry_nxp_imx8mcst(Entry_mkimage):
> >
> >       def ReadNode(self):
> >           super().ReadNode()
> > +        self.certpath =3D '/usr/src/nxp/cst-3.3.2/crts/';
>
> =3D , seems like your email is acting funny today indeed.
>
> >           self.loader_address =3D fdt_util.GetInt(self._node, 'nxp,loader-ad=
> > dress')
> >           self.srk_table =3D fdt_util.GetString(self._node,
> > 'nxp,srk-table', 'SRK_1_2_3_4_table.bin')
> > -        self.csf_crt =3D fdt_util.GetString(self._node, 'nxp,csf-crt',
> > 'CSF1_1_sha256_4096_65537_v3_usr_crt.pem')
> > -        self.img_crt =3D fdt_util.GetString(self._node, 'nxp,img-crt',
> > 'IMG1_1_sha256_4096_65537_v3_usr_crt.pem')
> > +        self.csf_crt =3D fdt_util.GetString(self._node, 'nxp,csf-crt',
> > self.certpath + '/CSF1_1_sha256_4096_65537_v3_usr_crt.pem')
> > +        self.img_crt =3D fdt_util.GetString(self._node, 'nxp,img-crt',
> > self.certpath + '/IMG1_1_sha256_4096_65537_v3_usr_crt.pem')
>
> What about this:
>
> diff --git a/tools/binman/etype/nxp_imx8mcst.py
> b/tools/binman/etype/nxp_imx8mcst.py
> index 132127ad482..9ead7488a2d 100644
> --- a/tools/binman/etype/nxp_imx8mcst.py
> +++ b/tools/binman/etype/nxp_imx8mcst.py
> @@ -68,9 +68,9 @@ class Entry_nxp_imx8mcst(Entry_mkimage):
>       def ReadNode(self):
>           super().ReadNode()
>           self.loader_address = fdt_util.GetInt(self._node,
> 'nxp,loader-address')
> -        self.srk_table = fdt_util.GetString(self._node,
> 'nxp,srk-table', 'SRK_1_2_3_4_table.bin')
> -        self.csf_crt = fdt_util.GetString(self._node, 'nxp,csf-crt',
> 'CSF1_1_sha256_4096_65537_v3_usr_crt.pem')
> -        self.img_crt = fdt_util.GetString(self._node, 'nxp,img-crt',
> 'IMG1_1_sha256_4096_65537_v3_usr_crt.pem')
> +        self.srk_table = os.getenv('SRK_TABLE',
> fdt_util.GetString(self._node, 'nxp,srk-table', 'SRK_1_2_3_4_table.bin'))
> +        self.csf_crt = os.getenv('CSF_KEY',
> fdt_util.GetString(self._node, 'nxp,csf-crt',
> 'CSF1_1_sha256_4096_65537_v3_usr_crt.pem'))
> +        self.img_crt = os.getenv('IMG_KEY',
> fdt_util.GetString(self._node, 'nxp,img-crt',
> 'IMG1_1_sha256_4096_65537_v3_usr_crt.pem'))
>           self.unlock = fdt_util.GetBool(self._node, 'nxp,unlock')
>           self.ReadEntries()
>
> Then you can also use the old behavior with keys supplied via env vars.
>
> This might in fact be useful for build systems too.
>

yes, I like that (with an added 'import os')

> >           self.unlock =3D fdt_util.GetBool(self._node, 'nxp,unlock')
> >           self.ReadEntries()
> >
> > $ make -j8
> >    BINMAN  .binman_stamp
> >    OFCHK   .config
> >
> > Strace indicatest the following with the above patch:
> > openat(AT_FDCWD,
> > "/usr/src/nxp/cst-3.3.2/crts//IMG1_1_sha256_4096_65537_v3_usr_crt.pem",
> > O_RDONLY)
> > ...
> > openat(AT_FDCWD,
> > "/usr/src/nxp/cst-3.3.2/keys//IMG1_1_sha256_4096_65537_v3_usr_key.pem",
> > O_RDONLY)
> > ^^^ look how it sneakily changes the PATH!
> >
> > And without the above patch using a key file without a path:
> > openat(AT_FDCWD, "IMG1_1_sha256_4096_65537_v3_usr_crt.pem", O_RDONLY)
> > ...
> > openat(AT_FDCWD, "IMG1_1_sha256_4096_65537_v3_usr_key.pem", O_RDONLY)
> > ENOENT (No such file or directory)
> > ^^^ fails
> >
> > Simply copying both usr_crt.pem and usr_key.pem to the build directory
> > still fails:
> > binman: Error 1 running 'cst -i
> > ./nxp.csf-config-txt.section.nxp-imx8mcst at 0 -o
> > ./nxp.csf-output-blob.section.nxp-imx8mcst at 0': Error:
> > Cannot open key file IMG1_1_sha256_4096_65537_v3_usr_key.pem
> > 0:error:06065064:digital envelope routines:EVP_DecryptFinal_ex:bad
> > decrypt:crypto/evp/evp_enc.c:612:
> > 0:error:23077074:PKCS12 routines:PKCS12_pbe_crypt:pkcs12 cipherfinal
> > error:crypto/pkcs12/p12_decr.c:62:
> > 0:error:2306A075:PKCS12 routines:PKCS12_item_decrypt_d2i:pkcs12 pbe
> > crypt error:crypto/pkcs12/p12_decr.c:93:
> > 0:error:0907B00D:PEM routines:PEM_read_bio_PrivateKey:ASN1
> > lib:crypto/pem/pem_pkey.c:88:
> >
> > Do you not run into this and if not is it because you have put full
> > paths in the dtsi overriding the defaults I'm using?
>
> I just do '$ cp -Lv /CST/{keys,crts}/* .' to copy the keys and certs
> into the build directory for testing.
>
> > Maybe this has
> > something to do with how my keys were generated or the version of cst
> > I'm using or maybe we just need to also add a directory which can be
> > symlinked to or something.
>
> I use the imx-code-signing-tool 3.4.0+dfsg-2+b1 from debian .
>
> > Another thing that I'm seeing is that this leaves a bunch of turd files around:
> >          cfg-out.section.nxp-imx8mcst at 0.nxp-imx8mimage
> >          cfg-out.section.nxp-imx8mimage
> >          input.section.nxp-imx8mcst at 0
> >          input.section.nxp-imx8mcst at 0.nxp-imx8mimage
> >          input.section.nxp-imx8mimage
> >          nxp.csf-config-txt.section.nxp-imx8mcst at 0
> >          nxp.cst-input-data.section.nxp-imx8mcst at 0
> >          nxp.imx8mimage.cfg.section.nxp-imx8mcst at 0.nxp-imx8mimage
> >          nxp.imx8mimage.cfg.section.nxp-imx8mimage
> >
> > These intermediate files should be cleaned up after signing is complete.
>
> Those are intermediate build artifacts, sort of like .o files or such,
> so they should be OK to keep around, right ?

then they should be added to .gitignore and removed with a 'make
clean'. Right now they clutter up 'git status'. Maybe they can be put
in the build dir which is in .gitignore (but strangely not cleaned).

With these two things and an update to the documentation showing the
methods of specifying the keys I think everything else in the series
looks good.

Best Regards,

Tim