Revert "u2f: Append hmac of auth time secret to versioned KH"

This reverts commit d2627d12bb21308f49a72cadaf47a0a86730a960.

Reason for revert: Causing crbug.com/1111182

Original change's description:
> u2f: Append hmac of auth time secret to versioned KH
>
> When generating versioned KHs, u2fd should send a public derivative
> (sha256) of the user's auth time secret to cr50. Cr50 derives an
> hmac of it and appends this authorization_hmac to the KH.
>
> When signing versioned KHs, u2fd may supply the unhashed auth time
> secret. Cr50 will check the authorization_hmac if no power button press.
> If the reconstructed hmac matches authorization_hmac, power button press
> is waived.
>
> Currently for v1, we will just prepare the authorization_hmac but not
> enforce it. This is because fingerprint and PIN are unable to unlock
> the same secret.
>
> While we waive power button press for v1, we can enforce
> authorization_hmac whenever auth-time secrets is ready.
>
> BUG=b:144861739
> TEST=- Use a known 32-byte "auth-time secret"
>      - Compute the sha256 of the auth-time secret (this is public)
>      - u2f_generate with the computed "authTimeSecretHash"
>      - Add code to u2f_sign command handler such that cr50 computes
>        the sha256 of the supplied auth-time secret at u2f_sign time
>        and require power button press if the hmac doesn't match.
>      - u2f_sign with the true auth-time secret -> observe in logging
>        that hmac matches, and no power button press required.
>      - u2f_sign with a wrong auth-time secret -> observe in logging
>        that hmac doesn't match, and power button press is required
>        for signing.
>
> Cq-Depend: chromium:2321731
> Change-Id: Ib9ae913667f8178ac7a4790f861d7dada972c4a0
> Signed-off-by: Yicheng Li <yichengli@chromium.org>
> Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/2317047
> Reviewed-by: Andrey Pronin <apronin@chromium.org>
> Reviewed-by: Vadim Sukhomlinov <sukhomlinov@chromium.org>

Bug: b:144861739
Cq-Depend: chromium:2327779
Exempt-From-Owner-Approval: Causing crbug.com/1111182
Change-Id: I8c8a594d148b92556b20a2753aa1007cf2c1676b
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/2327358
Tested-by: Archie Pusaka <apusaka@chromium.org>
Reviewed-by: Yicheng Li <yichengli@chromium.org>
Reviewed-by: Archie Pusaka <apusaka@chromium.org>
Commit-Queue: Archie Pusaka <apusaka@chromium.org>

4 files changed, 34 insertions, 103 deletions

