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-all...@ti.com>
> > > >>> Signed-off-by: Andreas Dannenberg <dannenb...@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

Reply via email to