[PATCH] binman: add fast authentication method for i.MX8M signing

[PATCH] binman: add fast authentication method for i.MX8M signing

[Brian Ruley]

Using the PKI tree with SRKs as intermediate CA isn't necessary or even
desirable in some situations (boot time, for example). Add the possbility
to use the "fast authentication" method where the image and CSF are both
signed using the SRK [1, p.63].

[1] https://community.nxp.com/pwmxy87654/attachments/pwmxy87654/imx-processors/202591/1/CST_UG.pdf

Signed-off-by: Brian Ruley <brian.ruley@gehealthcare.com>
Cc: Marek Vasut <marex@denx.de>

 tools/binman/etype/nxp_imx8mcst.py | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/tools/binman/etype/nxp_imx8mcst.py b/tools/binman/etype/nxp_imx8mcst.py
index 8221517b0c..d39b6a79de 100644
--- a/tools/binman/etype/nxp_imx8mcst.py
+++ b/tools/binman/etype/nxp_imx8mcst.py
@@ -36,6 +36,9 @@ csf_config_template = """
   File = "SRK_1_2_3_4_table.bin"
   Source index = 0

+[Install NOCAK]
+  File = "SRK1_sha256_4096_65537_v3_usr_crt.pem"
+
 [Install CSFK]
   File = "CSF1_1_sha256_4096_65537_v3_usr_crt.pem"

@@ -70,8 +73,13 @@ class Entry_nxp_imx8mcst(Entry_mkimage):
         super().ReadNode()
         self.loader_address = fdt_util.GetInt(self._node, 'nxp,loader-address')
         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.fast_auth = fdt_util.GetBool(self._node, 'nxp,fast-auth')
+        if not self.fast_auth:
+            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'))
+        else:
+            self.srk_crt = os.getenv('SRK_KEY', fdt_util.GetString(self._node, 'nxp,srk-crt', 'SRK1_sha256_2048_65537_v3_usr_crt.pem'))
+
         self.unlock = fdt_util.GetBool(self._node, 'nxp,unlock')
         self.ReadEntries()

@@ -125,8 +133,16 @@ class Entry_nxp_imx8mcst(Entry_mkimage):
         # Load configuration template and modify keys of interest
         config.read_string(csf_config_template)
         config['Install SRK']['File'] = '"' + self.srk_table + '"'
-        config['Install CSFK']['File'] = '"' + self.csf_crt + '"'
-        config['Install Key']['File'] = '"' + self.img_crt + '"'
+        if not self.fast_auth:
+            config.remove_section('Install NOCAK')
+            config['Install CSFK']['File'] = '"' + self.csf_crt + '"'
+            config['Install Key']['File'] = '"' + self.img_crt + '"'
+        else:
+            config.remove_section('Install CSFK')
+            config.remove_section('Install Key')
+            config['Install NOCAK']['File'] = '"' + self.srk_crt + '"'
+            config['Authenticate Data']['Verification index'] = '0'
+
         config['Authenticate Data']['Blocks'] = hex(signbase) + ' 0 ' + hex(len(data)) + ' "' + str(output_dname) + '"'
         if not self.unlock:
             config.remove_section('Unlock')
--
2.39.2

Re: [PATCH] binman: add fast authentication method for i.MX8M signing

[Simon Glass]

Hi,

On Fri, 27 Sept 2024 at 06:42, Brian Ruley <brian.ruley@gehealthcare.com> wrote:
>
> Using the PKI tree with SRKs as intermediate CA isn't necessary or even
> desirable in some situations (boot time, for example). Add the possbility
> to use the "fast authentication" method where the image and CSF are both
> signed using the SRK [1, p.63].
>
> [1] https://community.nxp.com/pwmxy87654/attachments/pwmxy87654/imx-processors/202591/1/CST_UG.pdf
>
> Signed-off-by: Brian Ruley <brian.ruley@gehealthcare.com>
> Cc: Marek Vasut <marex@denx.de>
>
>  tools/binman/etype/nxp_imx8mcst.py | 23 +++++++++++++++++++----
>  1 file changed, 19 insertions(+), 4 deletions(-)
>

Please can you coordinate with Marek as we need to sort out the test
coverage for this etype, before adding more functionality. I did a
starting point, now in -next, which should help.

> diff --git a/tools/binman/etype/nxp_imx8mcst.py b/tools/binman/etype/nxp_imx8mcst.py
> index 8221517b0c..d39b6a79de 100644
> --- a/tools/binman/etype/nxp_imx8mcst.py
> +++ b/tools/binman/etype/nxp_imx8mcst.py
> @@ -36,6 +36,9 @@ csf_config_template = """
>    File = "SRK_1_2_3_4_table.bin"
>    Source index = 0
>
> +[Install NOCAK]
> +  File = "SRK1_sha256_4096_65537_v3_usr_crt.pem"
> +
>  [Install CSFK]
>    File = "CSF1_1_sha256_4096_65537_v3_usr_crt.pem"
>
> @@ -70,8 +73,13 @@ class Entry_nxp_imx8mcst(Entry_mkimage):
>          super().ReadNode()
>          self.loader_address = fdt_util.GetInt(self._node, 'nxp,loader-address')
>          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.fast_auth = fdt_util.GetBool(self._node, 'nxp,fast-auth')
> +        if not self.fast_auth:
> +            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'))
> +        else:
> +            self.srk_crt = os.getenv('SRK_KEY', fdt_util.GetString(self._node, 'nxp,srk-crt', 'SRK1_sha256_2048_65537_v3_usr_crt.pem'))
> +
>          self.unlock = fdt_util.GetBool(self._node, 'nxp,unlock')
>          self.ReadEntries()
>
> @@ -125,8 +133,16 @@ class Entry_nxp_imx8mcst(Entry_mkimage):
>          # Load configuration template and modify keys of interest
>          config.read_string(csf_config_template)
>          config['Install SRK']['File'] = '"' + self.srk_table + '"'
> -        config['Install CSFK']['File'] = '"' + self.csf_crt + '"'
> -        config['Install Key']['File'] = '"' + self.img_crt + '"'
> +        if not self.fast_auth:
> +            config.remove_section('Install NOCAK')
> +            config['Install CSFK']['File'] = '"' + self.csf_crt + '"'
> +            config['Install Key']['File'] = '"' + self.img_crt + '"'
> +        else:
> +            config.remove_section('Install CSFK')
> +            config.remove_section('Install Key')
> +            config['Install NOCAK']['File'] = '"' + self.srk_crt + '"'
> +            config['Authenticate Data']['Verification index'] = '0'
> +
>          config['Authenticate Data']['Blocks'] = hex(signbase) + ' 0 ' + hex(len(data)) + ' "' + str(output_dname) + '"'
>          if not self.unlock:
>              config.remove_section('Unlock')
> --
> 2.39.2
>

Regards,
Simon

Re: [PATCH] binman: add fast authentication method for i.MX8M signing

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1180 bytes --]

On Fri, Sep 27, 2024 at 10:50:29AM -0600, Simon Glass wrote:
> Hi,
> 
> On Fri, 27 Sept 2024 at 06:42, Brian Ruley <brian.ruley@gehealthcare.com> wrote:
> >
> > Using the PKI tree with SRKs as intermediate CA isn't necessary or even
> > desirable in some situations (boot time, for example). Add the possbility
> > to use the "fast authentication" method where the image and CSF are both
> > signed using the SRK [1, p.63].
> >
> > [1] https://community.nxp.com/pwmxy87654/attachments/pwmxy87654/imx-processors/202591/1/CST_UG.pdf
> >
> > Signed-off-by: Brian Ruley <brian.ruley@gehealthcare.com>
> > Cc: Marek Vasut <marex@denx.de>
> >
> >  tools/binman/etype/nxp_imx8mcst.py | 23 +++++++++++++++++++----
> >  1 file changed, 19 insertions(+), 4 deletions(-)
> >
> 
> Please can you coordinate with Marek as we need to sort out the test
> coverage for this etype, before adding more functionality. I did a
> starting point, now in -next, which should help.

Well, when someone has both time and understanding of the tools and the
frameworks, we can expand the automatic tests while still having
functional testing as people use the feature.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH] binman: add fast authentication method for i.MX8M signing

[Fabio Estevam]

Hi Simon, Marek, and Tom,

On Fri, Sep 27, 2024 at 5:47 PM Tom Rini <trini@konsulko.com> wrote:

> > Please can you coordinate with Marek as we need to sort out the test
> > coverage for this etype, before adding more functionality. I did a
> > starting point, now in -next, which should help.
>
> Well, when someone has both time and understanding of the tools and the
> frameworks, we can expand the automatic tests while still having
> functional testing as people use the feature.

Do you think this patch should be applied as is? Could the tests be
handled later?

Please let me know.

Re: [PATCH] binman: add fast authentication method for i.MX8M signing

[Simon Glass]

Hi Fabio,

On Sun, 29 Sept 2024 at 14:53, Fabio Estevam <festevam@gmail.com> wrote:
>
> Hi Simon, Marek, and Tom,
>
> On Fri, Sep 27, 2024 at 5:47 PM Tom Rini <trini@konsulko.com> wrote:
>
> > > Please can you coordinate with Marek as we need to sort out the test
> > > coverage for this etype, before adding more functionality. I did a
> > > starting point, now in -next, which should help.
> >
> > Well, when someone has both time and understanding of the tools and the
> > frameworks, we can expand the automatic tests while still having
> > functional testing as people use the feature.
>
> Do you think this patch should be applied as is? Could the tests be
> handled later?
>
> Please let me know.

