From f87a358a46c3c8503f9e752f4d61f445e24b2b5c Mon Sep 17 00:00:00 2001 From: Dain Nilsson Date: Tue, 15 Oct 2024 17:09:54 +0200 Subject: [PATCH 01/13] Filter allow/exclude lists to only matched ids --- fido2/client.py | 69 +++++++++++++++++++++++++++++++------------------ 1 file changed, 44 insertions(+), 25 deletions(-) diff --git a/fido2/client.py b/fido2/client.py index 83171c96..8cd01e19 100644 --- a/fido2/client.py +++ b/fido2/client.py @@ -460,6 +460,37 @@ def __init__( self.extensions = extensions self.user_interaction = user_interaction + def _filter_creds(self, rp_id, cred_list, event, on_keepalive): + # Filter out credential IDs which are too long + max_len = self.info.max_cred_id_length + if max_len: + cred_list = [e for e in cred_list if len(e) <= max_len] + + max_creds = self.info.max_creds_in_list or 1 + chunks = [ + cred_list[i : i + max_creds] for i in range(0, len(cred_list), max_creds) + ] + + matches = [] + for chunk in chunks: + assertions = self.ctap2.get_assertions( + rp_id, + b"\0" * 32, + _cbor_list(chunk), + None, + {"up": False}, + None, + None, + event=event, + on_keepalive=on_keepalive, + ) + matches.extend([a.credential for a in assertions]) + + if len(matches) > max_creds: + # Too many matches? Just return as many as we can handle + return matches[:max_creds] + return matches + def selection(self, event): if "FIDO_2_1" in self.info.versions: self.ctap2.selection(event=event) @@ -575,16 +606,12 @@ def do_make_credential( enterprise_attestation, event, ): - if exclude_list: - # Filter out credential IDs which are too long - max_len = self.info.max_cred_id_length - if max_len: - exclude_list = [e for e in exclude_list if len(e) <= max_len] + on_keepalive = _user_keepalive(self.user_interaction) - # Reject the request if too many credentials remain. - max_creds = self.info.max_creds_in_list - if max_creds and len(exclude_list) > max_creds: - raise ClientError.ERR.BAD_REQUEST("exclude_list too long") + if exclude_list: + cred_list = self._filter_creds(rp.id, exclude_list, event, on_keepalive) + else: + cred_list = None # Process extensions client_inputs = extensions or {} @@ -603,8 +630,6 @@ def do_make_credential( except ValueError as e: raise ClientError.ERR.CONFIGURATION_UNSUPPORTED(e) - on_keepalive = _user_keepalive(self.user_interaction) - # Handle auth pin_protocol, pin_token, pin_auth, internal_uv = self._get_auth_params( client_data, rp.id, user_verification, permissions, event, on_keepalive @@ -624,7 +649,7 @@ def do_make_credential( _as_cbor(rp), _as_cbor(user), _cbor_list(key_params), - _cbor_list(exclude_list), + cred_list, extension_inputs or None, options, pin_auth, @@ -662,18 +687,14 @@ def do_get_assertion( user_verification, event, ): + on_keepalive = _user_keepalive(self.user_interaction) + if allow_list: - # Filter out credential IDs which are too long - max_len = self.info.max_cred_id_length - if max_len: - allow_list = [e for e in allow_list if len(e) <= max_len] - if not allow_list: + cred_list = self._filter_creds(rp_id, allow_list, event, on_keepalive) + if not cred_list: raise CtapError(CtapError.ERR.NO_CREDENTIALS) - - # Reject the request if too many credentials remain. - max_creds = self.info.max_creds_in_list - if max_creds and len(allow_list) > max_creds: - raise ClientError.ERR.BAD_REQUEST("allow_list too long") + else: + cred_list = None # Process extensions client_inputs = extensions or {} @@ -692,8 +713,6 @@ def do_get_assertion( except ValueError as e: raise ClientError.ERR.CONFIGURATION_UNSUPPORTED(e) - on_keepalive = _user_keepalive(self.user_interaction) - pin_protocol, pin_token, pin_auth, internal_uv = self._get_auth_params( client_data, rp_id, user_verification, permissions, event, on_keepalive ) @@ -702,7 +721,7 @@ def do_get_assertion( assertions = self.ctap2.get_assertions( rp_id, client_data.hash, - _cbor_list(allow_list), + cred_list, extension_inputs or None, options, pin_auth, From f54e60f64e6d381666bc9637fb34b0f8d9384df5 Mon Sep 17 00:00:00 2001 From: Dain Nilsson Date: Wed, 16 Oct 2024 11:21:30 +0200 Subject: [PATCH 02/13] Add pinAuth to preflight --- fido2/client.py | 104 ++++++++++++++++++++++++-------------- fido2/ctap2/extensions.py | 29 ++++++++--- 2 files changed, 89 insertions(+), 44 deletions(-) diff --git a/fido2/client.py b/fido2/client.py index 8cd01e19..e193a998 100644 --- a/fido2/client.py +++ b/fido2/client.py @@ -460,7 +460,9 @@ def __init__( self.extensions = extensions self.user_interaction = user_interaction - def _filter_creds(self, rp_id, cred_list, event, on_keepalive): + def _filter_creds( + self, rp_id, cred_list, pin_protocol, pin_token, event, on_keepalive + ): # Filter out credential IDs which are too long max_len = self.info.max_cred_id_length if max_len: @@ -471,20 +473,31 @@ def _filter_creds(self, rp_id, cred_list, event, on_keepalive): cred_list[i : i + max_creds] for i in range(0, len(cred_list), max_creds) ] + client_data_hash = b"\0" * 32 + if pin_token: + pin_auth = pin_protocol.authenticate(pin_token, client_data_hash) + version = pin_protocol.VERSION + else: + pin_auth = None + version = None matches = [] for chunk in chunks: assertions = self.ctap2.get_assertions( rp_id, - b"\0" * 32, + client_data_hash, _cbor_list(chunk), None, {"up": False}, - None, - None, + pin_auth, + version, event=event, on_keepalive=on_keepalive, ) - matches.extend([a.credential for a in assertions]) + if len(chunk) == 1 and len(assertions) == 1: + # Credential ID might be omitted from assertions + matches.append(_cbor_list(chunk)[0]) + else: + matches.extend([a.credential for a in assertions]) if len(matches) > max_creds: # Too many matches? Just return as many as we can handle @@ -568,21 +581,19 @@ def _get_token( def _get_auth_params( self, client_data, rp_id, user_verification, permissions, event, on_keepalive ): - mc = client_data.type == CollectedClientData.TYPE.CREATE self.info = self.ctap2.get_info() # Make sure we have "fresh" info pin_protocol = None pin_token = None pin_auth = None internal_uv = False - if self._should_use_uv(user_verification, mc) or permissions: + additional_perms = permissions & ~( + ClientPin.PERMISSION.MAKE_CREDENTIAL | ClientPin.PERMISSION.GET_ASSERTION + ) + mc = client_data.type == CollectedClientData.TYPE.CREATE + if self._should_use_uv(user_verification, mc) or additional_perms: client_pin = ClientPin(self.ctap2) - allow_internal_uv = not permissions - permissions |= ( - ClientPin.PERMISSION.MAKE_CREDENTIAL - if mc - else ClientPin.PERMISSION.GET_ASSERTION - ) + allow_internal_uv = not additional_perms pin_token = self._get_token( client_pin, permissions, rp_id, event, on_keepalive, allow_internal_uv ) @@ -608,33 +619,43 @@ def do_make_credential( ): on_keepalive = _user_keepalive(self.user_interaction) + # Gather up permissions + permissions = ClientPin.PERMISSION.MAKE_CREDENTIAL if exclude_list: - cred_list = self._filter_creds(rp.id, exclude_list, event, on_keepalive) + # We need this for filtering the exclude_list + permissions = ClientPin.PERMISSION.GET_ASSERTION + + # Get extension permissions + extension_instances = [cls(self.ctap2) for cls in self.extensions] + client_inputs = extensions or {} + for ext in extension_instances: + permissions |= ext.get_create_permissions(client_inputs) + + # Handle auth + pin_protocol, pin_token, pin_auth, internal_uv = self._get_auth_params( + client_data, rp.id, user_verification, permissions, event, on_keepalive + ) + + if exclude_list: + cred_list = self._filter_creds( + rp.id, exclude_list, pin_protocol, pin_token, event, on_keepalive + ) else: cred_list = None # Process extensions - client_inputs = extensions or {} extension_inputs = {} used_extensions = [] - permissions = ClientPin.PERMISSION(0) + permissions = ClientPin.PERMISSION.MAKE_CREDENTIAL try: - for ext in [cls(self.ctap2) for cls in self.extensions]: - auth_input, req_perms = ext.process_create_input_with_permissions( - client_inputs - ) + for ext in extension_instances: + auth_input = ext.process_create_input(client_inputs) if auth_input is not None: used_extensions.append(ext) - permissions |= req_perms extension_inputs[ext.NAME] = auth_input except ValueError as e: raise ClientError.ERR.CONFIGURATION_UNSUPPORTED(e) - # Handle auth - pin_protocol, pin_token, pin_auth, internal_uv = self._get_auth_params( - client_data, rp.id, user_verification, permissions, event, on_keepalive - ) - if not (rk or internal_uv): options = None else: @@ -689,35 +710,42 @@ def do_get_assertion( ): on_keepalive = _user_keepalive(self.user_interaction) + # Gather up permissions + permissions = ClientPin.PERMISSION.GET_ASSERTION + + # Get extension permissions + extension_instances = [cls(self.ctap2) for cls in self.extensions] + client_inputs = extensions or {} + for ext in extension_instances: + permissions |= ext.get_get_permissions(client_inputs) + + # Handle auth + pin_protocol, pin_token, pin_auth, internal_uv = self._get_auth_params( + client_data, rp_id, user_verification, permissions, event, on_keepalive + ) + if allow_list: - cred_list = self._filter_creds(rp_id, allow_list, event, on_keepalive) + cred_list = self._filter_creds( + rp_id, allow_list, pin_protocol, pin_token, event, on_keepalive + ) if not cred_list: raise CtapError(CtapError.ERR.NO_CREDENTIALS) else: cred_list = None # Process extensions - client_inputs = extensions or {} extension_inputs = {} used_extensions = [] - permissions = ClientPin.PERMISSION(0) try: - for ext in [cls(self.ctap2) for cls in self.extensions]: - auth_input, req_perms = ext.process_get_input_with_permissions( - client_inputs - ) + for ext in extension_instances: + auth_input = ext.process_get_input(client_inputs) if auth_input is not None: used_extensions.append(ext) - permissions |= req_perms extension_inputs[ext.NAME] = auth_input except ValueError as e: raise ClientError.ERR.CONFIGURATION_UNSUPPORTED(e) - pin_protocol, pin_token, pin_auth, internal_uv = self._get_auth_params( - client_data, rp_id, user_verification, permissions, event, on_keepalive - ) options = {"uv": True} if internal_uv else None - assertions = self.ctap2.get_assertions( rp_id, client_data.hash, diff --git a/fido2/ctap2/extensions.py b/fido2/ctap2/extensions.py index 9c754deb..d605893f 100644 --- a/fido2/ctap2/extensions.py +++ b/fido2/ctap2/extensions.py @@ -34,6 +34,7 @@ from enum import Enum, unique from typing import Dict, Tuple, Any, Optional import abc +import warnings class Ctap2Extension(abc.ABC): @@ -51,6 +52,9 @@ def is_supported(self) -> bool: """Whether or not the extension is supported by the authenticator.""" return self.NAME in self.ctap.info.extensions + def get_create_permissions(self, inputs: Dict[str, Any]) -> ClientPin.PERMISSION: + return ClientPin.PERMISSION(0) + def process_create_input(self, inputs: Dict[str, Any]) -> Any: """Returns a value to include in the authenticator extension input, or None. @@ -60,7 +64,11 @@ def process_create_input(self, inputs: Dict[str, Any]) -> Any: def process_create_input_with_permissions( self, inputs: Dict[str, Any] ) -> Tuple[Any, ClientPin.PERMISSION]: - return self.process_create_input(inputs), ClientPin.PERMISSION(0) + warnings.warn( + "This method is deprecated, use get_create_permissions.", DeprecationWarning + ) + + return self.process_create_input(inputs), self.get_create_permissions(inputs) def process_create_output( self, @@ -71,6 +79,9 @@ def process_create_output( """Return client extension output given attestation_response, or None.""" return None + def get_get_permissions(self, inputs: Dict[str, Any]) -> ClientPin.PERMISSION: + return ClientPin.PERMISSION(0) + def process_get_input(self, inputs: Dict[str, Any]) -> Any: """Returns a value to include in the authenticator extension input, or None. @@ -80,7 +91,10 @@ def process_get_input(self, inputs: Dict[str, Any]) -> Any: def process_get_input_with_permissions( self, inputs: Dict[str, Any] ) -> Tuple[Any, ClientPin.PERMISSION]: - return self.process_get_input(inputs), ClientPin.PERMISSION(0) + warnings.warn( + "This method is deprecated, use get_get_permissions.", DeprecationWarning + ) + return self.process_get_input(inputs), self.get_get_permissions(inputs) def process_get_output( self, @@ -209,9 +223,13 @@ def process_create_output(self, attestation_response, *args): "largeBlob": {"supported": attestation_response.large_blob_key is not None} } - def process_get_input_with_permissions(self, inputs): + def get_get_permissions(self, inputs): + if inputs.get("largeBlob", {}).get("write"): + return ClientPin.PERMISSION.LARGE_BLOB_WRITE + return ClientPin.PERMISSION(0) + + def process_get_input(self, inputs): data = inputs.get("largeBlob", {}) - permissions = ClientPin.PERMISSION(0) if data: if "support" in data or ("read" in data and "write" in data): raise ValueError("Invalid set of parameters") @@ -221,8 +239,7 @@ def process_get_input_with_permissions(self, inputs): self._action = True else: self._action = data.get("write") - permissions = ClientPin.PERMISSION.LARGE_BLOB_WRITE - return True if data else None, permissions + return True if data else None def process_get_output(self, assertion_response, token, pin_protocol): blob_key = assertion_response.large_blob_key From c94bda8a64ac8f1bc8a9a0a9361a46c308ef348f Mon Sep 17 00:00:00 2001 From: Dain Nilsson Date: Wed, 16 Oct 2024 12:57:34 +0200 Subject: [PATCH 03/13] Support PRF evalByCredential --- examples/prf.py | 16 ++++++++++++- fido2/client.py | 17 +++++++++----- fido2/ctap2/extensions.py | 48 +++++++++++++++++++++++++++++++++++---- 3 files changed, 69 insertions(+), 12 deletions(-) diff --git a/examples/prf.py b/examples/prf.py index 3b2e269d..f717526b 100644 --- a/examples/prf.py +++ b/examples/prf.py @@ -33,6 +33,7 @@ from fido2.hid import CtapHidDevice from fido2.server import Fido2Server from fido2.client import Fido2Client, WindowsClient, UserInteraction +from fido2.utils import websafe_encode from getpass import getpass import ctypes import sys @@ -119,6 +120,10 @@ def request_uv(self, permissions, rd_id): credential = result.attestation_object.auth_data.credential_data print("New credential created, with the PRF extension.") +# If created with UV, keep using UV +if result.attestation_object.auth_data.is_user_verified(): + uv = "required" + # Prepare parameters for getAssertion allow_list = [{"type": "public-key", "id": credential.credential_id}] @@ -155,7 +160,16 @@ def request_uv(self, permissions, rd_id): result = client.get_assertion( { **request_options["publicKey"], - "extensions": {"prf": {"eval": {"first": salt, "second": salt2}}}, + "extensions": { + "prf": { + "evalByCredential": { + websafe_encode(credential.credential_id): { + "first": salt, + "second": salt2, + } + } + } + }, } ) diff --git a/fido2/client.py b/fido2/client.py index e193a998..67aecb4b 100644 --- a/fido2/client.py +++ b/fido2/client.py @@ -37,6 +37,7 @@ Aaguid, AttestationObject, CollectedClientData, + PublicKeyCredentialDescriptor, PublicKeyCredentialCreationOptions, PublicKeyCredentialRequestOptions, AuthenticatorSelectionCriteria, @@ -466,7 +467,7 @@ def _filter_creds( # Filter out credential IDs which are too long max_len = self.info.max_cred_id_length if max_len: - cred_list = [e for e in cred_list if len(e) <= max_len] + cred_list = [c for c in cred_list if len(c.id) <= max_len] max_creds = self.info.max_creds_in_list or 1 chunks = [ @@ -495,9 +496,11 @@ def _filter_creds( ) if len(chunk) == 1 and len(assertions) == 1: # Credential ID might be omitted from assertions - matches.append(_cbor_list(chunk)[0]) + matches.append(chunk[0]) else: - matches.extend([a.credential for a in assertions]) + matches.extend( + [PublicKeyCredentialDescriptor(**a.credential) for a in assertions] + ) if len(matches) > max_creds: # Too many matches? Just return as many as we can handle @@ -670,7 +673,7 @@ def do_make_credential( _as_cbor(rp), _as_cbor(user), _cbor_list(key_params), - cred_list, + _cbor_list(cred_list), extension_inputs or None, options, pin_auth, @@ -738,7 +741,9 @@ def do_get_assertion( used_extensions = [] try: for ext in extension_instances: - auth_input = ext.process_get_input(client_inputs) + auth_input = ext._process_get_input_w_allow_list( + client_inputs, cred_list + ) if auth_input is not None: used_extensions.append(ext) extension_inputs[ext.NAME] = auth_input @@ -749,7 +754,7 @@ def do_get_assertion( assertions = self.ctap2.get_assertions( rp_id, client_data.hash, - cred_list, + _cbor_list(cred_list), extension_inputs or None, options, pin_auth, diff --git a/fido2/ctap2/extensions.py b/fido2/ctap2/extensions.py index d605893f..c692390b 100644 --- a/fido2/ctap2/extensions.py +++ b/fido2/ctap2/extensions.py @@ -30,11 +30,13 @@ from .base import AttestationResponse, AssertionResponse, Ctap2 from .pin import ClientPin, PinProtocol from .blob import LargeBlobs -from ..utils import sha256 +from ..utils import sha256, websafe_encode +from ..webauthn import PublicKeyCredentialDescriptor from enum import Enum, unique -from typing import Dict, Tuple, Any, Optional +from typing import Dict, Tuple, Any, Optional, List import abc import warnings +import inspect class Ctap2Extension(abc.ABC): @@ -82,12 +84,33 @@ def process_create_output( def get_get_permissions(self, inputs: Dict[str, Any]) -> ClientPin.PERMISSION: return ClientPin.PERMISSION(0) - def process_get_input(self, inputs: Dict[str, Any]) -> Any: + def process_get_input( + self, + inputs: Dict[str, Any], + allow_list: Optional[List[PublicKeyCredentialDescriptor]] = None, + ) -> Any: """Returns a value to include in the authenticator extension input, or None. """ return None + def _process_get_input_w_allow_list( + self, + inputs: Dict[str, Any], + allow_list: Optional[List[PublicKeyCredentialDescriptor]], + ) -> Any: + s = inspect.signature(self.process_get_input) + try: + s.bind(inputs, allow_list) + except TypeError: + warnings.warn( + f"{type(self)}.process_get_input() does not take allow_list, " + "which is deprecated.", + DeprecationWarning, + ) + return self.process_get_input(inputs) + return self.process_get_input(inputs, allow_list) + def process_get_input_with_permissions( self, inputs: Dict[str, Any] ) -> Tuple[Any, ClientPin.PERMISSION]: @@ -139,13 +162,28 @@ def process_create_output(self, attestation_response, *args): else: return {"hmacCreateSecret": enabled} - def process_get_input(self, inputs): + def process_get_input(self, inputs, allow_list=None): if not self.is_supported(): return data = inputs.get("prf") if data: - secrets = data.get("eval") + by_creds = data.get("evalByCredential") + if by_creds: + if not allow_list: + raise ValueError("evalByCredentials requires allowList") + for cred in allow_list: + key = websafe_encode(cred.id) + if key in by_creds: + secrets = by_creds[key] + # Remove any other creds from the allow_list + allow_list.clear() + allow_list.append(cred) + break + else: + raise ValueError("No matching credential ID found") + else: + secrets = data.get("eval") salts = ( _prf_salt(secrets["first"]), _prf_salt(secrets["second"]) if "second" in secrets else b"", From 55f14df6e1d5c87e668e0806195f77e289f1e0ae Mon Sep 17 00:00:00 2001 From: Dain Nilsson Date: Wed, 16 Oct 2024 13:37:46 +0200 Subject: [PATCH 04/13] Fix formatting of SW in traffic log --- fido2/pcsc.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/fido2/pcsc.py b/fido2/pcsc.py index 425ac149..965f7b70 100644 --- a/fido2/pcsc.py +++ b/fido2/pcsc.py @@ -112,9 +112,7 @@ def apdu_exchange( logger.log(LOG_LEVEL_TRAFFIC, "SEND: %s", apdu.hex()) resp, sw1, sw2 = self._conn.transmit(list(apdu), protocol) response = bytes(resp) - logger.log( - LOG_LEVEL_TRAFFIC, "RECV: %s SW=%04X", response.hex(), sw1 << 8 + sw2 - ) + logger.log(LOG_LEVEL_TRAFFIC, "RECV: %s SW=%02X%02X", response.hex(), sw1, sw2) return response, sw1, sw2 From 696a16d6f86b626d2f9d41ac32cb0e00fdf120f4 Mon Sep 17 00:00:00 2001 From: Dain Nilsson Date: Wed, 16 Oct 2024 14:15:21 +0200 Subject: [PATCH 05/13] Retry get_assertion on PUAT_REQUIRED --- examples/prf.py | 7 ++- fido2/client.py | 134 +++++++++++++++++++++++++++++------------------- 2 files changed, 86 insertions(+), 55 deletions(-) diff --git a/examples/prf.py b/examples/prf.py index f717526b..4447d5dc 100644 --- a/examples/prf.py +++ b/examples/prf.py @@ -149,12 +149,15 @@ def request_uv(self, permissions, rd_id): output1 = result.extension_results["prf"]["results"]["first"] print("Authenticated, secret:", output1.hex()) -# Authenticate again, using two salts to generate two secrets: +# Authenticate again, using two salts to generate two secrets. + +# This time we will use evalByCredential, which can be used if there are multiple +# credentials which use different salts. Here it is not needed, but provided for +# completeness of the example. # Generate a second salt for PRF: salt2 = os.urandom(32) print("Authenticate with second salt:", salt2.hex()) - # The first salt is reused, which should result in the same secret. result = client.get_assertion( diff --git a/fido2/client.py b/fido2/client.py index 67aecb4b..69833b59 100644 --- a/fido2/client.py +++ b/fido2/client.py @@ -530,13 +530,14 @@ def selection(self, event): return raise - def _should_use_uv(self, user_verification, mc): + def _should_use_uv(self, user_verification, permissions): uv_supported = any( k in self.info.options for k in ("uv", "clientPin", "bioEnroll") ) uv_configured = any( self.info.options.get(k) for k in ("uv", "clientPin", "bioEnroll") ) + mc = ClientPin.PERMISSION.MAKE_CREDENTIAL & permissions != 0 if ( user_verification == UserVerificationRequirement.REQUIRED @@ -582,19 +583,17 @@ def _get_token( ) def _get_auth_params( - self, client_data, rp_id, user_verification, permissions, event, on_keepalive + self, rp_id, user_verification, permissions, event, on_keepalive ): self.info = self.ctap2.get_info() # Make sure we have "fresh" info pin_protocol = None pin_token = None - pin_auth = None internal_uv = False additional_perms = permissions & ~( ClientPin.PERMISSION.MAKE_CREDENTIAL | ClientPin.PERMISSION.GET_ASSERTION ) - mc = client_data.type == CollectedClientData.TYPE.CREATE - if self._should_use_uv(user_verification, mc) or additional_perms: + if self._should_use_uv(user_verification, permissions) or additional_perms: client_pin = ClientPin(self.ctap2) allow_internal_uv = not additional_perms pin_token = self._get_token( @@ -602,10 +601,9 @@ def _get_auth_params( ) if pin_token: pin_protocol = client_pin.protocol - pin_auth = client_pin.protocol.authenticate(pin_token, client_data.hash) else: internal_uv = True - return pin_protocol, pin_token, pin_auth, internal_uv + return pin_protocol, pin_token, internal_uv def do_make_credential( self, @@ -635,8 +633,8 @@ def do_make_credential( permissions |= ext.get_create_permissions(client_inputs) # Handle auth - pin_protocol, pin_token, pin_auth, internal_uv = self._get_auth_params( - client_data, rp.id, user_verification, permissions, event, on_keepalive + pin_protocol, pin_token, internal_uv = self._get_auth_params( + rp.id, user_verification, permissions, event, on_keepalive ) if exclude_list: @@ -668,8 +666,16 @@ def do_make_credential( if internal_uv: options["uv"] = True + # Calculate pin_auth + client_data_hash = client_data.hash + if pin_token: + pin_auth = pin_protocol.authenticate(pin_token, client_data_hash) + else: + pin_auth = None + + # Perform make credential att_obj = self.ctap2.make_credential( - client_data.hash, + client_data_hash, _as_cbor(rp), _as_cbor(user), _cbor_list(key_params), @@ -722,54 +728,76 @@ def do_get_assertion( for ext in extension_instances: permissions |= ext.get_get_permissions(client_inputs) - # Handle auth - pin_protocol, pin_token, pin_auth, internal_uv = self._get_auth_params( - client_data, rp_id, user_verification, permissions, event, on_keepalive - ) - - if allow_list: - cred_list = self._filter_creds( - rp_id, allow_list, pin_protocol, pin_token, event, on_keepalive + def _do_auth(): + # Handle auth + pin_protocol, pin_token, internal_uv = self._get_auth_params( + rp_id, user_verification, permissions, event, on_keepalive ) - if not cred_list: - raise CtapError(CtapError.ERR.NO_CREDENTIALS) - else: - cred_list = None - # Process extensions - extension_inputs = {} - used_extensions = [] - try: - for ext in extension_instances: - auth_input = ext._process_get_input_w_allow_list( - client_inputs, cred_list + if allow_list: + cred_list = self._filter_creds( + rp_id, allow_list, pin_protocol, pin_token, event, on_keepalive ) - if auth_input is not None: - used_extensions.append(ext) - extension_inputs[ext.NAME] = auth_input - except ValueError as e: - raise ClientError.ERR.CONFIGURATION_UNSUPPORTED(e) + if not cred_list: + raise CtapError(CtapError.ERR.NO_CREDENTIALS) + else: + cred_list = None - options = {"uv": True} if internal_uv else None - assertions = self.ctap2.get_assertions( - rp_id, - client_data.hash, - _cbor_list(cred_list), - extension_inputs or None, - options, - pin_auth, - pin_protocol.VERSION if pin_protocol else None, - event=event, - on_keepalive=on_keepalive, - ) + # Process extensions + extension_inputs = {} + used_extensions = [] + try: + for ext in extension_instances: + auth_input = ext._process_get_input_w_allow_list( + client_inputs, cred_list + ) + if auth_input is not None: + used_extensions.append(ext) + extension_inputs[ext.NAME] = auth_input + except ValueError as e: + raise ClientError.ERR.CONFIGURATION_UNSUPPORTED(e) - return _Ctap2ClientAssertionSelection( - client_data, - assertions, - used_extensions, - pin_token, - pin_protocol, - ) + options = {"uv": True} if internal_uv else None + + # Calculate pin_auth + client_data_hash = client_data.hash + if pin_token: + pin_auth = pin_protocol.authenticate(pin_token, client_data_hash) + else: + pin_auth = None + + # Perform get assertion + assertions = self.ctap2.get_assertions( + rp_id, + client_data_hash, + _cbor_list(cred_list), + extension_inputs or None, + options, + pin_auth, + pin_protocol.VERSION if pin_protocol else None, + event=event, + on_keepalive=on_keepalive, + ) + + return _Ctap2ClientAssertionSelection( + client_data, + assertions, + used_extensions, + pin_token, + pin_protocol, + ) + + try: + return _do_auth() + except CtapError as e: + # The Authenticator may still require UV, try again + if ( + e.code == CtapError.ERR.PUAT_REQUIRED + and user_verification == UserVerificationRequirement.DISCOURAGED + ): + user_verification = UserVerificationRequirement.REQUIRED + return _do_auth() + raise class Fido2Client(WebAuthnClient, _BaseClient): From 9b5b22efdd9ba8d0b7491e7726e311d3645583a7 Mon Sep 17 00:00:00 2001 From: Dain Nilsson Date: Wed, 16 Oct 2024 14:51:36 +0200 Subject: [PATCH 06/13] Handle zero matching credentials in allow_list --- fido2/client.py | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/fido2/client.py b/fido2/client.py index 69833b59..954f1c00 100644 --- a/fido2/client.py +++ b/fido2/client.py @@ -483,17 +483,24 @@ def _filter_creds( version = None matches = [] for chunk in chunks: - assertions = self.ctap2.get_assertions( - rp_id, - client_data_hash, - _cbor_list(chunk), - None, - {"up": False}, - pin_auth, - version, - event=event, - on_keepalive=on_keepalive, - ) + try: + assertions = self.ctap2.get_assertions( + rp_id, + client_data_hash, + _cbor_list(chunk), + None, + {"up": False}, + pin_auth, + version, + event=event, + on_keepalive=on_keepalive, + ) + except CtapError as e: + if e.code == CtapError.ERR.NO_CREDENTIALS: + # All creds in chunk are discarded + continue + raise + if len(chunk) == 1 and len(assertions) == 1: # Credential ID might be omitted from assertions matches.append(chunk[0]) From 7d131bb72725976709385811925283e3237d1fcb Mon Sep 17 00:00:00 2001 From: Dain Nilsson Date: Wed, 16 Oct 2024 14:55:53 +0200 Subject: [PATCH 07/13] Fix adding permission --- fido2/client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fido2/client.py b/fido2/client.py index 954f1c00..dda30825 100644 --- a/fido2/client.py +++ b/fido2/client.py @@ -631,7 +631,7 @@ def do_make_credential( permissions = ClientPin.PERMISSION.MAKE_CREDENTIAL if exclude_list: # We need this for filtering the exclude_list - permissions = ClientPin.PERMISSION.GET_ASSERTION + permissions |= ClientPin.PERMISSION.GET_ASSERTION # Get extension permissions extension_instances = [cls(self.ctap2) for cls in self.extensions] From 4d8e7acd3a75e29ca623ae370ed9e0359bedcfdd Mon Sep 17 00:00:00 2001 From: Dain Nilsson Date: Wed, 16 Oct 2024 15:57:57 +0200 Subject: [PATCH 08/13] Retry make_credential on PUAT_REQUIRED --- examples/cred_blob.py | 4 +- examples/credential.py | 2 +- examples/large_blobs.py | 4 +- examples/resident_key.py | 4 +- fido2/client.py | 140 +++++++++++++++++++++++---------------- 5 files changed, 90 insertions(+), 64 deletions(-) diff --git a/examples/cred_blob.py b/examples/cred_blob.py index 769969cc..04d60bf1 100644 --- a/examples/cred_blob.py +++ b/examples/cred_blob.py @@ -82,9 +82,9 @@ def request_uv(self, permissions, rd_id): sys.exit(1) # Prefer UV token if supported - if client.info.options.get("pinUvAuthToken") or client.info.options.get("uv"): + if client.info.options.get("uv") or client.info.options.get("bioEnroll"): uv = "preferred" - print("Authenticator supports UV token") + print("Authenticator is configured for User Verification") server = Fido2Server({"id": "example.com", "name": "Example RP"}) diff --git a/examples/credential.py b/examples/credential.py index a1085b5a..8fef2118 100644 --- a/examples/credential.py +++ b/examples/credential.py @@ -79,7 +79,7 @@ def request_uv(self, permissions, rd_id): client = Fido2Client(dev, "https://example.com", user_interaction=CliInteraction()) # Prefer UV if supported and configured - if client.info.options.get("uv") or client.info.options.get("pinUvAuthToken"): + if client.info.options.get("uv") or client.info.options.get("bioEnroll"): uv = "preferred" print("Authenticator supports User Verification") diff --git a/examples/large_blobs.py b/examples/large_blobs.py index a3e4ac90..f4c55af4 100644 --- a/examples/large_blobs.py +++ b/examples/large_blobs.py @@ -88,9 +88,9 @@ def request_uv(self, permissions, rd_id): sys.exit(1) # Prefer UV token if supported - if client.info.options.get("pinUvAuthToken") or client.info.options.get("uv"): + if client.info.options.get("uv") or client.info.options.get("bioEnroll"): uv = "preferred" - print("Authenticator supports UV token") + print("Authenticator is configured for User Verification") server = Fido2Server({"id": "example.com", "name": "Example RP"}) diff --git a/examples/resident_key.py b/examples/resident_key.py index f85577be..2634a22d 100644 --- a/examples/resident_key.py +++ b/examples/resident_key.py @@ -83,9 +83,9 @@ def request_uv(self, permissions, rd_id): sys.exit(1) # Prefer UV if supported and configured - if client.info.options.get("uv") or client.info.options.get("pinUvAuthToken"): + if client.info.options.get("uv") or client.info.options.get("bioEnroll"): uv = "preferred" - print("Authenticator supports User Verification") + print("Authenticator is configured for User Verification") server = Fido2Server({"id": "example.com", "name": "Example RP"}, attestation="direct") diff --git a/fido2/client.py b/fido2/client.py index dda30825..7bf82a57 100644 --- a/fido2/client.py +++ b/fido2/client.py @@ -545,6 +545,9 @@ def _should_use_uv(self, user_verification, permissions): self.info.options.get(k) for k in ("uv", "clientPin", "bioEnroll") ) mc = ClientPin.PERMISSION.MAKE_CREDENTIAL & permissions != 0 + additional_perms = permissions & ~( + ClientPin.PERMISSION.MAKE_CREDENTIAL | ClientPin.PERMISSION.GET_ASSERTION + ) if ( user_verification == UserVerificationRequirement.REQUIRED @@ -561,6 +564,8 @@ def _should_use_uv(self, user_verification, permissions): return True elif mc and uv_configured and not self.info.options.get("makeCredUvNotRqd"): return True + elif uv_configured and additional_perms: + return True return False def _get_token( @@ -597,12 +602,16 @@ def _get_auth_params( pin_protocol = None pin_token = None internal_uv = False - additional_perms = permissions & ~( - ClientPin.PERMISSION.MAKE_CREDENTIAL | ClientPin.PERMISSION.GET_ASSERTION - ) - if self._should_use_uv(user_verification, permissions) or additional_perms: + if self._should_use_uv(user_verification, permissions): client_pin = ClientPin(self.ctap2) - allow_internal_uv = not additional_perms + allow_internal_uv = ( + permissions + & ~( + ClientPin.PERMISSION.MAKE_CREDENTIAL + | ClientPin.PERMISSION.GET_ASSERTION + ) + == 0 + ) pin_token = self._get_token( client_pin, permissions, rp_id, event, on_keepalive, allow_internal_uv ) @@ -635,66 +644,83 @@ def do_make_credential( # Get extension permissions extension_instances = [cls(self.ctap2) for cls in self.extensions] + used_extensions = [] client_inputs = extensions or {} for ext in extension_instances: permissions |= ext.get_create_permissions(client_inputs) - # Handle auth - pin_protocol, pin_token, internal_uv = self._get_auth_params( - rp.id, user_verification, permissions, event, on_keepalive - ) - - if exclude_list: - cred_list = self._filter_creds( - rp.id, exclude_list, pin_protocol, pin_token, event, on_keepalive + def _do_make(): + # Handle auth + pin_protocol, pin_token, internal_uv = self._get_auth_params( + rp.id, user_verification, permissions, event, on_keepalive ) - else: - cred_list = None - # Process extensions - extension_inputs = {} - used_extensions = [] - permissions = ClientPin.PERMISSION.MAKE_CREDENTIAL - try: - for ext in extension_instances: - auth_input = ext.process_create_input(client_inputs) - if auth_input is not None: - used_extensions.append(ext) - extension_inputs[ext.NAME] = auth_input - except ValueError as e: - raise ClientError.ERR.CONFIGURATION_UNSUPPORTED(e) + if exclude_list: + cred_list = self._filter_creds( + rp.id, exclude_list, pin_protocol, pin_token, event, on_keepalive + ) + else: + cred_list = None - if not (rk or internal_uv): - options = None - else: - options = {} - if rk: - options["rk"] = True - if internal_uv: - options["uv"] = True - - # Calculate pin_auth - client_data_hash = client_data.hash - if pin_token: - pin_auth = pin_protocol.authenticate(pin_token, client_data_hash) - else: - pin_auth = None + # Process extensions + extension_inputs = {} + try: + for ext in extension_instances: + auth_input = ext.process_create_input(client_inputs) + if auth_input is not None: + used_extensions.append(ext) + extension_inputs[ext.NAME] = auth_input + except ValueError as e: + raise ClientError.ERR.CONFIGURATION_UNSUPPORTED(e) - # Perform make credential - att_obj = self.ctap2.make_credential( - client_data_hash, - _as_cbor(rp), - _as_cbor(user), - _cbor_list(key_params), - _cbor_list(cred_list), - extension_inputs or None, - options, - pin_auth, - pin_protocol.VERSION if pin_protocol else None, - enterprise_attestation, - event=event, - on_keepalive=on_keepalive, - ) + if not (rk or internal_uv): + options = None + else: + options = {} + if rk: + options["rk"] = True + if internal_uv: + options["uv"] = True + + # Calculate pin_auth + client_data_hash = client_data.hash + if pin_token: + pin_auth = pin_protocol.authenticate(pin_token, client_data_hash) + else: + pin_auth = None + + # Perform make credential + return ( + self.ctap2.make_credential( + client_data_hash, + _as_cbor(rp), + _as_cbor(user), + _cbor_list(key_params), + _cbor_list(cred_list), + extension_inputs or None, + options, + pin_auth, + pin_protocol.VERSION if pin_protocol else None, + enterprise_attestation, + event=event, + on_keepalive=on_keepalive, + ), + pin_protocol, + pin_token, + ) + + try: + att_obj, pin_protocol, pin_token = _do_make() + except CtapError as e: + # The Authenticator may still require UV, try again + if ( + e.code == CtapError.ERR.PUAT_REQUIRED + and user_verification == UserVerificationRequirement.DISCOURAGED + ): + user_verification = UserVerificationRequirement.REQUIRED + att_obj, pin_protocol, pin_token = _do_make() + else: + raise # Process extenstion outputs extension_outputs = {} From 44f5e2349af219f556aacc14d902ea10057ad675 Mon Sep 17 00:00:00 2001 From: Dain Nilsson Date: Thu, 17 Oct 2024 13:40:13 +0200 Subject: [PATCH 09/13] Always filter down to a single credential ID --- fido2/client.py | 39 +++++++++++++++++---------------------- fido2/ctap2/extensions.py | 29 +++++++++++------------------ 2 files changed, 28 insertions(+), 40 deletions(-) diff --git a/fido2/client.py b/fido2/client.py index 7bf82a57..95768e5e 100644 --- a/fido2/client.py +++ b/fido2/client.py @@ -481,7 +481,7 @@ def _filter_creds( else: pin_auth = None version = None - matches = [] + for chunk in chunks: try: assertions = self.ctap2.get_assertions( @@ -495,24 +495,19 @@ def _filter_creds( event=event, on_keepalive=on_keepalive, ) + if len(chunk) == 1: + # Credential ID might be omitted from assertions + return chunk[0] + else: + return PublicKeyCredentialDescriptor(**assertions[0].credential) except CtapError as e: if e.code == CtapError.ERR.NO_CREDENTIALS: # All creds in chunk are discarded continue raise - if len(chunk) == 1 and len(assertions) == 1: - # Credential ID might be omitted from assertions - matches.append(chunk[0]) - else: - matches.extend( - [PublicKeyCredentialDescriptor(**a.credential) for a in assertions] - ) - - if len(matches) > max_creds: - # Too many matches? Just return as many as we can handle - return matches[:max_creds] - return matches + # No matches found + return None def selection(self, event): if "FIDO_2_1" in self.info.versions: @@ -656,11 +651,11 @@ def _do_make(): ) if exclude_list: - cred_list = self._filter_creds( + exclude_cred = self._filter_creds( rp.id, exclude_list, pin_protocol, pin_token, event, on_keepalive ) - else: - cred_list = None + if exclude_cred: + raise CtapError(CtapError.ERR.CREDENTIAL_EXCLUDED) # Process extensions extension_inputs = {} @@ -696,7 +691,7 @@ def _do_make(): _as_cbor(rp), _as_cbor(user), _cbor_list(key_params), - _cbor_list(cred_list), + None, extension_inputs or None, options, pin_auth, @@ -768,13 +763,13 @@ def _do_auth(): ) if allow_list: - cred_list = self._filter_creds( + allow_cred = self._filter_creds( rp_id, allow_list, pin_protocol, pin_token, event, on_keepalive ) - if not cred_list: + if not allow_cred: raise CtapError(CtapError.ERR.NO_CREDENTIALS) else: - cred_list = None + allow_cred = None # Process extensions extension_inputs = {} @@ -782,7 +777,7 @@ def _do_auth(): try: for ext in extension_instances: auth_input = ext._process_get_input_w_allow_list( - client_inputs, cred_list + client_inputs, allow_cred ) if auth_input is not None: used_extensions.append(ext) @@ -803,7 +798,7 @@ def _do_auth(): assertions = self.ctap2.get_assertions( rp_id, client_data_hash, - _cbor_list(cred_list), + [_as_cbor(allow_cred)] if allow_cred else None, extension_inputs or None, options, pin_auth, diff --git a/fido2/ctap2/extensions.py b/fido2/ctap2/extensions.py index c692390b..7b8464af 100644 --- a/fido2/ctap2/extensions.py +++ b/fido2/ctap2/extensions.py @@ -33,7 +33,7 @@ from ..utils import sha256, websafe_encode from ..webauthn import PublicKeyCredentialDescriptor from enum import Enum, unique -from typing import Dict, Tuple, Any, Optional, List +from typing import Dict, Tuple, Any, Optional import abc import warnings import inspect @@ -87,7 +87,7 @@ def get_get_permissions(self, inputs: Dict[str, Any]) -> ClientPin.PERMISSION: def process_get_input( self, inputs: Dict[str, Any], - allow_list: Optional[List[PublicKeyCredentialDescriptor]] = None, + allow_credential: Optional[PublicKeyCredentialDescriptor] = None, ) -> Any: """Returns a value to include in the authenticator extension input, or None. @@ -97,19 +97,19 @@ def process_get_input( def _process_get_input_w_allow_list( self, inputs: Dict[str, Any], - allow_list: Optional[List[PublicKeyCredentialDescriptor]], + allow_credential: Optional[PublicKeyCredentialDescriptor], ) -> Any: s = inspect.signature(self.process_get_input) try: - s.bind(inputs, allow_list) + s.bind(inputs, allow_credential) except TypeError: warnings.warn( - f"{type(self)}.process_get_input() does not take allow_list, " + f"{type(self)}.process_get_input() does not take allow_credential, " "which is deprecated.", DeprecationWarning, ) return self.process_get_input(inputs) - return self.process_get_input(inputs, allow_list) + return self.process_get_input(inputs, allow_credential) def process_get_input_with_permissions( self, inputs: Dict[str, Any] @@ -162,7 +162,7 @@ def process_create_output(self, attestation_response, *args): else: return {"hmacCreateSecret": enabled} - def process_get_input(self, inputs, allow_list=None): + def process_get_input(self, inputs, allow_credential=None): if not self.is_supported(): return @@ -170,17 +170,10 @@ def process_get_input(self, inputs, allow_list=None): if data: by_creds = data.get("evalByCredential") if by_creds: - if not allow_list: - raise ValueError("evalByCredentials requires allowList") - for cred in allow_list: - key = websafe_encode(cred.id) - if key in by_creds: - secrets = by_creds[key] - # Remove any other creds from the allow_list - allow_list.clear() - allow_list.append(cred) - break - else: + if not allow_credential: + raise ValueError("evalByCredentials requires allowCredentials") + secrets = by_creds.get(websafe_encode(allow_credential.id)) + if not secrets: raise ValueError("No matching credential ID found") else: secrets = data.get("eval") From c6d3598d6618f260491d038a5eed2a1cf3e03de1 Mon Sep 17 00:00:00 2001 From: Dain Nilsson Date: Thu, 17 Oct 2024 14:09:23 +0200 Subject: [PATCH 10/13] Don't abort early on excludeCredential match --- fido2/client.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/fido2/client.py b/fido2/client.py index 95768e5e..eba6416c 100644 --- a/fido2/client.py +++ b/fido2/client.py @@ -654,8 +654,11 @@ def _do_make(): exclude_cred = self._filter_creds( rp.id, exclude_list, pin_protocol, pin_token, event, on_keepalive ) - if exclude_cred: - raise CtapError(CtapError.ERR.CREDENTIAL_EXCLUDED) + # We know the request will fail if exclude_cred is not None here + # BUT DO NOT FAIL EARLY! We still need to prompt for UP, so we keep + # processing the request + else: + exclude_cred = None # Process extensions extension_inputs = {} @@ -691,7 +694,7 @@ def _do_make(): _as_cbor(rp), _as_cbor(user), _cbor_list(key_params), - None, + [_as_cbor(exclude_cred)] if exclude_cred else None, extension_inputs or None, options, pin_auth, From f58bdc628b56475519fdea8dd15a19e168a72f70 Mon Sep 17 00:00:00 2001 From: Dain Nilsson Date: Thu, 17 Oct 2024 14:35:33 +0200 Subject: [PATCH 11/13] Don't abort early on allowCredentials without a match --- fido2/client.py | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/fido2/client.py b/fido2/client.py index eba6416c..ecb51aeb 100644 --- a/fido2/client.py +++ b/fido2/client.py @@ -766,13 +766,14 @@ def _do_auth(): ) if allow_list: - allow_cred = self._filter_creds( + selected_cred = self._filter_creds( rp_id, allow_list, pin_protocol, pin_token, event, on_keepalive ) - if not allow_cred: - raise CtapError(CtapError.ERR.NO_CREDENTIALS) + # We know the request will fail if selected_cred is None here + # BUT DO NOT FAIL EARLY! We still need to prompt for UP, so we keep + # processing the request else: - allow_cred = None + selected_cred = None # Process extensions extension_inputs = {} @@ -780,7 +781,7 @@ def _do_auth(): try: for ext in extension_instances: auth_input = ext._process_get_input_w_allow_list( - client_inputs, allow_cred + client_inputs, selected_cred ) if auth_input is not None: used_extensions.append(ext) @@ -797,11 +798,16 @@ def _do_auth(): else: pin_auth = None + if allow_list and not selected_cred: + # We still need to send a dummy value if there was an allow_list + # but no matches were found: + selected_cred = PublicKeyCredentialDescriptor(allow_list[0].type, b"\0") + # Perform get assertion assertions = self.ctap2.get_assertions( rp_id, client_data_hash, - [_as_cbor(allow_cred)] if allow_cred else None, + [_as_cbor(selected_cred)] if selected_cred else None, extension_inputs or None, options, pin_auth, From c804a4ef856a69836060f5425014727813a7b4b3 Mon Sep 17 00:00:00 2001 From: Dain Nilsson Date: Thu, 17 Oct 2024 14:57:18 +0200 Subject: [PATCH 12/13] Pass full options to do_make_credential/do_get_assertion --- fido2/client.py | 90 ++++++++++++++++++++++++------------------------- 1 file changed, 44 insertions(+), 46 deletions(-) diff --git a/fido2/client.py b/fido2/client.py index ecb51aeb..167faaaf 100644 --- a/fido2/client.py +++ b/fido2/client.py @@ -325,22 +325,23 @@ def selection(self, event): def do_make_credential( self, + options, client_data, rp, - user, - key_params, - exclude_list, - extensions, - rk, - user_verification, - enterprise_attestation, + enterprise_rpid_list, event, ): + key_params = options.pub_key_cred_params + exclude_list = options.exclude_credentials + selection = options.authenticator_selection or AuthenticatorSelectionCriteria() + rk = selection.require_resident_key + user_verification = selection.user_verification + if ( rk or user_verification == UserVerificationRequirement.REQUIRED or ES256.ALGORITHM not in [p.alg for p in key_params] - or enterprise_attestation + or options.attestation == AttestationConveyancePreference.ENTERPRISE ): raise CtapError(CtapError.ERR.UNSUPPORTED_OPTION) @@ -383,13 +384,14 @@ def do_make_credential( def do_get_assertion( self, + options, client_data, - rp_id, - allow_list, - extensions, - user_verification, event, ): + rp_id = options.rp_id + allow_list = options.allow_credentials + user_verification = options.user_verification + if user_verification == UserVerificationRequirement.REQUIRED or not allow_list: raise CtapError(CtapError.ERR.UNSUPPORTED_OPTION) @@ -618,17 +620,31 @@ def _get_auth_params( def do_make_credential( self, + options, client_data, rp, - user, - key_params, - exclude_list, - extensions, - rk, - user_verification, - enterprise_attestation, + enterprise_rpid_list, event, ): + user = options.user + key_params = options.pub_key_cred_params + exclude_list = options.exclude_credentials + extensions = options.extensions + selection = options.authenticator_selection or AuthenticatorSelectionCriteria() + rk = selection.require_resident_key + user_verification = selection.user_verification + + enterprise_attestation = None + if options.attestation == AttestationConveyancePreference.ENTERPRISE: + if self.info.options.get("ep"): + if enterprise_rpid_list is not None: + # Platform facilitated + if rp.id in enterprise_rpid_list: + enterprise_attestation = 2 + else: + # Vendor facilitated + enterprise_attestation = 1 + on_keepalive = _user_keepalive(self.user_interaction) # Gather up permissions @@ -741,13 +757,15 @@ def _do_make(): def do_get_assertion( self, + options, client_data, - rp_id, - allow_list, - extensions, - user_verification, event, ): + rp_id = options.rp_id + allow_list = options.allow_credentials + extensions = options.extensions + user_verification = options.user_verification + on_keepalive = _user_keepalive(self.user_interaction) # Gather up permissions @@ -912,29 +930,12 @@ def make_credential( CollectedClientData.TYPE.CREATE, options.challenge ) - selection = options.authenticator_selection or AuthenticatorSelectionCriteria() - enterprise_attestation = None - if options.attestation == AttestationConveyancePreference.ENTERPRISE: - if self.info.options.get("ep"): - if self._enterprise_rpid_list is not None: - # Platform facilitated - if rp.id in self._enterprise_rpid_list: - enterprise_attestation = 2 - else: - # Vendor facilitated - enterprise_attestation = 1 - try: return self._backend.do_make_credential( + options, client_data, rp, - options.user, - options.pub_key_cred_params, - options.exclude_credentials, - options.extensions, - selection.require_resident_key, - selection.user_verification, - enterprise_attestation, + self._enterprise_rpid_list, event, ) except CtapError as e: @@ -970,11 +971,8 @@ def get_assertion( try: return self._backend.do_get_assertion( + options, client_data, - options.rp_id, - options.allow_credentials, - options.extensions, - options.user_verification, event, ) except CtapError as e: From 4b5a32577ba80da2a01d2f655b33ec41727edcee Mon Sep 17 00:00:00 2001 From: Dain Nilsson Date: Thu, 17 Oct 2024 15:39:53 +0200 Subject: [PATCH 13/13] Pass options to extensions --- fido2/client.py | 15 +++++++---- fido2/ctap2/extensions.py | 57 ++++++++++++++++++--------------------- 2 files changed, 36 insertions(+), 36 deletions(-) diff --git a/fido2/client.py b/fido2/client.py index 167faaaf..00493616 100644 --- a/fido2/client.py +++ b/fido2/client.py @@ -634,6 +634,9 @@ def do_make_credential( rk = selection.require_resident_key user_verification = selection.user_verification + on_keepalive = _user_keepalive(self.user_interaction) + + # Handle enterprise attestation enterprise_attestation = None if options.attestation == AttestationConveyancePreference.ENTERPRISE: if self.info.options.get("ep"): @@ -645,8 +648,6 @@ def do_make_credential( # Vendor facilitated enterprise_attestation = 1 - on_keepalive = _user_keepalive(self.user_interaction) - # Gather up permissions permissions = ClientPin.PERMISSION.MAKE_CREDENTIAL if exclude_list: @@ -658,6 +659,8 @@ def do_make_credential( used_extensions = [] client_inputs = extensions or {} for ext in extension_instances: + # TODO: Move options to the constructor instead + ext._create_options = options permissions |= ext.get_create_permissions(client_inputs) def _do_make(): @@ -775,6 +778,8 @@ def do_get_assertion( extension_instances = [cls(self.ctap2) for cls in self.extensions] client_inputs = extensions or {} for ext in extension_instances: + # TODO: Move options to get_get_permissions and process_get_input + ext._get_options = options permissions |= ext.get_get_permissions(client_inputs) def _do_auth(): @@ -798,9 +803,9 @@ def _do_auth(): used_extensions = [] try: for ext in extension_instances: - auth_input = ext._process_get_input_w_allow_list( - client_inputs, selected_cred - ) + # TODO: Move to process_get_input() + ext._selected = selected_cred + auth_input = ext.process_get_input(client_inputs) if auth_input is not None: used_extensions.append(ext) extension_inputs[ext.NAME] = auth_input diff --git a/fido2/ctap2/extensions.py b/fido2/ctap2/extensions.py index 7b8464af..6c4f13c0 100644 --- a/fido2/ctap2/extensions.py +++ b/fido2/ctap2/extensions.py @@ -31,12 +31,15 @@ from .pin import ClientPin, PinProtocol from .blob import LargeBlobs from ..utils import sha256, websafe_encode -from ..webauthn import PublicKeyCredentialDescriptor +from ..webauthn import ( + PublicKeyCredentialDescriptor, + PublicKeyCredentialCreationOptions, + PublicKeyCredentialRequestOptions, +) from enum import Enum, unique from typing import Dict, Tuple, Any, Optional import abc import warnings -import inspect class Ctap2Extension(abc.ABC): @@ -49,6 +52,10 @@ class Ctap2Extension(abc.ABC): def __init__(self, ctap: Ctap2): self.ctap = ctap + # TODO: Pass options and selected to the various methods that need them instead + self._create_options: PublicKeyCredentialCreationOptions + self._get_options: PublicKeyCredentialRequestOptions + self._selected: Optional[PublicKeyCredentialDescriptor] def is_supported(self) -> bool: """Whether or not the extension is supported by the authenticator.""" @@ -84,33 +91,12 @@ def process_create_output( def get_get_permissions(self, inputs: Dict[str, Any]) -> ClientPin.PERMISSION: return ClientPin.PERMISSION(0) - def process_get_input( - self, - inputs: Dict[str, Any], - allow_credential: Optional[PublicKeyCredentialDescriptor] = None, - ) -> Any: + def process_get_input(self, inputs: Dict[str, Any]) -> Any: """Returns a value to include in the authenticator extension input, or None. """ return None - def _process_get_input_w_allow_list( - self, - inputs: Dict[str, Any], - allow_credential: Optional[PublicKeyCredentialDescriptor], - ) -> Any: - s = inspect.signature(self.process_get_input) - try: - s.bind(inputs, allow_credential) - except TypeError: - warnings.warn( - f"{type(self)}.process_get_input() does not take allow_credential, " - "which is deprecated.", - DeprecationWarning, - ) - return self.process_get_input(inputs) - return self.process_get_input(inputs, allow_credential) - def process_get_input_with_permissions( self, inputs: Dict[str, Any] ) -> Tuple[Any, ClientPin.PERMISSION]: @@ -162,21 +148,30 @@ def process_create_output(self, attestation_response, *args): else: return {"hmacCreateSecret": enabled} - def process_get_input(self, inputs, allow_credential=None): + def process_get_input(self, inputs): if not self.is_supported(): return data = inputs.get("prf") if data: + secrets = data.get("eval") by_creds = data.get("evalByCredential") if by_creds: - if not allow_credential: + # Make sure all keys are valid IDs from allow_credentials + allow_list = self._get_options.allow_credentials + if not allow_list: raise ValueError("evalByCredentials requires allowCredentials") - secrets = by_creds.get(websafe_encode(allow_credential.id)) - if not secrets: - raise ValueError("No matching credential ID found") - else: - secrets = data.get("eval") + ids = {websafe_encode(c.id) for c in allow_list} + if not ids.issuperset(by_creds): + raise ValueError("evalByCredentials contains invalid key") + if self._selected: + key = websafe_encode(self._selected.id) + if key in by_creds: + secrets = by_creds[key] + + if not secrets: + return + salts = ( _prf_salt(secrets["first"]), _prf_salt(secrets["second"]) if "second" in secrets else b"",