Skip to content

Commit 255a16e

Browse files
committed
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`.
1 parent 0eec30a commit 255a16e

File tree

1 file changed

+35
-8
lines changed

1 file changed

+35
-8
lines changed

lightning/src/onion_message/dns_resolution.rs

Lines changed: 35 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -191,8 +191,8 @@ const REQUIRED_EXTRA_LEN: usize = ".user._bitcoin-payment.".len() + 1;
191191

192192
/// A struct containing the two parts of a BIP 353 Human Readable Name - the user and domain parts.
193193
///
194-
/// The `user` and `domain` parts, together, cannot exceed 231 bytes in length, and both must be
195-
/// non-empty.
194+
/// The `user` and `domain` parts combined cannot exceed 231 bytes in length;
195+
/// each DNS label within them must be non-empty and no longer than 63 bytes.
196196
///
197197
/// If you intend to handle non-ASCII `user` or `domain` parts, you must handle [Homograph Attacks]
198198
/// and do punycode en-/de-coding yourself. This struct will always handle only plain ASCII `user`
@@ -211,16 +211,21 @@ pub struct HumanReadableName {
211211
impl HumanReadableName {
212212
/// Constructs a new [`HumanReadableName`] from the `user` and `domain` parts. See the
213213
/// struct-level documentation for more on the requirements on each.
214-
pub fn new(user: &str, mut domain: &str) -> Result<HumanReadableName, ()> {
214+
pub fn new(user: &str, domain: &str) -> Result<HumanReadableName, ()> {
215215
// First normalize domain and remove the optional trailing `.`
216-
if domain.ends_with('.') {
217-
domain = &domain[..domain.len() - 1];
218-
}
216+
let domain = domain.strip_suffix(".").unwrap_or(domain);
219217
if user.len() + domain.len() + REQUIRED_EXTRA_LEN > 255 {
220218
return Err(());
221219
}
222-
if user.is_empty() || domain.is_empty() {
223-
return Err(());
220+
for label in user.split('.') {
221+
if label.is_empty() || label.len() > 63 {
222+
return Err(());
223+
}
224+
}
225+
for label in domain.split('.') {
226+
if label.is_empty() || label.len() > 63 {
227+
return Err(());
228+
}
224229
}
225230
if !Hostname::str_is_valid_hostname(&user) || !Hostname::str_is_valid_hostname(&domain) {
226231
return Err(());
@@ -558,6 +563,28 @@ mod tests {
558563
);
559564
}
560565

566+
#[test]
567+
fn test_hrn_validation() {
568+
assert!(HumanReadableName::new("user", "example.com").is_ok());
569+
assert!(HumanReadableName::new("user", "example.com.").is_ok());
570+
571+
assert!(HumanReadableName::new("user!", "example.com").is_err());
572+
assert!(HumanReadableName::new("user.", "example.com").is_err());
573+
assert!(HumanReadableName::new("user", "exa mple.com").is_err());
574+
assert!(HumanReadableName::new("", "example.com").is_err());
575+
assert!(HumanReadableName::new("user", "").is_err());
576+
577+
let max_label = "a".repeat(63);
578+
assert!(HumanReadableName::new(&max_label, "example.com").is_ok());
579+
580+
let long_label = "a".repeat(64);
581+
assert!(HumanReadableName::new(&long_label, "example.com").is_err());
582+
let domain_with_long_label = format!("{long_label}.com");
583+
assert!(HumanReadableName::new("user", &domain_with_long_label).is_err());
584+
let huge_domain = format!("{max_label}.{max_label}.{max_label}.{max_label}");
585+
assert!(HumanReadableName::new("user", &huge_domain).is_err());
586+
}
587+
561588
#[test]
562589
#[cfg(feature = "dnssec")]
563590
fn test_expiry() {

0 commit comments

Comments
 (0)