diff options
author | Vadim Sukhomlinov <sukhomlinov@google.com> | 2022-02-22 19:16:03 -0800 |
---|---|---|
committer | Commit Bot <commit-bot@chromium.org> | 2022-02-23 06:05:42 +0000 |
commit | aa7815dcfe09549c2b680e7ed604040b932c11e6 (patch) | |
tree | 0a84449022ade7710309495b0058ef7bd542c81f | |
parent | 1a1168ed6db553356b8c02d4fa0f086d69f8e502 (diff) | |
download | chrome-ec-aa7815dcfe09549c2b680e7ed604040b932c11e6.tar.gz |
cr50: fix nvmem logic in u2f_gen_kek_seed()stabilize-14536.B-cr50_stabstabilize-14532.B-cr50_stab
The problem is in the below chain invoked on processing TPM Clear command:
_plat__OwnerClearCallback()
u2f_gen_kek_seed()
u2f_get_state()
u2f_load_or_create_state()
write_tpm_nvmem_hidden()
NvCommit()
This chain is executed only if U2F data do not exist in the NVMEM.
The end result is write_tpm_nvmem_hidden() invoking nvmem_commit()
which removes the lock, which in turn causes the error when tmp command
processor tries to commit nvmem in the end of processing the command.
This is why the problem happens only once, after the first time U2F data
is present and the above chain is not traversed.
In the fix we avoid calling u2f_get_state() from u2f_gen_kek_seed() by
updating U2F state in memory if it is loaded and in nvmem directly.
Also discovered and fixing bug that resulted in platform owner
not being properly cleaned due incorrect error checking.
_plat__OwnerClearCallback() modified to print error status.
However, this fix doesn't address a case when tpm_test.py fails first
time on TPM2_Startup.
BUG=b:199981251
TEST=tcg tests now passes without errors from clean TPM state,
test/tpm_test/tpmtest.py passes U2F tests.
in CCD with CRYPTO_TEST=1
fips kek works after initial fw upload.
fips u2f
fips kek works with U2F state.
----------------------- Test Environment -------------------------------
Test Suite Version: 2.1a
Operating System: Linux
Processor Information: Intel(R) Core(TM) i9-10885H CPU @ 2.40GHz
TDDL Version: SocketTDDL
-------------------------- Test Object ---------------------------------
TPM Vendor: CROS
TPM Firmware Version: a77bf07 2
TPM Spec Version: 1.16
Vendor Specific Info: xCG , fTPM, ,
Tested Spec Version: 1.16
---------------------- Test Result Summary -----------------------------
Test executed on: Tue Feb 22 19:07:53 2022
Performed Tests: 248
Passed Tests: 248
Failed Tests: 0
Errors: 0
Warnings: 0
========================================================================
Signed-off-by: Vadim Sukhomlinov <sukhomlinov@google.com>
Change-Id: I452129bd696c5207dbef22ef1489fdab924677eb
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/3482484
Tested-by: Vadim Sukhomlinov <sukhomlinov@chromium.org>
Auto-Submit: Vadim Sukhomlinov <sukhomlinov@chromium.org>
Commit-Queue: Vadim Sukhomlinov <sukhomlinov@chromium.org>
Reviewed-by: Vadim Sukhomlinov <sukhomlinov@chromium.org>
Reviewed-by: Vadim Bendebury <vbendeb@chromium.org>
Commit-Queue: Vadim Bendebury <vbendeb@chromium.org>
-rw-r--r-- | board/cr50/dcrypto/u2f_impl.h | 7 | ||||
-rw-r--r-- | board/cr50/fips_cmd.c | 2 | ||||
-rw-r--r-- | board/cr50/tpm2/platform.c | 9 | ||||
-rw-r--r-- | board/cr50/u2f_state_load.c | 27 |
4 files changed, 31 insertions, 14 deletions
diff --git a/board/cr50/dcrypto/u2f_impl.h b/board/cr50/dcrypto/u2f_impl.h index be3fbd6b76..9003db4a03 100644 --- a/board/cr50/dcrypto/u2f_impl.h +++ b/board/cr50/dcrypto/u2f_impl.h @@ -201,14 +201,13 @@ struct u2f_state *u2f_get_state(void); bool u2f_load_or_create_state(struct u2f_state *state, bool force_create); /*** - * Generates and persists to nvram a new seed that will be used to - * derive kek in future calls to u2f_gen_kek(). + * Generates and persists to nvram a new key that will be used to + * sign U2F key handles and check they were created on this device. * - * @param commit whether to commit nvram changes before returning. * @return EC_SUCCESS if seed was successfully created * (and persisted if requested). */ -enum ec_error_list u2f_gen_kek_seed(int commit); +enum ec_error_list u2f_gen_kek_seed(void); /** * Zeroize U2F keys. Can be used to switch to FIPS-compliant path by diff --git a/board/cr50/fips_cmd.c b/board/cr50/fips_cmd.c index 8ed25914e8..c37766eba9 100644 --- a/board/cr50/fips_cmd.c +++ b/board/cr50/fips_cmd.c @@ -140,6 +140,8 @@ static int cmd_fips_status(int argc, char **argv) u2f_zeroize_keys()); else if (!strncmp(argv[1], "old", 3)) return fips_set_old_u2f_keys(); + else if (!strncmp(argv[1], "kek", 3)) + return u2f_gen_kek_seed(); else if (!strncmp(argv[1], "u2f", 3)) print_u2f_keys_status(); else if (!strncmp(argv[1], "gen", 3)) diff --git a/board/cr50/tpm2/platform.c b/board/cr50/tpm2/platform.c index 25d7bffcc5..42e3a95b53 100644 --- a/board/cr50/tpm2/platform.c +++ b/board/cr50/tpm2/platform.c @@ -7,6 +7,7 @@ #include "TPM_Types.h" #include "ccd_config.h" +#include "console.h" #include "pinweaver.h" #include "tpm_nvmem.h" #include "dcrypto.h" @@ -14,6 +15,8 @@ #include "util.h" #include "version.h" +#define CPRINTF(format, args...) cprintf(CC_EXTENSION, format, ## args) + uint16_t _cpri__GenerateRandom(size_t random_size, uint8_t *buffer) { @@ -94,6 +97,10 @@ BOOL _plat__ShallSurviveOwnerClear(uint32_t index) void _plat__OwnerClearCallback(void) { + enum ec_error_list rv; + /* Invalidate existing u2f registrations. */ - u2f_gen_kek_seed(0 /* commit */); + rv = u2f_gen_kek_seed(); + if (rv != EC_SUCCESS) + CPRINTF("%s: failed (%d)\n", __func__, rv); } diff --git a/board/cr50/u2f_state_load.c b/board/cr50/u2f_state_load.c index a1c8927dab..8e92199bb7 100644 --- a/board/cr50/u2f_state_load.c +++ b/board/cr50/u2f_state_load.c @@ -134,19 +134,28 @@ struct u2f_state *u2f_get_state(void) return u2f_state_loaded ? &u2f_state : NULL; } -enum ec_error_list u2f_gen_kek_seed(int commit) +enum ec_error_list u2f_gen_kek_seed(void) { - struct u2f_state *state = u2f_get_state(); - - if (!state) - return EC_ERROR_UNKNOWN; - - if (!u2f_generate_hmac_key(state)) + /** + * If U2F state is loaded, update HMAC key in memory, otherwise this + * is just temporary storage and will be updated (to the same value) + * in u2f_load_or_create_state() when u2f_get_state() will be called + * upon use of U2F. + */ + if (u2f_generate_hmac_key(&u2f_state) != EC_SUCCESS) return EC_ERROR_HW_INTERNAL; - if (write_tpm_nvmem_hidden(TPM_HIDDEN_U2F_KEK, sizeof(state->hmac_key), - state->hmac_key, commit) == TPM_WRITE_FAIL) + /* Store new U2F HMAC key in nvmem */ + if (write_tpm_nvmem_hidden(TPM_HIDDEN_U2F_KEK, + sizeof(u2f_state.hmac_key), + u2f_state.hmac_key, 0) == TPM_WRITE_FAIL) { + /** + * Failure to write means we now have inconsistent state + * between u2f_state and nvmem, so mark it as not loaded. + */ + u2f_state_loaded = false; return EC_ERROR_UNKNOWN; + } return EC_SUCCESS; } |