colin/hickory-dns
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Don't retry NOERROR with an empty set, from trusted resolvers

Browse Source
This commit is contained in:
Jeff Hiner 2022-12-15 11:07:23 -07:00 committed by Dirkjan Ochtman
parent 013307146d
commit 30aed7abc2
12 changed files with 130 additions and 58 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
crates/recursor/src/recursor.rs
View File

@ -378,8 +378,8 @@ impl Recursor {
let mut udp = NameServerConfig::new(SocketAddr::from((ip, 53)), Protocol::Udp);
let mut tcp = NameServerConfig::new(SocketAddr::from((ip, 53)), Protocol::Tcp);
udp.trust_nx_responses = true;
tcp.trust_nx_responses = true;
udp.trust_negative_responses = true;
tcp.trust_negative_responses = true;
config_group.push(udp);
config_group.push(tcp);

42
crates/resolver/src/config.rs
View File

@ -400,16 +400,18 @@ pub struct NameServerConfig {
pub tls_dns_name: Option<String>,
/// Whether to trust `NXDOMAIN` responses from upstream nameservers.
///
/// When this is `true`, and an empty `NXDOMAIN` response is received, the
/// query will not be retried against other configured name servers.
/// When this is `true`, and an empty `NXDOMAIN` response or `NOERROR`
/// with an empty answers set is received, the
/// query will not be retried against other configured name servers if
/// the response has the Authoritative flag set.
///
/// (On an empty `NoError` response, or a response with any other error
/// (On a response with any other error
/// response code, the query will still be retried regardless of this
/// configuration setting.)
///
/// Defaults to false.
#[cfg_attr(feature = "serde-config", serde(default))]
pub trust_nx_responses: bool,
pub trust_negative_responses: bool,
#[cfg(feature = "dns-over-rustls")]
#[cfg_attr(docsrs, doc(cfg(feature = "dns-over-rustls")))]
#[cfg_attr(feature = "serde-config", serde(skip))]
@ -425,7 +427,7 @@ impl NameServerConfig {
Self {
socket_addr,
protocol,
trust_nx_responses: true,
trust_negative_responses: true,
tls_dns_name: None,
#[cfg(feature = "dns-over-rustls")]
tls_config: None,
@ -502,7 +504,7 @@ impl NameServerConfigGroup {
/// Configure a NameServer address and port
///
/// This will create UDP and TCP connections, using the same port.
pub fn from_ips_clear(ips: &[IpAddr], port: u16, trust_nx_responses: bool) -> Self {
pub fn from_ips_clear(ips: &[IpAddr], port: u16, trust_negative_responses: bool) -> Self {
let mut name_servers = Self::with_capacity(ips.len());
for ip in ips {
@ -510,7 +512,7 @@ impl NameServerConfigGroup {
socket_addr: SocketAddr::new(*ip, port),
protocol: Protocol::Udp,
tls_dns_name: None,
trust_nx_responses,
trust_negative_responses,
#[cfg(feature = "dns-over-rustls")]
tls_config: None,
bind_addr: None,
@ -519,7 +521,7 @@ impl NameServerConfigGroup {
socket_addr: SocketAddr::new(*ip, port),
protocol: Protocol::Tcp,
tls_dns_name: None,
trust_nx_responses,
trust_negative_responses,
#[cfg(feature = "dns-over-rustls")]
tls_config: None,
bind_addr: None,
@ -538,7 +540,7 @@ impl NameServerConfigGroup {
port: u16,
tls_dns_name: String,
protocol: Protocol,
trust_nx_responses: bool,
trust_negative_responses: bool,
) -> Self {
assert!(protocol.is_encrypted());
@ -549,7 +551,7 @@ impl NameServerConfigGroup {
socket_addr: SocketAddr::new(*ip, port),
protocol,
tls_dns_name: Some(tls_dns_name.clone()),
trust_nx_responses,
trust_negative_responses,
#[cfg(feature = "dns-over-rustls")]
tls_config: None,
bind_addr: None,
@ -570,9 +572,15 @@ impl NameServerConfigGroup {
ips: &[IpAddr],
port: u16,
tls_dns_name: String,
trust_nx_responses: bool,
trust_negative_responses: bool,
) -> Self {
Self::from_ips_encrypted(ips, port, tls_dns_name, Protocol::Tls, trust_nx_responses)
Self::from_ips_encrypted(
ips,
port,
tls_dns_name,
Protocol::Tls,
trust_negative_responses,
)
}
/// Configure a NameServer address and port for DNS-over-HTTPS
@ -584,9 +592,15 @@ impl NameServerConfigGroup {
ips: &[IpAddr],
port: u16,
tls_dns_name: String,
trust_nx_responses: bool,
trust_negative_responses: bool,
) -> Self {
Self::from_ips_encrypted(ips, port, tls_dns_name, Protocol::Https, trust_nx_responses)
Self::from_ips_encrypted(
ips,
port,
tls_dns_name,
Protocol::Https,
trust_negative_responses,
)
}
/// Creates a default configuration, using `8.8.8.8`, `8.8.4.4` and `2001:4860:4860::8888`, `2001:4860:4860::8844` (thank you, Google).

8
crates/resolver/src/error.rs
View File

@ -185,12 +185,16 @@ impl ResolveError {
response_code @ ResponseCode::NoError
if !response.contains_answer() && !response.truncated() => {
// TODO: if authoritative, this is cacheable, store a TTL (currently that requires time, need a "now" here)
// let valid_until = if response.is_authoritative() { now + response.get_negative_ttl() };
// let valid_until = if response.authoritative() { now + response.negative_ttl() };
let mut response = response;
let soa = response.soa().cloned();
let negative_ttl = response.negative_ttl();
let trusted = if response_code == ResponseCode::NoError { false } else { trust_nx };
// Note: improperly configured servers may do recursive lookups and return bad SOA
// records here via AS112 (blackhole-1.iana.org. etc)
// Such servers should be marked not trusted, as they may break reverse lookups
// for local hosts.
let trusted = trust_nx && soa.is_some();
let query = response.take_queries().drain(..).next().unwrap_or_default();
let error_kind = ResolveErrorKind::NoRecordsFound {
query: Box::new(query),

12
crates/resolver/src/name_server/name_server.rs
View File

@ -149,7 +149,7 @@ impl<C: DnsHandle<Error = ResolveError>, P: ConnectionProvider<Conn = C>> NameSe
// First evaluate if the message succeeded.
let response =
ResolveError::from_response(response, self.config.trust_nx_responses)?;
ResolveError::from_response(response, self.config.trust_negative_responses)?;
// TODO: consider making message::take_edns...
let remote_edns = response.extensions().clone();
@ -176,7 +176,7 @@ impl<C: DnsHandle<Error = ResolveError>, P: ConnectionProvider<Conn = C>> NameSe
/// Specifies that this NameServer will treat negative responses as permanent failures and will not retry
pub fn trust_nx_responses(&self) -> bool {
self.config.trust_nx_responses
self.config.trust_negative_responses
}
}
@ -236,7 +236,7 @@ impl<C: DnsHandle<Error = ResolveError>, P: ConnectionProvider<Conn = C>> Eq for
pub(crate) fn mdns_nameserver<C, P>(
options: ResolverOpts,
conn_provider: P,
trust_nx_responses: bool,
trust_negative_responses: bool,
) -> NameServer<C, P>
where
C: DnsHandle<Error = ResolveError>,
@ -246,7 +246,7 @@ where
socket_addr: *MDNS_IPV4,
protocol: Protocol::Mdns,
tls_dns_name: None,
trust_nx_responses,
trust_negative_responses,
#[cfg(feature = "dns-over-rustls")]
tls_config: None,
bind_addr: None,
@ -278,7 +278,7 @@ mod tests {
socket_addr: SocketAddr::new(IpAddr::V4(Ipv4Addr::new(8, 8, 8, 8)), 53),
protocol: Protocol::Udp,
tls_dns_name: None,
trust_nx_responses: false,
trust_negative_responses: false,
#[cfg(feature = "dns-over-rustls")]
tls_config: None,
bind_addr: None,
@ -317,7 +317,7 @@ mod tests {
socket_addr: SocketAddr::new(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 252)), 252),
protocol: Protocol::Udp,
tls_dns_name: None,
trust_nx_responses: false,
trust_negative_responses: false,
#[cfg(feature = "dns-over-rustls")]
tls_config: None,
bind_addr: None,

6
crates/resolver/src/name_server/name_server_pool.rs
View File

@ -476,7 +476,7 @@ mod tests {
socket_addr: SocketAddr::new(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 252)), 253),
protocol: Protocol::Udp,
tls_dns_name: None,
trust_nx_responses: false,
trust_negative_responses: false,
#[cfg(feature = "dns-over-rustls")]
tls_config: None,
bind_addr: None,
@ -486,7 +486,7 @@ mod tests {
socket_addr: SocketAddr::new(IpAddr::V4(Ipv4Addr::new(8, 8, 8, 8)), 53),
protocol: Protocol::Udp,
tls_dns_name: None,
trust_nx_responses: false,
trust_negative_responses: false,
#[cfg(feature = "dns-over-rustls")]
tls_config: None,
bind_addr: None,
@ -548,7 +548,7 @@ mod tests {
socket_addr: SocketAddr::new(IpAddr::V4(Ipv4Addr::new(8, 8, 8, 8)), 53),
protocol: Protocol::Tcp,
tls_dns_name: None,
trust_nx_responses: false,
trust_negative_responses: false,
#[cfg(feature = "dns-over-rustls")]
tls_config: None,
bind_addr: None,

8
crates/resolver/src/system_conf/unix.rs
View File

@ -68,7 +68,7 @@ fn into_resolver_config(
socket_addr: SocketAddr::new(ip.into(), DEFAULT_PORT),
protocol: Protocol::Udp,
tls_dns_name: None,
trust_nx_responses: false,
trust_negative_responses: false,
#[cfg(feature = "dns-over-rustls")]
tls_config: None,
bind_addr: None,
@ -77,7 +77,7 @@ fn into_resolver_config(
socket_addr: SocketAddr::new(ip.into(), DEFAULT_PORT),
protocol: Protocol::Tcp,
tls_dns_name: None,
trust_nx_responses: false,
trust_negative_responses: false,
#[cfg(feature = "dns-over-rustls")]
tls_config: None,
bind_addr: None,
@ -129,7 +129,7 @@ mod tests {
socket_addr: addr,
protocol: Protocol::Udp,
tls_dns_name: None,
trust_nx_responses: false,
trust_negative_responses: false,
#[cfg(feature = "dns-over-rustls")]
tls_config: None,
bind_addr: None,
@ -138,7 +138,7 @@ mod tests {
socket_addr: addr,
protocol: Protocol::Tcp,
tls_dns_name: None,
trust_nx_responses: false,
trust_negative_responses: false,
#[cfg(feature = "dns-over-rustls")]
tls_config: None,
bind_addr: None,

4
crates/resolver/src/system_conf/windows.rs
View File

@ -32,7 +32,7 @@ fn get_name_servers() -> ResolveResult<Vec<NameServerConfig>> {
socket_addr,
protocol: Protocol::Udp,
tls_dns_name: None,
trust_nx_responses: false,
trust_negative_responses: false,
#[cfg(feature = "dns-over-rustls")]
tls_config: None,
bind_addr: None,
@ -41,7 +41,7 @@ fn get_name_servers() -> ResolveResult<Vec<NameServerConfig>> {
socket_addr,
protocol: Protocol::Tcp,
tls_dns_name: None,
trust_nx_responses: false,
trust_negative_responses: false,
#[cfg(feature = "dns-over-rustls")]
tls_config: None,
bind_addr: None,

4
crates/server/src/store/recursor/authority.rs
View File

@ -56,7 +56,7 @@ impl RecursiveAuthority {
socket_addr,
protocol: Protocol::Tcp,
tls_dns_name: None,
trust_nx_responses: false,
trust_negative_responses: false,
#[cfg(feature = "dns-over-rustls")]
tls_config: None,
bind_addr: None, // TODO: need to support bind addresses
@ -66,7 +66,7 @@ impl RecursiveAuthority {
socket_addr,
protocol: Protocol::Udp,
tls_dns_name: None,
trust_nx_responses: false,
trust_negative_responses: false,
#[cfg(feature = "dns-over-rustls")]
tls_config: None,
bind_addr: None,

7
tests/integration-tests/src/mock_client.rs
View File

@ -14,7 +14,7 @@ use futures::stream::{once, Stream};
use futures::{future, Future};
use trust_dns_client::op::{Message, Query};
use trust_dns_client::rr::{Name, RData, Record};
use trust_dns_client::rr::{rdata::SOA, Name, RData, Record};
use trust_dns_proto::error::ProtoError;
use trust_dns_proto::xfer::{DnsHandle, DnsRequest, DnsResponse};
@ -82,6 +82,11 @@ pub fn v4_record(name: Name, ip: Ipv4Addr) -> Record {
Record::from_rdata(name, 86400, RData::A(ip))
}
pub fn soa_record(name: Name, mname: Name) -> Record {
let soa = SOA::new(mname, Default::default(), 1, 3600, 60, 86400, 3600);
Record::from_rdata(name, 86400, RData::SOA(soa))
}
pub fn message(
query: Query,
answers: Vec<Record>,

85
tests/integration-tests/tests/name_server_pool_tests.rs
View File

@ -10,14 +10,14 @@ use std::task::Poll;
use futures::executor::block_on;
use futures::{future, Future};
use trust_dns_client::op::Query;
use trust_dns_client::op::{Query, ResponseCode};
use trust_dns_client::rr::{Name, RecordType};
use trust_dns_integration::mock_client::*;
use trust_dns_proto::error::ProtoError;
use trust_dns_proto::xfer::{DnsHandle, DnsResponse, FirstAnswer};
use trust_dns_proto::TokioTime;
use trust_dns_resolver::config::*;
use trust_dns_resolver::error::ResolveError;
use trust_dns_resolver::error::{ResolveError, ResolveErrorKind};
use trust_dns_resolver::name_server::{ConnectionProvider, NameServer, NameServerPool};
const DEFAULT_SERVER_ADDR: IpAddr = IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1));
@ -72,14 +72,14 @@ fn mock_nameserver_with_addr(
fn mock_nameserver_trust_nx(
messages: Vec<Result<DnsResponse, ResolveError>>,
options: ResolverOpts,
trust_nx_responses: bool,
trust_negative_responses: bool,
) -> MockedNameServer<DefaultOnSend> {
mock_nameserver_on_send_nx(
messages,
options,
DefaultOnSend,
DEFAULT_SERVER_ADDR,
trust_nx_responses,
trust_negative_responses,
)
}
@ -98,7 +98,7 @@ fn mock_nameserver_on_send_nx<O: OnSend + Unpin>(
options: ResolverOpts,
on_send: O,
addr: IpAddr,
trust_nx_responses: bool,
trust_negative_responses: bool,
) -> MockedNameServer<O> {
let conn_provider = MockConnProvider {
on_send: on_send.clone(),
@ -110,7 +110,7 @@ fn mock_nameserver_on_send_nx<O: OnSend + Unpin>(
socket_addr: SocketAddr::new(addr, 0),
protocol: Protocol::Udp,
tls_dns_name: None,
trust_nx_responses,
trust_negative_responses,
#[cfg(any(feature = "dns-over-rustls", feature = "dns-over-https-rustls"))]
tls_config: None,
bind_addr: None,
@ -275,9 +275,6 @@ fn test_datagram_fails_to_stream() {
#[test]
fn test_tcp_fallback_only_on_truncated() {
use trust_dns_proto::op::ResponseCode;
use trust_dns_resolver::error::{ResolveError, ResolveErrorKind};
// Lookup to UDP should fail with an error, and the resolver should not then try the query over
// TCP, because the default behavior is only to retry if the response was truncated.
@ -350,12 +347,14 @@ fn test_local_mdns() {
#[test]
fn test_trust_nx_responses_fails() {
use trust_dns_proto::op::ResponseCode;
use trust_dns_resolver::error::ResolveErrorKind;
let query = Query::query(Name::from_str("www.example.").unwrap(), RecordType::A);
let mut nx_message = message(query.clone(), vec![], vec![], vec![]);
// NXDOMAIN responses are only trusted if there's a SOA record, so make sure we return one.
let soa_record = soa_record(
query.name().clone(),
Name::from_str("example.com.").unwrap(),
);
let mut nx_message = message(query.clone(), vec![], vec![soa_record], vec![]);
nx_message.set_response_code(ResponseCode::NXDomain);
let success_msg = message(
@ -395,9 +394,62 @@ fn test_trust_nx_responses_fails() {
}
#[test]
fn test_distrust_nx_responses() {
use trust_dns_proto::op::ResponseCode;
fn test_noerror_doesnt_leak() {
let query = Query::query(Name::from_str("www.example.com.").unwrap(), RecordType::A);
let soa_record = soa_record(
query.name().clone(),
Name::from_str("example.com.").unwrap(),
);
let udp_message = message(query.clone(), vec![], vec![soa_record], vec![]);
let incorrect_success_msg = message(
query.clone(),
vec![v4_record(query.name().clone(), Ipv4Addr::new(127, 0, 0, 2))],
vec![],
vec![],
);
let udp_nameserver =
mock_nameserver_trust_nx(vec![Ok(udp_message.into())], Default::default(), true);
// Provide a fake A record; if this nameserver is queried the test should fail.
let second_nameserver = mock_nameserver_trust_nx(
vec![Ok(incorrect_success_msg.into())],
Default::default(),
true,
);
let mut options = ResolverOpts::default();
options.num_concurrent_reqs = 1;
options.server_ordering_strategy = ServerOrderingStrategy::UserProvidedOrder;
let mut pool = mock_nameserver_pool(
vec![udp_nameserver, second_nameserver],
vec![],
None,
options,
);
// lookup should only hit the first server
let request = message(query, vec![], vec![], vec![]);
let future = pool.send(request).first_answer();
match block_on(future).unwrap_err().kind() {
ResolveErrorKind::NoRecordsFound {
soa,
response_code,
trusted,
..
} => {
assert_eq!(response_code, &ResponseCode::NoError);
assert!(soa.is_some());
assert!(trusted);
}
x => panic!("Expected NoRecordsFound, got {:?}", x),
}
}
#[test]
fn test_distrust_nx_responses() {
let query = Query::query(Name::from_str("www.example.").unwrap(), RecordType::A);
const RETRYABLE_ERRORS: [ResponseCode; 9] = [
@ -514,9 +566,6 @@ fn test_user_provided_server_order() {
#[test]
fn test_return_error_from_highest_priority_nameserver() {
use trust_dns_proto::op::ResponseCode;
use trust_dns_resolver::error::ResolveErrorKind;
let query = Query::query(Name::from_str("www.example.").unwrap(), RecordType::A);
const ERROR_RESPONSE_CODES: [ResponseCode; 4] = [

4
util/src/recurse.rs
View File

@ -125,7 +125,7 @@ pub async fn main() -> Result<(), Box<dyn std::error::Error>> {
socket_addr: *socket_addr,
protocol: Protocol::Tcp,
tls_dns_name: None,
trust_nx_responses: false,
trust_negative_responses: false,
#[cfg(feature = "dns-over-rustls")]
tls_config: None,
bind_addr: opts.bind.map(|ip| SocketAddr::new(ip, 0)),
@ -135,7 +135,7 @@ pub async fn main() -> Result<(), Box<dyn std::error::Error>> {
socket_addr: *socket_addr,
protocol: Protocol::Udp,
tls_dns_name: None,
trust_nx_responses: false,
trust_negative_responses: false,
#[cfg(feature = "dns-over-rustls")]
tls_config: None,
bind_addr: opts.bind.map(|ip| SocketAddr::new(ip, 0)),

4
util/src/resolve.rs
View File

@ -275,7 +275,7 @@ pub async fn main() -> Result<(), Box<dyn std::error::Error>> {
socket_addr: *socket_addr,
protocol: Protocol::Tcp,
tls_dns_name: None,
trust_nx_responses: false,
trust_negative_responses: false,
#[cfg(feature = "dns-over-rustls")]
tls_config: None,
bind_addr: opts.bind.map(|ip| SocketAddr::new(ip, 0)),
@ -285,7 +285,7 @@ pub async fn main() -> Result<(), Box<dyn std::error::Error>> {
socket_addr: *socket_addr,
protocol: Protocol::Udp,
tls_dns_name: None,
trust_nx_responses: false,
trust_negative_responses: false,
#[cfg(feature = "dns-over-rustls")]
tls_config: None,
bind_addr: opts.bind.map(|ip| SocketAddr::new(ip, 0)),