cr50: add hardened crypto return codes, harden DCRYPTO_equals

1. Introduce enum dcrypto_result defining DCRYPTO_OK and DCRYPTO_FAIL
   constants such that they have large Hamming distance, thus becoming
   more fault-injection resistant.
2. Added value_barrier() and value_barrier_ptr() which prevents compiler
   from certain optimizations, removal of conditional execution.
3. Added hardened_select_if_zero() primitive which produce branch-less
   selection between values.
4. Added convenience function dcrypto_ok_if_zero() to convert zero into
   DCRYPTO_OK.
5. DCRYPTO_equals() implemented in a way that it also checks completion
   of all iterations in addition to comparing value. This makes it
   resistant to fault injection which would result in no comparisons
   made.
6. Updated uses of DCRYPTO_equals() to check for new return code.
7. Few memcmp() replaced with DCRYPTO_equals().

BUG=b:197893750
TEST=make BOARD=cr50 CRYPTO_TEST=1; tests/tpmtest.py
in console check that FIPS KAT tests passes.

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

10 files changed, 216 insertions, 44 deletions

diff --git a/board/cr50/dcrypto/compare.c b/board/cr50/dcrypto/compare.c
index db6193752b..09bbd36fd1 100644
--- a/board/cr50/dcrypto/compare.c
+++ b/board/cr50/dcrypto/compare.c
@@ -5,16 +5,68 @@
 
 #include "dcrypto.h"
 
-/* Constant time comparator. */
-int DCRYPTO_equals(const void *a, const void *b, size_t len)
+/**
+ * CRYPTO_FAST_COMPARE = 1 will enable machine word reads if performance
+ * is important, but will increase code size by ~100 bytes.
+ */
+#define CRYPTO_FAST_COMPARE 0
+
+/* Constant time, hardened comparator. */
+enum dcrypto_result __attribute__((optimize("-O1"))) DCRYPTO_equals(
+	const void *a, const void *b, size_t len)
 {
-	size_t i;
-	const uint8_t *pa = a;
-	const uint8_t *pb = b;
-	uint8_t diff = 0;
+	uintptr_t a_addr = (uintptr_t)a;
+	uintptr_t b_addr = (uintptr_t)b;
+	uintptr_t tail = a_addr + len;
+	uintptr_t tail_copy = value_barrier(tail);
+	uintptr_t diff = 0;
+
+#if CRYPTO_FAST_COMPARE
+	/* Set 'body' to the last word boundary. */
+	uintptr_t body = tail & ~WORD_MASK;
+
+	/* End of head is the tail if data is not aligned. */
+	uintptr_t head = tail;
+
+	/* Compute starting address if src and dest can be aligned. */
+	if (((a_addr & WORD_MASK) == (b_addr & WORD_MASK)) &&
+	    (len >= sizeof(uintptr_t)))
+		/* Set 'head' to the first word boundary. */
+		head = ((a_addr + WORD_MASK) & ~WORD_MASK);
+
+	/* Process misaligned head. */
+	while (a_addr < head) {
+		diff |= *((volatile uint8_t *)a_addr) ^
+			*((volatile uint8_t *)b_addr);
+		a_addr++;
+		b_addr++;
+	}
 
-	for (i = 0; i < len; i++)
-		diff |= pa[i] ^ pb[i];
+	/* Process aligned body (if any). */
+	while (a_addr < body) {
+		diff |= *((volatile uintptr_t *)a_addr) ^
+			*((volatile uintptr_t *)b_addr);
+		a_addr += sizeof(uintptr_t);
+		b_addr += sizeof(uintptr_t);
+	}
+#endif
+	/* Process remaining part. Also serves as fault resistance. */
+	while (a_addr < tail) {
+		diff |= *((volatile uint8_t *)a_addr) ^
+			*((volatile uint8_t *)b_addr);
+		a_addr++;
+		b_addr++;
+	}
 
-	return !diff;
+	/**
+	 *   b_addr = src2 + len
+	 *   tail, a_addr = src1 + len
+	 *   (src2 + len) - (src1 + len) + src1 - src2 = 0
+	 *   Any other result of expression will result in wrong value.
+	 *   Don't use 'src2_addr' as it is possible to make
+	 *   b_addr == a_addr
+	 */
+	return dcrypto_ok_if_zero((value_barrier(b_addr) - tail_copy +
+				   (uintptr_t)a - (uintptr_t)b) |
+				  diff);
 }
