| Message ID | 1466480052-25004-6-git-send-email-dannenberg@ti.com |
|---|---|
| State | New |
| Headers | show |
Hi Lokesh--- comment inlined... On Tue, Jun 21, 2016 at 10:01:54AM +0530, Lokesh Vutla wrote: > > > On Tuesday 21 June 2016 09:04 AM, Andreas Dannenberg wrote: > > Adds an API that verifies a signature attached to an image (binary > > blob). This API is basically a entry to a secure ROM service provided by > > the device and accessed via an SMC call, using a particular calling > > convention. > > > > Signed-off-by: Daniel Allred <d-allred@ti.com> > > Signed-off-by: Andreas Dannenberg <dannenberg@ti.com> > > --- > > arch/arm/cpu/armv7/omap-common/sec-common.c | 76 +++++++++++++++++++++++++++++ > > arch/arm/include/asm/omap_common.h | 9 ++++ > > 2 files changed, 85 insertions(+) > > > > diff --git a/arch/arm/cpu/armv7/omap-common/sec-common.c b/arch/arm/cpu/armv7/omap-common/sec-common.c > > index b9c0a42..dbb9078 100644 > > --- a/arch/arm/cpu/armv7/omap-common/sec-common.c > > +++ b/arch/arm/cpu/armv7/omap-common/sec-common.c > > @@ -16,6 +16,9 @@ > > #include <asm/arch/sys_proto.h> > > #include <asm/omap_common.h> > > > > +/* Index for signature verify ROM API */ > > +#define API_HAL_KM_VERIFYCERTIFICATESIGNATURE_INDEX (0x0000000E) > > + > > static uint32_t secure_rom_call_args[5] __aligned(ARCH_DMA_MINALIGN); > > > > u32 secure_rom_call(u32 service, u32 proc_id, u32 flag, ...) > > @@ -47,3 +50,76 @@ u32 secure_rom_call(u32 service, u32 proc_id, u32 flag, ...) > > > > return omap_smc_sec(service, proc_id, flag, secure_rom_call_args); > > } > > + > > +static u32 find_sig_start(char *image, size_t size) > > +{ > > + char *image_end = image + size; > > + char *sig_start_magic = "CERT_"; > > + int magic_str_len = strlen(sig_start_magic); > > + char *ch; > > + > > + while (--image_end > image) { > > + if (*image_end == '_') { > > + ch = image_end - magic_str_len + 1; > > + if (!strncmp(ch, sig_start_magic, magic_str_len)) > > + return (u32)ch; > > + } > > + } > > + return 0; > > +} > > + > > +int secure_boot_verify_image(void **image, size_t *size) > > +{ > > + int result = 1; > > + u32 cert_addr, sig_addr; > > + size_t cert_size; > > + > > + /* Perform cache writeback on input buffer */ > > + flush_dcache_range( > > + (u32)*image, > > + (u32)*image + roundup(*size, ARCH_DMA_MINALIGN)); > > + > > + cert_addr = (uint32_t)*image; > > + sig_addr = find_sig_start((char *)*image, *size); > > + > > + if (sig_addr == 0) { > > + printf("No signature found in image.\n"); > > + result = 1; > > + goto auth_exit; > > + } > > + > > + *size = sig_addr - cert_addr; /* Subtract out the signature size */ > > + cert_size = *size; > > + > > + /* Check if image load address is 32-bit aligned */ > > + if (0 != (0x3 & cert_addr)) { > > if (!IS_ALIGNED(cert_addr, 4)) { ? Good suggestion! Will simplify. > > + printf("Image is not 4-byte aligned.\n"); > > + result = 1; > > + goto auth_exit; > > + } > > + > > + /* Image size also should be multiple of 4 */ > > + if (0 != (0x3 & cert_size)) { > > if (!IS_ALIGNED(cert_size, 4)) { ? ditto. > > + printf("Image size is not 4-byte aligned.\n"); > > + result = 1; > > + goto auth_exit; > > + } > > + > > + /* Call ROM HAL API to verify certificate signature */ > > + debug("%s: load_addr = %x, size = %x, sig_addr = %x\n", __func__, > > + cert_addr, cert_size, sig_addr); > > + > > + result = secure_rom_call( > > + API_HAL_KM_VERIFYCERTIFICATESIGNATURE_INDEX, 0, 0, > > + 4, cert_addr, cert_size, sig_addr, 0xFFFFFFFF); > > +auth_exit: > > + if (result != 0) { > > + printf("Authentication failed!\n"); > > + printf("Return Value = %08X\n", result); > > + hang(); > > + } > > + > > + printf("Authentication passed: %s\n", (char *)sig_addr); > > Uart boot will break because of these prints during the FIT loading. Can > you make this as debug? > > Prints in the failed case is fine as we need to know if it is failed. Oh yes, the UART boot. We actually explicitly added this printf for successful authentication as the regular U-Boot verified boot also uses some form of "positive feedback" (in form of a '+' sign or so) and we felt its important feedback to _always_ provide some assurance that the chain of trust is actually established (and not interrupted by a build or setup error or something else). So let's say we do want to keep this output, are there any other options? Like suppressing it only in case of SPL UART boot, but not in any of the other modes (sounds ugly but maybe there is another way)? Thanks, Andreas > > Thanks and regards, > Lokesh > > > + > > + return result; > > +} > > diff --git a/arch/arm/include/asm/omap_common.h b/arch/arm/include/asm/omap_common.h > > index 293fc72..8887335 100644 > > --- a/arch/arm/include/asm/omap_common.h > > +++ b/arch/arm/include/asm/omap_common.h > > @@ -640,6 +640,15 @@ u32 omap_smc_sec(u32 service, u32 proc_id, u32 flag, u32 *params); > > */ > > u32 secure_rom_call(u32 service, u32 proc_id, u32 flag, ...); > > > > +/* > > + * Invoke a secure ROM API on high-secure (HS) device variants that can be used > > + * to verify a secure blob by authenticating and optionally decrypting it. The > > + * exact operation performed depends on how the certificate that was embedded > > + * into the blob during the signing/encryption step when the secure blob was > > + * first created. > > + */ > > +int secure_boot_verify_image(void **p_image, size_t *p_size); > > + > > void enable_edma3_clocks(void); > > void disable_edma3_clocks(void); > > > > _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
On Wed, Jun 22, 2016 at 03:13:04PM +0530, Lokesh Vutla wrote: > > > On Wednesday 22 June 2016 05:26 AM, Tom Rini wrote: > > On Tue, Jun 21, 2016 at 10:01:54AM +0530, Lokesh Vutla wrote: > >> > >> > >> On Tuesday 21 June 2016 09:04 AM, Andreas Dannenberg wrote: > >>> Adds an API that verifies a signature attached to an image (binary > >>> blob). This API is basically a entry to a secure ROM service provided by > >>> the device and accessed via an SMC call, using a particular calling > >>> convention. > >>> > >>> Signed-off-by: Daniel Allred <d-allred@ti.com> > >>> Signed-off-by: Andreas Dannenberg <dannenberg@ti.com> > >>> --- > >>> arch/arm/cpu/armv7/omap-common/sec-common.c | 76 +++++++++++++++++++++++++++++ > >>> arch/arm/include/asm/omap_common.h | 9 ++++ > >>> 2 files changed, 85 insertions(+) > >>> > >>> diff --git a/arch/arm/cpu/armv7/omap-common/sec-common.c b/arch/arm/cpu/armv7/omap-common/sec-common.c > >>> index b9c0a42..dbb9078 100644 > >>> --- a/arch/arm/cpu/armv7/omap-common/sec-common.c > >>> +++ b/arch/arm/cpu/armv7/omap-common/sec-common.c > >>> @@ -16,6 +16,9 @@ > >>> #include <asm/arch/sys_proto.h> > >>> #include <asm/omap_common.h> > >>> > >>> +/* Index for signature verify ROM API */ > >>> +#define API_HAL_KM_VERIFYCERTIFICATESIGNATURE_INDEX (0x0000000E) > >>> + > >>> static uint32_t secure_rom_call_args[5] __aligned(ARCH_DMA_MINALIGN); > >>> > >>> u32 secure_rom_call(u32 service, u32 proc_id, u32 flag, ...) > >>> @@ -47,3 +50,76 @@ u32 secure_rom_call(u32 service, u32 proc_id, u32 flag, ...) > >>> > >>> return omap_smc_sec(service, proc_id, flag, secure_rom_call_args); > >>> } > >>> + > >>> +static u32 find_sig_start(char *image, size_t size) > >>> +{ > >>> + char *image_end = image + size; > >>> + char *sig_start_magic = "CERT_"; > >>> + int magic_str_len = strlen(sig_start_magic); > >>> + char *ch; > >>> + > >>> + while (--image_end > image) { > >>> + if (*image_end == '_') { > >>> + ch = image_end - magic_str_len + 1; > >>> + if (!strncmp(ch, sig_start_magic, magic_str_len)) > >>> + return (u32)ch; > >>> + } > >>> + } > >>> + return 0; > >>> +} > >>> + > >>> +int secure_boot_verify_image(void **image, size_t *size) > >>> +{ > >>> + int result = 1; > >>> + u32 cert_addr, sig_addr; > >>> + size_t cert_size; > >>> + > >>> + /* Perform cache writeback on input buffer */ > >>> + flush_dcache_range( > >>> + (u32)*image, > >>> + (u32)*image + roundup(*size, ARCH_DMA_MINALIGN)); > >>> + > >>> + cert_addr = (uint32_t)*image; > >>> + sig_addr = find_sig_start((char *)*image, *size); > >>> + > >>> + if (sig_addr == 0) { > >>> + printf("No signature found in image.\n"); > >>> + result = 1; > >>> + goto auth_exit; > >>> + } > >>> + > >>> + *size = sig_addr - cert_addr; /* Subtract out the signature size */ > >>> + cert_size = *size; > >>> + > >>> + /* Check if image load address is 32-bit aligned */ > >>> + if (0 != (0x3 & cert_addr)) { > >> > >> if (!IS_ALIGNED(cert_addr, 4)) { ? > >> > >>> + printf("Image is not 4-byte aligned.\n"); > >>> + result = 1; > >>> + goto auth_exit; > >>> + } > >>> + > >>> + /* Image size also should be multiple of 4 */ > >>> + if (0 != (0x3 & cert_size)) { > >> > >> if (!IS_ALIGNED(cert_size, 4)) { ? > >> > >>> + printf("Image size is not 4-byte aligned.\n"); > >>> + result = 1; > >>> + goto auth_exit; > >>> + } > >>> + > >>> + /* Call ROM HAL API to verify certificate signature */ > >>> + debug("%s: load_addr = %x, size = %x, sig_addr = %x\n", __func__, > >>> + cert_addr, cert_size, sig_addr); > >>> + > >>> + result = secure_rom_call( > >>> + API_HAL_KM_VERIFYCERTIFICATESIGNATURE_INDEX, 0, 0, > >>> + 4, cert_addr, cert_size, sig_addr, 0xFFFFFFFF); > >>> +auth_exit: > >>> + if (result != 0) { > >>> + printf("Authentication failed!\n"); > >>> + printf("Return Value = %08X\n", result); > >>> + hang(); > >>> + } > >>> + > >>> + printf("Authentication passed: %s\n", (char *)sig_addr); > >> > >> Uart boot will break because of these prints during the FIT loading. Can > >> you make this as debug? > > > > Are you sure it will break? There's usually a print in between loading > > SPL via UART and then U-Boot itself via UART and Y-MODEM is smart enough > > to re-transmit. > > > > Yes, if the print is in between while Y-MODEM is transferring. The above > print falls in this case. Tom et al., so if this really breaks stuff I need to do something about it. As said I'd really like to keep the "Authentication passed: <certificate name>" message in the boot log. So if I implement something along the lines what Lokesh suggested: "...you can check if (spl_boot_device() != BOOT_DEVICE_UART) under the config CONFIG_SPL_YMODEM_SUPPORT. Not sure if it is a good way to do..." to selectivly suppress the message in case of UART boot, would this be acceptable? Or is there a better way? Thanks and Regards, Andreas _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
On Wed, Jun 22, 2016 at 10:36:27AM -0400, Tom Rini wrote: > On Wed, Jun 22, 2016 at 09:21:28AM -0500, Andreas Dannenberg wrote: > > On Wed, Jun 22, 2016 at 03:13:04PM +0530, Lokesh Vutla wrote: > > > > > > > > > On Wednesday 22 June 2016 05:26 AM, Tom Rini wrote: > > > > On Tue, Jun 21, 2016 at 10:01:54AM +0530, Lokesh Vutla wrote: > > > >> > > > >> > > > >> On Tuesday 21 June 2016 09:04 AM, Andreas Dannenberg wrote: > > > >>> Adds an API that verifies a signature attached to an image (binary > > > >>> blob). This API is basically a entry to a secure ROM service provided by > > > >>> the device and accessed via an SMC call, using a particular calling > > > >>> convention. > > > >>> > > > >>> Signed-off-by: Daniel Allred <d-allred@ti.com> > > > >>> Signed-off-by: Andreas Dannenberg <dannenberg@ti.com> > > > >>> --- > > > >>> arch/arm/cpu/armv7/omap-common/sec-common.c | 76 +++++++++++++++++++++++++++++ > > > >>> arch/arm/include/asm/omap_common.h | 9 ++++ > > > >>> 2 files changed, 85 insertions(+) > > > >>> > > > >>> diff --git a/arch/arm/cpu/armv7/omap-common/sec-common.c b/arch/arm/cpu/armv7/omap-common/sec-common.c > > > >>> index b9c0a42..dbb9078 100644 > > > >>> --- a/arch/arm/cpu/armv7/omap-common/sec-common.c > > > >>> +++ b/arch/arm/cpu/armv7/omap-common/sec-common.c > > > >>> @@ -16,6 +16,9 @@ > > > >>> #include <asm/arch/sys_proto.h> > > > >>> #include <asm/omap_common.h> > > > >>> > > > >>> +/* Index for signature verify ROM API */ > > > >>> +#define API_HAL_KM_VERIFYCERTIFICATESIGNATURE_INDEX (0x0000000E) > > > >>> + > > > >>> static uint32_t secure_rom_call_args[5] __aligned(ARCH_DMA_MINALIGN); > > > >>> > > > >>> u32 secure_rom_call(u32 service, u32 proc_id, u32 flag, ...) > > > >>> @@ -47,3 +50,76 @@ u32 secure_rom_call(u32 service, u32 proc_id, u32 flag, ...) > > > >>> > > > >>> return omap_smc_sec(service, proc_id, flag, secure_rom_call_args); > > > >>> } > > > >>> + > > > >>> +static u32 find_sig_start(char *image, size_t size) > > > >>> +{ > > > >>> + char *image_end = image + size; > > > >>> + char *sig_start_magic = "CERT_"; > > > >>> + int magic_str_len = strlen(sig_start_magic); > > > >>> + char *ch; > > > >>> + > > > >>> + while (--image_end > image) { > > > >>> + if (*image_end == '_') { > > > >>> + ch = image_end - magic_str_len + 1; > > > >>> + if (!strncmp(ch, sig_start_magic, magic_str_len)) > > > >>> + return (u32)ch; > > > >>> + } > > > >>> + } > > > >>> + return 0; > > > >>> +} > > > >>> + > > > >>> +int secure_boot_verify_image(void **image, size_t *size) > > > >>> +{ > > > >>> + int result = 1; > > > >>> + u32 cert_addr, sig_addr; > > > >>> + size_t cert_size; > > > >>> + > > > >>> + /* Perform cache writeback on input buffer */ > > > >>> + flush_dcache_range( > > > >>> + (u32)*image, > > > >>> + (u32)*image + roundup(*size, ARCH_DMA_MINALIGN)); > > > >>> + > > > >>> + cert_addr = (uint32_t)*image; > > > >>> + sig_addr = find_sig_start((char *)*image, *size); > > > >>> + > > > >>> + if (sig_addr == 0) { > > > >>> + printf("No signature found in image.\n"); > > > >>> + result = 1; > > > >>> + goto auth_exit; > > > >>> + } > > > >>> + > > > >>> + *size = sig_addr - cert_addr; /* Subtract out the signature size */ > > > >>> + cert_size = *size; > > > >>> + > > > >>> + /* Check if image load address is 32-bit aligned */ > > > >>> + if (0 != (0x3 & cert_addr)) { > > > >> > > > >> if (!IS_ALIGNED(cert_addr, 4)) { ? > > > >> > > > >>> + printf("Image is not 4-byte aligned.\n"); > > > >>> + result = 1; > > > >>> + goto auth_exit; > > > >>> + } > > > >>> + > > > >>> + /* Image size also should be multiple of 4 */ > > > >>> + if (0 != (0x3 & cert_size)) { > > > >> > > > >> if (!IS_ALIGNED(cert_size, 4)) { ? > > > >> > > > >>> + printf("Image size is not 4-byte aligned.\n"); > > > >>> + result = 1; > > > >>> + goto auth_exit; > > > >>> + } > > > >>> + > > > >>> + /* Call ROM HAL API to verify certificate signature */ > > > >>> + debug("%s: load_addr = %x, size = %x, sig_addr = %x\n", __func__, > > > >>> + cert_addr, cert_size, sig_addr); > > > >>> + > > > >>> + result = secure_rom_call( > > > >>> + API_HAL_KM_VERIFYCERTIFICATESIGNATURE_INDEX, 0, 0, > > > >>> + 4, cert_addr, cert_size, sig_addr, 0xFFFFFFFF); > > > >>> +auth_exit: > > > >>> + if (result != 0) { > > > >>> + printf("Authentication failed!\n"); > > > >>> + printf("Return Value = %08X\n", result); > > > >>> + hang(); > > > >>> + } > > > >>> + > > > >>> + printf("Authentication passed: %s\n", (char *)sig_addr); > > > >> > > > >> Uart boot will break because of these prints during the FIT loading. Can > > > >> you make this as debug? > > > > > > > > Are you sure it will break? There's usually a print in between loading > > > > SPL via UART and then U-Boot itself via UART and Y-MODEM is smart enough > > > > to re-transmit. > > > > > > > > > > Yes, if the print is in between while Y-MODEM is transferring. The above > > > print falls in this case. > > ... but Y-MODEM (the protocol) does retransmit. It should recover from > this message. > > > Tom et al., > > so if this really breaks stuff I need to do something about it. As said > > I'd really like to keep the "Authentication passed: <certificate name>" > > message in the boot log. So if I implement something along the lines > > what Lokesh suggested: > > > > "...you can check if (spl_boot_device() != BOOT_DEVICE_UART) under the > > config CONFIG_SPL_YMODEM_SUPPORT. Not sure if it is a good way to do..." > > > > to selectivly suppress the message in case of UART boot, would this be > > acceptable? Or is there a better way? > > At worst case, yes, we can case this around !CONFIG_SPL_YMODEM_SUPPORT. > But I keep thinking the world should recover from this too. ...hmmm, but it's so ugly :) Well I'm going to spend some time to play with it. Thanks for all your feedback. Andreas _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
diff --git a/arch/arm/cpu/armv7/omap-common/sec-common.c b/arch/arm/cpu/armv7/omap-common/sec-common.c index b9c0a42..dbb9078 100644 --- a/arch/arm/cpu/armv7/omap-common/sec-common.c +++ b/arch/arm/cpu/armv7/omap-common/sec-common.c @@ -16,6 +16,9 @@ #include <asm/arch/sys_proto.h> #include <asm/omap_common.h> +/* Index for signature verify ROM API */ +#define API_HAL_KM_VERIFYCERTIFICATESIGNATURE_INDEX (0x0000000E) + static uint32_t secure_rom_call_args[5] __aligned(ARCH_DMA_MINALIGN); u32 secure_rom_call(u32 service, u32 proc_id, u32 flag, ...) @@ -47,3 +50,76 @@ u32 secure_rom_call(u32 service, u32 proc_id, u32 flag, ...) return omap_smc_sec(service, proc_id, flag, secure_rom_call_args); } + +static u32 find_sig_start(char *image, size_t size) +{ + char *image_end = image + size; + char *sig_start_magic = "CERT_"; + int magic_str_len = strlen(sig_start_magic); + char *ch; + + while (--image_end > image) { + if (*image_end == '_') { + ch = image_end - magic_str_len + 1; + if (!strncmp(ch, sig_start_magic, magic_str_len)) + return (u32)ch; + } + } + return 0; +} + +int secure_boot_verify_image(void **image, size_t *size) +{ + int result = 1; + u32 cert_addr, sig_addr; + size_t cert_size; + + /* Perform cache writeback on input buffer */ + flush_dcache_range( + (u32)*image, + (u32)*image + roundup(*size, ARCH_DMA_MINALIGN)); + + cert_addr = (uint32_t)*image; + sig_addr = find_sig_start((char *)*image, *size); + + if (sig_addr == 0) { + printf("No signature found in image.\n"); + result = 1; + goto auth_exit; + } + + *size = sig_addr - cert_addr; /* Subtract out the signature size */ + cert_size = *size; + + /* Check if image load address is 32-bit aligned */ + if (0 != (0x3 & cert_addr)) { + printf("Image is not 4-byte aligned.\n"); + result = 1; + goto auth_exit; + } + + /* Image size also should be multiple of 4 */ + if (0 != (0x3 & cert_size)) { + printf("Image size is not 4-byte aligned.\n"); + result = 1; + goto auth_exit; + } + + /* Call ROM HAL API to verify certificate signature */ + debug("%s: load_addr = %x, size = %x, sig_addr = %x\n", __func__, + cert_addr, cert_size, sig_addr); + + result = secure_rom_call( + API_HAL_KM_VERIFYCERTIFICATESIGNATURE_INDEX, 0, 0, + 4, cert_addr, cert_size, sig_addr, 0xFFFFFFFF); +auth_exit: + if (result != 0) { + printf("Authentication failed!\n"); + printf("Return Value = %08X\n", result); + hang(); + } + + printf("Authentication passed: %s\n", (char *)sig_addr); + + return result; +} diff --git a/arch/arm/include/asm/omap_common.h b/arch/arm/include/asm/omap_common.h index 293fc72..8887335 100644 --- a/arch/arm/include/asm/omap_common.h +++ b/arch/arm/include/asm/omap_common.h @@ -640,6 +640,15 @@ u32 omap_smc_sec(u32 service, u32 proc_id, u32 flag, u32 *params); */ u32 secure_rom_call(u32 service, u32 proc_id, u32 flag, ...); +/* + * Invoke a secure ROM API on high-secure (HS) device variants that can be used + * to verify a secure blob by authenticating and optionally decrypting it. The + * exact operation performed depends on how the certificate that was embedded + * into the blob during the signing/encryption step when the secure blob was + * first created. + */ +int secure_boot_verify_image(void **p_image, size_t *p_size); + void enable_edma3_clocks(void); void disable_edma3_clocks(void);