move NX trust bit from ResolverOpts to NameServerConfig

This commit is contained in:
Dirkjan Ochtman 2020-09-24 13:11:44 +02:00 committed by Benjamin Fry
parent 63c1b6ca51
commit 183526ef66
9 changed files with 86 additions and 27 deletions

View File

@ -9,6 +9,7 @@ All notes should be prepended with the location of the change, e.g. `(proto)` or
### Changed
- (resolver) *BREAKING* Move `ResolverOpts::distrust_nx_responses` to `NameServerConfig::trust_nx_responses` (@djc) #1212
- (proto) `data-encoding` is now a required dependency #1208
- (all) minimum rustc version now `1.42`
- (resolver) For all NxDomain and NoError/NoData responses, `ResolveErrorKind::NoRecordsFound` will be returned #1197
@ -25,7 +26,7 @@ All notes should be prepended with the location of the change, e.g. `(proto)` or
### Fixed
- (resolver) Fix Glue records resolving (@wavenator) #1188
- (resolver) Fix Glue records resolving (@wavenator) #1188
- (resolver) Only fall back on TCP if cons are available (@lukaspustina) #1181
- (proto) fix empty option at end of edns (@jonasbb) #1143, #744
- (resolver) Return `REFUSED` instead of `NXDOMAIN` when server is not an authority (@AnIrishDuck) #1137
@ -343,7 +344,7 @@ All notes should be prepended with the location of the change, e.g. `(proto)` or
### Changed
- Large celanup of signing and verification paths in DNSSec (@briansmith)
- *breaking* changed `TrustAnchor::insert_trust_anchor` to more safely consume `PublicKey` rather than `Vec<u8>`
- *breaking* changed `TrustAnchor::insert_trust_anchor` to more safely consume `PublicKey` rather than `Vec<u8>`
## 0.11.2 (Client/Server)

View File