diff --git a/board/cr50/u2f.c b/board/cr50/u2f.c
index 8bca1d8271..7c6273823c 100644
--- a/board/cr50/u2f.c
+++ b/board/cr50/u2f.c
@@ -201,8 +201,7 @@ int u2f_origin_user_keyhandle(const uint8_t *origin, const uint8_t *user,
 
 int u2f_origin_user_versioned_keyhandle(
 	const uint8_t *origin, const uint8_t *user, const uint8_t *origin_seed,
-	uint8_t version,
-	struct u2f_versioned_key_handle_header *key_handle_header)
+	uint8_t version, struct u2f_versioned_key_handle *key_handle)
 {
 	LITE_HMAC_CTX ctx;
 	struct u2f_state *state = get_state();
@@ -210,17 +209,16 @@ int u2f_origin_user_versioned_keyhandle(
 	if (!state)
 		return EC_ERROR_UNKNOWN;
 
-	key_handle_header->version = version;
-	memcpy(key_handle_header->origin_seed, origin_seed, P256_NBYTES);
+	key_handle->version = version;
+	memcpy(key_handle->origin_seed, origin_seed, P256_NBYTES);
 
 	DCRYPTO_HMAC_SHA256_init(&ctx, state->salt_kek, SHA256_DIGEST_SIZE);
 	HASH_update(&ctx.hash, origin, P256_NBYTES);
 	HASH_update(&ctx.hash, user, P256_NBYTES);
 	HASH_update(&ctx.hash, origin_seed, P256_NBYTES);
-	HASH_update(&ctx.hash, &version, sizeof(key_handle_header->version));
+	HASH_update(&ctx.hash, &version, sizeof(key_handle->version));
 
-	memcpy(key_handle_header->kh_hmac, DCRYPTO_HMAC_final(&ctx),
-	       SHA256_DIGEST_SIZE);
+	memcpy(key_handle->hmac, DCRYPTO_HMAC_final(&ctx), SHA256_DIGEST_SIZE);
 
 	return EC_SUCCESS;
 }
@@ -252,24 +250,6 @@ int u2f_origin_user_keypair(const uint8_t *key_handle, size_t key_handle_size,
 	return EC_SUCCESS;
 }
 
-int u2f_authorization_hmac(const uint8_t *authorization_salt,
-			   const uint8_t *auth_time_secret_hash, uint8_t *hmac)
-{
-	LITE_HMAC_CTX ctx;
-	struct u2f_state *state = get_state();
-
-	if (!state)
-		return EC_ERROR_UNKNOWN;
-
-	DCRYPTO_HMAC_SHA256_init(&ctx, state->salt_kek, SHA256_DIGEST_SIZE);
-	HASH_update(&ctx.hash, authorization_salt, U2F_AUTHORIZATION_SALT_SIZE);
-	HASH_update(&ctx.hash, auth_time_secret_hash, P256_NBYTES);
-
-	memcpy(hmac, DCRYPTO_HMAC_final(&ctx), SHA256_DIGEST_SIZE);
-
-	return EC_SUCCESS;
-}
-
 int u2f_gen_kek(const uint8_t *origin, uint8_t *kek, size_t key_len)
 {
 	uint32_t buf[P256_NDIGITS];
diff --git a/common/u2f.c b/common/u2f.c
index 3f21187781..eaeb38b08c 100644
--- a/common/u2f.c
+++ b/common/u2f.c
@@ -10,7 +10,6 @@
 #include "cryptoc/sha256.h"
 #include "dcrypto.h"
 #include "extension.h"
-#include "fips_rand.h"
 #include "system.h"
 #include "u2f_impl.h"
 #include "u2f.h"
@@ -128,10 +127,8 @@ static enum vendor_cmd_rc u2f_generate(enum vendor_cmd_cc code, void *buf,
 		struct u2f_key_handle kh;
 		struct u2f_versioned_key_handle vkh;
 	} kh_buf;
-	size_t keypair_input_size =
-		(kh_version == 0) ?
-			sizeof(kh_buf.kh) :
-			sizeof(struct u2f_versioned_key_handle_header);
+	size_t kh_size = (kh_version == 0) ? sizeof(kh_buf.kh) :
+					     sizeof(kh_buf.vkh);
 
 	/* Whether key handle generation succeeded */
 	int generate_kh_rc;
@@ -171,14 +168,13 @@ static enum vendor_cmd_rc u2f_generate(enum vendor_cmd_cc code, void *buf,
 		else
 			generate_kh_rc = u2f_origin_user_versioned_keyhandle(
 				req->appId, req->userSecret, od_seed,
-				kh_version, &kh_buf.vkh.header);
+				kh_version, &kh_buf.vkh);
 
 		if (generate_kh_rc != EC_SUCCESS)
 			return VENDOR_RC_INTERNAL_ERROR;
 
 		generate_keypair_rc = u2f_origin_user_keypair(
-			(uint8_t *)&kh_buf, keypair_input_size, &od, &opk_x,
-			&opk_y);
+			(uint8_t *)&kh_buf, kh_size, &od, &opk_x, &opk_y);
 	} while (generate_keypair_rc == EC_ERROR_TRY_AGAIN);
 
 	if (generate_keypair_rc != EC_SUCCESS)
@@ -192,16 +188,6 @@ static enum vendor_cmd_rc u2f_generate(enum vendor_cmd_cc code, void *buf,
 		copy_kh_pubkey_out(&opk_x, &opk_y, &kh_buf.kh, buf);
 		*response_size = sizeof(struct u2f_generate_resp);
 	} else {
-		if (!fips_rand_bytes(kh_buf.vkh.authorization_salt,
-				     U2F_AUTHORIZATION_SALT_SIZE))
-			return VENDOR_RC_INTERNAL_ERROR;
-
-		if (u2f_authorization_hmac(kh_buf.vkh.authorization_salt,
-					   req->authTimeSecretHash,
-					   kh_buf.vkh.authorization_hmac) !=
-		    EC_SUCCESS)
-			return VENDOR_RC_INTERNAL_ERROR;
-
 		copy_versioned_kh_pubkey_out(&opk_x, &opk_y, &kh_buf.vkh, buf);
 		*response_size = sizeof(struct u2f_generate_versioned_resp);
 	}
