Merge branch 'tls-name' into 'master'

Validate TLS certificates according to Matrix spec

See merge request famedly/conduit!60

CROSS_COMPILE.md

@@ -6,6 +6,6 @@ $ sudo usermod -aG docker $USER
 $ exec sudo su -l $USER
 $ sudo systemctl start docker
 $ cargo install cross
-$ cross build --release --features tls_vendored --target armv7-unknown-linux-musleabihf
+$ cross build --release --target armv7-unknown-linux-musleabihf
 ```
 The cross-compiled binary is at target/armv7-unknown-linux-musleabihf/release/conduit

Cargo.lock

@@ -198,6 +198,8 @@ dependencies = [
  "rocket",
  "ruma",
  "rust-argon2",
+ "rustls",
+ "rustls-native-certs",
  "serde",
  "serde_json",
  "serde_yaml",
@@ -209,6 +211,7 @@ dependencies = [
  "tracing-opentelemetry",
  "tracing-subscriber",
  "trust-dns-resolver",
+ "webpki",
 ]
 
 [[package]]
@@ -420,21 +423,6 @@ version = "1.0.7"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "3f9eec918d3f24069decb9af1554cad7c880e2da24a9afd88aca000531ab82c1"
 
-[[package]]
-name = "foreign-types"
-version = "0.3.2"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "f6f339eb8adc052cd2ca78910fda869aefa38d22d5cb648e6485e4d3fc06f3b1"
-dependencies = [
- "foreign-types-shared",
-]
-
-[[package]]
-name = "foreign-types-shared"
-version = "0.1.1"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "00b0228411908ca8685dba7fc2cdd70ec9990a6e753e89b6ac91a84c40fbaf4b"
-
 [[package]]
 name = "form_urlencoded"
 version = "1.0.1"
@@ -716,16 +704,18 @@ dependencies = [
 ]
 
 [[package]]
-name = "hyper-tls"
+name = "hyper-rustls"
-version = "0.5.0"
+version = "0.22.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "d6183ddfa99b85da61a140bea0efc93fdf56ceaa041b37d553518030827f9905"
+checksum = "5f9f7a97316d44c0af9b0301e65010573a853a9fc97046d7331d7f6bc0fd5a64"
 dependencies = [
- "bytes",
+ "futures-util",
  "hyper",
- "native-tls",
+ "log",
+ "rustls",
  "tokio",
- "tokio-native-tls",
+ "tokio-rustls",
+ "webpki",
 ]
 
 [[package]]
@@ -1001,24 +991,6 @@ dependencies = [
  "winapi",
 ]
 
-[[package]]
-name = "native-tls"
-version = "0.2.7"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "b8d96b2e1c8da3957d58100b09f102c6d9cfdfced01b7ec5a8974044bb09dbd4"
-dependencies = [
- "lazy_static",
- "libc",
- "log",
- "openssl",
- "openssl-probe",
- "openssl-sys",
- "schannel",
- "security-framework",
- "security-framework-sys",
- "tempfile",
-]
-
 [[package]]
 name = "ntapi"
 version = "0.3.6"
@@ -1096,49 +1068,12 @@ version = "1.7.2"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "af8b08b04175473088b46763e51ee54da5f9a164bc162f615b91bc179dbf15a3"
 
-[[package]]
-name = "openssl"
-version = "0.10.33"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "a61075b62a23fef5a29815de7536d940aa35ce96d18ce0cc5076272db678a577"
-dependencies = [
- "bitflags",
- "cfg-if",
- "foreign-types",
- "libc",
- "once_cell",
- "openssl-sys",
-]
-
 [[package]]
 name = "openssl-probe"
 version = "0.1.2"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "77af24da69f9d9341038eba93a073b1fdaaa1b788221b00a69bce9e762cb32de"
 
-[[package]]
-name = "openssl-src"
-version = "111.15.0+1.1.1k"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "b1a5f6ae2ac04393b217ea9f700cd04fa9bf3d93fae2872069f3d15d908af70a"
-dependencies = [
- "cc",
-]
-
-[[package]]
-name = "openssl-sys"
-version = "0.9.61"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "313752393519e876837e09e1fa183ddef0be7735868dced3196f4472d536277f"
-dependencies = [
- "autocfg",
- "cc",
- "libc",
- "openssl-src",
- "pkg-config",
- "vcpkg",
-]
-
 [[package]]
 name = "opentelemetry"
 version = "0.12.0"
@@ -1280,12 +1215,6 @@ version = "0.1.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "8b870d8c151b6f2fb93e84a13146138f05d02ed11c7e7c54f8826aaaf7c9f184"
 
-[[package]]
-name = "pkg-config"
-version = "0.3.19"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "3831453b3449ceb48b6d9c7ad7c96d5ea673e9b470a1dc578c2ce6521230884c"
-
 [[package]]
 name = "png"
 version = "0.16.8"
@@ -1486,15 +1415,6 @@ version = "0.6.23"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "24d5f089152e60f62d28b835fbff2cd2e8dc0baf1ac13343bef92ab7eed84548"
 
-[[package]]
-name = "remove_dir_all"
-version = "0.5.3"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "3acd125665422973a33ac9d3dd2df85edad0f4ae9b00dafb1a05e43a9f5ef8e7"
-dependencies = [
- "winapi",
-]
-
 [[package]]
 name = "reqwest"
 version = "0.11.3"
@@ -1509,19 +1429,20 @@ dependencies = [
  "http",
  "http-body",
  "hyper",
- "hyper-tls",
+ "hyper-rustls",
  "ipnet",
  "js-sys",
  "lazy_static",
  "log",
  "mime",
- "native-tls",
  "percent-encoding",
  "pin-project-lite",
+ "rustls",
+ "rustls-native-certs",
  "serde",
  "serde_urlencoded",
  "tokio",
- "tokio-native-tls",
+ "tokio-rustls",
  "url",
  "wasm-bindgen",
  "wasm-bindgen-futures",
@@ -1886,6 +1807,18 @@ dependencies = [
  "webpki",
 ]
 
+[[package]]
+name = "rustls-native-certs"
+version = "0.5.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "5a07b7c1885bd8ed3831c289b7870b13ef46fe0e856d288c30d9cc17d75a2092"
+dependencies = [
+ "openssl-probe",
+ "rustls",
+ "schannel",
+ "security-framework",
+]
+
 [[package]]
 name = "ryu"
 version = "1.0.5"
@@ -2191,20 +2124,6 @@ dependencies = [
  "unicode-xid",
 ]
 
-[[package]]
-name = "tempfile"
-version = "3.2.0"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "dac1c663cfc93810f88aed9b8941d48cabf856a1b111c29a40439018d870eb22"
-dependencies = [
- "cfg-if",
- "libc",
- "rand",
- "redox_syscall 0.2.5",
- "remove_dir_all",
- "winapi",
-]
-
 [[package]]
 name = "termcolor"
 version = "1.1.2"
@@ -2358,16 +2277,6 @@ dependencies = [
  "syn",
 ]
 
-[[package]]
-name = "tokio-native-tls"
-version = "0.3.0"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "f7d995660bd2b7f8c1568414c1126076c13fbb725c40112dc0120b78eb9b717b"
-dependencies = [
- "native-tls",
- "tokio",
-]
-
 [[package]]
 name = "tokio-rustls"
 version = "0.22.0"
@@ -2613,12 +2522,6 @@ dependencies = [
  "percent-encoding",
 ]
 
-[[package]]
-name = "vcpkg"
-version = "0.2.11"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "b00bca6106a5e23f3eee943593759b7fcddb00554332e856d990c893966879fb"
-
 [[package]]
 name = "version_check"
 version = "0.9.3"

Cargo.toml

@@ -49,7 +49,11 @@ rand = "0.8.3"
 # Used to hash passwords
 rust-argon2 = "0.8.3"
 # Used to send requests
-reqwest = { version = "0.11.1" }
+reqwest = { version = "0.11.3", default-features = false, features = ["rustls-tls-native-roots"] }
+# Custom TLS verifier
+rustls = { version = "0.19", features = ["dangerous_configuration"] }
+rustls-native-certs = "0.5.0"
+webpki = "0.21.0"
 # Used for conduit::Error type
 thiserror = "1.0.24"
 # Used to generate thumbnails for images
@@ -75,7 +79,6 @@ pretty_env_logger = "0.4.0"
 [features]
 default = ["conduit_bin"]
 conduit_bin = [] # TODO: add rocket to this when it is optional
-tls_vendored = ["reqwest/native-tls-vendored"]
 
 [[bin]]
 name = "conduit"

src/database/globals.rs

@@ -1,9 +1,10 @@
 use crate::{database::Config, utils, Error, Result};
-use log::error;
+use log::{error, info};
 use ruma::{
     api::federation::discovery::{ServerSigningKeys, VerifyKey},
     ServerName, ServerSigningKeyId,
 };
+use rustls::{ServerCertVerifier, WebPKIVerifier};
 use std::{
     collections::{BTreeMap, HashMap},
     sync::{Arc, RwLock},
@@ -14,9 +15,11 @@ use trust_dns_resolver::TokioAsyncResolver;
 pub const COUNTER: &str = "c";
 
 type WellKnownMap = HashMap<Box<ServerName>, (String, String)>;
+type TlsNameMap = HashMap<String, webpki::DNSName>;
 #[derive(Clone)]
 pub struct Globals {
     pub actual_destination_cache: Arc<RwLock<WellKnownMap>>, // actual_destination, host
+    pub tls_name_override: Arc<RwLock<TlsNameMap>>,
     pub(super) globals: sled::Tree,
     config: Config,
     keypair: Arc<ruma::signatures::Ed25519KeyPair>,
@@ -26,6 +29,36 @@ pub struct Globals {
     pub(super) servertimeout_signingkey: sled::Tree, // ServerName + Timeout Timestamp -> algorithm:key + pubkey
 }
 
+struct MatrixServerVerifier {
+    inner: WebPKIVerifier,
+    tls_name_override: Arc<RwLock<TlsNameMap>>,
+}
+
+impl ServerCertVerifier for MatrixServerVerifier {
+    fn verify_server_cert(
+        &self,
+        roots: &rustls::RootCertStore,
+        presented_certs: &[rustls::Certificate],
+        dns_name: webpki::DNSNameRef<'_>,
+        ocsp_response: &[u8],
+    ) -> std::result::Result<rustls::ServerCertVerified, rustls::TLSError> {
+        if let Some(override_name) = self.tls_name_override.read().unwrap().get(dns_name.into()) {
+            let result = self.inner.verify_server_cert(
+                roots,
+                presented_certs,
+                override_name.as_ref(),
+                ocsp_response,
+            );
+            if result.is_ok() {
+                return result;
+            }
+            info!("Server {:?} is non-compliant, retrying TLS verification with original name", dns_name);
+        }
+        self.inner
+            .verify_server_cert(roots, presented_certs, dns_name, ocsp_response)
+    }
+}
+
 impl Globals {
     pub fn load(
         globals: sled::Tree,
@@ -66,10 +99,21 @@ impl Globals {
             }
         };
 
+        let tls_name_override = Arc::new(RwLock::new(TlsNameMap::new()));
+        let verifier = Arc::new(MatrixServerVerifier {
+            inner: WebPKIVerifier::new(),
+            tls_name_override: tls_name_override.clone(),
+        });
+        let mut tlsconfig = rustls::ClientConfig::new();
+        tlsconfig.dangerous().set_certificate_verifier(verifier);
+        tlsconfig.root_store =
+            rustls_native_certs::load_native_certs().expect("Error loading system certificates");
+
         let reqwest_client = reqwest::Client::builder()
             .connect_timeout(Duration::from_secs(30))
             .timeout(Duration::from_secs(60 * 3))
             .pool_max_idle_per_host(1)
+            .use_preconfigured_tls(tlsconfig)
             .build()
             .unwrap();
 
@@ -86,7 +130,8 @@ impl Globals {
             dns_resolver: TokioAsyncResolver::tokio_from_system_conf().map_err(|_| {
                 Error::bad_config("Failed to set up trust dns resolver with system config.")
             })?,
-            actual_destination_cache: Arc::new(RwLock::new(HashMap::new())),
+            actual_destination_cache: Arc::new(RwLock::new(WellKnownMap::new())),
+            tls_name_override,
             servertimeout_signingkey,
             jwt_decoding_key,
         })

src/server_server.rs

@@ -53,6 +53,49 @@ use std::{
 #[cfg(feature = "conduit_bin")]
 use rocket::{get, post, put};
 
+/// Wraps either an literal IP address plus port, or a hostname plus complement
+/// (colon-plus-port if it was specified).
+///
+/// Note: A `FedDest::Named` might contain an IP address in string form if there
+/// was no port specified to construct a SocketAddr with.
+///
+/// # Examples:
+/// ```rust,ignore
+/// FedDest::Literal("198.51.100.3:8448".parse()?);
+/// FedDest::Literal("[2001:db8::4:5]:443".parse()?);
+/// FedDest::Named("matrix.example.org".to_owned(), "".to_owned());
+/// FedDest::Named("matrix.example.org".to_owned(), ":8448".to_owned());
+/// FedDest::Named("198.51.100.5".to_owned(), "".to_owned());
+/// ```
+#[derive(Clone, Debug, PartialEq)]
+enum FedDest {
+    Literal(SocketAddr),
+    Named(String, String),
+}
+
+impl FedDest {
+    fn into_https_string(self) -> String {
+        match self {
+            Self::Literal(addr) => format!("https://{}", addr),
+            Self::Named(host, port) => format!("https://{}{}", host, port),
+        }
+    }
+
+    fn into_uri_string(self) -> String {
+        match self {
+            Self::Literal(addr) => addr.to_string(),
+            Self::Named(host, ref port) => host + port,
+        }
+    }
+
+    fn hostname(&self) -> String {
+        match &self {
+            Self::Literal(addr) => addr.ip().to_string(),
+            Self::Named(host, _) => host.clone(),
+        }
+    }
+}
+
 #[tracing::instrument(skip(globals))]
 pub async fn send_request<T: OutgoingRequest>(
     globals: &crate::database::globals::Globals,
@@ -77,12 +120,24 @@ where
         result
     } else {
         let result = find_actual_destination(globals, &destination).await;
+        let (actual_destination, host) = result.clone();
+        let result_string = (result.0.into_https_string(), result.1.into_uri_string());
         globals
             .actual_destination_cache
             .write()
             .unwrap()
-            .insert(Box::<ServerName>::from(destination), result.clone());
+            .insert(Box::<ServerName>::from(destination), result_string.clone());
-        result
+        let dest_hostname = actual_destination.hostname();
+        let host_hostname = host.hostname();
+        if dest_hostname != host_hostname {
+            globals.tls_name_override.write().unwrap().insert(
+                dest_hostname,
+                webpki::DNSNameRef::try_from_ascii_str(&host_hostname)
+                    .unwrap()
+                    .to_owned(),
+            );
+        }
+        result_string
     };
 
     let mut http_request = request
