fpsensor: Implement command to read positive_match_secret.

Add EC command to read positive_match_secret on match success.
If the attempt to read is 5 seconds after the match, the read is not
allowed (the readable bit for positive match secret is cleared).
Test that the command can read the data correctly and can
read for each finger only once. Test that attempt to read secret
after deadline will be rejected.

BRANCH=nocturne
BUG=chromium:927095
TEST=make buildall
TEST=tested enrollment, matching and multifinger on DUT nocturne
TEST=tested that if biod requests to download template and secret
for a finger that's not currently matched, reading secret will fail.

Change-Id: Idc734c6392d271e2aaee1cddf7c2c5b81b727b4a
Signed-off-by: Yicheng Li <yichengli@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/1679372
Reviewed-by: Nicolas Norvez <norvez@chromium.org>

9 files changed, 343 insertions, 10 deletions

diff --git a/common/fpsensor/fpsensor.c b/common/fpsensor/fpsensor.c
index 707ed1feab..5377b526b6 100644
--- a/common/fpsensor/fpsensor.c
+++ b/common/fpsensor/fpsensor.c
@@ -21,7 +21,6 @@
 #include "system.h"
 #include "task.h"
 #include "trng.h"
-#include "timer.h"
 #include "util.h"
 #include "watchdog.h"
 
@@ -52,7 +51,6 @@ static uint32_t matching_time_us;
 static uint32_t overall_time_us;
 static timestamp_t overall_t0;
 static uint8_t timestamps_invalid;
-static int8_t template_matched;
 
 BUILD_ASSERT(sizeof(struct ec_fp_template_encryption_metadata) % 4 == 0);
 
@@ -130,20 +128,21 @@ static uint32_t fp_process_match(void)
 	timestamp_t t0 = get_time();
 	int res = -1;
 	uint32_t updated = 0;
-	int32_t fgr = -1;
+	int32_t fgr = FP_NO_SUCH_TEMPLATE;
 
 	/* match finger against current templates */
-	template_matched = -1;
+	fp_disable_positive_match_secret(&positive_match_secret_state);
 	CPRINTS("Matching/%d ...", templ_valid);
 	if (templ_valid) {
 		res = fp_finger_match(fp_template[0], templ_valid, fp_buffer,
 				      &fgr, &updated);
 		CPRINTS("Match =>%d (finger %d)", res, fgr);
-		if (res < 0) {
+		if (res < 0 || fgr < 0 || fgr >= FP_MAX_FINGER_COUNT) {
 			res = EC_MKBP_FP_ERR_MATCH_NO_INTERNAL;
 			timestamps_invalid |= FPSTATS_MATCHING_INV;
 		} else {
-			template_matched = (int8_t)fgr;
+			fp_enable_positive_match_secret(fgr,
+				&positive_match_secret_state);
 		}
 		if (res == EC_MKBP_FP_ERR_MATCH_YES_UPDATED)
 			templ_dirty |= updated;
@@ -458,7 +457,11 @@ static enum ec_status fp_command_stats(struct host_cmd_handler_args *args)
 	r->overall_t0.lo = overall_t0.le.lo;
 	r->overall_t0.hi = overall_t0.le.hi;
 	r->timestamps_invalid = timestamps_invalid;
-	r->template_matched = template_matched;
+	/*
+	 * Note that this is set to FP_NO_SUCH_TEMPLATE when positive match
+	 * secret is read/disabled, and we are not using this field in biod.
+	 */
+	r->template_matched = positive_match_secret_state.template_matched;
 
 	args->response_size = sizeof(*r);
 	return EC_RES_SUCCESS;
diff --git a/common/fpsensor/fpsensor_state.c b/common/fpsensor/fpsensor_state.c
index 7618dbd859..82a548b8af 100644
--- a/common/fpsensor/fpsensor_state.c
+++ b/common/fpsensor/fpsensor_state.c
@@ -7,6 +7,7 @@
 #include "cryptoc/util.h"
 #include "ec_commands.h"
 #include "fpsensor.h"
+#include "fpsensor_crypto.h"
 #include "fpsensor_private.h"
 #include "fpsensor_state.h"
 #include "host_command.h"
@@ -30,6 +31,13 @@ uint8_t fp_enc_buffer[FP_ALGORITHM_ENCRYPTED_TEMPLATE_SIZE]
 /* Salt used in derivation of positive match secret. */
 uint8_t fp_positive_match_salt
 	[FP_MAX_FINGER_COUNT][FP_POSITIVE_MATCH_SALT_BYTES];
+
+struct positive_match_secret_state positive_match_secret_state = {
+	.template_matched = FP_NO_SUCH_TEMPLATE,
+	.readable = false,
+	.deadline.val = 0,
+};
+
 /* Number of used templates */
 uint32_t templ_valid;
 /* Bitmap of the templates with local modifications */
@@ -74,6 +82,7 @@ static void _fp_clear_context(void)
 	always_memset(fp_buffer, 0, sizeof(fp_buffer));
 	always_memset(fp_enc_buffer, 0, sizeof(fp_enc_buffer));
 	always_memset(user_id, 0, sizeof(user_id));
+	fp_disable_positive_match_secret(&positive_match_secret_state);
 	for (idx = 0; idx < FP_MAX_FINGER_COUNT; idx++)
 		fp_clear_finger_context(idx);
 }
@@ -231,3 +240,72 @@ static enum ec_status fp_command_context(struct host_cmd_handler_args *args)
 	return EC_RES_INVALID_PARAM;
 }
 DECLARE_HOST_COMMAND(EC_CMD_FP_CONTEXT, fp_command_context, EC_VER_MASK(1));
