[PATCH 3/5] common: Add OS anti-rollback validation using security devices

[Simon Glass]

Hi Sean,

On Fri, 11 Aug 2023 at 18:28, <seanedmond at linux.microsoft.com> wrote:
>
> From: Stephen Carlson <stcarlso at 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 <stcarlso at 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