| Message ID | 20180605210857.688122cf3f44bbd0e5bfa152@gmail.com |
|---|---|
| State | Deferred |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | [U-Boot,RFC] spl: vboot: Verify content before using load_addr | expand |
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>
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!
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,
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(-)