[U-Boot] [PATCH 3/5] x86: baytrail: secureboot: Add functions for verification of u-boot

Simon Glass sjg at chromium.org
Mon May 15 03:03:34 UTC 2017


Hi,

On 11 May 2017 at 09:14, Anatolij Gustschin <agust at denx.de> wrote:
> From: Markus Valentin <mv at denx.de>
>
> Introduce functions that check the integrity of u-boot by utilising the
> hashes stored in the oem-data block.

U-Boot

>
> The verification functions get called in fsp_init()
>
> Signed-off-by: Markus Valentin <mv at denx.de>
> ---
>  arch/x86/cpu/baytrail/Makefile                     |   1 +
>  arch/x86/cpu/baytrail/secure_boot.c                | 117 +++++++++++++++++++++
>  .../include/asm/arch-baytrail/fsp/fsp_configs.h    |   3 +
>  arch/x86/lib/fsp/fsp_support.c                     |  15 +++
>  4 files changed, 136 insertions(+)
>  create mode 100644 arch/x86/cpu/baytrail/secure_boot.c
>
> diff --git a/arch/x86/cpu/baytrail/Makefile b/arch/x86/cpu/baytrail/Makefile
> index a0216f3..dbf9a82 100644
> --- a/arch/x86/cpu/baytrail/Makefile
> +++ b/arch/x86/cpu/baytrail/Makefile
> @@ -8,4 +8,5 @@ obj-y += cpu.o
>  obj-y += early_uart.o
>  obj-y += fsp_configs.o
>  obj-y += valleyview.o
> +obj-$(CONFIG_BAYTRAIL_SECURE_BOOT) += secure_boot.o
>  obj-$(CONFIG_GENERATE_ACPI_TABLE) += acpi.o
> diff --git a/arch/x86/cpu/baytrail/secure_boot.c b/arch/x86/cpu/baytrail/secure_boot.c
> new file mode 100644
> index 0000000..37c83db
> --- /dev/null
> +++ b/arch/x86/cpu/baytrail/secure_boot.c
> @@ -0,0 +1,117 @@
> +/*
> + * Copyright (C) 2017 Markus Valentin <mv at denx.de>
> + *
> + * SPDX-License-Identifier:     GPL-2.0+
> + */
> +
> +#include <common.h>
> +
> +#define SB_MANIFEST_BASE               0xFFFE0000
> +#define SB_MANIFEST_SIZE               0x400
> +#define SB_MANIFEST_OEM_DATA_OFFSET    0x58
> +#define SB_MANIFEST_OEM_HASH_OFFSET    (SB_MANIFEST_OEM_DATA_OFFSET + 4)
> +#define SB_MANIFEST_OEM_HASH_BASE      (SB_MANIFEST_BASE +\
> +                                        SB_MANIFEST_OEM_HASH_OFFSET)
> +#define SB_MANIFEST_END                        (SB_MANIFEST_BASE + SB_MANIFEST_SIZE)
> +
> +#define PUB_KEY_MODULUS_SIZE           0x100
> +#define U_BOOT_STAGE_SIZE              0xDD360
> +#define U_BOOT_OFFSET                  0x2CA0
> +
> +#define U_BOOT_STAGE_START             (CONFIG_SYS_TEXT_BASE + U_BOOT_OFFSET)
> +#define U_BOOT_STAGE_END               (U_BOOT_STAGE_START + U_BOOT_STAGE_SIZE)
> +
> +#define SHA256_U_BOOT_STAGE_ID         0
> +#define SHA256_FSP_STAGE2_ID           1
> +#define SHA256_FIT_PUB_KEY_ID          2
> +
> +#define FIT_KEY_NAME                   "dev"
> +
> +/**
> + * This function compares a hash which gets retrieved from the oem data block

I think the function style we have settled on is:

/**
 * verify_oem_sha256() - one line summary
 *
 * More explanation here
 *
 * @hashid: ...
 * ...
 */

> + * with the runtime calculated hash of start_address+size. If they match,
> + * this function returns true. If not, it returns false.
> + *
> + * @param hash_id      offset of oem-data block for hash to compare
> + * @param start_address        address where the hash calculation should start
> + * @param size         length of the region for hash calculation
> + * @return true on success, false on error
> + */
> +static bool verify_oem_sha256(unsigned int hash_id,
> +                             void *start_address,
> +                             size_t size)
> +{
> +       uint8_t value[SHA256_SUM_LEN];
> +       int value_len;
> +
> +       /* calculate address of hash to compare in the oemdata block*/
> +       void *hash_to_verify = (void *)SB_MANIFEST_OEM_HASH_BASE +
> +                              (SHA256_SUM_LEN * hash_id);
> +#ifdef DEBUG
> +       unsigned int i = 0;
> +       uint8_t oem_value[SHA256_SUM_LEN];
> +
> +       memcpy(oem_value, hash_to_verify, SHA256_SUM_LEN);
> +       printf("SB: Hash to verify:\t");
> +       for (i = 0; i < SHA256_SUM_LEN; i++)
> +               printf("%X", oem_value[i]);
> +       printf("\n");
> +#endif
> +
> +       /* caluclate the hash of the binary */
> +       calculate_hash(start_address, size, "sha256", (unsigned char *)value,
> +                      &value_len);
> +
> +#ifdef DEBUG
> +       printf("SB: calculated hash:\t");
> +       for (i = 0; i < SHA256_SUM_LEN; i++)
> +               printf("%X", value[i]);
> +       printf("\n");
> +#endif
> +       /* compare the two hash  values */
> +       if (memcmp(hash_to_verify, value, SHA256_SUM_LEN))
> +               return false;
> +       return true;
> +}
> +
> +/**
> + * This function verifies the integrity for u-boot, its devicetree and the ucode
> + * appended or inserted to the devicetree.
> + *
> + * @return true on success, false on error
> + */

Can you put this comment in the header file?

> +bool verify_u_boot_bin(void)
> +{
> +       return verify_oem_sha256(SHA256_U_BOOT_STAGE_ID,
> +                                (void *)U_BOOT_STAGE_START,
> +                                U_BOOT_STAGE_SIZE);
> +}
> +
> +/**
> + * This function verifies the integrity for the modulus of the public key which
> + * is stored in the u-boot devicetree for fit image verification. It tries to
> + * find the "rsa,modulus" property in the dtb and then verifies it with the
> + * checksum stored in the oem-data block
> + *
> + * @return true on success, false on error
> + */
> +bool verify_public_key(void)
> +{
> +       void *fit_public_key_modulus;
> +
> +       int offset = fdt_node_offset_by_prop_value(gd->fdt_blob, -1,
> +                                                  "key-name-hint",
> +                                                  FIT_KEY_NAME,
> +                                                  4);
> +
> +       fit_public_key_modulus =  (void *)fdt_getprop(gd->fdt_blob, offset,
> +                                                     "rsa,modulus", NULL);
> +       if (!fit_public_key_modulus) {
> +               debug("SB: Could not fetch public key from U-Boot Devicetree\n");
> +               return false;
> +       }
> +
> +       return verify_oem_sha256(SHA256_FIT_PUB_KEY_ID,
> +                                fit_public_key_modulus,
> +                                PUB_KEY_MODULUS_SIZE);
> +}
> diff --git a/arch/x86/include/asm/arch-baytrail/fsp/fsp_configs.h b/arch/x86/include/asm/arch-baytrail/fsp/fsp_configs.h
> index e539890..b5dd5a4 100644
> --- a/arch/x86/include/asm/arch-baytrail/fsp/fsp_configs.h
> +++ b/arch/x86/include/asm/arch-baytrail/fsp/fsp_configs.h
> @@ -16,4 +16,7 @@ struct fspinit_rtbuf {
>         struct common_buf       common; /* FSP common runtime data structure */
>  };
>
> +bool verify_u_boot_bin(void);
> +bool verify_public_key(void);

These nee comments. Also how about an fsp_ prefix since they are in
the fsp file?

> +
>  #endif /* __FSP_CONFIGS_H__ */
> diff --git a/arch/x86/lib/fsp/fsp_support.c b/arch/x86/lib/fsp/fsp_support.c
> index 3a537d0..5669700 100644
> --- a/arch/x86/lib/fsp/fsp_support.c
> +++ b/arch/x86/lib/fsp/fsp_support.c
> @@ -149,6 +149,21 @@ void fsp_init(u32 stack_top, u32 boot_mode, void *nvs_buf)
>          */
>         printf("FSP: Secure Boot %sabled\n",
>                fsp_vpd->enable_secure_boot == 1 ? "en" : "dis");
> +       if (!verify_u_boot_bin()) {
> +               /* if our u-boot binary checksum isn't equal to

/*
 * If our ...

> +                * our expected checksum we need to stop booting
> +                */
> +               puts("SB: Failed to verify u-boot and dtb\n");
> +               hang();
> +       }
> +
> +       /*
> +        * verification of the public key happens with verification of
> +        * the devicetree binary (thats where its stored), this check is
> +        * not necessary, but nice to see its integer
> +        */
> +       if (!verify_public_key())
> +               puts("SB: Failed to verify public key for fit-image\n");
>  #endif
>         /* Copy default data from Flash */
>         memcpy(fsp_upd, (void *)(fsp_hdr->img_base + fsp_vpd->upd_offset),
> --
> 2.7.4
>

Regards,
Simon


More information about the U-Boot mailing list