@ -335,6 +335,10 @@ pub struct NameServerConfig {
pub protocol: Protocol,
/// SPKI name, only relevant for TLS connections
pub tls_dns_name: Option<String>,
/// Default is to distrust negative responses from upstream nameservers
///
/// Currently only SERVFAIL responses are continued on, this may be expanded to include NXDOMAIN or NoError/Empty responses
pub trust_nx_responses: bool,
#[cfg(feature = "dns-over-rustls")]
#[cfg_attr(feature = "serde-config", serde(skip))]
/// optional configuration for the tls client
@ -404,7 +408,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) -> Self {
pub fn from_ips_clear(ips: &[IpAddr], port: u16, trust_nx_responses: bool) -> Self {
let mut name_servers = Self::with_capacity(ips.len());
for ip in ips {
@ -412,6 +416,7 @@ impl NameServerConfigGroup {
socket_addr: SocketAddr::new(*ip, port),
protocol: Protocol::Udp,
tls_dns_name: None,
trust_nx_responses,
#[cfg(feature = "dns-over-rustls")]
tls_config: None,
};
@ -419,6 +424,7 @@ impl NameServerConfigGroup {
socket_addr: SocketAddr::new(*ip, port),
protocol: Protocol::Tcp,
tls_dns_name: None,
trust_nx_responses,
#[cfg(feature = "dns-over-rustls")]
tls_config: None,
};
@ -436,6 +442,7 @@ impl NameServerConfigGroup {
port: u16,
tls_dns_name: String,
protocol: Protocol,
trust_nx_responses: bool,
) -> Self {
assert!(protocol.is_encrypted());
@ -446,6 +453,7 @@ impl NameServerConfigGroup {
socket_addr: SocketAddr::new(*ip, port),
protocol,
tls_dns_name: Some(tls_dns_name.clone()),
trust_nx_responses,
#[cfg(feature = "dns-over-rustls")]
tls_config: None,
};
@ -460,16 +468,26 @@ impl NameServerConfigGroup {
///
/// This will create a TLS connections.
#[cfg(feature = "dns-over-tls")]
pub fn from_ips_tls(ips: &[IpAddr], port: u16, tls_dns_name: String) -> Self {
Self::from_ips_encrypted(ips, port, tls_dns_name, Protocol::Tls)
pub fn from_ips_tls(
ips: &[IpAddr],
port: u16,
tls_dns_name: String,
trust_nx_responses: bool,
) -> Self {
Self::from_ips_encrypted(ips, port, tls_dns_name, Protocol::Tls, trust_nx_responses)
}
/// Configure a NameServer address and port for DNS-over-HTTPS
///
/// This will create a HTTPS connections.
#[cfg(feature = "dns-over-https")]
pub fn from_ips_https(ips: &[IpAddr], port: u16, tls_dns_name: String) -> Self {
Self::from_ips_encrypted(ips, port, tls_dns_name, Protocol::Https)
pub fn from_ips_https(
ips: &[IpAddr],
port: u16,
tls_dns_name: String,
trust_nx_responses: bool,
) -> Self {
Self::from_ips_encrypted(ips, port, tls_dns_name, Protocol::Https, trust_nx_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).
@ -484,6 +502,7 @@ impl NameServerConfigGroup {
IpAddr::V6(Ipv6Addr::new(0x2001, 0x4860, 0x4860, 0, 0, 0, 0, 0x8844)),
],
53,
true,
)
}
@ -499,6 +518,7 @@ impl NameServerConfigGroup {
IpAddr::V6(Ipv6Addr::new(0x2606, 0x4700, 0x4700, 0, 0, 0, 0, 0x1001)),
],
53,
true,
)
}
@ -516,6 +536,7 @@ impl NameServerConfigGroup {
],
853,
"cloudflare-dns.com".to_string(),
true,
)
}
@ -533,6 +554,7 @@ impl NameServerConfigGroup {
],
443,
"cloudflare-dns.com".to_string(),
true,
)
}
@ -546,6 +568,7 @@ impl NameServerConfigGroup {
IpAddr::V6(Ipv6Addr::new(0x2620, 0x00fe, 0, 0, 0, 0, 0, 0x00fe)),
],
53,
true,
)
}
@ -561,6 +584,7 @@ impl NameServerConfigGroup {
],
853,
"dns.quad9.net".to_string(),
true,
)
}
@ -708,10 +732,6 @@ pub struct ResolverOpts {
///
/// [`MAX_TTL`]: ../dns_lru/const.MAX_TTL.html
pub negative_max_ttl: Option<Duration>,
/// Default is to distrust negative responses from upstream nameservers
///
/// Currently only SERVFAIL responses are continued on, this may be expanded to include NXDOMAIN or NoError/Empty responses
pub distrust_nx_responses: bool,
/// Number of concurrent requests per query
///
/// Where more than one nameserver is configured, this configures the resolver to send queries
@ -741,7 +761,6 @@ impl Default for ResolverOpts {
negative_min_ttl: None,
positive_max_ttl: None,
negative_max_ttl: None,
distrust_nx_responses: true,
num_concurrent_reqs: 2,
preserve_intermediates: false,
}

View File

@ -142,7 +142,7 @@ impl<C: DnsHandle<Error = ResolveError>, P: ConnectionProvider<Conn = C>> NameSe
// see https://github.com/bluejekyll/trust-dns/issues/606
// TODO: there are probably other return codes from the server we may want to
// retry on. We may also want to evaluate NoError responses that lack records as errors as well
let response = if self.options.distrust_nx_responses {
let response = if self.config.trust_nx_responses {
ResolveError::from_response(response)?
} else {
response
@ -238,7 +238,11 @@ impl<C: DnsHandle<Error = ResolveError>, P: ConnectionProvider<Conn = C>> Eq for
// TODO: once IPv6 is better understood, also make this a binary keep.
#[cfg(feature = "mdns")]
pub(crate) fn mdns_nameserver<C, P>(options: ResolverOpts, conn_provider: P) -> NameServer<C, P>
pub(crate) fn mdns_nameserver<C, P>(
options: ResolverOpts,
conn_provider: P,
trust_nx_responses: bool,
) -> NameServer<C, P>
where
C: DnsHandle<Error = ResolveError>,
P: ConnectionProvider<Conn = C>,
@ -247,6 +251,7 @@ where
socket_addr: *MDNS_IPV4,
protocol: Protocol::Mdns,
tls_dns_name: None,
trust_nx_responses,
#[cfg(feature = "dns-over-rustls")]
tls_config: None,
};
@ -279,6 +284,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,
#[cfg(feature = "dns-over-rustls")]
tls_config: None,
};
@ -312,6 +318,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,
#[cfg(feature = "dns-over-rustls")]
tls_config: None,
};

