Remove unnecessary RelyingPartyWrapper type

jschanck · jschanck · commit d37f5ad4b2df · 2023-09-26T11:50:55.000-07:00
diff --git a/src/ctap2/commands/get_assertion.rs b/src/ctap2/commands/get_assertion.rs
@@ -13,7 +13,7 @@ use crate::ctap2::commands::get_next_assertion::GetNextAssertion;
 use crate::ctap2::commands::make_credentials::UserVerification;
 use crate::ctap2::server::{
     AuthenticationExtensionsClientInputs, AuthenticationExtensionsClientOutputs,
-    PublicKeyCredentialDescriptor, PublicKeyCredentialUserEntity, RelyingPartyWrapper, RpIdHash,
+    PublicKeyCredentialDescriptor, PublicKeyCredentialUserEntity, RelyingParty, RpIdHash,
     UserVerificationRequirement,
 };
 use crate::ctap2::utils::{read_be_u32, read_byte};
@@ -159,7 +159,7 @@ impl GetAssertionExtensions {
 #[derive(Debug, Clone)]
 pub struct GetAssertion {
     pub client_data_hash: ClientDataHash,
-    pub rp: RelyingPartyWrapper,
+    pub rp: RelyingParty,
     pub allow_list: Vec<PublicKeyCredentialDescriptor>,
 
     // https://www.w3.org/TR/webauthn/#client-extension-input
@@ -176,7 +176,7 @@ pub struct GetAssertion {
 impl GetAssertion {
     pub fn new(
         client_data_hash: ClientDataHash,
-        rp: RelyingPartyWrapper,
+        rp: RelyingParty,
         allow_list: Vec<PublicKeyCredentialDescriptor>,
         options: GetAssertionOptions,
         extensions: GetAssertionExtensions,
@@ -195,10 +195,8 @@ impl GetAssertion {
         // Handle extensions whose outputs are not encoded in the authenticator data.
         // 1. appId
         if let Some(app_id) = &self.extensions.app_id {
-            result.extensions.app_id = Some(
-                result.assertion.auth_data.rp_id_hash
-                    == RelyingPartyWrapper::from(app_id.as_str()).hash(),
-            );
+            result.extensions.app_id =
+                Some(result.assertion.auth_data.rp_id_hash == RelyingParty::from(app_id).hash());
         }
     }
 }
@@ -225,11 +223,7 @@ impl PinUvAuthCommand for GetAssertion {
     }
 
     fn get_rp_id(&self) -> Option<&String> {
-        match &self.rp {
-            // CTAP1 case: We only have the hash, not the entire RpID
-            RelyingPartyWrapper::Hash(..) => None,
-            RelyingPartyWrapper::Data(r) => Some(&r.id),
-        }
+        Some(&self.rp.id)
     }
 
     fn can_skip_user_verification(
@@ -273,17 +267,7 @@ impl Serialize for GetAssertion {
         }
 
         let mut map = serializer.serialize_map(Some(map_len))?;
-        match self.rp {
-            RelyingPartyWrapper::Data(ref d) => {
-                map.serialize_entry(&1, &d.id)?;
-            }
-            _ => {
-                return Err(S::Error::custom(
-                    "Can't serialize a RelyingParty::Hash for CTAP2",
-                ));
-            }
-        }
-
+        map.serialize_entry(&1, &self.rp.id)?;
         map.serialize_entry(&2, &self.client_data_hash)?;
         if !self.allow_list.is_empty() {
             map.serialize_entry(&3, &self.allow_list)?;
@@ -621,8 +605,8 @@ pub mod test {
         do_credential_list_filtering_ctap1, do_credential_list_filtering_ctap2,
     };
     use crate::ctap2::server::{
-        PublicKeyCredentialDescriptor, PublicKeyCredentialUserEntity, RelyingParty,
-        RelyingPartyWrapper, RpIdHash, Transport,
+        PublicKeyCredentialDescriptor, PublicKeyCredentialUserEntity, RelyingParty, RpIdHash,
+        Transport,
     };
     use crate::transport::device_selector::Device;
     use crate::transport::hid::HIDDevice;
@@ -641,10 +625,7 @@ pub mod test {
         };
         let assertion = GetAssertion::new(
             client_data.hash().expect("failed to serialize client data"),
-            RelyingPartyWrapper::Data(RelyingParty {
-                id: String::from("example.com"),
-                name: Some(String::from("Acme")),
-            }),
+            RelyingParty::from("example.com"),
             vec![PublicKeyCredentialDescriptor {
                 id: vec![
                     0x3E, 0xBD, 0x89, 0xBF, 0x77, 0xEC, 0x50, 0x97, 0x55, 0xEE, 0x9C, 0x26, 0x35,
@@ -851,10 +832,7 @@ pub mod test {
         };
         let mut assertion = GetAssertion::new(
             client_data.hash().expect("failed to serialize client data"),
-            RelyingPartyWrapper::Data(RelyingParty {
-                id: String::from("example.com"),
-                name: Some(String::from("Acme")),
-            }),
+            RelyingParty::from("example.com"),
             vec![allowed_key.clone()],
             GetAssertionOptions {
                 user_presence: Some(true),
@@ -942,10 +920,7 @@ pub mod test {
         };
         let mut assertion = GetAssertion::new(
             client_data.hash().expect("failed to serialize client data"),
-            RelyingPartyWrapper::Data(RelyingParty {
-                id: String::from("example.com"),
-                name: Some(String::from("Acme")),
-            }),
+            RelyingParty::from("example.com"),
             vec![too_long_key_handle.clone()],
             GetAssertionOptions {
                 user_presence: Some(true),
@@ -1081,10 +1056,7 @@ pub mod test {
         };
         let assertion = GetAssertion::new(
             client_data.hash().expect("failed to serialize client data"),
-            RelyingPartyWrapper::Data(RelyingParty {
-                id: String::from("example.com"),
-                name: Some(String::from("Acme")),
-            }),
+            RelyingParty::from("example.com"),
             vec![
                 // This should never be tested, because it gets pre-filtered, since it is too long
                 // (see max_credential_id_length)
diff --git a/src/ctap2/commands/make_credentials.rs b/src/ctap2/commands/make_credentials.rs
@@ -15,8 +15,7 @@ use crate::ctap2::client_data::ClientDataHash;
 use crate::ctap2::server::{
     AuthenticationExtensionsClientInputs, AuthenticationExtensionsClientOutputs,
     CredentialProtectionPolicy, PublicKeyCredentialDescriptor, PublicKeyCredentialParameters,
-    PublicKeyCredentialUserEntity, RelyingParty, RelyingPartyWrapper, RpIdHash,
-    UserVerificationRequirement,
+    PublicKeyCredentialUserEntity, RelyingParty, RpIdHash, UserVerificationRequirement,
 };
 use crate::ctap2::utils::{read_byte, serde_parse_err};
 use crate::errors::AuthenticatorError;
@@ -25,7 +24,7 @@ use crate::transport::{FidoDevice, VirtualFidoDevice};
 use crate::u2ftypes::CTAP1RequestAPDU;
 use serde::{
     de::{Error as DesError, MapAccess, Unexpected, Visitor},
-    ser::{Error as SerError, SerializeMap},
+    ser::SerializeMap,
     Deserialize, Deserializer, Serialize, Serializer,
 };
 use serde_cbor::{self, de::from_slice, ser, Value};
@@ -259,7 +258,7 @@ impl From<AuthenticationExtensionsClientInputs> for MakeCredentialsExtensions {
 #[derive(Debug, Clone)]
 pub struct MakeCredentials {
     pub client_data_hash: ClientDataHash,
-    pub rp: RelyingPartyWrapper,
+    pub rp: RelyingParty,
     // Note(baloo): If none -> ctap1
     pub user: Option<PublicKeyCredentialUserEntity>,
     pub pub_cred_params: Vec<PublicKeyCredentialParameters>,
@@ -281,7 +280,7 @@ impl MakeCredentials {
     #[allow(clippy::too_many_arguments)]
     pub fn new(
         client_data_hash: ClientDataHash,
-        rp: RelyingPartyWrapper,
+        rp: RelyingParty,
         user: Option<PublicKeyCredentialUserEntity>,
         pub_cred_params: Vec<PublicKeyCredentialParameters>,
         exclude_list: Vec<PublicKeyCredentialDescriptor>,
@@ -350,11 +349,7 @@ impl PinUvAuthCommand for MakeCredentials {
     }
 
     fn get_rp_id(&self) -> Option<&String> {
-        match &self.rp {
-            // CTAP1 case: We only have the hash, not the entire RpID
-            RelyingPartyWrapper::Hash(..) => None,
-            RelyingPartyWrapper::Data(r) => Some(&r.id),
-        }
+        Some(&self.rp.id)
     }
 
     fn can_skip_user_verification(
@@ -417,16 +412,7 @@ impl Serialize for MakeCredentials {
 
         let mut map = serializer.serialize_map(Some(map_len))?;
         map.serialize_entry(&0x01, &self.client_data_hash)?;
-        match self.rp {
-            RelyingPartyWrapper::Data(ref d) => {
-                map.serialize_entry(&0x02, &d)?;
-            }
-            _ => {
-                return Err(S::Error::custom(
-                    "Can't serialize a RelyingParty::Hash for CTAP2",
-                ));
-            }
-        }
+        map.serialize_entry(&0x02, &self.rp)?;
         map.serialize_entry(&0x03, &self.user)?;
         map.serialize_entry(&0x04, &self.pub_cred_params)?;
         if !self.exclude_list.is_empty() {
@@ -561,10 +547,7 @@ pub(crate) fn dummy_make_credentials_cmd() -> MakeCredentials {
             208, 206, 230, 252, 125, 191, 89, 154, 145, 157, 184, 251, 149, 19, 17, 38, 159, 14,
             183, 129, 247, 132, 28, 108, 192, 84, 74, 217, 218, 52, 21, 75,
         ]),
-        RelyingPartyWrapper::Data(RelyingParty {
-            id: String::from("make.me.blink"),
-            ..Default::default()
-        }),
+        RelyingParty::from("make.me.blink"),
         Some(PublicKeyCredentialUserEntity {
             id: vec![0],
             name: Some(String::from("make.me.blink")),
@@ -599,7 +582,6 @@ pub mod test {
     use crate::ctap2::server::RpIdHash;
     use crate::ctap2::server::{
         PublicKeyCredentialParameters, PublicKeyCredentialUserEntity, RelyingParty,
-        RelyingPartyWrapper,
     };
     use crate::transport::device_selector::Device;
     use crate::transport::hid::HIDDevice;
@@ -618,10 +600,10 @@ pub mod test {
             }
             .hash()
             .expect("failed to serialize client data"),
-            RelyingPartyWrapper::Data(RelyingParty {
+            RelyingParty {
                 id: String::from("example.com"),
                 name: Some(String::from("Acme")),
-            }),
+            },
             Some(PublicKeyCredentialUserEntity {
                 id: base64::engine::general_purpose::URL_SAFE
                     .decode("MIIBkzCCATigAwIBAjCCAZMwggE4oAMCAQIwggGTMII=")
@@ -675,10 +657,7 @@ pub mod test {
             }
             .hash()
             .expect("failed to serialize client data"),
-            RelyingPartyWrapper::Data(RelyingParty {
-                id: String::from("example.com"),
-                name: Some(String::from("Acme")),
-            }),
+            RelyingParty::from("example.com"),
             Some(PublicKeyCredentialUserEntity {
                 id: base64::engine::general_purpose::URL_SAFE
                     .decode("MIIBkzCCATigAwIBAjCCAZMwggE4oAMCAQIwggGTMII=")
diff --git a/src/ctap2/mod.rs b/src/ctap2/mod.rs
@@ -35,8 +35,7 @@ use crate::ctap2::preflight::{
     silently_discover_credentials,
 };
 use crate::ctap2::server::{
-    CredentialProtectionPolicy, RelyingPartyWrapper, ResidentKeyRequirement,
-    UserVerificationRequirement,
+    CredentialProtectionPolicy, RelyingParty, ResidentKeyRequirement, UserVerificationRequirement,
 };
 use crate::errors::{AuthenticatorError, UnsupportedOption};
 use crate::statecallback::StateCallback;
@@ -470,7 +469,7 @@ pub fn register<Dev: FidoDevice>(
 
     let mut makecred = MakeCredentials::new(
         ClientDataHash(args.client_data_hash),
-        RelyingPartyWrapper::Data(args.relying_party),
+        args.relying_party,
         Some(args.user),
         args.pub_cred_params,
         args.exclude_list,
@@ -574,28 +573,25 @@ pub fn sign<Dev: FidoDevice>(
     }
 
     let mut allow_list = args.allow_list;
-    let mut rp_id = args.relying_party_id;
+    let mut rp_id = RelyingParty::from(args.relying_party_id);
     let client_data_hash = ClientDataHash(args.client_data_hash);
     if let Some(ref app_id) = args.extensions.app_id {
         if !allow_list.is_empty() {
             // Try to silently discover U2F credentials that require the FIDO App ID extension. If
             // any are found, we should use the alternate RP ID instead of the provided RP ID.
-            let silent_creds = silently_discover_credentials(
-                dev,
-                &allow_list,
-                &RelyingPartyWrapper::from(app_id.as_str()),
-                &client_data_hash,
-            );
+            let alt_rp_id = RelyingParty::from(app_id);
+            let silent_creds =
+                silently_discover_credentials(dev, &allow_list, &alt_rp_id, &client_data_hash);
             if !silent_creds.is_empty() {
                 allow_list = silent_creds;
-                rp_id = app_id.to_string();
+                rp_id = alt_rp_id;
             }
         }
     }
 
     let mut get_assertion = GetAssertion::new(
         client_data_hash,
-        RelyingPartyWrapper::from(rp_id.as_str()),
+        rp_id,
         allow_list,
         GetAssertionOptions {
             user_presence: Some(args.user_presence_req),
diff --git a/src/ctap2/preflight.rs b/src/ctap2/preflight.rs
@@ -3,7 +3,7 @@ use super::commands::get_assertion::{GetAssertion, GetAssertionExtensions, GetAs
 use super::commands::{PinUvAuthCommand, RequestCtap1, Retryable};
 use crate::consts::{PARAMETER_SIZE, U2F_AUTHENTICATE, U2F_CHECK_IS_REGISTERED};
 use crate::crypto::PinUvAuthToken;
-use crate::ctap2::server::{PublicKeyCredentialDescriptor, RelyingPartyWrapper};
+use crate::ctap2::server::{PublicKeyCredentialDescriptor, RelyingParty};
 use crate::errors::AuthenticatorError;
 use crate::transport::errors::{ApduErrorStatus, HIDError};
 use crate::transport::{FidoDevice, FidoProtocol, VirtualFidoDevice};
@@ -19,7 +19,7 @@ use sha2::{Digest, Sha256};
 pub struct CheckKeyHandle<'assertion> {
     pub key_handle: &'assertion [u8],
     pub client_data_hash: &'assertion [u8],
-    pub rp: &'assertion RelyingPartyWrapper,
+    pub rp: &'assertion RelyingParty,
 }
 
 impl<'assertion> RequestCtap1 for CheckKeyHandle<'assertion> {
@@ -81,7 +81,7 @@ impl<'assertion> RequestCtap1 for CheckKeyHandle<'assertion> {
 pub(crate) fn do_credential_list_filtering_ctap1<Dev: FidoDevice>(
     dev: &mut Dev,
     cred_list: &[PublicKeyCredentialDescriptor],
-    rp: &RelyingPartyWrapper,
+    rp: &RelyingParty,
     client_data_hash: &ClientDataHash,
 ) -> Option<PublicKeyCredentialDescriptor> {
     let key_handle = cred_list
@@ -112,7 +112,7 @@ pub(crate) fn do_credential_list_filtering_ctap1<Dev: FidoDevice>(
 pub(crate) fn do_credential_list_filtering_ctap2<Dev: FidoDevice>(
     dev: &mut Dev,
     cred_list: &[PublicKeyCredentialDescriptor],
-    rp: &RelyingPartyWrapper,
+    rp: &RelyingParty,
     pin_uv_auth_token: Option<PinUvAuthToken>,
 ) -> Result<Vec<PublicKeyCredentialDescriptor>, AuthenticatorError> {
     let info = dev
@@ -190,7 +190,7 @@ pub(crate) fn do_credential_list_filtering_ctap2<Dev: FidoDevice>(
 pub(crate) fn silently_discover_credentials<Dev: FidoDevice>(
     dev: &mut Dev,
     cred_list: &[PublicKeyCredentialDescriptor],
-    rp: &RelyingPartyWrapper,
+    rp: &RelyingParty,
     client_data_hash: &ClientDataHash,
 ) -> Vec<PublicKeyCredentialDescriptor> {
     if dev.get_protocol() == FidoProtocol::CTAP2 {
diff --git a/src/ctap2/server.rs b/src/ctap2/server.rs
