Generate `DNSSECError` messages when DNSSEC resolution fails
What changed, and why it matters
This commit adds a new error-reporting path in a Lightning DNS resolver. Previously, if a DNSSEC proof lookup failed, the resolver simply stayed silent. Now it sends back an explicit 'DNSSECError' message so the requester knows the lookup failed and can fall back or fail the payment faster. The change is a feature addition with a small, well-scoped security benefit: it removes a silent-failure mode that could cause payment delays or user confusion. There is no evidence of a vulnerability being fixed, and no attacker-controlled behavior is introduced.
No immediate security action required. Reviewers should verify that definitely_unresolvable is only set for authenticated NXDOMAIN cases and that other resolver failures (e.g., network timeouts, unsupported DNSSEC) do not incorrectly signal permanent unresolvability. Consider whether error messages should include additional context for debugging without leaking sensitive resolver state.
Security signals we found
Eliminates silent failure on DNSSEC proof resolution errors
Adds authenticated NXDOMAIN signaling to prevent indefinite fallback waits
No new unsafe code, unwraps, or cryptographic operations introduced
Test coverage added for the new error path
Evidence from the diff
The patch modifies lightning-dns-resolver/src/lib.rs so that build_txt_proof_async errors are no longer ignored. On success, a DNSSECProof message is queued as before. On failure, a new DNSSECError message is queued with a definitely_unresolvable flag set only when the underlying ProofBuildingError is NoSuchName (authenticated NXDOMAIN). The change also wires up the existing handle_dnssec_error test stub and adds a tokio test that queries a non-existent name under mattcorallo.com and asserts a DNSSECError with definitely_unresolvable=true is returned. The pending_query_count is still decremented in both paths, preserving the prior counter fix.
Changed components
lightning-dns-resolver/src/lib.rsDNSResolverMessage::DNSSECError handlingOMDomainResolver async resolution taskInspect captured patch +99 / −7
diff --git a/lightning-dns-resolver/src/lib.rs b/lightning-dns-resolver/src/lib.rs
index c89849e..a5b4a92 100644
--- a/lightning-dns-resolver/src/lib.rs
+++ b/lightning-dns-resolver/src/lib.rs
@@ -9,7 +9,7 @@ use std::net::SocketAddr;
use std::sync::atomic::{AtomicUsize, Ordering};
use std::sync::{Arc, Mutex};
-use dnssec_prover::query::build_txt_proof_async;
+use dnssec_prover::query::{build_txt_proof_async, ProofBuildingError};
use lightning::blinded_path::message::DNSResolverContext;
use lightning::ln::peer_handler::IgnoringMessageHandler;
@@ -127,11 +127,25 @@ impl<PH: DNSResolverMessageHandler> DNSResolverMessageHandler for OMDomainResolv
}
let us = Arc::clone(&self.state);
runtime.spawn(async move {
- if let Ok((proof, _ttl)) = build_txt_proof_async(us.resolver, &q.0).await {
- let contents = DNSResolverMessage::DNSSECProof(DNSSECProof { name: q.0, proof });
- let instructions = responder.respond().into_instructions();
- us.pending_replies.lock().unwrap().push((contents, instructions));
- }
+ let contents = match build_txt_proof_async(us.resolver, &q.0).await {
+ Ok((proof, _ttl)) => {
+ DNSResolverMessage::DNSSECProof(DNSSECProof { name: q.0, proof })
+ },
+ Err(e) => {
+ // We might get an Unauthenticated error if the DNS resolver does not support
+ // DNSSEC, so we only set `definitely_unresolvable` if we get an NXDOMAIN.
+ let definitely_unresolvable = matches!(
+ e.get_ref().and_then(|e| e.downcast_ref::<ProofBuildingError>()),
+ Some(ProofBuildingError::NoSuchName)
+ );
+ DNSResolverMessage::DNSSECError(DNSSECError {
+ name: q.0,
+ definitely_unresolvable,
+ })
+ },
+ };
+ let instructions = responder.respond().into_instructions();
+ us.pending_replies.lock().unwrap().push((contents, instructions));
us.pending_query_count.fetch_sub(1, Ordering::Relaxed);
});
None
@@ -217,6 +231,7 @@ mod test {
struct URIResolver {
resolved_uri: Mutex<Option<(HumanReadableName, PaymentId, String)>>,
+ resolved_error: Mutex<Option<(HumanReadableName, PaymentId, bool)>>,
resolver: OMNameResolver,
pending_messages: Mutex<Vec<(DNSResolverMessage, MessageSendInstructions)>>,
}
@@ -236,7 +251,13 @@ mod test {
assert!(result.is_none());
}
fn handle_dnssec_error(&self, msg: DNSSECError, context: DNSResolverContext) {
- // TODO
+ let definitely_unresolvable = msg.definitely_unresolvable;
+ let mut failed = self.resolver.handle_dnssec_error(msg, context);
+ assert_eq!(failed.len(), 1);
+ let (name, payment_id) = failed.pop().unwrap();
+ let mut result = Some((name, payment_id, definitely_unresolvable));
+ core::mem::swap(&mut *self.resolved_error.lock().unwrap(), &mut result);
+ assert!(result.is_none());
}
fn release_pending_messages(&self) -> Vec<(DNSResolverMessage, MessageSendInstructions)> {
core::mem::take(&mut *self.pending_messages.lock().unwrap())
@@ -286,6 +307,7 @@ mod test {
let payer_id = payer_keys.get_node_id(Recipient::Node).unwrap();
let payer = Arc::new(URIResolver {
resolved_uri: Mutex::new(None),
+ resolved_error: Mutex::new(None),
resolver: OMNameResolver::new(now as u32, 1),
pending_messages: Mutex::new(Vec::new()),
});
@@ -331,6 +353,75 @@ mod test {
assert!(resolution.2[.."bitcoin:".len()].eq_ignore_ascii_case("bitcoin:"));
}
+ #[tokio::test]
+ async fn resolution_failure_test() {
+ // Test that querying for a name which does not exist results in a `DNSSECError` with
+ // `definitely_unresolvable` set being returned (rather than a `DNSSECProof`).
+
+ let (resolver_messenger, resolver_id) = create_resolver();
+
+ let resolver_dest = Destination::Node(resolver_id);
+ let now = SystemTime::now().duration_since(SystemTime::UNIX_EPOCH).unwrap().as_secs();
+
+ let payment_id = PaymentId([43; 32]);
+ // `mattcorallo.com` is DNSSEC-signed, so a name which does not exist under it will result in
+ // an authenticated NXDOMAIN, i.e. a definitely-unresolvable name.
+ let name =
+ HumanReadableName::from_encoded("nonexistent-user-ldk-test@mattcorallo.com").unwrap();
+
+ let payer_keys = Arc::new(KeysManager::new(&[3; 32], 42, 43, true));
+ let payer_logger = TestLogger { node: "payer" };
+ let payer_id = payer_keys.get_node_id(Recipient::Node).unwrap();
+ let payer = Arc::new(URIResolver {
+ resolved_uri: Mutex::new(None),
+ resolved_error: Mutex::new(None),
+ resolver: OMNameResolver::new(now as u32, 1),
+ pending_messages: Mutex::new(Vec::new()),
+ });
+ let payer_messenger = Arc::new(OnionMessenger::new(
+ Arc::clone(&payer_keys),
+ Arc::clone(&payer_keys),
+ payer_logger,
+ DummyNodeLookup {},
+ DirectlyConnectedRouter {},
+ IgnoringMessageHandler {},
+ IgnoringMessageHandler {},
+ Arc::clone(&payer),
+ IgnoringMessageHandler {},
+ ));
+
+ let init_msg = get_om_init();
+ payer_messenger.peer_connected(resolver_id, &init_msg, true).unwrap();
+ resolver_messenger.get_om().peer_connected(payer_id, &init_msg, false).unwrap();
+
+ let messages = payer
+ .resolver
+ .resolve_name(payment_id, name.clone(), vec![resolver_dest], &*payer_keys)
+ .unwrap();
+ payer.pending_messages.lock().unwrap().extend(messages);
+
+ let query = payer_messenger.next_onion_message_for_peer(resolver_id).unwrap();
+ resolver_messenger.get_om().handle_onion_message(payer_id, &query);
+
+ assert!(resolver_messenger.get_om().next_onion_message_for_peer(payer_id).is_none());
+ let start = Instant::now();
+ let response = loop {
+ tokio::time::sleep(Duration::from_millis(10)).await;
+ if let Some(msg) = resolver_messenger.get_om().next_onion_message_for_peer(payer_id) {
+ break msg;
+ }
+ assert!(start.elapsed() < Duration::from_secs(10), "Resolution took too long");
+ };
+
+ payer_messenger.handle_onion_message(resolver_id, &response);
+ let (failed_name, failed_payment_id, definitely_unresolvable) =
+ payer.resolved_error.lock().unwrap().take().unwrap();
+ assert_eq!(failed_name, name);
+ assert_eq!(failed_payment_id, payment_id);
+ assert!(definitely_unresolvable);
+ assert!(payer.resolved_uri.lock().unwrap().is_none());
+ }
+
#[tokio::test]
async fn failed_query_does_not_leak_pending_counter() {
use std::sync::atomic::Ordering;
@@ -368,6 +459,7 @@ mod test {
let payer_id = payer_keys.get_node_id(Recipient::Node).unwrap();
let payer = Arc::new(URIResolver {
resolved_uri: Mutex::new(None),
+ resolved_error: Mutex::new(None),
resolver: OMNameResolver::new(now as u32, 1),
pending_messages: Mutex::new(Vec::new()),
});
Why this scored 23/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.