Merge pull request #647 from deltachat/tls_checks_config

Add certificate check configuration options

python/tests/conftest.py

@@ -157,6 +157,11 @@ def acfactory(pytestconfig, tmpdir, request, session_liveconfig):
             self.live_count += 1
             if "e2ee_enabled" not in configdict:
                 configdict["e2ee_enabled"] = "1"
+
+            # Enable strict certificate checks for online accounts
+            configdict["imap_certificate_checks"] = "1"
+            configdict["smtp_certificate_checks"] = "1"
+
             tmpdb = tmpdir.join("livedb%d" % self.live_count)
             ac = self.make_account(tmpdb.strpath, logid="ac{}".format(self.live_count))
             ac._evlogger.init_time = self.init_time

src/config.rs

@@ -19,10 +19,12 @@ pub enum Config {
     MailUser,
     MailPw,
     MailPort,
+    ImapCertificateChecks,
     SendServer,
     SendUser,
     SendPw,
     SendPort,
+    SmtpCertificateChecks,
     ServerFlags,
     #[strum(props(default = "INBOX"))]
     ImapFolder,
@@ -52,10 +54,12 @@ pub enum Config {
     ConfiguredMailPw,
     ConfiguredMailPort,
     ConfiguredMailSecurity,
+    ConfiguredImapCertificateChecks,
     ConfiguredSendServer,
     ConfiguredSendUser,
     ConfiguredSendPw,
     ConfiguredSendPort,
+    ConfiguredSmtpCertificateChecks,
     ConfiguredServerFlags,
     ConfiguredSendSecurity,
     ConfiguredE2EEEnabled,

src/imap.rs

@@ -11,7 +11,7 @@ use crate::dc_receive_imf::dc_receive_imf;
 use crate::error::Error;
 use crate::events::Event;
 use crate::job::{connect_to_inbox, job_add, Action};
-use crate::login_param::LoginParam;
+use crate::login_param::{dc_build_tls, CertificateChecks, LoginParam};
 use crate::message::{self, update_msg_move_state, update_server_uid};
 use crate::oauth2::dc_get_oauth2_access_token;
 use crate::param::Params;
@@ -108,14 +108,10 @@ impl Client {
     pub fn connect_secure<A: net::ToSocketAddrs, S: AsRef<str>>(
         addr: A,
         domain: S,
+        certificate_checks: CertificateChecks,
     ) -> imap::error::Result<Self> {
         let stream = net::TcpStream::connect(addr)?;
-        let tls = native_tls::TlsConnector::builder()
+        let tls = dc_build_tls(certificate_checks).unwrap();
-            // see also: https://github.com/deltachat/deltachat-core-rust/issues/203
-            .danger_accept_invalid_certs(true)
-            .danger_accept_invalid_hostnames(true)
-            .build()
-            .unwrap();
 
         let s = stream.try_clone().expect("cloning the stream failed");
         let tls_stream = native_tls::TlsConnector::connect(&tls, domain.as_ref(), s)?;
@@ -135,13 +131,14 @@ impl Client {
         Ok(Client::Insecure(client, stream))
     }
 
-    pub fn secure<S: AsRef<str>>(self, domain: S) -> imap::error::Result<Client> {
+    pub fn secure<S: AsRef<str>>(
+        self,
+        domain: S,
+        certificate_checks: CertificateChecks,
+    ) -> imap::error::Result<Client> {
         match self {
             Client::Insecure(client, stream) => {
-                let tls = native_tls::TlsConnector::builder()
+                let tls = dc_build_tls(certificate_checks).unwrap();
-                    .danger_accept_invalid_hostnames(true)
-                    .build()
-                    .unwrap();
 
                 let client_sec = client.secure(domain, &tls)?;
 
@@ -321,6 +318,7 @@ struct ImapConfig {
     pub imap_port: u16,
     pub imap_user: String,
     pub imap_pw: String,
+    pub certificate_checks: CertificateChecks,
     pub server_flags: usize,
     pub selected_folder: Option<String>,
     pub selected_mailbox: Option<imap::types::Mailbox>,
@@ -339,6 +337,7 @@ impl Default for ImapConfig {
             imap_port: 0,
             imap_user: "".into(),
             imap_pw: "".into(),
+            certificate_checks: Default::default(),
             server_flags: 0,
             selected_folder: None,
             selected_mailbox: None,
@@ -397,7 +396,7 @@ impl Imap {
 
                 Client::connect_insecure((imap_server, imap_port)).and_then(|client| {
                     if (server_flags & DC_LP_IMAP_SOCKET_STARTTLS) != 0 {
-                        client.secure(imap_server)
+                        client.secure(imap_server, config.certificate_checks)
                     } else {
                         Ok(client)
                     }
@@ -407,7 +406,11 @@ impl Imap {
                 let imap_server: &str = config.imap_server.as_ref();
                 let imap_port = config.imap_port;
 
-                Client::connect_secure((imap_server, imap_port), imap_server)
+                Client::connect_secure(
+                    (imap_server, imap_port),
+                    imap_server,
+                    config.certificate_checks,
+                )
             };
 
         let login_res = match connection_res {
@@ -534,6 +537,7 @@ impl Imap {
             config.imap_port = imap_port;
             config.imap_user = imap_user.to_string();
             config.imap_pw = imap_pw.to_string();
+            config.certificate_checks = lp.imap_certificate_checks;
             config.server_flags = server_flags;
         }
 

src/login_param.rs

@@ -4,6 +4,22 @@ use std::fmt;
 use crate::context::Context;
 use crate::error::Error;
 
+#[derive(Copy, Clone, Debug, Display, FromPrimitive)]
+#[repr(i32)]
+#[strum(serialize_all = "snake_case")]
+pub enum CertificateChecks {
+    Automatic = 0,
+    Strict = 1,
+    AcceptInvalidHostnames = 2,
+    AcceptInvalidCertificates = 3,
+}
+
+impl Default for CertificateChecks {
+    fn default() -> Self {
+        Self::Automatic
+    }
+}
+
 #[derive(Default, Debug)]
 pub struct LoginParam {
     pub addr: String,
@@ -11,10 +27,14 @@ pub struct LoginParam {
     pub mail_user: String,
     pub mail_pw: String,
     pub mail_port: i32,
+    /// IMAP TLS options: whether to allow invalid certificates and/or invalid hostnames
+    pub imap_certificate_checks: CertificateChecks,
     pub send_server: String,
     pub send_user: String,
     pub send_pw: String,
     pub send_port: i32,
+    /// SMTP TLS options: whether to allow invalid certificates and/or invalid hostnames
+    pub smtp_certificate_checks: CertificateChecks,
     pub server_flags: i32,
 }
 
@@ -48,6 +68,14 @@ impl LoginParam {
         let key = format!("{}mail_pw", prefix);
         let mail_pw = sql.get_config(context, key).unwrap_or_default();
 
+        let key = format!("{}imap_certificate_checks", prefix);
+        let imap_certificate_checks =
+            if let Some(certificate_checks) = sql.get_config_int(context, key) {
+                num_traits::FromPrimitive::from_i32(certificate_checks).unwrap_or_default()
+            } else {
+                Default::default()
+            };
+
         let key = format!("{}send_server", prefix);
         let send_server = sql.get_config(context, key).unwrap_or_default();
 
@@ -60,6 +88,14 @@ impl LoginParam {
         let key = format!("{}send_pw", prefix);
         let send_pw = sql.get_config(context, key).unwrap_or_default();
 
+        let key = format!("{}smtp_certificate_checks", prefix);
+        let smtp_certificate_checks =
+            if let Some(certificate_checks) = sql.get_config_int(context, key) {
+                num_traits::FromPrimitive::from_i32(certificate_checks).unwrap_or_default()
+            } else {
+                Default::default()
+            };
+
         let key = format!("{}server_flags", prefix);
         let server_flags = sql.get_config_int(context, key).unwrap_or_default();
 
@@ -69,10 +105,12 @@ impl LoginParam {
             mail_user,
             mail_pw,
             mail_port,
+            imap_certificate_checks,
             send_server,
             send_user,
             send_pw,
             send_port,
+            smtp_certificate_checks,
             server_flags,
         }
     }
@@ -105,6 +143,9 @@ impl LoginParam {
         let key = format!("{}mail_pw", prefix);
         sql.set_config(context, key, Some(&self.mail_pw))?;
 
+        let key = format!("{}imap_certificate_checks", prefix);
+        sql.set_config_int(context, key, self.imap_certificate_checks as i32)?;
+
         let key = format!("{}send_server", prefix);
         sql.set_config(context, key, Some(&self.send_server))?;
 
@@ -117,6 +158,9 @@ impl LoginParam {
         let key = format!("{}send_pw", prefix);
         sql.set_config(context, key, Some(&self.send_pw))?;
 
+        let key = format!("{}smtp_certificate_checks", prefix);
+        sql.set_config_int(context, key, self.smtp_certificate_checks as i32)?;
+
         let key = format!("{}server_flags", prefix);
         sql.set_config_int(context, key, self.server_flags)?;
 
@@ -133,16 +177,18 @@ impl fmt::Display for LoginParam {
 
         write!(
             f,
-            "{} {}:{}:{}:{} {}:{}:{}:{} {}",
+            "{} imap:{}:{}:{}:{}:cert_{} smtp:{}:{}:{}:{}:cert_{} {}",
             unset_empty(&self.addr),
             unset_empty(&self.mail_user),
             if !self.mail_pw.is_empty() { pw } else { unset },
             unset_empty(&self.mail_server),
             self.mail_port,
+            self.imap_certificate_checks,
             unset_empty(&self.send_user),
             if !self.send_pw.is_empty() { pw } else { unset },
             unset_empty(&self.send_server),
             self.send_port,
+            self.smtp_certificate_checks,
             flags_readable,
         )
     }
@@ -204,3 +250,41 @@ fn get_readable_flags(flags: i32) -> String {
 
     res
 }
+
+pub fn dc_build_tls(
+    certificate_checks: CertificateChecks,
+) -> Result<native_tls::TlsConnector, native_tls::Error> {
+    let mut tls_builder = native_tls::TlsConnector::builder();
+    match certificate_checks {
+        CertificateChecks::Automatic => {
+            // Same as AcceptInvalidCertificates for now.
+            // TODO: use provider database when it becomes available
+            tls_builder
+                .danger_accept_invalid_hostnames(true)
+                .danger_accept_invalid_certs(true)
+        }
+        CertificateChecks::Strict => &mut tls_builder,
+        CertificateChecks::AcceptInvalidHostnames => {
+            tls_builder.danger_accept_invalid_hostnames(true)
+        }
+        CertificateChecks::AcceptInvalidCertificates => tls_builder
+            .danger_accept_invalid_hostnames(true)
+            .danger_accept_invalid_certs(true),
+    }
+    .build()
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_certificate_checks_display() {
+        use std::string::ToString;
+
+        assert_eq!(
+            "accept_invalid_hostnames".to_string(),
+            CertificateChecks::AcceptInvalidHostnames.to_string()
+        );
+    }
+}

src/smtp.rs

@@ -5,7 +5,7 @@ use crate::constants::*;
 use crate::context::Context;
 use crate::error::Error;
 use crate::events::Event;
-use crate::login_param::LoginParam;
+use crate::login_param::{dc_build_tls, LoginParam};
 use crate::oauth2::*;
 
 #[derive(DebugStub)]
@@ -68,14 +68,7 @@ impl Smtp {
         let domain = &lp.send_server;
         let port = lp.send_port as u16;
 
-        let tls = native_tls::TlsConnector::builder()
+        let tls = dc_build_tls(lp.smtp_certificate_checks).unwrap();
-            // see also: https://github.com/deltachat/deltachat-core-rust/issues/203
-            .danger_accept_invalid_hostnames(true)
-            .danger_accept_invalid_certs(true)
-            .min_protocol_version(Some(DEFAULT_TLS_PROTOCOLS[0]))
-            .build()
-            .unwrap();
-
         let tls_parameters = ClientTlsParameters::new(domain.to_string(), tls);
 
         let creds = if 0 != lp.server_flags & (DC_LP_AUTH_OAUTH2 as i32) {