+
+int fp_enable_positive_match_secret(uint32_t fgr,
+				    struct positive_match_secret_state *state)
+{
+	timestamp_t now;
+
+	if (state->readable) {
+		CPRINTS("Error: positive match secret already readable.");
+		fp_disable_positive_match_secret(state);
+		return EC_ERROR_UNKNOWN;
+	}
+
+	now = get_time();
+	state->template_matched = fgr;
+	state->readable = true;
+	state->deadline.val = now.val + (5 * SECOND);
+	return EC_SUCCESS;
+}
+
+void fp_disable_positive_match_secret(
+	struct positive_match_secret_state *state)
+{
+	state->template_matched = FP_NO_SUCH_TEMPLATE;
+	state->readable = false;
+	state->deadline.val = 0;
+}
+
+static enum ec_status fp_command_read_match_secret(
+	struct host_cmd_handler_args *args)
+{
+	const struct ec_params_fp_read_match_secret *params = args->params;
+	struct ec_response_fp_read_match_secret *response = args->response;
+	int8_t fgr = params->fgr;
+	timestamp_t now = get_time();
+	struct positive_match_secret_state state_copy
+		= positive_match_secret_state;
+
+	fp_disable_positive_match_secret(&positive_match_secret_state);
+
+	if (fgr < 0 || fgr >= FP_MAX_FINGER_COUNT) {
+		CPRINTS("Invalid finger number %d", fgr);
+		return EC_RES_INVALID_PARAM;
+	}
+	if (timestamp_expired(state_copy.deadline, &now)) {
+		CPRINTS("Reading positive match secret disallowed: "
+			"deadline has passed.");
+		return EC_RES_TIMEOUT;
+	}
+	if (fgr != state_copy.template_matched || !state_copy.readable) {
+		CPRINTS("Positive match secret for finger %d is not meant to "
+			"be read now.", fgr);
+		return EC_RES_ACCESS_DENIED;
+	}
+
+	if (derive_positive_match_secret(response->positive_match_secret,
+					 fp_positive_match_salt[fgr])
+		!= EC_SUCCESS) {
+		CPRINTS("Failed to derive positive match secret for finger %d",
+			fgr);
+		/* Keep the template and encryption salt. */
+		return EC_RES_ERROR;
+	}
+	CPRINTS("Derived positive match secret for finger %d", fgr);
+	args->response_size = sizeof(*response);
+
+	return EC_RES_SUCCESS;
+}
+DECLARE_HOST_COMMAND(EC_CMD_FP_READ_MATCH_SECRET, fp_command_read_match_secret,
+		     EC_VER_MASK(0));
diff --git a/common/mock/build.mk b/common/mock/build.mk
index e89c658968..56ef715cf7 100644
--- a/common/mock/build.mk
+++ b/common/mock/build.mk
@@ -6,3 +6,4 @@
 
 mock-$(HAS_MOCK_FPSENSOR) += fpsensor_mock.o
 mock-$(HAS_MOCK_ROLLBACK) += rollback_mock.o