View File

@ -106,7 +106,7 @@ where
datagram_conns: Arc::new(datagram_conns),
stream_conns: Arc::new(stream_conns),
#[cfg(feature = "mdns")]
mdns_conns: name_server::mdns_nameserver(*options, conn_provider.clone()),
mdns_conns: name_server::mdns_nameserver(*options, conn_provider.clone(), false),
options: *options,
conn_provider,
}
@ -428,6 +428,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,
#[cfg(feature = "dns-over-rustls")]
tls_config: None,
};
@ -436,6 +437,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,
#[cfg(feature = "dns-over-rustls")]
tls_config: None,
};
@ -492,6 +494,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,
#[cfg(feature = "dns-over-rustls")]
tls_config: None,
};
@ -506,7 +509,7 @@ mod tests {
Arc::new(vec![]),
Arc::clone(&name_servers),
#[cfg(feature = "mdns")]
name_server::mdns_nameserver(opts, conn_provider.clone()),
name_server::mdns_nameserver(opts, conn_provider.clone(), false),
conn_provider,
);

View File

@ -64,6 +64,7 @@ fn into_resolver_config(
socket_addr: SocketAddr::new(ip.into(), DEFAULT_PORT),
protocol: Protocol::Udp,
tls_dns_name: None,
trust_nx_responses: false,
#[cfg(feature = "dns-over-rustls")]
tls_config: None,
});
@ -71,6 +72,7 @@ fn into_resolver_config(
socket_addr: SocketAddr::new(ip.into(), DEFAULT_PORT),
protocol: Protocol::Tcp,
tls_dns_name: None,
trust_nx_responses: false,
#[cfg(feature = "dns-over-rustls")]
tls_config: None,
});
@ -119,6 +121,7 @@ mod tests {
socket_addr: addr,
protocol: Protocol::Udp,
tls_dns_name: None,
trust_nx_responses: false,
#[cfg(feature = "dns-over-rustls")]
tls_config: None,
},
@ -126,6 +129,7 @@ mod tests {
socket_addr: addr,
protocol: Protocol::Tcp,
tls_dns_name: None,
trust_nx_responses: false,
#[cfg(feature = "dns-over-rustls")]
tls_config: None,
},

View File

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

View File

