From b1e4246b46e4b5278400b52743ff269ed5663ff3 Mon Sep 17 00:00:00 2001 From: matejcik Date: Thu, 8 Apr 2021 12:43:28 +0200 Subject: [PATCH] refactor(core/webauthn): make sure KEY_AGREEMENT_*KEY is generated once per power-up This is what the spec recommends and it has been the case before workflow-restarts, when `apps.webauthn.fido2` was imported exactly once per lifetime. With workflow-restarts, `fido2` is being imported repeatedly and the keys regenerated. This does not seem to be a problem per the spec -- a FIDO workflow will retain the same keys, and non-FIDO workflows can be seen as unplugs/replugs as far as the FIDO functionality is concerned. However, regenerating the keys is slow, which is a problem for the hardware-based unit tests. We can avoid the slowness by returning to the spec-mandated behavior and generating once per power-up. --- core/src/all_modules.py | 2 ++ core/src/apps/webauthn/fido2.py | 11 ++++------- core/src/main.py | 3 +++ core/src/storage/fido2.py | 12 ++++++++++++ 4 files changed, 21 insertions(+), 7 deletions(-) create mode 100644 core/src/storage/fido2.py diff --git a/core/src/all_modules.py b/core/src/all_modules.py index 54318d048..bef448e57 100644 --- a/core/src/all_modules.py +++ b/core/src/all_modules.py @@ -50,6 +50,8 @@ storage.debug import storage.debug storage.device import storage.device +storage.fido2 +import storage.fido2 storage.recovery import storage.recovery storage.recovery_shares diff --git a/core/src/apps/webauthn/fido2.py b/core/src/apps/webauthn/fido2.py index f4970a04d..fd687e553 100644 --- a/core/src/apps/webauthn/fido2.py +++ b/core/src/apps/webauthn/fido2.py @@ -5,6 +5,7 @@ from micropython import const import storage import storage.resident_credentials +from storage.fido2 import KEY_AGREEMENT_PRIVKEY, KEY_AGREEMENT_PUBKEY from trezor import config, io, log, loop, ui, utils, workflow from trezor.crypto import aes, der, hashlib, hmac, random from trezor.crypto.curve import nist256p1 @@ -211,10 +212,6 @@ _RESULT_DECLINE = const(2) # User declined. _RESULT_CANCEL = const(3) # Request was cancelled by _CMD_CANCEL. _RESULT_TIMEOUT = const(4) # Request exceeded _FIDO2_CONFIRM_TIMEOUT_MS. -# Generate the authenticatorKeyAgreementKey used for ECDH in authenticatorClientPIN getKeyAgreement. -_KEY_AGREEMENT_PRIVKEY = nist256p1.generate_secret() -_KEY_AGREEMENT_PUBKEY = nist256p1.publickey(_KEY_AGREEMENT_PRIVKEY, False) - # FIDO2 configuration. _ALLOW_FIDO2 = True _ALLOW_RESIDENT_CREDENTIALS = True @@ -1781,7 +1778,7 @@ def cbor_get_assertion_hmac_secret(cred: Credential, hmac_secret: dict) -> bytes raise CborError(_ERR_INVALID_LEN) # Compute the ECDH shared secret. - ecdh_result = nist256p1.multiply(_KEY_AGREEMENT_PRIVKEY, b"\04" + x + y) + ecdh_result = nist256p1.multiply(KEY_AGREEMENT_PRIVKEY, b"\04" + x + y) shared_secret = hashlib.sha256(ecdh_result[1:33]).digest() # Check the authentication tag and decrypt the salt. @@ -1904,8 +1901,8 @@ def cbor_client_pin(req: Cmd) -> Cmd: common.COSE_KEY_ALG: common.COSE_ALG_ECDH_ES_HKDF_256, common.COSE_KEY_KTY: common.COSE_KEYTYPE_EC2, common.COSE_KEY_CRV: common.COSE_CURVE_P256, - common.COSE_KEY_X: _KEY_AGREEMENT_PUBKEY[1:33], - common.COSE_KEY_Y: _KEY_AGREEMENT_PUBKEY[33:], + common.COSE_KEY_X: KEY_AGREEMENT_PUBKEY[1:33], + common.COSE_KEY_Y: KEY_AGREEMENT_PUBKEY[33:], } } diff --git a/core/src/main.py b/core/src/main.py index 2bd46e2bc..8516f6241 100644 --- a/core/src/main.py +++ b/core/src/main.py @@ -8,6 +8,9 @@ if __debug__: import storage.debug from trezor import config, pin, utils # noqa: F401 +if not utils.BITCOIN_ONLY: + import storage.fido2 # noqa: F401 + # Prepare the USB interfaces first. Do not connect to the host yet. import usb diff --git a/core/src/storage/fido2.py b/core/src/storage/fido2.py new file mode 100644 index 000000000..7a3863516 --- /dev/null +++ b/core/src/storage/fido2.py @@ -0,0 +1,12 @@ +# FIDO2 keys that should be generated once per power-up +# https://fidoalliance.org/specs/fido-v2.0-ps-20190130/fido-client-to-authenticator-protocol-v2.0-ps-20190130.html#authenticator-power-up-configuration +# While we could safely generate these keys per-wipe and save them in sessionless cache, +# this drags down the performance of our test suite. + +# We want to avoid importing `trezor.crypto.curve` because that would needlessly pollute +# our RAM space, while in the end importing the symbol from `trezorcrypto` directly anyway +from trezorcrypto import nist256p1 + +# the authenticatorKeyAgreementKey used for ECDH in authenticatorClientPIN getKeyAgreement. +KEY_AGREEMENT_PRIVKEY = nist256p1.generate_secret() +KEY_AGREEMENT_PUBKEY = nist256p1.publickey(KEY_AGREEMENT_PRIVKEY, False)