+mock-$(HAS_MOCK_TIMER) += timer_mock.o
diff --git a/common/mock/timer_mock.c b/common/mock/timer_mock.c
new file mode 100644
index 0000000000..21408b7fe6
--- /dev/null
+++ b/common/mock/timer_mock.c
@@ -0,0 +1,18 @@
+/* Copyright 2019 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 "mock/timer_mock.h"
+
+static timestamp_t now;
+
+void set_time(timestamp_t now_)
+{
+	now = now_;
+}
+
+timestamp_t get_time(void)
+{
+	return now;
+};
diff --git a/include/ec_commands.h b/include/ec_commands.h
index 2f83b713d3..f28b9ea3e2 100644
--- a/include/ec_commands.h
+++ b/include/ec_commands.h
@@ -6023,6 +6023,17 @@ struct ec_response_fp_encryption_status {
 	uint32_t status;
 } __ec_align4;
 
+#define EC_CMD_FP_READ_MATCH_SECRET 0x040A
+struct ec_params_fp_read_match_secret {
+	uint16_t fgr;
+} __ec_align4;
+
+/* The positive match secret has the length of the SHA256 digest. */
+#define FP_POSITIVE_MATCH_SECRET_BYTES 32
+struct ec_response_fp_read_match_secret {
+	uint8_t positive_match_secret[FP_POSITIVE_MATCH_SECRET_BYTES];
+} __ec_align4;
+
 /*****************************************************************************/
 /* Touchpad MCU commands: range 0x0500-0x05FF */
 
diff --git a/include/fpsensor_state.h b/include/fpsensor_state.h
index 63ebaa465d..624318837d 100644
--- a/include/fpsensor_state.h
+++ b/include/fpsensor_state.h
@@ -8,10 +8,12 @@
 #ifndef __CROS_EC_FPSENSOR_STATE_H
 #define __CROS_EC_FPSENSOR_STATE_H
 
+#include <stdbool.h>
 #include <stdint.h>
 #include "common.h"
 #include "ec_commands.h"
 #include "link_defs.h"
+#include "timer.h"
 
 /* if no special memory regions are defined, fallback on regular SRAM */
 #ifndef FP_FRAME_SECTION
@@ -33,7 +35,6 @@
 #define FP_ALGORITHM_TEMPLATE_SIZE 0
 #define FP_MAX_FINGER_COUNT 5
 #endif
