[U-Boot] [RFC PATCH] cmd: avb: Support A/B slots

[Sam Protsenko]

Hi Igor,

On Tue, Aug 6, 2019 at 4:07 PM Igor Opaniuk <igor.opaniuk at gmail.com> wrote:
>
> Hi Sam,
>
> Sorry for the late reply,
>
> On Fri, Aug 2, 2019 at 9:57 PM Sam Protsenko <semen.protsenko at linaro.org> wrote:
> >
> > Hi Igor, Jens,
> >
> > Can you please comments on next topics:
> >   1. With enabled A/B partitions, we have boot_a/boot_b,
> >      system_a/system_b, vendor_a/vendor_b partitions. Therefore
> >      requested_partitions[] should be slotted (which is done in this RFC
> >      patch). But this patch doesn' handle item (2) below.
> >   2. With dynamic partitions enabled, we don't have system/vendor
> >      anymore; instead we have single "super" partitions. Therefore
> >      requested_partitions[] table contains wrong partitions list for
> >      that particular case.
>
> This case can be handled in the latest libavb by
> 49936b4c010(libavb: Support vbmeta blobs in beginning of partition) [1].
> Anyway, this will require to pull the latest libavb sources into U-boot.
>
> >
> > Question: can we allow user to select which partition to verify, instead
> > of trying to verify hard-coded partitions from requested_partitions[]
> > table? This would solve both (1) and (2) items. But I'm not sure about
> > next possible issues:
> >   a. Wouldn't it break chain of trust somehow?
>
> It wont. If the user can obtain access to U-boot shell or edit U-boot
> env, the chain of trust is already broken (he can just wipe off
> `avb_verify` cmd invocation and that's it). But anyway, at first,
> check this solution [1].
>

Thanks for your reply. So as I understand we need to:
  1. Pull new libavb from AOSP to U-Boot
  2. Provide a parameter to 'avb verify' command, to pass partition to verify
  3. Rework AM57x boot procedure, and instead of just one 'avb verify'
call we should call 'avb verify' several times,one time per partition

Is that correct or you have another idea how to improve it?

> >   b. Is it ok to run avb_slot_verify() several times (one time per one
> >      partition?
> >
> > If (a) or (b) is of any concern, then maybe we can provide a way for the
> > user to pass any number of arguments to 'avb verify', like this:
> >
> >     => avb verify boot_a super_a dtbo_a
> >
> > so help synopsis for 'avb verify' can be like this:
> >
> >     avb verify <partition> ...
> >
> > What do you think about this? Which would be the best course of action
> > to fix both issues (1) and (2)?
> >
> > Thanks.
> >
> > Signed-off-by: Sam Protsenko <semen.protsenko at linaro.org>
> > ---
> >  cmd/avb.c | 13 +++++++++----
> >  1 file changed, 9 insertions(+), 4 deletions(-)
> >
> > diff --git a/cmd/avb.c b/cmd/avb.c
> > index 3f6fd763a0..d1942d6605 100644
> > --- a/cmd/avb.c
> > +++ b/cmd/avb.c
> > @@ -235,6 +235,7 @@ int do_avb_verify_part(cmd_tbl_t *cmdtp, int flag,
> >         AvbSlotVerifyData *out_data;
> >         char *cmdline;
> >         char *extra_args;
> > +       char *slot_suffix = "";
> >
> >         bool unlocked = false;
> >         int res = CMD_RET_FAILURE;
> > @@ -244,9 +245,12 @@ int do_avb_verify_part(cmd_tbl_t *cmdtp, int flag,
> >                 return CMD_RET_FAILURE;
> >         }
> >
> > -       if (argc != 1)
> > +       if (argc < 1 || argc > 2)
> >                 return CMD_RET_USAGE;
> >
> > +       if (argc == 2)
> > +               slot_suffix = argv[1];
> > +
> >         printf("## Android Verified Boot 2.0 version %s\n",
> >                avb_version_string());
> >
> > @@ -259,7 +263,7 @@ int do_avb_verify_part(cmd_tbl_t *cmdtp, int flag,
> >         slot_result =
> >                 avb_slot_verify(avb_ops,
> >                                 requested_partitions,
> > -                               "",
> > +                               slot_suffix,
> >                                 unlocked,
> >                                 AVB_HASHTREE_ERROR_MODE_RESTART_AND_INVALIDATE,
> >                                 &out_data);
> > @@ -419,7 +423,7 @@ static cmd_tbl_t cmd_avb[] = {
> >         U_BOOT_CMD_MKENT(read_part, 5, 0, do_avb_read_part, "", ""),
> >         U_BOOT_CMD_MKENT(read_part_hex, 4, 0, do_avb_read_part_hex, "", ""),
> >         U_BOOT_CMD_MKENT(write_part, 5, 0, do_avb_write_part, "", ""),
> > -       U_BOOT_CMD_MKENT(verify, 1, 0, do_avb_verify_part, "", ""),
> > +       U_BOOT_CMD_MKENT(verify, 2, 0, do_avb_verify_part, "", ""),
> >  #ifdef CONFIG_OPTEE_TA_AVB
> >         U_BOOT_CMD_MKENT(read_pvalue, 3, 0, do_avb_read_pvalue, "", ""),
> >         U_BOOT_CMD_MKENT(write_pvalue, 3, 0, do_avb_write_pvalue, "", ""),
> > @@ -462,6 +466,7 @@ U_BOOT_CMD(
> >         "avb read_pvalue <name> <bytes> - read a persistent value <name>\n"
> >         "avb write_pvalue <name> <value> - write a persistent value <name>\n"
> >  #endif
> > -       "avb verify - run verification process using hash data\n"
> > +       "avb verify [slot_suffix] - run verification process using hash data\n"
> >         "    from vbmeta structure\n"
> > +       "    [slot_suffix] - _a, _b, etc (if vbmeta partition is slotted)\n"
> >         );
> > --
> > 2.20.1
> >
>
> Thanks
>
> [1] https://android.googlesource.com/platform/external/avb/+/49936b4c0109411fdd38bd4ba3a32a01c40439a9
>
> --
> Best regards - Freundliche Grüsse - Meilleures salutations
>
> Igor Opaniuk
>
> mailto: igor.opaniuk at gmail.com
> skype: igor.opanyuk
> +380 (93) 836 40 67
> http://ua.linkedin.com/in/iopaniuk