cr50: replace direct calls to EC OS from FIPS module with callbacks

In order to implement self-integrity test for FIPS module we need to
make sure binary code of module in image doesn't change from build to
build. To do that we already place FIPS module as constant address.
However, any call to functions outside the module creates a relocation
which is changing depending on location of that external function in the
image. To prevent that we either need to bring these functions in the
module like it was done with memcpy() and some others or replace their
invocations with callbacks. Task & Memory management functions are hard
to bring in the module, so replace few invocations with indirect calls
using vtable. This way invocation code will remain the same.

1. Identify and minimize dependency on EC OS - remove few asserts
   and cprintfs.
2. Remove checking privilege level in TRNG init - we know that it is
   high by the order of initialization in board_init() and that our
   RO doesn't drop permissions. Correct initialization of TRNG is
   important for certification, so we can't just assume it may be
   initialized improperly.
3. Added vtable with EC OS functions, initialization of FIPS module
   vtable in board_init().
4. Switched to using vtable instead of direct calls. Note, we continue
   to use EC OS with CRYPTO_TEST=1 to reduce vtable size and image
   size.

BUG=b:138578318
TEST=make BOARD=cr50; tests

Signed-off-by: Vadim Sukhomlinov <sukhomlinov@google.com>
Change-Id: Ibd7bd2353fc4e7e5886f9bfef96b36dc64ff2359
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/3107847
Reviewed-by: Vadim Sukhomlinov <sukhomlinov@chromium.org>
Reviewed-by: Andrey Pronin <apronin@chromium.org>
Tested-by: Vadim Sukhomlinov <sukhomlinov@chromium.org>
Auto-Submit: Vadim Sukhomlinov <sukhomlinov@chromium.org>
Commit-Queue: Andrey Pronin <apronin@chromium.org>

9 files changed, 195 insertions, 44 deletions

diff --git a/board/cr50/board.c b/board/cr50/board.c
index 96f90f6cee..a564efae02 100644
--- a/board/cr50/board.c
+++ b/board/cr50/board.c
@@ -14,7 +14,9 @@
 #include "endian.h"
 #include "extension.h"
 #include "fips_rand.h"
+#include "fips.h"
 #include "flash.h"
+#include "flash_log.h"
 #include "flash_config.h"
 #include "gpio.h"
 #include "ite_sync.h"
@@ -29,6 +31,7 @@
 #include "recovery_button.h"
 #include "registers.h"
 #include "scratch_reg1.h"
+#include "shared_mem.h"
 #include "signed_header.h"
 #include "spi.h"
 #include "system.h"
@@ -43,6 +46,7 @@
 #include "usb_i2c.h"
 #include "usb_spi.h"
 #include "util.h"
+#include "watchdog.h"
 #include "wp.h"
 
 /* Define interrupt and gpio structs */
@@ -840,6 +844,28 @@ static void board_init(void)
 #else
 	static enum ccd_state ccd_init_state = CCD_STATE_LOCKED;
 #endif
+	static const struct fips_vtable fips_module_vtable = {
+		.shared_mem_acquire = shared_mem_acquire,
+		.shared_mem_release = shared_mem_release,
+#ifdef CONFIG_FLASH_LOG
+		.flash_log_add_event = flash_log_add_event,
+#endif
+		.get_time = get_time,
+		.task_enable_irq = task_enable_irq,
+		.task_wait_event_mask = task_wait_event_mask,
+		.task_set_event = task_set_event,
+		.task_get_current = task_get_current,
+		.task_start_irq_handler = task_start_irq_handler,
+		.task_resched_if_needed = task_resched_if_needed,
+		.mutex_lock = mutex_lock,
+		.mutex_unlock = mutex_unlock,
+#ifdef CONFIG_WATCHDOG
+		.watchdog_reload = watchdog_reload
+#endif
+	};
+
+	/* Provide callbacks to FIPS module as soon as possible. */
+	fips_set_callbacks(&fips_module_vtable);
 
 	/*
 	 * Deep sleep resets should be considered valid and should not impact
@@ -850,6 +876,7 @@ static void board_init(void)
 	configure_board_specific_gpios();
 	init_pmu();
 	reset_wake_logic();
+	/* It is important to init TRNG before dropping run level. */
 	fips_init_trng();
 	maybe_trigger_ite_sync();
 	init_jittery_clock(1);
