| Message ID | 20240507130650.713801-1-ch@denx.de |
|---|---|
| State | Changes Requested |
| Delegated to: | Marek Vasut |
| Headers | show |
| Series | imx: hab: add documentation about the required keys/certs | expand |
On 5/7/24 3:06 PM, Claudius Heine wrote: > For CST to find the certificates and keys for signing, some keys and > certs need to be copied into the u-boot build directory. Make sure to CC "NXP i.MX U-Boot Team" , else NXP is not informed. Use scripts/get_maintainer to get the full list or just reuse the CC list from patches in this thread. > diff --git a/doc/imx/habv4/guides/mx8m_spl_secure_boot.txt b/doc/imx/habv4/guides/mx8m_spl_secure_boot.txt > index ce1de659d8..42214df21a 100644 > --- a/doc/imx/habv4/guides/mx8m_spl_secure_boot.txt > +++ b/doc/imx/habv4/guides/mx8m_spl_secure_boot.txt > @@ -144,6 +144,22 @@ The signing is activated by wrapping SPL and fitImage sections into nxp-imx8mcst > etype, which is done automatically in arch/arm/dts/imx8m{m,n,p,q}-u-boot.dtsi > in case CONFIG_IMX_HAB Kconfig symbol is enabled. > > +Per default the HAB keys and certificates need to be located in the build > +directory, this means copying the following files from the HAB keys directory > +flat (e.g. removing the `keys` and `cert` subdirectory) into the u-boot build > +directory for the CST Code Signing Tool to locate them: Do symlink(s) work too ? > +- `crts/SRK_1_2_3_4_table.bin` > +- `crts/CSF1_1_sha256_4096_65537_v3_usr_crt.pem` > +- `keys/CSF1_1_sha256_4096_65537_v3_usr_key.pem` > +- `crts/IMG1_1_sha256_4096_65537_v3_usr_crt.pem` > +- `keys/IMG1_1_sha256_4096_65537_v3_usr_key.pem` > +- `keys/key_pass.txt` > + > +The paths to the SRK table and the certificates can be modified via changes to > +the nxp_imx8mcst device tree node "nodes", plural, there are two, one for SPL and one for fitImage. It would be good to mention the DT properties which govern the crypto material paths -- nxp,srk-table, nxp,csf-crt, nxp,img-crt -- somewhere around this sentence.
Hi Marek, On 2024-05-07 3:28 pm, Marek Vasut wrote: > On 5/7/24 3:06 PM, Claudius Heine wrote: >> For CST to find the certificates and keys for signing, some keys and >> certs need to be copied into the u-boot build directory. > > Make sure to CC "NXP i.MX U-Boot Team" , else NXP is not informed. Use > scripts/get_maintainer to get the full list or just reuse the CC list > from patches in this thread. I send the patch with `--to-cmd scripts/get_maintainer.pl`, maybe I should have used `--cc-cmd`, but that would not change the list of recipients. > >> diff --git a/doc/imx/habv4/guides/mx8m_spl_secure_boot.txt >> b/doc/imx/habv4/guides/mx8m_spl_secure_boot.txt >> index ce1de659d8..42214df21a 100644 >> --- a/doc/imx/habv4/guides/mx8m_spl_secure_boot.txt >> +++ b/doc/imx/habv4/guides/mx8m_spl_secure_boot.txt >> @@ -144,6 +144,22 @@ The signing is activated by wrapping SPL and >> fitImage sections into nxp-imx8mcst >> etype, which is done automatically in >> arch/arm/dts/imx8m{m,n,p,q}-u-boot.dtsi >> in case CONFIG_IMX_HAB Kconfig symbol is enabled. >> +Per default the HAB keys and certificates need to be located in the >> build >> +directory, this means copying the following files from the HAB keys >> directory >> +flat (e.g. removing the `keys` and `cert` subdirectory) into the >> u-boot build >> +directory for the CST Code Signing Tool to locate them: > > Do symlink(s) work too ? I have not tested it, but I don't see any reason why it would not. I also don't see a reason for mentioning it. I want to keep it simple, if the dev whats to do things differently, they are free to do so. > >> +- `crts/SRK_1_2_3_4_table.bin` >> +- `crts/CSF1_1_sha256_4096_65537_v3_usr_crt.pem` >> +- `keys/CSF1_1_sha256_4096_65537_v3_usr_key.pem` >> +- `crts/IMG1_1_sha256_4096_65537_v3_usr_crt.pem` >> +- `keys/IMG1_1_sha256_4096_65537_v3_usr_key.pem` >> +- `keys/key_pass.txt` >> + >> +The paths to the SRK table and the certificates can be modified via >> changes to >> +the nxp_imx8mcst device tree node > > "nodes", plural, there are two, one for SPL and one for fitImage. Well, I was thinking here more generally about the node type and was assuming that the person reading this knows how many they have of that type. But I can add a `s` in v2. > > It would be good to mention the DT properties which govern the crypto > material paths -- nxp,srk-table, nxp,csf-crt, nxp,img-crt -- somewhere > around this sentence. This is something that should be documented with the changes where that code was added, IMO. I only documented here what I found out and have used myself, I haven't used those. I would be interested in reading how to best overwrite those paths and the image structured from board u-boot.dtsi files myself. If you want to can pickup my patch and integrate it into your series and extend it. regards, Claudius
On 5/8/24 9:23 AM, Claudius Heine wrote: > Hi Marek, Hi, > On 2024-05-07 3:28 pm, Marek Vasut wrote: >> On 5/7/24 3:06 PM, Claudius Heine wrote: >>> For CST to find the certificates and keys for signing, some keys and >>> certs need to be copied into the u-boot build directory. >> >> Make sure to CC "NXP i.MX U-Boot Team" , else NXP is not informed. Use >> scripts/get_maintainer to get the full list or just reuse the CC list >> from patches in this thread. > > I send the patch with `--to-cmd scripts/get_maintainer.pl`, maybe I > should have used `--cc-cmd`, but that would not change the list of > recipients. Should now be fixed in [PATCH] ARM: imx: Add doc/imx/ to i.MX MAINTAINERS entry >>> diff --git a/doc/imx/habv4/guides/mx8m_spl_secure_boot.txt >>> b/doc/imx/habv4/guides/mx8m_spl_secure_boot.txt >>> index ce1de659d8..42214df21a 100644 >>> --- a/doc/imx/habv4/guides/mx8m_spl_secure_boot.txt >>> +++ b/doc/imx/habv4/guides/mx8m_spl_secure_boot.txt >>> @@ -144,6 +144,22 @@ The signing is activated by wrapping SPL and >>> fitImage sections into nxp-imx8mcst >>> etype, which is done automatically in >>> arch/arm/dts/imx8m{m,n,p,q}-u-boot.dtsi >>> in case CONFIG_IMX_HAB Kconfig symbol is enabled. >>> +Per default the HAB keys and certificates need to be located in the >>> build >>> +directory, this means copying the following files from the HAB keys >>> directory >>> +flat (e.g. removing the `keys` and `cert` subdirectory) into the >>> u-boot build >>> +directory for the CST Code Signing Tool to locate them: >> >> Do symlink(s) work too ? > > I have not tested it, but I don't see any reason why it would not. I > also don't see a reason for mentioning it. I want to keep it simple, if > the dev whats to do things differently, they are free to do so. " Per default the HAB keys and certificates need to be located in the build directory, this means {+creating a symbolic link or +}copying the following... " Please test it and add it in V2 if it works, I think symlink is better than bluntly copying files around, esp. for crypto material. >>> +- `crts/SRK_1_2_3_4_table.bin` >>> +- `crts/CSF1_1_sha256_4096_65537_v3_usr_crt.pem` >>> +- `keys/CSF1_1_sha256_4096_65537_v3_usr_key.pem` >>> +- `crts/IMG1_1_sha256_4096_65537_v3_usr_crt.pem` >>> +- `keys/IMG1_1_sha256_4096_65537_v3_usr_key.pem` >>> +- `keys/key_pass.txt` >>> + >>> +The paths to the SRK table and the certificates can be modified via >>> changes to >>> +the nxp_imx8mcst device tree node >> >> "nodes", plural, there are two, one for SPL and one for fitImage. > > Well, I was thinking here more generally about the node type and was > assuming that the person reading this knows how many they have of that > type. But I can add a `s` in v2. Use "node(s)" which covers both options. >> It would be good to mention the DT properties which govern the crypto >> material paths -- nxp,srk-table, nxp,csf-crt, nxp,img-crt -- somewhere >> around this sentence. > > This is something that should be documented with the changes where that > code was added, IMO. I only documented here what I found out and have > used myself, I haven't used those. > > I would be interested in reading how to best overwrite those paths and > the image structured from board u-boot.dtsi files myself. > > If you want to can pickup my patch and integrate it into your series and > extend it. I'll keep it in mind for V3.
On Sun, May 12, 2024 at 10:08 PM Marek Vasut <marex@denx.de> wrote: > > On 5/8/24 9:23 AM, Claudius Heine wrote: > > Hi Marek, > > Hi, > > > On 2024-05-07 3:28 pm, Marek Vasut wrote: > >> On 5/7/24 3:06 PM, Claudius Heine wrote: > >>> For CST to find the certificates and keys for signing, some keys and > >>> certs need to be copied into the u-boot build directory. > >> > >> Make sure to CC "NXP i.MX U-Boot Team" , else NXP is not informed. Use > >> scripts/get_maintainer to get the full list or just reuse the CC list > >> from patches in this thread. > > > > I send the patch with `--to-cmd scripts/get_maintainer.pl`, maybe I > > should have used `--cc-cmd`, but that would not change the list of > > recipients. > > Should now be fixed in > [PATCH] ARM: imx: Add doc/imx/ to i.MX MAINTAINERS entry > > >>> diff --git a/doc/imx/habv4/guides/mx8m_spl_secure_boot.txt > >>> b/doc/imx/habv4/guides/mx8m_spl_secure_boot.txt > >>> index ce1de659d8..42214df21a 100644 > >>> --- a/doc/imx/habv4/guides/mx8m_spl_secure_boot.txt > >>> +++ b/doc/imx/habv4/guides/mx8m_spl_secure_boot.txt > >>> @@ -144,6 +144,22 @@ The signing is activated by wrapping SPL and > >>> fitImage sections into nxp-imx8mcst > >>> etype, which is done automatically in > >>> arch/arm/dts/imx8m{m,n,p,q}-u-boot.dtsi > >>> in case CONFIG_IMX_HAB Kconfig symbol is enabled. > >>> +Per default the HAB keys and certificates need to be located in the > >>> build > >>> +directory, this means copying the following files from the HAB keys > >>> directory > >>> +flat (e.g. removing the `keys` and `cert` subdirectory) into the > >>> u-boot build > >>> +directory for the CST Code Signing Tool to locate them: > >> > >> Do symlink(s) work too ? > > > > I have not tested it, but I don't see any reason why it would not. I > > also don't see a reason for mentioning it. I want to keep it simple, if > > the dev whats to do things differently, they are free to do so. > > " > Per default the HAB keys and certificates need to be located in the > build directory, this means {+creating a symbolic link or +}copying the > following... > " > > Please test it and add it in V2 if it works, I think symlink is better > than bluntly copying files around, esp. for crypto material. Hi Marek and Claudius, Yes, this documentation is needed as well but I'm still unclear why the old method before this series did not require the usr_key.pem files, why I don't have the *usr_key.pem files in my crts dir created (long ago) with cst-3.3.1 and cst-3.3.2, and what I need to do to generate them now that they are apparently needed. Best Regards, Tim > > >>> +- `crts/SRK_1_2_3_4_table.bin` > >>> +- `crts/CSF1_1_sha256_4096_65537_v3_usr_crt.pem` > >>> +- `keys/CSF1_1_sha256_4096_65537_v3_usr_key.pem` > >>> +- `crts/IMG1_1_sha256_4096_65537_v3_usr_crt.pem` > >>> +- `keys/IMG1_1_sha256_4096_65537_v3_usr_key.pem` > >>> +- `keys/key_pass.txt` > >>> + > >>> +The paths to the SRK table and the certificates can be modified via > >>> changes to > >>> +the nxp_imx8mcst device tree node > >> > >> "nodes", plural, there are two, one for SPL and one for fitImage. > > > > Well, I was thinking here more generally about the node type and was > > assuming that the person reading this knows how many they have of that > > type. But I can add a `s` in v2. > > Use "node(s)" which covers both options. > > >> It would be good to mention the DT properties which govern the crypto > >> material paths -- nxp,srk-table, nxp,csf-crt, nxp,img-crt -- somewhere > >> around this sentence. > > > > This is something that should be documented with the changes where that > > code was added, IMO. I only documented here what I found out and have > > used myself, I haven't used those. > > > > I would be interested in reading how to best overwrite those paths and > > the image structured from board u-boot.dtsi files myself. > > > > If you want to can pickup my patch and integrate it into your series and > > extend it. > > I'll keep it in mind for V3.
On Tue, May 14, 2024 at 11:50 AM Tim Harvey <tharvey@gateworks.com> wrote: > > On Sun, May 12, 2024 at 10:08 PM Marek Vasut <marex@denx.de> wrote: > > > > On 5/8/24 9:23 AM, Claudius Heine wrote: > > > Hi Marek, > > > > Hi, > > > > > On 2024-05-07 3:28 pm, Marek Vasut wrote: > > >> On 5/7/24 3:06 PM, Claudius Heine wrote: > > >>> For CST to find the certificates and keys for signing, some keys and > > >>> certs need to be copied into the u-boot build directory. > > >> > > >> Make sure to CC "NXP i.MX U-Boot Team" , else NXP is not informed. Use > > >> scripts/get_maintainer to get the full list or just reuse the CC list > > >> from patches in this thread. > > > > > > I send the patch with `--to-cmd scripts/get_maintainer.pl`, maybe I > > > should have used `--cc-cmd`, but that would not change the list of > > > recipients. > > > > Should now be fixed in > > [PATCH] ARM: imx: Add doc/imx/ to i.MX MAINTAINERS entry > > > > >>> diff --git a/doc/imx/habv4/guides/mx8m_spl_secure_boot.txt > > >>> b/doc/imx/habv4/guides/mx8m_spl_secure_boot.txt > > >>> index ce1de659d8..42214df21a 100644 > > >>> --- a/doc/imx/habv4/guides/mx8m_spl_secure_boot.txt > > >>> +++ b/doc/imx/habv4/guides/mx8m_spl_secure_boot.txt > > >>> @@ -144,6 +144,22 @@ The signing is activated by wrapping SPL and > > >>> fitImage sections into nxp-imx8mcst > > >>> etype, which is done automatically in > > >>> arch/arm/dts/imx8m{m,n,p,q}-u-boot.dtsi > > >>> in case CONFIG_IMX_HAB Kconfig symbol is enabled. > > >>> +Per default the HAB keys and certificates need to be located in the > > >>> build > > >>> +directory, this means copying the following files from the HAB keys > > >>> directory > > >>> +flat (e.g. removing the `keys` and `cert` subdirectory) into the > > >>> u-boot build > > >>> +directory for the CST Code Signing Tool to locate them: > > >> > > >> Do symlink(s) work too ? > > > > > > I have not tested it, but I don't see any reason why it would not. I > > > also don't see a reason for mentioning it. I want to keep it simple, if > > > the dev whats to do things differently, they are free to do so. > > > > " > > Per default the HAB keys and certificates need to be located in the > > build directory, this means {+creating a symbolic link or +}copying the > > following... > > " > > > > Please test it and add it in V2 if it works, I think symlink is better > > than bluntly copying files around, esp. for crypto material. > > Hi Marek and Claudius, > > Yes, this documentation is needed as well but I'm still unclear why > the old method before this series did not require the usr_key.pem > files, why I don't have the *usr_key.pem files in my crts dir created > (long ago) with cst-3.3.1 and cst-3.3.2, and what I need to do to > generate them now that they are apparently needed. > > Best Regards, > > Tim > > > > > >>> +- `crts/SRK_1_2_3_4_table.bin` > > >>> +- `crts/CSF1_1_sha256_4096_65537_v3_usr_crt.pem` > > >>> +- `keys/CSF1_1_sha256_4096_65537_v3_usr_key.pem` > > >>> +- `crts/IMG1_1_sha256_4096_65537_v3_usr_crt.pem` > > >>> +- `keys/IMG1_1_sha256_4096_65537_v3_usr_key.pem` > > >>> +- `keys/key_pass.txt` > > >>> + > > >>> +The paths to the SRK table and the certificates can be modified via > > >>> changes to > > >>> +the nxp_imx8mcst device tree node > > >> > > >> "nodes", plural, there are two, one for SPL and one for fitImage. > > > > > > Well, I was thinking here more generally about the node type and was > > > assuming that the person reading this knows how many they have of that > > > type. But I can add a `s` in v2. > > > > Use "node(s)" which covers both options. > > > > >> It would be good to mention the DT properties which govern the crypto > > >> material paths -- nxp,srk-table, nxp,csf-crt, nxp,img-crt -- somewhere > > >> around this sentence. > > > > > > This is something that should be documented with the changes where that > > > code was added, IMO. I only documented here what I found out and have > > > used myself, I haven't used those. > > > > > > I would be interested in reading how to best overwrite those paths and > > > the image structured from board u-boot.dtsi files myself. > > > > > > If you want to can pickup my patch and integrate it into your series and > > > extend it. > > > > I'll keep it in mind for V3. Hi Marek, The documentation patch here by Claudius does resolve my issues discussed in the other thread and I can confirm symlinks work fine so I think something like the following should be added: CST_DIR=/usr/src/cst-3.3.2/ ln -s $CST_DIR/crts . ln -s $CST_DIR/keys . then with the following change to nxp_imx8mcst.py you can build a signed image without code modification: diff --git a/tools/binman/etype/nxp_imx8mcst.py b/tools/binman/etype/nxp_imx8mcst.py index 132127ad4827..7d8abc78fc89 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 = fdt_util.GetString(self._node, 'nxp,srk-table', 'crts/SRK_1_2_3_4_table.bin') + self.csf_crt = fdt_util.GetString(self._node, 'nxp,csf-crt', 'crts/CSF1_1_sha256_4096_65537_v3_usr_crt.pem') + self.img_crt = fdt_util.GetString(self._node, 'nxp,img-crt', 'crts/IMG1_1_sha256_4096_65537_v3_usr_crt.pem') self.unlock = fdt_util.GetBool(self._node, 'nxp,unlock') self.ReadEntries() If copying or symlinking the keys/certs directory is not desired are env vars exposed to binman's python classes? If so you can just require CST_DIR to be specified and use that for the paths? Best Regards, Tim
Hi Tim and Marek, On 2024-05-16 12:46 am, Tim Harvey wrote: > On Tue, May 14, 2024 at 11:50 AM Tim Harvey <tharvey@gateworks.com> wrote: >> On Sun, May 12, 2024 at 10:08 PM Marek Vasut <marex@denx.de> wrote: >>> On 5/8/24 9:23 AM, Claudius Heine wrote: >>>> On 2024-05-07 3:28 pm, Marek Vasut wrote: >>>>> It would be good to mention the DT properties which govern the crypto >>>>> material paths -- nxp,srk-table, nxp,csf-crt, nxp,img-crt -- somewhere >>>>> around this sentence. >>>> >>>> This is something that should be documented with the changes where that >>>> code was added, IMO. I only documented here what I found out and have >>>> used myself, I haven't used those. >>>> >>>> I would be interested in reading how to best overwrite those paths and >>>> the image structured from board u-boot.dtsi files myself. >>>> >>>> If you want to can pickup my patch and integrate it into your series and >>>> extend it. >>> >>> I'll keep it in mind for V3. > > Hi Marek, > > The documentation patch here by Claudius does resolve my issues > discussed in the other thread and I can confirm symlinks work fine so > I think something like the following should be added: > > CST_DIR=/usr/src/cst-3.3.2/ > ln -s $CST_DIR/crts . > ln -s $CST_DIR/keys . `keys` and `crts` are very short and generic names, and putting them into the build directory might cause issues at some point. But I would not be against putting them into a sub directory (`imx-hab/{keys,crts}`?). > > then with the following change to nxp_imx8mcst.py you can build a > signed image without code modification: > diff --git a/tools/binman/etype/nxp_imx8mcst.py > b/tools/binman/etype/nxp_imx8mcst.py > index 132127ad4827..7d8abc78fc89 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 = fdt_util.GetString(self._node, > 'nxp,srk-table', 'crts/SRK_1_2_3_4_table.bin') > + self.csf_crt = fdt_util.GetString(self._node, 'nxp,csf-crt', > 'crts/CSF1_1_sha256_4096_65537_v3_usr_crt.pem') > + self.img_crt = fdt_util.GetString(self._node, 'nxp,img-crt', > 'crts/IMG1_1_sha256_4096_65537_v3_usr_crt.pem') > self.unlock = fdt_util.GetBool(self._node, 'nxp,unlock') > self.ReadEntries() > > If copying or symlinking the keys/certs directory is not desired are > env vars exposed to binman's python classes? If so you can just > require CST_DIR to be specified and use that for the paths? I personally would prefer using (one) environment variable(s) to specify the path to all keys, that way whatever `cst` needs, it will find it there, and explicit symlinking/copying can be avoided. I would probably rather call it `HAB_DIR`/`HAB_BASE_DIR` or something, because it doesn't need to be pointing to the whole `cst` stuff just a directory for the keys and certs for the HAB. `CST_DIR` might leave the impression that the `cst` from that directory is used. And you can still allow environment variables like (`SRK_TABLE`, `CSF_KEY` and `IMG_KEY`) to overwrite the name of each, relative to the `HAB_DIR/{keys,certs}` if a `HAB_DIR` is set. This would be somewhat backwards compatible and allows simpler usage by setting just one variable (`HAB_DIR`) and leaving the rest to the dtb. kind regards, Claudius
On 16/05/2024 10.25, Claudius Heine wrote: > Hi Tim and Marek, > > On 2024-05-16 12:46 am, Tim Harvey wrote: >> On Tue, May 14, 2024 at 11:50 AM Tim Harvey <tharvey@gateworks.com> >> wrote: >>> On Sun, May 12, 2024 at 10:08 PM Marek Vasut <marex@denx.de> wrote: >>>> On 5/8/24 9:23 AM, Claudius Heine wrote: >>>>> On 2024-05-07 3:28 pm, Marek Vasut wrote: >>>>>> It would be good to mention the DT properties which govern the crypto >>>>>> material paths -- nxp,srk-table, nxp,csf-crt, nxp,img-crt -- >>>>>> somewhere >>>>>> around this sentence. >>>>> >>>>> This is something that should be documented with the changes where >>>>> that >>>>> code was added, IMO. I only documented here what I found out and have >>>>> used myself, I haven't used those. >>>>> >>>>> I would be interested in reading how to best overwrite those paths and >>>>> the image structured from board u-boot.dtsi files myself. >>>>> >>>>> If you want to can pickup my patch and integrate it into your >>>>> series and >>>>> extend it. >>>> >>>> I'll keep it in mind for V3. >> >> Hi Marek, >> >> The documentation patch here by Claudius does resolve my issues >> discussed in the other thread and I can confirm symlinks work fine so >> I think something like the following should be added: >> >> CST_DIR=/usr/src/cst-3.3.2/ >> ln -s $CST_DIR/crts . >> ln -s $CST_DIR/keys . > > `keys` and `crts` are very short and generic names, and putting them > into the build directory might cause issues at some point. But I would > not be against putting them into a sub directory (`imx-hab/{keys,crts}`?). It is probably useful to be aware of the quality of the cst code. For reference, I quote get_key_file() int32_t get_key_file(const char* cert_file, char* key_file) { /* Algorithm to locate key file from given cert file */ /* for now just assume the key to present in the */ /* same folder as cert file. The crt in the name will */ /* will be replaced with key */ char * folder; int32_t i = strlen(cert_file); /**< Index into key filename, initialized to filename length */ strcpy(key_file, cert_file); key_file[i] = 0; key_file[i-5] = 'y'; key_file[i-6] = 'e'; key_file[i-7] = 'k'; /* Search for folder name "certs" in the file and replace it with "keys" */ /* Keys are found in "keys" folder and certs are in "certs" folder */ folder = strstr(key_file, "crts"); if(folder) { folder[0] = 'k'; folder[1] = 'e'; folder[2] = 'y'; folder[3] = 's'; } return CAL_SUCCESS; } Ignoring the inconsistencies in the comments, obviously there are a lot of implicit assumptions on file names and paths. First, the assumption that the filename of they key corresponding to the certificate can be obtained by replacing [-7:-5] by "key". Second, and much more egregious, is the use of strstr() on key_file searching for "crts", and just blindly replacing the first such with "keys", and ignoring it if not found. So if that string appears anywhere in the path (say, my homedir is /home/dcrts/ and I have the key material somewhere below that) this will replace the wrong occurrence (and look in /home/dkeys/ ....). And of course it was unthinkable that this could have been written using the much shorter memcpy(..., "keys", 4) so that one could actually `git grep 'keys'` and figure out what was going on. Rasmus
Hi Rasmus, On 2024-05-16 11:50 am, Rasmus Villemoes wrote: > On 16/05/2024 10.25, Claudius Heine wrote: >> Hi Tim and Marek, >> >> On 2024-05-16 12:46 am, Tim Harvey wrote: >>> On Tue, May 14, 2024 at 11:50 AM Tim Harvey <tharvey@gateworks.com> >>> wrote: >>>> On Sun, May 12, 2024 at 10:08 PM Marek Vasut <marex@denx.de> wrote: >>>>> On 5/8/24 9:23 AM, Claudius Heine wrote: >>>>>> On 2024-05-07 3:28 pm, Marek Vasut wrote: >>>>>>> It would be good to mention the DT properties which govern the crypto >>>>>>> material paths -- nxp,srk-table, nxp,csf-crt, nxp,img-crt -- >>>>>>> somewhere >>>>>>> around this sentence. >>>>>> >>>>>> This is something that should be documented with the changes where >>>>>> that >>>>>> code was added, IMO. I only documented here what I found out and have >>>>>> used myself, I haven't used those. >>>>>> >>>>>> I would be interested in reading how to best overwrite those paths and >>>>>> the image structured from board u-boot.dtsi files myself. >>>>>> >>>>>> If you want to can pickup my patch and integrate it into your >>>>>> series and >>>>>> extend it. >>>>> >>>>> I'll keep it in mind for V3. >>> >>> Hi Marek, >>> >>> The documentation patch here by Claudius does resolve my issues >>> discussed in the other thread and I can confirm symlinks work fine so >>> I think something like the following should be added: >>> >>> CST_DIR=/usr/src/cst-3.3.2/ >>> ln -s $CST_DIR/crts . >>> ln -s $CST_DIR/keys . >> >> `keys` and `crts` are very short and generic names, and putting them >> into the build directory might cause issues at some point. But I would >> not be against putting them into a sub directory (`imx-hab/{keys,crts}`?). > > It is probably useful to be aware of the quality of the cst code. For > reference, I quote get_key_file() > > int32_t get_key_file(const char* cert_file, char* key_file) > { > /* Algorithm to locate key file from given cert file */ > /* for now just assume the key to present in the */ > /* same folder as cert file. The crt in the name will */ > /* will be replaced with key */ > char * folder; > int32_t i = strlen(cert_file); /**< Index into key filename, > initialized > to filename length */ > > strcpy(key_file, cert_file); > key_file[i] = 0; > > key_file[i-5] = 'y'; > key_file[i-6] = 'e'; > key_file[i-7] = 'k'; > > /* Search for folder name "certs" in the file and replace it with > "keys" */ > /* Keys are found in "keys" folder and certs are in "certs" folder > */ > > folder = strstr(key_file, "crts"); > if(folder) > { > folder[0] = 'k'; > folder[1] = 'e'; > folder[2] = 'y'; > folder[3] = 's'; > } > return CAL_SUCCESS; > } > > Ignoring the inconsistencies in the comments, obviously there are a lot > of implicit assumptions on file names and paths. First, the assumption > that the filename of they key corresponding to the certificate can be > obtained by replacing [-7:-5] by "key". Second, and much more egregious, > is the use of strstr() on key_file searching for "crts", and just > blindly replacing the first such with "keys", and ignoring it if not > found. So if that string appears anywhere in the path (say, my homedir > is /home/dcrts/ and I have the key material somewhere below that) this > will replace the wrong occurrence (and look in /home/dkeys/ ....). > > And of course it was unthinkable that this could have been written using > the much shorter memcpy(..., "keys", 4) so that one could actually `git > grep 'keys'` and figure out what was going on. Exactly. I had the pleasure to read cst code a bit as well to figure out some issue. This is also a reason I suggested to just set the base path to the CST/HAB files instead of setting the individual paths to the keys/certs in the hope that this is a more robust way for cst to find its implicitly required files. regards, Claudius
diff --git a/doc/imx/habv4/guides/mx8m_spl_secure_boot.txt b/doc/imx/habv4/guides/mx8m_spl_secure_boot.txt index ce1de659d8..42214df21a 100644 --- a/doc/imx/habv4/guides/mx8m_spl_secure_boot.txt +++ b/doc/imx/habv4/guides/mx8m_spl_secure_boot.txt @@ -144,6 +144,22 @@ The signing is activated by wrapping SPL and fitImage sections into nxp-imx8mcst etype, which is done automatically in arch/arm/dts/imx8m{m,n,p,q}-u-boot.dtsi in case CONFIG_IMX_HAB Kconfig symbol is enabled. +Per default the HAB keys and certificates need to be located in the build +directory, this means copying the following files from the HAB keys directory +flat (e.g. removing the `keys` and `cert` subdirectory) into the u-boot build +directory for the CST Code Signing Tool to locate them: + +- `crts/SRK_1_2_3_4_table.bin` +- `crts/CSF1_1_sha256_4096_65537_v3_usr_crt.pem` +- `keys/CSF1_1_sha256_4096_65537_v3_usr_key.pem` +- `crts/IMG1_1_sha256_4096_65537_v3_usr_crt.pem` +- `keys/IMG1_1_sha256_4096_65537_v3_usr_key.pem` +- `keys/key_pass.txt` + +The paths to the SRK table and the certificates can be modified via changes to +the nxp_imx8mcst device tree node, however the other files are required by the +CST tools as well, and will be searched for in relation to them. + Build of flash.bin target then produces a signed flash.bin automatically. 1.4 Closing the device
For CST to find the certificates and keys for signing, some keys and certs need to be copied into the u-boot build directory. Signed-off-by: Claudius Heine <ch@denx.de> --- doc/imx/habv4/guides/mx8m_spl_secure_boot.txt | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)