cr50: Update AES public APIsfactory-ambassador-14265.B-cr50_stab

To support FIPS mode we need to block access to crypto in case of
errors.

1) Added check for FIPS errors into DCRYPTO_aes_init()
2) Return codes updated to enum dcrypto_result
3) Call sites updated to check for return codes

BUG=b:197893750
TEST=make BOARD=cr50 CRYPTO_TEST=1; test/tpm_test/tpmtest.py

Signed-off-by: Vadim Sukhomlinov <sukhomlinov@google.com>
Change-Id: Id614cc346fe22537e9208196bf1322221a253b0c
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/3194985
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>

9 files changed, 118 insertions, 63 deletions

diff --git a/board/cr50/dcrypto/aes.c b/board/cr50/dcrypto/aes.c
index 327ce67257..a4895b7a6f 100644
--- a/board/cr50/dcrypto/aes.c
+++ b/board/cr50/dcrypto/aes.c
@@ -38,8 +38,9 @@ static int wait_read_data(volatile uint32_t *addr)
 	return empty ? 0 : 1;
 }
 
-int DCRYPTO_aes_init(const uint8_t *key, size_t key_len, const uint8_t *iv,
-		enum cipher_mode c_mode, enum encrypt_mode e_mode)
+enum dcrypto_result dcrypto_aes_init(const uint8_t *key, size_t key_len,
+				     const uint8_t *iv, enum cipher_mode c_mode,
+				     enum encrypt_mode e_mode)
 {
 	size_t i;
 	const struct access_helper *p;
@@ -57,9 +58,19 @@ int DCRYPTO_aes_init(const uint8_t *key, size_t key_len, const uint8_t *iv,
 		break;
 	default:
 		/* Invalid key length specified. */
-		return 0;
+		return DCRYPTO_FAIL;
+	}
+
+	switch (c_mode) {
+	case CIPHER_MODE_ECB:
+	case CIPHER_MODE_CTR:
+	case CIPHER_MODE_CBC:
+		set_control_register(c_mode, key_mode, e_mode);
+		break;
+	default:
+		/* Invalid mode specified. */
+		return DCRYPTO_FAIL;
 	}
-	set_control_register(c_mode, key_mode, e_mode);
 
 	/* Initialize hardware with AES key */
 	p = (struct access_helper *) key;
@@ -71,17 +82,26 @@ int DCRYPTO_aes_init(const uint8_t *key, size_t key_len, const uint8_t *iv,
 	/* Wait for key expansion. */
 	if (!wait_read_data(GREG32_ADDR(KEYMGR, AES_KEY_START))) {
 		/* Should not happen. */
-		return 0;
+		return DCRYPTO_FAIL;
 	}
 
 	/* Initialize IV for modes that require it. */
 	if (iv)
 		DCRYPTO_aes_write_iv(iv);
 
-	return 1;
+	return DCRYPTO_OK;
+}
+
+enum dcrypto_result DCRYPTO_aes_init(const uint8_t *key, size_t key_len,
+				     const uint8_t *iv, enum cipher_mode c_mode,
+				     enum encrypt_mode e_mode)
+{
+	if (!fips_crypto_allowed())
+		return DCRYPTO_FAIL;
+	return dcrypto_aes_init(key, key_len, iv, c_mode, e_mode);
 }
 
-int DCRYPTO_aes_block(const uint8_t *in, uint8_t *out)
+enum dcrypto_result DCRYPTO_aes_block(const uint8_t *in, uint8_t *out)
 {
 	int i;
 	uint32_t buf[4];
@@ -101,7 +121,7 @@ int DCRYPTO_aes_block(const uint8_t *in, uint8_t *out)
 	/* Wait for the result. */
 	if (!wait_read_data(GREG32_ADDR(KEYMGR, AES_RFIFO_EMPTY))) {
 		/* Should not happen, ciphertext not ready. */
-		return 0;
+		return DCRYPTO_FAIL;
 	}
 
 	/* Read ciphertext. */
@@ -116,7 +136,7 @@ int DCRYPTO_aes_block(const uint8_t *in, uint8_t *out)
 	if (out != (uint8_t *)outw)
 		memcpy(out, outw, sizeof(buf));
 
-	return 1;
+	return DCRYPTO_OK;
 }
 
 void DCRYPTO_aes_write_iv(const uint8_t *iv)
@@ -153,13 +173,14 @@ void DCRYPTO_aes_read_iv(uint8_t *iv)
 		memcpy(iv, ivw, sizeof(buf));
 }
 