diff --git a/board/cr50/dcrypto/crypto_common.h b/board/cr50/dcrypto/crypto_common.h
new file mode 100644
index 0000000000..36e5ebe9b7
--- /dev/null
+++ b/board/cr50/dcrypto/crypto_common.h
@@ -0,0 +1,34 @@
+/* Copyright 2021 The Chromium OS Authors. All rights reserved.
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+#ifndef __EC_FIPS_MODULE_COMMON_H
+#define __EC_FIPS_MODULE_COMMON_H
+
+/**
+ * This header file contains types shared between public API in dcrypto.h and
+ * internal functions in internal.h.
+ */
+
+#include "common.h"
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/**
+ * Result codes for crypto operations, targeting
+ * high Hamming distance from each other.
+ */
+enum dcrypto_result {
+	DCRYPTO_OK = 0xAA33AAFF, /* Success. */
+	DCRYPTO_FAIL = 0x55665501, /* Failure. */
+	DCRYPTO_RETRY = 0xA5775A33,
+	DCRYPTO_RESEED_NEEDED = 0x36AA6355,
+};
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* __EC_FIPS_MODULE_COMMON_H */
diff --git a/board/cr50/dcrypto/dcrypto.h b/board/cr50/dcrypto/dcrypto.h
index 284aa5dd51..760d388404 100644
--- a/board/cr50/dcrypto/dcrypto.h
+++ b/board/cr50/dcrypto/dcrypto.h
@@ -13,6 +13,7 @@
 extern "C" {
 #endif
 
+#include "crypto_common.h"
 #include "internal.h"
 
 #include <stddef.h>
@@ -373,7 +374,7 @@ int DCRYPTO_x509_gen_u2f_cert(const p256_int *d, const p256_int *pk_x,
 /*
  * Memory related functions.
  */
-int DCRYPTO_equals(const void *a, const void *b, size_t len);
+enum dcrypto_result DCRYPTO_equals(const void *a, const void *b, size_t len);
 
 /*
  * Key-ladder and application key related functions.
diff --git a/board/cr50/dcrypto/internal.h b/board/cr50/dcrypto/internal.h
index be217185c7..fd4fa62f43 100644
--- a/board/cr50/dcrypto/internal.h
+++ b/board/cr50/dcrypto/internal.h
@@ -9,6 +9,7 @@
 #include <string.h>
 
 #include "common.h"
+#include "crypto_common.h"
 #include "util.h"
 
 #include "hmacsha2.h"
@@ -297,6 +298,87 @@ static inline uint64_t rol64(uint64_t value, int bits)
 #define alloca __builtin_alloca
 #endif
 
+/* Define machine word alignment mask. */
+#define WORD_MASK (sizeof(uintptr_t) - 1)
+
+/**
+ * @brief Launders the machine register sized value `val`.
+ *
+ * It returns a value identical to the input, while introducing an optimization
+ * barrier that prevents the compiler from learning new information about the
+ * original value, based on operations on the laundered value. For example
+ * with code:
+ *  temp = b - a;
+ *  c = (cond)? temp + a : a;
+ * compiler will reduce code to:
+ *  c = (cond)? b : a;
+ * Which may not be desirable.
+ * making temp = value_barrier(b-a) prevents compiler from such optimization.
+ *
+ * Another case is for double checks against fault injection:
+ * if (cond == 0) { if (value_barrier(cond) != 0 ) panic(); }
+ * Without value_barrier the nested check will be removed.
+ *
+ * @param val A machine register size integer to launder.
+ * @return An integer which will happen to have the same value as `val` at
+ *         runtime.
+ */
+static inline uintptr_t value_barrier(uintptr_t val)
+{
+	/**
+	 * The +r constraint tells the compiler that this is an "inout"
+	 * parameter: it means that not only does the black box depend on `val`,
+	 * but it also mutates it in an unspecified way. This ensures that the
+	 * laundering operation occurs in the right place in the dependency
+	 * ordering.
+	 */
+	asm("" : "+r"(val));
+	return val;
+}
+static inline void *value_barrier_ptr(void *val)
+{
+	/**
+	 * The +r constraint tells the compiler that this is an "inout"
+	 * parameter: it means that not only does the black box depend on `val`,
+	 * but it also mutates it in an unspecified way. This ensures that the
+	 * laundering operation occurs in the right place in the dependency
+	 * ordering.
+	 */
+	asm("" : "+r"(val));
+	return val;
+}
+
+/**
+ * Hardened select of word without branches.
+ *
+ * @param test the value to test
+ * @param a the value returned if 'test' is not 0
+ * @param b the value returned if 'test' is zero
+ *
+ * @return b if test==0 and a otherwise;
+ *
+ * This function is mostly useful when test == 0 is a good result and
+ * hardening is required to prevent easy generation of 'b'.
+ *
+ * Example:
+ *   hardened_select_if_zero(test, CRYPTO_FAIL, CRYPTO_OK)
+ */
+static inline __attribute__((always_inline))
+uintptr_t hardened_select_if_zero(uintptr_t test, uintptr_t a, uintptr_t b)
+{
+	uintptr_t bma = value_barrier(b - a);
+
+	/* For test == 0, does nothing, otherwise damage bma. */
+	bma &= value_barrier(~test);
+	return (test == 0) ? bma + a : a;
+}
+
+/* Convenience wrapper to return DCRYPTO_OK iff val == 0. */
+static inline enum dcrypto_result dcrypto_ok_if_zero(uintptr_t val)
+{
+	return (enum dcrypto_result)hardened_select_if_zero(val, DCRYPTO_FAIL,
+							    DCRYPTO_OK);
+}
 
 /*
  * Key ladder.
diff --git a/board/cr50/dcrypto/p256_ecies.c b/board/cr50/dcrypto/p256_ecies.c
index 250e6e5aaf..8a140b1de6 100644
--- a/board/cr50/dcrypto/p256_ecies.c
+++ b/board/cr50/dcrypto/p256_ecies.c
@@ -158,8 +158,8 @@ size_t DCRYPTO_ecies_decrypt(
 	hmac_key = &key[AES_KEY_BYTES];
 	HMAC_SHA256_hw_init(&ctx, hmac_key, HMAC_KEY_BYTES);
 	HMAC_SHA256_update(&ctx, inp, in_len);
-	if (!DCRYPTO_equals(inp + in_len, HMAC_SHA256_hw_final(&ctx),
-				SHA256_DIGEST_SIZE))
+	if (DCRYPTO_equals(inp + in_len, HMAC_SHA256_hw_final(&ctx),
+			   SHA256_DIGEST_SIZE) != DCRYPTO_OK)
 		return 0;
 
 	memmove(outp, inp, auth_data_len);
diff --git a/board/cr50/dcrypto/rsa.c b/board/cr50/dcrypto/rsa.c
index 6b7146068d..8558c53afe 100644
--- a/board/cr50/dcrypto/rsa.c
+++ b/board/cr50/dcrypto/rsa.c
@@ -156,7 +156,9 @@ static int check_oaep_pad(uint8_t *out, uint32_t *out_len,
 		SHA256_hw_init(&ctx.sha256);
 	HASH_update(&ctx, label, label ? strlen(label) + 1 : 0);
 
-	bad = !DCRYPTO_equals(phash, HASH_final(&ctx)->b8, hash_size);
+	/* bad should be zero if CRYPTO_OK is returned. */
+	bad = DCRYPTO_equals(phash, HASH_final(&ctx)->b8, hash_size) -
+	      DCRYPTO_OK;
 	bad |= padded[0];
 
 	for (i = PS - padded; i <  padded_len; i++) {
@@ -370,10 +372,10 @@ static int check_pkcs1_type1_pad(const uint8_t *msg, uint32_t msg_len,
 
 	if (padded[i++] != 0)
 		return 0;
-	if (!DCRYPTO_equals(&padded[i], der, der_size))
+	if (DCRYPTO_equals(&padded[i], der, der_size) != DCRYPTO_OK)
 		return 0;
 	i += der_size;
-	return DCRYPTO_equals(msg, &padded[i], hash_size);
+	return DCRYPTO_equals(msg, &padded[i], hash_size) == DCRYPTO_OK;
 }
 
 /* sign */
@@ -473,7 +475,8 @@ static int check_pkcs1_pss_pad(const uint8_t *in, uint32_t in_len,
 	HASH_update(&ctx, zeros, sizeof(zeros));
 	HASH_update(&ctx, in, in_len);
 	HASH_update(&ctx, padded + db_len - salt_len, salt_len);
-	bad |= !DCRYPTO_equals(padded + db_len, HASH_final(&ctx), hash_size);
+	bad |= DCRYPTO_equals(padded + db_len, HASH_final(&ctx), hash_size) -
+	       DCRYPTO_OK;
 	return !bad;
 }
 
diff --git a/board/cr50/fips.c b/board/cr50/fips.c
index 19cb78ff69..349117d49b 100644
--- a/board/cr50/fips.c
+++ b/board/cr50/fips.c
@@ -154,7 +154,8 @@ static bool fips_sha256_kat(void)
 	SHA256_hw_init(&ctx);
 	SHA256_update(&ctx, in, sizeof(in));
 	return !(fips_break_cmd == FIPS_BREAK_SHA256) &&
-	       (memcmp(SHA256_final(&ctx), ans, SHA256_DIGEST_SIZE) == 0);
+	       (DCRYPTO_equals(SHA256_final(&ctx), ans, SHA256_DIGEST_SIZE) ==
+		DCRYPTO_OK);
 }
 
 /* KAT for HMAC-SHA256, test values from OpenSSL. */
@@ -180,8 +181,8 @@ static bool fips_hmac_sha256_kat(void)
 	HMAC_SHA256_hw_init(&ctx, k, sizeof(k));
 	HMAC_SHA256_update(&ctx, in, sizeof(in));
 	return !(fips_break_cmd == FIPS_BREAK_HMAC_SHA256) &&
-	       (memcmp(HMAC_SHA256_hw_final(&ctx), ans, SHA256_DIGEST_SIZE) ==
-		0);
+	       (DCRYPTO_equals(HMAC_SHA256_hw_final(&ctx), ans,
+			       SHA256_DIGEST_SIZE) == DCRYPTO_OK);
 }
 
 /**
@@ -275,8 +276,8 @@ static bool fips_hmac_drbg_instantiate_kat(struct drbg_ctx *ctx)
 		       drbg_nonce0, sizeof(drbg_nonce0), drbg_perso0,
 		       sizeof(drbg_perso0));
 
-	return (memcmp(ctx->v, V0, sizeof(V0)) == 0) &&
-	       (memcmp(ctx->k, K0, sizeof(K0)) == 0);
+	return (DCRYPTO_equals(ctx->v, V0, sizeof(V0)) == DCRYPTO_OK) &&
+	       (DCRYPTO_equals(ctx->k, K0, sizeof(K0)) == DCRYPTO_OK);
 }
 
 /* Known-answer test for HMAC_DRBG SHA256 reseed. */
@@ -293,8 +294,8 @@ static bool fips_hmac_drbg_reseed_kat(struct drbg_ctx *ctx)
 	hmac_drbg_reseed(ctx, drbg_entropy1, sizeof(drbg_entropy1),
 			 drbg_addtl_input1, sizeof(drbg_addtl_input1), NULL, 0);
 
-	return (memcmp(ctx->v, V1, sizeof(V1)) == 0) &&
-	       (memcmp(ctx->k, K1, sizeof(K1)) == 0);
+	return (DCRYPTO_equals(ctx->v, V1, sizeof(V1)) == DCRYPTO_OK) &&
+	       (DCRYPTO_equals(ctx->k, K1, sizeof(K1)) == DCRYPTO_OK);
 }
 
 /* Known-answer test for HMAC_DRBG SHA256 generate. */
