[U-Boot] [RFC][PATCH] spl: vboot: Verify content before using load_addr

[U-Boot] [RFC][PATCH] spl: vboot: Verify content before using load_addr

[Teddy Reed]

When using verified-boot in the SPL, the FIT content must be
verified before it can be used.

Currently the load_addr FIT property is read and used as input to
memcpy before the property is verified.

Signed-off-by: Teddy Reed <teddy.reed@gmail.com>
---
 common/spl/spl_fit.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
index 2321ebb0dde..a35c6092cee 100644
--- a/common/spl/spl_fit.c
+++ b/common/spl/spl_fit.c
@@ -244,6 +244,16 @@ static int spl_load_fit_image(struct spl_load_info *info, ulong sector,
 		src = (void *)data;
 	}
 
+#ifdef CONFIG_SPL_FIT_SIGNATURE
+	printf("## Checking hash(es) for Image %s ...\n",
+	       fit_get_name(fit, node, NULL));
+	ret = fit_image_verify_with_data(fit, node,
+					 (const void *)src, length);
+	printf("\n");
+	if (!ret)
+		return 1;
+#endif
+
 #ifdef CONFIG_SPL_FIT_IMAGE_POST_PROCESS
 	board_fit_image_post_process(&src, &length);
 #endif
@@ -269,16 +279,7 @@ static int spl_load_fit_image(struct spl_load_info *info, ulong sector,
 		image_info->entry_point = fdt_getprop_u32(fit, node, "entry");
 	}
 
-#ifdef CONFIG_SPL_FIT_SIGNATURE
-	printf("## Checking hash(es) for Image %s ...\n",
-	       fit_get_name(fit, node, NULL));
-	ret = fit_image_verify_with_data(fit, node,
-					 (const void *)load_addr, length);
-	printf("\n");
-	return !ret;
-#else
 	return 0;
-#endif
 }
 
 static int spl_fit_append_fdt(struct spl_image_info *spl_image,
-- 
2.13.5

[U-Boot] [RFC][PATCH] spl: vboot: Verify content before using load_addr

[Jun Nie]

2018-06-06 9:08 GMT+08:00 Teddy Reed <teddy.reed@gmail.com>:
> When using verified-boot in the SPL, the FIT content must be
> verified before it can be used.
>
> Currently the load_addr FIT property is read and used as input to
> memcpy before the property is verified.
>
> Signed-off-by: Teddy Reed <teddy.reed@gmail.com>
> ---

Reviewed-by: Jun Nie <jun.nie@linaro.org>

[U-Boot] [RFC][PATCH] spl: vboot: Verify content before using load_addr

[Teddy Reed]

On Wed, Jun 6, 2018 at 4:35 AM, Jun Nie <jun.nie@linaro.org> wrote:
> 2018-06-06 9:08 GMT+08:00 Teddy Reed <teddy.reed@gmail.com>:
>> When using verified-boot in the SPL, the FIT content must be
>> verified before it can be used.
>>
>> Currently the load_addr FIT property is read and used as input to
>> memcpy before the property is verified.
>>
>> Signed-off-by: Teddy Reed <teddy.reed@gmail.com>
>> ---
>
> Reviewed-by: Jun Nie <jun.nie@linaro.org>

Thanks for taking a look Jun! I did not see any sandbox tests
exercising the SPL/signature checking so I included 'RFC'. I know you
are familiar with using the signature checking within the SPL, and you
helped make it possible in the first place.

The three minor concerns I had when moving the validation earlier are:
(1) the signature was originally calculated on the potentially
uncompressed version, but this seems unlikely; similarly (2) the post
process board-specific methods can mutate the signed content; and (3)
now using the src address means reading directly from storage / etc
instead of potentially SRAM/DRAM.

For (1) see that line 256 was uncompressing the data before signature
checking. Then the signature check was applied to the uncompressed
region. To be clear I think this patch is the correct approach, and
the signature check should apply to the compressed content.

For (2) see that line 248 is board-specific, I am assuming that can
mutate the content. Thus the signature check should be placed before
that call.

Thanks again!

-- 
Teddy Reed V