diff --git a/libwebauthn/Cargo.lock b/libwebauthn/Cargo.lock index cd07b69..a0c9e85 100644 --- a/libwebauthn/Cargo.lock +++ b/libwebauthn/Cargo.lock @@ -1477,6 +1477,108 @@ version = "1.10.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6dbf3de79e51f3d586ab4cb9d5c3e2c14aa28ed23d180cf89b4df0454a69cc87" +[[package]] +name = "icu_collections" +version = "2.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4c6b649701667bbe825c3b7e6388cb521c23d88644678e83c0c4d0a621a34b43" +dependencies = [ + "displaydoc", + "potential_utf", + "yoke", + "zerofrom", + "zerovec", +] + +[[package]] +name = "icu_locale_core" +version = "2.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "edba7861004dd3714265b4db54a3c390e880ab658fec5f7db895fae2046b5bb6" +dependencies = [ + "displaydoc", + "litemap", + "tinystr", + "writeable", + "zerovec", +] + +[[package]] +name = "icu_normalizer" +version = "2.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5f6c8828b67bf8908d82127b2054ea1b4427ff0230ee9141c54251934ab1b599" +dependencies = [ + "icu_collections", + "icu_normalizer_data", + "icu_properties", + "icu_provider", + "smallvec", + "zerovec", +] + +[[package]] +name = "icu_normalizer_data" +version = "2.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7aedcccd01fc5fe81e6b489c15b247b8b0690feb23304303a9e560f37efc560a" + +[[package]] +name = "icu_properties" +version = "2.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "020bfc02fe870ec3a66d93e677ccca0562506e5872c650f893269e08615d74ec" +dependencies = [ + "icu_collections", + "icu_locale_core", + "icu_properties_data", + "icu_provider", + "zerotrie", + "zerovec", +] + +[[package]] +name = "icu_properties_data" +version = "2.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "616c294cf8d725c6afcd8f55abc17c56464ef6211f9ed59cccffe534129c77af" + +[[package]] +name = "icu_provider" +version = "2.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "85962cf0ce02e1e0a629cc34e7ca3e373ce20dda4c4d7294bbd0bf1fdb59e614" +dependencies = [ + "displaydoc", + "icu_locale_core", + "writeable", + "yoke", + "zerofrom", + "zerotrie", + "zerovec", +] + +[[package]] +name = "idna" +version = "1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3b0875f23caa03898994f6ddc501886a45c7d3d62d04d2d90788d47be1b1e4de" +dependencies = [ + "idna_adapter", + "smallvec", + "utf8_iter", +] + +[[package]] +name = "idna_adapter" +version = "1.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3acae9609540aa318d1bc588455225fb2085b9ed0c4f6bd0d9d5bcd86f1a0344" +dependencies = [ + "icu_normalizer", + "icu_properties", +] + [[package]] name = "image" version = "0.25.9" @@ -1676,6 +1778,7 @@ dependencies = [ "hidapi", "hkdf", "hmac 0.12.1", + "idna", "interchange", "littlefs2", "maplit", @@ -1728,6 +1831,12 @@ version = "0.11.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "df1d3c3b53da64cf5760482273a98e575c651a67eec7f77df96b5b642de8f039" +[[package]] +name = "litemap" +version = "0.8.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6373607a59f0be73a39b6fe456b8192fcc3585f602af20751600e974dd455e77" + [[package]] name = "littlefs2" version = "0.6.1" @@ -2231,6 +2340,15 @@ version = "0.1.5-pre" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7c68cb38ed13fd7bc9dd5db8f165b7c8d9c1a315104083a2b10f11354c2af97f" +[[package]] +name = "potential_utf" +version = "0.1.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b73949432f5e2a09657003c25bca5e19a0e9c84f8058ca374f49e0ebe605af77" +dependencies = [ + "zerovec", +] + [[package]] name = "powerfmt" version = "0.2.0" @@ -3134,6 +3252,16 @@ dependencies = [ "time-core", ] +[[package]] +name = "tinystr" +version = "0.8.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "42d3e9c45c09de15d06dd8acf5f4e0e399e85927b7f00711024eb7ae10fa4869" +dependencies = [ + "displaydoc", + "zerovec", +] + [[package]] name = "tokio" version = "1.48.0" @@ -3467,6 +3595,12 @@ version = "0.7.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "09cc8ee72d2a9becf2f2febe0205bbed8fc6615b7cb429ad062dc7b7ddd036a9" +[[package]] +name = "utf8_iter" +version = "1.0.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b6c140620e7ffbb22c2dee59cafe6084a59b5ffc27a8859a5f0d494b5d52b6be" + [[package]] name = "utf8parse" version = "0.2.2" @@ -3998,6 +4132,12 @@ version = "0.46.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f17a85883d4e6d00e8a97c586de764dabcc06133f7f1d55dce5cdc070ad7fe59" +[[package]] +name = "writeable" +version = "0.6.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9edde0db4769d2dc68579893f2306b26c6ecfbe0ef499b013d731b7b9247e0b9" + [[package]] name = "x509-parser" version = "0.17.0" @@ -4021,6 +4161,29 @@ version = "1.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2df5825faced2427b2da74d9100f1e2e93c533fff063506a81ede1cf517b2e7e" +[[package]] +name = "yoke" +version = "0.8.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "72d6e5c6afb84d73944e5cedb052c4680d5657337201555f9f2a16b7406d4954" +dependencies = [ + "stable_deref_trait", + "yoke-derive", + "zerofrom", +] + +[[package]] +name = "yoke-derive" +version = "0.8.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b659052874eb698efe5b9e8cf382204678a0086ebf46982b79d6ca3182927e5d" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.111", + "synstructure 0.13.2", +] + [[package]] name = "zerocopy" version = "0.8.31" @@ -4041,6 +4204,27 @@ dependencies = [ "syn 2.0.111", ] +[[package]] +name = "zerofrom" +version = "0.1.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "50cc42e0333e05660c3587f3bf9d0478688e15d870fab3346451ce7f8c9fbea5" +dependencies = [ + "zerofrom-derive", +] + +[[package]] +name = "zerofrom-derive" +version = "0.1.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d71e5d6e06ab090c67b5e44993ec16b72dcbaabc526db883a360057678b48502" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.111", + "synstructure 0.13.2", +] + [[package]] name = "zeroize" version = "1.8.2" @@ -4061,6 +4245,39 @@ dependencies = [ "syn 2.0.111", ] +[[package]] +name = "zerotrie" +version = "0.2.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2a59c17a5562d507e4b54960e8569ebee33bee890c70aa3fe7b97e85a9fd7851" +dependencies = [ + "displaydoc", + "yoke", + "zerofrom", +] + +[[package]] +name = "zerovec" +version = "0.11.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6c28719294829477f525be0186d13efa9a3c602f7ec202ca9e353d310fb9a002" +dependencies = [ + "yoke", + "zerofrom", + "zerovec-derive", +] + +[[package]] +name = "zerovec-derive" +version = "0.11.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "eadce39539ca5cb3985590102671f2567e659fca9666581ad3411d59207951f3" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.111", +] + [[package]] name = "zmij" version = "0.1.9" diff --git a/libwebauthn/Cargo.toml b/libwebauthn/Cargo.toml index 9f623b8..a9ee270 100644 --- a/libwebauthn/Cargo.toml +++ b/libwebauthn/Cargo.toml @@ -27,6 +27,7 @@ libnfc = [ base64-url = "3.0.0" dbus = "0.9.5" tracing = "0.1.29" +idna = "1.0.3" maplit = "1.0.2" sha2 = "0.10.2" uuid = { version = "1.5.0", features = ["serde", "v4"] } diff --git a/libwebauthn/src/ops/webauthn/get_assertion.rs b/libwebauthn/src/ops/webauthn/get_assertion.rs index 92dc906..41e65d1 100644 --- a/libwebauthn/src/ops/webauthn/get_assertion.rs +++ b/libwebauthn/src/ops/webauthn/get_assertion.rs @@ -83,6 +83,12 @@ pub enum GetAssertionRequestParsingError { #[error("Not supported: {0}")] NotSupported(String), + + #[error("Invalid relying party ID: {0}")] + InvalidRelyingPartyId(String), + + #[error("Mismatching relying party ID: {0} != {1}")] + MismatchingRelyingPartyId(String, String), } impl WebAuthnIDL for GetAssertionRequest { @@ -107,6 +113,18 @@ impl FromInnerModel Result { + if let Some(relying_party_id) = inner.relying_party_id.as_deref() { + let parsed = RelyingPartyId::try_from(relying_party_id).map_err(|err| { + GetAssertionRequestParsingError::InvalidRelyingPartyId(err.to_string()) + })?; + // TODO(#160): Add support for related origin per WebAuthn Level 3. + if parsed.0 != rpid.0 { + return Err(GetAssertionRequestParsingError::MismatchingRelyingPartyId( + parsed.0, + rpid.0.to_string(), + )); + } + } let hmac_or_prf = match inner.extensions.clone() { Some(ext) => { if let Some(prf) = ext.prf { @@ -624,13 +642,30 @@ mod tests { } #[test] - fn test_request_from_json_ignore_request_rp_id() { + fn test_request_from_json_invalid_rp_id() { let rpid = RelyingPartyId::try_from("example.org").unwrap(); - let req_json = json_field_rm(REQUEST_BASE_JSON, "rpId"); - let req_json = json_field_add(&req_json, "rpId", r#""another-example.org""#); + let req_json = json_field_add(&REQUEST_BASE_JSON, "rpId", r#""example.org.""#); - let req: GetAssertionRequest = GetAssertionRequest::from_json(&rpid, &req_json).unwrap(); - assert_eq!(req, request_base()); + let result = GetAssertionRequest::from_json(&rpid, &req_json); + assert!(matches!( + result, + Err(GetAssertionRequestParsingError::InvalidRelyingPartyId(_)) + )); + } + + #[test] + fn test_request_from_json_mismatching_rp_id() { + let rpid = RelyingPartyId::try_from("example.org").unwrap(); + let req_json = json_field_add(&REQUEST_BASE_JSON, "rpId", r#""other.example.org""#); + + let result = GetAssertionRequest::from_json(&rpid, &req_json); + assert!(matches!( + result, + Err(GetAssertionRequestParsingError::MismatchingRelyingPartyId( + _, + _ + )) + )); } #[test] diff --git a/libwebauthn/src/ops/webauthn/idl/rpid.rs b/libwebauthn/src/ops/webauthn/idl/rpid.rs index a92b12b..1c58912 100644 --- a/libwebauthn/src/ops/webauthn/idl/rpid.rs +++ b/libwebauthn/src/ops/webauthn/idl/rpid.rs @@ -1,5 +1,5 @@ use serde::Deserialize; -use std::{convert::TryFrom, ops::Deref}; +use std::{convert::TryFrom, net::IpAddr, ops::Deref}; #[derive(Clone, Debug)] pub struct RelyingPartyId(pub String); @@ -19,22 +19,77 @@ impl Into for RelyingPartyId { } #[derive(thiserror::Error, Debug, Clone)] -// TODO(#137): Validate RelyingPartyId pub enum Error { #[error("Empty Relying Party ID is not allowed")] EmptyRelyingPartyId, + #[error("Relying Party ID must be a valid domain string: {0}")] + InvalidRelyingPartyId(String), + #[error("Relying Party ID must not be an IP address: {0}")] + IpAddressNotAllowed(String), + #[error("Relying Party ID exceeds maximum length")] + DomainTooLong, + #[error("Relying Party ID label exceeds maximum length: {0}")] + LabelTooLong(String), } impl TryFrom<&str> for RelyingPartyId { type Error = Error; + /// Validates and normalizes a Relying Party ID per WebAuthn L3 §4. + /// + /// Required by spec: + /// - RP ID must be a "valid domain string" (WHATWG URL Standard §3.4) + /// - IDNA normalization via `domain_to_ascii` (URL Standard §3.3, UTS #46) + /// - IP addresses are not valid domains (URL Standard §3.1) + /// + /// DNS constraints (RFC 1035, enforced by IDNA with beStrict=true): + /// - Domain max length: 253 characters + /// - Label max length: 63 characters + /// - Labels must follow LDH rule (letters, digits, hyphens) + /// - Labels cannot start/end with hyphens fn try_from(value: &str) -> Result { - // TODO(#137): Validate RelyingPartyId, including IDNA normalization - // and checking for valid characters. - match value { - "" => Err(Error::EmptyRelyingPartyId), - _ => Ok(RelyingPartyId(value.to_string())), + if value.is_empty() { + return Err(Error::EmptyRelyingPartyId); } + + if value.parse::().is_ok() { + return Err(Error::IpAddressNotAllowed(value.to_string())); + } + + let ascii = idna::domain_to_ascii(value) + .map_err(|_| Error::InvalidRelyingPartyId(value.to_string()))?; + + if ascii.is_empty() { + return Err(Error::InvalidRelyingPartyId(value.to_string())); + } + + if ascii.len() > 253 { + return Err(Error::DomainTooLong); + } + + if ascii.starts_with('.') || ascii.ends_with('.') { + return Err(Error::InvalidRelyingPartyId(value.to_string())); + } + + for label in ascii.split('.') { + if label.is_empty() { + return Err(Error::InvalidRelyingPartyId(value.to_string())); + } + if label.len() > 63 { + return Err(Error::LabelTooLong(label.to_string())); + } + if label.starts_with('-') || label.ends_with('-') { + return Err(Error::InvalidRelyingPartyId(value.to_string())); + } + if !label + .chars() + .all(|ch| ch.is_ascii_alphanumeric() || ch == '-') + { + return Err(Error::InvalidRelyingPartyId(value.to_string())); + } + } + + Ok(RelyingPartyId(ascii)) } } @@ -47,3 +102,46 @@ impl<'de> Deserialize<'de> for RelyingPartyId { RelyingPartyId::try_from(s.as_str()).map_err(serde::de::Error::custom) } } + +#[cfg(test)] +mod tests { + use super::{Error, RelyingPartyId}; + use std::convert::TryFrom; + + #[test] + fn test_relying_party_id_valid() { + let rpid = RelyingPartyId::try_from("example.org").unwrap(); + assert_eq!(rpid.0, "example.org"); + } + + #[test] + fn test_relying_party_id_idna_normalization() { + // IDNA (UTS #46) example. + let rpid = RelyingPartyId::try_from("例え.テスト").unwrap(); + assert_eq!(rpid.0, "xn--r8jz45g.xn--zckzah"); + } + + #[test] + fn test_relying_party_id_empty() { + let result = RelyingPartyId::try_from(""); + assert!(matches!(result, Err(Error::EmptyRelyingPartyId))); + } + + #[test] + fn test_relying_party_id_rejects_ip_address() { + let result = RelyingPartyId::try_from("127.0.0.1"); + assert!(matches!(result, Err(Error::IpAddressNotAllowed(_)))); + } + + #[test] + fn test_relying_party_id_invalid_label_chars() { + let result = RelyingPartyId::try_from("bad_label.example"); + assert!(matches!(result, Err(Error::InvalidRelyingPartyId(_)))); + } + + #[test] + fn test_relying_party_id_invalid_trailing_dot() { + let result = RelyingPartyId::try_from("example.org."); + assert!(matches!(result, Err(Error::InvalidRelyingPartyId(_)))); + } +} diff --git a/libwebauthn/src/ops/webauthn/make_credential.rs b/libwebauthn/src/ops/webauthn/make_credential.rs index 3c19075..74f1360 100644 --- a/libwebauthn/src/ops/webauthn/make_credential.rs +++ b/libwebauthn/src/ops/webauthn/make_credential.rs @@ -368,6 +368,20 @@ impl FromInnerModel Result { + let rp_id = RelyingPartyId::try_from(inner.rp.id.as_str()).map_err(|err| { + MakeCredentialRequestParsingError::InvalidRelyingPartyId(err.to_string()) + })?; + // TODO(#160): Add support for related origin per WebAuthn Level 3. + if rp_id.0 != rpid.0 { + return Err( + MakeCredentialRequestParsingError::MismatchingRelyingPartyId( + rp_id.0, + rpid.0.to_string(), + ), + ); + } + let mut relying_party = inner.rp; + relying_party.id = rp_id.0; let resident_key = if inner .authenticator_selection .as_ref() @@ -398,7 +412,7 @@ impl FromInnerModel for MakeCredentialRequest { @@ -829,6 +847,38 @@ mod tests { ); } + #[test] + fn test_request_from_json_invalid_rp_id() { + let rpid = RelyingPartyId::try_from("example.org").unwrap(); + let req_json = json_field_add( + REQUEST_BASE_JSON, + "rp", + r#"{"id": "example.org.", "name": "example.org"}"#, + ); + + let result = MakeCredentialRequest::from_json(&rpid, &req_json); + assert!(matches!( + result, + Err(MakeCredentialRequestParsingError::InvalidRelyingPartyId(_)) + )); + } + + #[test] + fn test_request_from_json_mismatching_rp_id() { + let rpid = RelyingPartyId::try_from("example.org").unwrap(); + let req_json = json_field_add( + REQUEST_BASE_JSON, + "rp", + r#"{"id": "other.example.org", "name": "example.org"}"#, + ); + + let result = MakeCredentialRequest::from_json(&rpid, &req_json); + assert!(matches!( + result, + Err(MakeCredentialRequestParsingError::MismatchingRelyingPartyId(_, _)) + )); + } + // Tests for response JSON serialization fn create_test_response() -> MakeCredentialResponse {