From 9cd80daff9f6d9df08311a790a79632ab647a162 Mon Sep 17 00:00:00 2001 From: Vadim Sukhomlinov Date: Wed, 29 Sep 2021 15:02:49 -0700 Subject: cr50: Update AES public APIs 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 Change-Id: Id614cc346fe22537e9208196bf1322221a253b0c Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/3194985 Reviewed-by: Vadim Sukhomlinov Reviewed-by: Andrey Pronin Commit-Queue: Vadim Sukhomlinov Tested-by: Vadim Sukhomlinov --- board/cr50/dcrypto/aes.c | 55 ++++++++++++++++++++++++++++++------------- board/cr50/dcrypto/aes_cmac.c | 7 +++--- board/cr50/dcrypto/dcrypto.h | 27 +++++++++++++++------ board/cr50/dcrypto/fips.c | 22 ++++++++++------- board/cr50/dcrypto/internal.h | 6 +++++ board/cr50/tpm2/aes.c | 34 +++++++++++++++----------- chip/host/dcrypto/aes.c | 11 +++++---- common/pinweaver.c | 8 +++---- test/pinweaver.c | 11 +++++---- 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 @@ -187,6 +187,12 @@ void HMAC_SHA256_hw_init(struct hmac_sha256_ctx *ctx, const void *key, /* DCRYPTO HMAC-SHA256 final */ 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. */ 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*/ -- cgit v1.2.1