From 315a30b42bf42baf232cf9dda47fe6e06d08d18a Mon Sep 17 00:00:00 2001 From: Andrew Kozlik Date: Thu, 19 Sep 2019 11:30:42 +0200 Subject: [PATCH] core/webauthn: Disable FIDO2 signature counter for some relying parties --- common/defs/webauthn/apps/binance.json | 3 +- common/defs/webauthn/apps/github.json | 4 +- common/defs/webauthn/apps/microsoft.json | 3 +- common/defs/webauthn/gen.py | 16 ++++- common/protob/messages-webauthn.proto | 1 + core/src/apps/webauthn/__init__.py | 21 ++++-- core/src/apps/webauthn/confirm.py | 4 +- core/src/apps/webauthn/credential.py | 33 ++++++--- core/src/apps/webauthn/knownapps.py | 72 ++++++++++++------- .../webauthn/list_resident_credentials.py | 3 +- .../src/trezor/messages/WebAuthnCredential.py | 3 + core/tests/test_apps.webauthn.credential.py | 2 +- python/src/trezorlib/cli/trezorctl.py | 18 ++--- .../trezorlib/messages/WebAuthnCredential.py | 3 + 14 files changed, 127 insertions(+), 59 deletions(-) diff --git a/common/defs/webauthn/apps/binance.json b/common/defs/webauthn/apps/binance.json index 854a47ecd..39477a49e 100644 --- a/common/defs/webauthn/apps/binance.json +++ b/common/defs/webauthn/apps/binance.json @@ -1,4 +1,5 @@ { "label": "Binance", - "webauthn": "www.binance.com" + "webauthn": "www.binance.com", + "use_sign_count": false } diff --git a/common/defs/webauthn/apps/github.json b/common/defs/webauthn/apps/github.json index 4afd557e1..0f63bf22f 100644 --- a/common/defs/webauthn/apps/github.json +++ b/common/defs/webauthn/apps/github.json @@ -1,4 +1,6 @@ { "label": "GitHub", - "u2f": "https://github.com/u2f/trusted_facets" + "u2f": "https://github.com/u2f/trusted_facets", + "webauthn": "github.com", + "use_sign_count": true } diff --git a/common/defs/webauthn/apps/microsoft.json b/common/defs/webauthn/apps/microsoft.json index aa65cfc5f..719023faf 100644 --- a/common/defs/webauthn/apps/microsoft.json +++ b/common/defs/webauthn/apps/microsoft.json @@ -1,4 +1,5 @@ { "label": "Microsoft", - "webauthn": "login.microsoft.com" + "webauthn": "login.microsoft.com", + "use_sign_count": false } diff --git a/common/defs/webauthn/gen.py b/common/defs/webauthn/gen.py index 420cd6602..994cae37a 100755 --- a/common/defs/webauthn/gen.py +++ b/common/defs/webauthn/gen.py @@ -21,12 +21,22 @@ def gen_core(data): for d in data: if "u2f" in d: url, label = d["u2f"], d["label"] - print(' "%s": "%s",' % (url, label)) + print(' "%s": {"label": "%s", "use_sign_count": True},' % (url, label)) print(" # WebAuthn") for d in data: if "webauthn" in d: - origin, label = d["webauthn"], d["label"] - print(' "%s": "%s",' % (origin, label)) + origin, label, use_sign_count = ( + d["webauthn"], + d["label"], + d.get("use_sign_count", None), + ) + if use_sign_count is None: + print(' "%s": {"label": "%s"},' % (origin, label)) + else: + print( + ' "%s": {"label": "%s", "use_sign_count": %s},' + % (origin, label, use_sign_count) + ) print("}") diff --git a/common/protob/messages-webauthn.proto b/common/protob/messages-webauthn.proto index 1f2fcdd3d..ebdd26e56 100644 --- a/common/protob/messages-webauthn.proto +++ b/common/protob/messages-webauthn.proto @@ -52,5 +52,6 @@ message WebAuthnCredentials { optional string user_display_name = 7; optional uint32 creation_time = 8; optional bool hmac_secret = 9; + optional bool use_sign_count = 10; } } diff --git a/core/src/apps/webauthn/__init__.py b/core/src/apps/webauthn/__init__.py index ce66aa61b..149aa7a69 100644 --- a/core/src/apps/webauthn/__init__.py +++ b/core/src/apps/webauthn/__init__.py @@ -1152,18 +1152,20 @@ def msg_authenticate(req: Msg, dialog_mgr: DialogManager) -> Cmd: # sign the authentication challenge and return if __debug__: log.info(__name__, "signing authentication") - buf = msg_authenticate_sign(auth.chal, auth.appId, cred.private_key()) + buf = msg_authenticate_sign(auth.chal, auth.appId, cred) dialog_mgr.reset() return Cmd(req.cid, _CMD_MSG, buf) -def msg_authenticate_sign(challenge: bytes, rp_id_hash: bytes, privkey: bytes) -> bytes: +def msg_authenticate_sign( + challenge: bytes, rp_id_hash: bytes, cred: Credential +) -> bytes: flags = bytes([_AUTH_FLAG_UP]) # get next counter - ctr = storage.device.next_u2f_counter() + ctr = cred.next_signature_counter() ctrbuf = ustruct.pack(">L", ctr) # hash input data together with counter @@ -1174,7 +1176,7 @@ def msg_authenticate_sign(challenge: bytes, rp_id_hash: bytes, privkey: bytes) - dig.update(challenge) # uint8_t chal[32]; # sign the digest and convert to der - sig = nist256p1.sign(privkey, dig.digest(), False) + sig = nist256p1.sign(cred.private_key(), dig.digest(), False) sig = der.encode_seq((sig[1:33], sig[33:])) # pack to a response @@ -1206,6 +1208,8 @@ def cbor_error(cid: int, code: int) -> Cmd: def cbor_make_credential(req: Cmd, dialog_mgr: DialogManager) -> Optional[Cmd]: + from apps.webauthn.knownapps import knownapps + if not storage.is_initialized(): if __debug__: log.warning(__name__, "not initialized") @@ -1260,6 +1264,8 @@ def cbor_make_credential(req: Cmd, dialog_mgr: DialogManager) -> Optional[Cmd]: except Exception: return cbor_error(req.cid, _ERR_INVALID_CBOR) + cred.use_sign_count = knownapps.get(rp_id_hash, {}).get("use_sign_count", True) + # Check data types. if ( not cred.check_data_types() @@ -1333,10 +1339,12 @@ def cbor_make_credential_sign( extensions = cbor.encode({"hmac-secret": True}) flags |= _AUTH_FLAG_ED + ctr = cred.next_signature_counter() + authenticator_data = ( cred.rp_id_hash + bytes([flags]) - + b"\x00\x00\x00\x00" + + ctr.to_bytes(4, "big") + att_cred_data + extensions ) @@ -1541,7 +1549,8 @@ def cbor_get_assertion_sign( flags |= _AUTH_FLAG_ED encoded_extensions = cbor.encode(extensions) - ctr = storage.device.next_u2f_counter() or 0 + ctr = cred.next_signature_counter() + authenticator_data = ( rp_id_hash + bytes([flags]) + ctr.to_bytes(4, "big") + encoded_extensions ) diff --git a/core/src/apps/webauthn/confirm.py b/core/src/apps/webauthn/confirm.py index 6cb0ae08a..0395fd8c3 100644 --- a/core/src/apps/webauthn/confirm.py +++ b/core/src/apps/webauthn/confirm.py @@ -20,10 +20,10 @@ class ConfirmInfo: def load_icon(self, rp_id_hash: bytes) -> None: from trezor import res - from apps.webauthn import knownapps + from apps.webauthn.knownapps import knownapps try: - namepart = knownapps.knownapps[rp_id_hash].lower().replace(" ", "_") + namepart = knownapps[rp_id_hash]["label"].lower().replace(" ", "_") icon = res.load("apps/webauthn/res/icon_%s.toif" % namepart) except Exception as e: icon = res.load("apps/webauthn/res/icon_webauthn.toif") diff --git a/core/src/apps/webauthn/credential.py b/core/src/apps/webauthn/credential.py index dd48b500c..c31bcabfa 100644 --- a/core/src/apps/webauthn/credential.py +++ b/core/src/apps/webauthn/credential.py @@ -23,6 +23,7 @@ _CRED_ID_USER_NAME = const(0x04) _CRED_ID_USER_DISPLAY_NAME = const(0x05) _CRED_ID_CREATION_TIME = const(0x06) _CRED_ID_HMAC_SECRET = const(0x07) +_CRED_ID_USE_SIGN_COUNT = const(0x08) # Key paths _U2F_KEY_PATH = const(0x80553246) @@ -30,7 +31,7 @@ _U2F_KEY_PATH = const(0x80553246) class Credential: def __init__(self) -> None: - self.index = None # type Optional[int] + self.index = None # type: Optional[int] self.id = b"" # type: bytes self.rp_id = "" # type: str self.rp_id_hash = b"" # type: bytes @@ -48,6 +49,9 @@ class Credential: def hmac_secret_key(self) -> Optional[bytes]: return None + def next_signature_counter(self) -> int: + return storage.device.next_u2f_counter() or 0 + @staticmethod def from_bytes(data: bytes, rp_id_hash: bytes) -> Optional["Credential"]: cred = Fido2Credential.from_cred_id( @@ -65,19 +69,20 @@ class Fido2Credential(Credential): self.rp_name = None # type: Optional[str] self.user_name = None # type: Optional[str] self.user_display_name = None # type: Optional[str] - self._creation_time = 0 # type: int + self.creation_time = 0 # type: int self.hmac_secret = False # type: bool + self.use_sign_count = False # type: bool def __lt__(self, other: Credential) -> bool: # Sort FIDO2 credentials newest first amongst each other. if isinstance(other, Fido2Credential): - return self._creation_time > other._creation_time + return self.creation_time > other.creation_time # Sort FIDO2 credentials before U2F credentials. return True def generate_id(self) -> None: - self._creation_time = storage.device.next_u2f_counter() or 0 + self.creation_time = storage.device.next_u2f_counter() or 0 data = cbor.encode( { @@ -88,8 +93,9 @@ class Fido2Credential(Credential): (_CRED_ID_USER_ID, self.user_id), (_CRED_ID_USER_NAME, self.user_name), (_CRED_ID_USER_DISPLAY_NAME, self.user_display_name), - (_CRED_ID_CREATION_TIME, self._creation_time), + (_CRED_ID_CREATION_TIME, self.creation_time), (_CRED_ID_HMAC_SECRET, self.hmac_secret), + (_CRED_ID_USE_SIGN_COUNT, self.use_sign_count), ) if value } @@ -148,8 +154,9 @@ class Fido2Credential(Credential): cred.user_id = data.get(_CRED_ID_USER_ID, None) cred.user_name = data.get(_CRED_ID_USER_NAME, None) cred.user_display_name = data.get(_CRED_ID_USER_DISPLAY_NAME, None) - cred._creation_time = data.get(_CRED_ID_CREATION_TIME, 0) + cred.creation_time = data.get(_CRED_ID_CREATION_TIME, 0) cred.hmac_secret = data.get(_CRED_ID_HMAC_SECRET, False) + cred.use_sign_count = data.get(_CRED_ID_USE_SIGN_COUNT, False) cred.id = cred_id if ( @@ -165,7 +172,7 @@ class Fido2Credential(Credential): return ( self.rp_id is not None and self.user_id is not None - and self._creation_time is not None + and self.creation_time is not None ) def check_data_types(self) -> bool: @@ -176,7 +183,8 @@ class Fido2Credential(Credential): and isinstance(self.user_name, (str, type(None))) and isinstance(self.user_display_name, (str, type(None))) and isinstance(self.hmac_secret, bool) - and isinstance(self._creation_time, (int, type(None))) + and isinstance(self.use_sign_count, bool) + and isinstance(self.creation_time, (int, type(None))) and isinstance(self.id, (bytes, bytearray)) ) @@ -212,6 +220,11 @@ class Fido2Credential(Credential): return node.key() + def next_signature_counter(self) -> int: + if not self.use_sign_count: + return 0 + return super().next_signature_counter() + class U2fCredential(Credential): def __init__(self) -> None: @@ -249,9 +262,9 @@ class U2fCredential(Credential): self.id = keypath + mac.digest() def app_name(self) -> str: - from apps.webauthn import knownapps + from apps.webauthn.knownapps import knownapps - app_name = knownapps.knownapps.get(self.rp_id_hash, None) + app_name = knownapps.get(self.rp_id_hash, {}).get("label", None) if app_name is None: app_name = "%s...%s" % ( hexlify(self.rp_id_hash[:4]).decode(), diff --git a/core/src/apps/webauthn/knownapps.py b/core/src/apps/webauthn/knownapps.py index faccbfd0b..7cd31db22 100644 --- a/core/src/apps/webauthn/knownapps.py +++ b/core/src/apps/webauthn/knownapps.py @@ -6,32 +6,54 @@ from trezor.crypto.hashlib import sha256 _knownapps = { # U2F - "https://bitbucket.org": "Bitbucket", - "https://www.bitfinex.com": "Bitfinex", - "https://vault.bitwarden.com/app-id.json": "Bitwarden", - "https://www.dashlane.com": "Dashlane", - "https://www.dropbox.com/u2f-app-id.json": "Dropbox", - "https://api-9dcf9b83.duosecurity.com": "Duo", - "https://www.fastmail.com": "FastMail", - "https://id.fedoraproject.org/u2f-origins.json": "Fedora", - "https://account.gandi.net/api/u2f/trusted_facets.json": "Gandi", - "https://github.com/u2f/trusted_facets": "GitHub", - "https://gitlab.com": "GitLab", - "https://www.gstatic.com/securitykey/origins.json": "Google", - "https://keepersecurity.com": "Keeper", - "https://lastpass.com": "LastPass", - "https://slushpool.com/static/security/u2f.json": "Slush Pool", - "https://dashboard.stripe.com": "Stripe", - "https://u2f.bin.coffee": "u2f.bin.coffee", + "https://bitbucket.org": {"label": "Bitbucket", "use_sign_count": True}, + "https://www.bitfinex.com": {"label": "Bitfinex", "use_sign_count": True}, + "https://vault.bitwarden.com/app-id.json": { + "label": "Bitwarden", + "use_sign_count": True, + }, + "https://www.dashlane.com": {"label": "Dashlane", "use_sign_count": True}, + "https://www.dropbox.com/u2f-app-id.json": { + "label": "Dropbox", + "use_sign_count": True, + }, + "https://api-9dcf9b83.duosecurity.com": {"label": "Duo", "use_sign_count": True}, + "https://www.fastmail.com": {"label": "FastMail", "use_sign_count": True}, + "https://id.fedoraproject.org/u2f-origins.json": { + "label": "Fedora", + "use_sign_count": True, + }, + "https://account.gandi.net/api/u2f/trusted_facets.json": { + "label": "Gandi", + "use_sign_count": True, + }, + "https://github.com/u2f/trusted_facets": { + "label": "GitHub", + "use_sign_count": True, + }, + "https://gitlab.com": {"label": "GitLab", "use_sign_count": True}, + "https://www.gstatic.com/securitykey/origins.json": { + "label": "Google", + "use_sign_count": True, + }, + "https://keepersecurity.com": {"label": "Keeper", "use_sign_count": True}, + "https://lastpass.com": {"label": "LastPass", "use_sign_count": True}, + "https://slushpool.com/static/security/u2f.json": { + "label": "Slush Pool", + "use_sign_count": True, + }, + "https://dashboard.stripe.com": {"label": "Stripe", "use_sign_count": True}, + "https://u2f.bin.coffee": {"label": "u2f.bin.coffee", "use_sign_count": True}, # WebAuthn - "www.binance.com": "Binance", - "www.dropbox.com": "Dropbox", - "secure.login.gov": "login.gov", - "login.microsoft.com": "Microsoft", - "webauthn.bin.coffee": "webauthn.bin.coffee", - "webauthn.io": "WebAuthn.io", - "webauthn.me": "WebAuthn.me", - "demo.yubico.com": "demo.yubico.com", + "www.binance.com": {"label": "Binance", "use_sign_count": False}, + "www.dropbox.com": {"label": "Dropbox"}, + "github.com": {"label": "GitHub", "use_sign_count": True}, + "secure.login.gov": {"label": "login.gov"}, + "login.microsoft.com": {"label": "Microsoft", "use_sign_count": False}, + "webauthn.bin.coffee": {"label": "webauthn.bin.coffee"}, + "webauthn.io": {"label": "WebAuthn.io"}, + "webauthn.me": {"label": "WebAuthn.me"}, + "demo.yubico.com": {"label": "demo.yubico.com"}, } knownapps = {sha256(k.encode()).digest(): v for (k, v) in _knownapps.items()} diff --git a/core/src/apps/webauthn/list_resident_credentials.py b/core/src/apps/webauthn/list_resident_credentials.py index 4d7b0020a..a54062a58 100644 --- a/core/src/apps/webauthn/list_resident_credentials.py +++ b/core/src/apps/webauthn/list_resident_credentials.py @@ -30,8 +30,9 @@ async def list_resident_credentials( user_id=cred.user_id, user_name=cred.user_name, user_display_name=cred.user_display_name, - creation_time=cred._creation_time, + creation_time=cred.creation_time, hmac_secret=cred.hmac_secret, + use_sign_count=cred.use_sign_count, ) for cred in get_resident_credentials() ] diff --git a/core/src/trezor/messages/WebAuthnCredential.py b/core/src/trezor/messages/WebAuthnCredential.py index a47d2b44a..a2155c1d4 100644 --- a/core/src/trezor/messages/WebAuthnCredential.py +++ b/core/src/trezor/messages/WebAuthnCredential.py @@ -23,6 +23,7 @@ class WebAuthnCredential(p.MessageType): user_display_name: str = None, creation_time: int = None, hmac_secret: bool = None, + use_sign_count: bool = None, ) -> None: self.index = index self.id = id @@ -33,6 +34,7 @@ class WebAuthnCredential(p.MessageType): self.user_display_name = user_display_name self.creation_time = creation_time self.hmac_secret = hmac_secret + self.use_sign_count = use_sign_count @classmethod def get_fields(cls) -> Dict: @@ -46,4 +48,5 @@ class WebAuthnCredential(p.MessageType): 7: ('user_display_name', p.UnicodeType, 0), 8: ('creation_time', p.UVarintType, 0), 9: ('hmac_secret', p.BoolType, 0), + 10: ('use_sign_count', p.BoolType, 0), } diff --git a/core/tests/test_apps.webauthn.credential.py b/core/tests/test_apps.webauthn.credential.py index 272909995..70109ff4b 100644 --- a/core/tests/test_apps.webauthn.credential.py +++ b/core/tests/test_apps.webauthn.credential.py @@ -49,7 +49,7 @@ class TestCredential(unittest.TestCase): self.assertEqual(cred.rp_id_hash, rp_id_hash) self.assertEqual(hexlify(cred.user_id), user_id) self.assertEqual(cred.user_name, user_name) - self.assertEqual(cred._creation_time, 2) + self.assertEqual(cred.creation_time, creation_time) self.assertTrue(cred.hmac_secret) self.assertIsNone(cred.rp_name) self.assertIsNone(cred.user_display_name) diff --git a/python/src/trezorlib/cli/trezorctl.py b/python/src/trezorlib/cli/trezorctl.py index c23cd1582..98594a24e 100755 --- a/python/src/trezorlib/cli/trezorctl.py +++ b/python/src/trezorlib/cli/trezorctl.py @@ -1973,20 +1973,22 @@ def webauthn_list_credentials(connect): click.echo("") click.echo("WebAuthn credential at index {}:".format(cred.index)) if cred.rp_id is not None: - click.echo(" Relying party ID: {}".format(cred.rp_id)) + click.echo(" Relying party ID: {}".format(cred.rp_id)) if cred.rp_name is not None: - click.echo(" Relying party name: {}".format(cred.rp_name)) + click.echo(" Relying party name: {}".format(cred.rp_name)) if cred.user_id is not None: - click.echo(" User ID: {}".format(cred.user_id.hex())) + click.echo(" User ID: {}".format(cred.user_id.hex())) if cred.user_name is not None: - click.echo(" User name: {}".format(cred.user_name)) + click.echo(" User name: {}".format(cred.user_name)) if cred.user_display_name is not None: - click.echo(" User display name: {}".format(cred.user_display_name)) + click.echo(" User display name: {}".format(cred.user_display_name)) if cred.creation_time is not None: - click.echo(" Creation time: {}".format(cred.creation_time)) + click.echo(" Creation time: {}".format(cred.creation_time)) if cred.hmac_secret is not None: - click.echo(" hmac-secret enabled: {}".format(cred.hmac_secret)) - click.echo(" Credential ID: {}".format(cred.id.hex())) + click.echo(" hmac-secret enabled: {}".format(cred.hmac_secret)) + if cred.use_sign_count is not None: + click.echo(" Use signature counter: {}".format(cred.use_sign_count)) + click.echo(" Credential ID: {}".format(cred.id.hex())) if not creds: click.echo("There are no resident credentials stored on the device.") diff --git a/python/src/trezorlib/messages/WebAuthnCredential.py b/python/src/trezorlib/messages/WebAuthnCredential.py index 05f5091b8..5294fa1a2 100644 --- a/python/src/trezorlib/messages/WebAuthnCredential.py +++ b/python/src/trezorlib/messages/WebAuthnCredential.py @@ -23,6 +23,7 @@ class WebAuthnCredential(p.MessageType): user_display_name: str = None, creation_time: int = None, hmac_secret: bool = None, + use_sign_count: bool = None, ) -> None: self.index = index self.id = id @@ -33,6 +34,7 @@ class WebAuthnCredential(p.MessageType): self.user_display_name = user_display_name self.creation_time = creation_time self.hmac_secret = hmac_secret + self.use_sign_count = use_sign_count @classmethod def get_fields(cls) -> Dict: @@ -46,4 +48,5 @@ class WebAuthnCredential(p.MessageType): 7: ('user_display_name', p.UnicodeType, 0), 8: ('creation_time', p.UVarintType, 0), 9: ('hmac_secret', p.BoolType, 0), + 10: ('use_sign_count', p.BoolType, 0), }