From 255a16e769e5a7d6702615a778b22442c9b07f73 Mon Sep 17 00:00:00 2001 From: Andrei Date: Fri, 24 Oct 2025 00:00:00 +0000 Subject: [PATCH] Enhance `HumanReadableName` validation Add check that every label of user or domain is not longer than 63. It better alings with validation in `dnssec_prover::rr::Name`. --- lightning/src/onion_message/dns_resolution.rs | 43 +++++++++++++++---- 1 file changed, 35 insertions(+), 8 deletions(-) diff --git a/lightning/src/onion_message/dns_resolution.rs b/lightning/src/onion_message/dns_resolution.rs index 54eb16b5266..6d9d4d61428 100644 --- a/lightning/src/onion_message/dns_resolution.rs +++ b/lightning/src/onion_message/dns_resolution.rs @@ -191,8 +191,8 @@ const REQUIRED_EXTRA_LEN: usize = ".user._bitcoin-payment.".len() + 1; /// A struct containing the two parts of a BIP 353 Human Readable Name - the user and domain parts. /// -/// The `user` and `domain` parts, together, cannot exceed 231 bytes in length, and both must be -/// non-empty. +/// The `user` and `domain` parts combined cannot exceed 231 bytes in length; +/// each DNS label within them must be non-empty and no longer than 63 bytes. /// /// If you intend to handle non-ASCII `user` or `domain` parts, you must handle [Homograph Attacks] /// and do punycode en-/de-coding yourself. This struct will always handle only plain ASCII `user` @@ -211,16 +211,21 @@ pub struct HumanReadableName { impl HumanReadableName { /// Constructs a new [`HumanReadableName`] from the `user` and `domain` parts. See the /// struct-level documentation for more on the requirements on each. - pub fn new(user: &str, mut domain: &str) -> Result { + pub fn new(user: &str, domain: &str) -> Result { // First normalize domain and remove the optional trailing `.` - if domain.ends_with('.') { - domain = &domain[..domain.len() - 1]; - } + let domain = domain.strip_suffix(".").unwrap_or(domain); if user.len() + domain.len() + REQUIRED_EXTRA_LEN > 255 { return Err(()); } - if user.is_empty() || domain.is_empty() { - return Err(()); + for label in user.split('.') { + if label.is_empty() || label.len() > 63 { + return Err(()); + } + } + for label in domain.split('.') { + if label.is_empty() || label.len() > 63 { + return Err(()); + } } if !Hostname::str_is_valid_hostname(&user) || !Hostname::str_is_valid_hostname(&domain) { return Err(()); @@ -558,6 +563,28 @@ mod tests { ); } + #[test] + fn test_hrn_validation() { + assert!(HumanReadableName::new("user", "example.com").is_ok()); + assert!(HumanReadableName::new("user", "example.com.").is_ok()); + + assert!(HumanReadableName::new("user!", "example.com").is_err()); + assert!(HumanReadableName::new("user.", "example.com").is_err()); + assert!(HumanReadableName::new("user", "exa mple.com").is_err()); + assert!(HumanReadableName::new("", "example.com").is_err()); + assert!(HumanReadableName::new("user", "").is_err()); + + let max_label = "a".repeat(63); + assert!(HumanReadableName::new(&max_label, "example.com").is_ok()); + + let long_label = "a".repeat(64); + assert!(HumanReadableName::new(&long_label, "example.com").is_err()); + let domain_with_long_label = format!("{long_label}.com"); + assert!(HumanReadableName::new("user", &domain_with_long_label).is_err()); + let huge_domain = format!("{max_label}.{max_label}.{max_label}.{max_label}"); + assert!(HumanReadableName::new("user", &huge_domain).is_err()); + } + #[test] #[cfg(feature = "dnssec")] fn test_expiry() {