u2f: do not commit state changes on TPM command context.

g2f_attestation_cert() is another function which is invoked on the TPM
command context, when virtual TPM NVMEM spaces are read.

One of the side effects of invoking of g2f_attestation_cert() is the
creation of the U2F state, if it did not exist before. In this case
the state should not be immediately committed to the NVMEM, the commit
will happen when the TPM command execution is completed.

BUG=b:199981251
TEST=running ./test/tpm_test/tpmtest.py does not trigger the 'attempt
     to commit in unlocked state' message any more.
     'make buildall' and 'make CRYTPO_TEST=1 BOARD=cr50' pass

Signed-off-by: Vadim Bendebury <vbendeb@chromium.org>
Change-Id: I708e8807ffd3207cc6ab84a0e380908e715f7a15
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/3482487
Reviewed-by: Mary Ruthven <mruthven@chromium.org>

6 files changed, 48 insertions, 13 deletions

diff --git a/board/cr50/dcrypto/u2f_impl.h b/board/cr50/dcrypto/u2f_impl.h
index 9003db4a03..8e6b4a0575 100644
--- a/board/cr50/dcrypto/u2f_impl.h
+++ b/board/cr50/dcrypto/u2f_impl.h
@@ -186,11 +186,23 @@ enum ec_error_list u2f_attest(const struct u2f_state *state,
 /**
  * Get the current u2f state from the board.
  *
+ * Create a state if one does not exist, commit the created state into NVMEM.
+ *
  * @return pointer to static state if successful, NULL otherwise
  */
 struct u2f_state *u2f_get_state(void);
 
 /**
+ * Get the current u2f state from the board.
+ *
+ * Create a state if one does not exist, do not force committing of the
+ * created state into NVMEM.
+ *
+ * @return pointer to static state if successful, NULL otherwise
+ */
+struct u2f_state *u2f_get_state_no_commit(void);
+
+/**
  * Try to load U2F keys or create if failed.
  *
  * @param state - buffer for state to load/create
@@ -198,7 +210,8 @@ struct u2f_state *u2f_get_state(void);
  *
  * @return true if state is properly initialized and will persist in flash.
  */
-bool u2f_load_or_create_state(struct u2f_state *state, bool force_create);
+bool u2f_load_or_create_state(struct u2f_state *state, bool force_create,
+			      bool commit);
 
 /***
  * Generates and persists to nvram a new key that will be used to
diff --git a/board/cr50/fips_cmd.c b/board/cr50/fips_cmd.c
index c37766eba9..8e66c36c92 100644
--- a/board/cr50/fips_cmd.c
+++ b/board/cr50/fips_cmd.c
@@ -81,7 +81,7 @@ static void print_u2f_keys_status(void)
 
 	hmac_len = read_tpm_nvmem_size(TPM_HIDDEN_U2F_KEK);
 	drbg_len = read_tpm_nvmem_size(TPM_HIDDEN_U2F_KH_SALT);
-	load_result = u2f_load_or_create_state(&state, false);
+	load_result = u2f_load_or_create_state(&state, false, false);
 
 	CPRINTS("U2F HMAC len: %u, U2F Entropy len: %u, U2F load:%u, "
 		"State DRBG len:%u", hmac_len,
diff --git a/board/cr50/u2f_state_load.c b/board/cr50/u2f_state_load.c
index 8e92199bb7..4f602b9029 100644
--- a/board/cr50/u2f_state_load.c
+++ b/board/cr50/u2f_state_load.c
@@ -19,7 +19,8 @@ static const uint8_t k_salt_deprecated = NVMEM_VAR_U2F_SALT;
 
 #define CPRINTF(format, args...) cprintf(CC_EXTENSION, format, ##args)
 
-bool u2f_load_or_create_state(struct u2f_state *state, bool force_create)
+bool u2f_load_or_create_state(struct u2f_state *state, bool force_create,
+			      bool commit)
 {
 	bool g2f_secret_was_created = false;
 
@@ -62,7 +63,7 @@ bool u2f_load_or_create_state(struct u2f_state *state, bool force_create)
 
 		if (write_tpm_nvmem_hidden(
 			    TPM_HIDDEN_U2F_KEK, sizeof(state->hmac_key),
-			    state->hmac_key, 1 /* commit */) == TPM_WRITE_FAIL)
+			    state->hmac_key, commit) == TPM_WRITE_FAIL)
 			return false;
 	}
 
