[U-Boot] [PATCH 0/5] Introduce secure boot for Baytrail

[U-Boot] [PATCH 0/5] Introduce secure boot for Baytrail

[Anatolij Gustschin]

From: Markus Valentin <mv@denx.de>

This patch series makes the hardware mechanisms for verified boot on
baytrail based platforms usable in/for U-Boot. The series contains
a tool which allows to easily create and assemble a secure boot
manifest. The manifest gets utilized by the Trusted Execution Engine
on the Soc.

Markus Valentin (5):
  x86: congatec: add secureboot enabled defconfig for
    conga-qeval20-qa3-e3845
  x86: baytrail: Add fsp-header verification for secure boot fsp
  x86: baytrail: secureboot: Add functions for verification of u-boot
  tools: add secure_boot_helper.py
  doc: x86: Add section about secure boot on Baytrail

 arch/x86/Kconfig                                   |  13 +-
 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/include/asm/fsp/fsp_support.h             |   2 +
 arch/x86/lib/fsp/fsp_support.c                     |  31 ++
 ...0-qa3-e3845-internal-uart-secure-boot_defconfig |  77 +++++
 doc/README.x86                                     |  41 +++
 tools/secure_boot_helper.py                        | 313 +++++++++++++++++++++
 9 files changed, 597 insertions(+), 1 deletion(-)
 create mode 100644 arch/x86/cpu/baytrail/secure_boot.c
 create mode 100644 configs/conga-qeval20-qa3-e3845-internal-uart-secure-boot_defconfig
 create mode 100644 tools/secure_boot_helper.py

-- 
2.7.4

[U-Boot] [PATCH 1/5] x86: congatec: add secureboot enabled defconfig for conga-qeval20-qa3-e3845

[Anatolij Gustschin]

From: Markus Valentin <mv@denx.de>

Signed-off-by: Markus Valentin <mv@denx.de>
[agust: rebased, fixed to build with v2017.05]
Signed-off-by: Anatolij Gustschin <agust@denx.de>
---
 ...0-qa3-e3845-internal-uart-secure-boot_defconfig | 77 ++++++++++++++++++++++
 1 file changed, 77 insertions(+)
 create mode 100644 configs/conga-qeval20-qa3-e3845-internal-uart-secure-boot_defconfig

diff --git a/configs/conga-qeval20-qa3-e3845-internal-uart-secure-boot_defconfig b/configs/conga-qeval20-qa3-e3845-internal-uart-secure-boot_defconfig
new file mode 100644
index 0000000..b78f90e
--- /dev/null
+++ b/configs/conga-qeval20-qa3-e3845-internal-uart-secure-boot_defconfig
@@ -0,0 +1,77 @@
+CONFIG_X86=y
+CONFIG_VENDOR_CONGATEC=y
+CONFIG_TARGET_CONGA_QEVAL20_QA3_E3845=y
+CONFIG_DEFAULT_DEVICE_TREE="conga-qeval20-qa3-e3845"
+CONFIG_INTERNAL_UART=y
+CONFIG_HAVE_INTEL_ME=y
+CONFIG_BAYTRAIL_SECURE_BOOT=y
+CONFIG_ENABLE_MRC_CACHE=y
+CONFIG_SMP=y
+CONFIG_HAVE_VGA_BIOS=y
+CONFIG_VGA_BIOS_ADDR=0xfffa0000
+CONFIG_GENERATE_PIRQ_TABLE=y
+CONFIG_GENERATE_MP_TABLE=y
+CONFIG_GENERATE_ACPI_TABLE=y
+CONFIG_SEABIOS=y
+CONFIG_FIT=y
+CONFIG_FIT_SIGNATURE=y
+CONFIG_BOOTSTAGE=y
+CONFIG_BOOTSTAGE_REPORT=y
+CONFIG_SYS_CONSOLE_INFO_QUIET=y
+CONFIG_HUSH_PARSER=y
+CONFIG_CMD_CPU=y
+# CONFIG_CMD_IMLS is not set
+# CONFIG_CMD_FLASH is not set
+CONFIG_CMD_MMC=y
+CONFIG_CMD_SF=y
+CONFIG_CMD_SPI=y
+CONFIG_CMD_I2C=y
+CONFIG_CMD_USB=y
+CONFIG_CMD_GPIO=y
+# CONFIG_CMD_SETEXPR is not set
+CONFIG_CMD_DHCP=y
+# CONFIG_CMD_NFS is not set
+CONFIG_CMD_PING=y
+CONFIG_CMD_TIME=y
+CONFIG_CMD_BOOTSTAGE=y
+CONFIG_CMD_EXT2=y
+CONFIG_CMD_EXT4=y
+CONFIG_CMD_EXT4_WRITE=y
+CONFIG_CMD_FAT=y
+CONFIG_CMD_FS_GENERIC=y
+CONFIG_OF_CONTROL=y
+CONFIG_REGMAP=y
+CONFIG_SYSCON=y
+CONFIG_CPU=y
+CONFIG_DM_I2C=y
+CONFIG_SYS_I2C_INTEL=y
+CONFIG_WINBOND_W83627=y
+CONFIG_MMC=y
+CONFIG_MMC_PCI=y
+CONFIG_MMC_SDHCI=y
+CONFIG_MMC_SDHCI_SDMA=y
+CONFIG_SPI_FLASH=y
+CONFIG_SPI_FLASH_GIGADEVICE=y
+CONFIG_SPI_FLASH_MACRONIX=y
+CONFIG_SPI_FLASH_STMICRO=y
+CONFIG_SPI_FLASH_WINBOND=y
+CONFIG_DM_ETH=y
+CONFIG_E1000=y
+CONFIG_DM_PCI=y
+CONFIG_DM_RTC=y
+CONFIG_DEBUG_UART=y
+CONFIG_DEBUG_UART_BASE=0x3f8
+CONFIG_DEBUG_UART_CLOCK=1843200
+CONFIG_SYS_NS16550=y
+CONFIG_ICH_SPI=y
+CONFIG_TIMER=y
+CONFIG_USB=y
+CONFIG_DM_USB=y
+CONFIG_USB_STORAGE=y
+CONFIG_USB_KEYBOARD=y
+CONFIG_DM_VIDEO=y
+CONFIG_VIDEO_VESA=y
+CONFIG_FRAMEBUFFER_SET_VESA_MODE=y
+CONFIG_FRAMEBUFFER_VESA_MODE_114=y
+CONFIG_CONSOLE_SCROLL_LINES=5
+CONFIG_USE_PRIVATE_LIBGCC=y
-- 
2.7.4

