From ed1532bf81e3738eda104db226f46d5a457fec30 Mon Sep 17 00:00:00 2001 From: nagendra modadugu Date: Tue, 9 May 2017 18:39:13 -0700 Subject: [PATCH] CR50: replace dcrypto_memset with always_memset always_memset() implements a version of memset that survives compiler optimization. This change replaces instances of the (placeholder) call dcrypto_memset() with always_memset(). Also add a couple of missing memsets and fix related TODOs by replacing memset() with always_memset(). BRANCH=none BUG=none TEST=TCG tests pass Change-Id: I742393852ed5be9f74048eea7244af7be027dd0e Signed-off-by: nagendra modadugu Reviewed-on: https://chromium-review.googlesource.com/501368 Commit-Ready: Nagendra Modadugu Tested-by: Nagendra Modadugu Reviewed-by: Vadim Bendebury Reviewed-by: Andrey Pronin --- board/cr50/tpm2/ecc.c | 19 +++++++++++++------ board/cr50/tpm2/endorsement.c | 7 ++++--- board/cr50/tpm2/rsa.c | 13 ++++++------- chip/g/dcrypto/bn.c | 12 +++++++----- chip/g/dcrypto/hkdf.c | 3 ++- chip/g/dcrypto/hmac.c | 3 ++- chip/g/dcrypto/internal.h | 6 ------ chip/g/dcrypto/p256.c | 3 ++- chip/g/dcrypto/rsa.c | 19 ++++++++++--------- 9 files changed, 46 insertions(+), 39 deletions(-) diff --git a/board/cr50/tpm2/ecc.c b/board/cr50/tpm2/ecc.c index 962a9e1f67..cbd384093d 100644 --- a/board/cr50/tpm2/ecc.c +++ b/board/cr50/tpm2/ecc.c @@ -15,6 +15,7 @@ #include "cryptoc/p256.h" #include "cryptoc/p256_ecdsa.h" +#include "cryptoc/util.h" static void reverse_tpm2b(TPM2B *b) { @@ -171,6 +172,7 @@ CRYPT_RESULT _cpri__GenerateKeyEcc( HASH_update(&hmac.hash, "ECC", 4); memcpy(local_seed.t.buffer, DCRYPTO_HMAC_final(&hmac), local_seed.t.size); + always_memset(&hmac, 0, sizeof(hmac)); /* TODO(ngm): CRBUG/P/55260: the personalize code uses only * the first 4 bytes of extra. */ @@ -204,8 +206,9 @@ CRYPT_RESULT _cpri__GenerateKeyEcc( break; } } - /* TODO(ngm): implement secure memset. */ - memset(local_seed.t.buffer, 0, local_seed.t.size); + + always_memset(local_seed.t.buffer, 0, local_seed.t.size); + always_memset(key_bytes, 0, sizeof(key_bytes)); if (count == 0) FAIL(FATAL_ERROR_INTERNAL); @@ -312,6 +315,7 @@ CRYPT_RESULT _cpri__ValidateSignatureEcc( CRYPT_RESULT _cpri__GetEphemeralEcc(TPMS_ECC_POINT *q, TPM2B_ECC_PARAMETER *d, TPM_ECC_CURVE curve_id) { + int result; uint8_t key_bytes[P256_NBYTES] __aligned(4); if (curve_id != TPM_ECC_NIST_P256) @@ -319,10 +323,13 @@ CRYPT_RESULT _cpri__GetEphemeralEcc(TPMS_ECC_POINT *q, TPM2B_ECC_PARAMETER *d, rand_bytes(key_bytes, sizeof(key_bytes)); - if (DCRYPTO_p256_key_from_bytes((p256_int *) q->x.b.buffer, - (p256_int *) q->y.b.buffer, - (p256_int *) d->b.buffer, - key_bytes)) { + result = DCRYPTO_p256_key_from_bytes((p256_int *) q->x.b.buffer, + (p256_int *) q->y.b.buffer, + (p256_int *) d->b.buffer, + key_bytes); + always_memset(key_bytes, 0, sizeof(key_bytes)); + + if (result) { q->x.b.size = sizeof(p256_int); q->y.b.size = sizeof(p256_int); reverse_tpm2b(&q->x.b); diff --git a/board/cr50/tpm2/endorsement.c b/board/cr50/tpm2/endorsement.c index 468a74f485..dc0e09a789 100644 --- a/board/cr50/tpm2/endorsement.c +++ b/board/cr50/tpm2/endorsement.c @@ -30,6 +30,7 @@ #include "dcrypto.h" #include +#include #include #include @@ -445,7 +446,7 @@ static int get_decrypted_eps(uint8_t eps[PRIMARY_SEED_SIZE]) if (flash_physical_info_read_word( INFO1_EPS_OFFSET + i, &word) != EC_SUCCESS) { - memset(frk2, 0, sizeof(frk2)); + always_memset(frk2, 0, sizeof(frk2)); return 0; /* Flash read INFO1 failed. */ } memcpy(eps + i, &word, sizeof(word)); @@ -458,7 +459,7 @@ static int get_decrypted_eps(uint8_t eps[PRIMARY_SEED_SIZE]) for (i = 0; i < PRIMARY_SEED_SIZE; i++) eps[i] ^= frk2[i]; - memset(frk2, 0, sizeof(frk2)); + always_memset(frk2, 0, sizeof(frk2)); return 1; } @@ -650,6 +651,6 @@ int tpm_endorse(void) result = 1; } while (0); - memset(eps, 0, sizeof(eps)); + always_memset(eps, 0, sizeof(eps)); return result; } diff --git a/board/cr50/tpm2/rsa.c b/board/cr50/tpm2/rsa.c index 52507c985f..70e14fba97 100644 --- a/board/cr50/tpm2/rsa.c +++ b/board/cr50/tpm2/rsa.c @@ -10,6 +10,8 @@ #include "dcrypto.h" #include "trng.h" +#include "cryptoc/util.h" + #include TPM2B_BYTE_VALUE(4); @@ -427,8 +429,7 @@ CRYPT_RESULT _cpri__GenerateKeyRSA( &counter)) { if (counter_in != NULL) *counter_in = counter; - /* TODO(ngm): implement secure memset. */ - memset(local_seed.t.buffer, 0, local_seed.t.size); + always_memset(local_seed.t.buffer, 0, local_seed.t.size); return CRYPT_FAIL; } @@ -438,8 +439,7 @@ CRYPT_RESULT _cpri__GenerateKeyRSA( &counter)) { if (counter_in != NULL) *counter_in = counter; - /* TODO(ngm): implement secure memset. */ - memset(local_seed.t.buffer, 0, local_seed.t.size); + always_memset(local_seed.t.buffer, 0, local_seed.t.size); return CRYPT_FAIL; } @@ -451,9 +451,8 @@ CRYPT_RESULT _cpri__GenerateKeyRSA( DCRYPTO_bn_mul(&N, &p, &q); reverse_tpm2b(N_buf); reverse_tpm2b(p_buf); - /* TODO(ngm): replace with secure memset. */ - memset(q_buf, 0, sizeof(q_buf)); - memset(local_seed.t.buffer, 0, local_seed.t.size); + always_memset(q_buf, 0, sizeof(q_buf)); + always_memset(local_seed.t.buffer, 0, local_seed.t.size); return CRYPT_SUCCESS; } diff --git a/chip/g/dcrypto/bn.c b/chip/g/dcrypto/bn.c index 17d1bae72e..b44280c2e0 100644 --- a/chip/g/dcrypto/bn.c +++ b/chip/g/dcrypto/bn.c @@ -8,6 +8,8 @@ #include "trng.h" +#include "cryptoc/util.h" + #include #ifdef CONFIG_WATCHDOG @@ -19,7 +21,7 @@ static inline void watchdog_reload(void) { } void bn_init(struct LITE_BIGNUM *b, void *buf, size_t len) { DCRYPTO_bn_wrap(b, buf, len); - dcrypto_memset(buf, 0x00, len); + always_memset(buf, 0x00, len); } void DCRYPTO_bn_wrap(struct LITE_BIGNUM *b, void *buf, size_t len) @@ -387,9 +389,9 @@ void bn_mont_modexp(struct LITE_BIGNUM *output, const struct LITE_BIGNUM *input, bn_add(output, N); /* Final reduce. */ output->dmax = N->dmax; - dcrypto_memset(RR_buf, 0, sizeof(RR_buf)); - dcrypto_memset(acc_buf, 0, sizeof(acc_buf)); - dcrypto_memset(aR_buf, 0, sizeof(aR_buf)); + always_memset(RR_buf, 0, sizeof(RR_buf)); + always_memset(acc_buf, 0, sizeof(acc_buf)); + always_memset(aR_buf, 0, sizeof(aR_buf)); } /* c[] += a * b[] */ @@ -1218,6 +1220,6 @@ int DCRYPTO_bn_generate_prime(struct LITE_BIGNUM *p) } } - memset(composites_buf, 0, sizeof(composites_buf)); + always_memset(composites_buf, 0, sizeof(composites_buf)); return 0; } diff --git a/chip/g/dcrypto/hkdf.c b/chip/g/dcrypto/hkdf.c index 9a647361ce..3afdc6b2eb 100644 --- a/chip/g/dcrypto/hkdf.c +++ b/chip/g/dcrypto/hkdf.c @@ -8,6 +8,7 @@ #include "internal.h" #include "cryptoc/sha256.h" +#include "cryptoc/util.h" static int hkdf_extract(uint8_t *PRK, const uint8_t *salt, size_t salt_len, const uint8_t *IKM, size_t IKM_len) @@ -77,6 +78,6 @@ int DCRYPTO_hkdf(uint8_t *OKM, size_t OKM_len, return 0; result = hkdf_expand(OKM, OKM_len, PRK, info, info_len); - memset(PRK, 0, sizeof(PRK)); + always_memset(PRK, 0, sizeof(PRK)); return result; } diff --git a/chip/g/dcrypto/hmac.c b/chip/g/dcrypto/hmac.c index 1ba2833a41..d6f2d4e775 100644 --- a/chip/g/dcrypto/hmac.c +++ b/chip/g/dcrypto/hmac.c @@ -9,6 +9,7 @@ #include #include "cryptoc/sha256.h" +#include "cryptoc/util.h" /* TODO(ngm): add support for hardware hmac. */ static void HMAC_init(LITE_HMAC_CTX *ctx, const void *key, unsigned int len) @@ -53,6 +54,6 @@ const uint8_t *DCRYPTO_HMAC_final(LITE_HMAC_CTX *ctx) DCRYPTO_SHA256_init(&ctx->hash, 0); HASH_update(&ctx->hash, ctx->opad, sizeof(ctx->opad)); HASH_update(&ctx->hash, digest, HASH_size(&ctx->hash)); - memset(&ctx->opad[0], 0, sizeof(ctx->opad)); /* wipe key */ + always_memset(&ctx->opad[0], 0, sizeof(ctx->opad)); /* wipe key */ return HASH_final(&ctx->hash); } diff --git a/chip/g/dcrypto/internal.h b/chip/g/dcrypto/internal.h index dc3abe860a..3189c22912 100644 --- a/chip/g/dcrypto/internal.h +++ b/chip/g/dcrypto/internal.h @@ -126,10 +126,4 @@ enum dcrypto_appid; /* Forward declaration. */ int dcrypto_ladder_compute_usr(enum dcrypto_appid id, const uint32_t usr_salt[8]); -/* - * Utility functions. - */ -/* TODO(ngm): memset that doesn't get optimized out. */ -#define dcrypto_memset(p, b, len) memset((p), (b), (len)) - #endif /* ! __EC_CHIP_G_DCRYPTO_INTERNAL_H */ diff --git a/chip/g/dcrypto/p256.c b/chip/g/dcrypto/p256.c index ab60c91cdc..665144e31b 100644 --- a/chip/g/dcrypto/p256.c +++ b/chip/g/dcrypto/p256.c @@ -6,6 +6,7 @@ #include "dcrypto.h" #include "cryptoc/p256.h" +#include "cryptoc/util.h" static const p256_int p256_one = P256_ONE; @@ -22,7 +23,7 @@ int DCRYPTO_p256_key_from_bytes(p256_int *x, p256_int *y, p256_int *d, if (p256_cmp(&SECP256r1_nMin2, &key) < 0) return 0; p256_add(&key, &p256_one, d); - dcrypto_memset(&key, 0, sizeof(key)); + always_memset(&key, 0, sizeof(key)); if (x == NULL || y == NULL) return 1; return dcrypto_p256_base_point_mul(d, x, y); diff --git a/chip/g/dcrypto/rsa.c b/chip/g/dcrypto/rsa.c index dc4a06a503..dc86006fae 100644 --- a/chip/g/dcrypto/rsa.c +++ b/chip/g/dcrypto/rsa.c @@ -13,6 +13,7 @@ #include "cryptoc/sha.h" #include "cryptoc/sha256.h" +#include "cryptoc/util.h" /* Extend the MSB throughout the word. */ static uint32_t msb_extend(uint32_t a) @@ -98,7 +99,7 @@ static int oaep_pad(uint8_t *output, uint32_t output_len, if (msg_len > output_len - 2 - 2 * hash_size) return 0; /* Input message too large for key size. */ - dcrypto_memset(output, 0, output_len); + always_memset(output, 0, output_len); for (i = 0; i < hash_size;) { uint32_t r = rand(); @@ -338,7 +339,7 @@ static int pkcs1_type1_pad(uint8_t *padded, uint32_t padded_len, *(padded++) = 0; *(padded++) = 1; - dcrypto_memset(padded, 0xFF, ps_len); + always_memset(padded, 0xFF, ps_len); padded += ps_len; *(padded++) = 0; memcpy(padded, der, der_size); @@ -550,8 +551,8 @@ int DCRYPTO_rsa_encrypt(struct RSA *rsa, uint8_t *out, uint32_t *out_len, reverse((uint8_t *) encrypted.d, bn_size(&encrypted)); *out_len = bn_size(&encrypted); - dcrypto_memset(padded_buf, 0, sizeof(padded_buf)); - dcrypto_memset(e_buf, 0, sizeof(e_buf)); + always_memset(padded_buf, 0, sizeof(padded_buf)); + always_memset(e_buf, 0, sizeof(e_buf)); return 1; } @@ -609,8 +610,8 @@ int DCRYPTO_rsa_decrypt(struct RSA *rsa, uint8_t *out, uint32_t *out_len, break; } - dcrypto_memset(encrypted_buf, 0, sizeof(encrypted_buf)); - dcrypto_memset(padded_buf, 0, sizeof(padded_buf)); + always_memset(encrypted_buf, 0, sizeof(encrypted_buf)); + always_memset(padded_buf, 0, sizeof(padded_buf)); return ret; } @@ -651,7 +652,7 @@ int DCRYPTO_rsa_sign(struct RSA *rsa, uint8_t *out, uint32_t *out_len, reverse((uint8_t *) signature.d, bn_size(&signature)); *out_len = bn_size(&rsa->N); - dcrypto_memset(padded_buf, 0, sizeof(padded_buf)); + always_memset(padded_buf, 0, sizeof(padded_buf)); return 1; } @@ -705,8 +706,8 @@ int DCRYPTO_rsa_verify(const struct RSA *rsa, const uint8_t *digest, break; } - dcrypto_memset(padded_buf, 0, sizeof(padded_buf)); - dcrypto_memset(signature_buf, 0, sizeof(signature_buf)); + always_memset(padded_buf, 0, sizeof(padded_buf)); + always_memset(signature_buf, 0, sizeof(signature_buf)); return ret; }