Re: [PATCH] common: image-android: Add signature verification feature

2022-07-12 Thread Simon Glass
Hi,

On Thu, 7 Jul 2022 at 05:14,  wrote:
>
> From: qianfan Zhao 
>
> Not all Android tools use the id field for signing the image with sha1,
> so make this feature as optional and disabled default.
>
> Signed-off-by: qianfan Zhao 
> ---
>  common/Kconfig.boot| 14 +++
>  common/image-android.c | 55 +-
>  2 files changed, 63 insertions(+), 6 deletions(-)

Please update the test to cover this too:

test/py/tests/test_android/test_avb.py

Regards,
Simon


[PATCH] common: image-android: Add signature verification feature

2022-07-07 Thread qianfanguijin
From: qianfan Zhao 

Not all Android tools use the id field for signing the image with sha1,
so make this feature as optional and disabled default.

Signed-off-by: qianfan Zhao 
---
 common/Kconfig.boot| 14 +++
 common/image-android.c | 55 +-
 2 files changed, 63 insertions(+), 6 deletions(-)

diff --git a/common/Kconfig.boot b/common/Kconfig.boot
index a8d4be23a9..11fc8410a9 100644
--- a/common/Kconfig.boot
+++ b/common/Kconfig.boot
@@ -9,6 +9,20 @@ config ANDROID_BOOT_IMAGE
  This enables support for booting images which use the Android
  image format header.
 
+if ANDROID_BOOT_IMAGE
+
+config ANDROID_BOOT_IMAGE_VERIFY
+   bool "Enable signature verification"
+   select SHA1
+   default n
+   help
+ This option enables signature verification of Android Boot Images,
+ using a hash signed and verified using SHA1.
+ Not all Android tools use the id field for signing the image with
+ sha1, if you are unsure about this, Say N here.
+
+endif # ANDROID_BOOT_IMAGE
+
 config FIT
bool "Support Flattened Image Tree"
select HASH
diff --git a/common/image-android.c b/common/image-android.c
index 1fa1eb..12b84d6fdc 100644
--- a/common/image-android.c
+++ b/common/image-android.c
@@ -44,11 +44,54 @@ static ulong android_image_get_kernel_addr(const struct 
andr_img_hdr *hdr)
return hdr->kernel_addr;
 }
 
+#if CONFIG_IS_ENABLED(ANDROID_BOOT_IMAGE_VERIFY)
+/*
+ * Not all Android tools use the id field for signing the image with
+ * sha1 (or anything) so we make this as an optional.
+ */
+static void android_image_update_sha1(sha1_context *ctx, const void *buf,
+ u32 size)
+{
+   if (size > 0) {
+   sha1_update(ctx, buf, size);
+   size = cpu_to_le32(size);
+   sha1_update(ctx, (unsigned char *), sizeof(size));
+   }
+}
+
+static int android_image_verify_sha1(const struct andr_img_hdr *hdr)
+{
+   unsigned char output[20];
+   sha1_context ctx;
+   const void *p;
+
+   sha1_starts();
+
+   p = (void *)hdr + hdr->page_size; /* kernel */
+   android_image_update_sha1(, p, hdr->kernel_size);
+
+   p += ALIGN(hdr->kernel_size, hdr->page_size); /* ramdisk */
+   android_image_update_sha1(, p, hdr->ramdisk_size);
+
+   p += ALIGN(hdr->ramdisk_size, hdr->page_size); /* second */
+   android_image_update_sha1(, p, hdr->second_size);
+
+   sha1_finish(, output);
+
+   return memcmp(output, (unsigned char *)hdr->id, sizeof(output));
+}
+#else
+static int android_image_verify_sha1(const struct andr_img_hdr *hdr)
+{
+   return 0;
+}
+#endif
+
 /**
  * android_image_get_kernel() - processes kernel part of Android boot images
  * @hdr:   Pointer to image header, which is at the start
  * of the image.
- * @verify:Checksum verification flag. Currently unimplemented.
+ * @verify:Checksum verification flag.
  * @os_data:   Pointer to a ulong variable, will hold os data start
  * address.
  * @os_len:Pointer to a ulong variable, will hold os data length.
@@ -66,11 +109,11 @@ int android_image_get_kernel(const struct andr_img_hdr 
*hdr, int verify,
const struct image_header *ihdr = (const struct image_header *)
((uintptr_t)hdr + hdr->page_size);
 
-   /*
-* Not all Android tools use the id field for signing the image with
-* sha1 (or anything) so we don't check it. It is not obvious that the
-* string is null terminated so we take care of this.
-*/
+   if (verify && android_image_verify_sha1(hdr) < 0) {
+   puts("Bad Data Hash\n");
+   return -EACCES;
+   }
+
strncpy(andr_tmp_str, hdr->name, ANDR_BOOT_NAME_SIZE);
andr_tmp_str[ANDR_BOOT_NAME_SIZE] = '\0';
if (strlen(andr_tmp_str))
-- 
2.25.1