diff --git a/board/cr50/dcrypto/bn.c b/board/cr50/dcrypto/bn.c
index 671ce6256e..4f0963d902 100644
--- a/board/cr50/dcrypto/bn.c
+++ b/board/cr50/dcrypto/bn.c
@@ -8,19 +8,11 @@
 #endif
 
 #include "dcrypto.h"
+#include "fips.h"
 #include "internal.h"
 
 #include "trng.h"
 
-
-#include <assert.h>
-
-#ifdef CONFIG_WATCHDOG
-extern void watchdog_reload(void);
-#else
-static inline void watchdog_reload(void) { }
-#endif
-
 void bn_init(struct LITE_BIGNUM *b, void *buf, size_t len)
 {
 	DCRYPTO_bn_wrap(b, buf, len);
@@ -29,8 +21,12 @@ void bn_init(struct LITE_BIGNUM *b, void *buf, size_t len)
 
 void DCRYPTO_bn_wrap(struct LITE_BIGNUM *b, void *buf, size_t len)
 {
-	/* Only word-multiple sized buffers accepted. */
-	assert((len & 0x3) == 0);
+	/* Note: only word-multiple sized buffers accepted. */
+	if (len & 3) {
+		fips_throw_err(FIPS_FATAL_BN_MATH);
+		return;
+	}
+
 	b->dmax = len / LITE_BN_BYTES;
 	b->d = (struct access_helper *) buf;
 }
@@ -309,8 +305,23 @@ static void bn_compute_RR(struct LITE_BIGNUM *RR, const struct LITE_BIGNUM *N)
 
 	/* Repeat 2 * R % N, log2(R) times. */
 	for (i = 0; i < N->dmax * LITE_BN_BITS2; i++) {
+		/**
+		 * assume RR = N - x, where x is positive, less than N,
+		 * let n = RR & N bit size (RR created same size as N).
+		 *
+		 * if RR * 2 overflows it means 2^n > RR >= 2^(n-1),
+		 * 2^n > N - x >= 2^(n-1)
+		 *    ==>  N >= 2^(n-1) + x
+		 *
+		 * 2*RR - 2^n < N because:
+		 * ((2^(n-1) + x) - x)*2 - 2^n < 2^(n-1) + x
+		 *         0 < 2^(n-1) + x
+		 */
 		if (bn_lshift(RR))
-			assert(bn_sub(RR, N) == -1);
+			if (bn_sub(RR, N) != -1)
+				fips_throw_err(FIPS_FATAL_BN_MATH);
+
+		/* RR < N is invariant of the loop */
 		if (bn_gte(RR, N))
 			bn_sub(RR, N);
 	}
@@ -370,7 +381,9 @@ static void bn_modexp_internal(struct LITE_BIGNUM *output,
 		 * TODO(ngm): may be unnecessary with
 		 * a faster implementation.
 		 */
-		watchdog_reload();
+#ifdef CONFIG_WATCHDOG
+		fips_vtable->watchdog_reload();
+#endif
 	}
 
 	bn_mont_mul(output, NULL, &acc, nprime, N);     /* Convert out. */
diff --git a/board/cr50/dcrypto/dcrypto_runtime.c b/board/cr50/dcrypto/dcrypto_runtime.c
index 7de990ea41..db0ab292d7 100644
--- a/board/cr50/dcrypto/dcrypto_runtime.c
+++ b/board/cr50/dcrypto/dcrypto_runtime.c
@@ -4,6 +4,7 @@
  */
 
 #include "flash_log.h"
+#include "fips.h"
 #include "internal.h"
 #include "registers.h"
 #include "task.h"
@@ -51,8 +52,8 @@ static void dcrypto_wipe_imem(void)
 
 void dcrypto_init_and_lock(void)
 {
-	mutex_lock(&dcrypto_mutex);
-	my_task_id = task_get_current();
+	fips_vtable->mutex_lock(&dcrypto_mutex);
+	my_task_id = fips_vtable->task_get_current();
 
 	if (dcrypto_is_initialized)
 		return;
@@ -74,14 +75,14 @@ void dcrypto_init_and_lock(void)
 	GREG32(CRYPTO, INT_STATE) = -1;   /* Reset all the status bits. */
 	GREG32(CRYPTO, INT_ENABLE) = -1;  /* Enable all status bits. */
 
-	task_enable_irq(GC_IRQNUM_CRYPTO0_HOST_CMD_DONE_INT);
+	fips_vtable->task_enable_irq(GC_IRQNUM_CRYPTO0_HOST_CMD_DONE_INT);
 
 	dcrypto_is_initialized = 1;
 }
 
 void dcrypto_unlock(void)
 {
-	mutex_unlock(&dcrypto_mutex);
+	fips_vtable->mutex_unlock(&dcrypto_mutex);
 }
 
 #ifndef DCRYPTO_CALL_TIMEOUT_US
@@ -105,7 +106,7 @@ uint32_t dcrypto_call(uint32_t adr)
 
 	GREG32(CRYPTO, HOST_CMD) = 0x08000000 + adr; /* Call imem:adr. */
 
-	event = task_wait_event_mask(TASK_EVENT_DCRYPTO_DONE,
+	event = fips_vtable->task_wait_event_mask(TASK_EVENT_DCRYPTO_DONE,
 				     DCRYPTO_CALL_TIMEOUT_US);
 	/* TODO(ngm): switch return value to an enum. */
 	switch (event) {
@@ -126,19 +127,22 @@ uint32_t dcrypto_call(uint32_t adr)
 		dcrypto_reset_and_wipe();
 #ifdef CONFIG_FLASH_LOG
 		/* State value of zero indicates event timeout. */
-		flash_log_add_event(FE_LOG_DCRYPTO_FAILURE,
+		fips_vtable->flash_log_add_event(FE_LOG_DCRYPTO_FAILURE,
 				    sizeof(state), &state);
 #endif
 		return 1;
 	}
 }
-
+/**
+ * DECLARE_IRQ()  referencing this function moved outside FIPS module
+ * into fips_cmd.c to remove direct references to EC OS functions hard-
+ * coded into macro.
+ */
 void __keep dcrypto_done_interrupt(void)
 {
 	GREG32(CRYPTO, INT_STATE) = GC_CRYPTO_INT_STATE_HOST_CMD_DONE_MASK;
-	task_set_event(my_task_id, TASK_EVENT_DCRYPTO_DONE, 0);
+	fips_vtable->task_set_event(my_task_id, TASK_EVENT_DCRYPTO_DONE, 0);
 }
-DECLARE_IRQ(GC_IRQNUM_CRYPTO0_HOST_CMD_DONE_INT, dcrypto_done_interrupt, 1);
 
 void dcrypto_imem_load(size_t offset, const uint32_t *opcodes,
 			size_t n_opcodes)
diff --git a/board/cr50/dcrypto/sha_hw.c b/board/cr50/dcrypto/sha_hw.c
index acad1ba29d..82f5c6dacf 100644
--- a/board/cr50/dcrypto/sha_hw.c
+++ b/board/cr50/dcrypto/sha_hw.c
@@ -3,6 +3,7 @@
  * found in the LICENSE file.
  */
 #include "dcrypto.h"
+#include "fips.h"
 #include "internal.h"
 #include "registers.h"
 
@@ -27,21 +28,21 @@ int dcrypto_grab_sha_hw(void)
 {
 	int rv = 0;
 
-	mutex_lock(&hw_busy_mutex);
+	fips_vtable->mutex_lock(&hw_busy_mutex);
 	if (!hw_busy) {
 		rv = 1;
 		hw_busy = true;
 	}
-	mutex_unlock(&hw_busy_mutex);
+	fips_vtable->mutex_unlock(&hw_busy_mutex);
 
 	return rv;
 }
 
 void dcrypto_release_sha_hw(void)
 {
-	mutex_lock(&hw_busy_mutex);
+	fips_vtable->mutex_lock(&hw_busy_mutex);
 	hw_busy = false;
-	mutex_unlock(&hw_busy_mutex);
+	fips_vtable->mutex_unlock(&hw_busy_mutex);
 }
 
 #endif /* ! SECTION_IS_RO */
diff --git a/board/cr50/dcrypto/trng.c b/board/cr50/dcrypto/trng.c
index 6045243615..03a9f756c2 100644
--- a/board/cr50/dcrypto/trng.c
+++ b/board/cr50/dcrypto/trng.c
@@ -47,16 +47,16 @@
 
 void fips_init_trng(void)
 {
-#if (!(defined(CONFIG_CUSTOMIZED_RO) && defined(SECTION_IS_RO)))
 	/*
 	 * Most of the trng initialization requires high permissions. If RO has
 	 * dropped the permission level, dont try to read or write these high
 	 * permission registers because it will cause rolling reboots. RO
 	 * should do the TRNG initialization before dropping the level.
+	 *
+	 * For Cr50 RO doesn't drop permission level and init_trng() is called
+	 * by board_init() before dropping permissions.
 	 */
-	if (!runlevel_is_high())
-		return;
-#endif
+
 	/**
 	 * According to NIST SP 800-90B only vetted conditioning mechanism
 	 * should be used for post-processing raw entropy.
@@ -127,7 +127,10 @@ uint64_t read_rand(void)
 		    empty_count > TRNG_EMPTY_COUNT) {
 			/* TRNG timed out, restart */
 			GWRITE(TRNG, STOP_WORK, 1);
-			flash_log_add_event(FE_LOG_TRNG_STALL, 0, NULL);
+#ifdef CONFIG_FLASH_LOG
+			fips_vtable->flash_log_add_event(FE_LOG_TRNG_STALL, 0,
+							 NULL);
+#endif
 			GWRITE(TRNG, GO_EVENT, 1);
 			empty_count = 0;
 			reset_count++;
diff --git a/board/cr50/fips.c b/board/cr50/fips.c
index 77c467d555..19e6137d29 100644
--- a/board/cr50/fips.c
+++ b/board/cr50/fips.c
@@ -10,6 +10,8 @@
 #include "extension.h"
 #include "fips.h"
 #include "fips_rand.h"
+#include "flash.h"
+#include "flash_info.h"
 #include "flash_log.h"
 #include "hooks.h"
 #include "new_nvmem.h"
@@ -69,10 +71,13 @@ void fips_throw_err(enum fips_status err)
 	if ((_fips_status & err) == err)
 		return;
 	fips_set_status(err);
+#ifdef CONFIG_FLASH_LOG
 	if (_fips_status & FIPS_ERROR_MASK) {
-		flash_log_add_event(FE_LOG_FIPS_FAILURE, sizeof(_fips_status),
-				    &_fips_status);
+		fips_vtable->flash_log_add_event(FE_LOG_FIPS_FAILURE,
+						 sizeof(_fips_status),
+						 &_fips_status);
 	}
+#endif
 }
 
 /**
@@ -589,7 +594,7 @@ void fips_power_up_tests(void)
 	void *stack;
 	uint64_t starttime;
 
-	starttime = get_time().val;
+	starttime = fips_vtable->get_time().val;
 
 	if (!fips_self_integrity())
 		_fips_status |= FIPS_FATAL_SELF_INTEGRITY;
@@ -599,7 +604,7 @@ void fips_power_up_tests(void)
 	 * shared memory for KAT tests temporary larger stack.
 	 */
 	if (EC_SUCCESS ==
-	    shared_mem_acquire(FIPS_KAT_STACK_SIZE, &stack_buf)) {
+	    fips_vtable->shared_mem_acquire(FIPS_KAT_STACK_SIZE, &stack_buf)) {
 		stack = stack_buf + FIPS_KAT_STACK_SIZE;
 		if (!call_on_stack(stack, &fips_sha256_kat))
 			_fips_status |= FIPS_FATAL_SHA256;
@@ -645,7 +650,7 @@ void fips_power_up_tests(void)
 			_fips_status |= FIPS_FATAL_HMAC_DRBG;
 #endif
 		dcrypto_release_sha_hw();
-		shared_mem_release(stack_buf);
+		fips_vtable->shared_mem_release(stack_buf);
 
 		/* Second call to TRNG warm-up. */
 		fips_trng_startup(1);
@@ -653,16 +658,18 @@ void fips_power_up_tests(void)
 		/* If no errors, set not to run tests on wake from sleep. */
 		if (fips_is_no_crypto_error())
 			fips_set_power_up(true);
+#ifdef CONFIG_FLASH_LOG
 		else /* write combined error to flash log */
-			flash_log_add_event(FE_LOG_FIPS_FAILURE,
-					    sizeof(_fips_status),
-					    &_fips_status);
+			fips_vtable->flash_log_add_event(FE_LOG_FIPS_FAILURE,
+							 sizeof(_fips_status),
+							 &_fips_status);
+#endif
 		/* Set the bit that power-up tests completed, even if failed. */
 		_fips_status |= FIPS_POWER_UP_TEST_DONE;
 	} else
 		_fips_status |= FIPS_FATAL_OTHER;
 
-	fips_last_kat_test_duration = get_time().val - starttime;
+	fips_last_kat_test_duration = fips_vtable->get_time().val - starttime;
 }
 
 /* Initialize FIPS mode. Executed during power-up and resume from sleep. */
@@ -690,3 +697,54 @@ static void fips_power_on(void)
 
 /* FIPS initialization is last init hook, HOOK_PRIO_FIPS > HOOK_PRIO_LAST */
 DECLARE_HOOK(HOOK_INIT, fips_power_on, HOOK_PRIO_INIT_FIPS);
+
+const struct fips_vtable *fips_vtable;
+
+/**
+ * Check that given address is in same half of flash as FIPS code.
+ * This rejects addresses in SRAM and provides additional security.
+ */
+static bool is_flash_address(const void *ptr)
+{
+	uintptr_t my_addr =
+		(uintptr_t)is_flash_address - CONFIG_PROGRAM_MEMORY_BASE;
+	uintptr_t offset = (uintptr_t)ptr - CONFIG_PROGRAM_MEMORY_BASE;
+
+	if (my_addr >= CONFIG_RW_MEM_OFF &&
+	    my_addr < CFG_TOP_A_OFF)
+		return (offset >= CONFIG_RW_MEM_OFF) &&
+		       (offset <= CFG_TOP_A_OFF);
+	if (my_addr >= CONFIG_RW_B_MEM_OFF &&
+		 my_addr < CFG_TOP_B_OFF)
+		return (offset >= CONFIG_RW_B_MEM_OFF) &&
+		       (offset <= CFG_TOP_B_OFF);
+
+	/* Otherwise, we don't know what's going on, don't accept it. */
+	return false;
+}
+
+void fips_set_callbacks(const struct fips_vtable *vtable)
+{
+	if (is_flash_address(vtable) &&
+	    is_flash_address(vtable->shared_mem_acquire) &&
+	    is_flash_address(vtable->shared_mem_release) &&
+#ifdef CONFIG_FLASH_LOG
+	    is_flash_address(vtable->flash_log_add_event) &&
+#endif
+#ifdef CONFIG_WATCHDOG
+	    is_flash_address(vtable->watchdog_reload) &&
+#endif
+	    is_flash_address(vtable->get_time) &&
+	    is_flash_address(vtable->task_enable_irq) &&
+	    is_flash_address(vtable->task_wait_event_mask) &&
+	    is_flash_address(vtable->task_set_event) &&
+	    is_flash_address(vtable->task_get_current) &&
+	    is_flash_address(vtable->task_start_irq_handler) &&
+	    is_flash_address(vtable->task_resched_if_needed) &&
+	    is_flash_address(vtable->mutex_lock) &&
+	    is_flash_address(vtable->mutex_unlock))
+
+		fips_vtable = vtable;
+	else
+		fips_vtable = NULL;
+}
diff --git a/board/cr50/fips.h b/board/cr50/fips.h
index a9017e46a9..13d78514d5 100644
--- a/board/cr50/fips.h
+++ b/board/cr50/fips.h
@@ -6,6 +6,8 @@
 #define __EC_BOARD_CR50_FIPS_H__
 
 #include "common.h"
+#include "timer.h"
+#include "task.h"
 
 #ifdef __cplusplus
 extern "C" {
@@ -34,6 +36,7 @@ enum fips_status {
 	FIPS_FATAL_AES256 = 1 << 9,
 #endif
 	FIPS_FATAL_SELF_INTEGRITY = 1 << 10,
+	FIPS_FATAL_BN_MATH = 1 << 11,
 	FIPS_FATAL_OTHER = 1 << 15,
 	FIPS_ERROR_MASK = 0xffff,
 	FIPS_RFU_MASK = 0x7fff0000
@@ -115,6 +118,42 @@ void fips_throw_err(enum fips_status err);
  */
 void fips_power_up_tests(void);
 
+struct fips_vtable {
+	int (*shared_mem_acquire)(int size, char **dest_ptr);
+	void (*shared_mem_release)(void *ptr);
+#ifdef CONFIG_FLASH_LOG
+	void (*flash_log_add_event)(uint8_t type, uint8_t size, void *payload);
+#endif
+	void (*cflush)(void);
+	timestamp_t (*get_time)(void);
+
+	void (*task_enable_irq)(int irq);
+	uint32_t (*task_wait_event_mask)(uint32_t event_mask, int timeout_us);
+	uint32_t (*task_set_event)(task_id_t tskid, uint32_t event, int wait);
+	task_id_t (*task_get_current)(void);
+	void (*task_start_irq_handler)(void *excep_return);
+	void (*task_resched_if_needed)(void *excep_return);
+	void (*mutex_lock)(struct mutex *mtx);
+	void (*mutex_unlock)(struct mutex *mtx);
+#ifdef CONFIG_WATCHDOG
+	void (*watchdog_reload)(void);
+#endif
+};
+
+/* Pointer to external callbacks used by FIPS module. */
+extern const struct fips_vtable *fips_vtable;
+
+/**
+ * Set FIPS module vtable. Called during board_init() phase to provide
+ * pointers to several system functions required by module to function.
+ *
+ * This should be called before any other FIPS functions are invoked as
+ * vtable is used during FIPS power-up tests. Internally it checks that
+ * provided vtable and referenced functions are in the same flash bank
+ * as the FIPS module for additional security.
+ */
+void fips_set_callbacks(const struct fips_vtable *vtable);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/board/cr50/fips_cmd.c b/board/cr50/fips_cmd.c
index 304e23a0da..6642bd3396 100644
--- a/board/cr50/fips_cmd.c
+++ b/board/cr50/fips_cmd.c
@@ -19,9 +19,18 @@
 #include "scratch_reg1.h"
 #include "shared_mem.h"
 #include "system.h"
+#include "task.h"
 #include "tpm_nvmem_ops.h"
 #include "u2f_impl.h"
 
+/**
+ * Create IRQ handler calling FIPS module's dcrypto_done_interrupt() on
+ * interrupt. Generated code calls some of the EC OS task management
+ * functions which are not security/crypto related, so to avoid rewriting
+ * macro using FIPS vtable, move it outside FIPS module.
+ */
+DECLARE_IRQ(GC_IRQNUM_CRYPTO0_HOST_CMD_DONE_INT, dcrypto_done_interrupt, 1);
+
 #define CPRINTS(format, args...) cprints(CC_SYSTEM, format, ##args)
 
 /* Print on console current FIPS mode. */
diff --git a/board/cr50/fips_rand.c b/board/cr50/fips_rand.c
index e5488bcf29..f6f66903d8 100644
--- a/board/cr50/fips_rand.c
+++ b/board/cr50/fips_rand.c
@@ -287,8 +287,7 @@ enum hmac_result fips_hmac_drbg_generate_reseed(struct drbg_ctx *ctx, void *out,
 	while (err == HMAC_DRBG_RESEED_REQUIRED) {
 		/* read another 512 bits of noise */
 		if (!fips_trng_bytes(&entropy_fifo, sizeof(entropy_fifo))) {
-			/* TODO: Report FIPS error properly */
-			ccprintf("FIPS trng bytes failed\n");
+			/* FIPS error is reported by failed TRNG test. */
 			return HMAC_DRBG_RESEED_REQUIRED;
 		}
 
@@ -296,8 +295,6 @@ enum hmac_result fips_hmac_drbg_generate_reseed(struct drbg_ctx *ctx, void *out,
 				 0, NULL, 0);
 		err = hmac_drbg_generate(ctx, out, out_len, input, input_len);
 	}
-	if (err != HMAC_DRBG_SUCCESS)
-		ccprintf("DRBG error %d\n", err);
 	return err;
 }