[U-Boot] [PATCH 2/5] x86: baytrail: Add fsp-header verification for secure boot fsp

[Anatolij Gustschin]

From: Markus Valentin <mv@denx.de>

Introduce a new Kconfig variable for secure boot on baytrail based
platforms. If this variable is set the build process tries to use
fsp-sb.bin instead of fsp.bin (-sb is the secure boot enabled fsp).

Also check the two fsp headers against each other and print if secure
boot is enabled or not.

Signed-off-by: Markus Valentin <mv@denx.de>
---
 arch/x86/Kconfig                       | 13 ++++++++++++-
 arch/x86/include/asm/fsp/fsp_support.h |  2 ++
 arch/x86/lib/fsp/fsp_support.c         | 16 ++++++++++++++++
 3 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 9ead3eb..8cea393 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -348,7 +348,8 @@ config HAVE_FSP
 config FSP_FILE
 	string "Firmware Support Package binary filename"
 	depends on HAVE_FSP
-	default "fsp.bin"
+	default "fsp.bin" if !BAYTRAIL_SECURE_BOOT
+	default "fsp-sb.bin" if BAYTRAIL_SECURE_BOOT
 	help
 	  The filename of the file to use as Firmware Support Package binary
 	  in the board directory.
@@ -400,6 +401,16 @@ config FSP_BROKEN_HOB
 	  do not overwrite the important boot service data which is used by
 	  FSP, otherwise the subsequent call to fsp_notify() will fail.
 
+config BAYTRAIL_SECURE_BOOT
+	bool "Enable Secure Boot on BayTrail"
+	depends on HAVE_FSP
+	default n
+	help
+	  Use the SecureBoot Features of the BayTrail platform. This switch
+	  enables the usage of the secure-boot enabled fsp.bin(fsp-sb.bin)
+	  for your board you need to provide this yourself. You can reconfigure
+	  your fsp with the Intel BCT tool to enable SecureBoot.
+
 config ENABLE_MRC_CACHE
 	bool "Enable MRC cache"
 	depends on !EFI && !SYS_COREBOOT
diff --git a/arch/x86/include/asm/fsp/fsp_support.h b/arch/x86/include/asm/fsp/fsp_support.h
index 61d811f..bae17bc 100644
--- a/arch/x86/include/asm/fsp/fsp_support.h
+++ b/arch/x86/include/asm/fsp/fsp_support.h
@@ -21,6 +21,8 @@
 #define FSP_LOWMEM_BASE		0x100000UL
 #define FSP_HIGHMEM_BASE	0x100000000ULL
 #define UPD_TERMINATOR		0x55AA
+#define FSP_FIRST_HEADER_OFFSET		0x94
+#define FSP_SECOND_HEADER_OFFSET	0x20494
 
 
 /**
diff --git a/arch/x86/lib/fsp/fsp_support.c b/arch/x86/lib/fsp/fsp_support.c
index a480361..3a537d0 100644
--- a/arch/x86/lib/fsp/fsp_support.c
+++ b/arch/x86/lib/fsp/fsp_support.c
@@ -119,6 +119,13 @@ void fsp_init(u32 stack_top, u32 boot_mode, void *nvs_buf)
 		/* No valid FSP info header was found */
 		panic("Invalid FSP header");
 	}
+#ifdef CONFIG_BAYTRAIL_SECURE_BOOT
+	/* compare primary and secondary header */
+	if (memcmp((void *)(CONFIG_FSP_ADDR + FSP_FIRST_HEADER_OFFSET),
+		   (void *)(CONFIG_FSP_ADDR + FSP_SECOND_HEADER_OFFSET),
+		   fsp_hdr->hdr_len))
+		panic("SB: first & secondary FSP headers don't match");
+#endif
 
 	config_data.common.fsp_hdr = fsp_hdr;
 	config_data.common.stack_top = stack_top;
