From 3a3d6c36902f1570887b2322075e64a682ebe296 Mon Sep 17 00:00:00 2001 From: Bobby Casey Date: Wed, 17 Aug 2022 22:42:49 -0400 Subject: test: Add test for fpsensor trivial key failure Add a test for derive_positive_match_secret when compute_hmac_sha256 returns a trivial key (either 0x00 or 0xFF). This required mocking compute_hmac_sha256 to get the proper test behavior. Since other tests depend upon a proper SHA256 hash to be generated, either all key/message combinations would need to be mocked or some calls to compute_hmac_sha256 need to be passed through to hmac_SHA256. The latter option was chosen for the best flexibility moving forward. BRANCH=none BUG=b:242720910 TEST=make runhosttests Signed-off-by: Bobby Casey Change-Id: I959e23cfadb5460e62af90ffba74a0cd3b9d9a7f Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/3838935 Reviewed-by: Tom Hughes --- common/fpsensor/fpsensor_crypto.c | 6 ++- common/mock/build.mk | 1 + common/mock/fpsensor_crypto_mock.c | 63 ++++++++++++++++++++++++++ include/mock/fpsensor_crypto_mock.h | 30 +++++++++++++ test/fpsensor.mocklist | 1 + test/fpsensor_crypto.c | 89 +++++++++++++++++++++++++++++++++++++ 6 files changed, 188 insertions(+), 2 deletions(-) create mode 100644 common/mock/fpsensor_crypto_mock.c create mode 100644 include/mock/fpsensor_crypto_mock.h diff --git a/common/fpsensor/fpsensor_crypto.c b/common/fpsensor/fpsensor_crypto.c index ad1de6eeeb..8279e0d3d3 100644 --- a/common/fpsensor/fpsensor_crypto.c +++ b/common/fpsensor/fpsensor_crypto.c @@ -44,8 +44,10 @@ static int get_ikm(uint8_t *ikm) return EC_SUCCESS; } -void compute_hmac_sha256(uint8_t *output, const uint8_t *key, const int key_len, - const uint8_t *message, const int message_len) +test_mockable void compute_hmac_sha256(uint8_t *output, const uint8_t *key, + const int key_len, + const uint8_t *message, + const int message_len) { hmac_SHA256(output, key, key_len, message, message_len); } diff --git a/common/mock/build.mk b/common/mock/build.mk index fd87f40990..c34cb9efe4 100644 --- a/common/mock/build.mk +++ b/common/mock/build.mk @@ -8,6 +8,7 @@ mock-$(HAS_MOCK_ADC) += adc_mock.o mock-$(HAS_MOCK_BATTERY) += battery_mock.o mock-$(HAS_MOCK_CHARGE_MANAGER) += charge_manager_mock.o mock-$(HAS_MOCK_FP_SENSOR) += fp_sensor_mock.o +mock-$(HAS_MOCK_FPSENSOR_CRYPTO) += fpsensor_crypto_mock.o mock-$(HAS_MOCK_FPSENSOR_DETECT) += fpsensor_detect_mock.o mock-$(HAS_MOCK_FPSENSOR_STATE) += fpsensor_state_mock.o mock-$(HAS_MOCK_MKBP_EVENTS) += mkbp_events_mock.o diff --git a/common/mock/fpsensor_crypto_mock.c b/common/mock/fpsensor_crypto_mock.c new file mode 100644 index 0000000000..6d26d9c8dc --- /dev/null +++ b/common/mock/fpsensor_crypto_mock.c @@ -0,0 +1,63 @@ +/* Copyright 2022 The ChromiumOS Authors. + * Use of this source code is governed by a BSD-style license that can be + * found in the LICENSE file. + */ + +/** + * @file fpsensor_crypto_mock.c + * @brief Mock fpsensor_crypto library + */ +#include "sha256.h" +#include +#include +#include +#include "assert.h" +#include "compile_time_macros.h" +#include "console.h" +#include "ec_commands.h" +#include "fpsensor_private.h" +#include "mock/fpsensor_crypto_mock.h" + +#ifndef TEST_BUILD +#error "Mocks should only be in the test build." +#endif + +struct mock_ctrl_fpsensor_crypto mock_ctrl_fpsensor_crypto = + MOCK_CTRL_DEFAULT_FPSENSOR_CRYPTO; + +#define MESSAGE_ZERO_IDX 0 +#define MESSAGE_FF_IDX 1 +typedef uint8_t key_message_pair[FP_POSITIVE_MATCH_SECRET_BYTES]; + +key_message_pair fake_fpsensor_crypto[] = { + { 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 }, + { 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF } +}; + +BUILD_ASSERT(sizeof(key_message_pair) == FP_POSITIVE_MATCH_SECRET_BYTES); + +/* Mock compute_hmac_sha256 for unit or fuzz tests. */ +void compute_hmac_sha256(uint8_t *output, const uint8_t *key, const int key_len, + const uint8_t *message, const int message_len) +{ + switch (mock_ctrl_fpsensor_crypto.output_type) { + case MOCK_CTRL_FPSENSOR_CRYPTO_SHA256_TYPE_REAL: + hmac_SHA256(output, key, key_len, message, message_len); + break; + case MOCK_CTRL_FPSENSOR_CRYPTO_SHA256_TYPE_ZEROS: + memcpy(output, fake_fpsensor_crypto[MESSAGE_ZERO_IDX], + FP_POSITIVE_MATCH_SECRET_BYTES); + break; + case MOCK_CTRL_FPSENSOR_CRYPTO_SHA256_TYPE_FF: + memcpy(output, fake_fpsensor_crypto[MESSAGE_FF_IDX], + FP_POSITIVE_MATCH_SECRET_BYTES); + break; + default: + assert(0); + break; + }; +} diff --git a/include/mock/fpsensor_crypto_mock.h b/include/mock/fpsensor_crypto_mock.h new file mode 100644 index 0000000000..f3ca84734e --- /dev/null +++ b/include/mock/fpsensor_crypto_mock.h @@ -0,0 +1,30 @@ +/* Copyright 2022 The ChromiumOS Authors. + * Use of this source code is governed by a BSD-style license that can be + * found in the LICENSE file. + */ + +/** + * @file fpsensor_crypto_mock.h + * @brief Controls for the mock fpsensor_crypto library + */ + +#ifndef __MOCK_FPSENSOR_CRYPTO_MOCK_H +#define __MOCK_FPSENSOR_CRYPTO_MOCK_H + +enum mock_ctrl_fpsensor_crypto_sha256_type { + MOCK_CTRL_FPSENSOR_CRYPTO_SHA256_TYPE_REAL, + MOCK_CTRL_FPSENSOR_CRYPTO_SHA256_TYPE_ZEROS, + MOCK_CTRL_FPSENSOR_CRYPTO_SHA256_TYPE_FF, +}; + +struct mock_ctrl_fpsensor_crypto { + enum mock_ctrl_fpsensor_crypto_sha256_type output_type; +}; + +#define MOCK_CTRL_DEFAULT_FPSENSOR_CRYPTO \ + ((struct mock_ctrl_fpsensor_crypto){ \ + .output_type = MOCK_CTRL_FPSENSOR_CRYPTO_SHA256_TYPE_REAL }) + +extern struct mock_ctrl_fpsensor_crypto mock_ctrl_fpsensor_crypto; + +#endif /* __MOCK_FPSENSOR_CRYPTO_MOCK_H */ diff --git a/test/fpsensor.mocklist b/test/fpsensor.mocklist index 3968a04e7e..6052c108c2 100644 --- a/test/fpsensor.mocklist +++ b/test/fpsensor.mocklist @@ -6,6 +6,7 @@ #ifdef BOARD_HOST #define CONFIG_TEST_MOCK_LIST \ MOCK(FP_SENSOR) \ + MOCK(FPSENSOR_CRYPTO) \ MOCK(FPSENSOR_DETECT) \ MOCK(FPSENSOR_STATE) \ MOCK(MKBP_EVENTS) \ diff --git a/test/fpsensor_crypto.c b/test/fpsensor_crypto.c index 7ec54a9115..a71d66a5d7 100644 --- a/test/fpsensor_crypto.c +++ b/test/fpsensor_crypto.c @@ -10,6 +10,7 @@ #include "ec_commands.h" #include "fpsensor_crypto.h" #include "fpsensor_state.h" +#include "mock/fpsensor_crypto_mock.h" #include "mock/fpsensor_state_mock.h" #include "mock/rollback_mock.h" #include "mock/timer_mock.h" @@ -413,6 +414,92 @@ test_static int test_derive_positive_match_secret_fail_salt_trivial(void) return EC_SUCCESS; } +test_static int test_derive_positive_match_secret_fail_trivial_key_0x00(void) +{ + static uint8_t output[FP_POSITIVE_MATCH_SECRET_BYTES]; + + /* GIVEN that the user ID is set to a known value. */ + memcpy(user_id, fake_user_id, sizeof(fake_user_id)); + + /* + * GIVEN that the TPM seed is set, and reading the rollback secret will + * succeed. + */ + TEST_ASSERT(fp_tpm_seed_is_set() && + !mock_ctrl_rollback.get_secret_fail); + + /* GIVEN that the salt is not trivial. */ + TEST_ASSERT(!bytes_are_trivial(fake_positive_match_salt, + sizeof(fake_positive_match_salt))); + + /* GIVEN that the sha256 output is trivial (0x00) */ + mock_ctrl_fpsensor_crypto.output_type = + MOCK_CTRL_FPSENSOR_CRYPTO_SHA256_TYPE_ZEROS; + + /* THEN the derivation will fail with EC_ERROR_HW_INTERNAL. */ + TEST_ASSERT(derive_positive_match_secret(output, + fake_positive_match_salt) == + EC_ERROR_HW_INTERNAL); + + /* Now verify success is possible after reverting */ + + /* GIVEN that the sha256 output is non-trivial */ + mock_ctrl_fpsensor_crypto.output_type = + MOCK_CTRL_FPSENSOR_CRYPTO_SHA256_TYPE_REAL; + + /* THEN the derivation will succeed */ + TEST_ASSERT(derive_positive_match_secret( + output, fake_positive_match_salt) == EC_SUCCESS); + + /* Clean up any mock changes */ + mock_ctrl_fpsensor_crypto = MOCK_CTRL_DEFAULT_FPSENSOR_CRYPTO; + + return EC_SUCCESS; +} + +test_static int test_derive_positive_match_secret_fail_trivial_key_0xff(void) +{ + static uint8_t output[FP_POSITIVE_MATCH_SECRET_BYTES]; + + /* GIVEN that the user ID is set to a known value. */ + memcpy(user_id, fake_user_id, sizeof(fake_user_id)); + + /* + * GIVEN that the TPM seed is set, and reading the rollback secret will + * succeed. + */ + TEST_ASSERT(fp_tpm_seed_is_set() && + !mock_ctrl_rollback.get_secret_fail); + + /* GIVEN that the salt is not trivial. */ + TEST_ASSERT(!bytes_are_trivial(fake_positive_match_salt, + sizeof(fake_positive_match_salt))); + + /* GIVEN that the sha256 output is trivial (0xFF) */ + mock_ctrl_fpsensor_crypto.output_type = + MOCK_CTRL_FPSENSOR_CRYPTO_SHA256_TYPE_FF; + + /* THEN the derivation will fail with EC_ERROR_HW_INTERNAL. */ + TEST_ASSERT(derive_positive_match_secret(output, + fake_positive_match_salt) == + EC_ERROR_HW_INTERNAL); + + /* Now verify success is possible after reverting */ + + /* GIVEN that the sha256 output is non-trivial */ + mock_ctrl_fpsensor_crypto.output_type = + MOCK_CTRL_FPSENSOR_CRYPTO_SHA256_TYPE_REAL; + + /* THEN the derivation will succeed */ + TEST_ASSERT(derive_positive_match_secret( + output, fake_positive_match_salt) == EC_SUCCESS); + + /* Clean up any mock changes */ + mock_ctrl_fpsensor_crypto = MOCK_CTRL_DEFAULT_FPSENSOR_CRYPTO; + + return EC_SUCCESS; +} + static int test_enable_positive_match_secret_once( struct positive_match_secret_state *dumb_state) { @@ -616,6 +703,8 @@ void run_test(int argc, char **argv) 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_derive_positive_match_secret_fail_trivial_key_0x00); + RUN_TEST(test_derive_positive_match_secret_fail_trivial_key_0xff); RUN_TEST(test_enable_positive_match_secret); RUN_TEST(test_disable_positive_match_secret); RUN_TEST(test_command_read_match_secret); -- cgit v1.2.1