| Message ID | 20240111-b4-upstream-fit-signature-enforce-v1-1-2b91be31866e@ti.com |
|---|---|
| State | RFC |
| Delegated to: | Simon Glass |
| Headers | show |
| Series | [RFC] Kconfig: Add support for FIT image signature enforcing | expand |
On 1/11/24 4:34 AM, Manorit Chawdhry wrote: > FIT_SIGNATURE doesn't enforce the U-boot setup to be correct for booting > the FIT images, the DTB might not have all the proper nodes and it just > boots up without any warning. This makes it difficult to get the correct > setup working. > > Adds an enforcement flag that doesn't allow the setup to have problems > and enforces the environment to only pick the signature node from DTB > and don't rely on anything else. > > Signed-off-by: Manorit Chawdhry <m-chawdhry@ti.com> > --- > A very crude implementation of [0], not exactly sure if the Kconfig > should just protect the getting key part of if we can make it more > generic and handle this as some lockdown where we can disable boot for > any other method and just allow FIT Images to bootup. > Disabling other boot modes could be a different Kconfig, then we would have a generic "lockdown/secure" Kconfig that goes and selects the various smaller Kconfigs (like this one) that together would prevent bypassing security checks. > [0]: https://lore.kernel.org/u-boot/CAPnjgZ3B9hWTNAMr2QjSN8P1AsOme4XfSAYUKSg=tRRiJ9drTg@mail.gmail.com/ > --- > boot/Kconfig | 11 +++++++++++ > boot/image-fit-sig.c | 31 +++++++++++++++++++++---------- > 2 files changed, 32 insertions(+), 10 deletions(-) > > diff --git a/boot/Kconfig b/boot/Kconfig > index fbc49c5bca47..ed85fd3dc3c5 100644 > --- a/boot/Kconfig > +++ b/boot/Kconfig > @@ -78,6 +78,17 @@ config FIT_SIGNATURE > format support in this case, enable it using > CONFIG_LEGACY_IMAGE_FORMAT. > > +config FIT_SIGNATURE_ENFORCE > + bool "Enforce the signature in FIT Images" > + default y if TI_SECURE_DEVICE > + depends on FIT_SIGNATURE > + help > + Enabling FIT_SIGNATURE by default doesn't enforce the U-boot DTB to be > + having keys and allows booting the images without having proper setup. s/be having/have Andrew > + This option enforces the FIT signature mechanism to contain the keys in > + the DTB and enforce the nodes to be authenticated without relying on > + the "required" node in the DTB. > + > config FIT_SIGNATURE_MAX_SIZE > hex "Max size of signed FIT structures" > depends on FIT_SIGNATURE > diff --git a/boot/image-fit-sig.c b/boot/image-fit-sig.c > index 12369896fe3f..8a324733e7e4 100644 > --- a/boot/image-fit-sig.c > +++ b/boot/image-fit-sig.c > @@ -490,15 +490,24 @@ static int fit_config_verify_required_keys(const void *fit, int conf_noffset, > /* Work out what we need to verify */ > key_node = fdt_subnode_offset(key_blob, 0, FIT_SIG_NODENAME); > if (key_node < 0) { > - debug("%s: No signature node found: %s\n", __func__, > - fdt_strerror(key_node)); > - return 0; > + if (IS_ENABLED(CONFIG_FIT_SIGNATURE_ENFORCE)) { > + printf("%s: No signature node found: %s\n", __func__, > + fdt_strerror(key_node)); > + return -EPERM; > + } else { > + debug("%s: No signature node found: %s\n", __func__, > + fdt_strerror(key_node)); > + return 0; > + } > } > > /* Get required-mode policy property from DTB */ > - reqd_mode = fdt_getprop(key_blob, key_node, "required-mode", NULL); > - if (reqd_mode && !strcmp(reqd_mode, "any")) > - reqd_policy_all = false; > + if (!IS_ENABLED(CONFIG_FIT_SIGNATURE_ENFORCE)) { > + reqd_mode = > + fdt_getprop(key_blob, key_node, "required-mode", NULL); > + if (reqd_mode && !strcmp(reqd_mode, "any")) > + reqd_policy_all = false; > + } > > debug("%s: required-mode policy set to '%s'\n", __func__, > reqd_policy_all ? "all" : "any"); > @@ -514,10 +523,12 @@ static int fit_config_verify_required_keys(const void *fit, int conf_noffset, > const char *required; > int ret; > > - required = fdt_getprop(key_blob, noffset, FIT_KEY_REQUIRED, > - NULL); > - if (!required || strcmp(required, "conf")) > - continue; > + if (!IS_ENABLED(CONFIG_FIT_SIGNATURE_ENFORCE)) { > + required = fdt_getprop(key_blob, noffset, > + FIT_KEY_REQUIRED, NULL); > + if (!required || strcmp(required, "conf")) > + continue; > + } > > reqd_sigs++; > > > --- > base-commit: a803f87202aa48974bdff4d8100464a8288931e4 > change-id: 20231107-b4-upstream-fit-signature-enforce-185a0db88b7f > > Best regards,
diff --git a/boot/Kconfig b/boot/Kconfig index fbc49c5bca47..ed85fd3dc3c5 100644 --- a/boot/Kconfig +++ b/boot/Kconfig @@ -78,6 +78,17 @@ config FIT_SIGNATURE format support in this case, enable it using CONFIG_LEGACY_IMAGE_FORMAT. +config FIT_SIGNATURE_ENFORCE + bool "Enforce the signature in FIT Images" + default y if TI_SECURE_DEVICE + depends on FIT_SIGNATURE + help + Enabling FIT_SIGNATURE by default doesn't enforce the U-boot DTB to be + having keys and allows booting the images without having proper setup. + This option enforces the FIT signature mechanism to contain the keys in + the DTB and enforce the nodes to be authenticated without relying on + the "required" node in the DTB. + config FIT_SIGNATURE_MAX_SIZE hex "Max size of signed FIT structures" depends on FIT_SIGNATURE diff --git a/boot/image-fit-sig.c b/boot/image-fit-sig.c index 12369896fe3f..8a324733e7e4 100644 --- a/boot/image-fit-sig.c +++ b/boot/image-fit-sig.c @@ -490,15 +490,24 @@ static int fit_config_verify_required_keys(const void *fit, int conf_noffset, /* Work out what we need to verify */ key_node = fdt_subnode_offset(key_blob, 0, FIT_SIG_NODENAME); if (key_node < 0) { - debug("%s: No signature node found: %s\n", __func__, - fdt_strerror(key_node)); - return 0; + if (IS_ENABLED(CONFIG_FIT_SIGNATURE_ENFORCE)) { + printf("%s: No signature node found: %s\n", __func__, + fdt_strerror(key_node)); + return -EPERM; + } else { + debug("%s: No signature node found: %s\n", __func__, + fdt_strerror(key_node)); + return 0; + } } /* Get required-mode policy property from DTB */ - reqd_mode = fdt_getprop(key_blob, key_node, "required-mode", NULL); - if (reqd_mode && !strcmp(reqd_mode, "any")) - reqd_policy_all = false; + if (!IS_ENABLED(CONFIG_FIT_SIGNATURE_ENFORCE)) { + reqd_mode = + fdt_getprop(key_blob, key_node, "required-mode", NULL); + if (reqd_mode && !strcmp(reqd_mode, "any")) + reqd_policy_all = false; + } debug("%s: required-mode policy set to '%s'\n", __func__, reqd_policy_all ? "all" : "any"); @@ -514,10 +523,12 @@ static int fit_config_verify_required_keys(const void *fit, int conf_noffset, const char *required; int ret; - required = fdt_getprop(key_blob, noffset, FIT_KEY_REQUIRED, - NULL); - if (!required || strcmp(required, "conf")) - continue; + if (!IS_ENABLED(CONFIG_FIT_SIGNATURE_ENFORCE)) { + required = fdt_getprop(key_blob, noffset, + FIT_KEY_REQUIRED, NULL); + if (!required || strcmp(required, "conf")) + continue; + } reqd_sigs++;
FIT_SIGNATURE doesn't enforce the U-boot setup to be correct for booting the FIT images, the DTB might not have all the proper nodes and it just boots up without any warning. This makes it difficult to get the correct setup working. Adds an enforcement flag that doesn't allow the setup to have problems and enforces the environment to only pick the signature node from DTB and don't rely on anything else. Signed-off-by: Manorit Chawdhry <m-chawdhry@ti.com> --- A very crude implementation of [0], not exactly sure if the Kconfig should just protect the getting key part of if we can make it more generic and handle this as some lockdown where we can disable boot for any other method and just allow FIT Images to bootup. [0]: https://lore.kernel.org/u-boot/CAPnjgZ3B9hWTNAMr2QjSN8P1AsOme4XfSAYUKSg=tRRiJ9drTg@mail.gmail.com/ --- boot/Kconfig | 11 +++++++++++ boot/image-fit-sig.c | 31 +++++++++++++++++++++---------- 2 files changed, 32 insertions(+), 10 deletions(-) --- base-commit: a803f87202aa48974bdff4d8100464a8288931e4 change-id: 20231107-b4-upstream-fit-signature-enforce-185a0db88b7f Best regards,