cr50: added DCRYPTO_p256_is_valid_point() to public API.

To cleanly split internal API in internal.h from external API in
dcrypto.h we need to add missing DCRYPTO_p256_is_valid_point().

While adding this switch to enum dcrypto_result for both internal and
external versions.

Added check that provided point is valid to DCRYPTO_p256_point_mul() as
important security precaution. Currently this check is still in
tpm2/ecc.c, but it will be removed in next CLs with switching to
enum dcrypto_result.

Added comments on input parameters and behavior.

BUG=b:134594373
TEST=make BOARD=cr50; test/tpm_test/tpmtest.py; TCG tests
-------------------------- Test Result Summary -------------------------
  Test executed on:		Tue Sep 14 18:24:10 2021
  Performed Tests:		248
  Passed Tests:			248
  Failed Tests:			0
  Errors:			0
  Warnings:			0
========================================================================

Signed-off-by: Vadim Sukhomlinov <sukhomlinov@google.com>
Change-Id: I4637f7b61b5a502854d9cad03e8e603529278873
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/3161507
Reviewed-by: Vadim Sukhomlinov <sukhomlinov@chromium.org>
Reviewed-by: Andrey Pronin <apronin@chromium.org>
Tested-by: Vadim Sukhomlinov <sukhomlinov@chromium.org>
Commit-Queue: Vadim Sukhomlinov <sukhomlinov@chromium.org>

6 files changed, 44 insertions, 22 deletions

