Fix dns bad nxdomain (#3299)

Some dns servers return NXDOMAIN for queries where the address exists
but there is no
answer for the given query type(e.g. AAAA-only records). This is not up
to spec and
musl PROPERLY assumes that means there is no record of any type. Saddly,
this happens even
with google DNS so we can expect it to happen everywhere. So we use
getaddrinfo to separate
requests for A and AAAA queries and preventing this.

Seems to work locally, though the exact situation where we have a record
that returns NXDOMAIN while it exists is easier to reproduce in staging,
we should test it after we merge.

Fixes #3215
This commit is contained in:
Gabi
2024-01-17 23:30:30 -03:00
committed by GitHub
parent 1b42e577da
commit 973e48cf68
5 changed files with 67 additions and 4 deletions

14
rust/Cargo.lock generated
View File

@@ -1221,6 +1221,7 @@ dependencies = [
"base64 0.21.7",
"boringtun",
"chrono",
"dns-lookup",
"domain",
"futures",
"futures-util",
@@ -1634,6 +1635,18 @@ dependencies = [
"syn 2.0.48",
]
[[package]]
name = "dns-lookup"
version = "2.0.4"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "e5766087c2235fec47fafa4cfecc81e494ee679d0fd4a59887ea0919bfb0e4fc"
dependencies = [
"cfg-if",
"libc",
"socket2 0.5.5",
"windows-sys 0.48.0",
]
[[package]]
name = "domain"
version = "0.9.3"
@@ -2057,6 +2070,7 @@ dependencies = [
"bytes",
"chrono",
"connlib-shared",
"dns-lookup",
"domain",
"futures",
"futures-bounded",

View File

@@ -30,6 +30,7 @@ webrtc = "0.9"
str0m = "0.4"
futures-bounded = "0.2.1"
domain = { version = "0.9", features = ["serde"] }
dns-lookup = "2.0"
connlib-client-android = { path = "connlib/clients/android"}
connlib-client-apple = { path = "connlib/clients/apple"}

View File

@@ -37,6 +37,7 @@ ring = "0.17"
hickory-resolver = { workspace = true }
domain = { workspace = true }
libc = "0.2"
dns-lookup = { workspace = true }
# Needed for Android logging until tracing is working
log = "0.4"

View File

@@ -29,6 +29,7 @@ futures-bounded = { workspace = true }
hickory-resolver = { workspace = true }
arc-swap = "1.6.0"
bimap = "0.6"
dns-lookup = { workspace = true }
# TODO: research replacing for https://github.com/algesten/str0m
webrtc = { workspace = true }

View File

@@ -101,10 +101,9 @@ where
return Err(Error::InvalidResource);
}
(domain.to_string(), 0)
.to_socket_addrs()?
.map(|addrs| addrs.ip().into())
.collect()
// TODO: we should make this async, this is acceptable for now though
// in the future we will use hickory-resolver for this anyways.
resolve_addresses(&domain.to_string())?
}
ResourceDescription::Cidr(ref cidr) => vec![cidr.address],
};
@@ -262,3 +261,50 @@ where
Ok(())
}
}
#[cfg(target_os = "windows")]
fn resolve_addresses(_: &str) -> std::io::Result<Vec<IpNetwork>> {
unimplemented!()
}
#[cfg(not(target_os = "windows"))]
fn resolve_addresses(addr: &str) -> std::io::Result<Vec<IpNetwork>> {
use libc::{AF_INET, AF_INET6};
let addr_v4: std::io::Result<Vec<_>> = resolve_address_family(addr, AF_INET)
.map_err(|e| e.into())
.and_then(|a| a.collect());
let addr_v6: std::io::Result<Vec<_>> = resolve_address_family(addr, AF_INET6)
.map_err(|e| e.into())
.and_then(|a| a.collect());
match (addr_v4, addr_v6) {
(Ok(v4), Ok(v6)) => Ok(v6
.iter()
.map(|a| a.sockaddr.ip().into())
.chain(v4.iter().map(|a| a.sockaddr.ip().into()))
.collect()),
(Ok(v4), Err(_)) => Ok(v4.iter().map(|a| a.sockaddr.ip().into()).collect()),
(Err(_), Ok(v6)) => Ok(v6.iter().map(|a| a.sockaddr.ip().into()).collect()),
(Err(e), Err(_)) => Err(e),
}
}
#[cfg(not(target_os = "windows"))]
use dns_lookup::{AddrInfoHints, AddrInfoIter, LookupError};
#[cfg(not(target_os = "windows"))]
fn resolve_address_family(
addr: &str,
family: i32,
) -> std::result::Result<AddrInfoIter, LookupError> {
use libc::SOCK_STREAM;
dns_lookup::getaddrinfo(
Some(addr),
None,
Some(AddrInfoHints {
socktype: SOCK_STREAM,
address: family,
..Default::default()
}),
)
}