@@ -256,14 +242,14 @@ static int verify_kh_owned(const uint8_t *user_secret, const uint8_t *app_id,
 	return rc;
 }
 
-static int verify_versioned_kh_owned(
-	const uint8_t *user_secret, const uint8_t *app_id,
-	const struct u2f_versioned_key_handle_header *key_handle_header,
-	int *owned)
+static int
+verify_versioned_kh_owned(const uint8_t *user_secret, const uint8_t *app_id,
+			  const struct u2f_versioned_key_handle *key_handle,
+			  int *owned)
 {
 	int rc;
 	/* Re-created key handle. */
-	struct u2f_versioned_key_handle_header recreated_kh_header;
+	struct u2f_versioned_key_handle recreated_kh;
 
 	/*
 	 * Re-create the key handle and compare against that which
@@ -272,13 +258,13 @@ static int verify_versioned_kh_owned(
 	 */
 
 	rc = u2f_origin_user_versioned_keyhandle(app_id, user_secret,
-						 key_handle_header->origin_seed,
-						 key_handle_header->version,
-						 &recreated_kh_header);
+						 key_handle->origin_seed,
+						 key_handle->version,
+						 &recreated_kh);
 
 	if (rc == EC_SUCCESS)
-		*owned = safe_memcmp(&recreated_kh_header, key_handle_header,
-				     sizeof(recreated_kh_header)) == 0;
+		*owned = safe_memcmp(&recreated_kh, key_handle,
+				     sizeof(recreated_kh)) == 0;
 
 	return rc;
 }
@@ -335,8 +321,8 @@ static enum vendor_cmd_rc u2f_sign(enum vendor_cmd_cc code, void *buf,
 	/* Version of KH; 0 if KH is not versioned. */
 	uint8_t version;
 
-	/* Size of the part of KH used to derive keypair, in bytes. */
-	size_t keypair_input_size;
+	/* Size of KH in bytes. */
+	size_t kh_size;
 
 	int verify_owned_rc;
 
@@ -348,19 +334,18 @@ static enum vendor_cmd_rc u2f_sign(enum vendor_cmd_cc code, void *buf,
 		key_handle = (uint8_t *)&req->keyHandle;
 		hash = req->hash;
 		flags = req->flags;
-		keypair_input_size = sizeof(struct u2f_key_handle);
+		kh_size = sizeof(struct u2f_key_handle);
 		verify_owned_rc = verify_kh_owned(req->userSecret, req->appId,
 						  &req->keyHandle, &kh_owned);
 	} else if (input_size == sizeof(struct u2f_sign_versioned_req)) {
-		version = req_versioned->keyHandle.header.version;
+		version = req_versioned->keyHandle.version;
 		key_handle = (uint8_t *)&req_versioned->keyHandle;
 		hash = req_versioned->hash;
 		flags = req_versioned->flags;
-		keypair_input_size =
-			sizeof(struct u2f_versioned_key_handle_header);
+		kh_size = sizeof(struct u2f_versioned_key_handle);
 		verify_owned_rc = verify_versioned_kh_owned(
 			req_versioned->userSecret, req_versioned->appId,
-			&req_versioned->keyHandle.header, &kh_owned);
+			&req_versioned->keyHandle, &kh_owned);
 	} else {
 		return VENDOR_RC_BOGUS_ARGS;
 	}