@@ -134,6 +141,15 @@ void fsp_init(u32 stack_top, u32 boot_mode, void *nvs_buf)
 
 	fsp_upd = &config_data.fsp_upd;
 
+#ifdef CONFIG_BAYTRAIL_SECURE_BOOT
+	/*
+	 * if the enable secure boot flag is not 1, secure boot has not
+	 * been activated in the FSP which results in the TXE-Engine not
+	 * getting loaded
+	 */
+	printf("FSP: Secure Boot %sabled\n",
+	       fsp_vpd->enable_secure_boot == 1 ? "en" : "dis");
+#endif
 	/* Copy default data from Flash */
 	memcpy(fsp_upd, (void *)(fsp_hdr->img_base + fsp_vpd->upd_offset),
 	       sizeof(struct upd_region));
-- 
2.7.4

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

[Anatolij Gustschin]

From: Markus Valentin <mv@denx.de>

Introduce functions that check the integrity of u-boot by utilising the
hashes stored in the oem-data block.

The verification functions get called in fsp_init()

Signed-off-by: Markus Valentin <mv@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@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
+ * 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
+ */
+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);
+
 #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
+		 * 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

[U-Boot] [PATCH 4/5] tools: add secure_boot_helper.py

[Anatolij Gustschin]

From: Markus Valentin <mv@denx.de>

This script should be used for simple creation of secure bootable
images for baytrail platforms

Signed-off-by: Markus Valentin <mv@denx.de>
---
 tools/secure_boot_helper.py | 313 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 313 insertions(+)
 create mode 100644 tools/secure_boot_helper.py

