+ added also David Zeuthen to CC (I hope he doesn't mind :) ), who actually introduced all these changes in libavb.

On Tue, Aug 6, 2019 at 4:38 PM Sam Protsenko semen.protsenko@linaro.org wrote:

Hi Igor,

On Tue, Aug 6, 2019 at 4:07 PM Igor Opaniuk igor.opaniuk@gmail.com wrote:

...

Hi Sam,

Sorry for the late reply,

On Fri, Aug 2, 2019 at 9:57 PM Sam Protsenko semen.protsenko@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

Right

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

In the latest libavb you can even provide empty requested_partitions[] array, as a list of partitions to verify can be obtained from vbmeta image (this is done implicitly in avb_slot_verify() here [2]). I assume this was also possible at the time when I added initial snapshot of libavb to U-boot, and I hard-coded "vendor"/"system" in requested_partitions[] by mistake (as it wasn't obvious for me either).

Regarding the additional param, you'll definitely need to provide a slot suffix, which now can be extracted from android a/b meta partition by `ab_select` cmd. Example about how avb_slot_verify() is called with proper slot suffix is here [3].

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@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/+/49936b4c0109411fdd3...

-- Best regards - Freundliche Grüsse - Meilleures salutations

Igor Opaniuk

mailto: igor.opaniuk@gmail.com skype: igor.opanyuk +380 (93) 836 40 67 http://ua.linkedin.com/in/iopaniuk

[2] https://android.googlesource.com/platform/external/avb/+/master/libavb/avb_s... [3] https://android.googlesource.com/platform/external/avb/+/master/test/avb_slo...