@@ -210,22 +265,23 @@ where
 }
 
 #[tracing::instrument]
-fn get_ip_with_port(destination_str: String) -> Option<String> {
+fn get_ip_with_port(destination_str: &str) -> Option<FedDest> {
-    if destination_str.parse::<SocketAddr>().is_ok() {
+    if let Ok(destination) = destination_str.parse::<SocketAddr>() {
-        Some(destination_str)
+        Some(FedDest::Literal(destination))
     } else if let Ok(ip_addr) = destination_str.parse::<IpAddr>() {
-        Some(SocketAddr::new(ip_addr, 8448).to_string())
+        Some(FedDest::Literal(SocketAddr::new(ip_addr, 8448)))
     } else {
         None
     }
 }
 
 #[tracing::instrument]
-fn add_port_to_hostname(destination_str: String) -> String {
+fn add_port_to_hostname(destination_str: &str) -> FedDest {
-    match destination_str.find(':') {
+    let (host, port) = match destination_str.find(':') {
-        None => destination_str.to_owned() + ":8448",
+        None => (destination_str, ":8448"),
-        Some(_) => destination_str.to_string(),
+        Some(pos) => destination_str.split_at(pos),
-    }
+    };
+    FedDest::Named(host.to_string(), port.to_string())
 }
 
 /// Returns: actual_destination, host header
@@ -235,36 +291,37 @@ fn add_port_to_hostname(destination_str: String) -> String {
 async fn find_actual_destination(
     globals: &crate::database::globals::Globals,
     destination: &'_ ServerName,
-) -> (String, String) {
+) -> (FedDest, FedDest) {
     let destination_str = destination.as_str().to_owned();
-    let mut host = destination_str.clone();
+    let mut hostname = destination_str.clone();
-    let actual_destination = "https://".to_owned()
+    let actual_destination = match get_ip_with_port(&destination_str) {
-        + &match get_ip_with_port(destination_str.clone()) {
         Some(host_port) => {
             // 1: IP literal with provided or default port
             host_port
         }
         None => {
-                if destination_str.find(':').is_some() {
+            if let Some(pos) = destination_str.find(':') {
                 // 2: Hostname with included port
-                    destination_str
+                let (host, port) = destination_str.split_at(pos);
+                FedDest::Named(host.to_string(), port.to_string())
             } else {
                 match request_well_known(globals, &destination.as_str()).await {
                     // 3: A .well-known file is available
                     Some(delegated_hostname) => {
-                            host = delegated_hostname.clone();
+                        hostname = delegated_hostname.clone();
-                            match get_ip_with_port(delegated_hostname.clone()) {
+                        match get_ip_with_port(&delegated_hostname) {
                             Some(host_and_port) => host_and_port, // 3.1: IP literal in .well-known file
                             None => {
-                                    if destination_str.find(':').is_some() {
+                                if let Some(pos) = destination_str.find(':') {
                                     // 3.2: Hostname with port in .well-known file
-                                        destination_str
+                                    let (host, port) = destination_str.split_at(pos);
+                                    FedDest::Named(host.to_string(), port.to_string())
                                 } else {
                                     match query_srv_record(globals, &delegated_hostname).await {
                                         // 3.3: SRV lookup successful
                                         Some(hostname) => hostname,
                                         // 3.4: No SRV records, just use the hostname from .well-known
-                                            None => add_port_to_hostname(delegated_hostname),
+                                        None => add_port_to_hostname(&delegated_hostname),
                                     }
                                 }
                             }
@@ -276,7 +333,7 @@ async fn find_actual_destination(
                             // 4: SRV record found
                             Some(hostname) => hostname,
                             // 5: No SRV record found
-                                None => add_port_to_hostname(destination_str.to_string()),
+                            None => add_port_to_hostname(&destination_str),
                         }
                     }
                 }
@@ -284,24 +341,39 @@ async fn find_actual_destination(
         }
     };
 
-    (actual_destination, host)
+    // Can't use get_ip_with_port here because we don't want to add a port
+    // to an IP address if it wasn't specified
+    let hostname = if let Ok(addr) = hostname.parse::<SocketAddr>() {
+        FedDest::Literal(addr)
+    } else if let Ok(addr) = hostname.parse::<IpAddr>() {
+        FedDest::Named(addr.to_string(), "".to_string())
+    } else if let Some(pos) = hostname.find(':') {
+        let (host, port) = hostname.split_at(pos);
+        FedDest::Named(host.to_string(), port.to_string())
+    } else {
+        FedDest::Named(hostname, "".to_string())
+    };
+    (actual_destination, hostname)
 }
 
 #[tracing::instrument(skip(globals))]
 async fn query_srv_record(
     globals: &crate::database::globals::Globals,
     hostname: &'_ str,
-) -> Option<String> {
+) -> Option<FedDest> {
     if let Ok(Some(host_port)) = globals
         .dns_resolver()
         .srv_lookup(format!("_matrix._tcp.{}", hostname))
         .await
         .map(|srv| {
             srv.iter().next().map(|result| {
-                format!(
+                FedDest::Named(
-                    "{}:{}",
+                    result
-                    result.target().to_string().trim_end_matches('.'),
+                        .target()
-                    result.port().to_string()
+                        .to_string()
+                        .trim_end_matches('.')
+                        .to_string(),
+                    format!(":{}", result.port()),
                 )
             })
         })
@@ -2133,45 +2205,45 @@ pub async fn fetch_required_signing_keys(
 
 #[cfg(test)]
 mod tests {
-    use super::{add_port_to_hostname, get_ip_with_port};
+    use super::{FedDest, add_port_to_hostname, get_ip_with_port};
 
     #[test]
     fn ips_get_default_ports() {
         assert_eq!(
-            get_ip_with_port(String::from("1.1.1.1")),
+            get_ip_with_port("1.1.1.1"),
-            Some(String::from("1.1.1.1:8448"))
+            Some(FedDest::Literal("1.1.1.1:8448".parse().unwrap()))
         );
         assert_eq!(
-            get_ip_with_port(String::from("dead:beef::")),
+            get_ip_with_port("dead:beef::"),
-            Some(String::from("[dead:beef::]:8448"))
+            Some(FedDest::Literal("[dead:beef::]:8448".parse().unwrap()))
         );
     }
 
     #[test]
     fn ips_keep_custom_ports() {
         assert_eq!(
-            get_ip_with_port(String::from("1.1.1.1:1234")),
+            get_ip_with_port("1.1.1.1:1234"),
-            Some(String::from("1.1.1.1:1234"))
+            Some(FedDest::Literal("1.1.1.1:1234".parse().unwrap()))
         );
         assert_eq!(
-            get_ip_with_port(String::from("[dead::beef]:8933")),
+            get_ip_with_port("[dead::beef]:8933"),
-            Some(String::from("[dead::beef]:8933"))
+            Some(FedDest::Literal("[dead::beef]:8933".parse().unwrap()))
         );
     }
 
     #[test]
     fn hostnames_get_default_ports() {
         assert_eq!(
-            add_port_to_hostname(String::from("example.com")),
+            add_port_to_hostname("example.com"),
-            "example.com:8448"
+            FedDest::Named(String::from("example.com"), String::from(":8448"))
         )
     }
 
     #[test]
     fn hostnames_keep_custom_ports() {
         assert_eq!(
-            add_port_to_hostname(String::from("example.com:1337")),
+            add_port_to_hostname("example.com:1337"),
-            "example.com:1337"
+            FedDest::Named(String::from("example.com"), String::from(":1337"))
         )
     }
 }