diff --git a/tools/secure_boot_helper.py b/tools/secure_boot_helper.py
new file mode 100644
index 0000000..884786e
--- /dev/null
+++ b/tools/secure_boot_helper.py
@@ -0,0 +1,313 @@
+#!/usr/bin/env python3
+"""
+ Copyright (C) 2017 Markus Valentin <mv@denx.de>
+
+ SPDX-License-Identifier:     GPL-2.0+
+ """
+
+
+import argparse
+import binascii
+
+from hashlib import sha256
+from os.path import basename, isfile, splitext
+from os.path import join as pjoin
+from struct import pack
+
+import OpenSSL
+from OpenSSL import crypto
+
+from cryptography import x509
+from cryptography.hazmat.backends import default_backend
+
+
+FSP_FILE_NAME = "fsp-sb.bin"
+FSP_STAGE2_FILE_NAME = "fsp_stage2.bin"
+U_BOOT_ROM_FILE_NAME = 'u-boot.rom'
+U_BOOT_TO_SIGN_FILE_NAME = 'u-boot-to-sign.bin'
+IBB_FILE_NAME = 'ibb.bin'
+FPF_CONFIG_FILE_NAME = 'fpf_config.txt'
+SIGNED_MANIFEST_FILE_NAME = 'signed_manifest.bin'
+UNSIGNED_MANIFEST_FILE_NAME = 'un'+SIGNED_MANIFEST_FILE_NAME
+OEM_FILE_NAME = 'oemdata.bin'
+
+OEM_PRIV_KEY_FILE_NAME = 'oemkey.pem'
+OEM_PUB_KEY_FILE_NAME = 'pub_oemkey.pem'
+OEM_PUBKEY_BIN_FILE_NAME = 'pub_oemkey.bin'
+OEM_PUBKEY_AND_SIG_FILE_NAME = 'oem_pub_sig.bin'
+
+FIT_PUB_KEY_FILE_NAME = "dev.crt"
+
+FSP_STAGE_2_SIZE = 0x1f400
+IBB_SIZE = 0x1fc00
+MANIFEST_SIZE = 0x400
+OEM_BLOCK_MAX_SIZE = 0x190
+U_BOOT_ROM_SIZE = 0x800000
+ROMFILE_SYS_TEXT_BASE = 0x00700000
+U_BOOT_TO_SIGN_OFFSET = 0x2CA0
+
+
+MANIFEST_IDENTIFIER = b'$VBM'
+VERSION = 1
+SECURE_VERSION_NUMBER = 2
+OEM_DATA_PREAMBLE = '01000200'
+
+oem_data_hash_files = []
+
+
+def append_binary_files(first_file, second_file, new_file):
+    with open(new_file, 'wb') as f:
+        f.write(bytearray(open(first_file, 'rb').read()))
+        f.write(bytearray(open(second_file, 'rb').read()))
+
+
+# this function creates a oemdata data block which gets constructed
+# as stated in section 3.2 of the document "Secure-Boot for Intel
+# Bay Trail based platfomrs with U-Boot"
+def assemble_oem_data(file_path):
+    file_size = 0
+    with open(file_path, 'wb') as f:
+        f.write(binascii.unhexlify(OEM_DATA_PREAMBLE))
+        file_size += 4
+        for hash_file in oem_data_hash_files:
+            f.write(open(hash_file, 'rb').read())
+            file_size += 32
+        pad_file_with_zeros(f, OEM_BLOCK_MAX_SIZE-file_size)
+
+
+# this function creates the final u-boot-verified.rom from
+# the original u-boot.rom and the signed Initial Boot Block
+# which contains the secure boot manifest
+def assemble_secure_boot_image(u_boot_rom, signed_ibb):
+    data = bytearray(open(u_boot_rom, 'rb').read())
+    ibb = bytearray(open(signed_ibb, 'rb').read())
+    data[-(MANIFEST_SIZE+IBB_SIZE):] = ibb
+    open("u-boot-verified.rom", 'wb').write(data)
+
+
+# when calling this function it constructs a complete secure-boot manifest
+# which is just missing oem-publickey and the manifest-signature (see
+# section 3.1)
+def create_unsigned_secure_boot_manifest(unsigned_manifest,
+                                         oem_file='oemdata.bin',
+                                         ibb='ibb.bin'):
+    with open(unsigned_manifest, 'wb') as f:
+        f.write(MANIFEST_IDENTIFIER)
+        f.write(pack('i', VERSION))
+        f.write(pack('i', MANIFEST_SIZE))
+        f.write(pack('i', SECURE_VERSION_NUMBER))
+        pad_file_with_zeros(f, 4)
+        hash_function = sha256()
+        hash_function.update(bytearray(open(ibb, 'rb').read()))
+        f.write(hash_function.digest()[::-1])
+        pad_file_with_zeros(f, 36)
+        f.write(bytearray(open(oem_file, 'rb').read()))
+        pad_file_with_zeros(f, 20)
+
+
+# fetch a subpart of a binary from byte to byte and write this part to a
+# secondary file
+def extract_binary_part(binary_to_extract_from, to_file, from_byte, to_byte):
+    data = open(binary_to_extract_from, 'rb').read()
+    open(to_file, 'wb').write(data[from_byte:to_byte])
+
+
+# calculate a sha256 checksum over a file and write a file with it to a
+# file next to the original file, if given change endianness (sometimes needed
+# because the txe engine wants a other byteorder)
+def sha256_to_file(binary_dir, file_to_hash, change_endianess=False):
+    # we collect the hashes in a list(in the correct order) to be able
+    # to put them later to the oem section
+    if not oem_data_hash_files.__contains__(hashfile_path(binary_dir,
+                                            file_to_hash)):
+        oem_data_hash_files.append(hashfile_path(binary_dir, file_to_hash))
+    with open(file_to_hash, 'rb') as f:
+        hash_function = sha256()
+        hash_function.update(f.read())
+        # write as little to file
+        if change_endianess:
+            open(hashfile_path(binary_dir, file_to_hash),
+                 'wb').write(hash_function.digest())
+        else:
+            open(hashfile_path(binary_dir, file_to_hash),
+                 'wb').write(hash_function.digest()[::-1])
+
+
+# create hashfile name using the file-to-hash name
+def hashfile_path(binary_dir, file_to_hash):
+    hash_file_name = splitext(
+                basename(file_to_hash))[0].__add__('.sha256')
+    return pjoin(binary_dir, hash_file_name)
+
+
+# pad the given files with a given byte number of zeros
+# byte count must be dividable by 4
+def pad_file_with_zeros(file_handle, byte_count):
+    if byte_count % 4 != 0:
+        print("Given  byte count is not 4-divideable exiting")
+        exit()
+    pad_count = 0
+    while pad_count < byte_count:
+        file_handle.write(pack('i', 0))
+        pad_count += 4
+
+
+# extract the modulus of a public key the txe-engine gets the publickey
+# split in modulus and exponent (for this reason we need to extract it)
+def get_modulus_from_pubkey(public_key_path):
+    public_key = open(public_key_path, 'rb').read()
+    cert = x509.load_pem_x509_certificate(public_key, default_backend())
+    return ("%X" % (cert.public_key().public_numbers().n))
+
+
+# save a given modulus and exponent to a file as binary for use within
+# the manifest
+def save_binary_public_key(pub_key_file_path, modulus, exponent=0x10001):
+    with open(pub_key_file_path, 'wb') as f:
+        f.write(binascii.unhexlify(modulus)[::-1])
+        f.write(pack('i', exponent))
+
+
+# replace the public key hash in the fuse configuration text file
+# and set the lock bit
+def replace_oem_pubkey_hash(pubkey_hash, fpf_config_path, lock_fuses):
+    data = binascii.hexlify(pubkey_hash)
+
+    new_line_hash = "FUSE_FILE_OEM_KEY_HASH_1:{:s}:{}\n"\
+                    .format(data.upper().decode('ascii'),
+                            str(lock_fuses).upper())
+    new_line_sb_enabled = "FUSE_FILE_SECURE_BOOT_EN:01:{}\n"\
+                          .format(str(lock_fuses).upper())
+
+    with open(fpf_config_path, 'w') as f:
+        f.write(new_line_sb_enabled)
+        f.write(new_line_hash)
+
+
+# for the txe engine one needs to change the endianness
+def reverse_endianess(file_to_reverse):
+    data = open(file_to_reverse, 'rb').read()
+    open(file_to_reverse, 'wb').write(data[::-1])
+
+
+# sign the given file with the given private key and
+# write it to the signature_file using openssl
+def sign_file(unsigned_file, private_key, signature_file):
+    key = open(private_key, 'r').read()
+    pkey_obj = crypto.load_privatekey(crypto.FILETYPE_PEM, key)
+    data = open(unsigned_file, 'rb').read()
+    signature = OpenSSL.crypto.sign(pkey_obj, data, "sha256")
+    open(signature_file, 'wb').write(signature)
+
+
+if __name__ == "__main__":
+    parser = argparse.ArgumentParser(description="This script assembles a " +
+                                     "verified boot enabled u-boot using " +
+                                     "openssl")
+    parser.add_argument("-c", "--fpf-config", default="./fpf_config.txt",
+                        help="Path to the fpf-config file defaults to" +
+                        " ./fpf_config.txt",
+                        required=True)
+    parser.add_argument("-I", "--board-dir", help="set directory to get the " +
+                        " fsp and other board related files from.")
+    parser.add_argument("-b", "--binary_dir", default=".",
+                        help="directory to fetch binaries from and " +
+                        "save created binaries to.")
+    parser.add_argument("-k", "--key-dir", default="./mykeys",
+                        help="directory to fetch keys from")
+    parser.add_argument('--lock-fuses', action='store_true',
+                        help="Set this flag to configure fuses to " +
+                        "lock the values")
+
+    args = parser.parse_args()
+
+    # assemble correct paths
+    fsp = pjoin(args.board_dir, FSP_FILE_NAME)
+    fsp_stage2 = pjoin(args.board_dir, FSP_STAGE2_FILE_NAME)
+
+    fit_public_key = pjoin(args.key_dir, FIT_PUB_KEY_FILE_NAME)
+    fit_public_key_modulus = pjoin(args.key_dir, FIT_PUB_KEY_FILE_NAME+".mod")
+
+    u_boot_rom = pjoin(args.binary_dir, U_BOOT_ROM_FILE_NAME)
+    u_boot_to_sign = pjoin(args.binary_dir, U_BOOT_TO_SIGN_FILE_NAME)
+
+    ibb = pjoin(args.binary_dir, IBB_FILE_NAME)
+
+    signed_ibb = pjoin(args.binary_dir, "signed_"+IBB_FILE_NAME)
+    unsigned_manifest = pjoin(args.binary_dir, UNSIGNED_MANIFEST_FILE_NAME)
+    signed_manifest = pjoin(args.binary_dir, SIGNED_MANIFEST_FILE_NAME)
+
+    manifest_signature = pjoin(args.binary_dir, splitext(
+                               basename(UNSIGNED_MANIFEST_FILE_NAME))[0].
+                               __add__(".signature"))
+
+    oem_file = pjoin(args.binary_dir, OEM_FILE_NAME)
+    oem_private_key = pjoin(args.key_dir, OEM_PRIV_KEY_FILE_NAME)
+    oem_public_key = pjoin(args.key_dir, OEM_PUB_KEY_FILE_NAME)
+    oem_pubkey_binary = pjoin(args.key_dir, OEM_PUBKEY_BIN_FILE_NAME)
+    oem_pubkey_and_sig = pjoin(args.key_dir,
+                               OEM_PUBKEY_AND_SIG_FILE_NAME)
+
+    # check for needed files to be available
+    for f in [fsp, u_boot_rom, fit_public_key, oem_private_key]:
+        if not isfile(f):
+            print("%s not found ... exiting" % (f))
+            exit()
+
+    # get everything from rom-file execept IBB+Manfifest(128k) and write it to
+    # seperated file and calculate hash
+    extract_binary_part(u_boot_rom, u_boot_to_sign,
+                        (ROMFILE_SYS_TEXT_BASE+U_BOOT_TO_SIGN_OFFSET),
+                        (U_BOOT_ROM_SIZE-(IBB_SIZE+MANIFEST_SIZE)))
+    sha256_to_file(args.binary_dir, u_boot_to_sign, True)
+
+    # extract stage2 of the fsp and calculate a hash about
+    # the file
+    extract_binary_part(fsp, fsp_stage2, 0, FSP_STAGE_2_SIZE)
+    sha256_to_file(args.binary_dir, fsp_stage2)
+
+    with open(fit_public_key_modulus, 'wb') as f:
+        f.write(binascii.unhexlify(get_modulus_from_pubkey(fit_public_key)))
+    sha256_to_file(args.binary_dir, fit_public_key_modulus, True)
+
+    # assemble oemdata
+    print("Assembling oem data from %d hashes: \n %s" %
+          (oem_data_hash_files.__len__(), oem_data_hash_files))
+    assemble_oem_data(oem_file)
+
+    print("Extracting last 127K:\n from %s as %s"
+          % (u_boot_rom, ibb))
+    extract_binary_part(u_boot_rom, ibb,
+                        (U_BOOT_ROM_SIZE-IBB_SIZE), U_BOOT_ROM_SIZE)
+
+    print("Creating Secure Boot Manifest")
+    create_unsigned_secure_boot_manifest(unsigned_manifest,
+                                         oem_file,
+                                         ibb)
+
+    print("Signing manifest with openssl and private key %s"
+          % (oem_private_key))
+    sign_file(unsigned_manifest, oem_private_key, manifest_signature)
+
+    print("Append public key and signature to unsigned Manifest")
+    oem_pub_key_modulus = get_modulus_from_pubkey(oem_public_key)
+    save_binary_public_key(oem_pubkey_binary, oem_pub_key_modulus)
+
+    reverse_endianess(manifest_signature)
+    append_binary_files(oem_pubkey_binary, manifest_signature,
+                        oem_pubkey_and_sig)
+
+    append_binary_files(unsigned_manifest, oem_pubkey_and_sig,
+                        signed_manifest)
+
+    hash_function = sha256()
+    hash_function.update(bytearray(open(oem_pubkey_binary, 'rb').read()))
+    replace_oem_pubkey_hash(hash_function.digest()[::-1], args.fpf_config,
+                            args.lock_fuses)
+
+    print("Append manifest with signature to ibb")
+    append_binary_files(signed_manifest, ibb, signed_ibb)
+
+    print("Assemble u-boot-verified.rom from:\n %s and %s"
+          % (u_boot_rom, signed_manifest))
+    assemble_secure_boot_image(u_boot_rom, signed_ibb)
-- 
2.7.4

