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 <ngm@google.com>
Reviewed-on: https://chromium-review.googlesource.com/501368
Commit-Ready: Nagendra Modadugu <ngm@google.com>
Tested-by: Nagendra Modadugu <ngm@google.com>
Reviewed-by: Vadim Bendebury <vbendeb@chromium.org>
Reviewed-by: Andrey Pronin <apronin@chromium.org>

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);

board/cr50/tpm2/endorsement.c

@@ -30,6 +30,7 @@
 #include "dcrypto.h"
 
 #include <cryptoc/sha256.h>
+#include <cryptoc/util.h>
 
 #include <endian.h>
 #include <string.h>
@@ -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;
 }

board/cr50/tpm2/rsa.c

@@ -10,6 +10,8 @@
 #include "dcrypto.h"
 #include "trng.h"
 
+#include "cryptoc/util.h"
+
 #include <assert.h>
 
 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;
 }
 

chip/g/dcrypto/bn.c

@@ -8,6 +8,8 @@
 
 #include "trng.h"
 
+#include "cryptoc/util.h"
+
 #include <assert.h>
 
 #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;
 }

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;
 }

chip/g/dcrypto/hmac.c

@@ -9,6 +9,7 @@
 #include <stdint.h>
 
 #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);
 }

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 */

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);

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;
 }