diff mbox series

[2/2] cmd: Add CONFIG_FIT_SIGNATURE_STRICT

Message ID 20210916130958.306964-3-oleksandr.suvorov@foundries.io
State Changes Requested
Delegated to: Tom Rini
Headers show
Series Enable strict signature verification for FIT | expand

Commit Message

Oleksandr Suvorov Sept. 16, 2021, 1:09 p.m. UTC 
From: Ricardo Salveti <ricardo@foundries.io>

Add CONFIG_FIT_SIGNATURE_STRICT to require a valid FIT configuration
signature for each command that is able to manipulate FIT images.

Signed-off-by: Ricardo Salveti <ricardo@foundries.io>
Co-developed-by: Oleksandr Suvorov <oleksandr.suvorov@foundries.io>
Signed-off-by: Oleksandr Suvorov <oleksandr.suvorov@foundries.io>
---

 cmd/fpga.c          | 14 ++++++++++++++
 cmd/source.c        | 14 ++++++++++++++
 cmd/ximg.c          | 14 ++++++++++++++
 common/Kconfig.boot |  4 ++++
 4 files changed, 46 insertions(+)

Comments

Igor Opaniuk Sept. 16, 2021, 3:09 p.m. UTC | #1 
Hi Oleksandr,

On Thu, Sep 16, 2021 at 4:10 PM Oleksandr Suvorov
<oleksandr.suvorov@foundries.io> wrote:
>
> From: Ricardo Salveti <ricardo@foundries.io>
>
> Add CONFIG_FIT_SIGNATURE_STRICT to require a valid FIT configuration
> signature for each command that is able to manipulate FIT images.
>
> Signed-off-by: Ricardo Salveti <ricardo@foundries.io>
> Co-developed-by: Oleksandr Suvorov <oleksandr.suvorov@foundries.io>
> Signed-off-by: Oleksandr Suvorov <oleksandr.suvorov@foundries.io>
> ---
>
>  cmd/fpga.c          | 14 ++++++++++++++
>  cmd/source.c        | 14 ++++++++++++++
>  cmd/ximg.c          | 14 ++++++++++++++
>  common/Kconfig.boot |  4 ++++
>  4 files changed, 46 insertions(+)
>
> diff --git a/cmd/fpga.c b/cmd/fpga.c
> index 3fdd0b35e80..16d329590fa 100644
> --- a/cmd/fpga.c
> +++ b/cmd/fpga.c
> @@ -335,6 +335,20 @@ static int do_fpga_loadmk(struct cmd_tbl *cmdtp, int flag, int argc,
>                         return CMD_RET_FAILURE;
>                 }
>
> +               if (CONFIG_IS_ENABLED(FIT_SIGNATURE_STRICT)) {
> +                       /* validate required fit config entry */
> +                       noffset = fit_conf_get_node(fit_hdr, NULL);
> +                       if (noffset >= 0) {
> +                               if (fit_config_verify(fit_hdr, noffset)) {
> +                                       puts("Cannot verify FIT config node\n");
> +                                       return CMD_RET_FAILURE;
> +                               }
> +                       } else {
> +                               puts("FIT_SIGNATURE_STRICT requires a config node\n");
> +                               return CMD_RET_FAILURE;
> +                       }
> +               }
> +
>                 /* get fpga component image node offset */
>                 noffset = fit_image_get_node(fit_hdr, fit_uname);
>                 if (noffset < 0) {
> diff --git a/cmd/source.c b/cmd/source.c
> index 81e015b64ef..b08406dfcbf 100644
> --- a/cmd/source.c
> +++ b/cmd/source.c
> @@ -112,6 +112,20 @@ int image_source_script(ulong addr, const char *fit_uname)
>                         return 1;
>                 }
>
> +               if (CONFIG_IS_ENABLED(FIT_SIGNATURE_STRICT)) {
> +                       /* validate required fit config entry */
> +                       noffset = fit_conf_get_node(fit_hdr, NULL);
> +                       if (noffset >= 0) {
> +                               if (fit_config_verify(fit_hdr, noffset)) {
> +                                       puts("Cannot verify FIT config node\n");
> +                                       return 1;
> +                               }
> +                       } else {
> +                               puts("FIT_SIGNATURE_STRICT requires a config node\n");
> +                               return 1;
> +                       }
> +               }
> +
>                 if (!fit_uname)
>                         fit_uname = get_default_image(fit_hdr);
>
> diff --git a/cmd/ximg.c b/cmd/ximg.c
> index 65ba41320a0..39fccd8179c 100644
> --- a/cmd/ximg.c
> +++ b/cmd/ximg.c
> @@ -141,6 +141,20 @@ do_imgextract(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
>                         return 1;
>                 }
>
> +               if (CONFIG_IS_ENABLED(FIT_SIGNATURE_STRICT)) {
> +                       /* validate required fit config entry */
> +                       noffset = fit_conf_get_node(fit_hdr, NULL);
> +                       if (noffset >= 0) {
> +                               if (fit_config_verify(fit_hdr, noffset)) {
> +                                       puts("Cannot verify FIT config node\n");
> +                                       return 1;
> +                               }
> +                       } else {
> +                               puts("FIT_SIGNATURE_STRICT requires a config node\n");
> +                               return 1;
> +                       }
> +               }
> +
>                 /* get subimage node offset */
>                 noffset = fit_image_get_node(fit_hdr, uname);
>                 if (noffset < 0) {
> diff --git a/common/Kconfig.boot b/common/Kconfig.boot
> index 6f95d009dfa..ca7d9a8d971 100644
> --- a/common/Kconfig.boot
> +++ b/common/Kconfig.boot
> @@ -77,6 +77,10 @@ config FIT_SIGNATURE_MAX_SIZE
>           device memory. Assure this size does not extend past expected storage
>           space.
>
> +config FIT_SIGNATURE_STRICT
> +       bool "Requires a valid FIT configuration signature for every image"
> +       select FIT_SIGNATURE
> +
>  config FIT_RSASSA_PSS
>         bool "Support rsassa-pss signature scheme of FIT image contents"
>         depends on FIT_SIGNATURE
> --
> 2.31.1
>

There is duplication of the same piece of code in 3 different files,
maybe you could rework
that and move to some common function?
Oleksandr Suvorov Sept. 16, 2021, 5:54 p.m. UTC | #2 
Hi Igor,

On Thu, Sep 16, 2021 at 6:09 PM Igor Opaniuk <igor.opaniuk@foundries.io> wrote:
>
> Hi Oleksandr,
>
> On Thu, Sep 16, 2021 at 4:10 PM Oleksandr Suvorov
> <oleksandr.suvorov@foundries.io> wrote:
> >
> > From: Ricardo Salveti <ricardo@foundries.io>
> >
> > Add CONFIG_FIT_SIGNATURE_STRICT to require a valid FIT configuration
> > signature for each command that is able to manipulate FIT images.
> >
> > Signed-off-by: Ricardo Salveti <ricardo@foundries.io>
> > Co-developed-by: Oleksandr Suvorov <oleksandr.suvorov@foundries.io>
> > Signed-off-by: Oleksandr Suvorov <oleksandr.suvorov@foundries.io>
> > ---
> >
> >  cmd/fpga.c          | 14 ++++++++++++++
> >  cmd/source.c        | 14 ++++++++++++++
> >  cmd/ximg.c          | 14 ++++++++++++++
> >  common/Kconfig.boot |  4 ++++
> >  4 files changed, 46 insertions(+)
> >
> > diff --git a/cmd/fpga.c b/cmd/fpga.c
> > index 3fdd0b35e80..16d329590fa 100644
> > --- a/cmd/fpga.c
> > +++ b/cmd/fpga.c
> > @@ -335,6 +335,20 @@ static int do_fpga_loadmk(struct cmd_tbl *cmdtp, int flag, int argc,
> >                         return CMD_RET_FAILURE;
> >                 }
> >
> > +               if (CONFIG_IS_ENABLED(FIT_SIGNATURE_STRICT)) {
> > +                       /* validate required fit config entry */
> > +                       noffset = fit_conf_get_node(fit_hdr, NULL);
> > +                       if (noffset >= 0) {
> > +                               if (fit_config_verify(fit_hdr, noffset)) {
> > +                                       puts("Cannot verify FIT config node\n");
> > +                                       return CMD_RET_FAILURE;
> > +                               }
> > +                       } else {
> > +                               puts("FIT_SIGNATURE_STRICT requires a config node\n");
> > +                               return CMD_RET_FAILURE;
> > +                       }
> > +               }
> > +
> >                 /* get fpga component image node offset */
> >                 noffset = fit_image_get_node(fit_hdr, fit_uname);
> >                 if (noffset < 0) {
> > diff --git a/cmd/source.c b/cmd/source.c
> > index 81e015b64ef..b08406dfcbf 100644
> > --- a/cmd/source.c
> > +++ b/cmd/source.c
> > @@ -112,6 +112,20 @@ int image_source_script(ulong addr, const char *fit_uname)
> >                         return 1;
> >                 }
> >
> > +               if (CONFIG_IS_ENABLED(FIT_SIGNATURE_STRICT)) {
> > +                       /* validate required fit config entry */
> > +                       noffset = fit_conf_get_node(fit_hdr, NULL);
> > +                       if (noffset >= 0) {
> > +                               if (fit_config_verify(fit_hdr, noffset)) {
> > +                                       puts("Cannot verify FIT config node\n");
> > +                                       return 1;
> > +                               }
> > +                       } else {
> > +                               puts("FIT_SIGNATURE_STRICT requires a config node\n");
> > +                               return 1;
> > +                       }
> > +               }
> > +
> >                 if (!fit_uname)
> >                         fit_uname = get_default_image(fit_hdr);
> >
> > diff --git a/cmd/ximg.c b/cmd/ximg.c
> > index 65ba41320a0..39fccd8179c 100644
> > --- a/cmd/ximg.c
> > +++ b/cmd/ximg.c
> > @@ -141,6 +141,20 @@ do_imgextract(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
> >                         return 1;
> >                 }
> >
> > +               if (CONFIG_IS_ENABLED(FIT_SIGNATURE_STRICT)) {
> > +                       /* validate required fit config entry */
> > +                       noffset = fit_conf_get_node(fit_hdr, NULL);
> > +                       if (noffset >= 0) {
> > +                               if (fit_config_verify(fit_hdr, noffset)) {
> > +                                       puts("Cannot verify FIT config node\n");
> > +                                       return 1;
> > +                               }
> > +                       } else {
> > +                               puts("FIT_SIGNATURE_STRICT requires a config node\n");
> > +                               return 1;
> > +                       }
> > +               }
> > +
> >                 /* get subimage node offset */
> >                 noffset = fit_image_get_node(fit_hdr, uname);
> >                 if (noffset < 0) {
> > diff --git a/common/Kconfig.boot b/common/Kconfig.boot
> > index 6f95d009dfa..ca7d9a8d971 100644
> > --- a/common/Kconfig.boot
> > +++ b/common/Kconfig.boot
> > @@ -77,6 +77,10 @@ config FIT_SIGNATURE_MAX_SIZE
> >           device memory. Assure this size does not extend past expected storage
> >           space.
> >
> > +config FIT_SIGNATURE_STRICT
> > +       bool "Requires a valid FIT configuration signature for every image"
> > +       select FIT_SIGNATURE
> > +
> >  config FIT_RSASSA_PSS
> >         bool "Support rsassa-pss signature scheme of FIT image contents"
> >         depends on FIT_SIGNATURE
> > --
> > 2.31.1
> >
>
> There is duplication of the same piece of code in 3 different files,
> maybe you could rework
> that and move to some common function?

Yeah, makes sense. I'm reworking this patch. Thanks!

> --
> Best regards - Freundliche Grüsse - Meilleures salutations
>
> Igor Opaniuk
> Embedded Software Engineer
> T:  +380 938364067
> E: igor.opaniuk@foundries.io
> W: www.foundries.io
diff mbox series

Patch

diff --git a/cmd/fpga.c b/cmd/fpga.c
index 3fdd0b35e80..16d329590fa 100644
--- a/cmd/fpga.c
+++ b/cmd/fpga.c
@@ -335,6 +335,20 @@  static int do_fpga_loadmk(struct cmd_tbl *cmdtp, int flag, int argc,
 			return CMD_RET_FAILURE;
 		}
 
+		if (CONFIG_IS_ENABLED(FIT_SIGNATURE_STRICT)) {
+			/* validate required fit config entry */
+			noffset = fit_conf_get_node(fit_hdr, NULL);
+			if (noffset >= 0) {
+				if (fit_config_verify(fit_hdr, noffset)) {
+					puts("Cannot verify FIT config node\n");
+					return CMD_RET_FAILURE;
+				}
+			} else {
+				puts("FIT_SIGNATURE_STRICT requires a config node\n");
+				return CMD_RET_FAILURE;
+			}
+		}
+
 		/* get fpga component image node offset */
 		noffset = fit_image_get_node(fit_hdr, fit_uname);
 		if (noffset < 0) {
diff --git a/cmd/source.c b/cmd/source.c
index 81e015b64ef..b08406dfcbf 100644
--- a/cmd/source.c
+++ b/cmd/source.c
@@ -112,6 +112,20 @@  int image_source_script(ulong addr, const char *fit_uname)
 			return 1;
 		}
 
+		if (CONFIG_IS_ENABLED(FIT_SIGNATURE_STRICT)) {
+			/* validate required fit config entry */
+			noffset = fit_conf_get_node(fit_hdr, NULL);
+			if (noffset >= 0) {
+				if (fit_config_verify(fit_hdr, noffset)) {
+					puts("Cannot verify FIT config node\n");
+					return 1;
+				}
+			} else {
+				puts("FIT_SIGNATURE_STRICT requires a config node\n");
+				return 1;
+			}
+		}
+
 		if (!fit_uname)
 			fit_uname = get_default_image(fit_hdr);
 
diff --git a/cmd/ximg.c b/cmd/ximg.c
index 65ba41320a0..39fccd8179c 100644
--- a/cmd/ximg.c
+++ b/cmd/ximg.c
@@ -141,6 +141,20 @@  do_imgextract(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
 			return 1;
 		}
 
+		if (CONFIG_IS_ENABLED(FIT_SIGNATURE_STRICT)) {
+			/* validate required fit config entry */
+			noffset = fit_conf_get_node(fit_hdr, NULL);
+			if (noffset >= 0) {
+				if (fit_config_verify(fit_hdr, noffset)) {
+					puts("Cannot verify FIT config node\n");
+					return 1;
+				}
+			} else {
+				puts("FIT_SIGNATURE_STRICT requires a config node\n");
+				return 1;
+			}
+		}
+
 		/* get subimage node offset */
 		noffset = fit_image_get_node(fit_hdr, uname);
 		if (noffset < 0) {
diff --git a/common/Kconfig.boot b/common/Kconfig.boot
index 6f95d009dfa..ca7d9a8d971 100644
--- a/common/Kconfig.boot
+++ b/common/Kconfig.boot
@@ -77,6 +77,10 @@  config FIT_SIGNATURE_MAX_SIZE
 	  device memory. Assure this size does not extend past expected storage
 	  space.
 
+config FIT_SIGNATURE_STRICT
+	bool "Requires a valid FIT configuration signature for every image"
+	select FIT_SIGNATURE
+
 config FIT_RSASSA_PSS
 	bool "Support rsassa-pss signature scheme of FIT image contents"
 	depends on FIT_SIGNATURE