-int DCRYPTO_aes_ctr(uint8_t *out, const uint8_t *key, uint32_t key_bits,
-		const uint8_t *iv, const uint8_t *in, size_t in_len)
+enum dcrypto_result DCRYPTO_aes_ctr(uint8_t *out, const uint8_t *key,
+				    uint32_t key_bits, const uint8_t *iv,
+				    const uint8_t *in, size_t in_len)
 {
 	/* Initialize AES hardware. */
-	if (!DCRYPTO_aes_init(key, key_bits, iv,
-				CIPHER_MODE_CTR, ENCRYPT_MODE))
-		return 0;
+	if (DCRYPTO_aes_init(key, key_bits, iv, CIPHER_MODE_CTR,
+			     ENCRYPT_MODE) != DCRYPTO_OK)
+		return DCRYPTO_FAIL;
 
 	while (in_len > 0) {
 		uint32_t tmpin[4];
@@ -176,8 +197,8 @@ int DCRYPTO_aes_ctr(uint8_t *out, const uint8_t *key, uint32_t key_bits,
 			inp = in;
 			outp = out;
 		}
-		if (!DCRYPTO_aes_block(inp, outp))
-			return 0;
+		if (DCRYPTO_aes_block(inp, outp) != DCRYPTO_OK)
+			return DCRYPTO_FAIL;
 		if (outp != out)
 			memcpy(out, outp, count);
 
@@ -185,5 +206,5 @@ int DCRYPTO_aes_ctr(uint8_t *out, const uint8_t *key, uint32_t key_bits,
 		out += count;
 		in_len -= count;
 	}
-	return 1;
+	return DCRYPTO_OK;
 }
diff --git a/board/cr50/dcrypto/aes_cmac.c b/board/cr50/dcrypto/aes_cmac.c
index 4f996f42b6..a921bc589b 100644
--- a/board/cr50/dcrypto/aes_cmac.c
+++ b/board/cr50/dcrypto/aes_cmac.c
@@ -54,10 +54,11 @@ static int aes128(const uint8_t *K, const uint32_t in[4], uint32_t out[4])
 {
 	const uint32_t zero[4] = {0, 0, 0, 0};
 
-	if (!DCRYPTO_aes_init((const uint8_t *)K, 128, (const uint8_t *) zero,
-				CIPHER_MODE_ECB, ENCRYPT_MODE))
+	if (DCRYPTO_aes_init((const uint8_t *)K, 128, (const uint8_t *)zero,
+			     CIPHER_MODE_ECB, ENCRYPT_MODE) != DCRYPTO_OK)
 		return 0;
-	if (!DCRYPTO_aes_block((const uint8_t *) in, (uint8_t *) out))
+	if (DCRYPTO_aes_block((const uint8_t *)in, (uint8_t *)out) !=
+	    DCRYPTO_OK)
 		return 0;
 	return 1;
 }
diff --git a/board/cr50/dcrypto/dcrypto.h b/board/cr50/dcrypto/dcrypto.h
index bbb8e0b2ef..bd87c1efe9 100644
--- a/board/cr50/dcrypto/dcrypto.h
+++ b/board/cr50/dcrypto/dcrypto.h
@@ -610,18 +610,31 @@ static inline const struct sha512_digest *HMAC_SHA512_final(
  */
 #define AES256_BLOCK_CIPHER_KEY_SIZE 32
 
-int DCRYPTO_aes_init(const uint8_t *key, size_t key_len, const uint8_t *iv,
-		     enum cipher_mode c_mode, enum encrypt_mode e_mode);
-int DCRYPTO_aes_block(const uint8_t *in, uint8_t *out);
+/**
+ * Initialize AES hardware into specified mode
+ *
+ * @param key AES key
+ * @param key_len key length in bits 128/192/256
+ * @param iv initialization vector for CTR/CBC mode
+ * @param c_mode cipher mode ECB/CTR/CBC
+ * @param e_mode encryption mode (encrypt/decrypt)
+ * @return DCRYPTO_OK if successful
+ */
+enum dcrypto_result DCRYPTO_aes_init(const uint8_t *key, size_t key_len,
+				     const uint8_t *iv, enum cipher_mode c_mode,
+				     enum encrypt_mode e_mode);
 
-void DCRYPTO_aes_write_iv(const uint8_t *iv);
-void DCRYPTO_aes_read_iv(uint8_t *iv);
+enum dcrypto_result DCRYPTO_aes_block(const uint8_t *in, uint8_t *out);
 
 /* AES-CTR-128/192/256
  * NIST Special Publication 800-38A
  */
-int DCRYPTO_aes_ctr(uint8_t *out, const uint8_t *key, uint32_t key_bits,
-		    const uint8_t *iv, const uint8_t *in, size_t in_len);
+enum dcrypto_result DCRYPTO_aes_ctr(uint8_t *out, const uint8_t *key,
+				    uint32_t key_bits, const uint8_t *iv,
+				    const uint8_t *in, size_t in_len);
+
+void DCRYPTO_aes_write_iv(const uint8_t *iv);
+void DCRYPTO_aes_read_iv(uint8_t *iv);
 
 /* AES-GCM-128/192/256
  * NIST Special Publication 800-38D, IV is provided externally
diff --git a/board/cr50/dcrypto/fips.c b/board/cr50/dcrypto/fips.c
index 9fdad94df7..7e70146522 100644
--- a/board/cr50/dcrypto/fips.c
+++ b/board/cr50/dcrypto/fips.c
@@ -482,6 +482,7 @@ static bool fips_aes256_kat(void)
 	uint8_t enc[AES_BLOCK_LEN];
 	uint8_t dec[AES_BLOCK_LEN];
 	uint8_t iv[AES_BLOCK_LEN];
+	enum dcrypto_result result;
 
 	static const uint8_t kat_aes128_k[AES256_BLOCK_CIPHER_KEY_SIZE] = {
 		0x65, 0x74, 0x61, 0x6f, 0x6e, 0x72, 0x69, 0x73,
@@ -500,16 +501,21 @@ static bool fips_aes256_kat(void)
 	};
 
 	memset(iv, 0, sizeof(iv));
-	DCRYPTO_aes_init(kat_aes128_k, 256, iv, CIPHER_MODE_CBC, ENCRYPT_MODE);
-	DCRYPTO_aes_block(kat_aes128_msg, enc);
-	if (memcmp(enc, ans_aes128, AES_BLOCK_LEN))
-		return false;
+	/* Use internal function as we are not yet in FIPS mode. */
+	result = dcrypto_aes_init(kat_aes128_k, 256, iv, CIPHER_MODE_CBC,
+				  ENCRYPT_MODE);
+	result |= DCRYPTO_aes_block(kat_aes128_msg, enc);
+	result |= DCRYPTO_equals(enc, ans_aes128, AES_BLOCK_LEN);
+
+	if (fips_break_cmd == FIPS_BREAK_AES256)
+		enc[1] ^= 1;
 
-	DCRYPTO_aes_init(kat_aes128_k, 256, iv, CIPHER_MODE_CBC, DECRYPT_MODE);
-	DCRYPTO_aes_block(enc, dec);
+	result |= dcrypto_aes_init(kat_aes128_k, 256, iv, CIPHER_MODE_CBC,
+				   DECRYPT_MODE);
+	result |= DCRYPTO_aes_block(enc, dec);
+	result |= DCRYPTO_equals(kat_aes128_msg, dec, AES_BLOCK_LEN);
 
-	return !(fips_break_cmd == FIPS_BREAK_AES256) &&
-	       (memcmp(kat_aes128_msg, dec, AES_BLOCK_LEN) == 0);
+	return result == DCRYPTO_OK;
 }
 #endif
 
diff --git a/board/cr50/dcrypto/internal.h b/board/cr50/dcrypto/internal.h
index 638c6357f8..696ad2de9d 100644
--- a/board/cr50/dcrypto/internal.h
+++ b/board/cr50/dcrypto/internal.h
@@ -188,6 +188,12 @@ void HMAC_SHA256_hw_init(struct hmac_sha256_ctx *ctx, const void *key,
 const struct sha256_digest *HMAC_SHA256_hw_final(struct hmac_sha256_ctx *ctx);
 
 /*
+ * Hardware AES.
+ */
+enum dcrypto_result dcrypto_aes_init(const uint8_t *key, size_t key_len,
+				     const uint8_t *iv, enum cipher_mode c_mode,
+				     enum encrypt_mode e_mode);
+/*
  * BIGNUM.
  */
 #define LITE_BN_BITS2        32
diff --git a/board/cr50/tpm2/aes.c b/board/cr50/tpm2/aes.c
index 7adca75f3b..f515d0364b 100644
--- a/board/cr50/tpm2/aes.c
+++ b/board/cr50/tpm2/aes.c
@@ -24,7 +24,8 @@ CRYPT_RESULT _cpri__AESDecryptCBC(
 		return CRYPT_SUCCESS;
 	assert(key != NULL && iv != NULL && in != NULL && out != NULL);
 	assert(len <= INT32_MAX);
-	if (!DCRYPTO_aes_init(key, num_bits, iv, CIPHER_MODE_CBC, DECRYPT_MODE))
+	if (DCRYPTO_aes_init(key, num_bits, iv, CIPHER_MODE_CBC,
+			     DECRYPT_MODE) != DCRYPTO_OK)
 		return CRYPT_PARAMETER;
 
 	result = _cpri__AESBlock(out, len, in);
@@ -44,18 +45,19 @@ CRYPT_RESULT _cpri__AESDecryptCFB(uint8_t *out, uint32_t num_bits,
 	assert(key != NULL && iv != NULL && out != NULL && in != NULL);
 
 	/* Initialize AES hardware. */
-	if (!DCRYPTO_aes_init(key, num_bits, NULL,
-				CIPHER_MODE_ECB, ENCRYPT_MODE))
+	if (DCRYPTO_aes_init(key, num_bits, NULL, CIPHER_MODE_ECB,
+			     ENCRYPT_MODE) != DCRYPTO_OK)
 		return CRYPT_PARAMETER;
 
 	while (len > 0) {
-		int i;
+		size_t i;
 		size_t chunk_len;
 		uint8_t mask[16];
 
 		chunk_len = MIN(len, 16);
 
-		DCRYPTO_aes_block(iv, mask);
+		if (DCRYPTO_aes_block(iv, mask) != DCRYPTO_OK)
+			return CRYPT_FAIL;
 
 		memcpy(iv, in, chunk_len);
 		if (chunk_len != 16)
@@ -92,7 +94,8 @@ static CRYPT_RESULT _cpri__AESBlock(
 		return CRYPT_PARAMETER;
 
 	for (; slen > 0; slen -= 16) {
-		DCRYPTO_aes_block(in, out);
+		if (DCRYPTO_aes_block(in, out) != DCRYPTO_OK)
+			return CRYPT_FAIL;
 		in = &in[16];
 		out = &out[16];
 	}
@@ -106,7 +109,8 @@ CRYPT_RESULT _cpri__AESEncryptCBC(
 	CRYPT_RESULT result;
 
 	assert(key != NULL && iv != NULL);
-	if (!DCRYPTO_aes_init(key, num_bits, iv, CIPHER_MODE_CBC, ENCRYPT_MODE))
+	if (DCRYPTO_aes_init(key, num_bits, iv, CIPHER_MODE_CBC,
+			     ENCRYPT_MODE) != DCRYPTO_OK)
 		return CRYPT_PARAMETER;
 
 	result = _cpri__AESBlock(out, len, in);
@@ -126,11 +130,13 @@ CRYPT_RESULT _cpri__AESEncryptCFB(
 
 	assert(out != NULL && key != NULL && iv != NULL && in != NULL);
 	assert(len <= INT32_MAX);
-	if (!DCRYPTO_aes_init(key, num_bits, iv, CIPHER_MODE_CTR, ENCRYPT_MODE))
+	if (DCRYPTO_aes_init(key, num_bits, iv, CIPHER_MODE_CTR,
+			     ENCRYPT_MODE) != DCRYPTO_OK)
 		return CRYPT_PARAMETER;
 
 	for (; len >= 16; len -= 16, in += 16, out += 16) {
-		DCRYPTO_aes_block(in, out);
+		if (DCRYPTO_aes_block(in, out) != DCRYPTO_OK)
+			return CRYPT_FAIL;
 		DCRYPTO_aes_write_iv(out);
 	}
 	if (len > 0) {
@@ -158,7 +164,7 @@ CRYPT_RESULT _cpri__AESEncryptCTR(
 	assert(out != NULL && key != NULL && iv != NULL && in != NULL);
 	assert(len <= INT32_MAX);
 
-	if (!DCRYPTO_aes_ctr(out, key, num_bits, iv, in, len))
+	if (DCRYPTO_aes_ctr(out, key, num_bits, iv, in, len) != DCRYPTO_OK)
 		return CRYPT_PARAMETER;
 	else
 		return CRYPT_SUCCESS;
@@ -170,8 +176,8 @@ CRYPT_RESULT _cpri__AESEncryptECB(
 {
 	assert(key != NULL);
 	/* Initialize AES hardware. */
-	if (!DCRYPTO_aes_init(key, num_bits, NULL,
-			      CIPHER_MODE_ECB, ENCRYPT_MODE))
+	if (DCRYPTO_aes_init(key, num_bits, NULL, CIPHER_MODE_ECB,
+			     ENCRYPT_MODE) != DCRYPTO_OK)
 		return CRYPT_PARAMETER;
 	return _cpri__AESBlock(out, len, in);
 }
@@ -191,8 +197,8 @@ CRYPT_RESULT _cpri__AESEncryptOFB(
 	assert(len <= INT32_MAX);
 	slen = (int32_t) len;
 	/* Initialize AES hardware. */
-	if (!DCRYPTO_aes_init(key, num_bits, NULL,
-				CIPHER_MODE_ECB, ENCRYPT_MODE))
+	if (DCRYPTO_aes_init(key, num_bits, NULL, CIPHER_MODE_ECB,
+			     ENCRYPT_MODE) != DCRYPTO_OK)
 		return CRYPT_PARAMETER;
 
 	for (; slen > 0; slen -= 16) {
diff --git a/chip/host/dcrypto/aes.c b/chip/host/dcrypto/aes.c
index cc57168cbb..4556b4b5dd 100644
--- a/chip/host/dcrypto/aes.c
+++ b/chip/host/dcrypto/aes.c
@@ -10,16 +10,17 @@
 #include "dcrypto.h"
 #include "registers.h"
 
-int DCRYPTO_aes_ctr(uint8_t *out, const uint8_t *key, uint32_t key_bits,
-		const uint8_t *iv, const uint8_t *in, size_t in_len)
+enum dcrypto_result DCRYPTO_aes_ctr(uint8_t *out, const uint8_t *key,
+				    uint32_t key_bits, const uint8_t *iv,
+				    const uint8_t *in, size_t in_len)
 {
 	EVP_CIPHER_CTX *ctx;
-	int ret = 0;
+	enum dcrypto_result ret = DCRYPTO_FAIL;
 	int out_len = 0;
 
 	ctx = EVP_CIPHER_CTX_new();
 	if (!ctx)
-		return 0;
+		return DCRYPTO_FAIL;
 
 	if (EVP_EncryptInit_ex(ctx, EVP_aes_256_ctr(), NULL, key, iv) != 1)
 		goto cleanup;
@@ -29,7 +30,7 @@ int DCRYPTO_aes_ctr(uint8_t *out, const uint8_t *key, uint32_t key_bits,
 
 	if (EVP_EncryptFinal(ctx, out + out_len, &out_len) != 1)
 		goto cleanup;
-	ret = 1;
+	ret = DCRYPTO_OK;
 
 cleanup:
 	EVP_CIPHER_CTX_free(ctx);
diff --git a/common/pinweaver.c b/common/pinweaver.c
index 9b7ad0b6d6..1443bebd7b 100644
--- a/common/pinweaver.c
+++ b/common/pinweaver.c
@@ -289,11 +289,11 @@ static int encrypt_leaf_data(const struct merkle_tree_t *merkle_tree,
 		return PW_ERR_CRYPTO_FAILURE;
 	memcpy(&wrapped_leaf_data->pub, &leaf_data->pub,
 	       sizeof(leaf_data->pub));
-	if (!DCRYPTO_aes_ctr(wrapped_leaf_data->cipher_text,
+	if (DCRYPTO_aes_ctr(wrapped_leaf_data->cipher_text,
 			    merkle_tree->wrap_key,
 			    sizeof(merkle_tree->wrap_key) << 3,
 			    wrapped_leaf_data->iv, (uint8_t *)&leaf_data->sec,
-			    sizeof(leaf_data->sec))) {
+			    sizeof(leaf_data->sec)) != DCRYPTO_OK) {
 		return PW_ERR_CRYPTO_FAILURE;
 	}
 	return EC_SUCCESS;
@@ -308,11 +308,11 @@ static int decrypt_leaf_data(
 	memcpy(&leaf_data->pub, imported_leaf_data->pub,
 	       MIN(imported_leaf_data->head->pub_len,
 		   sizeof(struct leaf_public_data_t)));
-	if (!DCRYPTO_aes_ctr((uint8_t *)&leaf_data->sec, merkle_tree->wrap_key,
+	if (DCRYPTO_aes_ctr((uint8_t *)&leaf_data->sec, merkle_tree->wrap_key,
 			    sizeof(merkle_tree->wrap_key) << 3,
 			    imported_leaf_data->iv,
 			    imported_leaf_data->cipher_text,
-			    sizeof(leaf_data->sec))) {
+			    sizeof(leaf_data->sec)) != DCRYPTO_OK) {
 		return PW_ERR_CRYPTO_FAILURE;
 	}
 	return EC_SUCCESS;
diff --git a/test/pinweaver.c b/test/pinweaver.c
index df7327887f..8f2890d34b 100644
--- a/test/pinweaver.c
+++ b/test/pinweaver.c
@@ -936,16 +936,17 @@ const struct sha256_digest *HMAC_SHA256_final(struct hmac_sha256_ctx *ctx)
 /* Perform a symmetric transformation of the data to simulate AES without
  * requiring a full AES-CTR implementation.
  *
- * 1 for success 0 for fail
+ * DCRYPTO_OK for success DCRYPTO_FAIL for fail
  */
-int DCRYPTO_aes_ctr(uint8_t *out, const uint8_t *key, uint32_t key_bits,
-		    const uint8_t *iv, const uint8_t *in, size_t in_len)
+enum dcrypto_result DCRYPTO_aes_ctr(uint8_t *out, const uint8_t *key,
+				    uint32_t key_bits, const uint8_t *iv,
+				    const uint8_t *in, size_t in_len)
 {
 	size_t x;
 
 	if (MOCK_aes_fail) {
 		--MOCK_aes_fail;
-		return 0;
+		return DCRYPTO_FAIL;
 	}
 
 	TEST_ASSERT(key_bits == 256);
@@ -956,7 +957,7 @@ int DCRYPTO_aes_ctr(uint8_t *out, const uint8_t *key, uint32_t key_bits,
 
 	for (x = 0; x < in_len; ++x)
 		out[x] = MOCK_AES_XOR_BYTE(x) ^ in[x];
-	return 1;
+	return DCRYPTO_OK;
 }
 
 /* 1 for success 0 for fail*/