diff --git a/board/cr50/dcrypto/dcrypto.h b/board/cr50/dcrypto/dcrypto.h
index accb2f3a43..d65bbea0c6 100644
--- a/board/cr50/dcrypto/dcrypto.h
+++ b/board/cr50/dcrypto/dcrypto.h
@@ -235,14 +235,33 @@ int DCRYPTO_rsa_key_compute(struct LITE_BIGNUM *N, struct LITE_BIGNUM *d,
  *  EC.
  */
 
+/**
+ * Check if point is on NIST P-256 curve
+ *
+ * @param x point coordinate
+ * @param y point coordinate
+ * @return DCRYPTO_OK if (x,y) is a valid point, DCRYPTO_FAIL otherwise
+ */
+enum dcrypto_result DCRYPTO_p256_is_valid_point(const p256_int *x,
+						const p256_int *y);
+
 /* DCRYPTO_p256_base_point_mul sets {out_x,out_y} = nG, where n is < the
  * order of the group.
  */
 int DCRYPTO_p256_base_point_mul(p256_int *out_x, p256_int *out_y,
 				const p256_int *n);
 
-/* DCRYPTO_p256_point_mul sets {out_x,out_y} = n*{in_x,in_y}, where n is <
- * the order of the group.
+/**
+ * DCRYPTO_p256_point_mul sets {out_x,out_y} = n*{in_x,in_y}, where n is <
+ * the order of the group. Prior to computation check than {in_x,in_y} is
+ * on NIST P-256 curve.
+ *
+ * @param out_x output shared coordinate x
+ * @param out_y output shared coordinate y
+ * @param n private key
+ * @param in_x input public point x
+ * @param in_y input public point y
+ * @return 1 if success
  */
 int DCRYPTO_p256_point_mul(p256_int *out_x, p256_int *out_y, const p256_int *n,
 			   const p256_int *in_x, const p256_int *in_y);
diff --git a/board/cr50/dcrypto/dcrypto_p256.c b/board/cr50/dcrypto/dcrypto_p256.c
index 02c4edd9eb..841259cef7 100644
--- a/board/cr50/dcrypto/dcrypto_p256.c
+++ b/board/cr50/dcrypto/dcrypto_p256.c
@@ -282,7 +282,8 @@ int dcrypto_p256_ecdsa_verify(const p256_int *key_x, const p256_int *key_y,
 	return result == 0;
 }
 
-int dcrypto_p256_is_valid_point(const p256_int *x, const p256_int *y)
+enum dcrypto_result dcrypto_p256_is_valid_point(const p256_int *x,
+						const p256_int *y)
 {
 	int i, result;
 
@@ -299,5 +300,5 @@ int dcrypto_p256_is_valid_point(const p256_int *x, const p256_int *y)
 		result |= (dmem_ecc->r.a[i] ^ dmem_ecc->s.a[i]);
 
 	dcrypto_unlock();
-	return result == 0;
+	return dcrypto_ok_if_zero(result);
 }
diff --git a/board/cr50/dcrypto/internal.h b/board/cr50/dcrypto/internal.h
index d681a1c1fb..ef092f6fb5 100644
--- a/board/cr50/dcrypto/internal.h
+++ b/board/cr50/dcrypto/internal.h
@@ -246,7 +246,8 @@ int dcrypto_p256_ecdsa_verify(const p256_int *key_x, const p256_int *key_y,
 		const p256_int *message, const p256_int *r,
 		const p256_int *s)
 	__attribute__((warn_unused_result));
-int dcrypto_p256_is_valid_point(const p256_int *x, const p256_int *y)
+enum dcrypto_result dcrypto_p256_is_valid_point(const p256_int *x,
+						const p256_int *y)
 	__attribute__((warn_unused_result));
 
 /* Wipe content of rnd with pseudo-random values. */
diff --git a/board/cr50/dcrypto/p256_ec.c b/board/cr50/dcrypto/p256_ec.c
index fe0f69c92b..b681d7ddef 100644
--- a/board/cr50/dcrypto/p256_ec.c
+++ b/board/cr50/dcrypto/p256_ec.c
@@ -23,18 +23,23 @@ int DCRYPTO_p256_base_point_mul(p256_int *out_x, p256_int *out_y,
 	return dcrypto_p256_base_point_mul(n, out_x, out_y);
 }
 
+enum dcrypto_result DCRYPTO_p256_is_valid_point(const p256_int *x,
+						const p256_int *y)
+{
+	return dcrypto_p256_is_valid_point(x, y);
+}
+
 /* DCRYPTO_p256_point_mul sets {out_x,out_y} = n*{in_x,in_y}, where n is <
  * the order of the group. */
-int DCRYPTO_p256_point_mul(p256_int *out_x, p256_int *out_y,
-			const p256_int *n, const p256_int *in_x,
-			const p256_int *in_y)
+int DCRYPTO_p256_point_mul(p256_int *out_x, p256_int *out_y, const p256_int *n,
+			   const p256_int *in_x, const p256_int *in_y)
 {
-	if (p256_is_zero(n) != 0) {
+	if (p256_is_zero(n) != 0 ||
+	    (dcrypto_p256_is_valid_point(in_x, in_y) != DCRYPTO_OK)) {
 		p256_clear(out_x);
 		p256_clear(out_y);
 		return 0;
 	}
-
 	return dcrypto_p256_point_mul(n, in_x, in_y, out_x, out_y);
 }
 
diff --git a/board/cr50/dcrypto/p256_ecies.c b/board/cr50/dcrypto/p256_ecies.c
index 8a140b1de6..1edef5f67d 100644
--- a/board/cr50/dcrypto/p256_ecies.c
+++ b/board/cr50/dcrypto/p256_ecies.c
@@ -55,7 +55,7 @@ size_t DCRYPTO_ecies_encrypt(
 					&eph_d, pub_x, pub_y))
 		return 0;
 	/* Check for computational errors. */
-	if (!dcrypto_p256_is_valid_point(&secret_x, &secret_y))
+	if (dcrypto_p256_is_valid_point(&secret_x, &secret_y) != DCRYPTO_OK)
 		return 0;
 	/* Convert secret to big-endian. */
 	reverse(&secret_x, sizeof(secret_x));
@@ -136,15 +136,14 @@ size_t DCRYPTO_ecies_decrypt(
 	p256_from_bin(inp, &eph_y);
 	inp += P256_NBYTES;
 
-	/* Verify that the public point is on the curve. */
-	if (!dcrypto_p256_is_valid_point(&eph_x, &eph_y))
-		return 0;
-	/* Compute the DH point. */
-	if (!DCRYPTO_p256_point_mul(&secret_x, &secret_y,
-					d, &eph_x, &eph_y))
+	/**
+	 * Verify that the public point is on the curve and compute the DH
+	 * point.
+	 */
+	if (!DCRYPTO_p256_point_mul(&secret_x, &secret_y, d, &eph_x, &eph_y))
 		return 0;
 	/* Check for computational errors. */
-	if (!dcrypto_p256_is_valid_point(&secret_x, &secret_y))
+	if (!dcrypto_p256_is_valid_point(&secret_x, &secret_y) != DCRYPTO_OK)
 		return 0;
 	/* Convert secret to big-endian. */
 	reverse(&secret_x, sizeof(secret_x));
diff --git a/board/cr50/tpm2/ecc.c b/board/cr50/tpm2/ecc.c
index 8f9392af28..18e3ca7980 100644
--- a/board/cr50/tpm2/ecc.c
+++ b/board/cr50/tpm2/ecc.c
@@ -19,7 +19,6 @@ TPM2B_BYTE_VALUE(32);
 
 BOOL _cpri__EccIsPointOnCurve(TPM_ECC_CURVE curve_id, TPMS_ECC_POINT *q)
 {
-	int result;
 	p256_int x, y;
 
 	switch (curve_id) {
@@ -29,9 +28,7 @@ BOOL _cpri__EccIsPointOnCurve(TPM_ECC_CURVE curve_id, TPMS_ECC_POINT *q)
 		    !p256_from_be_bin_size(q->y.b.buffer, q->y.b.size, &y))
 			return FALSE;
 
-		result = dcrypto_p256_is_valid_point(&x, &y);
-
-		if (result)
+		if (DCRYPTO_p256_is_valid_point(&x, &y) == DCRYPTO_OK)
 			return TRUE;
 		else
 			return FALSE;