From 40e9174c5522229b85ce6cd5fcabb4b3caa5f7bc Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Sat, 28 Jun 2025 22:50:16 +0000 Subject: [PATCH 1/4] Correct docs on `OMNameResolver::new_best_block` We decided against implementing `OMNameResolver` using completion callbacks, but the documentation on `new_best_block` was never updated to match. --- lightning/src/onion_message/dns_resolution.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lightning/src/onion_message/dns_resolution.rs b/lightning/src/onion_message/dns_resolution.rs index 43eba33810f..a28072f160c 100644 --- a/lightning/src/onion_message/dns_resolution.rs +++ b/lightning/src/onion_message/dns_resolution.rs @@ -332,7 +332,8 @@ impl OMNameResolver { /// Informs the [`OMNameResolver`] of the passage of time in the form of a new best Bitcoin /// block. /// - /// This will call back to resolve some pending queries which have timed out. + /// This is used to prune stale requests (by block height) and keep track of the current time + /// to validate that DNSSEC proofs are current. pub fn new_best_block(&self, height: u32, time: u32) { self.latest_block_time.store(time as usize, Ordering::Release); self.latest_block_height.store(height as usize, Ordering::Release); From 28fd617dd3530ce73bbe1a799e1bcfc803f43592 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Sat, 28 Jun 2025 22:56:13 +0000 Subject: [PATCH 2/4] Add a method to expire specific resolutions from `OMNameResolver` In `bitcoin-payment-instructions` we can figure out when a lookup has timed out by looking at if the `Future` was dropped. However, while we can clean up internal state in `bitcoin-payment-instructions` we'd leave stale entries in `OMNameResolver` (at least until they time out). Instead, here we add a method to explicitly time out a lookup. --- lightning/src/onion_message/dns_resolution.rs | 65 ++++++++++++++++++- 1 file changed, 64 insertions(+), 1 deletion(-) diff --git a/lightning/src/onion_message/dns_resolution.rs b/lightning/src/onion_message/dns_resolution.rs index a28072f160c..7d7bb379b06 100644 --- a/lightning/src/onion_message/dns_resolution.rs +++ b/lightning/src/onion_message/dns_resolution.rs @@ -344,6 +344,30 @@ impl OMNameResolver { }); } + /// Removes any pending resolutions for the given `name` and `payment_id`. + /// + /// Any future calls to [`Self::handle_dnssec_proof_for_offer`] or + /// [`Self::handle_dnssec_proof_for_uri`] will no longer return a result for the given + /// resolution. + pub fn expire_pending_resolution(&self, name: &HumanReadableName, payment_id: PaymentId) { + let dns_name = + Name::try_from(format!("{}.user._bitcoin-payment.{}.", name.user(), name.domain())); + debug_assert!( + dns_name.is_ok(), + "The HumanReadableName constructor shouldn't allow names which are too long" + ); + if let Ok(name) = dns_name { + let mut pending_resolves = self.pending_resolves.lock().unwrap(); + if let hash_map::Entry::Occupied(mut entry) = pending_resolves.entry(name) { + let resolutions = entry.get_mut(); + resolutions.retain(|resolution| resolution.payment_id != payment_id); + if resolutions.is_empty() { + entry.remove(); + } + } + } + } + /// Begins the process of resolving a BIP 353 Human Readable Name. /// /// Returns a [`DNSSECQuery`] onion message and a [`DNSResolverContext`] which should be sent @@ -483,7 +507,7 @@ impl OMNameResolver { #[cfg(test)] mod tests { - use super::HumanReadableName; + use super::*; #[test] fn test_hrn_display_format() { @@ -500,4 +524,43 @@ mod tests { "HumanReadableName display format mismatch" ); } + + #[test] + #[cfg(feature = "dnssec")] + fn test_expiry() { + let keys = crate::sign::KeysManager::new(&[33; 32], 0, 0); + let resolver = OMNameResolver::new(42, 42); + let name = HumanReadableName::new("user", "example.com").unwrap(); + + // Queue up a resolution + resolver.resolve_name(PaymentId([0; 32]), name.clone(), &keys).unwrap(); + assert_eq!(resolver.pending_resolves.lock().unwrap().len(), 1); + // and check that it expires after two blocks + resolver.new_best_block(44, 42); + assert_eq!(resolver.pending_resolves.lock().unwrap().len(), 0); + + // Queue up another resolution + resolver.resolve_name(PaymentId([1; 32]), name.clone(), &keys).unwrap(); + assert_eq!(resolver.pending_resolves.lock().unwrap().len(), 1); + // it won't expire after one block + resolver.new_best_block(45, 42); + assert_eq!(resolver.pending_resolves.lock().unwrap().len(), 1); + assert_eq!(resolver.pending_resolves.lock().unwrap().iter().next().unwrap().1.len(), 1); + // and queue up a second and third resolution of the same name + resolver.resolve_name(PaymentId([2; 32]), name.clone(), &keys).unwrap(); + resolver.resolve_name(PaymentId([3; 32]), name.clone(), &keys).unwrap(); + assert_eq!(resolver.pending_resolves.lock().unwrap().len(), 1); + assert_eq!(resolver.pending_resolves.lock().unwrap().iter().next().unwrap().1.len(), 3); + // after another block the first will expire, but the second and third won't + resolver.new_best_block(46, 42); + assert_eq!(resolver.pending_resolves.lock().unwrap().len(), 1); + assert_eq!(resolver.pending_resolves.lock().unwrap().iter().next().unwrap().1.len(), 2); + // Check manual expiry + resolver.expire_pending_resolution(&name, PaymentId([3; 32])); + assert_eq!(resolver.pending_resolves.lock().unwrap().len(), 1); + assert_eq!(resolver.pending_resolves.lock().unwrap().iter().next().unwrap().1.len(), 1); + // after one more block all the requests will have expired + resolver.new_best_block(47, 42); + assert_eq!(resolver.pending_resolves.lock().unwrap().len(), 0); + } } From 072675b26e3a4b22313a3fd3e2531268f7437e97 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Sun, 29 Jun 2025 17:36:29 +0000 Subject: [PATCH 3/4] Allow use of `OMNameResolver` without validating DNSSEC expiry While all `OMNameResolver` users should at least use the block time to avoid accepting stale DNSSEC proofs, in some cases its annoying to pipe that data through and time may not be available (e.g. in a no-std environment). Here we enable such use by exposing an additional constructor and disabling expiry validation when it is used. --- lightning/src/onion_message/dns_resolution.rs | 38 ++++++++++++++----- 1 file changed, 29 insertions(+), 9 deletions(-) diff --git a/lightning/src/onion_message/dns_resolution.rs b/lightning/src/onion_message/dns_resolution.rs index 7d7bb379b06..42c7e3c1b9c 100644 --- a/lightning/src/onion_message/dns_resolution.rs +++ b/lightning/src/onion_message/dns_resolution.rs @@ -329,6 +329,24 @@ impl OMNameResolver { } } + /// Builds a new [`OMNameResolver`] which will not validate the time limits on DNSSEC proofs + /// (at least until [`Self::new_best_block`] is called). + /// + /// If possible, you should prefer [`Self::new`] so that providing stale proofs is not + /// possible, however in no-std environments where there is some trust in the resolver used and + /// no time source is available, this may be acceptable. + /// + /// Note that not calling [`Self::new_best_block`] will result in requests not timing out and + /// unresolved requests leaking memory. You must instead call + /// [`Self::expire_pending_resolution`] as unresolved requests expire. + pub fn new_without_expiry_validation() -> Self { + Self { + pending_resolves: Mutex::new(new_hash_map()), + latest_block_time: AtomicUsize::new(0), + latest_block_height: AtomicUsize::new(0), + } + } + /// Informs the [`OMNameResolver`] of the passage of time in the form of a new best Bitcoin /// block. /// @@ -461,15 +479,17 @@ impl OMNameResolver { parsed_rrs.as_ref().and_then(|rrs| verify_rr_stream(rrs).map_err(|_| &())); if let Ok(validated_rrs) = validated_rrs { let block_time = self.latest_block_time.load(Ordering::Acquire) as u64; - // Block times may be up to two hours in the future and some time into the past - // (we assume no more than two hours, though the actual limits are rather - // complicated). - // Thus, we have to let the proof times be rather fuzzy. - if validated_rrs.valid_from > block_time + 60 * 2 { - return None; - } - if validated_rrs.expires < block_time - 60 * 2 { - return None; + if block_time != 0 { + // Block times may be up to two hours in the future and some time into the past + // (we assume no more than two hours, though the actual limits are rather + // complicated). + // Thus, we have to let the proof times be rather fuzzy. + if validated_rrs.valid_from > block_time + 60 * 2 { + return None; + } + if validated_rrs.expires < block_time - 60 * 2 { + return None; + } } let resolved_rrs = validated_rrs.resolve_name(&entry.key()); if resolved_rrs.is_empty() { From 18551845fb1eca0a7db9dba990e1575c6fe7fa17 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Mon, 30 Jun 2025 12:23:29 +0000 Subject: [PATCH 4/4] If we're built with `std`, always use real time to validate DNSSEC In cases where we're built with the `std` feature, we can assume that `SystemTime` works, so we should use it to validate that DNSSEC proofs we receive are currently valid, even if we don't have block time data. --- lightning/src/onion_message/dns_resolution.rs | 20 +++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/lightning/src/onion_message/dns_resolution.rs b/lightning/src/onion_message/dns_resolution.rs index 42c7e3c1b9c..e927d645ece 100644 --- a/lightning/src/onion_message/dns_resolution.rs +++ b/lightning/src/onion_message/dns_resolution.rs @@ -330,7 +330,7 @@ impl OMNameResolver { } /// Builds a new [`OMNameResolver`] which will not validate the time limits on DNSSEC proofs - /// (at least until [`Self::new_best_block`] is called). + /// (for builds without the "std" feature and until [`Self::new_best_block`] is called). /// /// If possible, you should prefer [`Self::new`] so that providing stale proofs is not /// possible, however in no-std environments where there is some trust in the resolver used and @@ -339,7 +339,7 @@ impl OMNameResolver { /// Note that not calling [`Self::new_best_block`] will result in requests not timing out and /// unresolved requests leaking memory. You must instead call /// [`Self::expire_pending_resolution`] as unresolved requests expire. - pub fn new_without_expiry_validation() -> Self { + pub fn new_without_no_std_expiry_validation() -> Self { Self { pending_resolves: Mutex::new(new_hash_map()), latest_block_time: AtomicUsize::new(0), @@ -478,16 +478,24 @@ impl OMNameResolver { let validated_rrs = parsed_rrs.as_ref().and_then(|rrs| verify_rr_stream(rrs).map_err(|_| &())); if let Ok(validated_rrs) = validated_rrs { - let block_time = self.latest_block_time.load(Ordering::Acquire) as u64; - if block_time != 0 { + #[allow(unused_assignments, unused_mut)] + let mut time = self.latest_block_time.load(Ordering::Acquire) as u64; + #[cfg(feature = "std")] + { + use std::time::{SystemTime, UNIX_EPOCH}; + let now = SystemTime::now().duration_since(UNIX_EPOCH); + time = now.expect("Time must be > 1970").as_secs(); + } + if time != 0 { // Block times may be up to two hours in the future and some time into the past // (we assume no more than two hours, though the actual limits are rather // complicated). // Thus, we have to let the proof times be rather fuzzy. - if validated_rrs.valid_from > block_time + 60 * 2 { + let max_time_offset = if cfg!(feature = "std") { 0 } else { 60 * 2 }; + if validated_rrs.valid_from > time + max_time_offset { return None; } - if validated_rrs.expires < block_time - 60 * 2 { + if validated_rrs.expires < time - max_time_offset { return None; } }