Update hickory-dns monorepo to 0.25.2 (#8391)

the PR has some notes about expected changes we'll see in a future 0.26.0. also, DNS resolvers used in most tests are no longer pinned to distrusting authoritative no-answer responses - this is not expected to be a meaningful change as we only query one DNS server for most tests.

otherwise, little functional difference. some simpler APIs, some typos fixed up, and cleanup along the way.

Co-authored-by: oxide-renovate[bot] <146848827+oxide-renovate[bot]@users.noreply.github.com>
Co-authored-by: Adam H. Leventhal <ahl@oxide.computer>
Co-authored-by: iximeow <iximeow@oxide.computer>

Author: 3 people

Cargo.lock

@@ -456,10 +456,10 @@ headers = "0.4.1"
 heck = "0.5"
 hex = "0.4.3"
 hex-literal = "0.4.1"
-hickory-client = "0.24.4"
-hickory-proto = "0.24.4"
-hickory-resolver = "0.24.4"
-hickory-server = "0.24.4"
+hickory-client = "0.25.2"
+hickory-proto = "0.25.2"
+hickory-resolver = "0.25.2"
+hickory-server = "0.25.2"
 highway = "1.3.0"
 hkdf = "0.12.4"
 hmac = "0.12.1"

Cargo.toml

@@ -7,10 +7,11 @@
 use anyhow::Context;
 use anyhow::anyhow;
 use futures::FutureExt;
-use hickory_resolver::TokioAsyncResolver;
+use hickory_resolver::TokioResolver;
 use hickory_resolver::config::{
-    NameServerConfig, Protocol, ResolverConfig, ResolverOpts,
+    NameServerConfig, ResolverConfig, ResolverOpts,
 };
+use hickory_resolver::name_server::TokioConnectionProvider;
 use std::net::SocketAddr;
 use std::sync::Arc;
 use thiserror::Error;
@@ -29,32 +30,35 @@ progenitor::generate_api!(
 /// for Nexus.  This is often useful when trying to connect with Nexus using
 /// TLS, since you need to come in via the DNS name to do that.
 ///
-/// This is a thin wrapper around `TokioAsyncResolver`
+/// This is a thin wrapper around `TokioResolver`
 pub struct CustomDnsResolver {
     dns_addr: SocketAddr,
     // The lifetime constraints on the `Resolve` trait make it hard to avoid an
     // Arc here.
-    resolver: Arc<TokioAsyncResolver>,
+    resolver: Arc<TokioResolver>,
 }
 
 impl CustomDnsResolver {
     /// Make a new custom resolver that uses the DNS server at the specified
     /// address
     pub fn new(dns_addr: SocketAddr) -> anyhow::Result<CustomDnsResolver> {
         let mut resolver_config = ResolverConfig::new();
-        resolver_config.add_name_server(NameServerConfig {
-            socket_addr: dns_addr,
-            protocol: Protocol::Udp,
-            tls_dns_name: None,
-            trust_negative_responses: false,
-            bind_addr: None,
-        });
+        resolver_config.add_name_server(NameServerConfig::new(
+            dns_addr,
+            hickory_resolver::proto::xfer::Protocol::Udp,
+        ));
         let mut resolver_opts = ResolverOpts::default();
         // Enable edns for potentially larger records
         resolver_opts.edns0 = true;
 
-        let resolver =
-            Arc::new(TokioAsyncResolver::tokio(resolver_config, resolver_opts));
+        let resolver = Arc::new(
+            TokioResolver::builder_with_config(
+                resolver_config,
+                TokioConnectionProvider::default(),
+            )
+            .with_options(resolver_opts)
+            .build(),
+        );
         Ok(CustomDnsResolver { dns_addr, resolver })
     }
 
@@ -63,8 +67,8 @@ impl CustomDnsResolver {
         self.dns_addr
     }
 
-    /// Returns the underlying `TokioAsyncResolver
-    pub fn resolver(&self) -> &TokioAsyncResolver {
+    /// Returns the underlying `TokioResolver`
+    pub fn resolver(&self) -> &TokioResolver {
         &self.resolver
     }
 }

clients/oxide-client/src/lib.rs

@@ -484,7 +484,7 @@ task: "nat_v4_garbage_collector"
   currently executing: no
   last completed activation: <REDACTED ITERATIONS>, triggered by a periodic timer firing
     started at <REDACTED_TIMESTAMP> (<REDACTED DURATION>s ago) and ran for <REDACTED DURATION>ms
-    last completion reported error: failed to resolve addresses for Dendrite services: no record found for Query { name: Name("_dendrite._tcp.control-plane.oxide.internal."), query_type: SRV, query_class: IN }
+    last completion reported error: failed to resolve addresses for Dendrite services: proto error: no records found for Query { name: Name("_dendrite._tcp.control-plane.oxide.internal."), query_type: SRV, query_class: IN }
 
 task: "blueprint_loader"
   configured period: every <REDACTED_DURATION>m <REDACTED_DURATION>s
@@ -523,7 +523,7 @@ task: "bfd_manager"
   currently executing: no
   last completed activation: <REDACTED ITERATIONS>, triggered by a periodic timer firing
     started at <REDACTED_TIMESTAMP> (<REDACTED DURATION>s ago) and ran for <REDACTED DURATION>ms
-    last completion reported error: failed to resolve addresses for Dendrite services: no record found for Query { name: Name("_dendrite._tcp.control-plane.oxide.internal."), query_type: SRV, query_class: IN }
+    last completion reported error: failed to resolve addresses for Dendrite services: proto error: no records found for Query { name: Name("_dendrite._tcp.control-plane.oxide.internal."), query_type: SRV, query_class: IN }
 
 task: "blueprint_planner"
   configured period: every <REDACTED_DURATION>m
@@ -1010,7 +1010,7 @@ task: "nat_v4_garbage_collector"
   currently executing: no
   last completed activation: <REDACTED ITERATIONS>, triggered by a periodic timer firing
     started at <REDACTED_TIMESTAMP> (<REDACTED DURATION>s ago) and ran for <REDACTED DURATION>ms
-    last completion reported error: failed to resolve addresses for Dendrite services: no record found for Query { name: Name("_dendrite._tcp.control-plane.oxide.internal."), query_type: SRV, query_class: IN }
+    last completion reported error: failed to resolve addresses for Dendrite services: proto error: no records found for Query { name: Name("_dendrite._tcp.control-plane.oxide.internal."), query_type: SRV, query_class: IN }
 
 task: "blueprint_loader"
   configured period: every <REDACTED_DURATION>m <REDACTED_DURATION>s
@@ -1049,7 +1049,7 @@ task: "bfd_manager"
   currently executing: no
   last completed activation: <REDACTED ITERATIONS>, triggered by a periodic timer firing
     started at <REDACTED_TIMESTAMP> (<REDACTED DURATION>s ago) and ran for <REDACTED DURATION>ms
-    last completion reported error: failed to resolve addresses for Dendrite services: no record found for Query { name: Name("_dendrite._tcp.control-plane.oxide.internal."), query_type: SRV, query_class: IN }
+    last completion reported error: failed to resolve addresses for Dendrite services: proto error: no records found for Query { name: Name("_dendrite._tcp.control-plane.oxide.internal."), query_type: SRV, query_class: IN }
 
 task: "blueprint_planner"
   configured period: every <REDACTED_DURATION>m

dev-tools/omdb/tests/successes.out

@@ -17,6 +17,7 @@ use hickory_proto::op::ResponseCode;
 use hickory_proto::rr::RData;
 use hickory_proto::rr::Record;
 use hickory_proto::rr::RecordType;
+use hickory_proto::rr::rdata::NS;
 use hickory_proto::rr::rdata::SRV;
 use hickory_proto::serialize::binary::BinDecodable;
 use hickory_proto::serialize::binary::BinDecoder;
@@ -224,19 +225,11 @@ fn dns_record_to_record(
 ) -> Result<Record, RequestError> {
     match record {
         DnsRecord::A(addr) => {
-            let mut a = Record::new();
-            a.set_name(name.clone())
-                .set_rr_type(RecordType::A)
-                .set_data(Some(RData::A((*addr).into())));
-            Ok(a)
+            Ok(Record::from_rdata(name.clone(), 0, RData::A((*addr).into())))
         }
 
         DnsRecord::Aaaa(addr) => {
-            let mut aaaa = Record::new();
-            aaaa.set_name(name.clone())
-                .set_rr_type(RecordType::AAAA)
-                .set_data(Some(RData::AAAA((*addr).into())));
-            Ok(aaaa)
+            Ok(Record::from_rdata(name.clone(), 0, RData::AAAA((*addr).into())))
         }
 
         DnsRecord::Srv(Srv { prio, weight, port, target }) => {
@@ -247,11 +240,11 @@ fn dns_record_to_record(
                     error
                 ))
             })?;
-            let mut srv = Record::new();
-            srv.set_name(name.clone()).set_rr_type(RecordType::SRV).set_data(
-                Some(RData::SRV(SRV::new(*prio, *weight, *port, tgt))),
-            );
-            Ok(srv)
+            Ok(Record::from_rdata(
+                name.clone(),
+                0,
+                RData::SRV(SRV::new(*prio, *weight, *port, tgt)),
+            ))
         }
 
         DnsRecord::Ns(nsdname) => {
@@ -262,12 +255,7 @@ fn dns_record_to_record(
                     error
                 ))
             })?;
