[RFC PATCH 1/1] image: add anti rollback protection for FIT Images

[Tom Rini <trini@konsulko.com>]

On Mon, Sep 14, 2020 at 11:18:25PM -0700, Thirupathaiah Annapureddy wrote:
> Hi Simon,
> 
> Thanks for the review.
> 
> On 9/6/2020 6:43 PM, Simon Glass wrote:
> >>
> >> diff --git a/Kconfig b/Kconfig
> >> index 883e3f71d0..3959a6592c 100644
> >> --- a/Kconfig
> >> +++ b/Kconfig
> >> @@ -533,6 +533,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
> > 
> > How about using ROLLBACK instead of ARBP. It is easier to understand.Looks good to me. I will change it in the next version of the patch.
> 
> >> +{
> >> +       uint8_t type;
> >> +       uint32_t image_arbvn;
> >> +       uint32_t plat_arbvn = 0;
> > 
> > Those three can be uint.
> fit_image_get_type() returns type as uint8_t. 
> I can change it for the other two variables. 
> 
> >>  static int fit_config_verify_sig(const void *fit, int conf_noffset,
> >>                                  const void *sig_blob, int sig_offset)
> >>  {
> >> @@ -401,6 +472,14 @@ static int fit_config_verify_sig(const void *fit, int conf_noffset,
> >>                 goto error;
> >>         }
> >>
> >> +#if !defined(USE_HOSTCC)
> > 
> > Do we need this ?ifdef, or can we rely on IMAGE_ENABLE_ARBP?
> I believe we can rely on just IMAGE_ENABLE_ARBP.
> 
> >>  #define FIT_LOAD_PROP          "load"
> >> +#define FIT_ARBVN_PROP         "arbvn"
> > 
> > ROLLBACK / "rollback"
> I will fix it in the next version.
> 
> > 
> >>
> >>  /* configuration node */
> >>  #define FIT_KERNEL_PROP                "kernel"
> >> @@ -1085,6 +1086,7 @@ int fit_image_get_data_size_unciphered(const void *fit, int noffset,
> >>                                        size_t *data_size);
> >>  int fit_image_get_data_and_size(const void *fit, int noffset,
> >>                                 const void **data, size_t *size);
> >> +int fit_image_get_arbvn(const void *fit, int noffset, uint32_t *arbvn);
> > 
> > Please add a full function comment
> comment was added before the function definition to be consistent
> with other functions.
> 
> >> +int board_get_arbvn(uint8_t ih_type, uint32_t *arbvn);
> > 
> > This needs a driver since the rollback counter may be implemented by a
> > TPM or anything. 
> Board specific hooks can leverage TPM library functions in that case.
> May I know why a driver is needed?

Sorry for not getting in to this series sooner.  One thing that I think
would be very helpful is to see is a full demonstration on say a
Raspberry Pi.  I know I have a TPM2 module that supports Pi sitting
around here.  I assume you've also tested this on some HW platform.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200915/b25f6483/attachment.sig>