serve: parse resolv.conf ourselves

The resolv.conf parsing lib is super strict and does not allow unknown
options, given we do not care about options search domains or really
anything else in this file parse it ourselves with very lax rules.

Basically we try to find the nameserver lines. Anything else is ignored,
the only error we produce is if we fail to parse the nameserver ip
address.

Fixes #418
Fixes https://issues.redhat.com/browse/RHEL-57695

Signed-off-by: Paul Holzinger <pholzing@redhat.com>

5 files changed, 121 insertions, 43 deletions

diff --git a/Cargo.lock b/Cargo.lock
index 6a7db2d..d45bd9a 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -17,7 +17,6 @@ dependencies = [
  "libc",
  "log",
  "nix",
- "resolv-conf",
  "syslog",
  "tokio",
 ]
@@ -684,12 +683,6 @@ dependencies = [
 ]
 
 [[package]]
-name = "quick-error"
-version = "1.2.3"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "a1d01941d82fa2ab50be1e79e6714289dd7cde78eba4c074bc5a4374f650dfe0"
-
-[[package]]
 name = "quote"
 version = "1.0.36"
 source = "registry+https://github.com/rust-lang/crates.io-index"
@@ -739,15 +732,6 @@ dependencies = [
 ]
 
 [[package]]
-name = "resolv-conf"
-version = "0.7.0"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "52e44394d2086d010551b14b53b1f24e31647570cd1deb0379e2c21b329aba00"
-dependencies = [
- "quick-error",
-]
-
-[[package]]
 name = "rustc-demangle"
 version = "0.1.24"
 source = "registry+https://github.com/rust-lang/crates.io-index"
diff --git a/Cargo.toml b/Cargo.toml
index cf59e6c..a3448c8 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -30,7 +30,6 @@ hickory-proto = { version = "0.24.1", features = ["tokio-runtime"] }
 hickory-client = "0.24.1"
 futures-util = { version = "0.3.30", default-features = false }
 tokio = { version = "1.40.0", features = ["macros", "rt-multi-thread", "net", "signal"] }
-resolv-conf = "0.7.0"
 nix = { version = "0.29.0", features = ["fs", "signal"] }
 libc = "0.2.159"
 arc-swap = "1.7.1"
diff --git a/src/dns/coredns.rs b/src/dns/coredns.rs
index 71ca70e..d5b817a 100644
--- a/src/dns/coredns.rs
+++ b/src/dns/coredns.rs
@@ -17,8 +17,6 @@ use hickory_proto::{
     DnsStreamHandle,
 };
 use log::{debug, error, trace, warn};
-use resolv_conf;
-use resolv_conf::ScopedIp;
 use std::io::Error;
 use std::net::{IpAddr, SocketAddr};
 use std::sync::Arc;