[U-Boot] [PATCH 5/5] doc: x86: Add section about secure boot on Baytrail

[Anatolij Gustschin]

From: Markus Valentin <mv@denx.de>

Signed-off-by: Markus Valentin <mv@denx.de>
[agust: slightly reworded and fixed alignment]
Signed-off-by: Anatolij Gustschin <agust@denx.de>
---
 doc/README.x86 | 41 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/doc/README.x86 b/doc/README.x86
index a38cc1b..8ba64b3 100644
--- a/doc/README.x86
+++ b/doc/README.x86
@@ -1056,6 +1056,45 @@ provides the same EFI run-time services) is not currently supported on x86.
 
 See README.efi for details of EFI support in U-Boot.
 
+Secure Boot for BayTrail
+------------------------
+U-Boot for BayTrail based platforms supports to boot in a verified manner using
+the Trusted Execution Enginge(TXE). To enable secure boot you need to enable
+ the Kconfig parameter CONFIG_BAYTRAIL_SECURE_BOOT.
+
+The verification of U-Boot happens by a public key appended to the so called
+secure boot manifest. The manifest gets created by the secure_boot_helper.py
+script which is located in the tools directory.
+
+To be able to perform a verified boot with U-Boot you need:
+ * A OEM-keypair which we use to sign U-Boot. Create this yourself like below:
+	mkdir mykeys && \
+	openssl req -batch -x509 -nodes -newkey rsa:2048 \
+		 -keyout 'mykeys/oemkey.pem' -out 'mykeys/pub_oemkey.pem'
+ * fpf_config.txt gets created by the helper script. It stores the fuse
+	register configuration to a text file which can be used by the Intel
+	FPT tool to write fuses (the FPT is provided in the TXE Firmware Kit).
+	It contains a hash over the public part of the OEM-keypair.
+	(To burn fuses run "FPT -writebatch fpf_config.txt" on the target)
+ * A secure boot enabled FSP[18] which we can assemble with the BCT Tool[19]
+	(the secure boot enabled fsp should be placed as fsp-sb.bin in the
+	board directory)
+
+If these prerequisites are met, you can build u-boot and call the helper script.
+The following commands give an example flow for the congatec conga-QA3 SoM:
+	make conga-qeval20-qa3-e3845-internal-uart-secure-boot_defconfig
+	make all
+	make u-boot.rom
+	python3 ./tools/secure_boot_helper.py \
+		-I board/congatec/conga-qeval20-qa3-e3845 \
+		-c fpf_config.txt \
+		--lock-fuses
+
+This creates a "u-boot-verified.rom", this file can be used as the normal
+u-boot.rom. For enabling the verification you need to configure the fuses
+either by burning them or by using the FPF-Mirroring feature for development.
+Further authentication can be done with the fit-image mechanism.
+
 64-bit Support
 --------------
 U-Boot supports booting a 64-bit kernel directly and is able to change to
@@ -1098,3 +1137,5 @@ References
 [15] doc/device-tree-bindings/misc/intel,irq-router.txt
 [16] http://www.acpi.info
 [17] https://www.acpica.org/downloads
+[18] https://github.com/IntelFsp/FSP.git
+[19] https://github.com/IntelFsp/BCT.git
-- 
2.7.4

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

[Lothar Waßmann]

Hi,

On Thu, 11 May 2017 17:14:54 +0200 Anatolij Gustschin wrote:
> From: Markus Valentin <mv@denx.de>
> 
> Introduce functions that check the integrity of u-boot by utilising the
> hashes stored in the oem-data block.
> 
> The verification functions get called in fsp_init()
> 
> Signed-off-by: Markus Valentin <mv@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@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
> + * 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];
>
'unsigned char' here ...

> +	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);
>
... would avoid the '(unsigned char *)' cast here.

> +
> +#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
> + */
> +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;
>
'const void *' here ...
> +
> +	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);
>
... would eliminate the need for the '(void *)' cast here.

> +	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);
> +}


Lothar Waßmann

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

[Anatolij Gustschin]

Hi,

On Fri, 12 May 2017 10:25:50 +0200
Lothar Waßmann LW at KARO-electronics.de wrote:
...
> > +static bool verify_oem_sha256(unsigned int hash_id,
> > +			      void *start_address,
> > +			      size_t size)
> > +{
> > +	uint8_t value[SHA256_SUM_LEN];
> >  
> 'unsigned char' here ...
> 
> > +	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);
> >  
> ... would avoid the '(unsigned char *)' cast here.