@@ -92,10 +93,9 @@ bool u2f_load_or_create_state(struct u2f_state *state, bool force_create)
 		if (!g2f_secret_was_created)
 			state->drbg_entropy_size = 32;
 
-		if (write_tpm_nvmem_hidden(TPM_HIDDEN_U2F_KH_SALT,
-					   state->drbg_entropy_size,
-					   state->drbg_entropy,
-					   1 /* commit */) == TPM_WRITE_FAIL) {
+		if (write_tpm_nvmem_hidden(
+			    TPM_HIDDEN_U2F_KH_SALT, state->drbg_entropy_size,
+			    state->drbg_entropy, commit) == TPM_WRITE_FAIL) {
 			state->drbg_entropy_size = 0;
 			return false;
 		}
@@ -126,12 +126,23 @@ bool u2f_load_or_create_state(struct u2f_state *state, bool force_create)
 static bool u2f_state_loaded;
 static struct u2f_state u2f_state;
 
+static struct u2f_state *u2f_get_state_common(bool commit)
+{
+	if (!u2f_state_loaded) {
+		u2f_state_loaded =
+			u2f_load_or_create_state(&u2f_state, false, commit);
+	}
+	return u2f_state_loaded ? &u2f_state : NULL;
+}
+
 struct u2f_state *u2f_get_state(void)
 {
-	if (!u2f_state_loaded)
-		u2f_state_loaded = u2f_load_or_create_state(&u2f_state, false);
+	return u2f_get_state_common(true);
+}
 
-	return u2f_state_loaded ? &u2f_state : NULL;
+struct u2f_state *u2f_get_state_no_commit(void)
+{
+	return u2f_get_state_common(false);
 }
 
 enum ec_error_list u2f_gen_kek_seed(void)
@@ -193,7 +204,8 @@ enum ec_error_list u2f_update_keys(void)
 	if (!state || state->drbg_entropy_size != sizeof(state->drbg_entropy)) {
 		result = u2f_zeroize_keys();
 		/* Force creation of new keys. */
-		u2f_state_loaded = u2f_load_or_create_state(&u2f_state, true);
+		u2f_state_loaded =
+			u2f_load_or_create_state(&u2f_state, true, true);
 
 		/* try to load again */
 		state = u2f_get_state();
diff --git a/common/u2f.c b/common/u2f.c
index d446887423..cccc6b06b9 100644
--- a/common/u2f.c
+++ b/common/u2f.c
@@ -25,7 +25,7 @@ size_t g2f_attestation_cert(uint8_t *buf)
 {
 	uint8_t *serial;
 
-	const struct u2f_state *state = u2f_get_state();
+	const struct u2f_state *state = u2f_get_state_no_commit();
 
 	if (!state)
 		return 0;
diff --git a/fuzz/u2f_fuzz.cc b/fuzz/u2f_fuzz.cc
index 0c1a682375..6f11212c84 100644
--- a/fuzz/u2f_fuzz.cc
+++ b/fuzz/u2f_fuzz.cc
@@ -83,6 +83,11 @@ struct u2f_state *u2f_get_state(void)
 	return &ustate;
 }
 
+struct u2f_state *u2f_get_state_no_commit(void)
+{
+	return &ustate;
+}
+
 static enum touch_state tstate;
 enum touch_state pop_check_presence(int consume)
 {
diff --git a/test/u2f.c b/test/u2f.c
index c99dc7d631..baa58bf489 100644
--- a/test/u2f.c
+++ b/test/u2f.c
@@ -90,6 +90,11 @@ struct u2f_state *u2f_get_state(void)
 	return &state;
 }
 
+struct u2f_state *u2f_get_state_no_commit(void)
+{
+	return u2f_get_state();
+}
+
 enum touch_state pop_check_presence(int consume)
 {
 	enum touch_state ret = presence ? POP_TOUCH_YES : POP_TOUCH_NO;