Re: [U-Boot] [PATCH v2 1/1] avb: add support for named persistent values
Update: Patch for OP-TEE AVB trusted application (which introduces
implementation for persistent named values support on secure world
side) was successfully merged [1].
[1]: https://github.com/OP-TEE/optee_os/pull/2699
On Fri, 14 Dec 2018 at 19:45, Igor Opaniuk wrote:
>
> AVB version 1.1 introduces support for named persistent values
> that must be tamper evident and allows AVB to store arbitrary key-value
> pairs [1].
>
> Introduce implementation of two additional AVB operations
> read_persistent_value()/write_persistent_value() for retrieving/storing
> named persistent values.
>
> Correspondent pull request in the OP-TEE OS project repo [2].
>
> [1]:
> https://android.googlesource.com/platform/external/avb/+/android-9.0.0_r22
> [2]: https://github.com/OP-TEE/optee_os/pull/2699
>
> Signed-off-by: Igor Opaniuk
> ---
>
> Changes in v2:
> - fix output format for avb read_pvalue/write_pvalue commands
> - fix issue with named value buffer size
>
> cmd/avb.c | 78 +
> common/avb_verify.c| 122
> +
> include/tee.h | 2 +
> include/tee/optee_ta_avb.h | 16 ++
> 4 files changed, 218 insertions(+)
>
> diff --git a/cmd/avb.c b/cmd/avb.c
> index ff00be4..8387cc7 100644
> --- a/cmd/avb.c
> +++ b/cmd/avb.c
> @@ -340,6 +340,76 @@ int do_avb_is_unlocked(cmd_tbl_t *cmdtp, int flag,
> return CMD_RET_FAILURE;
> }
>
> +int do_avb_read_pvalue(cmd_tbl_t *cmdtp, int flag, int argc,
> + char * const argv[])
> +{
> + const char *name;
> + size_t bytes;
> + size_t bytes_read;
> + void *buffer;
> +
> + if (!avb_ops) {
> + printf("AVB 2.0 is not initialized, run 'avb init' first\n");
> + return CMD_RET_FAILURE;
> + }
> +
> + if (argc != 3)
> + return CMD_RET_USAGE;
> +
> + name = argv[1];
> + bytes = simple_strtoul(argv[2], NULL, 10);
> + buffer = malloc(bytes);
> + if (!buffer)
> + return CMD_RET_FAILURE;
> +
> + printf("Reading persistent value, name = %s, bytes = %ld\n",
> + name, bytes);
> + if (avb_ops->read_persistent_value(avb_ops, name, bytes, buffer,
> + &bytes_read) == AVB_IO_RESULT_OK) {
> + printf("Read %ld bytes, value = %s\n", bytes_read,
> + (char *)buffer);
> + free(buffer);
> + return CMD_RET_SUCCESS;
> + }
> +
> + printf("Failed to write in partition\n");
> +
> + free(buffer);
> +
> + return CMD_RET_FAILURE;
> +}
> +
> +int do_avb_write_pvalue(cmd_tbl_t *cmdtp, int flag, int argc,
> + char * const argv[])
> +{
> + const char *name;
> + const char *value;
> +
> + if (!avb_ops) {
> + printf("AVB 2.0 is not initialized, run 'avb init' first\n");
> + return CMD_RET_FAILURE;
> + }
> +
> + if (argc != 3)
> + return CMD_RET_USAGE;
> +
> + name = argv[1];
> + value = argv[2];
> +
> + printf("Writing persistent value, name = %s, value = %s\n",
> + name, value);
> + if (avb_ops->write_persistent_value(avb_ops, name, strlen(value) + 1,
> + (const uint8_t *)value) ==
> + AVB_IO_RESULT_OK) {
> + printf("Wrote %ld bytes\n", strlen(value) + 1);
> + return CMD_RET_SUCCESS;
> + }
> +
> + printf("Failed to write persistent value\n");
> +
> + return CMD_RET_FAILURE;
> +}
> +
> static cmd_tbl_t cmd_avb[] = {
> U_BOOT_CMD_MKENT(init, 2, 0, do_avb_init, "", ""),
> U_BOOT_CMD_MKENT(read_rb, 2, 0, do_avb_read_rb, "", ""),
> @@ -350,6 +420,10 @@ static cmd_tbl_t cmd_avb[] = {
> 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, "", ""),
> +#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, "", ""),
> +#endif
> };
>
> static int do_avb(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> @@ -384,6 +458,10 @@ U_BOOT_CMD(
> "partition and print to stdout\n"
> "avb write_part - write bytes
> to\n"
> " by using data from \n"
> +#ifdef CONFIG_OPTEE_TA_AVB
> + "avb read_pvalue - read a persistent value \n"
> + "avb write_pvalue - write a persistent value \n"
> +#endif
> "avb verify - run verification process using hash data\n"
> "from vbmeta structure\n"
> );
> diff --git a/common/avb_verify.c b/common/avb_verify.c
> index a8c5a3e..292ad8f 100644
> --- a/common/avb_verify.c
> +++ b/common/avb_verify.c
> @@ -647,6 +647
Re: [U-Boot] [PATCH v2 1/1] avb: add support for named persistent values
Hi Igor, On Fri, 14 Dec 2018 at 10:45, Igor Opaniuk wrote: > > AVB version 1.1 introduces support for named persistent values > that must be tamper evident and allows AVB to store arbitrary key-value > pairs [1]. > > Introduce implementation of two additional AVB operations > read_persistent_value()/write_persistent_value() for retrieving/storing > named persistent values. > > Correspondent pull request in the OP-TEE OS project repo [2]. > > [1]: > https://android.googlesource.com/platform/external/avb/+/android-9.0.0_r22 > [2]: https://github.com/OP-TEE/optee_os/pull/2699 > > Signed-off-by: Igor Opaniuk > --- > > Changes in v2: > - fix output format for avb read_pvalue/write_pvalue commands > - fix issue with named value buffer size > > cmd/avb.c | 78 + > common/avb_verify.c| 122 > + > include/tee.h | 2 + > include/tee/optee_ta_avb.h | 16 ++ > 4 files changed, 218 insertions(+) Doesn't this need an update to the Android update test? Regards, Simon ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2 1/1] avb: add support for named persistent values
Hi Simon, Could you please point me to the update test you mean? (I assume it's "test_avb.py"?) Thanks BR, Igor On Sat, 22 Dec 2018 at 22:52, Simon Glass wrote: > > Hi Igor, > > On Fri, 14 Dec 2018 at 10:45, Igor Opaniuk wrote: > > > > AVB version 1.1 introduces support for named persistent values > > that must be tamper evident and allows AVB to store arbitrary key-value > > pairs [1]. > > > > Introduce implementation of two additional AVB operations > > read_persistent_value()/write_persistent_value() for retrieving/storing > > named persistent values. > > > > Correspondent pull request in the OP-TEE OS project repo [2]. > > > > [1]: > > https://android.googlesource.com/platform/external/avb/+/android-9.0.0_r22 > > [2]: https://github.com/OP-TEE/optee_os/pull/2699 > > > > Signed-off-by: Igor Opaniuk > > --- > > > > Changes in v2: > > - fix output format for avb read_pvalue/write_pvalue commands > > - fix issue with named value buffer size > > > > cmd/avb.c | 78 + > > common/avb_verify.c| 122 > > + > > include/tee.h | 2 + > > include/tee/optee_ta_avb.h | 16 ++ > > 4 files changed, 218 insertions(+) > > Doesn't this need an update to the Android update test? > > Regards, > Simon -- Regards, Igor Opaniuk ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2 1/1] avb: add support for named persistent values
Hi Igor, On Thu, 27 Dec 2018 at 07:50, Igor Opaniuk wrote: > > Hi Simon, > > Could you please point me to the update test you mean? (I assume it's > "test_avb.py"?) Yes that's right. The test should cover the functionality of the feature. Regards, Simon ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2 1/1] avb: add support for named persistent values
ok, np. will send in v3 patch On Thu, 27 Dec 2018 at 17:12, Simon Glass wrote: > > Hi Igor, > > On Thu, 27 Dec 2018 at 07:50, Igor Opaniuk wrote: > > > > Hi Simon, > > > > Could you please point me to the update test you mean? (I assume it's > > "test_avb.py"?) > > Yes that's right. The test should cover the functionality of the feature. > > Regards, > Simon Thanks for the notice, will be added in v3! -- Regards, Igor Opaniuk ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2 1/1] avb: add support for named persistent values
Hi Igor,
Some comments below.
On Fri, Dec 14, 2018 at 07:45:03PM +0200, Igor Opaniuk wrote:
> AVB version 1.1 introduces support for named persistent values
> that must be tamper evident and allows AVB to store arbitrary key-value
> pairs [1].
>
> Introduce implementation of two additional AVB operations
> read_persistent_value()/write_persistent_value() for retrieving/storing
> named persistent values.
>
> Correspondent pull request in the OP-TEE OS project repo [2].
>
> [1]:
> https://android.googlesource.com/platform/external/avb/+/android-9.0.0_r22
> [2]: https://github.com/OP-TEE/optee_os/pull/2699
>
> Signed-off-by: Igor Opaniuk
> ---
>
> Changes in v2:
> - fix output format for avb read_pvalue/write_pvalue commands
> - fix issue with named value buffer size
>
> cmd/avb.c | 78 +
> common/avb_verify.c| 122
> +
> include/tee.h | 2 +
> include/tee/optee_ta_avb.h | 16 ++
> 4 files changed, 218 insertions(+)
>
> diff --git a/cmd/avb.c b/cmd/avb.c
> index ff00be4..8387cc7 100644
> --- a/cmd/avb.c
> +++ b/cmd/avb.c
> @@ -340,6 +340,76 @@ int do_avb_is_unlocked(cmd_tbl_t *cmdtp, int flag,
> return CMD_RET_FAILURE;
> }
>
> +int do_avb_read_pvalue(cmd_tbl_t *cmdtp, int flag, int argc,
> +char * const argv[])
> +{
> + const char *name;
> + size_t bytes;
> + size_t bytes_read;
> + void *buffer;
> +
> + if (!avb_ops) {
> + printf("AVB 2.0 is not initialized, run 'avb init' first\n");
> + return CMD_RET_FAILURE;
> + }
> +
> + if (argc != 3)
> + return CMD_RET_USAGE;
> +
> + name = argv[1];
> + bytes = simple_strtoul(argv[2], NULL, 10);
This parses without error check and would for instance parse 123hello as
123.
> + buffer = malloc(bytes);
> + if (!buffer)
> + return CMD_RET_FAILURE;
> +
> + printf("Reading persistent value, name = %s, bytes = %ld\n",
> +name, bytes);
> + if (avb_ops->read_persistent_value(avb_ops, name, bytes, buffer,
> +&bytes_read) == AVB_IO_RESULT_OK) {
> + printf("Read %ld bytes, value = %s\n", bytes_read,
> +(char *)buffer);
> + free(buffer);
> + return CMD_RET_SUCCESS;
> + }
> +
> + printf("Failed to write in partition\n");
read
> +
> + free(buffer);
> +
> + return CMD_RET_FAILURE;
> +}
> +
> +int do_avb_write_pvalue(cmd_tbl_t *cmdtp, int flag, int argc,
> + char * const argv[])
> +{
> + const char *name;
> + const char *value;
> +
> + if (!avb_ops) {
> + printf("AVB 2.0 is not initialized, run 'avb init' first\n");
> + return CMD_RET_FAILURE;
> + }
> +
> + if (argc != 3)
> + return CMD_RET_USAGE;
> +
> + name = argv[1];
> + value = argv[2];
> +
> + printf("Writing persistent value, name = %s, value = %s\n",
> +name, value);
> + if (avb_ops->write_persistent_value(avb_ops, name, strlen(value) + 1,
> + (const uint8_t *)value) ==
> + AVB_IO_RESULT_OK) {
> + printf("Wrote %ld bytes\n", strlen(value) + 1);
> + return CMD_RET_SUCCESS;
> + }
> +
> + printf("Failed to write persistent value\n");
> +
> + return CMD_RET_FAILURE;
> +}
> +
> static cmd_tbl_t cmd_avb[] = {
> U_BOOT_CMD_MKENT(init, 2, 0, do_avb_init, "", ""),
> U_BOOT_CMD_MKENT(read_rb, 2, 0, do_avb_read_rb, "", ""),
> @@ -350,6 +420,10 @@ static cmd_tbl_t cmd_avb[] = {
> 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, "", ""),
> +#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, "", ""),
> +#endif
> };
>
> static int do_avb(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> @@ -384,6 +458,10 @@ U_BOOT_CMD(
> "partition and print to stdout\n"
> "avb write_part - write bytes
> to\n"
> " by using data from \n"
> +#ifdef CONFIG_OPTEE_TA_AVB
> + "avb read_pvalue - read a persistent value \n"
> + "avb write_pvalue - write a persistent value \n"
> +#endif
> "avb verify - run verification process using hash data\n"
> "from vbmeta structure\n"
> );
> diff --git a/common/avb_verify.c b/common/avb_verify.c
> index a8c5a3e..292ad8f 100644
> --- a/common/avb_verify.c
> +++ b/common/avb_verify.c
> @@ -647,6 +647,10 @@ static AvbIOResult invoke_func(struct AvbOpsData
> *ops_data, u32 func,
> return AVB_IO_RESULT_OK;
> case TEE_ERROR_OUT_OF_MEMORY:
> return AVB_IO_RESULT_ERROR_OOM;
> +