@ -59,7 +59,16 @@ fn mock_nameserver(
messages: Vec<Result<DnsResponse, ResolveError>>,
options: ResolverOpts,
) -> MockedNameServer<DefaultOnSend> {
mock_nameserver_on_send(messages, options, DefaultOnSend)
mock_nameserver_on_send_nx(messages, options, DefaultOnSend, false)
}
#[cfg(test)]
fn mock_nameserver_trust_nx(
messages: Vec<Result<DnsResponse, ResolveError>>,
options: ResolverOpts,
trust_nx_responses: bool,
) -> MockedNameServer<DefaultOnSend> {
mock_nameserver_on_send_nx(messages, options, DefaultOnSend, trust_nx_responses)
}
#[cfg(test)]
@ -67,6 +76,16 @@ fn mock_nameserver_on_send<O: OnSend + Unpin>(
messages: Vec<Result<DnsResponse, ResolveError>>,
options: ResolverOpts,
on_send: O,
) -> MockedNameServer<O> {
mock_nameserver_on_send_nx(messages, options, on_send, false)
}
#[cfg(test)]
fn mock_nameserver_on_send_nx<O: OnSend + Unpin>(
messages: Vec<Result<DnsResponse, ResolveError>>,
options: ResolverOpts,
on_send: O,
trust_nx_responses: bool,
) -> MockedNameServer<O> {
let conn_provider = MockConnProvider {
on_send: on_send.clone(),
@ -78,6 +97,7 @@ fn mock_nameserver_on_send<O: OnSend + Unpin>(
socket_addr: SocketAddr::new(Ipv4Addr::new(127, 0, 0, 1).into(), 0),
protocol: Protocol::Udp,
tls_dns_name: None,
trust_nx_responses,
#[cfg(any(feature = "dns-over-rustls", feature = "dns-over-https-rustls"))]
tls_config: None,
},
@ -250,8 +270,7 @@ fn test_local_mdns() {
fn test_trust_nx_responses_fails_servfail() {
use trust_dns_proto::op::ResponseCode;
let mut options = ResolverOpts::default();
options.distrust_nx_responses = false;
let options = ResolverOpts::default();
let query = Query::query(Name::from_str("www.example.").unwrap(), RecordType::A);
@ -266,16 +285,18 @@ fn test_trust_nx_responses_fails_servfail() {
let udp_message = success_msg;
// fail the first udp request
let udp_nameserver = mock_nameserver(
let udp_nameserver = mock_nameserver_trust_nx(
vec![
Ok(udp_message.into()),
servfail_message.clone().map(Into::into),
],
options,
false,
);
let tcp_nameserver = mock_nameserver(
let tcp_nameserver = mock_nameserver_trust_nx(
vec![Err(ResolveError::from("Forced Testing Error"))],
options,
false,
);
let mut pool = mock_nameserver_pool(vec![udp_nameserver], vec![tcp_nameserver], None, options);
@ -304,8 +325,7 @@ fn test_trust_nx_responses_fails_servfail() {
fn test_distrust_nx_responses() {
use trust_dns_proto::op::ResponseCode;
let mut options = ResolverOpts::default();
options.distrust_nx_responses = true;
let options = ResolverOpts::default();
let query = Query::query(Name::from_str("www.example.").unwrap(), RecordType::A);
@ -320,8 +340,9 @@ fn test_distrust_nx_responses() {
//let udp_message = success_msg;
// fail the first udp request
let udp_nameserver = mock_nameserver(vec![servfail_message.map(Into::into)], options);
let tcp_nameserver = mock_nameserver(vec![Ok(tcp_message.into())], options);
let udp_nameserver =
mock_nameserver_trust_nx(vec![servfail_message.map(Into::into)], options, true);
let tcp_nameserver = mock_nameserver_trust_nx(vec![Ok(tcp_message.into())], options, true);
let mut pool = mock_nameserver_pool(vec![udp_nameserver], vec![tcp_nameserver], None, options);

View File

@ -37,5 +37,5 @@ zone_type = "Forward"
## remember the port, defaults: 53 for Udp & Tcp, 853 for Tls and 443 for Https.
## Tls and/or Https require features dns-over-tls and/or dns-over-https
stores = { type = "forward", name_servers = [{ socket_addr = "8.8.8.8:53", protocol = "Udp" },
{ socket_addr = "8.8.8.8:53", protocol = "Tcp" }] }
stores = { type = "forward", name_servers = [{ socket_addr = "8.8.8.8:53", protocol = "Udp", trust_nx_responses = false },
{ socket_addr = "8.8.8.8:53", protocol = "Tcp", trust_nx_responses = false }] }

View File

@ -134,12 +134,14 @@ 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,
});
name_servers.push(NameServerConfig {
socket_addr: *socket_addr,
protocol: Protocol::Udp,
tls_dns_name: None,
trust_nx_responses: false,
});
}