-            let mut ns = Record::new();
-            use hickory_proto::rr::rdata::NS;
-            ns.set_name(name.clone())
-                .set_rr_type(RecordType::NS)
-                .set_data(Some(RData::NS(NS(nsdname))));
-            Ok(ns)
+            Ok(Record::from_rdata(name.clone(), 0, RData::NS(NS(nsdname))))
         }
     }
 }
@@ -287,9 +275,27 @@ async fn handle_dns_message(
     // have to decide if the error is authoritative.
     header.set_authoritative(true);
 
-    let query = mr.query();
+    let query = match mr.queries() {
+        [] => {
+            // A request with no queries could be legitimate (such as RFC 7873's Server Cookie
+            // query), but we won't look enough to find out because we don't support that.
+            return Err(RequestError::ServFail(anyhow!(
+                "request with no query?"
+            )));
+        }
+        [query] => query,
+        _ => {
+            // A request that is a query (opcode 0) with more than one question
+            // is invalid according to RFC 9619. We don't currently check that
+            // the opcode is actually QUERY, but even for other opcodes we don't
+            // support more than one question.
+            return Err(RequestError::ServFail(anyhow!(
+                "request with too many queries"
+            )));
+        }
+    };
     let name = query.original().name().clone();
-    let answer = store.query(mr)?;
+    let answer = store.query(query)?;
     let rb = MessageResponseBuilder::from_message_request(mr);
     let mut additional_records = vec![];
 
@@ -315,29 +321,22 @@ async fn handle_dns_message(
     if name_records.is_empty() {
         return Err(RequestError::NxDomain(answer.queried_fqdn()));
     }
-
     let response_records = name_records
         .into_iter()
-        .filter(|record| {
-            let ty = query.query_type();
-            if ty == RecordType::ANY {
-                return true;
-            }
-
-            match (ty, record.data()) {
-                (RecordType::A, Some(RData::A(_))) => true,
-                (RecordType::AAAA, Some(RData::AAAA(_))) => true,
-                (RecordType::SRV, Some(RData::SRV(_))) => true,
-                (RecordType::NS, Some(RData::NS(_))) => true,
-                (RecordType::SOA, Some(RData::SOA(_))) => true,
-                _ => false,
-            }
+        .filter(|record| match (query.query_type(), record.data()) {
+            (RecordType::ANY, _) => true,
+            (RecordType::A, RData::A(_)) => true,
+            (RecordType::AAAA, RData::AAAA(_)) => true,
+            (RecordType::SRV, RData::SRV(_)) => true,
+            (RecordType::NS, RData::NS(_)) => true,
+            (RecordType::SOA, RData::SOA(_)) => true,
+            _ => false,
         })
         .map(|record| {
             // DNS allows for the server to return additional records that
             // weren't explicitly asked for by the client but that the server
             // expects the client will want. SRV and NS records both use names
-            // for their referents (rather than IP addresses dierctly). If
+            // for their referents (rather than IP addresses directly). If
             // someone has queried for one of those kinds of records, they'll
             // almost certainly be needing the IP addresses that go with them as
             // well. We opportunistically attempt to resolve the target here and
@@ -346,8 +345,8 @@ async fn handle_dns_message(
             // NOTE: we only do this one-layer deep. If the target of a SRV or
             // NS is a CNAME instead of A/AAAA directly, it will be lost here.
             let additionals_target = match record.data() {
-                Some(RData::SRV(srv)) => Some(srv.target()),
-                Some(RData::NS(ns)) => Some(&ns.0),
+                RData::SRV(srv) => Some(srv.target()),
+                RData::NS(ns) => Some(&ns.0),
                 _ => None,
             };
 

dns-server/src/dns_server.rs

@@ -56,11 +56,11 @@ pub mod http_server;
 pub mod storage;
 
 use anyhow::{Context, anyhow};
-use hickory_resolver::TokioAsyncResolver;
+use hickory_resolver::TokioResolver;
 use hickory_resolver::config::NameServerConfig;
-use hickory_resolver::config::Protocol;
 use hickory_resolver::config::ResolverConfig;
 use hickory_resolver::config::ResolverOpts;
+use hickory_resolver::name_server::TokioConnectionProvider;
 use internal_dns_types::config::DnsConfigParams;
 use slog::o;
 use std::net::SocketAddr;
@@ -179,20 +179,23 @@ impl TransientServer {
         Ok(())
     }
 
-    pub async fn resolver(&self) -> Result<TokioAsyncResolver, anyhow::Error> {
+    pub async fn resolver(&self) -> Result<TokioResolver, anyhow::Error> {
         let mut resolver_config = ResolverConfig::new();
-        resolver_config.add_name_server(NameServerConfig {
-            socket_addr: self.dns_server.local_address(),
-            protocol: Protocol::Udp,
-            tls_dns_name: None,
-            trust_negative_responses: false,
-            bind_addr: None,
-        });
+        resolver_config.add_name_server(NameServerConfig::new(
+            self.dns_server.local_address(),
+            hickory_proto::xfer::Protocol::Udp,
+        ));
         let mut resolver_opts = ResolverOpts::default();
         // Enable edns for potentially larger records
         resolver_opts.edns0 = true;
-        let resolver =
-            TokioAsyncResolver::tokio(resolver_config, resolver_opts);
+
+        let resolver = TokioResolver::builder_with_config(
+            resolver_config,
+            TokioConnectionProvider::default(),
+        )
+        .with_options(resolver_opts)
+        .build();
+
         Ok(resolver)
     }
 }

dns-server/src/lib.rs

@@ -94,7 +94,7 @@
 
 use anyhow::{Context, anyhow};
 use camino::Utf8PathBuf;
-use hickory_proto::rr::LowerName;
+use hickory_proto::{op::LowerQuery, rr::LowerName};
 use hickory_resolver::Name;
 use internal_dns_types::{
     config::{DnsConfig, DnsConfigParams, DnsConfigZone, DnsRecord},
@@ -439,23 +439,23 @@ impl Store {
             })
             .map(name_from_str)??;
 
-        let mut record = hickory_proto::rr::Record::new();
         let soa_name = name_from_str(&answer.queried_fqdn())?;
         let rname = name_from_str(format!("admin.{}", answer.zone.as_str()))?;
-        record
-            .set_name(soa_name)
-            .set_rr_type(hickory_proto::rr::RecordType::SOA)
-            .set_data(Some(hickory_proto::rr::RData::SOA(
-                hickory_proto::rr::rdata::SOA::new(
-                    preferred_nameserver,
-                    rname,
-                    answer.serial,
-                    3600,
-                    600,
-                    1800,
-                    600,
-                ),
-            )));
+
+        let record = hickory_proto::rr::Record::from_rdata(
+            soa_name,
+            0,
+            hickory_proto::rr::RData::SOA(hickory_proto::rr::rdata::SOA::new(
+                preferred_nameserver,
+                rname,
+                answer.serial,
+                3600,
+                600,
+                1800,
+                600,
+            )),
+        );
+
         Ok(record)
     }
 
@@ -759,10 +759,10 @@ impl Store {
     /// If the name does not match any zone, returns `QueryError::NoZone`.
     pub(crate) fn query(
         &self,
-        mr: &hickory_server::authority::MessageRequest,
+        query: &LowerQuery,
     ) -> Result<Answer, QueryError> {
-        let name = mr.query().name();
-        let orig_name = mr.query().original().name();
+        let name = query.name();
+        let orig_name = query.original().name();
         self.query_raw(name, orig_name)
     }