@@ -40,10 +38,10 @@ pub struct CoreDns {
 
 #[derive(Clone)]
 struct CoreDnsData {
-    network_name: String,                   // raw network name
-    backend: &'static ArcSwap<DNSBackend>,  // server's data store
-    no_proxy: bool,                         // do not forward to external resolvers
-    nameservers: Arc<Mutex<Vec<ScopedIp>>>, // host nameservers from resolv.conf
+    network_name: String,                  // raw network name
+    backend: &'static ArcSwap<DNSBackend>, // server's data store
+    no_proxy: bool,                        // do not forward to external resolvers
+    nameservers: Arc<Mutex<Vec<IpAddr>>>,  // host nameservers from resolv.conf
 }
 
 enum Protocol {
@@ -59,7 +57,7 @@ impl CoreDns {
         backend: &'static ArcSwap<DNSBackend>,
         rx: flume::Receiver<()>,
         no_proxy: bool,
-        nameservers: Arc<Mutex<Vec<ScopedIp>>>,
+        nameservers: Arc<Mutex<Vec<IpAddr>>>,
     ) -> Self {
         CoreDns {
             rx,
@@ -219,18 +217,18 @@ impl CoreDns {
                 "Forwarding dns request for {} type: {}",
                 &request_name_string, record_type
             );
-            let mut nameservers: Vec<ScopedIp> = Vec::new();
+            let mut nameservers: Vec<IpAddr> = Vec::new();
             // Add resolvers configured for container
             if let Some(Some(dns_servers)) = backend.ctr_dns_server.get(&src_address.ip()) {
                 for dns_server in dns_servers.iter() {
-                    nameservers.push(ScopedIp::from(*dns_server));
+                    nameservers.push(*dns_server);
                 }
                 // Add network scoped resolvers only if container specific resolvers were not configured
             } else if let Some(network_dns_servers) =
                 backend.get_network_scoped_resolvers(&src_address.ip())
             {
                 for dns_server in network_dns_servers.iter() {
-                    nameservers.push(ScopedIp::from(*dns_server));
+                    nameservers.push(*dns_server);
                 }
             }
             // Use host resolvers if no custom resolvers are set for the container.
@@ -257,7 +255,7 @@ impl CoreDns {
     }
 
     async fn forward_to_servers(
-        nameservers: Vec<ScopedIp>,
+        nameservers: Vec<IpAddr>,
         mut sender: BufDnsStreamHandle,
         src_address: SocketAddr,
         req: Message,
@@ -265,7 +263,7 @@ impl CoreDns {
     ) {
         // forward dns request to hosts's /etc/resolv.conf
         for nameserver in &nameservers {
-            let addr = SocketAddr::new(nameserver.into(), 53);
+            let addr = SocketAddr::new(*nameserver, 53);
             let (client, handle) = match proto {
                 Protocol::Udp => {
                     let stream = UdpClientStream::<UdpSocket>::new(addr);
diff --git a/src/error.rs b/src/error.rs
index 6c81f53..fa256cb 100644
--- a/src/error.rs
+++ b/src/error.rs
@@ -8,7 +8,7 @@ pub enum AardvarkError {
     IOError(std::io::Error),
     Chain(String, Box<Self>),
     List(AardvarkErrorList),
-    ResolvConfParseError(resolv_conf::ParseError),
+    AddrParseError(std::net::AddrParseError),
 }
 
 impl AardvarkError {
@@ -59,7 +59,7 @@ impl fmt::Display for AardvarkError {
             Self::Message(s) => write!(f, "{s}"),
             Self::Chain(s, e) => write!(f, "{s}: {e}"),
             Self::IOError(e) => write!(f, "IO error: {e}"),
-            Self::ResolvConfParseError(e) => write!(f, "parse resolv.conf: {e}"),
+            Self::AddrParseError(e) => write!(f, "parse address: {e}"),
             Self::List(list) => {
                 // some extra code to only add \n when it contains multiple errors
                 let mut iter = list.0.iter();
@@ -87,9 +87,9 @@ impl From<nix::Error> for AardvarkError {
     }
 }
 
-impl From<resolv_conf::ParseError> for AardvarkError {
-    fn from(err: resolv_conf::ParseError) -> Self {
-        Self::ResolvConfParseError(err)
+impl From<std::net::AddrParseError> for AardvarkError {
+    fn from(err: std::net::AddrParseError) -> Self {
+        Self::AddrParseError(err)
     }
 }
 
diff --git a/src/server/serve.rs b/src/server/serve.rs
index 6f30ed5..261ba6b 100644
--- a/src/server/serve.rs
+++ b/src/server/serve.rs
@@ -10,7 +10,6 @@ use arc_swap::ArcSwap;
 use log::{debug, error, info};
 use nix::unistd;
 use nix::unistd::dup2;
-use resolv_conf::ScopedIp;
 use std::collections::HashMap;
 use std::collections::HashSet;
 use std::env;
@@ -126,7 +125,7 @@ async fn stop_and_start_threads<Ip>(
     listen_ips: HashMap<String, Vec<Ip>>,
     thread_handles: &mut ThreadHandleMap<Ip>,
     no_proxy: bool,
-    nameservers: Arc<Mutex<Vec<ScopedIp>>>,
+    nameservers: Arc<Mutex<Vec<IpAddr>>>,
 ) -> AardvarkResult<()>
 where
     Ip: Eq + Hash + Copy + Into<IpAddr> + Send + 'static,
@@ -248,7 +247,7 @@ async fn start_dns_server(
     backend: &'static ArcSwap<DNSBackend>,
     rx: flume::Receiver<()>,
     no_proxy: bool,
-    nameservers: Arc<Mutex<Vec<ScopedIp>>>,
+    nameservers: Arc<Mutex<Vec<IpAddr>>>,
 ) -> AardvarkResult<()> {
     let server = CoreDns::new(name, backend, rx, no_proxy, nameservers);
     server
@@ -263,7 +262,7 @@ async fn read_config_and_spawn(
     filter_search_domain: &str,
     handles_v4: &mut ThreadHandleMap<Ipv4Addr>,
     handles_v6: &mut ThreadHandleMap<Ipv6Addr>,
-    nameservers: Arc<Mutex<Vec<ScopedIp>>>,
+    nameservers: Arc<Mutex<Vec<IpAddr>>>,
     no_proxy: bool,
 ) -> AardvarkResult<()> {
     let (conf, listen_ip_v4, listen_ip_v6) =
@@ -314,6 +313,7 @@ async fn read_config_and_spawn(
             Vec::new()
         }
     };
+    debug!("Using the following upstream servers: {upstream_resolvers:?}");
 
     {
         // use new scope to only lock for a short time
@@ -373,10 +373,107 @@ fn daemonize() -> Result<(), Error> {
 }
 
 // read /etc/resolv.conf and return all nameservers
-fn get_upstream_resolvers() -> AardvarkResult<Vec<ScopedIp>> {
+fn get_upstream_resolvers() -> AardvarkResult<Vec<IpAddr>> {
     let mut f = File::open("/etc/resolv.conf").wrap("open resolv.conf")?;
-    let mut buf = Vec::with_capacity(4096);
-    f.read_to_end(&mut buf).wrap("read resolv.conf")?;
-    let conf = resolv_conf::Config::parse(buf)?;
-    Ok(conf.nameservers)
+    let mut buf = String::with_capacity(4096);
+    f.read_to_string(&mut buf).wrap("read resolv.conf")?;
+
+    parse_resolv_conf(&buf)
+}
+
+fn parse_resolv_conf(content: &str) -> AardvarkResult<Vec<IpAddr>> {
+    let mut nameservers: Vec<IpAddr> = Vec::new();
+    for line in content.split('\n') {
+        // split of comments
+        let line = match line.split_once(|s| s == '#' || s == ';') {
+            Some((f, _)) => f,
+            None => line,
+        };
+        let mut line_parts = line.split_whitespace();
+        match line_parts.next() {
+            Some(first) => {
+                if first == "nameserver" {
+                    if let Some(ip) = line_parts.next() {
+                        // split of zone, we do not support the link local zone currently with ipv6 addresses
+                        let ip = match ip.split_once("%s") {
+                            Some((f, _)) => f,
+                            None => ip,
+                        };
+                        nameservers.push(ip.parse().wrap(ip)?);
+                    }
+                }
+            }
+            None => continue,
+        }
+    }
+    Ok(nameservers)
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    const IP_1_1_1_1: IpAddr = IpAddr::V4(Ipv4Addr::new(1, 1, 1, 1));
+    const IP_1_1_1_2: IpAddr = IpAddr::V4(Ipv4Addr::new(1, 1, 1, 2));
+    const IP_1_1_1_3: IpAddr = IpAddr::V4(Ipv4Addr::new(1, 1, 1, 3));
+
+    #[test]
+    fn test_parse_resolv_conf() {
+        let res = parse_resolv_conf("nameserver 1.1.1.1").expect("failed to parse");
+        assert_eq!(res, vec![IP_1_1_1_1]);
+    }
+
+    #[test]
+    fn test_parse_resolv_conf_multiple() {
+        let res = parse_resolv_conf(
+            "nameserver 1.1.1.1
+nameserver 1.1.1.2
+nameserver 1.1.1.3",
+        )
+        .expect("failed to parse");
+        assert_eq!(res, vec![IP_1_1_1_1, IP_1_1_1_2, IP_1_1_1_3]);
+    }
+
+    #[test]
+    fn test_parse_resolv_conf_search_and_options() {
+        let res = parse_resolv_conf(
+            "nameserver 1.1.1.1
+nameserver 1.1.1.2
+nameserver 1.1.1.3
+search test.podman
+options rotate",
+        )
+        .expect("failed to parse");
+        assert_eq!(res, vec![IP_1_1_1_1, IP_1_1_1_2, IP_1_1_1_3]);
+    }
+    #[test]
+    fn test_parse_resolv_conf_with_comment() {
+        let res = parse_resolv_conf(
+            "# mytest
+            nameserver 1.1.1.1 # space
+nameserver 1.1.1.2#nospace
+     #leading spaces
+nameserver 1.1.1.3",
+        )
+        .expect("failed to parse");
+        assert_eq!(res, vec![IP_1_1_1_1, IP_1_1_1_2, IP_1_1_1_3]);
+    }
+
+    #[test]
+    fn test_parse_resolv_conf_with_invalid_content() {
+        let res = parse_resolv_conf(
+            "hey I am not known
+nameserver 1.1.1.1
+nameserver 1.1.1.2 somestuff here
+abc
+nameserver 1.1.1.3",
+        )
+        .expect("failed to parse");
+        assert_eq!(res, vec![IP_1_1_1_1, IP_1_1_1_2, IP_1_1_1_3]);
+    }
+
+    #[test]
+    fn test_parse_resolv_conf_with_invalid_ip() {
+        parse_resolv_conf("nameserver abc").expect_err("invalid ip must error");
+    }
 }