From 1975d7ff433c141000e48da453199f66ece4aa69 Mon Sep 17 00:00:00 2001 From: Alfie Fresta Date: Fri, 26 Dec 2025 18:56:53 -0500 Subject: [PATCH 1/2] Fix #137: Add RP ID validation with IDNA normalization --- libwebauthn/Cargo.lock | 217 ++++++++++++++++++ libwebauthn/Cargo.toml | 1 + libwebauthn/src/ops/webauthn/get_assertion.rs | 46 +++- libwebauthn/src/ops/webauthn/idl/rpid.rs | 112 ++++++++- .../src/ops/webauthn/make_credential.rs | 52 ++++- 5 files changed, 415 insertions(+), 13 deletions(-) diff --git a/libwebauthn/Cargo.lock b/libwebauthn/Cargo.lock index f4e6b95..185fafb 100644 --- a/libwebauthn/Cargo.lock +++ b/libwebauthn/Cargo.lock @@ -1420,6 +1420,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" @@ -1603,6 +1705,7 @@ dependencies = [ "hidapi", "hkdf", "hmac 0.12.1", + "idna", "interchange", "littlefs2", "maplit", @@ -1649,6 +1752,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" @@ -2146,6 +2255,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" @@ -3039,6 +3157,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.49.0" @@ -3372,6 +3500,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" @@ -3891,6 +4025,12 @@ version = "0.51.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d7249219f66ced02969388cf2bb044a09756a083d0fab1e566056b04d9fbcaa5" +[[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" @@ -3914,6 +4054,29 @@ version = "1.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b8aa498d22c9bbaf482329839bc5620c46be275a19a812e9a22a2b07529a642a" +[[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.114", + "synstructure 0.13.2", +] + [[package]] name = "zerocopy" version = "0.8.34" @@ -3934,6 +4097,27 @@ dependencies = [ "syn 2.0.114", ] +[[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.114", + "synstructure 0.13.2", +] + [[package]] name = "zeroize" version = "1.8.2" @@ -3954,6 +4138,39 @@ dependencies = [ "syn 2.0.114", ] +[[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.114", +] + [[package]] name = "zmij" version = "1.0.17" diff --git a/libwebauthn/Cargo.toml b/libwebauthn/Cargo.toml index ee0a6e2..3ccbd94 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 e8db340..bc129de 100644 --- a/libwebauthn/src/ops/webauthn/get_assertion.rs +++ b/libwebauthn/src/ops/webauthn/get_assertion.rs @@ -57,6 +57,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 { @@ -81,6 +87,19 @@ 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 prf = match inner.extensions.as_ref() { Some(ext) => match &ext.prf { Some(prf_json) => Some(PrfInput::try_from(prf_json.clone())?), @@ -507,13 +526,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 b568714..b2d04fc 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 From for String { } #[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 a99bdfb..5ba789b 100644 --- a/libwebauthn/src/ops/webauthn/make_credential.rs +++ b/libwebauthn/src/ops/webauthn/make_credential.rs @@ -195,6 +195,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() @@ -232,7 +246,7 @@ impl FromInnerModel for MakeCredentialRequest { @@ -662,4 +680,36 @@ mod tests { UserVerificationRequirement::Preferred ); } + + #[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(_, _)) + )); + } } From efc97a20a35f2fd5dc5b8ce9fd722854555e2146 Mon Sep 17 00:00:00 2001 From: Alfie Fresta Date: Sat, 24 Jan 2026 17:19:53 -0500 Subject: [PATCH 2/2] Feedback: Add IPv6 address handling, including unbracketed. --- libwebauthn/src/ops/webauthn/idl/rpid.rs | 44 ++++++++++++++++++++++-- 1 file changed, 41 insertions(+), 3 deletions(-) diff --git a/libwebauthn/src/ops/webauthn/idl/rpid.rs b/libwebauthn/src/ops/webauthn/idl/rpid.rs index b2d04fc..e763a6a 100644 --- a/libwebauthn/src/ops/webauthn/idl/rpid.rs +++ b/libwebauthn/src/ops/webauthn/idl/rpid.rs @@ -52,6 +52,7 @@ impl TryFrom<&str> for RelyingPartyId { return Err(Error::EmptyRelyingPartyId); } + // Check for IP addresses (both IPv4 and IPv6) if value.parse::().is_ok() { return Err(Error::IpAddressNotAllowed(value.to_string())); } @@ -128,9 +129,46 @@ mod tests { } #[test] - fn test_relying_party_id_rejects_ip_address() { - let result = RelyingPartyId::try_from("127.0.0.1"); - assert!(matches!(result, Err(Error::IpAddressNotAllowed(_)))); + fn test_relying_party_id_rejects_ipv4_address() { + let ipv4_addresses = ["127.0.0.1", "192.168.1.1", "10.0.0.1", "255.255.255.255"]; + for ip in ipv4_addresses { + let result = RelyingPartyId::try_from(ip); + assert!( + matches!(result, Err(Error::IpAddressNotAllowed(_))), + "Expected IPv4 address '{}' to be rejected", + ip + ); + } + } + + #[test] + fn test_relying_party_id_rejects_ipv6_address() { + // Unbracketed format - must be rejected as IP address + let ipv6_addresses = ["::1", "2001:db8::1", "fe80::1", "::ffff:192.168.1.1"]; + for ip in ipv6_addresses { + let result = RelyingPartyId::try_from(ip); + assert!( + matches!(result, Err(Error::IpAddressNotAllowed(_))), + "Expected IPv6 address '{}' to be rejected as IP address", + ip + ); + } + + // Bracketed format (RFC 2732) - must be rejected (either as IP or invalid domain) + let bracketed_ipv6 = [ + "[::1]", + "[2001:db8::1]", + "[fe80::1]", + "[::ffff:192.168.1.1]", + ]; + for ip in bracketed_ipv6 { + let result = RelyingPartyId::try_from(ip); + assert!( + result.is_err(), + "Expected bracketed IPv6 address '{}' to be rejected", + ip + ); + } } #[test]