@@ -327,8 +328,8 @@ static bool fips_hmac_drbg_generate_kat(struct drbg_ctx *ctx)
 
 	hmac_drbg_generate(ctx, buf, sizeof(buf), NULL, 0);
 	/* Verify internal drbg state */
-	if (memcmp(ctx->v, V2, sizeof(V2)) ||
-	    memcmp(ctx->k, K2, sizeof(K2))) {
+	if (DCRYPTO_equals(ctx->v, V2, sizeof(V2)) != DCRYPTO_OK ||
+	    DCRYPTO_equals(ctx->k, K2, sizeof(K2)) != DCRYPTO_OK) {
 		return false;
 	}
 
@@ -340,7 +341,7 @@ static bool fips_hmac_drbg_generate_kat(struct drbg_ctx *ctx)
 	 */
 	hmac_drbg_generate(ctx, buf, sizeof(buf), NULL, 0);
 	return !(fips_break_cmd == FIPS_BREAK_HMAC_DRBG) &&
-	       (memcmp(buf, KA, sizeof(KA)) == 0);
+	       DCRYPTO_equals(buf, KA, sizeof(KA) == DCRYPTO_OK);
 }
 
 /* Known-answer test for HMAC_DRBG SHA256. */
@@ -573,7 +574,7 @@ static bool call_on_stack(void *new_stack, bool (*func)(void))
 const struct sha256_digest fips_integrity
 	__attribute__((section(".rodata.fips.checksum")));
 
