cr50: improve g2f implementation

Replace int to size_t in DCRYPTO_x509_* functions to indicate that
returned value is actually a size.

Replaced int to enum dcrypto_result and removed arithmetic on enum in
DCRYPTO_x509_gen_u2f_cert_name() to make code clear.

Added intermediate variable certificate_len in GetG2fCert to make logic
clear. However, virtual nvmem requires further refactoring to replace
void with size_t to escalate errors if any.

Added check that G2F certificate is not all zeroes in tpm_test.py

BUG=b:212517336
TEST=test/tpm_test.py

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

6 files changed, 39 insertions, 28 deletions

diff --git a/board/cr50/dcrypto/dcrypto.h b/board/cr50/dcrypto/dcrypto.h
index cef877969a..0dc0af78f0 100644
--- a/board/cr50/dcrypto/dcrypto.h
+++ b/board/cr50/dcrypto/dcrypto.h
@@ -949,30 +949,34 @@ enum dcrypto_result DCRYPTO_x509_verify(const uint8_t *cert, size_t len,
 
 /* Generate U2F Certificate and sign it
  * Use ECDSA with NIST P-256 curve, and SHA2-256 digest
- * @param d: key handle, used for NIST SP 800-90A HMAC DRBG
+ * @param d: private key to use
  * @param pk_x, pk_y: public key
  * @param serial: serial number for certificate
  * @param name: certificate issuer and subject
  * @param cert: output buffer for certificate
  * @param n: max size of cert
+ *
+ * @returns size of certificate or 0 if failure
  */
-int DCRYPTO_x509_gen_u2f_cert_name(const p256_int *d, const p256_int *pk_x,
-				   const p256_int *pk_y, const p256_int *serial,
-				   const char *name, uint8_t *cert,
-				   const int n);
+size_t DCRYPTO_x509_gen_u2f_cert_name(const p256_int *d, const p256_int *pk_x,
+				      const p256_int *pk_y,
+				      const p256_int *serial, const char *name,
+				      uint8_t *cert, const size_t n);
 
 /* Generate U2F Certificate with DCRYPTO_x509_gen_u2f_cert_name
  * Providing certificate issuer as BOARD or U2F
- * @param d: key handle, used for NIST SP 800-90A HMAC DRBG
+ * @param d: private key to use
  * @param pk_x, pk_y: public key
  * @param serial: serial number for certificate
  * @param name: certificate issuer and subject
  * @param cert: output buffer for certificate
  * @param n: max size of cert
+ *
+ * @returns size of certificate or 0 if failure
  */
-int DCRYPTO_x509_gen_u2f_cert(const p256_int *d, const p256_int *pk_x,
-			      const p256_int *pk_y, const p256_int *serial,
-			      uint8_t *cert, const int n);
+size_t DCRYPTO_x509_gen_u2f_cert(const p256_int *d, const p256_int *pk_x,
+				 const p256_int *pk_y, const p256_int *serial,
+				 uint8_t *cert, const size_t n);
 
 /*
  * Memory related functions.
diff --git a/board/cr50/dcrypto/x509.c b/board/cr50/dcrypto/x509.c
index 4417f1701b..1ffefceb94 100644
--- a/board/cr50/dcrypto/x509.c
+++ b/board/cr50/dcrypto/x509.c
@@ -421,15 +421,16 @@ static void add_common_name(struct asn1 *ctx, const char *cname)
 	SEQ_END(*ctx);
 }
 
-int DCRYPTO_x509_gen_u2f_cert_name(const p256_int *d, const p256_int *pk_x,
-				   const p256_int *pk_y, const p256_int *serial,
-				   const char *name, uint8_t *cert, const int n)
+size_t DCRYPTO_x509_gen_u2f_cert_name(const p256_int *d, const p256_int *pk_x,
+				      const p256_int *pk_y,
+				      const p256_int *serial, const char *name,
+				      uint8_t *cert, const size_t n)
 {
 	struct asn1 ctx = {cert, 0};
 	struct sha256_ctx sha;
 	p256_int h, r, s;
 	struct drbg_ctx drbg;
-	int result;
+	enum dcrypto_result result;
 
 	SEQ_START(ctx, V_SEQ, SEQ_LARGE) {  /* outer seq */
 	/*
@@ -519,9 +520,9 @@ int DCRYPTO_x509_gen_u2f_cert_name(const p256_int *d, const p256_int *pk_x,
 	SHA256_update(&sha, body, (ctx.p + ctx.n) - body);
 	p256_from_bin(SHA256_final(&sha)->b8, &h);
 	hmac_drbg_init_rfc6979(&drbg, d, &h);
-	result = dcrypto_p256_ecdsa_sign(&drbg, d, &h, &r, &s) - DCRYPTO_OK;
+	result = dcrypto_p256_ecdsa_sign(&drbg, d, &h, &r, &s);
 	drbg_exit(&drbg);
-	if (result)
+	if (result != DCRYPTO_OK)
 		return 0;
 
 	/* Append X509 signature */
@@ -539,9 +540,9 @@ int DCRYPTO_x509_gen_u2f_cert_name(const p256_int *d, const p256_int *pk_x,
 	return ctx.n;
 }
 
-int DCRYPTO_x509_gen_u2f_cert(const p256_int *d, const p256_int *pk_x,
-			      const p256_int *pk_y, const p256_int *serial,
-			      uint8_t *cert, const int n)
+size_t DCRYPTO_x509_gen_u2f_cert(const p256_int *d, const p256_int *pk_x,
+				 const p256_int *pk_y, const p256_int *serial,
+				 uint8_t *cert, const size_t n)
 {
 	return DCRYPTO_x509_gen_u2f_cert_name(d, pk_x, pk_y, serial,
 					      serial ? STRINGIFY(BOARD) : "U2F",
diff --git a/board/cr50/tpm2/virtual_nvmem.c b/board/cr50/tpm2/virtual_nvmem.c
index 196f4c0416..aa4ce95724 100644
--- a/board/cr50/tpm2/virtual_nvmem.c
+++ b/board/cr50/tpm2/virtual_nvmem.c
@@ -290,11 +290,14 @@ BUILD_ASSERT(VIRTUAL_NV_INDEX_SN_DATA_SIZE == sizeof(struct sn_data));
 static void GetG2fCert(BYTE *to, size_t offset, size_t size)
 {
 	uint8_t cert[G2F_ATTESTATION_CERT_MAX_LEN] = { 0 };
+	size_t certificate_len;
 
-	if (!g2f_attestation_cert(cert))
-		memset(cert, 0, G2F_ATTESTATION_CERT_MAX_LEN);
+	certificate_len = g2f_attestation_cert(cert);
 
-	memcpy(to, ((BYTE *) cert) + offset, size);
+	if (certificate_len == 0)
+		memset(cert, 0, sizeof(cert));
+
+	memcpy(to, ((BYTE *)cert) + offset, size);
 }
 BUILD_ASSERT(VIRTUAL_NV_INDEX_G2F_CERT_SIZE == G2F_ATTESTATION_CERT_MAX_LEN);
 
diff --git a/fuzz/u2f_fuzz.cc b/fuzz/u2f_fuzz.cc
index dfb46c966a..0c1a682375 100644
--- a/fuzz/u2f_fuzz.cc
+++ b/fuzz/u2f_fuzz.cc
@@ -30,9 +30,10 @@ int system_get_chip_unique_id(uint8_t **id)
 /******************************************************************************/
 /* Mock implementations of Dcrypto functionality.
  */
-int DCRYPTO_x509_gen_u2f_cert_name(const p256_int *d, const p256_int *pk_x,
-				   const p256_int *pk_y, const p256_int *serial,
-				   const char *name, uint8_t *cert, const int n)
+size_t DCRYPTO_x509_gen_u2f_cert_name(const p256_int *d, const p256_int *pk_x,
+				      const p256_int *pk_y,
+				      const p256_int *serial, const char *name,
+				      uint8_t *cert, const size_t n)
 {
 	memset(cert, 1, n);
 	return n;
diff --git a/test/tpm_test/u2f_test.py b/test/tpm_test/u2f_test.py
index 97ca4a4141..a0118c7b13 100644
--- a/test/tpm_test/u2f_test.py
+++ b/test/tpm_test/u2f_test.py
@@ -102,9 +102,10 @@ def g2f_get_cert(tpm):
       ]
     g2f_read_cmd = bytes(g2f_read)
     response = tpm.command(g2f_read_cmd)
-    if len(response) <= 10:
+    if len(response) <= 16 or response.count(0) > 100:
          raise subcmd.TpmTestError('Unexpected G2F response: '
                                        + utils.hex_dump(response))
+
     print('G2F cert len', len(response))
     return response
 
diff --git a/test/u2f.c b/test/u2f.c
index 3ddf38616b..c99dc7d631 100644
--- a/test/u2f.c
+++ b/test/u2f.c
@@ -41,9 +41,10 @@ bool fips_trng_bytes(void *buffer, size_t len)
 	return true;
 }
 
-int DCRYPTO_x509_gen_u2f_cert_name(const p256_int *d, const p256_int *pk_x,
-				   const p256_int *pk_y, const p256_int *serial,
-				   const char *name, uint8_t *cert, const int n)
+size_t DCRYPTO_x509_gen_u2f_cert_name(const p256_int *d, const p256_int *pk_x,
+				      const p256_int *pk_y,
+				      const p256_int *serial, const char *name,
+				      uint8_t *cert, const size_t n)
 {
 	/* Return the size of certificate, 0 means error. */
 	return 0;