@@ -393,28 +378,17 @@ static enum vendor_cmd_rc u2f_sign(enum vendor_cmd_cc code, void *buf,
 	if ((flags & U2F_AUTH_CHECK_ONLY) == U2F_AUTH_CHECK_ONLY)
 		return VENDOR_RC_SUCCESS;
 
-	/*
-	 * Enforce user presence for version 0 KHs, with optional consume.
-	 */
-	if (pop_check_presence(flags & G2F_CONSUME) != POP_TOUCH_YES) {
-		if (version != U2F_KH_VERSION_1)
-			return VENDOR_RC_NOT_ALLOWED;
-		if ((flags & U2F_AUTH_FLAG_TUP) != 0)
-			return VENDOR_RC_NOT_ALLOWED;
-		/*
-		 * TODO(yichengli): When auth-time secrets is ready, enforce
-		 * authorization hmac when no power button press.
-		 */
-	}
+	/* Always enforce user presence, with optional consume. */
+	if (pop_check_presence(flags & G2F_CONSUME) != POP_TOUCH_YES)
+		return VENDOR_RC_NOT_ALLOWED;
 
 	/* Re-create origin-specific key. */
 	if (legacy_kh) {
 		if (u2f_origin_key(legacy_origin_seed, &origin_d) != EC_SUCCESS)
 			return VENDOR_RC_INTERNAL_ERROR;
 	} else {
-		if (u2f_origin_user_keypair(key_handle, keypair_input_size,
-					    &origin_d, NULL,
-					    NULL) != EC_SUCCESS)
+		if (u2f_origin_user_keypair(key_handle, kh_size, &origin_d,
+					    NULL, NULL) != EC_SUCCESS)
 			return VENDOR_RC_INTERNAL_ERROR;
 	}
 
diff --git a/include/u2f.h b/include/u2f.h
index 3454ba82e8..61b9677185 100644
--- a/include/u2f.h
+++ b/include/u2f.h
@@ -53,24 +53,15 @@ struct u2f_ec_point {
 
 #define U2F_KH_VERSION_1 0x01
 
-#define U2F_AUTHORIZATION_SALT_SIZE 16
-
 struct u2f_key_handle {
 	uint8_t origin_seed[U2F_P256_SIZE];
 	uint8_t hmac[U2F_P256_SIZE];
 };
 
-struct u2f_versioned_key_handle_header {
+struct u2f_versioned_key_handle {
 	uint8_t version;
 	uint8_t origin_seed[U2F_P256_SIZE];
-	uint8_t kh_hmac[U2F_P256_SIZE];
-};
-
-struct u2f_versioned_key_handle {
-	struct u2f_versioned_key_handle_header header;
-	/* Optionally checked in u2f_sign. */
-	uint8_t authorization_salt[U2F_AUTHORIZATION_SALT_SIZE];
-	uint8_t authorization_hmac[U2F_P256_SIZE];
+	uint8_t hmac[U2F_P256_SIZE];
 };
 
 /* TODO(louiscollard): Add Descriptions. */
@@ -79,11 +70,6 @@ struct u2f_generate_req {
 	uint8_t appId[U2F_APPID_SIZE]; /* Application id */
 	uint8_t userSecret[U2F_P256_SIZE];
 	uint8_t flags;
-	/*
-	 * If generating versioned KH, derive an hmac from it and append to
-	 * the key handle. Otherwise unused.
-	 */
-	uint8_t authTimeSecretHash[U2F_P256_SIZE];
 };
 
 struct u2f_generate_resp {
@@ -107,7 +93,6 @@ struct u2f_sign_req {
 struct u2f_sign_versioned_req {
 	uint8_t appId[U2F_APPID_SIZE]; /* Application id */
 	uint8_t userSecret[U2F_P256_SIZE];
-	uint8_t authTimeSecret[U2F_P256_SIZE];
 	uint8_t hash[U2F_P256_SIZE];
 	uint8_t flags;
 	struct u2f_versioned_key_handle keyHandle;
diff --git a/include/u2f_impl.h b/include/u2f_impl.h
index 9b66c348bf..5bd69309c6 100644
--- a/include/u2f_impl.h
+++ b/include/u2f_impl.h
@@ -76,14 +76,13 @@ int u2f_origin_user_keyhandle(const uint8_t *origin, const uint8_t *user,
  * @param user pointer to user secret
  * @param seed pointer to origin-specific random seed
  * @param version the version byte to pack; should be greater than 0.
- * @param key_handle_header buffer to hold the output key handle header
+ * @param key_handle buffer to hold the output key handle
  *
  * @return EC_SUCCESS if a valid keypair was created.
  */
 int u2f_origin_user_versioned_keyhandle(
 	const uint8_t *origin, const uint8_t *user, const uint8_t *seed,
-	uint8_t version,
-	struct u2f_versioned_key_handle_header *key_handle_header);
+	uint8_t version, struct u2f_versioned_key_handle *key_handle);
 
 /**
  * Generate an origin and user-specific ECDSA keypair from the specified
@@ -102,13 +101,6 @@ int u2f_origin_user_versioned_keyhandle(
 int u2f_origin_user_keypair(const uint8_t *key_handle, size_t key_handle_size,
 			    p256_int *d, p256_int *pk_x, p256_int *pk_y);
 
-/**
- * Derive an hmac from the given salt and hash. The seed is to make sure the
- * hmac is different for different key handles of one user.
- */
-int u2f_authorization_hmac(const uint8_t *authorization_salt,
-			   const uint8_t *auth_time_secret_hash, uint8_t *hmac);
-
 /***
  * Generate a hardware derived 256b private key.
  *