-#define FP_POSITIVE_MATCH_SECRET_BYTES 32
 #define SBP_ENC_KEY_LEN 16
 #define FP_ALGORITHM_ENCRYPTED_TEMPLATE_SIZE \
 	(FP_ALGORITHM_TEMPLATE_SIZE + \
@@ -44,6 +45,8 @@
 #define TASK_EVENT_SENSOR_IRQ     TASK_EVENT_CUSTOM_BIT(0)
 #define TASK_EVENT_UPDATE_CONFIG  TASK_EVENT_CUSTOM_BIT(1)
 
+#define FP_NO_SUCH_TEMPLATE -1
+
 /* --- Global variables defined in fpsensor_state.c --- */
 
 /* Last acquired frame (aligned as it is used by arbitrary binary libraries) */
@@ -73,6 +76,17 @@ extern uint32_t fp_events;
 
 extern uint32_t sensor_mode;
 
+struct positive_match_secret_state {
+	/* Index of the most recently matched template. */
+	int8_t template_matched;
+	/* Flag indicating positive match secret can be read. */
+	bool readable;
+	/* Deadline to read positive match secret. */
+	timestamp_t deadline;
+};
+
+extern struct positive_match_secret_state positive_match_secret_state;
+
 /* Simulation for unit tests. */
 void fp_task_simulate(void);
 
@@ -112,4 +126,22 @@ int fp_tpm_seed_is_set(void);
  */
 int fp_set_sensor_mode(uint32_t mode, uint32_t *mode_output);
 
+/**
+ * Allow reading positive match secret for |fgr| in the next 5 seconds.
+ *
+ * @param fgr the index of template to enable positive match secret.
+ * @param state the state of positive match secret, e.g. readable or not.
+ * @return EC_SUCCESS if the request is valid, error code otherwise.
+ */
+int fp_enable_positive_match_secret(uint32_t fgr,
+	struct positive_match_secret_state *state);
+
+/**
+ * Disallow positive match secret for any finger to be read.
+ *
+ * @param state the state of positive match secret, e.g. readable or not.
+ */
+void fp_disable_positive_match_secret(
+	struct positive_match_secret_state *state);
+
 #endif /* __CROS_EC_FPSENSOR_STATE_H */
diff --git a/include/mock/timer_mock.h b/include/mock/timer_mock.h
new file mode 100644
index 0000000000..04dc01e9ab
--- /dev/null
+++ b/include/mock/timer_mock.h
@@ -0,0 +1,15 @@
+/* Copyright 2019 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.
+ */
+
+#ifndef __MOCK_TIMER_MOCK_H
+#define __MOCK_TIMER_MOCK_H
+
+#include "timer.h"
+
+void set_time(timestamp_t now_);
+
+timestamp_t get_time(void);
+
+#endif  /* __MOCK_TIMER_MOCK_H */
diff --git a/test/fpsensor.c b/test/fpsensor.c
index b31aa3b259..962295afbf 100644
--- a/test/fpsensor.c
+++ b/test/fpsensor.c
@@ -8,6 +8,7 @@
 #include "fpsensor_crypto.h"
 #include "fpsensor_state.h"
 #include "host_command.h"
+#include "mock/timer_mock.h"
 #include "test_util.h"
 #include "util.h"
 
@@ -38,7 +39,8 @@ static const uint8_t fake_user_id[] = {
 };
 
 /*
- * |expected_positive_match_secret| is obtained by running BoringSSL locally.
+ * |expected_positive_match_secret_for_empty_user_id| is obtained by running
+ * BoringSSL locally.
  * From https://boringssl.googlesource.com/boringssl
  * commit 365b7a0fcbf273b1fa704d151059e419abd6cfb8
  *
@@ -601,6 +603,172 @@ test_static int test_fp_set_sensor_mode(void)
 	return EC_SUCCESS;
 }
 
+test_static int test_enable_positive_match_secret(void)
+{
+	struct positive_match_secret_state dumb_state = {
+		.template_matched = FP_NO_SUCH_TEMPLATE,
+		.readable = false,
+		.deadline.val = 0,
+	};
+	timestamp_t now = get_time();
+
+	TEST_ASSERT(fp_enable_positive_match_secret(0, &dumb_state) ==
+		EC_SUCCESS);
+	TEST_ASSERT(dumb_state.template_matched == 0);
+	TEST_ASSERT(dumb_state.readable == true);
+	TEST_ASSERT(dumb_state.deadline.val == now.val + (5 * SECOND));
+
+	/* Trying to enable again before reading secret should fail. */
+	TEST_ASSERT(fp_enable_positive_match_secret(0, &dumb_state) ==
+		EC_ERROR_UNKNOWN);
+	TEST_ASSERT(dumb_state.template_matched == FP_NO_SUCH_TEMPLATE);
+	TEST_ASSERT(dumb_state.readable == false);
+	TEST_ASSERT(dumb_state.deadline.val == 0);
+
+	return EC_SUCCESS;
+}
+
+test_static int test_disable_positive_match_secret(void)
+{
+	struct positive_match_secret_state dumb_state;
+
+	TEST_ASSERT(fp_enable_positive_match_secret(0, &dumb_state) ==
+		EC_SUCCESS);
+	fp_disable_positive_match_secret(&dumb_state);
+	TEST_ASSERT(dumb_state.template_matched == FP_NO_SUCH_TEMPLATE);
+	TEST_ASSERT(dumb_state.readable == false);
+	TEST_ASSERT(dumb_state.deadline.val == 0);
+
+	return EC_SUCCESS;
+}
+
+test_static int test_command_read_match_secret(void)
+{
+	int rv;
+	struct ec_params_fp_read_match_secret params;
+	struct ec_response_fp_read_match_secret resp;
+	timestamp_t now = get_time();
+
+	/* Invalid finger index should be rejected. */
+	params.fgr = FP_NO_SUCH_TEMPLATE;
+	rv = test_send_host_command(EC_CMD_FP_READ_MATCH_SECRET, 0, &params,
+				    sizeof(params), NULL, 0);
+	TEST_ASSERT(rv == EC_RES_INVALID_PARAM);
+	params.fgr = FP_MAX_FINGER_COUNT;
+	rv = test_send_host_command(EC_CMD_FP_READ_MATCH_SECRET, 0, &params,
+				    sizeof(params), NULL, 0);
+	TEST_ASSERT(rv == EC_RES_INVALID_PARAM);
+
+	memset(&resp, 0, sizeof(resp));
+	/* GIVEN that finger index is valid. */
+	params.fgr = 0;
+
+	/* GIVEN that positive match secret is enabled. */
+	fp_enable_positive_match_secret(params.fgr,
+					&positive_match_secret_state);
+
+	/* GIVEN that salt is non-trivial. */
+	memcpy(fp_positive_match_salt[0], fake_positive_match_salt,
+	       sizeof(fp_positive_match_salt[0]));
+	/* THEN reading positive match secret should succeed. */
+	rv = test_send_host_command(EC_CMD_FP_READ_MATCH_SECRET, 0, &params,
+				    sizeof(params), &resp, sizeof(resp));
+	if (rv != EC_RES_SUCCESS) {
+		ccprintf("%s:%s(): rv = %d\n", __FILE__, __func__, rv);
+		return -1;
+	}
+	/* AND the readable bit should be cleared after the read. */
+	TEST_ASSERT(positive_match_secret_state.readable == false);
+
+	TEST_ASSERT_ARRAY_EQ(
+		resp.positive_match_secret,
+		expected_positive_match_secret_for_empty_user_id,
+		sizeof(expected_positive_match_secret_for_empty_user_id));
+
+	/*
+	 * Now try reading secret again.
+	 * EVEN IF the deadline has not passed.
+	 */
+	positive_match_secret_state.deadline.val = now.val + 1 * SECOND;
+	rv = test_send_host_command(EC_CMD_FP_READ_MATCH_SECRET, 0, &params,
+				    sizeof(params), NULL, 0);
+	/*
+	 * This time the command should fail because the
+	 * fp_pos_match_secret_readable bit is cleared when the secret was read
+	 * the first time.
+	 */
+	TEST_ASSERT(rv == EC_RES_ACCESS_DENIED);
+
+	return EC_SUCCESS;
+}
+
+test_static int test_command_read_match_secret_wrong_finger(void)
+{
+	int rv;
+	struct ec_params_fp_read_match_secret params;
+
+	/* GIVEN that the finger is not the matched or enrolled finger. */
+	params.fgr = 0;
+	/*
+	 * GIVEN that positive match secret is enabled for a different
+	 * finger.
+	 */
+	fp_enable_positive_match_secret(params.fgr + 1,
+					&positive_match_secret_state);
+
+	/* Reading secret will fail. */
+	rv = test_send_host_command(EC_CMD_FP_READ_MATCH_SECRET, 0, &params,
+				    sizeof(params), NULL, 0);
+	TEST_ASSERT(rv == EC_RES_ACCESS_DENIED);
+	return EC_SUCCESS;
+}
+
+test_static int test_command_read_match_secret_timeout(void)
+{
+	int rv;
+	struct ec_params_fp_read_match_secret params;
+
+	params.fgr = 0;
+	/* GIVEN that the read is too late. */
+	fp_enable_positive_match_secret(params.fgr,
+					&positive_match_secret_state);
+	set_time(positive_match_secret_state.deadline);
+
+	/* EVEN IF encryption salt is non-trivial. */
+	memcpy(fp_positive_match_salt[0], fake_positive_match_salt,
+	       sizeof(fp_positive_match_salt[0]));
+	/* Reading secret will fail. */
+	rv = test_send_host_command(EC_CMD_FP_READ_MATCH_SECRET, 0, &params,
+				    sizeof(params), NULL, 0);
+	TEST_ASSERT(rv == EC_RES_TIMEOUT);
+	return EC_SUCCESS;
+}
+
+test_static int test_command_read_match_secret_unreadable(void)
+{
+	int rv;
+	struct ec_params_fp_read_match_secret params;
+
+	params.fgr = 0;
+	/* GIVEN that the readable bit is not set. */
+	fp_enable_positive_match_secret(params.fgr,
+					&positive_match_secret_state);
+	positive_match_secret_state.readable = false;
+
+	/* EVEN IF the finger is just matched. */
+	TEST_ASSERT(positive_match_secret_state.template_matched
+		== params.fgr);
+
+	/* EVEN IF encryption salt is non-trivial. */
+	memcpy(fp_positive_match_salt[0], fake_positive_match_salt,
+	       sizeof(fp_positive_match_salt[0]));
+	/* Reading secret will fail. */
+	rv = test_send_host_command(EC_CMD_FP_READ_MATCH_SECRET, 0, &params,
+				    sizeof(params), NULL, 0);
+	TEST_ASSERT(rv == EC_RES_ACCESS_DENIED);
+	return EC_SUCCESS;
+}
+
 void run_test(void)
 {
 	/* These are independent of global state. */
@@ -622,6 +790,12 @@ void run_test(void)
 	RUN_TEST(test_derive_new_pos_match_secret);
 	RUN_TEST(test_derive_positive_match_secret_fail_rollback_fail);
 	RUN_TEST(test_derive_positive_match_secret_fail_salt_trivial);
+	RUN_TEST(test_enable_positive_match_secret);
+	RUN_TEST(test_disable_positive_match_secret);
+	RUN_TEST(test_command_read_match_secret);
+	RUN_TEST(test_command_read_match_secret_wrong_finger);
+	RUN_TEST(test_command_read_match_secret_timeout);
+	RUN_TEST(test_command_read_match_secret_unreadable);
 
 	test_print_result();
 }
diff --git a/test/fpsensor.mocklist b/test/fpsensor.mocklist
index 9bed932526..9ddc52a0d1 100644
--- a/test/fpsensor.mocklist
+++ b/test/fpsensor.mocklist
@@ -4,4 +4,5 @@
  */
 
  #define CONFIG_TEST_MOCK_LIST \
-        MOCK(FPSENSOR)
+        MOCK(FPSENSOR)	       \
+	MOCK(TIMER)