Who is going to handle the tests later and when?

Regards,
Simon

Re: [PATCH] binman: add fast authentication method for i.MX8M signing

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1027 bytes --]

On Sun, Sep 29, 2024 at 04:49:17PM -0600, Simon Glass wrote:
> Hi Fabio,
> 
> On Sun, 29 Sept 2024 at 14:53, Fabio Estevam <festevam@gmail.com> wrote:
> >
> > Hi Simon, Marek, and Tom,
> >
> > On Fri, Sep 27, 2024 at 5:47 PM Tom Rini <trini@konsulko.com> wrote:
> >
> > > > Please can you coordinate with Marek as we need to sort out the test
> > > > coverage for this etype, before adding more functionality. I did a
> > > > starting point, now in -next, which should help.
> > >
> > > Well, when someone has both time and understanding of the tools and the
> > > frameworks, we can expand the automatic tests while still having
> > > functional testing as people use the feature.
> >
> > Do you think this patch should be applied as is? Could the tests be
> > handled later?
> >
> > Please let me know.
> 
> Who is going to handle the tests later and when?

Someone, once how to write and run the tests is documented. That's a
big hurdle this private thread has shown, to me at least.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

[PATCH v2] binman: add fast authentication method for i.MX8M signing

[Brian Ruley]

Using the PKI tree with SRKs as intermediate CA isn't necessary or even
desirable in some situations (boot time, for example). Add the possbility
to use the "fast authentication" method where the image and CSF are both
signed using the SRK [1, p.63].

[1] https://community.nxp.com/pwmxy87654/attachments/pwmxy87654/imx-processors/202591/1/CST_UG.pdf

Signed-off-by: Brian Ruley <brian.ruley@gehealthcare.com>
Cc: Marek Vasut <marex@denx.de>
---
Changes for v2:
- fixed default key length (s/2048/4096) for srk-crt node 

 tools/binman/etype/nxp_imx8mcst.py | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)
---
 tools/binman/etype/nxp_imx8mcst.py | 24 ++++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/tools/binman/etype/nxp_imx8mcst.py b/tools/binman/etype/nxp_imx8mcst.py
