+ 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.protse...@linaro.org> wrote: > > Hi Igor, > > On Tue, Aug 6, 2019 at 4:07 PM Igor Opaniuk <igor.opan...@gmail.com> wrote: > > > > Hi Sam, > > > > Sorry for the late reply, > > > > On Fri, Aug 2, 2019 at 9:57 PM Sam Protsenko <semen.protse...@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.protse...@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.opan...@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_slot_verify.c#1461 [3] https://android.googlesource.com/platform/external/avb/+/master/test/avb_slot_verify_unittest.cc#110 -- Best regards - Freundliche Grüsse - Meilleures salutations Igor Opaniuk mailto: igor.opan...@gmail.com skype: igor.opanyuk +380 (93) 836 40 67 http://ua.linkedin.com/in/iopaniuk _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot