x86: baytrail: secureboot: Add functions for verification of U-Boot

From: Markus Valentin <[email protected]>

Introduce functions that check the integrity of U-Boot by utilising the hashes stored in the OEM-data block in Secure Boot Manifest.

The verification functions get called in fsp_init()

Signed-off-by Markus Valentin <[email protected]>

Signed-off-by Anatolij Gustschin <[email protected]>

---
Changes in v3:
- lower case in hex numbers
- fix RAM stage payload hash calculation and add comments for associated macros
- add comments explaining used stage indexes, s/*_ID/*_IDX
- fixed two spaces in comment
- s/devicetree/device tree
- extend the output messages to give more hints when FIT key verification fails
-

Changes in v2:
- use 'const void *' for fdt property ptr, drop cast
- s/u-boot/U-Boot/
- fix function comment style and move comments to header with prototypes. Use fsp_ prefix
- fix multiline comment style
- s/SB:/Secure Boot/ for non-debug messages

 arch/x86/cpu/baytrail/Makefile                     |   1 +
 arch/x86/cpu/baytrail/secure_boot.c                | 117 +++++++++++++++++++++
 .../include/asm/arch-baytrail/fsp/fsp_configs.h    |  24 +++++
 arch/x86/lib/fsp/fsp_support.c                     |  18 ++++
 4 files changed, 160 insertions(+)
 create mode 100644 arch/x86/cpu/baytrail/secure_boot.c

arch/x86/cpu/baytrail/Makefile +1 -0

index a0216f3059..dbf9a82c39 100644
@@ -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

arch/x86/cpu/baytrail/secure_boot.c new file +0 -0

new file mode 100644
index 0000000000..eaf35c6e24

arch/x86/cpu/baytrail/secure_boot.c new file +22 -0

+#define PUB_KEY_MODULUS_SIZE           0x100
+
+/*
+ * U-Boot in RAM stage payload from reset vector to FSP Stage2:
+ * 0xfff0000 to 0xfffc0000
+ */
+#define U_BOOT_STAGE_START             CONFIG_SYS_TEXT_BASE
+#define U_BOOT_STAGE_SIZE              0xc0000
+
+/*
+ * Indexes of 32-byte blocks in the OEM-data area containing sha256 hashes
+ * of RAM stage payload, FSP Stage2 and other OEM specific data. The order
+ * of RAM stage payload and FSP Stage2 blocks is fixed (idx 0 and 1),
+ * do not change it. We do not verify FSP Stage2 here, it will be verified
+ * in FSP (FSP Stage2 idx 1 is not used). So, the first index of OEM specific
+ * data block hash must start from 2.
+ */
+#define SHA256_U_BOOT_STAGE_IDX                0
+#define SHA256_FIT_PUB_KEY_IDX         2
+
+/**
+ * verify_oem_sha256() - oem data block verification

arch/x86/cpu/baytrail/secure_boot.c new file +25 -0

+ */
+static bool verify_oem_sha256(unsigned int hash_idx,
+                             const 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_idx);
+#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("%02X", oem_value[i]);
+       printf("\n");
+#endif
+
+       /* Calculate the hash of the binary */
+       calculate_hash(start_address, size, "sha256", value, &value_len);
+

arch/x86/cpu/baytrail/secure_boot.c new file +33 -0

+}
+
+bool fsp_verify_u_boot_bin(void)
+{
+       return verify_oem_sha256(SHA256_U_BOOT_STAGE_IDX,
+                                (const void *)U_BOOT_STAGE_START,
+                                U_BOOT_STAGE_SIZE);
+}
+
+bool fsp_verify_public_key(void)
+{
+       const void *fit_public_key_modulus;
+       int offset = fdt_node_offset_by_prop_value(gd->fdt_blob, -1,
+                                                  "key-name-hint",
+                                                  FIT_KEY_NAME,
+                                                  4);
+       int ret;
+
+       fit_public_key_modulus =  fdt_getprop(gd->fdt_blob, offset,
+                                             "rsa,modulus", NULL);
+       if (!fit_public_key_modulus) {
+               puts("No FIT public key in U-Boot device tree\n");
+               return false;
+       }
+
+       ret = verify_oem_sha256(SHA256_FIT_PUB_KEY_IDX,
+                               fit_public_key_modulus,
+                               PUB_KEY_MODULUS_SIZE);
+       if (!ret)
+               puts("FIT public key checksum wrong\n");\
+
+       return ret;
+}

arch/x86/include/asm/arch-baytrail/fsp/fsp_configs.h +24 -0

index 1c6c2479f0..2c8c32b9bf 100644
@@ -103,4 +103,28 @@ struct fspinit_rtbuf {
 #define SCC_MODE_ACPI                  0
 #define SCC_MODE_PCI                   1
 
+#ifndef __ASSEMBLY__
+/**
+ * fsp_verify_u_boot_bin() - U-Boot binary verification
+ *
+ * This function verifies the integrity for U-Boot, its device tree and the
+ * ucode appended or inserted to the device tree.
+ *
+ * @retval:    true on success, false on error
+ */
+bool fsp_verify_u_boot_bin(void);
+
+/**
+ * fsp_verify_public_key() - integrity of public key for fit image verification
+ *
+ * This function verifies the integrity for the modulus of the public key which
+ * is stored in the U-Boot device tree 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
+ *
+ * @retval:    true on success, false on error
+ */
+bool fsp_verify_public_key(void);
+#endif
+
 #endif /* __FSP_CONFIGS_H__ */

arch/x86/lib/fsp/fsp_support.c +4 -0

index d79a6e900a..c7e0a9eff4 100644
@@ -152,6 +152,24 @@ 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 (!fsp_verify_u_boot_bin()) {
+               /*
+                * If our U-Boot binary checksum isn't equal to

arch/x86/lib/fsp/fsp_support.c +4 -1

+        */
+       if (!fsp_verify_public_key())
+               printf("%s Failed to verify public key for FIT image\n",
+                      SB_PRFX);
        #endif
 
        /* Copy default data from Flash */
--

2.11.0

Hi Anatolij,

Quoted 29 lines · Anatolij Gustschin

From: Markus Valentin <[email protected]>

Introduce functions that check the integrity of U-Boot by utilising the hashes stored in the OEM-data block in Secure Boot Manifest.

The verification functions get called in fsp_init()

Signed-off-by Markus Valentin <[email protected]>

Signed-off-by Anatolij Gustschin <[email protected]>

---
Changes in v3:
- lower case in hex numbers
- fix RAM stage payload hash calculation and add comments for associated macros
- add comments explaining used stage indexes, s/*_ID/*_IDX
- fixed two spaces in comment
- s/devicetree/device tree
- extend the output messages to give more hints when FIT key verification fails
-

Changes in v2:
- use 'const void *' for fdt property ptr, drop cast
- s/u-boot/U-Boot/
- fix function comment style and move comments to header with prototypes. Use fsp_ prefix
- fix multiline comment style
- s/SB:/Secure Boot/ for non-debug messages

Quoted 7 lines

 arch/x86/cpu/baytrail/Makefile                     |   1 +
 arch/x86/cpu/baytrail/secure_boot.c                | 117 +++++++++++++++++++++
 .../include/asm/arch-baytrail/fsp/fsp_configs.h    |  24 +++++
 arch/x86/lib/fsp/fsp_support.c                     |  18 ++++
 4 files changed, 160 insertions(+)
 create mode 100644 arch/x86/cpu/baytrail/secure_boot.c

Reviewed-by Simon Glass <[email protected]>

Some nits below.

Quoted 20 lines

arch/x86/cpu/baytrail/Makefile +1 -0

index a0216f3059..dbf9a82c39 100644
@@ -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

arch/x86/cpu/baytrail/secure_boot.c new file +4 -0

new file mode 100644
index 0000000000..eaf35c6e24
@@ -0,0 +1,117 @@
+/*
+ * Copyright (C) 2017 Markus Valentin <[email protected]>
+ *
+ * SPDX-License-Identifier:     GPL-2.0+

arch/x86/cpu/baytrail/secure_boot.c line 9

Quoted 5 lines

arch/x86/cpu/baytrail/secure_boot.c new file +5 -0

+ */
+
+#include <common.h>
+
+#define SB_MANIFEST_BASE               0xfffe0000  

Is this something inside a binary blob, or can it be changed by the user? I'm just wondering if it should go in Kconfig.

Quoted 5 lines

arch/x86/cpu/baytrail/secure_boot.c new file +5 -0

+#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)

arch/x86/cpu/baytrail/secure_boot.c line 17

Quoted 3 lines

arch/x86/cpu/baytrail/secure_boot.c new file +3 -0

+#define SB_MANIFEST_END                        (SB_MANIFEST_BASE + SB_MANIFEST_SIZE)
+
+#define FIT_KEY_NAME                   "dev"  

This is just supposed to be a hint. I think you should probably check all keys, since you don't really know which one was used to sign. You could check this one first I suppose. But any signature that works should be good enough.

arch/x86/cpu/baytrail/secure_boot.c line 49

Quoted 12 lines

arch/x86/cpu/baytrail/secure_boot.c new file +12 -0

+/**
+ * verify_oem_sha256() - oem data block verification
+ *
+ * This function compares a hash which gets retrieved from the oem data block
+ * with the runtime calculated hash of start_address+size. If they match,
+ * this function returns true. If not, it returns false.
+ *
+ * @hash_idx:          index of oem-data block with hash to compare
+ * @start_address:     address where the hash calculation should start
+ * @size:              length of the region for hash calculation
+ *
+ * @retval:            true on success, false on error  

@returns

Quoted 7 lines

arch/x86/cpu/baytrail/secure_boot.c new file +7 -0

+       /* Calculate the hash of the binary */
+       calculate_hash(start_address, size, "sha256", value, &value_len);
+
+#ifdef DEBUG
+       printf("SB: calculated hash:\t");
+       for (i = 0; i < SHA256_SUM_LEN; i++)
+               printf("%02X", value[i]);

arch/x86/cpu/baytrail/secure_boot.c line 84

Quoted 6 lines

arch/x86/cpu/baytrail/secure_boot.c new file +6 -0

+       printf("\n");
+#endif
+       /* Compare the two hash values */
+       if (memcmp(hash_to_verify, value, SHA256_SUM_LEN))
+               return false;
+       return true;  

I suggest returning 0 on success and an negative error on failure.

Quoted 17 lines

arch/x86/lib/fsp/fsp_support.c +9 -0

index d79a6e900a..c7e0a9eff4 100644
@@ -152,6 +152,24 @@ 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 (!fsp_verify_u_boot_bin()) {
+               /*
+                * If our U-Boot binary checksum isn't equal to
+                * our expected checksum we need to stop booting
+                */
+               printf("%s Failed to verify U-Boot and dtb\n", SB_PRFX);
+               hang();
+       }

arch/x86/lib/fsp/fsp_support.c void fsp_init(u32 stack_top, u32 boot_mode, void *nvs_buf) · line 168

Quoted 5 lines

arch/x86/lib/fsp/fsp_support.c +5 -0

+
+       /*
+        * Verification of the public key happens with verification of
+        * the device tree binary (that's where it's stored), this check
+        * is not necessary, but nice to see it's integer  

What does "it's integer" mean? Do you mean 'its'? Can you reword to explain what integer you mean?