I'll drop this cast, the fourth argument of calculate_hash()
is uint8_t *.

...
> > +bool verify_public_key(void)
> > +{
> > +	void *fit_public_key_modulus;
> >  
> 'const void *' here ...
> > +
> > +	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);
> >  
> ... would eliminate the need for the '(void *)' cast here.

OK, will fix. Thanks!

--
Anatolij

[U-Boot] [PATCH 1/5] x86: congatec: add secureboot enabled defconfig for conga-qeval20-qa3-e3845

[Simon Glass]

On 11 May 2017 at 09:14, Anatolij Gustschin <agust@denx.de> wrote:
> From: Markus Valentin <mv@denx.de>
>
> Signed-off-by: Markus Valentin <mv@denx.de>
> [agust: rebased, fixed to build with v2017.05]
> Signed-off-by: Anatolij Gustschin <agust@denx.de>
> ---
>  ...0-qa3-e3845-internal-uart-secure-boot_defconfig | 77 ++++++++++++++++++++++
>  1 file changed, 77 insertions(+)
>  create mode 100644 configs/conga-qeval20-qa3-e3845-internal-uart-secure-boot_defconfig

Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH 2/5] x86: baytrail: Add fsp-header verification for secure boot fsp

[Simon Glass]

On 11 May 2017 at 09:14, Anatolij Gustschin <agust@denx.de> wrote:
> From: Markus Valentin <mv@denx.de>
>
> Introduce a new Kconfig variable for secure boot on baytrail based
> platforms. If this variable is set the build process tries to use
> fsp-sb.bin instead of fsp.bin (-sb is the secure boot enabled fsp).
>
> Also check the two fsp headers against each other and print if secure
> boot is enabled or not.
>
> Signed-off-by: Markus Valentin <mv@denx.de>
> ---
>  arch/x86/Kconfig                       | 13 ++++++++++++-
>  arch/x86/include/asm/fsp/fsp_support.h |  2 ++
>  arch/x86/lib/fsp/fsp_support.c         | 16 ++++++++++++++++
>  3 files changed, 30 insertions(+), 1 deletion(-)
>

Reviewed-by: Simon Glass <sjg@chromium.org>

But please see below

> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 9ead3eb..8cea393 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -348,7 +348,8 @@ config HAVE_FSP
>  config FSP_FILE
>         string "Firmware Support Package binary filename"
>         depends on HAVE_FSP
> -       default "fsp.bin"
> +       default "fsp.bin" if !BAYTRAIL_SECURE_BOOT
> +       default "fsp-sb.bin" if BAYTRAIL_SECURE_BOOT
>         help
>           The filename of the file to use as Firmware Support Package binary
>           in the board directory.
> @@ -400,6 +401,16 @@ config FSP_BROKEN_HOB
>           do not overwrite the important boot service data which is used by
>           FSP, otherwise the subsequent call to fsp_notify() will fail.
>
> +config BAYTRAIL_SECURE_BOOT
> +       bool "Enable Secure Boot on BayTrail"
> +       depends on HAVE_FSP
> +       default n
> +       help
> +         Use the SecureBoot Features of the BayTrail platform. This switch
> +         enables the usage of the secure-boot enabled fsp.bin(fsp-sb.bin)
> +         for your board you need to provide this yourself. You can reconfigure
> +         your fsp with the Intel BCT tool to enable SecureBoot.
> +
>  config ENABLE_MRC_CACHE
>         bool "Enable MRC cache"
>         depends on !EFI && !SYS_COREBOOT
> diff --git a/arch/x86/include/asm/fsp/fsp_support.h b/arch/x86/include/asm/fsp/fsp_support.h
> index 61d811f..bae17bc 100644
> --- a/arch/x86/include/asm/fsp/fsp_support.h
> +++ b/arch/x86/include/asm/fsp/fsp_support.h
> @@ -21,6 +21,8 @@
>  #define FSP_LOWMEM_BASE                0x100000UL
>  #define FSP_HIGHMEM_BASE       0x100000000ULL
>  #define UPD_TERMINATOR         0x55AA
> +#define FSP_FIRST_HEADER_OFFSET                0x94
> +#define FSP_SECOND_HEADER_OFFSET       0x20494
>
>
>  /**
> diff --git a/arch/x86/lib/fsp/fsp_support.c b/arch/x86/lib/fsp/fsp_support.c
> index a480361..3a537d0 100644
> --- a/arch/x86/lib/fsp/fsp_support.c
> +++ b/arch/x86/lib/fsp/fsp_support.c
> @@ -119,6 +119,13 @@ void fsp_init(u32 stack_top, u32 boot_mode, void *nvs_buf)
>                 /* No valid FSP info header was found */
>                 panic("Invalid FSP header");
>         }
> +#ifdef CONFIG_BAYTRAIL_SECURE_BOOT

