diff options
| -rw-r--r-- | board/cr50/dcrypto/compare.c | 70 | ||||
| -rw-r--r-- | board/cr50/dcrypto/crypto_common.h | 34 | ||||
| -rw-r--r-- | board/cr50/dcrypto/dcrypto.h | 3 | ||||
| -rw-r--r-- | board/cr50/dcrypto/internal.h | 82 | ||||
| -rw-r--r-- | board/cr50/dcrypto/p256_ecies.c | 4 | ||||
| -rw-r--r-- | board/cr50/dcrypto/rsa.c | 11 | ||||
| -rw-r--r-- | board/cr50/fips.c | 25 | ||||
| -rw-r--r-- | board/cr50/tpm2/ecc.c | 14 | ||||
| -rw-r--r-- | board/cr50/tpm2/endorsement.c | 6 | ||||
| -rw-r--r-- | board/cr50/tpm2/rsa.c | 11 |
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 |