-static bool fips_self_integrity(void)
+static enum dcrypto_result fips_self_integrity(void)
 {
 	struct sha256_digest digest;
 	size_t module_length = &__fips_module_end - &__fips_module_start;
@@ -606,7 +607,7 @@ void fips_power_up_tests(void)
 
 	starttime = fips_vtable->get_time().val;
 
-	if (!fips_self_integrity())
+	if (fips_self_integrity() != DCRYPTO_OK)
 		_fips_status |= FIPS_FATAL_SELF_INTEGRITY;
 
 	/* Make sure hardware is properly configured. */
diff --git a/board/cr50/tpm2/ecc.c b/board/cr50/tpm2/ecc.c
index a8263b643e..39d5dcf607 100644
--- a/board/cr50/tpm2/ecc.c
+++ b/board/cr50/tpm2/ecc.c
@@ -470,14 +470,12 @@ static int point_equals(const TPMS_ECC_POINT *a, const TPMS_ECC_POINT *b)
 {
 	int diff = 0;
 
-	diff = a->x.b.size != b->x.b.size;
-	diff |= a->y.b.size != b->y.b.size;
-	if (!diff) {
-		diff |= !DCRYPTO_equals(
-			a->x.b.buffer, b->x.b.buffer, a->x.b.size);
-		diff |= !DCRYPTO_equals(
-			a->y.b.buffer, b->y.b.buffer, a->y.b.size);
-	}
+	diff = a->x.b.size - b->x.b.size;
+	diff |= a->y.b.size - b->y.b.size;
+	diff |= DCRYPTO_equals(a->x.b.buffer, b->x.b.buffer, a->x.b.size) -
+		DCRYPTO_OK;
+	diff |= DCRYPTO_equals(a->y.b.buffer, b->y.b.buffer, a->y.b.size) -
+		DCRYPTO_OK;
 
 	return !diff;
 }
diff --git a/board/cr50/tpm2/endorsement.c b/board/cr50/tpm2/endorsement.c
index 49f542f782..f929f1c405 100644
--- a/board/cr50/tpm2/endorsement.c
+++ b/board/cr50/tpm2/endorsement.c
@@ -594,9 +594,9 @@ enum manufacturing_status tpm_endorse(void)
 		HMAC_SHA256_update(&hmac, "RSA", 4);
 		HMAC_SHA256_hw_init(&hmac, HMAC_SHA256_hw_final(&hmac), 32);
 		HMAC_SHA256_update(&hmac, p, RO_CERTS_REGION_SIZE - 32);
-		if (!DCRYPTO_equals(p + RO_CERTS_REGION_SIZE - 32,
-				   HMAC_SHA256_hw_final(&hmac), 32)) {
-
+		if (DCRYPTO_equals(p + RO_CERTS_REGION_SIZE - 32,
+				   HMAC_SHA256_hw_final(&hmac),
+				   32) != DCRYPTO_OK) {
 			CPRINTF("%s: bad cert region hmac;", __func__);
 #ifdef CR50_INCLUDE_FALLBACK_CERT
 			/* HMAC verification failure indicates either
diff --git a/board/cr50/tpm2/rsa.c b/board/cr50/tpm2/rsa.c
index f40996c9f5..0dc0404b79 100644
--- a/board/cr50/tpm2/rsa.c
+++ b/board/cr50/tpm2/rsa.c
@@ -376,11 +376,11 @@ CRYPT_RESULT _cpri__GenerateKeyRSA(
 	 * template instead.
 	 */
 	if (extra->size == sizeof(TPM2_RSA_EK_NAME_TEMPLATE) &&
-		DCRYPTO_equals(extra->buffer, TPM2_RSA_EK_NAME_TEMPLATE,
-			       sizeof(TPM2_RSA_EK_NAME_TEMPLATE)) &&
-		seed == &endorsement_seed->b) {
+	    DCRYPTO_equals(extra->buffer, TPM2_RSA_EK_NAME_TEMPLATE,
+			   sizeof(TPM2_RSA_EK_NAME_TEMPLATE)) == DCRYPTO_OK &&
+	    seed == &endorsement_seed->b) {
 		memcpy(local_extra.b.buffer, TPM2_RSA_EK_NAME_CR50,
-			sizeof(TPM2_RSA_EK_NAME_CR50));
+		       sizeof(TPM2_RSA_EK_NAME_CR50));
 		extra = &local_extra.b;
 	}
 
@@ -389,7 +389,8 @@ CRYPT_RESULT _cpri__GenerateKeyRSA(
 	 */
 #ifdef CRYPTO_TEST_SETUP
 	if (seed->size == sizeof(VERIFY_SEED) &&
-		DCRYPTO_equals(seed->buffer, VERIFY_SEED, seed->size)) {
+	    DCRYPTO_equals(seed->buffer, VERIFY_SEED, seed->size) ==
+		    DCRYPTO_OK) {
 		/* Test seed has already been hashed down. */
 		memcpy(local_seed.t.buffer, seed->buffer, seed->size);
 	} else