| Message ID | 1524662285-19617-1-git-send-email-igor.opaniuk@linaro.org |
|---|---|
| Headers | show |
| Series | Initial integration of AVB2.0 | expand |
Hi Igor, It's great to see the patch set to support AVB2.0, the upstream libavb(from aosp) combine the AVB with A/B which I think should be two separate feature, are you going to split them? BTW, do you have plan to update boot_android cmd to support avb? the command is too weak for use now. And any plan to add opptee_client/smcc to talk to OPTEE/ATF? Thanks, - Kever On 04/25/2018 09:17 PM, Igor Opaniuk wrote: > This series of patches introduces support of Android Verified Boot 2.0, > which provides integrity checking of Android partitions on MMC. > > It integrates libavb/libavb_ab into the U-boot, provides implementation of > AvbOps, subset of `avb` commands to run verification chain (and for debugging > purposes), and it enables AVB2.0 verification on AM57xx HS SoC by default. > > Currently, there is still no support for verification of A/B boot slots > and no rollback protection (for storing rollback indexes > there are plans to use eMMC RPMB) > > Libavb/libavb_ab will be deviated from AOSP upstream in the future, > that's why minimal amount of changes were introduced into the lib sources, > so checkpatch may fail. > > For additional details check [1] AVB 2.0 README and doc/README.avb2, which > is a part of this patchset. > > [1] https://android.googlesource.com/platform/external/avb/+/master/README.md > > Igor Opaniuk (8): > avb2.0: add Android Verified Boot 2.0 libraries > avb2.0: integrate avb 2.0 into the build system > avb2.0: implement AVB ops > cmd: avb2.0: avb command for performing verification > avb2.0: add boot states and dm-verity support > am57xx_hs: avb2.0: add support of AVB 2.0 > test/py: avb2.0: add tests for avb commands > doc: avb2.0: add README about AVB2.0 integration > > cmd/Kconfig | 15 + > cmd/Makefile | 3 + > cmd/avb.c | 366 ++++++++ > common/Makefile | 2 + > common/avb_verify.c | 748 ++++++++++++++++ > configs/am57xx_hs_evm_defconfig | 3 + > doc/README.avb2 | 100 +++ > include/avb/avb_ab_flow.h | 235 ++++++ > include/avb/avb_ab_ops.h | 61 ++ > include/avb/avb_chain_partition_descriptor.h | 54 ++ > include/avb/avb_crypto.h | 147 ++++ > include/avb/avb_descriptor.h | 113 +++ > include/avb/avb_footer.h | 68 ++ > include/avb/avb_hash_descriptor.h | 55 ++ > include/avb/avb_hashtree_descriptor.h | 65 ++ > include/avb/avb_kernel_cmdline_descriptor.h | 63 ++ > include/avb/avb_ops.h | 196 +++++ > include/avb/avb_property_descriptor.h | 89 ++ > include/avb/avb_rsa.h | 55 ++ > include/avb/avb_sha.h | 72 ++ > include/avb/avb_slot_verify.h | 239 ++++++ > include/avb/avb_sysdeps.h | 97 +++ > include/avb/avb_util.h | 259 ++++++ > include/avb/avb_vbmeta_image.h | 272 ++++++ > include/avb/avb_version.h | 45 + > include/avb/libavb.h | 32 + > include/avb/libavb_ab.h | 22 + > include/avb_verify.h | 97 +++ > include/configs/am57xx_evm.h | 11 + > include/environment/ti/boot.h | 15 + > lib/Kconfig | 20 + > lib/Makefile | 2 + > lib/libavb/Makefile | 15 + > lib/libavb/avb_chain_partition_descriptor.c | 46 + > lib/libavb/avb_crypto.c | 355 ++++++++ > lib/libavb/avb_descriptor.c | 142 ++++ > lib/libavb/avb_footer.c | 36 + > lib/libavb/avb_hash_descriptor.c | 43 + > lib/libavb/avb_hashtree_descriptor.c | 51 ++ > lib/libavb/avb_kernel_cmdline_descriptor.c | 40 + > lib/libavb/avb_property_descriptor.c | 167 ++++ > lib/libavb/avb_rsa.c | 277 ++++++ > lib/libavb/avb_sha256.c | 364 ++++++++ > lib/libavb/avb_sha512.c | 362 ++++++++ > lib/libavb/avb_slot_verify.c | 1169 ++++++++++++++++++++++++++ > lib/libavb/avb_sysdeps_posix.c | 57 ++ > lib/libavb/avb_util.c | 385 +++++++++ > lib/libavb/avb_vbmeta_image.c | 290 +++++++ > lib/libavb/avb_version.c | 16 + > lib/libavb_ab/Makefile | 9 + > lib/libavb_ab/avb_ab_flow.c | 502 +++++++++++ > test/py/tests/test_avb.py | 111 +++ > 52 files changed, 8058 insertions(+) > create mode 100644 cmd/avb.c > create mode 100644 common/avb_verify.c > create mode 100644 doc/README.avb2 > create mode 100644 include/avb/avb_ab_flow.h > create mode 100644 include/avb/avb_ab_ops.h > create mode 100644 include/avb/avb_chain_partition_descriptor.h > create mode 100644 include/avb/avb_crypto.h > create mode 100644 include/avb/avb_descriptor.h > create mode 100644 include/avb/avb_footer.h > create mode 100644 include/avb/avb_hash_descriptor.h > create mode 100644 include/avb/avb_hashtree_descriptor.h > create mode 100644 include/avb/avb_kernel_cmdline_descriptor.h > create mode 100644 include/avb/avb_ops.h > create mode 100644 include/avb/avb_property_descriptor.h > create mode 100644 include/avb/avb_rsa.h > create mode 100644 include/avb/avb_sha.h > create mode 100644 include/avb/avb_slot_verify.h > create mode 100644 include/avb/avb_sysdeps.h > create mode 100644 include/avb/avb_util.h > create mode 100644 include/avb/avb_vbmeta_image.h > create mode 100644 include/avb/avb_version.h > create mode 100644 include/avb/libavb.h > create mode 100644 include/avb/libavb_ab.h > create mode 100644 include/avb_verify.h > create mode 100644 lib/libavb/Makefile > create mode 100644 lib/libavb/avb_chain_partition_descriptor.c > create mode 100644 lib/libavb/avb_crypto.c > create mode 100644 lib/libavb/avb_descriptor.c > create mode 100644 lib/libavb/avb_footer.c > create mode 100644 lib/libavb/avb_hash_descriptor.c > create mode 100644 lib/libavb/avb_hashtree_descriptor.c > create mode 100644 lib/libavb/avb_kernel_cmdline_descriptor.c > create mode 100644 lib/libavb/avb_property_descriptor.c > create mode 100644 lib/libavb/avb_rsa.c > create mode 100644 lib/libavb/avb_sha256.c > create mode 100644 lib/libavb/avb_sha512.c > create mode 100644 lib/libavb/avb_slot_verify.c > create mode 100644 lib/libavb/avb_sysdeps_posix.c > create mode 100644 lib/libavb/avb_util.c > create mode 100644 lib/libavb/avb_vbmeta_image.c > create mode 100644 lib/libavb/avb_version.c > create mode 100644 lib/libavb_ab/Makefile > create mode 100644 lib/libavb_ab/avb_ab_flow.c > create mode 100644 test/py/tests/test_avb.py >
On 26 April 2018 at 06:05, Kever Yang <kever.yang@rock-chips.com> wrote: > Hi Igor, > > It's great to see the patch set to support AVB2.0, the upstream > libavb(from aosp) combine the AVB with A/B which I think should be > two separate feature, are you going to split them? Hi Kever, Right, support of verification of A/B slots is going to be in a separate patch-set. > BTW, do you have plan to update boot_android cmd to support avb? > the command is too weak for use now. > And any plan to add opptee_client/smcc to talk to OPTEE/ATF? Did you mean boot_android cmd from this patch https://lists.denx.de/pipermail/u-boot/2017-April/285867.html, that was never up-streamed? I guess the main suggestion was to extend existing bootm (by adding detection and parsing of Android boot images) instead of introducing brand new command for booting Android. As currently major amount of boards use sequence of mmc/bootm commands for this purposes and bootm obviously is supposed to boot something from RAM, I assumed that it would be wrong to invoke avb verification from bootm itself. Because of this reason I've introduced avb set of commands for explicitly triggering the verification process. You can check the example how AVB2.0 is enabled on AM57xx HS (check "am57xx_hs: avb2.0: add support of AVB 2.0" patch). The only one prerequisite is that U-boot env itself should be also a part of chain of trust (so it can't be tampered and "avb verify" removed) Best regards, Igor > > Thanks, > - Kever > On 04/25/2018 09:17 PM, Igor Opaniuk wrote: >> This series of patches introduces support of Android Verified Boot 2.0, >> which provides integrity checking of Android partitions on MMC. >> >> It integrates libavb/libavb_ab into the U-boot, provides implementation of >> AvbOps, subset of `avb` commands to run verification chain (and for debugging >> purposes), and it enables AVB2.0 verification on AM57xx HS SoC by default. >> >> Currently, there is still no support for verification of A/B boot slots >> and no rollback protection (for storing rollback indexes >> there are plans to use eMMC RPMB) >> >> Libavb/libavb_ab will be deviated from AOSP upstream in the future, >> that's why minimal amount of changes were introduced into the lib sources, >> so checkpatch may fail. >> >> For additional details check [1] AVB 2.0 README and doc/README.avb2, which >> is a part of this patchset. >> >> [1] https://android.googlesource.com/platform/external/avb/+/master/README.md >> >> Igor Opaniuk (8): >> avb2.0: add Android Verified Boot 2.0 libraries >> avb2.0: integrate avb 2.0 into the build system >> avb2.0: implement AVB ops >> cmd: avb2.0: avb command for performing verification >> avb2.0: add boot states and dm-verity support >> am57xx_hs: avb2.0: add support of AVB 2.0 >> test/py: avb2.0: add tests for avb commands >> doc: avb2.0: add README about AVB2.0 integration >> >> cmd/Kconfig | 15 + >> cmd/Makefile | 3 + >> cmd/avb.c | 366 ++++++++ >> common/Makefile | 2 + >> common/avb_verify.c | 748 ++++++++++++++++ >> configs/am57xx_hs_evm_defconfig | 3 + >> doc/README.avb2 | 100 +++ >> include/avb/avb_ab_flow.h | 235 ++++++ >> include/avb/avb_ab_ops.h | 61 ++ >> include/avb/avb_chain_partition_descriptor.h | 54 ++ >> include/avb/avb_crypto.h | 147 ++++ >> include/avb/avb_descriptor.h | 113 +++ >> include/avb/avb_footer.h | 68 ++ >> include/avb/avb_hash_descriptor.h | 55 ++ >> include/avb/avb_hashtree_descriptor.h | 65 ++ >> include/avb/avb_kernel_cmdline_descriptor.h | 63 ++ >> include/avb/avb_ops.h | 196 +++++ >> include/avb/avb_property_descriptor.h | 89 ++ >> include/avb/avb_rsa.h | 55 ++ >> include/avb/avb_sha.h | 72 ++ >> include/avb/avb_slot_verify.h | 239 ++++++ >> include/avb/avb_sysdeps.h | 97 +++ >> include/avb/avb_util.h | 259 ++++++ >> include/avb/avb_vbmeta_image.h | 272 ++++++ >> include/avb/avb_version.h | 45 + >> include/avb/libavb.h | 32 + >> include/avb/libavb_ab.h | 22 + >> include/avb_verify.h | 97 +++ >> include/configs/am57xx_evm.h | 11 + >> include/environment/ti/boot.h | 15 + >> lib/Kconfig | 20 + >> lib/Makefile | 2 + >> lib/libavb/Makefile | 15 + >> lib/libavb/avb_chain_partition_descriptor.c | 46 + >> lib/libavb/avb_crypto.c | 355 ++++++++ >> lib/libavb/avb_descriptor.c | 142 ++++ >> lib/libavb/avb_footer.c | 36 + >> lib/libavb/avb_hash_descriptor.c | 43 + >> lib/libavb/avb_hashtree_descriptor.c | 51 ++ >> lib/libavb/avb_kernel_cmdline_descriptor.c | 40 + >> lib/libavb/avb_property_descriptor.c | 167 ++++ >> lib/libavb/avb_rsa.c | 277 ++++++ >> lib/libavb/avb_sha256.c | 364 ++++++++ >> lib/libavb/avb_sha512.c | 362 ++++++++ >> lib/libavb/avb_slot_verify.c | 1169 ++++++++++++++++++++++++++ >> lib/libavb/avb_sysdeps_posix.c | 57 ++ >> lib/libavb/avb_util.c | 385 +++++++++ >> lib/libavb/avb_vbmeta_image.c | 290 +++++++ >> lib/libavb/avb_version.c | 16 + >> lib/libavb_ab/Makefile | 9 + >> lib/libavb_ab/avb_ab_flow.c | 502 +++++++++++ >> test/py/tests/test_avb.py | 111 +++ >> 52 files changed, 8058 insertions(+) >> create mode 100644 cmd/avb.c >> create mode 100644 common/avb_verify.c >> create mode 100644 doc/README.avb2 >> create mode 100644 include/avb/avb_ab_flow.h >> create mode 100644 include/avb/avb_ab_ops.h >> create mode 100644 include/avb/avb_chain_partition_descriptor.h >> create mode 100644 include/avb/avb_crypto.h >> create mode 100644 include/avb/avb_descriptor.h >> create mode 100644 include/avb/avb_footer.h >> create mode 100644 include/avb/avb_hash_descriptor.h >> create mode 100644 include/avb/avb_hashtree_descriptor.h >> create mode 100644 include/avb/avb_kernel_cmdline_descriptor.h >> create mode 100644 include/avb/avb_ops.h >> create mode 100644 include/avb/avb_property_descriptor.h >> create mode 100644 include/avb/avb_rsa.h >> create mode 100644 include/avb/avb_sha.h >> create mode 100644 include/avb/avb_slot_verify.h >> create mode 100644 include/avb/avb_sysdeps.h >> create mode 100644 include/avb/avb_util.h >> create mode 100644 include/avb/avb_vbmeta_image.h >> create mode 100644 include/avb/avb_version.h >> create mode 100644 include/avb/libavb.h >> create mode 100644 include/avb/libavb_ab.h >> create mode 100644 include/avb_verify.h >> create mode 100644 lib/libavb/Makefile >> create mode 100644 lib/libavb/avb_chain_partition_descriptor.c >> create mode 100644 lib/libavb/avb_crypto.c >> create mode 100644 lib/libavb/avb_descriptor.c >> create mode 100644 lib/libavb/avb_footer.c >> create mode 100644 lib/libavb/avb_hash_descriptor.c >> create mode 100644 lib/libavb/avb_hashtree_descriptor.c >> create mode 100644 lib/libavb/avb_kernel_cmdline_descriptor.c >> create mode 100644 lib/libavb/avb_property_descriptor.c >> create mode 100644 lib/libavb/avb_rsa.c >> create mode 100644 lib/libavb/avb_sha256.c >> create mode 100644 lib/libavb/avb_sha512.c >> create mode 100644 lib/libavb/avb_slot_verify.c >> create mode 100644 lib/libavb/avb_sysdeps_posix.c >> create mode 100644 lib/libavb/avb_util.c >> create mode 100644 lib/libavb/avb_vbmeta_image.c >> create mode 100644 lib/libavb/avb_version.c >> create mode 100644 lib/libavb_ab/Makefile >> create mode 100644 lib/libavb_ab/avb_ab_flow.c >> create mode 100644 test/py/tests/test_avb.py >> > >
Hi Kever, libavb and libavb_ab are different things, and we split them for a reason. Adding libavb is great, but you don't need to add libavb_ab as an A/B implementation. The boot_android command referenced by Igor doesn't use that as an A/B implementation, but uses the structs already defined in the Boot Control Block (BCB) and the android bootloader flow. I would recommend to include the libavb only. Igor, What changes did you need to do to libavb to import it to U-Boot? The idea with libavb is that it should be easy to integrate into your bootloader without changes; and therefore easy to update and integrate new patches when we release new versions of libavb. We would like to avoid diverting from it to reduce the maintenance cost. Best regards, Alex Le jeu. 26 avr. 2018 à 05:05, Kever Yang <kever.yang@rock-chips.com> a écrit : > Hi Igor, > > It's great to see the patch set to support AVB2.0, the upstream > libavb(from aosp) combine the AVB with A/B which I think should be > two separate feature, are you going to split them? > > BTW, do you have plan to update boot_android cmd to support avb? > the command is too weak for use now. > And any plan to add opptee_client/smcc to talk to OPTEE/ATF? > > Thanks, > - Kever > On 04/25/2018 09:17 PM, Igor Opaniuk wrote: > > This series of patches introduces support of Android Verified Boot 2.0, > > which provides integrity checking of Android partitions on MMC. > > > > It integrates libavb/libavb_ab into the U-boot, provides implementation > of > > AvbOps, subset of `avb` commands to run verification chain (and for > debugging > > purposes), and it enables AVB2.0 verification on AM57xx HS SoC by > default. > > > > Currently, there is still no support for verification of A/B boot slots > > and no rollback protection (for storing rollback indexes > > there are plans to use eMMC RPMB) > > > > Libavb/libavb_ab will be deviated from AOSP upstream in the future, > > that's why minimal amount of changes were introduced into the lib > sources, > > so checkpatch may fail. > > > > For additional details check [1] AVB 2.0 README and doc/README.avb2, > which > > is a part of this patchset. > > > > [1] > https://android.googlesource.com/platform/external/avb/+/master/README.md > > > > Igor Opaniuk (8): > > avb2.0: add Android Verified Boot 2.0 libraries > > avb2.0: integrate avb 2.0 into the build system > > avb2.0: implement AVB ops > > cmd: avb2.0: avb command for performing verification > > avb2.0: add boot states and dm-verity support > > am57xx_hs: avb2.0: add support of AVB 2.0 > > test/py: avb2.0: add tests for avb commands > > doc: avb2.0: add README about AVB2.0 integration > > > > cmd/Kconfig | 15 + > > cmd/Makefile | 3 + > > cmd/avb.c | 366 ++++++++ > > common/Makefile | 2 + > > common/avb_verify.c | 748 ++++++++++++++++ > > configs/am57xx_hs_evm_defconfig | 3 + > > doc/README.avb2 | 100 +++ > > include/avb/avb_ab_flow.h | 235 ++++++ > > include/avb/avb_ab_ops.h | 61 ++ > > include/avb/avb_chain_partition_descriptor.h | 54 ++ > > include/avb/avb_crypto.h | 147 ++++ > > include/avb/avb_descriptor.h | 113 +++ > > include/avb/avb_footer.h | 68 ++ > > include/avb/avb_hash_descriptor.h | 55 ++ > > include/avb/avb_hashtree_descriptor.h | 65 ++ > > include/avb/avb_kernel_cmdline_descriptor.h | 63 ++ > > include/avb/avb_ops.h | 196 +++++ > > include/avb/avb_property_descriptor.h | 89 ++ > > include/avb/avb_rsa.h | 55 ++ > > include/avb/avb_sha.h | 72 ++ > > include/avb/avb_slot_verify.h | 239 ++++++ > > include/avb/avb_sysdeps.h | 97 +++ > > include/avb/avb_util.h | 259 ++++++ > > include/avb/avb_vbmeta_image.h | 272 ++++++ > > include/avb/avb_version.h | 45 + > > include/avb/libavb.h | 32 + > > include/avb/libavb_ab.h | 22 + > > include/avb_verify.h | 97 +++ > > include/configs/am57xx_evm.h | 11 + > > include/environment/ti/boot.h | 15 + > > lib/Kconfig | 20 + > > lib/Makefile | 2 + > > lib/libavb/Makefile | 15 + > > lib/libavb/avb_chain_partition_descriptor.c | 46 + > > lib/libavb/avb_crypto.c | 355 ++++++++ > > lib/libavb/avb_descriptor.c | 142 ++++ > > lib/libavb/avb_footer.c | 36 + > > lib/libavb/avb_hash_descriptor.c | 43 + > > lib/libavb/avb_hashtree_descriptor.c | 51 ++ > > lib/libavb/avb_kernel_cmdline_descriptor.c | 40 + > > lib/libavb/avb_property_descriptor.c | 167 ++++ > > lib/libavb/avb_rsa.c | 277 ++++++ > > lib/libavb/avb_sha256.c | 364 ++++++++ > > lib/libavb/avb_sha512.c | 362 ++++++++ > > lib/libavb/avb_slot_verify.c | 1169 > ++++++++++++++++++++++++++ > > lib/libavb/avb_sysdeps_posix.c | 57 ++ > > lib/libavb/avb_util.c | 385 +++++++++ > > lib/libavb/avb_vbmeta_image.c | 290 +++++++ > > lib/libavb/avb_version.c | 16 + > > lib/libavb_ab/Makefile | 9 + > > lib/libavb_ab/avb_ab_flow.c | 502 +++++++++++ > > test/py/tests/test_avb.py | 111 +++ > > 52 files changed, 8058 insertions(+) > > create mode 100644 cmd/avb.c > > create mode 100644 common/avb_verify.c > > create mode 100644 doc/README.avb2 > > create mode 100644 include/avb/avb_ab_flow.h > > create mode 100644 include/avb/avb_ab_ops.h > > create mode 100644 include/avb/avb_chain_partition_descriptor.h > > create mode 100644 include/avb/avb_crypto.h > > create mode 100644 include/avb/avb_descriptor.h > > create mode 100644 include/avb/avb_footer.h > > create mode 100644 include/avb/avb_hash_descriptor.h > > create mode 100644 include/avb/avb_hashtree_descriptor.h > > create mode 100644 include/avb/avb_kernel_cmdline_descriptor.h > > create mode 100644 include/avb/avb_ops.h > > create mode 100644 include/avb/avb_property_descriptor.h > > create mode 100644 include/avb/avb_rsa.h > > create mode 100644 include/avb/avb_sha.h > > create mode 100644 include/avb/avb_slot_verify.h > > create mode 100644 include/avb/avb_sysdeps.h > > create mode 100644 include/avb/avb_util.h > > create mode 100644 include/avb/avb_vbmeta_image.h > > create mode 100644 include/avb/avb_version.h > > create mode 100644 include/avb/libavb.h > > create mode 100644 include/avb/libavb_ab.h > > create mode 100644 include/avb_verify.h > > create mode 100644 lib/libavb/Makefile > > create mode 100644 lib/libavb/avb_chain_partition_descriptor.c > > create mode 100644 lib/libavb/avb_crypto.c > > create mode 100644 lib/libavb/avb_descriptor.c > > create mode 100644 lib/libavb/avb_footer.c > > create mode 100644 lib/libavb/avb_hash_descriptor.c > > create mode 100644 lib/libavb/avb_hashtree_descriptor.c > > create mode 100644 lib/libavb/avb_kernel_cmdline_descriptor.c > > create mode 100644 lib/libavb/avb_property_descriptor.c > > create mode 100644 lib/libavb/avb_rsa.c > > create mode 100644 lib/libavb/avb_sha256.c > > create mode 100644 lib/libavb/avb_sha512.c > > create mode 100644 lib/libavb/avb_slot_verify.c > > create mode 100644 lib/libavb/avb_sysdeps_posix.c > > create mode 100644 lib/libavb/avb_util.c > > create mode 100644 lib/libavb/avb_vbmeta_image.c > > create mode 100644 lib/libavb/avb_version.c > > create mode 100644 lib/libavb_ab/Makefile > > create mode 100644 lib/libavb_ab/avb_ab_flow.c > > create mode 100644 test/py/tests/test_avb.py > > > > > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > https://lists.denx.de/listinfo/u-boot >
Hi Alex, I've replaced licence texts in source file headers to SPDX short identifiers (suggestion from Tom Rini). As far as I know that was the only one major change I introduced to libavb/libavb_ab. I also did remove crc32() implementation and used existing in U-boot because of licence ambiguity, frankly I wasn't sure if it was GPL-2.0 compatible ( https://android.googlesource.com/platform/external/avb/+/master/libavb/avb_crc32.c). Regarding libavb_ab, I guess I can exclude it for now, as there is no any functionality implemented for A/B slots support in this patch-set. And, btw, I've noticed the note in the latest README.md for AVB2.0 on googlesource "This code is DEPRECATED and you must define AVB_AB_I_UNDERSTAND_LIBAVB_AB_IS_DEPRECATED to use it. The code will be removed Jun 1 2018.". Does it mean that a/b stuff will be included in libavb instead of seperate libavb_av lib? Thanks! Regards, Igor On 26 April 2018 at 19:35, Alex Deymo <deymo+@google.com> wrote: > Hi Kever, > libavb and libavb_ab are different things, and we split them for a reason. > Adding libavb is great, but you don't need to add libavb_ab as an A/B > implementation. The boot_android command referenced by Igor doesn't use that > as an A/B implementation, but uses the structs already defined in the Boot > Control Block (BCB) and the android bootloader flow. I would recommend to > include the libavb only. > > Igor, > What changes did you need to do to libavb to import it to U-Boot? The idea > with libavb is that it should be easy to integrate into your bootloader > without changes; and therefore easy to update and integrate new patches when > we release new versions of libavb. We would like to avoid diverting from it > to reduce the maintenance cost. > > Best regards, > Alex > > > Le jeu. 26 avr. 2018 à 05:05, Kever Yang <kever.yang@rock-chips.com> a écrit > : >> >> Hi Igor, >> >> It's great to see the patch set to support AVB2.0, the upstream >> libavb(from aosp) combine the AVB with A/B which I think should be >> two separate feature, are you going to split them? >> >> BTW, do you have plan to update boot_android cmd to support avb? >> the command is too weak for use now. >> And any plan to add opptee_client/smcc to talk to OPTEE/ATF? >> >> Thanks, >> - Kever >> On 04/25/2018 09:17 PM, Igor Opaniuk wrote: >> > This series of patches introduces support of Android Verified Boot 2.0, >> > which provides integrity checking of Android partitions on MMC. >> > >> > It integrates libavb/libavb_ab into the U-boot, provides implementation >> > of >> > AvbOps, subset of `avb` commands to run verification chain (and for >> > debugging >> > purposes), and it enables AVB2.0 verification on AM57xx HS SoC by >> > default. >> > >> > Currently, there is still no support for verification of A/B boot slots >> > and no rollback protection (for storing rollback indexes >> > there are plans to use eMMC RPMB) >> > >> > Libavb/libavb_ab will be deviated from AOSP upstream in the future, >> > that's why minimal amount of changes were introduced into the lib >> > sources, >> > so checkpatch may fail. >> > >> > For additional details check [1] AVB 2.0 README and doc/README.avb2, >> > which >> > is a part of this patchset. >> > >> > [1] >> > https://android.googlesource.com/platform/external/avb/+/master/README.md >> > >> > Igor Opaniuk (8): >> > avb2.0: add Android Verified Boot 2.0 libraries >> > avb2.0: integrate avb 2.0 into the build system >> > avb2.0: implement AVB ops >> > cmd: avb2.0: avb command for performing verification >> > avb2.0: add boot states and dm-verity support >> > am57xx_hs: avb2.0: add support of AVB 2.0 >> > test/py: avb2.0: add tests for avb commands >> > doc: avb2.0: add README about AVB2.0 integration >> > >> > cmd/Kconfig | 15 + >> > cmd/Makefile | 3 + >> > cmd/avb.c | 366 ++++++++ >> > common/Makefile | 2 + >> > common/avb_verify.c | 748 ++++++++++++++++ >> > configs/am57xx_hs_evm_defconfig | 3 + >> > doc/README.avb2 | 100 +++ >> > include/avb/avb_ab_flow.h | 235 ++++++ >> > include/avb/avb_ab_ops.h | 61 ++ >> > include/avb/avb_chain_partition_descriptor.h | 54 ++ >> > include/avb/avb_crypto.h | 147 ++++ >> > include/avb/avb_descriptor.h | 113 +++ >> > include/avb/avb_footer.h | 68 ++ >> > include/avb/avb_hash_descriptor.h | 55 ++ >> > include/avb/avb_hashtree_descriptor.h | 65 ++ >> > include/avb/avb_kernel_cmdline_descriptor.h | 63 ++ >> > include/avb/avb_ops.h | 196 +++++ >> > include/avb/avb_property_descriptor.h | 89 ++ >> > include/avb/avb_rsa.h | 55 ++ >> > include/avb/avb_sha.h | 72 ++ >> > include/avb/avb_slot_verify.h | 239 ++++++ >> > include/avb/avb_sysdeps.h | 97 +++ >> > include/avb/avb_util.h | 259 ++++++ >> > include/avb/avb_vbmeta_image.h | 272 ++++++ >> > include/avb/avb_version.h | 45 + >> > include/avb/libavb.h | 32 + >> > include/avb/libavb_ab.h | 22 + >> > include/avb_verify.h | 97 +++ >> > include/configs/am57xx_evm.h | 11 + >> > include/environment/ti/boot.h | 15 + >> > lib/Kconfig | 20 + >> > lib/Makefile | 2 + >> > lib/libavb/Makefile | 15 + >> > lib/libavb/avb_chain_partition_descriptor.c | 46 + >> > lib/libavb/avb_crypto.c | 355 ++++++++ >> > lib/libavb/avb_descriptor.c | 142 ++++ >> > lib/libavb/avb_footer.c | 36 + >> > lib/libavb/avb_hash_descriptor.c | 43 + >> > lib/libavb/avb_hashtree_descriptor.c | 51 ++ >> > lib/libavb/avb_kernel_cmdline_descriptor.c | 40 + >> > lib/libavb/avb_property_descriptor.c | 167 ++++ >> > lib/libavb/avb_rsa.c | 277 ++++++ >> > lib/libavb/avb_sha256.c | 364 ++++++++ >> > lib/libavb/avb_sha512.c | 362 ++++++++ >> > lib/libavb/avb_slot_verify.c | 1169 >> > ++++++++++++++++++++++++++ >> > lib/libavb/avb_sysdeps_posix.c | 57 ++ >> > lib/libavb/avb_util.c | 385 +++++++++ >> > lib/libavb/avb_vbmeta_image.c | 290 +++++++ >> > lib/libavb/avb_version.c | 16 + >> > lib/libavb_ab/Makefile | 9 + >> > lib/libavb_ab/avb_ab_flow.c | 502 +++++++++++ >> > test/py/tests/test_avb.py | 111 +++ >> > 52 files changed, 8058 insertions(+) >> > create mode 100644 cmd/avb.c >> > create mode 100644 common/avb_verify.c >> > create mode 100644 doc/README.avb2 >> > create mode 100644 include/avb/avb_ab_flow.h >> > create mode 100644 include/avb/avb_ab_ops.h >> > create mode 100644 include/avb/avb_chain_partition_descriptor.h >> > create mode 100644 include/avb/avb_crypto.h >> > create mode 100644 include/avb/avb_descriptor.h >> > create mode 100644 include/avb/avb_footer.h >> > create mode 100644 include/avb/avb_hash_descriptor.h >> > create mode 100644 include/avb/avb_hashtree_descriptor.h >> > create mode 100644 include/avb/avb_kernel_cmdline_descriptor.h >> > create mode 100644 include/avb/avb_ops.h >> > create mode 100644 include/avb/avb_property_descriptor.h >> > create mode 100644 include/avb/avb_rsa.h >> > create mode 100644 include/avb/avb_sha.h >> > create mode 100644 include/avb/avb_slot_verify.h >> > create mode 100644 include/avb/avb_sysdeps.h >> > create mode 100644 include/avb/avb_util.h >> > create mode 100644 include/avb/avb_vbmeta_image.h >> > create mode 100644 include/avb/avb_version.h >> > create mode 100644 include/avb/libavb.h >> > create mode 100644 include/avb/libavb_ab.h >> > create mode 100644 include/avb_verify.h >> > create mode 100644 lib/libavb/Makefile >> > create mode 100644 lib/libavb/avb_chain_partition_descriptor.c >> > create mode 100644 lib/libavb/avb_crypto.c >> > create mode 100644 lib/libavb/avb_descriptor.c >> > create mode 100644 lib/libavb/avb_footer.c >> > create mode 100644 lib/libavb/avb_hash_descriptor.c >> > create mode 100644 lib/libavb/avb_hashtree_descriptor.c >> > create mode 100644 lib/libavb/avb_kernel_cmdline_descriptor.c >> > create mode 100644 lib/libavb/avb_property_descriptor.c >> > create mode 100644 lib/libavb/avb_rsa.c >> > create mode 100644 lib/libavb/avb_sha256.c >> > create mode 100644 lib/libavb/avb_sha512.c >> > create mode 100644 lib/libavb/avb_slot_verify.c >> > create mode 100644 lib/libavb/avb_sysdeps_posix.c >> > create mode 100644 lib/libavb/avb_util.c >> > create mode 100644 lib/libavb/avb_vbmeta_image.c >> > create mode 100644 lib/libavb/avb_version.c >> > create mode 100644 lib/libavb_ab/Makefile >> > create mode 100644 lib/libavb_ab/avb_ab_flow.c >> > create mode 100644 test/py/tests/test_avb.py >> > >> >> >> _______________________________________________ >> U-Boot mailing list >> U-Boot@lists.denx.de >> https://lists.denx.de/listinfo/u-boot
Hi Igor, The "libavb_ab" is an implementation of A/B flow, which you could use, but you are not required to use in your A/B bootloader to make an Android device. This ended up not being used in general, for different reasons. The integration of the A/B error modes into an existing device depended on other factors about your device, and it is closer to the rest of the bootloader than it is to libavb. You can read more details about the reasons here: https://android.googlesource.com/platform/external/avb/+/ 37f5946d0e1159273eff61dd8041377fedbf55a9 Anyway, once you have selected a slot to boot from (using your own A/B logic) or if your device has only one slot, the verification process is done by libavb. There's the feedback loop from libavb to your A/B logic to tell you that the slot is certainly broken, but it isn't the only feedback your A/B logic is supposed to get. If the slot boots past libavb but the kernel crashes and reboots later on after a broken update, the counters in the bootloader's A/B logic shold also figure out that the slot is broken; but there's no A/B flow logic in libavb itself. libavb is just about verifying your boot end-to-end handling all the different ways you can sign these images. There's an extension to libavb, which is used in Android Things (libavb_atx). That one is useful to include if you expect to boot Android Things images (in Android Things you have multiple different products using the exact same hardware SoM, which are not interchangeable). Anyway, thanks for doing this. I think maybe a quick shell script to update libavb (automate the changes/renames you did) would be good, but I don't think is needed now. Regards, Alex. 2018-04-27 11:53 GMT+02:00 Igor Opaniuk <igor.opaniuk@linaro.org>: > Hi Alex, > > I've replaced licence texts in source file headers to SPDX short > identifiers (suggestion from Tom Rini). > As far as I know that was the only one major change I introduced to > libavb/libavb_ab. > I also did remove crc32() implementation and used existing in U-boot > because of licence ambiguity, > frankly I wasn't sure if it was GPL-2.0 compatible ( > https://android.googlesource.com/platform/external/avb/+/mas > ter/libavb/avb_crc32.c). > > Regarding libavb_ab, I guess I can exclude it for now, as there is no > any functionality implemented for A/B > slots support in this patch-set. And, btw, I've noticed the note in > the latest README.md for AVB2.0 > on googlesource "This code is DEPRECATED and you must define > AVB_AB_I_UNDERSTAND_LIBAVB_AB_IS_DEPRECATED to use it. The code will > be removed Jun 1 2018.". > Does it mean that a/b stuff will be included in libavb instead of > seperate libavb_av lib? > > Thanks! > > Regards, > Igor
Hello Igor, Alex, Kever,
Having these patches in mainline would be great, as this would reduce
the delta between our own and community U-boot trees. After having a
quick look at this series, I have some questions/review findings.
These patches appear to be slightly older than what is available in [1].
More precisely, ignoring:
- change of license
- drop of avb_crc32.c
- updates in avb_sysdeps_posix.c
- 's^#include "^#include "avb/^'
The version of libavb imported in this patch series seems to match
commit b60834f7a4a8 ("uefi: Set both androidboot.slot and
androidboot.slot_suffix.") from [1].
FYI, there are 12 more non-merge commits in the avb repository [2]. Is
there a strong reason not to include them?
I am not an expert in AVB, but IMHO, as suggested by Alex, we shouldn't
assume that the in-tree libavb will diverge from the vanilla one. IMHO
a cleaner workflow would be to contribute any bugfixes to the original
repository and update the U-boot libavb from time to time, similar
to how it's done, as example, for dtc in kernel [3]. Or maybe you see
some obstacles to achieve this in practice?
Assuming that:
1) libavb will undergo regular updates from its original repository.
2) most of libavb headers are not designed to be included from
non-libavb code, throwing below:
#error "Never include this file directly, include libavb.h instead."
I wonder if it would be possible to keep the internal libavb headers in
lib/libavb (similar to how it's done by NXP in [4]), since this would
allow not rewriting the original include paths for such headers in every
*.c file and hence minimize the delta between in-tree vs out-of-tree
code, as well as potentially speed up the update process (and/or
simplify any update script which will take care of it, as also
mentioned by Alex).
My final question is what's your opinion about the NXP-specific libavb
wrapper implementation found at `lib/avb/fsl/` in [5]/[6] which seems
to rely on libavb and libavb_ab. Do you see it as a good model to be
followed by other platforms (both from point of view of contents and
placement in the tree)? I am asking because we are in the middle
of some decision-making for similar/alternative implementation on a
different SoC, so your feedback will be extremely helpful and greatly
appreciated.
Thank you!
Best regards,
Eugeniu.
[1] https://android.googlesource.com/platform/external/avb/+/master
[2] Postt-b60834f7a4a8 libavb commits in [1]
$> git log --oneline --no-merges b60834f7a4a8..avb_upstream/master -- libavb
30dd8e5a1757 libavb: Add new routine to calculate a digest of all vbmeta images.
fd0ba0d49101 Implement support for on-device persistent digests.
97740e537ad1 Split kernel cmdline code in separate file.
fcadbf1d1a71 Support (boot) partition preloading.
061e33b39e27 Add avb_div_by_10() sysdep operation.
36e5c43f58d2 Fix incorrect variable names in avb_replace
fc2531374e30 Fix AvbAlgorithmType type-limits error
047ecf7d2361 libavb: Avoid conflict with system-provided crc32 symbol.
0922bf8970fd Make it possible to disable verification.
01ca9962bd0d libavb: Only load and verify hash partition if requested.
8221811c5da1 libavb: Allow specifying dm-verity error handling.
27a291fcc194 libavb: Load entire partition if |allow_verification_error| is true.
[3] The way in-tree kernel DTC is aligned to upsteam:
$> git log --oneline --no-merges -i --grep "update.*upstream" -- scripts/dtc
9130ba884640 scripts/dtc: Update to upstream version v1.4.6-9-gaadd0b65c987
e45fe7f788dd scripts/dtc: Update to upstream version v1.4.5-6-gc1e55a5513e9
4201d057ea91 scripts/dtc: Update to upstream version v1.4.5-3-gb1a60033c110
89d123106a97 scripts/dtc: Update to upstream version v1.4.4-8-g756ffc4f52f6
[4] http://git.freescale.com/git/cgit.cgi/imx/uboot-imx.git/commit/?h=60e14a6a07c5
[5] http://git.freescale.com/git/cgit.cgi/imx/uboot-imx.git/tree/lib/avb/fsl?h=imx_v2015.04_brillo
[6] http://git.freescale.com/git/cgit.cgi/imx/uboot-imx.git/tree/lib/avb/fsl?h=imx_v2017.03_4.9.11_1.0.0_ga
On Sun, May 06, 2018 at 01:31:18PM +0200, Eugeniu Rosca wrote: > Hello Igor, Alex, Kever, > > I wonder if it would be possible to keep the internal libavb headers in > lib/libavb (similar to how it's done by NXP in [4]), since this would > allow not rewriting the original include paths for such headers in every > *.c file and hence minimize the delta between in-tree vs out-of-tree > code, as well as potentially speed up the update process (and/or > simplify any update script which will take care of it, as also > mentioned by Alex). > > [4] http://git.freescale.com/git/cgit.cgi/imx/uboot-imx.git/commit/?h=60e14a6a07c5 Here is what I mean. Three lzma headers are exposed to internal users/consumers: $ git grep '"../../lib/' ---<-snip->--- include/lzma/LzmaDec.h:#include "../../lib/lzma/LzmaDec.h" include/lzma/LzmaTools.h:#include "../../lib/lzma/LzmaTools.h" include/lzma/LzmaTypes.h:#include "../../lib/lzma/Types.h" ---<-snip->--- A few places where lzma headers are used: $ git grep 'include <lzma/Lzma' cmd/lzmadec.c:#include <lzma/LzmaTools.h> common/bootm.c:#include <lzma/LzmaTypes.h> common/bootm.c:#include <lzma/LzmaDec.h> common/bootm.c:#include <lzma/LzmaTools.h> lib/lzma/LzmaTools.h:#include <lzma/LzmaTypes.h> test/compression.c:#include <lzma/LzmaTypes.h> test/compression.c:#include <lzma/LzmaDec.h> test/compression.c:#include <lzma/LzmaTools.h> For libavb this would translate to: $ cat include/avb/libavb.h ---<-snip->--- #include "../../lib/libavb/avb_chain_partition_descriptor.h" #include "../../lib/libavb/avb_crypto.h" #include "../../lib/libavb/avb_descriptor.h" #include "../../lib/libavb/avb_footer.h" #include "../../lib/libavb/avb_hash_descriptor.h" ---<-snip->--- And with that, various consumers (mainly libavb_avb?) would do: #include <avb/libavb.h> As said, this would make integration of new libavb versions much easier. Would appreciate your thoughts. Best regards, Eugeniu.
Hi Eugeniu, You're totally right regarding avb internal headers, they all should remain in lib/libavb. v2 patchset (planning to send it by the end of this week) will include these changes you're talking about (+ will include libavb updates from [1]). > My final question is what's your opinion about the NXP-specific libavb > wrapper implementation found at `lib/avb/fsl/` in [5]/[6] which seems > to rely on libavb and libavb_ab. Do you see it as a good model to be > followed by other platforms (both from point of view of contents and > placement in the tree)? I am asking because we are in the middle > of some decision-making for similar/alternative implementation on a > different SoC, so your feedback will be extremely helpful and greatly > appreciated. I've taken a quick look at patches from [2], and noticed a few issues (IMHO): 1) It includes custom implementation of RPMB operations, although generic RPMB functionality already exists in the U-boot mainline (check drivers/mmc/rpmb.c) 2) It introduces a brand-new "boota" command for booting Android, although existing bootm does already support Android boot image parsing and booting. 3) It doesn't have any dm-verify enforcement policies in case if the bootloader is in the "locked" state (at least, I didn't manage to grep it) 4) There are a bunch of platform-specific code introduced to fastboot driver, and it's used as a base for working with GPT in avb ops (sounds strange, but this is what I see here), although mainline U-boot does have API for this purposes. Obviously, the root cause of all these issues is a big divergence from the U-boot mainline codebase. Thanks! Regards, Igor [1] https://android.googlesource.com/platform/external/avb/+/master [2] http://git.freescale.com/git/cgit.cgi/imx/uboot-imx.git/tree/lib/avb/fsl?h=imx_v2017.03_4.9.11_1.0.0_ga On 15 May 2018 at 18:31, Eugeniu Rosca <erosca@de.adit-jv.com> wrote: > On Sun, May 06, 2018 at 01:31:18PM +0200, Eugeniu Rosca wrote: >> Hello Igor, Alex, Kever, >> >> I wonder if it would be possible to keep the internal libavb headers in >> lib/libavb (similar to how it's done by NXP in [4]), since this would >> allow not rewriting the original include paths for such headers in every >> *.c file and hence minimize the delta between in-tree vs out-of-tree >> code, as well as potentially speed up the update process (and/or >> simplify any update script which will take care of it, as also >> mentioned by Alex). >> >> [4] http://git.freescale.com/git/cgit.cgi/imx/uboot-imx.git/commit/?h=60e14a6a07c5 > > Here is what I mean. > Three lzma headers are exposed to internal users/consumers: > > $ git grep '"../../lib/' > ---<-snip->--- > include/lzma/LzmaDec.h:#include "../../lib/lzma/LzmaDec.h" > include/lzma/LzmaTools.h:#include "../../lib/lzma/LzmaTools.h" > include/lzma/LzmaTypes.h:#include "../../lib/lzma/Types.h" > ---<-snip->--- > > A few places where lzma headers are used: > > $ git grep 'include <lzma/Lzma' > cmd/lzmadec.c:#include <lzma/LzmaTools.h> > common/bootm.c:#include <lzma/LzmaTypes.h> > common/bootm.c:#include <lzma/LzmaDec.h> > common/bootm.c:#include <lzma/LzmaTools.h> > lib/lzma/LzmaTools.h:#include <lzma/LzmaTypes.h> > test/compression.c:#include <lzma/LzmaTypes.h> > test/compression.c:#include <lzma/LzmaDec.h> > test/compression.c:#include <lzma/LzmaTools.h> > > For libavb this would translate to: > > $ cat include/avb/libavb.h > ---<-snip->--- > #include "../../lib/libavb/avb_chain_partition_descriptor.h" > #include "../../lib/libavb/avb_crypto.h" > #include "../../lib/libavb/avb_descriptor.h" > #include "../../lib/libavb/avb_footer.h" > #include "../../lib/libavb/avb_hash_descriptor.h" > ---<-snip->--- > > And with that, various consumers (mainly libavb_avb?) would do: > #include <avb/libavb.h> > > As said, this would make integration of new libavb versions much easier. > Would appreciate your thoughts. > > Best regards, > Eugeniu.
Hi Igor, On Tue, May 15, 2018 at 07:58:51PM +0300, Igor Opaniuk wrote: > Hi Eugeniu, > > You're totally right regarding avb internal headers, they all > should remain in lib/libavb. > v2 patchset (planning to send it by the end of this week) will > include these changes you're talking about (+ will include libavb > updates from [1]). Good to hear that! > > I've taken a quick look at patches from [2], and noticed a few issues (IMHO): > 1) It includes custom implementation of RPMB operations, although > generic RPMB functionality already exists in the U-boot mainline > (check drivers/mmc/rpmb.c) > 2) It introduces a brand-new "boota" command for booting Android, > although existing bootm does already support Android boot image > parsing and booting. > 3) It doesn't have any dm-verify enforcement policies in case > if the bootloader is in the "locked" state (at least, I didn't manage > to grep it) > 4) There are a bunch of platform-specific code introduced to > fastboot driver, and it's used as a base for working with GPT > in avb ops (sounds strange, but this is what I see here), although > mainline U-boot does have API for this purposes. Thanks for the detailed reply. I am looking forward for a cleaner and better partitioned porting of libavb/libavb_ab to the main tree. Your effort is very appreciated! > Regards, > Igor > > [1] https://android.googlesource.com/platform/external/avb/+/master > [2] http://git.freescale.com/git/cgit.cgi/imx/uboot-imx.git/tree/lib/avb/fsl?h=imx_v2017.03_4.9.11_1.0.0_ga Best regards, Eugeniu.