From 0561bdb94126a51c36e7731cadb1de56d5c89b1f Mon Sep 17 00:00:00 2001
From: Dain Nilsson <dain@yubico.com>
Date: Mon, 18 Nov 2024 17:29:09 +0100
Subject: [PATCH] Improve handling of RP ID when omitted from options

---
 fido2/client.py | 65 ++++++++++++++++++++++++++++++++++---------------
 1 file changed, 45 insertions(+), 20 deletions(-)

diff --git a/fido2/client.py b/fido2/client.py
index 057b971..e42111f 100644
--- a/fido2/client.py
+++ b/fido2/client.py
@@ -41,6 +41,7 @@
     Aaguid,
     AttestationObject,
     CollectedClientData,
+    PublicKeyCredentialRpEntity,
     PublicKeyCredentialDescriptor,
     PublicKeyCredentialCreationOptions,
     PublicKeyCredentialRequestOptions,
@@ -57,7 +58,6 @@
 from .utils import sha256
 from enum import IntEnum, unique
 from urllib.parse import urlparse
-from dataclasses import replace
 from threading import Timer, Event
 from typing import (
     Type,
@@ -303,11 +303,25 @@ def selection(self, event: Optional[Event]) -> None:
         raise NotImplementedError()
 
     @abc.abstractmethod
-    def do_make_credential(self, *args) -> AuthenticatorAttestationResponse:
+    def do_make_credential(
+        self,
+        options: PublicKeyCredentialCreationOptions,
+        client_data: CollectedClientData,
+        rp: PublicKeyCredentialRpEntity,
+        rp_id: str,
+        enterprise_rpid_list: Optional[Sequence[str]],
+        event: Event,
+    ) -> AuthenticatorAttestationResponse:
         raise NotImplementedError()
 
     @abc.abstractmethod
-    def do_get_assertion(self, *args) -> AssertionSelection:
+    def do_get_assertion(
+        self,
+        options: PublicKeyCredentialRequestOptions,
+        client_data: CollectedClientData,
+        rp_id: str,
+        event: Event,
+    ) -> AssertionSelection:
         raise NotImplementedError()
 
 
@@ -333,6 +347,7 @@ def do_make_credential(
         options,
         client_data,
         rp,
+        rp_id,
         enterprise_rpid_list,
         event,
     ):
@@ -350,7 +365,7 @@ def do_make_credential(
         ):
             raise CtapError(CtapError.ERR.UNSUPPORTED_OPTION)
 
-        app_param = sha256(rp.id.encode())
+        app_param = sha256(rp_id.encode())
 
         dummy_param = b"\0" * 32
         for cred in exclude_list or []:
@@ -391,9 +406,9 @@ def do_get_assertion(
         self,
         options,
         client_data,
+        rp_id,
         event,
     ):
-        rp_id = options.rp_id
         allow_list = options.allow_credentials
         user_verification = options.user_verification
 
@@ -636,6 +651,7 @@ def do_make_credential(
         options,
         client_data,
         rp,
+        rp_id,
         enterprise_rpid_list,
         event,
     ):
@@ -653,7 +669,7 @@ def do_make_credential(
             if self.info.options.get("ep"):
                 if enterprise_rpid_list is not None:
                     # Platform facilitated
-                    if rp.id in enterprise_rpid_list:
+                    if rp_id in enterprise_rpid_list:
                         enterprise_attestation = 2
                 else:
                     # Vendor facilitated
@@ -676,12 +692,12 @@ def do_make_credential(
         def _do_make():
             # Handle auth
             pin_protocol, pin_token, internal_uv = self._get_auth_params(
-                rp.id, user_verification, permissions, event, on_keepalive
+                rp_id, user_verification, permissions, event, on_keepalive
             )
 
             if exclude_list:
                 exclude_cred = self._filter_creds(
-                    rp.id, exclude_list, pin_protocol, pin_token, event, on_keepalive
+                    rp_id, exclude_list, pin_protocol, pin_token, event, on_keepalive
                 )
                 # 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
@@ -719,7 +735,7 @@ def _do_make():
 
             # Calculate pin_auth
             client_data_hash = client_data.hash
-            if pin_token:
+            if pin_protocol and pin_token:
                 pin_auth = pin_protocol.authenticate(pin_token, client_data_hash)
             else:
                 pin_auth = None
@@ -790,6 +806,7 @@ def do_get_assertion(
         self,
         options,
         client_data,
+        rp_id,
         event,
     ):
         rp_id = options.rp_id
@@ -939,6 +956,17 @@ def selection(self, event: Optional[Event] = None) -> None:
         except CtapError as e:
             raise _ctap2client_err(e)
 
+    def _get_rp_id(self, rp_id: Optional[str]) -> str:
+        if rp_id is None:
+            url = urlparse(self.origin)
+            if url.scheme != "https" or not url.netloc:
+                raise ClientError.ERR.BAD_REQUEST(
+                    "RP ID required for non-https origin."
+                )
+            return url.netloc
+        else:
+            return rp_id
+
     def make_credential(
         self,
         options: PublicKeyCredentialCreationOptions,
@@ -958,16 +986,10 @@ def make_credential(
             timer.start()
 
         rp = options.rp
-        if rp.id is None:
-            url = urlparse(self.origin)
-            if url.scheme != "https" or not url.netloc:
-                raise ClientError.ERR.BAD_REQUEST(
-                    "RP ID required for non-https origin."
-                )
-            rp = replace(rp, id=url.netloc)
+        rp_id = self._get_rp_id(rp.id)
 
-        logger.debug(f"Register a new credential for RP ID: {rp.id}")
-        self._verify_rp_id(rp.id)
+        logger.debug(f"Register a new credential for RP ID: {rp_id}")
+        self._verify_rp_id(rp_id)
 
         client_data = self._build_client_data(
             CollectedClientData.TYPE.CREATE, options.challenge
@@ -978,6 +1000,7 @@ def make_credential(
                 options,
                 client_data,
                 rp,
+                rp_id,
                 self._enterprise_rpid_list,
                 event,
             )
@@ -1005,8 +1028,9 @@ def get_assertion(
             timer.daemon = True
             timer.start()
 
-        logger.debug(f"Assert a credential for RP ID: {options.rp_id}")
-        self._verify_rp_id(options.rp_id)
+        rp_id = self._get_rp_id(options.rp_id)
+        logger.debug(f"Assert a credential for RP ID: {rp_id}")
+        self._verify_rp_id(rp_id)
 
         client_data = self._build_client_data(
             CollectedClientData.TYPE.GET, options.challenge
@@ -1016,6 +1040,7 @@ def get_assertion(
             return self._backend.do_get_assertion(
                 options,
                 client_data,
+                rp_id,
                 event,
             )
         except CtapError as e: