CR50: add a constant time buffer equals implementation

Various cryptographic operations leak timing
information if comparisons are not executed
in constant time.  This change adds DCRYPTO_equals(),
a constant runtime comparator.

Also replace crypto related callsites that used
memcmp() as a binary comparator.

BUG=none
BRANCH=none
TEST=tcg tests pass

Change-Id: I3d3da3c0524c3a349d60675902d1f2d338ad455f
Signed-off-by: nagendra modadugu <ngm@google.com>
Reviewed-on: https://chromium-review.googlesource.com/410163
Commit-Ready: Nagendra Modadugu <ngm@google.com>
Tested-by: Nagendra Modadugu <ngm@google.com>
Reviewed-by: Marius Schilder <mschilder@chromium.org>
Reviewed-by: Bill Richardson <wfrichar@chromium.org>

5 files changed, 32 insertions, 7 deletions

diff --git a/chip/g/build.mk b/chip/g/build.mk
index 5c31358439..bb0573958b 100644
--- a/chip/g/build.mk
+++ b/chip/g/build.mk
@@ -31,6 +31,7 @@ endif
 chip-$(CONFIG_DCRYPTO)+= dcrypto/aes.o
 chip-$(CONFIG_DCRYPTO)+= dcrypto/bn.o
 chip-$(CONFIG_DCRYPTO)+= dcrypto/bn_hw.o
+chip-$(CONFIG_DCRYPTO)+= dcrypto/compare.o
 chip-$(CONFIG_DCRYPTO)+= dcrypto/dcrypto_runtime.o
 chip-$(CONFIG_DCRYPTO)+= dcrypto/hkdf.o
 chip-$(CONFIG_DCRYPTO)+= dcrypto/hmac.o
diff --git a/chip/g/dcrypto/compare.c b/chip/g/dcrypto/compare.c
new file mode 100644
index 0000000000..db6193752b
--- /dev/null
+++ b/chip/g/dcrypto/compare.c
@@ -0,0 +1,20 @@
+/* Copyright 2016 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.
+ */
+
+#include "dcrypto.h"
+
+/* Constant time comparator. */
+int 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;
+
+	for (i = 0; i < len; i++)
+		diff |= pa[i] ^ pb[i];
+
+	return !diff;
+}
diff --git a/chip/g/dcrypto/dcrypto.h b/chip/g/dcrypto/dcrypto.h
index 4f4c2df5ff..cfe24ff0f6 100644
--- a/chip/g/dcrypto/dcrypto.h
+++ b/chip/g/dcrypto/dcrypto.h
@@ -179,4 +179,9 @@ int DCRYPTO_bn_div(struct LITE_BIGNUM *quotient, struct LITE_BIGNUM *remainder,
 int DCRYPTO_x509_verify(const uint8_t *cert, size_t len,
 			const struct RSA *ca_pub_key);
 
+/*
+ * Memory related functions.
+ */
+int DCRYPTO_equals(const void *a, const void *b, size_t len);
+
 #endif  /* ! __EC_CHIP_G_DCRYPTO_DCRYPTO_H */
diff --git a/chip/g/dcrypto/p256_ecies.c b/chip/g/dcrypto/p256_ecies.c
index 8272014495..8164ff87db 100644
--- a/chip/g/dcrypto/p256_ecies.c
+++ b/chip/g/dcrypto/p256_ecies.c
@@ -161,9 +161,8 @@ size_t DCRYPTO_ecies_decrypt(
 	hmac_key = &key[AES_KEY_BYTES];
 	DCRYPTO_HMAC_SHA256_init(&ctx, hmac_key, HMAC_KEY_BYTES);
 	HASH_update(&ctx.hash, inp, in_len);
-	/* TODO(ngm): replace with constant time verify. */
-	if (memcmp(inp + in_len, DCRYPTO_HMAC_final(&ctx),
-			SHA256_DIGEST_SIZE) != 0)
+	if (!DCRYPTO_equals(inp + in_len, DCRYPTO_HMAC_final(&ctx),
+				SHA256_DIGEST_SIZE))
 		return 0;
 
 	memmove(outp, inp, auth_data_len);
diff --git a/chip/g/dcrypto/rsa.c b/chip/g/dcrypto/rsa.c
index ace9961169..45ec414dc7 100644
--- a/chip/g/dcrypto/rsa.c
+++ b/chip/g/dcrypto/rsa.c
@@ -137,7 +137,7 @@ static int check_oaep_pad(uint8_t *out, uint32_t *out_len,
 		DCRYPTO_SHA256_init(&ctx, 0);
 	HASH_update(&ctx, label, label ? strlen(label) + 1 : 0);
 
-	bad = memcmp(phash, HASH_final(&ctx), hash_size);
+	bad = !DCRYPTO_equals(phash, HASH_final(&ctx), hash_size);
 	bad |= padded[0];
 
 	for (i = PS - padded; i <  padded_len; i++) {
@@ -295,10 +295,10 @@ static int check_pkcs1_type1_pad(const uint8_t *msg, uint32_t msg_len,
 
 	if (padded[i++] != 0)
 		return 0;
-	if (memcmp(&padded[i], der, der_size) != 0)
+	if (!DCRYPTO_equals(&padded[i], der, der_size))
 		return 0;
 	i += der_size;
-	return memcmp(msg, &padded[i], hash_size) == 0;
+	return DCRYPTO_equals(msg, &padded[i], hash_size);
 }
 
 /* sign */
@@ -398,7 +398,7 @@ 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 |= memcmp(padded + db_len, HASH_final(&ctx), hash_size);
+	bad |= !DCRYPTO_equals(padded + db_len, HASH_final(&ctx), hash_size);
 	return !bad;
 }