Introduce DNS cache for IMAP connections

DNS cache is used as a fallback if TCP connection
to all IP addresses returned by DNS failed.
If strict TLS checks are disabled,
DNS cache results are stored, but not used.

GitHub Pull Request: <https://github.com/deltachat/deltachat-core-rust/pull/3970>

CHANGELOG.md

@@ -4,6 +4,7 @@
 
 ### Changes
 - Pipeline SMTP commands #3924
+- Cache DNS results #3970
 
 ### Fixes
 - Securejoin: Fix adding and handling Autocrypt-Gossip headers #3914

src/configure.rs

@@ -665,6 +665,7 @@ async fn nicer_configuration_error(context: &Context, errors: Vec<ConfigurationE
 
     if errors.iter().all(|e| {
         e.msg.to_lowercase().contains("could not resolve")
+            || e.msg.to_lowercase().contains("no dns resolution results")
             || e.msg
                 .to_lowercase()
                 .contains("temporary failure in name resolution")

src/imap.rs

@@ -308,6 +308,7 @@ impl Imap {
             if let Some(socks5_config) = &config.socks5_config {
                 if config.lp.security == Socket::Starttls {
                     Client::connect_starttls_socks5(
+                        context,
                         imap_server,
                         imap_port,
                         socks5_config.clone(),
@@ -315,13 +316,18 @@ impl Imap {
                     )
                     .await
                 } else {
-                    Client::connect_insecure_socks5(imap_server, imap_port, socks5_config.clone())
+                    Client::connect_insecure_socks5(
-                        .await
+                        context,
+                        imap_server,
+                        imap_port,
+                        socks5_config.clone(),
+                    )
+                    .await
                 }
             } else if config.lp.security == Socket::Starttls {
-                Client::connect_starttls(imap_server, imap_port, config.strict_tls).await
+                Client::connect_starttls(context, imap_server, imap_port, config.strict_tls).await
             } else {
-                Client::connect_insecure((imap_server, imap_port)).await
+                Client::connect_insecure(context, imap_server, imap_port).await
             }
         } else {
             let config = &self.config;
@@ -330,6 +336,7 @@ impl Imap {
 
             if let Some(socks5_config) = &config.socks5_config {
                 Client::connect_secure_socks5(
+                    context,
                     imap_server,
                     imap_port,
                     config.strict_tls,
@@ -337,7 +344,7 @@ impl Imap {
                 )
                 .await
             } else {
-                Client::connect_secure(imap_server, imap_port, config.strict_tls).await
+                Client::connect_secure(context, imap_server, imap_port, config.strict_tls).await
             }
         };
 

src/imap/client.rs

@@ -7,11 +7,11 @@ use anyhow::{Context as _, Result};
 use async_imap::Client as ImapClient;
 use async_imap::Session as ImapSession;
 use tokio::io::BufWriter;
-use tokio::net::ToSocketAddrs;
 
 use super::capabilities::Capabilities;
 use super::session::Session;
 use super::session::SessionStream;
+use crate::context::Context;
 use crate::login_param::build_tls;
 use crate::net::connect_tcp;
 use crate::socks::Socks5Config;
@@ -88,8 +88,13 @@ impl Client {
         Ok(Session::new(session, capabilities))
     }
 
-    pub async fn connect_secure(hostname: &str, port: u16, strict_tls: bool) -> Result<Self> {
+    pub async fn connect_secure(
-        let tcp_stream = connect_tcp((hostname, port), IMAP_TIMEOUT).await?;
+        context: &Context,
+        hostname: &str,
+        port: u16,
+        strict_tls: bool,
+    ) -> Result<Self> {
+        let tcp_stream = connect_tcp(context, hostname, port, IMAP_TIMEOUT, strict_tls).await?;
         let tls = build_tls(strict_tls);
         let tls_stream = tls.connect(hostname, tcp_stream).await?;
         let buffered_stream = BufWriter::new(tls_stream);
@@ -104,8 +109,8 @@ impl Client {
         Ok(Client { inner: client })
     }
 
-    pub async fn connect_insecure(addr: impl ToSocketAddrs) -> Result<Self> {
+    pub async fn connect_insecure(context: &Context, hostname: &str, port: u16) -> Result<Self> {
-        let tcp_stream = connect_tcp(addr, IMAP_TIMEOUT).await?;
+        let tcp_stream = connect_tcp(context, hostname, port, IMAP_TIMEOUT, false).await?;
         let buffered_stream = BufWriter::new(tcp_stream);
         let session_stream: Box<dyn SessionStream> = Box::new(buffered_stream);
         let mut client = ImapClient::new(session_stream);
@@ -117,8 +122,13 @@ impl Client {
         Ok(Client { inner: client })
     }
 
-    pub async fn connect_starttls(hostname: &str, port: u16, strict_tls: bool) -> Result<Self> {
+    pub async fn connect_starttls(
-        let tcp_stream = connect_tcp((hostname, port), IMAP_TIMEOUT).await?;
+        context: &Context,
+        hostname: &str,
+        port: u16,
+        strict_tls: bool,
+    ) -> Result<Self> {
+        let tcp_stream = connect_tcp(context, hostname, port, IMAP_TIMEOUT, strict_tls).await?;
 
         // Run STARTTLS command and convert the client back into a stream.
         let mut client = ImapClient::new(tcp_stream);
@@ -146,12 +156,15 @@ impl Client {
     }
 
     pub async fn connect_secure_socks5(
+        context: &Context,
         domain: &str,
         port: u16,
         strict_tls: bool,
         socks5_config: Socks5Config,
     ) -> Result<Self> {
-        let socks5_stream = socks5_config.connect(domain, port, IMAP_TIMEOUT).await?;
+        let socks5_stream = socks5_config
+            .connect(context, domain, port, IMAP_TIMEOUT, strict_tls)
+            .await?;
         let tls = build_tls(strict_tls);
         let tls_stream = tls.connect(domain, socks5_stream).await?;
         let buffered_stream = BufWriter::new(tls_stream);
@@ -166,11 +179,14 @@ impl Client {
     }
 
     pub async fn connect_insecure_socks5(
+        context: &Context,
         domain: &str,
         port: u16,
         socks5_config: Socks5Config,
     ) -> Result<Self> {
-        let socks5_stream = socks5_config.connect(domain, port, IMAP_TIMEOUT).await?;
+        let socks5_stream = socks5_config
+            .connect(context, domain, port, IMAP_TIMEOUT, false)
+            .await?;
         let buffered_stream = BufWriter::new(socks5_stream);
         let session_stream: Box<dyn SessionStream> = Box::new(buffered_stream);
         let mut client = ImapClient::new(session_stream);
@@ -183,12 +199,15 @@ impl Client {
     }
 
     pub async fn connect_starttls_socks5(
+        context: &Context,
         hostname: &str,
         port: u16,
         socks5_config: Socks5Config,
         strict_tls: bool,
     ) -> Result<Self> {
-        let socks5_stream = socks5_config.connect(hostname, port, IMAP_TIMEOUT).await?;
+        let socks5_stream = socks5_config
+            .connect(context, hostname, port, IMAP_TIMEOUT, strict_tls)
+            .await?;
 
         // Run STARTTLS command and convert the client back into a stream.
         let mut client = ImapClient::new(socks5_stream);

src/net.rs

@@ -1,25 +1,180 @@
 ///! # Common network utilities.
+use std::net::{IpAddr, SocketAddr};
 use std::pin::Pin;
+use std::str::FromStr;
 use std::time::Duration;
 
-use anyhow::{Context as _, Result};
+use anyhow::{Context as _, Error, Result};
-use tokio::net::{TcpStream, ToSocketAddrs};
+use tokio::net::{lookup_host, TcpStream};
 use tokio::time::timeout;
 use tokio_io_timeout::TimeoutStream;
 
+use crate::context::Context;
+use crate::tools::time;
+
+async fn connect_tcp_inner(addr: SocketAddr, timeout_val: Duration) -> Result<TcpStream> {
+    let tcp_stream = timeout(timeout_val, TcpStream::connect(addr))
+        .await
+        .context("connection timeout")?
+        .context("connection failure")?;
+    Ok(tcp_stream)
+}
+
+async fn lookup_host_with_timeout(
+    hostname: &str,
+    port: u16,
+    timeout_val: Duration,
+) -> Result<Vec<SocketAddr>> {
+    let res = timeout(timeout_val, lookup_host((hostname, port)))
+        .await
+        .context("DNS lookup timeout")?
+        .context("DNS lookup failure")?;
+    Ok(res.collect())
+}
+
+/// Looks up hostname and port using DNS and updates the address resolution cache.
+///
+/// If `load_cache` is true, appends cached results not older than 30 days to the end.
+async fn lookup_host_with_cache(
+    context: &Context,
+    hostname: &str,
+    port: u16,
+    timeout_val: Duration,
+    load_cache: bool,
+) -> Result<Vec<SocketAddr>> {
+    let now = time();
+    let mut resolved_addrs = match lookup_host_with_timeout(hostname, port, timeout_val).await {
+        Ok(res) => res,
+        Err(err) => {
+            warn!(
+                context,
+                "DNS resolution for {}:{} failed: {:#}.", hostname, port, err
+            );
+            Vec::new()
+        }
+    };
+
+    for addr in resolved_addrs.iter() {
+        let ip_string = addr.ip().to_string();
+        if ip_string == hostname {
+            // IP address resolved into itself, not interesting to cache.
+            continue;
+        }
+
+        info!(context, "Resolved {}:{} into {}.", hostname, port, &addr);
+
+        // Update the cache.
+        context
+            .sql
+            .execute(
+                "INSERT INTO dns_cache
+                 (hostname, address, timestamp)
+                 VALUES (?, ?, ?)
+                 ON CONFLICT (hostname, address)
+                 DO UPDATE SET timestamp=excluded.timestamp",
+                paramsv![hostname, ip_string, now],
+            )
+            .await?;
+    }
+
+    if load_cache {
+        for cached_address in context
+            .sql
+            .query_map(
+                "SELECT address
+                 FROM dns_cache
+                 WHERE hostname = ?
+                 AND ? < timestamp + 30 * 24 * 3600
+                 ORDER BY timestamp DESC",
+                paramsv![hostname, now],
+                |row| {
+                    let address: String = row.get(0)?;
+                    Ok(address)
+                },
+                |rows| {
+                    rows.collect::<std::result::Result<Vec<_>, _>>()
+                        .map_err(Into::into)
+                },
+            )
+            .await?
+        {
+            match IpAddr::from_str(&cached_address) {
+                Ok(ip_addr) => {
+                    let addr = SocketAddr::new(ip_addr, port);
+                    if !resolved_addrs.contains(&addr) {
+                        resolved_addrs.push(addr);
+                    }
+                }
+                Err(err) => {
+                    warn!(
+                        context,
+                        "Failed to parse cached address {:?}: {:#}.", cached_address, err
+                    );
+                }
+            }
+        }
+    }
+
+    Ok(resolved_addrs)
+}
+
 /// Returns a TCP connection stream with read/write timeouts set
 /// and Nagle's algorithm disabled with `TCP_NODELAY`.
 ///
 /// `TCP_NODELAY` ensures writing to the stream always results in immediate sending of the packet
 /// to the network, which is important to reduce the latency of interactive protocols such as IMAP.
+///
+/// If `load_cache` is true, may use cached DNS results.
+/// Because the cache may be poisoned with incorrect results by networks hijacking DNS requests,
+/// this option should only be used when connection is authenticated,
+/// for example using TLS.
+/// If TLS is not used or invalid TLS certificates are allowed,
+/// this option should be disabled.
 pub(crate) async fn connect_tcp(
-    addr: impl ToSocketAddrs,
+    context: &Context,
+    host: &str,
+    port: u16,
     timeout_val: Duration,
+    load_cache: bool,
 ) -> Result<Pin<Box<TimeoutStream<TcpStream>>>> {
-    let tcp_stream = timeout(timeout_val, TcpStream::connect(addr))
+    let mut tcp_stream = None;
-        .await
+    let mut last_error = None;
-        .context("connection timeout")?
+
-        .context("connection failure")?;
+    for resolved_addr in
+        lookup_host_with_cache(context, host, port, timeout_val, load_cache).await?
+    {
+        match connect_tcp_inner(resolved_addr, timeout_val).await {
+            Ok(stream) => {
+                tcp_stream = Some(stream);
+
+                // Maximize priority of this cached entry.
+                context
+                    .sql
+                    .execute(
+                        "UPDATE dns_cache
+                         SET timestamp = ?
+                         WHERE address = ?",
+                        paramsv![time(), resolved_addr.ip().to_string()],
+                    )
+                    .await?;
+                break;
+            }
+            Err(err) => {
+                warn!(
+                    context,
+                    "Failed to connect to {}: {:#}.", resolved_addr, err
+                );
+                last_error = Some(err);
+            }
+        }
+    }
+
+    let tcp_stream = match tcp_stream {
+        Some(tcp_stream) => tcp_stream,
+        None => {
+            return Err(last_error.unwrap_or_else(|| Error::msg("no DNS resolution results")));
+        }
+    };
 
     // Disable Nagle's algorithm.
     tcp_stream.set_nodelay(true)?;

src/socks.rs

@@ -56,13 +56,18 @@ impl Socks5Config {
         }
     }
 
+    /// If `load_dns_cache` is true, loads cached DNS resolution results.
+    /// Use this only if the connection is going to be protected with TLS checks.
     pub async fn connect(
         &self,
+        context: &Context,
         target_host: &str,
         target_port: u16,
         timeout_val: Duration,
+        load_dns_cache: bool,
     ) -> Result<Socks5Stream<Pin<Box<TimeoutStream<TcpStream>>>>> {
-        let tcp_stream = connect_tcp((self.host.clone(), self.port), timeout_val).await?;
+        let tcp_stream =
+            connect_tcp(context, &self.host, self.port, timeout_val, load_dns_cache).await?;
 
         let authentication_method = if let Some((username, password)) = self.user_password.as_ref()
         {

src/sql/migrations.rs

@@ -671,6 +671,18 @@ CREATE INDEX smtp_messageid ON imap(rfc724_mid);
         )
         .await?;
     }
+    if dbversion < 97 {
+        sql.execute_migration(
+            "CREATE TABLE dns_cache (
+               hostname TEXT NOT NULL,
+               address TEXT NOT NULL, -- IPv4 or IPv6 address
+               timestamp INTEGER NOT NULL,
+               UNIQUE (hostname, address)
+             )",
+            97,
+        )
+        .await?;
+    }
 
     let new_version = sql
         .get_raw_config_int(VERSION_CFG)