[U-Boot] [PATCH v4 1/2] powerpc/mpc85xx: SECURE BOOT- Enable chain of trust in SPL

Simon Glass sjg at chromium.org
Fri Jun 10 02:35:00 CEST 2016


Hi,

On 9 June 2016 at 09:05, Andreas Dannenberg <dannenberg at ti.com> wrote:
> Hi Sumit,
>
> On Thu, Jun 02, 2016 at 08:16:37AM -0400, Sumit Garg wrote:
>> As part of Chain of Trust for Secure boot, the SPL U-Boot will validate
>> the next level U-boot image. Add a new function spl_validate_uboot to
>> perform the validation.
>
> I noticed your patch series just now and I certainly don't want to
> derail what you are trying to do here but I wanted to see what you think
> about something that's pretty much related.
>
> In order to be more flexible in how we go from SPL to U-Boot (we have a
> need to inject ROM-API calls for decryption in addition to
> authentication) I've been experimenting with a method that basically
> injects a generic post-process function call into spl_load_simple_fit()
> that the platform-specific layer can provide/override. This function
> will basically get invoked on the U-Boot firmware itself as well as for
> the selected DTB after it gets loaded, allowing to do things like custom
> authentication and/or decryption through plugging in for example our ROM
> API calls. But this should also be a starting point for someone to plug
> in the U-Boot RSA libraries directly in a way that doesn't require using
> CONFIG_SPL_DM (for memory-constrained devices).
>
> So mostly I wanted to see if this is something that could be useful for
> you as well, and any comments/concerns you may have on that.
>
> Thanks and Regards,
>
> --
> Andreas Dannenberg
> Texas Instruments Inc
>
>
>> Enable hardware crypto operations in SPL using SEC block.
>> In case of Secure Boot, PAMU is not bypassed. For allowing SEC block
>> access to CPC configured as SRAM, configure PAMU.
>>
>> Reviewed-by: Ruchika Gupta <ruchika.gupta at nxp.com>
>> Signed-off-by: Aneesh Bansal <aneesh.bansal at nxp.com>
>> Signed-off-by: Sumit Garg <sumit.garg at nxp.com>
>> ---
>> Changes in v2:
>> Patches rebased
>>
>> Changes in v3:
>> Patches rebased
>>
>> Changes in v4:
>> Generic changes in lib, drivers, common Makefiles removed from
>> this patchset. Rebased this patchset on top of patch [1], so this
>> patchset is dependent on patch [1].
>>
>> [1]https://patchwork.ozlabs.org/patch/627664/
>>
>>  arch/powerpc/cpu/mpc8xxx/fsl_pamu.c         |  8 +++++
>>  arch/powerpc/cpu/mpc8xxx/pamu_table.c       |  8 +++++
>>  arch/powerpc/include/asm/fsl_secure_boot.h  | 28 ++++++++++++++++
>>  board/freescale/common/fsl_chain_of_trust.c | 50 +++++++++++++++++++++++++++++
>>  drivers/crypto/fsl/jr.c                     | 16 +++++++++
>>  drivers/mtd/nand/fsl_ifc_spl.c              | 24 ++++++++++++++
>>  include/fsl_validate.h                      |  1 +
>>  7 files changed, 135 insertions(+)
>>
>> diff --git a/arch/powerpc/cpu/mpc8xxx/fsl_pamu.c b/arch/powerpc/cpu/mpc8xxx/fsl_pamu.c
>> index 9421f1e..ede8e66 100644
>> --- a/arch/powerpc/cpu/mpc8xxx/fsl_pamu.c
>> +++ b/arch/powerpc/cpu/mpc8xxx/fsl_pamu.c
>> @@ -239,15 +239,23 @@ int pamu_init(void)
>>       spaact_size = sizeof(struct paace) * NUM_SPAACT_ENTRIES;
>>
>>       /* Allocate space for Primary PAACT Table */
>> +#if (defined(CONFIG_SPL_BUILD) && defined(CONFIG_SPL_PPAACT_ADDR))
>> +     ppaact = (void *)CONFIG_SPL_PPAACT_ADDR;
>> +#else
>>       ppaact = memalign(PAMU_TABLE_ALIGNMENT, ppaact_size);
>>       if (!ppaact)
>>               return -1;
>> +#endif
>>       memset(ppaact, 0, ppaact_size);
>>
>>       /* Allocate space for Secondary PAACT Table */
>> +#if (defined(CONFIG_SPL_BUILD) && defined(CONFIG_SPL_SPAACT_ADDR))
>> +     sec = (void *)CONFIG_SPL_SPAACT_ADDR;
>> +#else
>>       sec = memalign(PAMU_TABLE_ALIGNMENT, spaact_size);
>>       if (!sec)
>>               return -1;
>> +#endif
>>       memset(sec, 0, spaact_size);
>>
>>       ppaact_phys = virt_to_phys((void *)ppaact);
>> diff --git a/arch/powerpc/cpu/mpc8xxx/pamu_table.c b/arch/powerpc/cpu/mpc8xxx/pamu_table.c
>> index 26c5ea4..a8e6f51 100644
>> --- a/arch/powerpc/cpu/mpc8xxx/pamu_table.c
>> +++ b/arch/powerpc/cpu/mpc8xxx/pamu_table.c
>> @@ -28,6 +28,14 @@ void construct_pamu_addr_table(struct pamu_addr_tbl *tbl, int *num_entries)
>>
>>       i++;
>>  #endif
>> +#if (defined(CONFIG_SPL_BUILD) && (CONFIG_SYS_INIT_L3_VADDR))
>> +     tbl->start_addr[i] =
>> +             (uint64_t)virt_to_phys((void *)CONFIG_SYS_INIT_L3_VADDR);
>> +     tbl->size[i] = 256 * 1024; /* 256K CPC flash */
>> +     tbl->end_addr[i] = tbl->start_addr[i] +  tbl->size[i] - 1;
>> +
>> +     i++;
>> +#endif
>>       debug("PAMU address\t\t\tsize\n");
>>       for (j = 0; j < i ; j++)
>>               debug("%llx \t\t\t%llx\n",  tbl->start_addr[j],  tbl->size[j]);
>> diff --git a/arch/powerpc/include/asm/fsl_secure_boot.h b/arch/powerpc/include/asm/fsl_secure_boot.h
>> index 826f9c9..99eec7f 100644
>> --- a/arch/powerpc/include/asm/fsl_secure_boot.h
>> +++ b/arch/powerpc/include/asm/fsl_secure_boot.h
>> @@ -72,6 +72,32 @@
>>
>>  #ifdef CONFIG_CHAIN_OF_TRUST
>>
>> +#ifdef CONFIG_SPL_BUILD
>> +#define CONFIG_SPL_DM                        1
>> +#define CONFIG_SPL_CRYPTO_SUPPORT
>> +#define CONFIG_SPL_HASH_SUPPORT
>> +#define CONFIG_SPL_RSA
>> +#define CONFIG_SPL_DRIVERS_MISC_SUPPORT
>> +/*
>> + * PPAACT and SPAACT table for PAMU must be placed on DDR after DDR init
>> + * due to space crunch on CPC and thus malloc will not work.
>> + */
>> +#define CONFIG_SPL_PPAACT_ADDR               0x2e000000
>> +#define CONFIG_SPL_SPAACT_ADDR               0x2f000000
>> +#define CONFIG_SPL_JR0_LIODN_S               454
>> +#define CONFIG_SPL_JR0_LIODN_NS              458
>> +/*
>> + * Define the key hash for U-Boot here if public/private key pair used to
>> + * sign U-boot are different from the SRK hash put in the fuse
>> + * Example of defining KEY_HASH is
>> + * #define CONFIG_SPL_UBOOT_KEY_HASH \
>> + *      "41066b564c6ffcef40ccbc1e0a5d0d519604000c785d97bbefd25e4d288d1c8b"
>> + * else leave it defined as NULL
>> + */
>> +
>> +#define CONFIG_SPL_UBOOT_KEY_HASH    NULL
>> +#endif /* ifdef CONFIG_SPL_BUILD */
>> +
>>  #define CONFIG_CMD_ESBC_VALIDATE
>>  #define CONFIG_CMD_BLOB
>>  #define CONFIG_FSL_SEC_MON
>> @@ -82,6 +108,7 @@
>>  #define CONFIG_FSL_CAAM
>>  #endif
>>
>> +#ifndef CONFIG_SPL_BUILD
>>  /* fsl_setenv_chain_of_trust() must be called from
>>   * board_late_init()
>>   */
>> @@ -119,5 +146,6 @@
>>  #endif /* #ifdef CONFIG_BOOTSCRIPT_COPY_RAM */
>>
>>  #include <config_fsl_chain_trust.h>
>> +#endif /* #ifndef CONFIG_SPL_BUILD */
>>  #endif /* #ifdef CONFIG_CHAIN_OF_TRUST */
>>  #endif
>> diff --git a/board/freescale/common/fsl_chain_of_trust.c b/board/freescale/common/fsl_chain_of_trust.c
>> index ecfcc82..992babf 100644
>> --- a/board/freescale/common/fsl_chain_of_trust.c
>> +++ b/board/freescale/common/fsl_chain_of_trust.c
>> @@ -6,7 +6,17 @@
>>
>>  #include <common.h>
>>  #include <fsl_validate.h>
>> +#include <fsl_secboot_err.h>
>>  #include <fsl_sfp.h>
>> +#include <dm/root.h>
>> +
>> +#ifdef CONFIG_ADDR_MAP
>> +#include <asm/mmu.h>
>> +#endif
>> +
>> +#ifdef CONFIG_FSL_CORENET
>> +#include <asm/fsl_pamu.h>
>> +#endif
>>
>>  #ifdef CONFIG_LS102XA
>>  #include <asm/arch/immap_ls102xa.h>
>> @@ -52,6 +62,7 @@ int fsl_check_boot_mode_secure(void)
>>       return 0;
>>  }
>>
>> +#ifndef CONFIG_SPL_BUILD
>>  int fsl_setenv_chain_of_trust(void)
>>  {
>>       /* Check Boot Mode
>> @@ -68,3 +79,42 @@ int fsl_setenv_chain_of_trust(void)
>>       setenv("bootcmd", CONFIG_CHAIN_BOOT_CMD);
>>       return 0;
>>  }
>> +#endif
>> +
>> +#ifdef CONFIG_SPL_BUILD
>> +void spl_validate_uboot(uint32_t hdr_addr, uintptr_t img_addr)
>> +{
>> +     int res;
>> +
>> +     /* Check Boot Mode
>> +      * If Boot Mode is Non-Secure, skip validation
>> +      */
>> +     if (fsl_check_boot_mode_secure() == 0)
>> +             return;
>> +
>> +     printf("SPL: Validating U-Boot image\n");
>> +
>> +#ifdef CONFIG_ADDR_MAP
>> +     init_addr_map();
>> +#endif
>> +
>> +#ifdef CONFIG_FSL_CORENET
>> +     if (pamu_init() < 0)
>> +             fsl_secboot_handle_error(ERROR_ESBC_PAMU_INIT);
>> +#endif
>> +
>> +#ifdef CONFIG_FSL_CAAM
>> +     if (sec_init() < 0)
>> +             fsl_secboot_handle_error(ERROR_ESBC_SEC_INIT);
>> +#endif
>> +
>> +#if defined(CONFIG_DM)
>> +     dm_init_and_scan(false);

Eek, this is duplicating code and adding a board-specific hack. I
agree that it would be better to use SPL FIT to handle this.

>> +#endif
>> +     res = fsl_secboot_validate(hdr_addr, CONFIG_SPL_UBOOT_KEY_HASH,
>> +                                &img_addr);
>> +
>> +     if (res == 0)
>> +             printf("SPL: Validation of U-boot successful\n");
>> +}
>> +#endif
>> diff --git a/drivers/crypto/fsl/jr.c b/drivers/crypto/fsl/jr.c
>> index 510fa4e..1d4dd32 100644
>> --- a/drivers/crypto/fsl/jr.c
>> +++ b/drivers/crypto/fsl/jr.c
>> @@ -599,10 +599,26 @@ int sec_init_idx(uint8_t sec_idx)
>>       sec_out32(&sec->mcfgr, mcr);
>>
>>  #ifdef CONFIG_FSL_CORENET
>> +#ifdef CONFIG_SPL_BUILD
>> +     /* For SPL Build, Set the Liodns in SEC JR0 for
>> +      * creating PAMU entries corresponding to these.
>> +      * For normal build, these are set in set_liodns().
>> +      */
>> +     liodn_ns = CONFIG_SPL_JR0_LIODN_NS & JRNSLIODN_MASK;
>> +     liodn_s = CONFIG_SPL_JR0_LIODN_S & JRSLIODN_MASK;
>> +
>> +     liodnr = sec_in32(&sec->jrliodnr[0].ls) &
>> +              ~(JRNSLIODN_MASK | JRSLIODN_MASK);
>> +     liodnr = liodnr |
>> +              (liodn_ns << JRNSLIODN_SHIFT) |
>> +              (liodn_s << JRSLIODN_SHIFT);
>> +     sec_out32(&sec->jrliodnr[0].ls, liodnr);
>> +#else
>>       liodnr = sec_in32(&sec->jrliodnr[0].ls);
>>       liodn_ns = (liodnr & JRNSLIODN_MASK) >> JRNSLIODN_SHIFT;
>>       liodn_s = (liodnr & JRSLIODN_MASK) >> JRSLIODN_SHIFT;
>>  #endif
>> +#endif
>>
>>       ret = jr_init(sec_idx);
>>       if (ret < 0) {
>> diff --git a/drivers/mtd/nand/fsl_ifc_spl.c b/drivers/mtd/nand/fsl_ifc_spl.c
>> index cbeb74a..30aa966 100644
>> --- a/drivers/mtd/nand/fsl_ifc_spl.c
>> +++ b/drivers/mtd/nand/fsl_ifc_spl.c
>> @@ -11,6 +11,9 @@
>>  #include <asm/io.h>
>>  #include <fsl_ifc.h>
>>  #include <linux/mtd/nand.h>
>> +#ifdef CONFIG_CHAIN_OF_TRUST
>> +#include <fsl_validate.h>
>> +#endif
>>
>>  static inline int is_blank(uchar *addr, int page_size)
>>  {
>> @@ -268,6 +271,27 @@ void nand_boot(void)
>>        */
>>       flush_cache(CONFIG_SYS_NAND_U_BOOT_DST, CONFIG_SYS_NAND_U_BOOT_SIZE);
>>  #endif
>> +
>> +#ifdef CONFIG_CHAIN_OF_TRUST
>> +     /*
>> +      * As U-Boot header is appended at end of U-boot image, so
>> +      * calculate U-boot header address using U-boot header size.
>> +      */
>> +#define CONFIG_U_BOOT_HDR_ADDR \
>> +             ((CONFIG_SYS_NAND_U_BOOT_START + \
>> +               CONFIG_SYS_NAND_U_BOOT_SIZE) - \
>> +              CONFIG_U_BOOT_HDR_SIZE)
>> +     spl_validate_uboot(CONFIG_U_BOOT_HDR_ADDR,
>> +                        CONFIG_SYS_NAND_U_BOOT_START);
>> +     /*
>> +      * In case of failure in validation, spl_validate_uboot would
>> +      * not return back in case of Production environment with ITS=1.
>> +      * Thus U-Boot will not start.
>> +      * In Development environment (ITS=0 and SB_EN=1), the function
>> +      * may return back in case of non-fatal failures.
>> +      */
>> +#endif
>> +
>>       uboot = (void *)CONFIG_SYS_NAND_U_BOOT_START;
>>       uboot();
>>  }
>> diff --git a/include/fsl_validate.h b/include/fsl_validate.h
>> index a71e1ce..7695b30 100644
>> --- a/include/fsl_validate.h
>> +++ b/include/fsl_validate.h
>> @@ -254,4 +254,5 @@ int fsl_secboot_blob_decap(cmd_tbl_t *cmdtp, int flag, int argc,
>>
>>  int fsl_check_boot_mode_secure(void);
>>  int fsl_setenv_chain_of_trust(void);
>> +void spl_validate_uboot(uint32_t hdr_addr, uintptr_t img_addr);
>>  #endif
>> --
>> 1.8.1.4
>>

Regards,
Simon


More information about the U-Boot mailing list