Can you use if (IS_ENABLED(CONFIG_BAYTRAIL_SECURE_BOOT) instead of
#ifdef? It reduces the number of build paths.

> +       /* compare primary and secondary header */
> +       if (memcmp((void *)(CONFIG_FSP_ADDR + FSP_FIRST_HEADER_OFFSET),
> +                  (void *)(CONFIG_FSP_ADDR + FSP_SECOND_HEADER_OFFSET),
> +                  fsp_hdr->hdr_len))
> +               panic("SB: first & secondary FSP headers don't match");

How about s/SB/Secure Boot/?

> +#endif
>
>         config_data.common.fsp_hdr = fsp_hdr;
>         config_data.common.stack_top = stack_top;
> @@ -134,6 +141,15 @@ void fsp_init(u32 stack_top, u32 boot_mode, void *nvs_buf)
>
>         fsp_upd = &config_data.fsp_upd;
>
> +#ifdef CONFIG_BAYTRAIL_SECURE_BOOT
> +       /*
> +        * if the enable secure boot flag is not 1, secure boot has not
> +        * been activated in the FSP which results in the TXE-Engine not
> +        * getting loaded
> +        */
> +       printf("FSP: Secure Boot %sabled\n",
> +              fsp_vpd->enable_secure_boot == 1 ? "en" : "dis");
> +#endif
>         /* Copy default data from Flash */
>         memcpy(fsp_upd, (void *)(fsp_hdr->img_base + fsp_vpd->upd_offset),
>                sizeof(struct upd_region));
> --
> 2.7.4
>

[U-Boot] [PATCH 4/5] tools: add secure_boot_helper.py

[Simon Glass]

Hi,

On 11 May 2017 at 09:14, Anatolij Gustschin <agust@denx.de> wrote:
> From: Markus Valentin <mv@denx.de>
>
> This script should be used for simple creation of secure bootable
> images for baytrail platforms
>
> Signed-off-by: Markus Valentin <mv@denx.de>
> ---
>  tools/secure_boot_helper.py | 313 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 313 insertions(+)
>  create mode 100644 tools/secure_boot_helper.py

It seems to me that this should use binman rather than a separate tool.

Regards,
Simon

[U-Boot] [PATCH 5/5] doc: x86: Add section about secure boot on Baytrail

[Simon Glass]

On 11 May 2017 at 09:14, Anatolij Gustschin <agust@denx.de> wrote:
> From: Markus Valentin <mv@denx.de>
>
> Signed-off-by: Markus Valentin <mv@denx.de>
> [agust: slightly reworded and fixed alignment]
> Signed-off-by: Anatolij Gustschin <agust@denx.de>
> ---
>  doc/README.x86 | 41 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 41 insertions(+)
>

Looks good. Please use 'U-Boot' consistently. May need adjusting to use binman.

Reviewed-by: Simon Glass <sjg@chromium.org>

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

[Simon Glass]

Hi,

On 11 May 2017 at 09:14, Anatolij Gustschin <agust@denx.de> wrote:
> From: Markus Valentin <mv@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@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@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

[U-Boot] [PATCH 2/5] x86: baytrail: Add fsp-header verification for secure boot fsp

[Anatolij Gustschin]

Hi Simon,

On Sun, 14 May 2017 21:03:27 -0600
Simon Glass sjg at chromium.org wrote:
...
> > @@ -119,6 +119,13 @@ void fsp_init(u32 stack_top, u32 boot_mode, void *nvs_buf)
> >                 /* No valid FSP info header was found */
> >                 panic("Invalid FSP header");
> >         }
> > +#ifdef CONFIG_BAYTRAIL_SECURE_BOOT  
> 
> Can you use if (IS_ENABLED(CONFIG_BAYTRAIL_SECURE_BOOT) instead of
> #ifdef? It reduces the number of build paths.

OK, will fix it.

> > +       /* compare primary and secondary header */
> > +       if (memcmp((void *)(CONFIG_FSP_ADDR + FSP_FIRST_HEADER_OFFSET),
> > +                  (void *)(CONFIG_FSP_ADDR + FSP_SECOND_HEADER_OFFSET),
> > +                  fsp_hdr->hdr_len))
> > +               panic("SB: first & secondary FSP headers don't match");  
> 
> How about s/SB/Secure Boot/?

OK, I'll change it. Thanks!

--
Anatolij

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

[Anatolij Gustschin]

Hi Simon,

On Sun, 14 May 2017 21:03:34 -0600
Simon Glass sjg at chromium.org wrote:
...
> > Introduce functions that check the integrity of u-boot by utilising the
> > hashes stored in the oem-data block.  
> 
> U-Boot

yes, will fix it.

...
> > +/**
> > + * 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: ...
>  * ...
>  */

OK, I'll rework and resubmit.

...
> > +
> > +/**
> > + * 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?

yes, will do.

...
> > +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?

OK, I'll move comments from functions in .c file to this header and
use fsp_ prefix.

...
> > +       if (!verify_u_boot_bin()) {
> > +               /* if our u-boot binary checksum isn't equal to  
> 
> /*
>  * If our ...

OK, thanks!

--
Anatolij