Hi Sean,
On Fri, 11 Aug 2023 at 18:28, <seanedm...@linux.microsoft.com> wrote:
>
> From: Stephen Carlson <stcar...@linux.microsoft.com>
>
> New config CONFIG_ARBP to enable enforcement of OS anti-rollback counter
> during image loading.
>
> Images with an anti-rollback counter value "arbvn" declared in the FDT will
> be compared against the current device anti-rollback counter value, and
> older images will not pass signature validation. If the image is newer, the
> device anti-rollback counter value will be updated.
>
> Signed-off-by: Stephen Carlson <stcar...@linux.microsoft.com>
> ---
> boot/Kconfig | 9 +++++
> boot/image-fit-sig.c | 89 ++++++++++++++++++++++++++++++++++++++++++++
> boot/image-fit.c | 23 ++++++++++++
> include/image.h | 4 ++
> 4 files changed, 125 insertions(+)
>
> diff --git a/boot/Kconfig b/boot/Kconfig
> index e8fb03b801..e08c274b7c 100644
> --- a/boot/Kconfig
> +++ b/boot/Kconfig
> @@ -103,6 +103,15 @@ config FIT_CIPHER
> Enable the feature of data ciphering/unciphering in the tool mkimage
> and in the u-boot support of the FIT image.
>
> +config FIT_ARBP
FIT_ROLLBACK would be better
arbp is really horrible :-)
> + bool "Enable Anti rollback version check for FIT images"
> + depends on FIT_SIGNATURE
> + default n
> + help
> + Enables FIT image anti-rollback protection. This feature is required
> + when a platform needs to retire previous versions of FIT images due
> to
> + security flaws and prevent devices from being reverted to them.
> +
> config FIT_VERBOSE
> bool "Show verbose messages when FIT images fail"
> depends on FIT
> diff --git a/boot/image-fit-sig.c b/boot/image-fit-sig.c
> index 12369896fe..bf3b81a3a3 100644
> --- a/boot/image-fit-sig.c
> +++ b/boot/image-fit-sig.c
> @@ -11,6 +11,8 @@
> #include <log.h>
> #include <malloc.h>
> #include <asm/global_data.h>
> +#include <dm.h>
> +#include <dm-security.h>
You don't need dm- in your headerfiles. I think this should be
rolllback.h and that should be the name of your uclass.
> DECLARE_GLOBAL_DATA_PTR;
> #endif /* !USE_HOSTCC*/
> #include <fdt_region.h>
> @@ -63,6 +65,39 @@ struct image_region *fit_region_make_list(const void *fit,
> return region;
> }
>
> +#if !defined(USE_HOSTCC)
Can we drop that?
> +static int fit_image_verify_arbvn(const void *fit, int image_noffset)
> +{
> + u64 image_arbvn;
> + u64 plat_arbvn = 0ULL;
> + struct udevice *dev;
> + int ret;
> +
> + ret = fit_image_get_arbvn(fit, image_noffset, &image_arbvn);
> + if (ret)
> + return 0;
?? Isn't this an error?
> +
> + ret = uclass_first_device_err(UCLASS_SECURITY, &dev);
> + if (ret)
> + return -ENODEV;
return ret
> +
> + ret = dm_security_arbvn_get(dev, &plat_arbvn);
> + if (ret)
> + return -EIO;
> +
> + if (image_arbvn < plat_arbvn) {
> + return -EPERM;
> + } else if (image_arbvn > plat_arbvn) {
> + ret = dm_security_arbvn_set(dev, image_arbvn);
> + printf(" Updating OS anti-rollback to %llu from %llu\n",
> + image_arbvn, plat_arbvn);
So the update happens in U-Boot? Don't we want to update it when we
know it boots?
> + return ret;
> + }
> +
> + return 0;
> +}
> +#endif
> +
> static int fit_image_setup_verify(struct image_sign_info *info,
> const void *fit, int noffset,
> const void *key_blob, int required_keynode,
> @@ -175,6 +210,16 @@ static int fit_image_verify_sig(const void *fit, int
> image_noffset,
> goto error;
> }
>
> +#if !defined(USE_HOSTCC)
Can you use
if (!tools_build())
?
This seems to be adding to FIT so the FIT docs should be updated.
Regards,
Simon
