From b4c553e79c160604c8d67cd21c30f460d623de0f Mon Sep 17 00:00:00 2001
From: Colin <colin@uninsane.org>
Date: Wed, 17 Jul 2024 15:12:15 +0000
Subject: [PATCH] Recursor: handle NS responses with a different type and no
 SOA

e.g. `dig m.wikipedia.org. NS` replies with `m.wikipedia.org. CNAME
dyna.wikimedia.org` and NO SOA record. the CNAME is worthless, but it's
evidence that the nameserver we queried is in fact authoritative for
`m.wikipedia.org.`.
---
 crates/recursor/src/recursor.rs | 42 ++++++++++++++++++++-------------
 1 file changed, 25 insertions(+), 17 deletions(-)

diff --git a/crates/recursor/src/recursor.rs b/crates/recursor/src/recursor.rs
index dddd6203..df6c77d8 100644
--- a/crates/recursor/src/recursor.rs
+++ b/crates/recursor/src/recursor.rs
@@ -357,6 +357,7 @@ impl<P: ConnectionProvider + Default> Recursor<P> {
             Ok(r) => {
                 let mut r = r.into_message();
                 info!("response: {}", r.header());
+                let mut got_answer_of_any_type = false;
                 let mut soa = None;
                 let records = r
                     .take_answers()
@@ -374,6 +375,9 @@ impl<P: ConnectionProvider + Default> Recursor<P> {
                             true
                         }
                     }).map(|x| {
+                        if x.name() == query.name() {
+                            got_answer_of_any_type = true;
+                        }
                         if x.record_type().is_soa() {
                             soa = Some(x.name().clone());
                         }
@@ -381,27 +385,31 @@ impl<P: ConnectionProvider + Default> Recursor<P> {
                     });
 
                 let lookup = self.record_cache.insert_records(query.clone(), records, now);
-                let lookup = lookup.or_else(|| {
-                    if query.query_type().is_ns() || query.query_type().is_cname() || query.query_type().is_any() {
-                        None
-                    } else {
+                let lookup = match lookup {
+                    Some(q) => Some(Ok(q)),
+                    None => if query.query_type().is_ns() {
+                        debug!("no records for {:?}: try SOA {:?}", &query, &soa);
+                        if let Some(soa_name) = soa {
+                            Some(Err(Error::from(ErrorKind::Forward(soa_name))))
+                        } else if got_answer_of_any_type {
+                            // if we query `@ns1 foo.example.org. NS` and get a response for
+                            // `foo.example.org.` of a different type (e.g. CNAME), we learn that
+                            // `ns1` is authoritative for the zone to which `foo.example.org.`
+                            // records (of other types) belong.
+                            Some(Err(Error::from(ErrorKind::Forward(ns.zone().clone()))))
+                        } else {
+                            None
+                        }
+                    } else if !(query.query_type().is_ns() || query.query_type().is_cname() || query.query_type().is_any()) {
                         debug!("no records for {:?}: checking for cached CNAME", &query);
                         let mut cname_query = query.clone();
                         cname_query.set_query_type(RecordType::CNAME);
-                        self.record_cache.get(&cname_query, now).and_then(Result::ok)
+                        self.record_cache.get(&cname_query, now).and_then(|r| Some(r)).map(|m| m.map_err(Error::from))
+                    } else {
+                        None
                     }
-                });
-
-                lookup.ok_or_else(|| {
-                    if query.query_type().is_ns() {
-                        debug!("no records for {:?}: returning SOA {:?}", &query, &soa);
-                        if let Some(soa_name) = soa {
-                            return Error::from(ErrorKind::Forward(soa_name));
-                        }
-                    }
-
-                    Error::from("no records found")
-                })
+                };
+                lookup.unwrap_or_else(|| Err(Error::from("no records found")))
             }
             Err(e) => {
                 warn!("lookup error: {e}");