index 8221517b0c..93fec1a914 100644
--- a/tools/binman/etype/nxp_imx8mcst.py
+++ b/tools/binman/etype/nxp_imx8mcst.py
@@ -36,6 +36,9 @@ csf_config_template = """
   File = "SRK_1_2_3_4_table.bin"
   Source index = 0
 
+[Install NOCAK]
+  File = "SRK1_sha256_4096_65537_v3_usr_crt.pem"
+
 [Install CSFK]
   File = "CSF1_1_sha256_4096_65537_v3_usr_crt.pem"
 
@@ -70,8 +73,13 @@ class Entry_nxp_imx8mcst(Entry_mkimage):
         super().ReadNode()
         self.loader_address = fdt_util.GetInt(self._node, 'nxp,loader-address')
         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.fast_auth = fdt_util.GetBool(self._node, 'nxp,fast-auth')
+        if not self.fast_auth:
+            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'))
+        else:
+            self.srk_crt = os.getenv('SRK_KEY', fdt_util.GetString(self._node, 'nxp,srk-crt', 'SRK1_sha256_4096_65537_v3_usr_crt.pem'))
+
         self.unlock = fdt_util.GetBool(self._node, 'nxp,unlock')
         self.ReadEntries()
 
@@ -125,8 +133,16 @@ class Entry_nxp_imx8mcst(Entry_mkimage):
         # Load configuration template and modify keys of interest
         config.read_string(csf_config_template)
         config['Install SRK']['File'] = '"' + self.srk_table + '"'
-        config['Install CSFK']['File'] = '"' + self.csf_crt + '"'
-        config['Install Key']['File'] = '"' + self.img_crt + '"'
+        if not self.fast_auth:
+            config.remove_section('Install NOCAK')
+            config['Install CSFK']['File'] = '"' + self.csf_crt + '"'
+            config['Install Key']['File'] = '"' + self.img_crt + '"'
+        else:
+            config.remove_section('Install CSFK')
+            config.remove_section('Install Key')
+            config['Install NOCAK']['File'] = '"' + self.srk_crt + '"'
+            config['Authenticate Data']['Verification index'] = '0'
+
         config['Authenticate Data']['Blocks'] = hex(signbase) + ' 0 ' + hex(len(data)) + ' "' + str(output_dname) + '"'
         if not self.unlock:
             config.remove_section('Unlock')
-- 
2.39.5

Re: [PATCH] binman: add fast authentication method for i.MX8M signing

[Brian Ruley]

On Sun, Sep 29, 2024 at 06:46:23PM -0600, Tom Rini wrote:
> On Sun, Sep 29, 2024 at 04:49:17PM -0600, Simon Glass wrote:
> > Hi Fabio,
> > 
> > On Sun, 29 Sept 2024 at 14:53, Fabio Estevam <festevam@gmail.com> wrote:
> > >
> > > Hi Simon, Marek, and Tom,
> > >
> > > On Fri, Sep 27, 2024 at 5:47???PM Tom Rini <trini@konsulko.com> wrote:
> > >
> > > > > Please can you coordinate with Marek as we need to sort out the test
> > > > > coverage for this etype, before adding more functionality. I did a
> > > > > starting point, now in -next, which should help.
> > > >
> > > > Well, when someone has both time and understanding of the tools and the
> > > > frameworks, we can expand the automatic tests while still having
> > > > functional testing as people use the feature.
> > >
> > > Do you think this patch should be applied as is? Could the tests be
> > > handled later?
> > >
> > > Please let me know.
> > 
> > Who is going to handle the tests later and when?
> 
> Someone, once how to write and run the tests is documented. That's a
> big hurdle this private thread has shown, to me at least.
> 
> -- 
> Tom

Hi,

I fixed a minor issue in the patch, and sent a revised version.

Seems that you don't have the testing quite defined yet, so I don't know
what I can contribute there. Maybe something can be done with the CSF
parser to check that the signing works correctly?

However, I think this feature is quite benign, and it would be great to
get some functional testing in for this feature, as Tom said. For us,
this is an important feature, so we have done extensive testing
internally to verify that it works.

Best,
Brian

Re: [PATCH] binman: add fast authentication method for i.MX8M signing

[Simon Glass]

Hi Brian,

On Mon, 30 Sept 2024 at 07:01, Brian Ruley <brian.ruley@gehealthcare.com> wrote:
>
> On Sun, Sep 29, 2024 at 06:46:23PM -0600, Tom Rini wrote:
> > On Sun, Sep 29, 2024 at 04:49:17PM -0600, Simon Glass wrote:
> > > Hi Fabio,
> > >
> > > On Sun, 29 Sept 2024 at 14:53, Fabio Estevam <festevam@gmail.com> wrote:
> > > >
> > > > Hi Simon, Marek, and Tom,
> > > >
> > > > On Fri, Sep 27, 2024 at 5:47???PM Tom Rini <trini@konsulko.com> wrote:
> > > >
> > > > > > Please can you coordinate with Marek as we need to sort out the test
> > > > > > coverage for this etype, before adding more functionality. I did a
> > > > > > starting point, now in -next, which should help.
> > > > >
> > > > > Well, when someone has both time and understanding of the tools and the
> > > > > frameworks, we can expand the automatic tests while still having
> > > > > functional testing as people use the feature.
> > > >
> > > > Do you think this patch should be applied as is? Could the tests be
> > > > handled later?
> > > >
> > > > Please let me know.
> > >
> > > Who is going to handle the tests later and when?
> >
> > Someone, once how to write and run the tests is documented. That's a
> > big hurdle this private thread has shown, to me at least.
> >
> > --
> > Tom
>
> Hi,
>
> I fixed a minor issue in the patch, and sent a revised version.
>
> Seems that you don't have the testing quite defined yet, so I don't know
> what I can contribute there. Maybe something can be done with the CSF
> parser to check that the signing works correctly?

Just to be clear, the testing is fully defined...this is actually the
only entry type which doesn't have a proper test. Every other user was
able to add a test. The goal here isn't really to check that external
tools are working (they should have their own tests), more to check
that Binman is doing the right thing.

I will get some sort of patch out by tomorrow morning to help this process.

>
> However, I think this feature is quite benign, and it would be great to
> get some functional testing in for this feature, as Tom said. For us,
> this is an important feature, so we have done extensive testing
> internally to verify that it works.

I fully understand that...but of course without an automated test in
Binman it may break at any moment, as Binman continues to be expanded.

BTW, minor code-style things: U-Boot's Python code uses single quotes
and the line limit is 80cols.

Regards,
Simon

Re: [PATCH] binman: add fast authentication method for i.MX8M signing

[Brian Ruley]

On Mon, Sep 30, 2024 at 08:10:49AM -0600, Simon Glass wrote:
> 
> WARNING: This email originated from outside of GE HealthCare. Please validate the sender's email address before clicking on links or attachments as they may not be safe.
> 
> Hi Brian,
> 
> On Mon, 30 Sept 2024 at 07:01, Brian Ruley <brian.ruley@gehealthcare.com> wrote:
> >
> > On Sun, Sep 29, 2024 at 06:46:23PM -0600, Tom Rini wrote:
> > > On Sun, Sep 29, 2024 at 04:49:17PM -0600, Simon Glass wrote:
> > > > Hi Fabio,
> > > >
> > > > On Sun, 29 Sept 2024 at 14:53, Fabio Estevam <festevam@gmail.com> wrote:
> > > > >
> > > > > Hi Simon, Marek, and Tom,
> > > > >
> > > > > On Fri, Sep 27, 2024 at 5:47???PM Tom Rini <trini@konsulko.com> wrote:
> > > > >
> > > > > > > Please can you coordinate with Marek as we need to sort out the test
> > > > > > > coverage for this etype, before adding more functionality. I did a
> > > > > > > starting point, now in -next, which should help.
> > > > > >
> > > > > > Well, when someone has both time and understanding of the tools and the
> > > > > > frameworks, we can expand the automatic tests while still having
> > > > > > functional testing as people use the feature.
> > > > >
> > > > > Do you think this patch should be applied as is? Could the tests be
> > > > > handled later?
> > > > >
> > > > > Please let me know.
> > > >
> > > > Who is going to handle the tests later and when?
> > >
> > > Someone, once how to write and run the tests is documented. That's a
> > > big hurdle this private thread has shown, to me at least.
> > >
> > > --
> > > Tom
> >
> > Hi,
> >
> > I fixed a minor issue in the patch, and sent a revised version.
> >
> > Seems that you don't have the testing quite defined yet, so I don't know
> > what I can contribute there. Maybe something can be done with the CSF
> > parser to check that the signing works correctly?
> 
> Just to be clear, the testing is fully defined...this is actually the
> only entry type which doesn't have a proper test. Every other user was
> able to add a test. The goal here isn't really to check that external
> tools are working (they should have their own tests), more to check
> that Binman is doing the right thing.
> 
> I will get some sort of patch out by tomorrow morning to help this process.
> 
> >
> > However, I think this feature is quite benign, and it would be great to
> > get some functional testing in for this feature, as Tom said. For us,
> > this is an important feature, so we have done extensive testing
> > internally to verify that it works.
> 
> I fully understand that...but of course without an automated test in
> Binman it may break at any moment, as Binman continues to be expanded.
> 
> BTW, minor code-style things: U-Boot's Python code uses single quotes
> and the line limit is 80cols.
> 
> Regards,
> Simon

Hi Simon,

> Just to be clear, the testing is fully defined...this is actually the
> only entry type which doesn't have a proper test. Every other user was
> able to add a test. The goal here isn't really to check that external
> tools are working (they should have their own tests), more to check
> that Binman is doing the right thing.
> 
> I will get some sort of patch out by tomorrow morning to help this process.

Alrighty. I'll include a test once we get this sorted out.

> BTW, minor code-style things: U-Boot's Python code uses single quotes
> and the line limit is 80cols.

I don't understand what you mean because I did use single quotes :)
Perhaps you were looking at the CSF template, which is not Python code?

As for the column width, sorry, I was only trying to follow the pattern
in the file. I thought it was 120 cols. I will include a predecessor
patch to fix the column width to 80.

Best,
Brian

Re: [PATCH] binman: add fast authentication method for i.MX8M signing

[Simon Glass]

Hi Brian,

On Mon, 30 Sept 2024 at 08:47, Brian Ruley <brian.ruley@gehealthcare.com> wrote:
>
> On Mon, Sep 30, 2024 at 08:10:49AM -0600, Simon Glass wrote:
> >
> > WARNING: This email originated from outside of GE HealthCare. Please validate the sender's email address before clicking on links or attachments as they may not be safe.
> >
> > Hi Brian,
> >
> > On Mon, 30 Sept 2024 at 07:01, Brian Ruley <brian.ruley@gehealthcare.com> wrote:
> > >
> > > On Sun, Sep 29, 2024 at 06:46:23PM -0600, Tom Rini wrote:
> > > > On Sun, Sep 29, 2024 at 04:49:17PM -0600, Simon Glass wrote:
> > > > > Hi Fabio,
> > > > >
> > > > > On Sun, 29 Sept 2024 at 14:53, Fabio Estevam <festevam@gmail.com> wrote:
> > > > > >
> > > > > > Hi Simon, Marek, and Tom,
> > > > > >
> > > > > > On Fri, Sep 27, 2024 at 5:47???PM Tom Rini <trini@konsulko.com> wrote:
> > > > > >
> > > > > > > > Please can you coordinate with Marek as we need to sort out the test
> > > > > > > > coverage for this etype, before adding more functionality. I did a
> > > > > > > > starting point, now in -next, which should help.
> > > > > > >
> > > > > > > Well, when someone has both time and understanding of the tools and the
> > > > > > > frameworks, we can expand the automatic tests while still having
> > > > > > > functional testing as people use the feature.
> > > > > >
> > > > > > Do you think this patch should be applied as is? Could the tests be
> > > > > > handled later?
> > > > > >
> > > > > > Please let me know.
> > > > >
> > > > > Who is going to handle the tests later and when?
> > > >
> > > > Someone, once how to write and run the tests is documented. That's a
> > > > big hurdle this private thread has shown, to me at least.
> > > >
> > > > --
> > > > Tom
> > >
> > > Hi,
> > >
> > > I fixed a minor issue in the patch, and sent a revised version.
> > >
> > > Seems that you don't have the testing quite defined yet, so I don't know
> > > what I can contribute there. Maybe something can be done with the CSF
> > > parser to check that the signing works correctly?
> >
> > Just to be clear, the testing is fully defined...this is actually the
> > only entry type which doesn't have a proper test. Every other user was
> > able to add a test. The goal here isn't really to check that external
> > tools are working (they should have their own tests), more to check
> > that Binman is doing the right thing.
> >
> > I will get some sort of patch out by tomorrow morning to help this process.
> >
> > >
> > > However, I think this feature is quite benign, and it would be great to
> > > get some functional testing in for this feature, as Tom said. For us,
> > > this is an important feature, so we have done extensive testing
> > > internally to verify that it works.
> >
> > I fully understand that...but of course without an automated test in
> > Binman it may break at any moment, as Binman continues to be expanded.
> >
> > BTW, minor code-style things: U-Boot's Python code uses single quotes
> > and the line limit is 80cols.
> >
> > Regards,
> > Simon
>
> Hi Simon,
>
> > Just to be clear, the testing is fully defined...this is actually the
> > only entry type which doesn't have a proper test. Every other user was
> > able to add a test. The goal here isn't really to check that external
> > tools are working (they should have their own tests), more to check
> > that Binman is doing the right thing.
> >
> > I will get some sort of patch out by tomorrow morning to help this process.
>
> Alrighty. I'll include a test once we get this sorted out.
>
> > BTW, minor code-style things: U-Boot's Python code uses single quotes
> > and the line limit is 80cols.
>
> I don't understand what you mean because I did use single quotes :)
> Perhaps you were looking at the CSF template, which is not Python code?

Ah OK, I see. It would be less confusing if you used f-strings.
Unfortunately Binman does not use them always and there are tons of
pylint warnings about it.

>
> As for the column width, sorry, I was only trying to follow the pattern
> in the file. I thought it was 120 cols. I will include a predecessor
> patch to fix the column width to 80.

Oh, OK, yes I wasn't around when the original patch was sent so didn't
review it.

Regards,
SImon

[PATCH v3 1/2] binman: cosmetic: code formatting fixes

[Brian Ruley]

Conform to the style guide used in the project by making the following
changes:
* Use single quotes for multiline strings (except docstrings)
* Fix line width to 79 cols
* Use f-string instead of formatting a regular string

Signed-off-by: Brian Ruley <brian.ruley@gehealthcare.com>
---
 tools/binman/etype/nxp_imx8mcst.py | 28 +++++++++++++++++++++-------
 1 file changed, 21 insertions(+), 7 deletions(-)

diff --git a/tools/binman/etype/nxp_imx8mcst.py b/tools/binman/etype/nxp_imx8mcst.py
index 8221517b0c..0c744a00d7 100644
--- a/tools/binman/etype/nxp_imx8mcst.py
+++ b/tools/binman/etype/nxp_imx8mcst.py
@@ -23,7 +23,7 @@ from u_boot_pylib import tools
 MAGIC_NXP_IMX_IVT = 0x412000d1
 MAGIC_FITIMAGE    = 0xedfe0dd0
 
-csf_config_template = """
+csf_config_template = '''
 [Header]
   Version = 4.3
   Hash Algorithm = sha256
@@ -53,7 +53,7 @@ csf_config_template = """
 [Authenticate Data]
   Verification index = 2
   Blocks = 0x1234 0x78 0xabcd "data.bin"
-"""
+'''
 
 class Entry_nxp_imx8mcst(Entry_mkimage):
     """NXP i.MX8M CST .cfg file generator and cst invoker
@@ -69,9 +69,21 @@ 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 = 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.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()
 
@@ -118,7 +130,7 @@ class Entry_nxp_imx8mcst(Entry_mkimage):
         tools.write_file(output_dname, data)
 
         # Generate CST configuration file used to sign payload
-        cfg_fname = tools.get_output_filename('nxp.csf-config-txt.%s' % uniq)
+        cfg_fname = tools.get_output_filename(f'nxp.csf-config-txt.{uniq}')
         config = configparser.ConfigParser()
         # Do not make key names lowercase
         config.optionxform = str
@@ -127,7 +139,9 @@ class Entry_nxp_imx8mcst(Entry_mkimage):
         config['Install SRK']['File'] = '"' + self.srk_table + '"'
         config['Install CSFK']['File'] = '"' + self.csf_crt + '"'
         config['Install Key']['File'] = '"' + self.img_crt + '"'
-        config['Authenticate Data']['Blocks'] = hex(signbase) + ' 0 ' + hex(len(data)) + ' "' + str(output_dname) + '"'
+        config['Authenticate Data']['Blocks'] = (hex(signbase) + ' 0 '
+                                                 + hex(len(data)) + ' "'
+                                                 + str(output_dname) + '"')
         if not self.unlock:
             config.remove_section('Unlock')
         with open(cfg_fname, 'w') as cfgf:
-- 
2.39.5

[PATCH v3 2/2] binman: add fast authentication method for i.MX8M signing

[Brian Ruley]

Using the PKI tree with SRKs as intermediate CA isn't necessary or even
desirable in some situations (boot time, for example). Add the possbility
to use the "fast authentication" method where the image and CSF are both
signed using the SRK [1, p.63].

[1] https://community.nxp.com/pwmxy87654/attachments/pwmxy87654/imx-processors/202591/1/CST_UG.pdf

Signed-off-by: Brian Ruley <brian.ruley@gehealthcare.com>
Cc: Marek Vasut <marex@denx.de>

 tools/binman/etype/nxp_imx8mcst.py | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)
---
 tools/binman/etype/nxp_imx8mcst.py | 44 ++++++++++++++++++++++--------
 1 file changed, 32 insertions(+), 12 deletions(-)

diff --git a/tools/binman/etype/nxp_imx8mcst.py b/tools/binman/etype/nxp_imx8mcst.py
index 0c744a00d7..a80cb94499 100644
--- a/tools/binman/etype/nxp_imx8mcst.py
+++ b/tools/binman/etype/nxp_imx8mcst.py
@@ -36,6 +36,9 @@ csf_config_template = '''
   File = "SRK_1_2_3_4_table.bin"
   Source index = 0
 
+[Install NOCAK]
+  File = "SRK1_sha256_4096_65537_v3_usr_crt.pem"
+
 [Install CSFK]
   File = "CSF1_1_sha256_4096_65537_v3_usr_crt.pem"
 
@@ -74,16 +77,25 @@ class Entry_nxp_imx8mcst(Entry_mkimage):
                             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.fast_auth = fdt_util.GetBool(self._node, 'nxp,fast-auth')
+        if not self.fast_auth:
+            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'
+                           ))
+        else:
+            self.srk_crt = os.getenv(
+                'SRK_KEY', fdt_util.GetString(
+                               self._node, 'nxp,srk-crt',
+                               'SRK1_sha256_4096_65537_v3_usr_crt.pem'
+                           ))
+
         self.unlock = fdt_util.GetBool(self._node, 'nxp,unlock')
         self.ReadEntries()
 
@@ -137,8 +149,16 @@ class Entry_nxp_imx8mcst(Entry_mkimage):
         # Load configuration template and modify keys of interest
         config.read_string(csf_config_template)
         config['Install SRK']['File'] = '"' + self.srk_table + '"'
-        config['Install CSFK']['File'] = '"' + self.csf_crt + '"'
-        config['Install Key']['File'] = '"' + self.img_crt + '"'
+        if not self.fast_auth:
+            config.remove_section('Install NOCAK')
+            config['Install CSFK']['File'] = '"' + self.csf_crt + '"'
+            config['Install Key']['File'] = '"' + self.img_crt + '"'
+        else:
+            config.remove_section('Install CSFK')
+            config.remove_section('Install Key')
+            config['Install NOCAK']['File'] = '"' + self.srk_crt + '"'
+            config['Authenticate Data']['Verification index'] = '0'
+
         config['Authenticate Data']['Blocks'] = (hex(signbase) + ' 0 '
                                                  + hex(len(data)) + ' "'
                                                  + str(output_dname) + '"')
-- 
2.39.5

Re: [PATCH v3 2/2] binman: add fast authentication method for i.MX8M signing

[Simon Glass]

Hi Brian,

On Mon, 30 Sept 2024 at 10:10, Brian Ruley <brian.ruley@gehealthcare.com> wrote:
>
> Using the PKI tree with SRKs as intermediate CA isn't necessary or even
> desirable in some situations (boot time, for example). Add the possibility

spelling

> to use the "fast authentication" method where the image and CSF are both
> signed using the SRK [1, p.63].
>
> [1] https://community.nxp.com/pwmxy87654/attachments/pwmxy87654/imx-processors/202591/1/CST_UG.pdf
>
> Signed-off-by: Brian Ruley <brian.ruley@gehealthcare.com>
> Cc: Marek Vasut <marex@denx.de>
>
>  tools/binman/etype/nxp_imx8mcst.py | 23 +++++++++++++++++++----
>  1 file changed, 19 insertions(+), 4 deletions(-)

That should be below the --- (you can use patman to get this right
automatically)
> ---
>  tools/binman/etype/nxp_imx8mcst.py | 44 ++++++++++++++++++++++--------
>  1 file changed, 32 insertions(+), 12 deletions(-)
>
> diff --git a/tools/binman/etype/nxp_imx8mcst.py b/tools/binman/etype/nxp_imx8mcst.py
> index 0c744a00d7..a80cb94499 100644
> --- a/tools/binman/etype/nxp_imx8mcst.py
> +++ b/tools/binman/etype/nxp_imx8mcst.py
> @@ -36,6 +36,9 @@ csf_config_template = '''
>    File = "SRK_1_2_3_4_table.bin"
>    Source index = 0
>
> +[Install NOCAK]
> +  File = "SRK1_sha256_4096_65537_v3_usr_crt.pem"
> +
>  [Install CSFK]
>    File = "CSF1_1_sha256_4096_65537_v3_usr_crt.pem"

Since 'sha256_4096_65537_v3_usr_crt.' is common to everything, could
you have a variable, say keyname, and use that everywhere?

>
> @@ -74,16 +77,25 @@ class Entry_nxp_imx8mcst(Entry_mkimage):
>                              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.fast_auth = fdt_util.GetBool(self._node, 'nxp,fast-auth')
> +        if not self.fast_auth:
> +            self.csf_crt = os.getenv(
> +                'CSF_KEY', fdt_util.GetString(
> +                               self._node, 'nxp,csf-crt',
> +                               'CSF1_1_sha256_4096_65537_v3_usr_crt.pem'

e.g. f'CSF1_1_{keyname}'

> +                           ))
> +            self.img_crt = os.getenv(
> +                'IMG_KEY', fdt_util.GetString(
> +                               self._node, 'nxp,img-crt',
> +                               'IMG1_1_sha256_4096_65537_v3_usr_crt.pem'
> +                           ))
> +        else:
> +            self.srk_crt = os.getenv(
> +                'SRK_KEY', fdt_util.GetString(
> +                               self._node, 'nxp,srk-crt',
> +                               'SRK1_sha256_4096_65537_v3_usr_crt.pem'
> +                           ))

All three options seem to read the 'nxp,srk-crt' property, so you can
do that once the if() to reduce the amount of duplicated code.

> +
>          self.unlock = fdt_util.GetBool(self._node, 'nxp,unlock')
>          self.ReadEntries()
>
> @@ -137,8 +149,16 @@ class Entry_nxp_imx8mcst(Entry_mkimage):
>          # Load configuration template and modify keys of interest
>          config.read_string(csf_config_template)
>          config['Install SRK']['File'] = '"' + self.srk_table + '"'

This is what I mean by the f-string:

f'"{self.srk_table}"'

> -        config['Install CSFK']['File'] = '"' + self.csf_crt + '"'
> -        config['Install Key']['File'] = '"' + self.img_crt + '"'
> +        if not self.fast_auth:
> +            config.remove_section('Install NOCAK')
> +            config['Install CSFK']['File'] = '"' + self.csf_crt + '"'
> +            config['Install Key']['File'] = '"' + self.img_crt + '"'
> +        else:
> +            config.remove_section('Install CSFK')
> +            config.remove_section('Install Key')
> +            config['Install NOCAK']['File'] = '"' + self.srk_crt + '"'
> +            config['Authenticate Data']['Verification index'] = '0'
> +
>          config['Authenticate Data']['Blocks'] = (hex(signbase) + ' 0 '
>                                                   + hex(len(data)) + ' "'
>                                                   + str(output_dname) + '"')

Can use f-strings here too, e.g.

f'{signbase:#x} 0 {len(data):#x} ...

> --
> 2.39.5
>

Regards,
Simon

Re: [PATCH v3 1/2] binman: cosmetic: code formatting fixes

[Simon Glass]

Hi Brian,

On Mon, 30 Sept 2024 at 10:10, Brian Ruley <brian.ruley@gehealthcare.com> wrote:
>
> Conform to the style guide used in the project by making the following
> changes:
> * Use single quotes for multiline strings (except docstrings)
> * Fix line width to 79 cols
> * Use f-string instead of formatting a regular string
>
> Signed-off-by: Brian Ruley <brian.ruley@gehealthcare.com>
> ---
>  tools/binman/etype/nxp_imx8mcst.py | 28 +++++++++++++++++++++-------
>  1 file changed, 21 insertions(+), 7 deletions(-)

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

Thanks for doing this.

Some of my comments on the other patch could be applied here, if you prefer.

>
> diff --git a/tools/binman/etype/nxp_imx8mcst.py b/tools/binman/etype/nxp_imx8mcst.py
> index 8221517b0c..0c744a00d7 100644
> --- a/tools/binman/etype/nxp_imx8mcst.py
> +++ b/tools/binman/etype/nxp_imx8mcst.py
> @@ -23,7 +23,7 @@ from u_boot_pylib import tools
>  MAGIC_NXP_IMX_IVT = 0x412000d1
>  MAGIC_FITIMAGE    = 0xedfe0dd0
>
> -csf_config_template = """
> +csf_config_template = '''
>  [Header]
>    Version = 4.3
>    Hash Algorithm = sha256
> @@ -53,7 +53,7 @@ csf_config_template = """
>  [Authenticate Data]
>    Verification index = 2
>    Blocks = 0x1234 0x78 0xabcd "data.bin"
> -"""
> +'''
>
>  class Entry_nxp_imx8mcst(Entry_mkimage):
>      """NXP i.MX8M CST .cfg file generator and cst invoker
> @@ -69,9 +69,21 @@ 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 = 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.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()
>
> @@ -118,7 +130,7 @@ class Entry_nxp_imx8mcst(Entry_mkimage):
>          tools.write_file(output_dname, data)
>
>          # Generate CST configuration file used to sign payload
> -        cfg_fname = tools.get_output_filename('nxp.csf-config-txt.%s' % uniq)
> +        cfg_fname = tools.get_output_filename(f'nxp.csf-config-txt.{uniq}')
>          config = configparser.ConfigParser()
>          # Do not make key names lowercase
>          config.optionxform = str
> @@ -127,7 +139,9 @@ class Entry_nxp_imx8mcst(Entry_mkimage):
>          config['Install SRK']['File'] = '"' + self.srk_table + '"'
>          config['Install CSFK']['File'] = '"' + self.csf_crt + '"'
>          config['Install Key']['File'] = '"' + self.img_crt + '"'
> -        config['Authenticate Data']['Blocks'] = hex(signbase) + ' 0 ' + hex(len(data)) + ' "' + str(output_dname) + '"'
> +        config['Authenticate Data']['Blocks'] = (hex(signbase) + ' 0 '
> +                                                 + hex(len(data)) + ' "'
> +                                                 + str(output_dname) + '"')
>          if not self.unlock:
>              config.remove_section('Unlock')
>          with open(cfg_fname, 'w') as cfgf:
> --
> 2.39.5
>

Regards,
Simon

[PATCH v4 1/2] binman: cosmetic: refactor `nxp_imx8mcst' etype code

[Brian Ruley]

Simplify code and conform to the style guide used in the project by
making the following changes:
* Capitalize global constants
* Use single quotes for multiline strings (except docstrings)
* Fix line width to 79 cols
* Use f-string instead of formatting a regular string or using a
  complicated concatenation
* Move common suffix used in keys to a global variable "KEY_NAME"
  to reduce the likelihood of typos and making future changes
  easier

Signed-off-by: Brian Ruley <brian.ruley@gehealthcare.com>
Cc: Marek Vasut <marex@denx.de>
---
Changes for v4:
- expand f-string usage, add common information to variable, capitalize
  constants

 tools/binman/etype/nxp_imx8mcst.py | 36 +++++++++++++++++++-----------
 1 file changed, 23 insertions(+), 13 deletions(-)

diff --git a/tools/binman/etype/nxp_imx8mcst.py b/tools/binman/etype/nxp_imx8mcst.py
index 8221517b0c..bd5a5d805f 100644
--- a/tools/binman/etype/nxp_imx8mcst.py
+++ b/tools/binman/etype/nxp_imx8mcst.py
@@ -23,7 +23,9 @@ from u_boot_pylib import tools
 MAGIC_NXP_IMX_IVT = 0x412000d1
 MAGIC_FITIMAGE    = 0xedfe0dd0
 
-csf_config_template = """
+KEY_NAME = 'sha256_4096_65537_v3_usr_crt'
+
+CSF_CONFIG_TEMPLATE = f'''
 [Header]
   Version = 4.3
   Hash Algorithm = sha256
@@ -37,7 +39,7 @@ csf_config_template = """
   Source index = 0
 
 [Install CSFK]
-  File = "CSF1_1_sha256_4096_65537_v3_usr_crt.pem"
+  File = "CSF1_1_{KEY_NAME}.pem"
 
 [Authenticate CSF]
 
@@ -48,12 +50,12 @@ csf_config_template = """
 [Install Key]
   Verification index = 0
   Target Index = 2
-  File = "IMG1_1_sha256_4096_65537_v3_usr_crt.pem"
+  File = "IMG1_1_{KEY_NAME}.pem"
 
 [Authenticate Data]
   Verification index = 2
   Blocks = 0x1234 0x78 0xabcd "data.bin"
-"""
+'''
 
 class Entry_nxp_imx8mcst(Entry_mkimage):
     """NXP i.MX8M CST .cfg file generator and cst invoker
@@ -69,9 +71,15 @@ 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 = 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.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',
+                                          f'CSF1_1_{KEY_NAME}.pem'))
+        self.img_crt = os.getenv(
+            'IMG_KEY', fdt_util.GetString(self._node, 'nxp,img-crt',
+                                          f'IMG1_1_{KEY_NAME}.pem'))
         self.unlock = fdt_util.GetBool(self._node, 'nxp,unlock')
         self.ReadEntries()
 
@@ -118,16 +126,18 @@ class Entry_nxp_imx8mcst(Entry_mkimage):
         tools.write_file(output_dname, data)
 
         # Generate CST configuration file used to sign payload
-        cfg_fname = tools.get_output_filename('nxp.csf-config-txt.%s' % uniq)
+        cfg_fname = tools.get_output_filename(f'nxp.csf-config-txt.{uniq}')
         config = configparser.ConfigParser()
         # Do not make key names lowercase
         config.optionxform = str
         # Load configuration template and modify keys of interest
-        config.read_string(csf_config_template)
-        config['Install SRK']['File'] = '"' + self.srk_table + '"'
-        config['Install CSFK']['File'] = '"' + self.csf_crt + '"'
-        config['Install Key']['File'] = '"' + self.img_crt + '"'
-        config['Authenticate Data']['Blocks'] = hex(signbase) + ' 0 ' + hex(len(data)) + ' "' + str(output_dname) + '"'
+        config.read_string(CSF_CONFIG_TEMPLATE)
+        config['Install SRK']['File']  = f'"{self.srk_table}"'
+        config['Install CSFK']['File'] = f'"{self.csf_crt}"'
+        config['Install Key']['File']  = f'"{self.img_crt}"'
+        config['Authenticate Data']['Blocks'] = \
+            f'{signbase:#x} 0 {len(data):#x} "{output_dname}"'
+
         if not self.unlock:
             config.remove_section('Unlock')
         with open(cfg_fname, 'w') as cfgf:
-- 
2.39.5

[PATCH v4 2/2] binman: add fast authentication method for i.MX8M signing

[Brian Ruley]

Using the PKI tree with SRKs as intermediate CA isn't necessary or even
desirable in some situations (boot time, for example). Add the possibility
to use the "fast authentication" method where the image and CSF are both
signed using the SRK [1, p.63].

[1] https://community.nxp.com/pwmxy87654/attachments/pwmxy87654/imx-processors/202591/1/CST_UG.pdf

Signed-off-by: Brian Ruley <brian.ruley@gehealthcare.com>
Cc: Marek Vasut <marex@denx.de>
---
Changes for v2:
- fixed default key length (s/2048/4096) for srk-crt node
Changes for v3:
- code formatting
Changes for v4:
- fix spelling in commit message
- code formatting

 tools/binman/etype/nxp_imx8mcst.py | 34 +++++++++++++++++++++++-------
 1 file changed, 26 insertions(+), 8 deletions(-)

diff --git a/tools/binman/etype/nxp_imx8mcst.py b/tools/binman/etype/nxp_imx8mcst.py
index bd5a5d805f..a7d8db4eec 100644
--- a/tools/binman/etype/nxp_imx8mcst.py
+++ b/tools/binman/etype/nxp_imx8mcst.py
@@ -38,6 +38,9 @@ CSF_CONFIG_TEMPLATE = f'''
   File = "SRK_1_2_3_4_table.bin"
   Source index = 0
 
+[Install NOCAK]
+  File = "SRK1_{KEY_NAME}.pem"
+
 [Install CSFK]
   File = "CSF1_1_{KEY_NAME}.pem"
 
@@ -74,12 +77,19 @@ class Entry_nxp_imx8mcst(Entry_mkimage):
         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',
-                                          f'CSF1_1_{KEY_NAME}.pem'))
-        self.img_crt = os.getenv(
-            'IMG_KEY', fdt_util.GetString(self._node, 'nxp,img-crt',
-                                          f'IMG1_1_{KEY_NAME}.pem'))
+        self.fast_auth = fdt_util.GetBool(self._node, 'nxp,fast-auth')
+        if not self.fast_auth:
+            self.csf_crt = os.getenv(
+                'CSF_KEY', fdt_util.GetString(self._node, 'nxp,csf-crt',
+                                              f'CSF1_1_{KEY_NAME}.pem'))
+            self.img_crt = os.getenv(
+                'IMG_KEY', fdt_util.GetString(self._node, 'nxp,img-crt',
+                                              f'IMG1_1_{KEY_NAME}.pem'))
+        else:
+            self.srk_crt = os.getenv(
+                'SRK_KEY', fdt_util.GetString(self._node, 'nxp,srk-crt',
+                                              f'SRK1_{KEY_NAME}.pem'))
+
         self.unlock = fdt_util.GetBool(self._node, 'nxp,unlock')
         self.ReadEntries()
 
@@ -133,8 +143,16 @@ class Entry_nxp_imx8mcst(Entry_mkimage):
         # Load configuration template and modify keys of interest
         config.read_string(CSF_CONFIG_TEMPLATE)
         config['Install SRK']['File']  = f'"{self.srk_table}"'
-        config['Install CSFK']['File'] = f'"{self.csf_crt}"'
-        config['Install Key']['File']  = f'"{self.img_crt}"'
+        if not self.fast_auth:
+            config.remove_section('Install NOCAK')
+            config['Install CSFK']['File'] = f'"{self.csf_crt}"'
+            config['Install Key']['File']  = f'"{self.img_crt}"'
+        else:
+            config.remove_section('Install CSFK')
+            config.remove_section('Install Key')
+            config['Install NOCAK']['File'] = f'"{self.srk_crt}"'
+            config['Authenticate Data']['Verification index'] = '0'
+
         config['Authenticate Data']['Blocks'] = \
             f'{signbase:#x} 0 {len(data):#x} "{output_dname}"'
 
-- 
2.39.5

Re: [PATCH v3 1/2] binman: cosmetic: code formatting fixes

[Brian Ruley]

On Mon, Sep 30, 2024 at 12:52:24PM -0600, Simon Glass wrote:
> 
> WARNING: This email originated from outside of GE HealthCare. Please validate the sender's email address before clicking on links or attachments as they may not be safe.
> 
> Hi Brian,
> 
> On Mon, 30 Sept 2024 at 10:10, Brian Ruley <brian.ruley@gehealthcare.com> wrote:
> >
> > Conform to the style guide used in the project by making the following
> > changes:
> > * Use single quotes for multiline strings (except docstrings)
> > * Fix line width to 79 cols
> > * Use f-string instead of formatting a regular string
> >
> > Signed-off-by: Brian Ruley <brian.ruley@gehealthcare.com>
> > ---
> >  tools/binman/etype/nxp_imx8mcst.py | 28 +++++++++++++++++++++-------
> >  1 file changed, 21 insertions(+), 7 deletions(-)
> 
> Reviewed-by: Simon Glass <sjg@chromium.org>
> 
> Thanks for doing this.
> 
> Some of my comments on the other patch could be applied here, if you prefer.
> 
> >
> > diff --git a/tools/binman/etype/nxp_imx8mcst.py b/tools/binman/etype/nxp_imx8mcst.py
> > index 8221517b0c..0c744a00d7 100644
> > --- a/tools/binman/etype/nxp_imx8mcst.py
> > +++ b/tools/binman/etype/nxp_imx8mcst.py
> > @@ -23,7 +23,7 @@ from u_boot_pylib import tools
> >  MAGIC_NXP_IMX_IVT = 0x412000d1
> >  MAGIC_FITIMAGE    = 0xedfe0dd0
> >
> > -csf_config_template = """
> > +csf_config_template = '''
> >  [Header]
> >    Version = 4.3
> >    Hash Algorithm = sha256
> > @@ -53,7 +53,7 @@ csf_config_template = """
> >  [Authenticate Data]
> >    Verification index = 2
> >    Blocks = 0x1234 0x78 0xabcd "data.bin"
> > -"""
> > +'''
> >
> >  class Entry_nxp_imx8mcst(Entry_mkimage):
> >      """NXP i.MX8M CST .cfg file generator and cst invoker
> > @@ -69,9 +69,21 @@ 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 = 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.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()
> >
> > @@ -118,7 +130,7 @@ class Entry_nxp_imx8mcst(Entry_mkimage):
> >          tools.write_file(output_dname, data)
> >
> >          # Generate CST configuration file used to sign payload
> > -        cfg_fname = tools.get_output_filename('nxp.csf-config-txt.%s' % uniq)
> > +        cfg_fname = tools.get_output_filename(f'nxp.csf-config-txt.{uniq}')
> >          config = configparser.ConfigParser()
> >          # Do not make key names lowercase
> >          config.optionxform = str
> > @@ -127,7 +139,9 @@ class Entry_nxp_imx8mcst(Entry_mkimage):
> >          config['Install SRK']['File'] = '"' + self.srk_table + '"'
> >          config['Install CSFK']['File'] = '"' + self.csf_crt + '"'
> >          config['Install Key']['File'] = '"' + self.img_crt + '"'
> > -        config['Authenticate Data']['Blocks'] = hex(signbase) + ' 0 ' + hex(len(data)) + ' "' + str(output_dname) + '"'
> > +        config['Authenticate Data']['Blocks'] = (hex(signbase) + ' 0 '
> > +                                                 + hex(len(data)) + ' "'
> > +                                                 + str(output_dname) + '"')
> >          if not self.unlock:
> >              config.remove_section('Unlock')
> >          with open(cfg_fname, 'w') as cfgf:
> > --
> > 2.39.5
> >
> 
> Regards,
> Simon

Hi Simon,

Sure thing, I'm glad to contribute. I'm quite new to upstreaming so this
is a good learning experience for me, and I thank you for your patience.

I didn't have time to respond yesterday, but I sent a new revision
based on your comments.

Only one I didn't quite get was

> All three options seem to read the 'nxp,srk-crt' property, so you can
> do that once the if() to reduce the amount of duplicated code.

Each is reading a different property "img-crt", "csf-crt", and
"srk-crt", with the latter read only if "fast-auth" is set.

As for the testing part, after I got `binman test` working, I
experimented with creating a test for the `nxp-imx8mcst` etype, using
what you had done as a starting point. FYI, it doesn't depend on the
`nxp-imx8mimage` because it's for invoking `mkimage` when building
SPL, for example. So, what I did is just create a fake FIT image
inside the nxp-imx8mcst node, and run the test. I can send the patch
if you wish.

The test, however, was unsuccessful because a PKI tree is needed to
sign the assets. I thought maybe you would have a comment on that?

Best,
Brian

Re: [PATCH v3 1/2] binman: cosmetic: code formatting fixes

[Simon Glass]

Hi Brian,

On Wed, 2 Oct 2024 at 00:41, Brian Ruley <brian.ruley@gehealthcare.com> wrote:
>
> On Mon, Sep 30, 2024 at 12:52:24PM -0600, Simon Glass wrote:
> >
> > WARNING: This email originated from outside of GE HealthCare. Please validate the sender's email address before clicking on links or attachments as they may not be safe.
> >
> > Hi Brian,
> >
> > On Mon, 30 Sept 2024 at 10:10, Brian Ruley <brian.ruley@gehealthcare.com> wrote:
> > >
> > > Conform to the style guide used in the project by making the following
> > > changes:
> > > * Use single quotes for multiline strings (except docstrings)
> > > * Fix line width to 79 cols
> > > * Use f-string instead of formatting a regular string
> > >
> > > Signed-off-by: Brian Ruley <brian.ruley@gehealthcare.com>
> > > ---
> > >  tools/binman/etype/nxp_imx8mcst.py | 28 +++++++++++++++++++++-------
> > >  1 file changed, 21 insertions(+), 7 deletions(-)
> >
> > Reviewed-by: Simon Glass <sjg@chromium.org>
> >
> > Thanks for doing this.
> >
> > Some of my comments on the other patch could be applied here, if you prefer.
> >
> > >
> > > diff --git a/tools/binman/etype/nxp_imx8mcst.py b/tools/binman/etype/nxp_imx8mcst.py
> > > index 8221517b0c..0c744a00d7 100644
> > > --- a/tools/binman/etype/nxp_imx8mcst.py
> > > +++ b/tools/binman/etype/nxp_imx8mcst.py
> > > @@ -23,7 +23,7 @@ from u_boot_pylib import tools
> > >  MAGIC_NXP_IMX_IVT = 0x412000d1
> > >  MAGIC_FITIMAGE    = 0xedfe0dd0
> > >
> > > -csf_config_template = """
> > > +csf_config_template = '''
> > >  [Header]
> > >    Version = 4.3
> > >    Hash Algorithm = sha256
> > > @@ -53,7 +53,7 @@ csf_config_template = """
> > >  [Authenticate Data]
> > >    Verification index = 2
> > >    Blocks = 0x1234 0x78 0xabcd "data.bin"
> > > -"""
> > > +'''
> > >
> > >  class Entry_nxp_imx8mcst(Entry_mkimage):
> > >      """NXP i.MX8M CST .cfg file generator and cst invoker
> > > @@ -69,9 +69,21 @@ 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 = 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.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()
> > >
> > > @@ -118,7 +130,7 @@ class Entry_nxp_imx8mcst(Entry_mkimage):
> > >          tools.write_file(output_dname, data)
> > >
> > >          # Generate CST configuration file used to sign payload
> > > -        cfg_fname = tools.get_output_filename('nxp.csf-config-txt.%s' % uniq)
> > > +        cfg_fname = tools.get_output_filename(f'nxp.csf-config-txt.{uniq}')
> > >          config = configparser.ConfigParser()
> > >          # Do not make key names lowercase
> > >          config.optionxform = str
> > > @@ -127,7 +139,9 @@ class Entry_nxp_imx8mcst(Entry_mkimage):
> > >          config['Install SRK']['File'] = '"' + self.srk_table + '"'
> > >          config['Install CSFK']['File'] = '"' + self.csf_crt + '"'
> > >          config['Install Key']['File'] = '"' + self.img_crt + '"'
> > > -        config['Authenticate Data']['Blocks'] = hex(signbase) + ' 0 ' + hex(len(data)) + ' "' + str(output_dname) + '"'
> > > +        config['Authenticate Data']['Blocks'] = (hex(signbase) + ' 0 '
> > > +                                                 + hex(len(data)) + ' "'
> > > +                                                 + str(output_dname) + '"')
> > >          if not self.unlock:
> > >              config.remove_section('Unlock')
> > >          with open(cfg_fname, 'w') as cfgf:
> > > --
> > > 2.39.5
> > >
> >
> > Regards,
> > Simon
>
> Hi Simon,
>
> Sure thing, I'm glad to contribute. I'm quite new to upstreaming so this
> is a good learning experience for me, and I thank you for your patience.

Well you seem to have figured it out :-)

>
> I didn't have time to respond yesterday, but I sent a new revision
> based on your comments.
>
> Only one I didn't quite get was
>
> > All three options seem to read the 'nxp,srk-crt' property, so you can
> > do that once the if() to reduce the amount of duplicated code.
>
> Each is reading a different property "img-crt", "csf-crt", and
> "srk-crt", with the latter read only if "fast-auth" is set.

Yes I see that now.

>
> As for the testing part, after I got `binman test` working, I
> experimented with creating a test for the `nxp-imx8mcst` etype, using
> what you had done as a starting point. FYI, it doesn't depend on the
> `nxp-imx8mimage` because it's for invoking `mkimage` when building
> SPL, for example. So, what I did is just create a fake FIT image
> inside the nxp-imx8mcst node, and run the test. I can send the patch
> if you wish.
>
> The test, however, was unsuccessful because a PKI tree is needed to
> sign the assets. I thought maybe you would have a comment on that?

Could you use a test file? There is another etype which has test
yaml-files in tools/binman/test/yaml/

There is also tools/binman/test/key.key , I think for the vblock etype.

Regards,
Simon

Re: [PATCH v3 1/2] binman: cosmetic: code formatting fixes

[Brian Ruley]

On Wed, Oct 02, 2024 at 04:55:30PM -0600, Simon Glass wrote:
> 
> Hi Brian,
> 
> On Wed, 2 Oct 2024 at 00:41, Brian Ruley <brian.ruley@gehealthcare.com> wrote:
> >
> > On Mon, Sep 30, 2024 at 12:52:24PM -0600, Simon Glass wrote:
> > >
> > > WARNING: This email originated from outside of GE HealthCare. Please validate the sender's email address before clicking on links or attachments as they may not be safe.
> > >
> > > Hi Brian,
> > >
> > > On Mon, 30 Sept 2024 at 10:10, Brian Ruley <brian.ruley@gehealthcare.com> wrote:
> > > >
> > > > Conform to the style guide used in the project by making the following
> > > > changes:
> > > > * Use single quotes for multiline strings (except docstrings)
> > > > * Fix line width to 79 cols
> > > > * Use f-string instead of formatting a regular string
> > > >
> > > > Signed-off-by: Brian Ruley <brian.ruley@gehealthcare.com>
> > > > ---
> > > >  tools/binman/etype/nxp_imx8mcst.py | 28 +++++++++++++++++++++-------
> > > >  1 file changed, 21 insertions(+), 7 deletions(-)
> > >
> > > Reviewed-by: Simon Glass <sjg@chromium.org>
> > >
> > > Thanks for doing this.
> > >
> > > Some of my comments on the other patch could be applied here, if you prefer.
> > >
> > > >
> > > > diff --git a/tools/binman/etype/nxp_imx8mcst.py b/tools/binman/etype/nxp_imx8mcst.py
> > > > index 8221517b0c..0c744a00d7 100644
> > > > --- a/tools/binman/etype/nxp_imx8mcst.py
> > > > +++ b/tools/binman/etype/nxp_imx8mcst.py
> > > > @@ -23,7 +23,7 @@ from u_boot_pylib import tools
> > > >  MAGIC_NXP_IMX_IVT = 0x412000d1
> > > >  MAGIC_FITIMAGE    = 0xedfe0dd0
> > > >
> > > > -csf_config_template = """
> > > > +csf_config_template = '''
> > > >  [Header]
> > > >    Version = 4.3
> > > >    Hash Algorithm = sha256
> > > > @@ -53,7 +53,7 @@ csf_config_template = """
> > > >  [Authenticate Data]
> > > >    Verification index = 2
> > > >    Blocks = 0x1234 0x78 0xabcd "data.bin"
> > > > -"""
> > > > +'''
> > > >
> > > >  class Entry_nxp_imx8mcst(Entry_mkimage):
> > > >      """NXP i.MX8M CST .cfg file generator and cst invoker
> > > > @@ -69,9 +69,21 @@ 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 = 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.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()
> > > >
> > > > @@ -118,7 +130,7 @@ class Entry_nxp_imx8mcst(Entry_mkimage):
> > > >          tools.write_file(output_dname, data)
> > > >
> > > >          # Generate CST configuration file used to sign payload
> > > > -        cfg_fname = tools.get_output_filename('nxp.csf-config-txt.%s' % uniq)
> > > > +        cfg_fname = tools.get_output_filename(f'nxp.csf-config-txt.{uniq}')
> > > >          config = configparser.ConfigParser()
> > > >          # Do not make key names lowercase
> > > >          config.optionxform = str
> > > > @@ -127,7 +139,9 @@ class Entry_nxp_imx8mcst(Entry_mkimage):
> > > >          config['Install SRK']['File'] = '"' + self.srk_table + '"'
> > > >          config['Install CSFK']['File'] = '"' + self.csf_crt + '"'
> > > >          config['Install Key']['File'] = '"' + self.img_crt + '"'
> > > > -        config['Authenticate Data']['Blocks'] = hex(signbase) + ' 0 ' + hex(len(data)) + ' "' + str(output_dname) + '"'
> > > > +        config['Authenticate Data']['Blocks'] = (hex(signbase) + ' 0 '
> > > > +                                                 + hex(len(data)) + ' "'
> > > > +                                                 + str(output_dname) + '"')
> > > >          if not self.unlock:
> > > >              config.remove_section('Unlock')
> > > >          with open(cfg_fname, 'w') as cfgf:
> > > > --
> > > > 2.39.5
> > > >
> > >
> > > Regards,
> > > Simon
> >
> > Hi Simon,
> >
> > Sure thing, I'm glad to contribute. I'm quite new to upstreaming so this
> > is a good learning experience for me, and I thank you for your patience.
> 
> Well you seem to have figured it out :-)
> 
> >
> > I didn't have time to respond yesterday, but I sent a new revision
> > based on your comments.
> >
> > Only one I didn't quite get was
> >
> > > All three options seem to read the 'nxp,srk-crt' property, so you can
> > > do that once the if() to reduce the amount of duplicated code.
> >
> > Each is reading a different property "img-crt", "csf-crt", and
> > "srk-crt", with the latter read only if "fast-auth" is set.
> 
> Yes I see that now.
> 
> >
> > As for the testing part, after I got `binman test` working, I
> > experimented with creating a test for the `nxp-imx8mcst` etype, using
> > what you had done as a starting point. FYI, it doesn't depend on the
> > `nxp-imx8mimage` because it's for invoking `mkimage` when building
> > SPL, for example. So, what I did is just create a fake FIT image
> > inside the nxp-imx8mcst node, and run the test. I can send the patch
> > if you wish.
> >
> > The test, however, was unsuccessful because a PKI tree is needed to
> > sign the assets. I thought maybe you would have a comment on that?
> 
> Could you use a test file? There is another etype which has test
> yaml-files in tools/binman/test/yaml/
> 
> There is also tools/binman/test/key.key , I think for the vblock etype.
> 
> Regards,
> Simon

Hi Simon,

I got the test working.

> Could you use a test file? There is another etype which has test
> yaml-files in tools/binman/test/yaml/

Like a test FIT image or what? The image has to be either a FIT or
mkimage.

> There is also tools/binman/test/key.key , I think for the vblock etype.

I tried using it but didn't get it to work (tbh, I didn't try that many
times). Regardless, it's just easier to create keys of the type CST
expects, and include them in the test. I'll send the patch, so you can
take a look.

Also, you haven't responded to the v4 patch I sent -- is it good? Just
want to make sure if there's something I need to do still regarding
that.

Best,
Brian

Re: [PATCH v4 2/2] binman: add fast authentication method for i.MX8M signing

[Fabio Estevam]

Hi Simon,

On Tue, Oct 1, 2024 at 2:27 PM Brian Ruley <brian.ruley@gehealthcare.com> wrote:
>
> Using the PKI tree with SRKs as intermediate CA isn't necessary or even
> desirable in some situations (boot time, for example). Add the possibility
> to use the "fast authentication" method where the image and CSF are both
> signed using the SRK [1, p.63].
>
> [1] https://community.nxp.com/pwmxy87654/attachments/pwmxy87654/imx-processors/202591/1/CST_UG.pdf
>
> Signed-off-by: Brian Ruley <brian.ruley@gehealthcare.com>
> Cc: Marek Vasut <marex@denx.de>
> ---
> Changes for v2:
> - fixed default key length (s/2048/4096) for srk-crt node
> Changes for v3:
> - code formatting
> Changes for v4:
> - fix spelling in commit message
> - code formatting

Are you happy with this series?

Re: [PATCH v3 1/2] binman: cosmetic: code formatting fixes

[Simon Glass]

Hi Brian,

On Mon, 7 Oct 2024 at 06:33, Brian Ruley <brian.ruley@gehealthcare.com> wrote:
>
> On Wed, Oct 02, 2024 at 04:55:30PM -0600, Simon Glass wrote:
> >
> > Hi Brian,
> >
> > On Wed, 2 Oct 2024 at 00:41, Brian Ruley <brian.ruley@gehealthcare.com> wrote:
> > >
> > > On Mon, Sep 30, 2024 at 12:52:24PM -0600, Simon Glass wrote:
> > > >
> > > > WARNING: This email originated from outside of GE HealthCare. Please validate the sender's email address before clicking on links or attachments as they may not be safe.
> > > >
> > > > Hi Brian,
> > > >
> > > > On Mon, 30 Sept 2024 at 10:10, Brian Ruley <brian.ruley@gehealthcare.com> wrote:
> > > > >
> > > > > Conform to the style guide used in the project by making the following
> > > > > changes:
> > > > > * Use single quotes for multiline strings (except docstrings)
> > > > > * Fix line width to 79 cols
> > > > > * Use f-string instead of formatting a regular string
> > > > >
> > > > > Signed-off-by: Brian Ruley <brian.ruley@gehealthcare.com>
> > > > > ---
> > > > >  tools/binman/etype/nxp_imx8mcst.py | 28 +++++++++++++++++++++-------
> > > > >  1 file changed, 21 insertions(+), 7 deletions(-)
> > > >
> > > > Reviewed-by: Simon Glass <sjg@chromium.org>
> > > >
> > > > Thanks for doing this.
> > > >
> > > > Some of my comments on the other patch could be applied here, if you prefer.
> > > >
> > > > >
> > > > > diff --git a/tools/binman/etype/nxp_imx8mcst.py b/tools/binman/etype/nxp_imx8mcst.py
> > > > > index 8221517b0c..0c744a00d7 100644
> > > > > --- a/tools/binman/etype/nxp_imx8mcst.py
> > > > > +++ b/tools/binman/etype/nxp_imx8mcst.py
> > > > > @@ -23,7 +23,7 @@ from u_boot_pylib import tools
> > > > >  MAGIC_NXP_IMX_IVT = 0x412000d1
> > > > >  MAGIC_FITIMAGE    = 0xedfe0dd0
> > > > >
> > > > > -csf_config_template = """
> > > > > +csf_config_template = '''
> > > > >  [Header]
> > > > >    Version = 4.3
> > > > >    Hash Algorithm = sha256
> > > > > @@ -53,7 +53,7 @@ csf_config_template = """
> > > > >  [Authenticate Data]
> > > > >    Verification index = 2
> > > > >    Blocks = 0x1234 0x78 0xabcd "data.bin"
> > > > > -"""
> > > > > +'''
> > > > >
> > > > >  class Entry_nxp_imx8mcst(Entry_mkimage):
> > > > >      """NXP i.MX8M CST .cfg file generator and cst invoker
> > > > > @@ -69,9 +69,21 @@ 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 = 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.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()
> > > > >
> > > > > @@ -118,7 +130,7 @@ class Entry_nxp_imx8mcst(Entry_mkimage):
> > > > >          tools.write_file(output_dname, data)
> > > > >
> > > > >          # Generate CST configuration file used to sign payload
> > > > > -        cfg_fname = tools.get_output_filename('nxp.csf-config-txt.%s' % uniq)
> > > > > +        cfg_fname = tools.get_output_filename(f'nxp.csf-config-txt.{uniq}')
> > > > >          config = configparser.ConfigParser()
> > > > >          # Do not make key names lowercase
> > > > >          config.optionxform = str
> > > > > @@ -127,7 +139,9 @@ class Entry_nxp_imx8mcst(Entry_mkimage):
> > > > >          config['Install SRK']['File'] = '"' + self.srk_table + '"'
> > > > >          config['Install CSFK']['File'] = '"' + self.csf_crt + '"'
> > > > >          config['Install Key']['File'] = '"' + self.img_crt + '"'
> > > > > -        config['Authenticate Data']['Blocks'] = hex(signbase) + ' 0 ' + hex(len(data)) + ' "' + str(output_dname) + '"'
> > > > > +        config['Authenticate Data']['Blocks'] = (hex(signbase) + ' 0 '
> > > > > +                                                 + hex(len(data)) + ' "'
> > > > > +                                                 + str(output_dname) + '"')
> > > > >          if not self.unlock:
> > > > >              config.remove_section('Unlock')
> > > > >          with open(cfg_fname, 'w') as cfgf:
> > > > > --
> > > > > 2.39.5
> > > > >
> > > >
> > > > Regards,
> > > > Simon
> > >
> > > Hi Simon,
> > >
> > > Sure thing, I'm glad to contribute. I'm quite new to upstreaming so this
> > > is a good learning experience for me, and I thank you for your patience.
> >
> > Well you seem to have figured it out :-)
> >
> > >
> > > I didn't have time to respond yesterday, but I sent a new revision
> > > based on your comments.
> > >
> > > Only one I didn't quite get was
> > >
> > > > All three options seem to read the 'nxp,srk-crt' property, so you can
> > > > do that once the if() to reduce the amount of duplicated code.
> > >
> > > Each is reading a different property "img-crt", "csf-crt", and
> > > "srk-crt", with the latter read only if "fast-auth" is set.
> >
> > Yes I see that now.
> >
> > >
> > > As for the testing part, after I got `binman test` working, I
> > > experimented with creating a test for the `nxp-imx8mcst` etype, using
> > > what you had done as a starting point. FYI, it doesn't depend on the
> > > `nxp-imx8mimage` because it's for invoking `mkimage` when building
> > > SPL, for example. So, what I did is just create a fake FIT image
> > > inside the nxp-imx8mcst node, and run the test. I can send the patch
> > > if you wish.
> > >
> > > The test, however, was unsuccessful because a PKI tree is needed to
> > > sign the assets. I thought maybe you would have a comment on that?
> >
> > Could you use a test file? There is another etype which has test
> > yaml-files in tools/binman/test/yaml/
> >
> > There is also tools/binman/test/key.key , I think for the vblock etype.
> >
> > Regards,
> > Simon
>
> Hi Simon,
>
> I got the test working.

Great!

>
> > Could you use a test file? There is another etype which has test
> > yaml-files in tools/binman/test/yaml/
>
> Like a test FIT image or what? The image has to be either a FIT or
> mkimage.

Sure, if that is really necessary. Remember that it doesn't actually
need to create a valid image. It just needs to test the code in the
entry type. But if FIT is the easiest thing to do, then that's fine.

>
> > There is also tools/binman/test/key.key , I think for the vblock etype.
>
> I tried using it but didn't get it to work (tbh, I didn't try that many
> times). Regardless, it's just easier to create keys of the type CST
> expects, and include them in the test. I'll send the patch, so you can
> take a look.
>
> Also, you haven't responded to the v4 patch I sent -- is it good? Just
> want to make sure if there's something I need to do still regarding
> that.

I just haven't had time for anything recently...it happens from time
to time and I normally catch up eventually. But if there is no review
for long enough, Tom will likely apply it after having a look.

Regards,
Simon

Re: [PATCH v4 2/2] binman: add fast authentication method for i.MX8M signing

[Simon Glass]

On Tue, 1 Oct 2024 at 07:58, Brian Ruley <brian.ruley@gehealthcare.com> wrote:
>
> Using the PKI tree with SRKs as intermediate CA isn't necessary or even
> desirable in some situations (boot time, for example). Add the possibility
> to use the "fast authentication" method where the image and CSF are both
> signed using the SRK [1, p.63].
>
> [1] https://community.nxp.com/pwmxy87654/attachments/pwmxy87654/imx-processors/202591/1/CST_UG.pdf
>
> Signed-off-by: Brian Ruley <brian.ruley@gehealthcare.com>
> Cc: Marek Vasut <marex@denx.de>
> ---
> Changes for v2:
> - fixed default key length (s/2048/4096) for srk-crt node
> Changes for v3:
> - code formatting
> Changes for v4:
> - fix spelling in commit message
> - code formatting
>
>  tools/binman/etype/nxp_imx8mcst.py | 34 +++++++++++++++++++++++-------
>  1 file changed, 26 insertions(+), 8 deletions(-)

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

(since I am seeing progress on testing, thank you)

Re: [PATCH v4 2/2] binman: add fast authentication method for i.MX8M signing

[Fabio Estevam]

On Tue, Oct 8, 2024 at 11:17 PM Simon Glass <sjg@chromium.org> wrote:
>
> On Tue, 1 Oct 2024 at 07:58, Brian Ruley <brian.ruley@gehealthcare.com> wrote:
> >
> > Using the PKI tree with SRKs as intermediate CA isn't necessary or even
> > desirable in some situations (boot time, for example). Add the possibility
> > to use the "fast authentication" method where the image and CSF are both
> > signed using the SRK [1, p.63].
> >
> > [1] https://community.nxp.com/pwmxy87654/attachments/pwmxy87654/imx-processors/202591/1/CST_UG.pdf
> >
> > Signed-off-by: Brian Ruley <brian.ruley@gehealthcare.com>
> > Cc: Marek Vasut <marex@denx.de>

> Reviewed-by: Simon Glass <sjg@chromium.org>
>
> (since I am seeing progress on testing, thank you)

Applied both, thanks.