diff --git a/src/action/client_options.rs b/src/action/client_options.rs index 1de6f4946..f258b5de5 100644 --- a/src/action/client_options.rs +++ b/src/action/client_options.rs @@ -51,10 +51,10 @@ impl ClientOptions { /// * `socketTimeoutMS`: unsupported, does not map to any field /// * `ssl`: an alias of the `tls` option /// * `tls`: maps to the TLS variant of the `tls` field`. - /// * `tlsInsecure`: relaxes the TLS constraints on connections being made; currently is just - /// an alias of `tlsAllowInvalidCertificates`, but more behavior may be added to this option - /// in the future - /// * `tlsAllowInvalidCertificates`: maps to the `allow_invalidCertificates` field of the + /// * `tlsInsecure`: relaxes the TLS constraints on connections being made; if set, the + /// `allow_invalid_certificates` and `allow_invalid_hostnames` fields of the `tls` field are + /// set to its value + /// * `tlsAllowInvalidCertificates`: maps to the `allow_invalid_certificates` field of the /// `tls` field /// * `tlsCAFile`: maps to the `ca_file_path` field of the `tls` field /// * `tlsCertificateKeyFile`: maps to the `cert_key_file_path` field of the `tls` field diff --git a/src/action/insert_one.rs b/src/action/insert_one.rs index 057312f0f..33d64e649 100644 --- a/src/action/insert_one.rs +++ b/src/action/insert_one.rs @@ -59,7 +59,7 @@ impl crate::sync::Collection { } } -/// Inserts a document into a collection. Construct with ['Collection::insert_one`]. +/// Inserts a document into a collection. Construct with [`Collection::insert_one`]. #[must_use] pub struct InsertOne<'a> { coll: CollRef<'a>, diff --git a/src/client/options.rs b/src/client/options.rs index 407875ca4..daeeb5a9c 100644 --- a/src/client/options.rs +++ b/src/client/options.rs @@ -55,6 +55,10 @@ pub(crate) use resolver_config::ResolverConfig; pub(crate) const DEFAULT_PORT: u16 = 27017; +const TLS_INSECURE: &str = "tlsinsecure"; +const TLS_ALLOW_INVALID_CERTIFICATES: &str = "tlsallowinvalidcertificates"; +#[cfg(feature = "openssl-tls")] +const TLS_ALLOW_INVALID_HOSTNAMES: &str = "tlsallowinvalidhostnames"; const URI_OPTIONS: &[&str] = &[ "appname", "authmechanism", @@ -82,8 +86,8 @@ const URI_OPTIONS: &[&str] = &[ "sockettimeoutms", "tls", "ssl", - "tlsinsecure", - "tlsallowinvalidcertificates", + TLS_INSECURE, + TLS_ALLOW_INVALID_CERTIFICATES, "tlscafile", "tlscertificatekeyfile", "uuidRepresentation", @@ -1646,7 +1650,9 @@ impl ConnectionString { } /// Relax TLS constraints as much as possible (e.g. allowing invalid certificates or hostname - /// mismatches). Not supported by the Rust driver. + /// mismatches). This option can only be set in a URI. If it is set in a URI provided to + /// [`ConnectionString::parse`], [`TlsOptions::allow_invalid_certificates`] and + /// [`TlsOptions::allow_invalid_hostnames`] are set to its value. pub fn tls_insecure(&self) -> Option { self.tls_insecure } @@ -1661,42 +1667,47 @@ impl ConnectionString { return Ok(parts); } - let mut keys: Vec<&str> = Vec::new(); + let mut keys = HashSet::new(); for option_pair in options.split('&') { - let (key, value) = match option_pair.find('=') { - Some(index) => option_pair.split_at(index), + let (key, value) = match option_pair.split_once('=') { + Some((key, value)) => (key.to_lowercase(), value), None => { - return Err(ErrorKind::InvalidArgument { - message: format!( - "connection string options is not a `key=value` pair: {}", - option_pair, - ), - } - .into()) + return Err(Error::invalid_argument(format!( + "connection string option is not a 'key=value' pair: {option_pair}" + ))) } }; - if key.to_lowercase() != "readpreferencetags" && keys.contains(&key) { - return Err(ErrorKind::InvalidArgument { - message: "repeated options are not allowed in the connection string" - .to_string(), - } - .into()); - } else { - keys.push(key); + if !keys.insert(key.clone()) && key != "readpreferencetags" { + return Err(Error::invalid_argument( + "repeated options are not allowed in the connection string", + )); } - // Skip leading '=' in value. self.parse_option_pair( &mut parts, - &key.to_lowercase(), - percent_encoding::percent_decode(&value.as_bytes()[1..]) + &key, + percent_encoding::percent_decode(value.as_bytes()) .decode_utf8_lossy() .as_ref(), )?; } + if keys.contains(TLS_INSECURE) { + #[cfg(feature = "openssl-tls")] + let disallowed = [TLS_ALLOW_INVALID_CERTIFICATES, TLS_ALLOW_INVALID_HOSTNAMES]; + #[cfg(not(feature = "openssl-tls"))] + let disallowed = [TLS_ALLOW_INVALID_CERTIFICATES]; + for option in disallowed { + if keys.contains(option) { + return Err(Error::invalid_argument(format!( + "cannot set both {TLS_INSECURE} and {option} in the connection string" + ))); + } + } + } + if let Some(tags) = parts.read_preference_tags.take() { self.read_preference = match self.read_preference.take() { Some(read_pref) => Some(read_pref.with_tags(tags)?), @@ -2042,63 +2053,63 @@ impl ConnectionString { k @ "tls" | k @ "ssl" => { let tls = get_bool!(value, k); - match (self.tls.as_ref(), tls) { - (Some(Tls::Disabled), true) | (Some(Tls::Enabled(..)), false) => { - return Err(ErrorKind::InvalidArgument { - message: "All instances of `tls` and `ssl` must have the same - value" - .to_string(), + match self.tls { + Some(Tls::Enabled(_)) if !tls => { + return Err(Error::invalid_argument( + "cannot set {key}={tls} if other TLS options are set", + )) + } + Some(Tls::Disabled) if tls => { + return Err(Error::invalid_argument( + "cannot set {key}={tls} if TLS is disabled", + )) + } + None => { + if tls { + self.tls = Some(Tls::Enabled(Default::default())) + } else { + self.tls = Some(Tls::Disabled) } - .into()); } _ => {} - }; - - if self.tls.is_none() { - let tls = if tls { - Tls::Enabled(Default::default()) - } else { - Tls::Disabled - }; - - self.tls = Some(tls); } } - k @ "tlsinsecure" | k @ "tlsallowinvalidcertificates" => { - let val = get_bool!(value, k); - - let allow_invalid_certificates = if k == "tlsinsecure" { !val } else { val }; + TLS_INSECURE => { + let val = get_bool!(value, key); + self.tls_insecure = Some(val); - match self.tls { - Some(Tls::Disabled) => { - return Err(ErrorKind::InvalidArgument { - message: "'tlsInsecure' can't be set if tls=false".into(), + match self + .tls + .get_or_insert_with(|| Tls::Enabled(Default::default())) + { + Tls::Enabled(ref mut options) => { + options.allow_invalid_certificates = Some(val); + #[cfg(feature = "openssl-tls")] + { + options.allow_invalid_hostnames = Some(val); } - .into()) } - Some(Tls::Enabled(ref options)) - if options.allow_invalid_certificates.is_some() - && options.allow_invalid_certificates - != Some(allow_invalid_certificates) => - { - return Err(ErrorKind::InvalidArgument { - message: "all instances of 'tlsInsecure' and \ - 'tlsAllowInvalidCertificates' must be consistent (e.g. \ - 'tlsInsecure' cannot be true when \ - 'tlsAllowInvalidCertificates' is false, or vice-versa)" - .into(), - } - .into()); + Tls::Disabled => { + return Err(Error::invalid_argument(format!( + "cannot set {key} when TLS is disabled" + ))); } - Some(Tls::Enabled(ref mut options)) => { - options.allow_invalid_certificates = Some(allow_invalid_certificates); + } + } + TLS_ALLOW_INVALID_CERTIFICATES => { + let val = get_bool!(value, key); + + match self + .tls + .get_or_insert_with(|| Tls::Enabled(Default::default())) + { + Tls::Enabled(ref mut options) => { + options.allow_invalid_certificates = Some(val); } - None => { - self.tls = Some(Tls::Enabled( - TlsOptions::builder() - .allow_invalid_certificates(allow_invalid_certificates) - .build(), - )) + Tls::Disabled => { + return Err(Error::invalid_argument(format!( + "cannot set {key} when TLS is disabled" + ))) } } } diff --git a/src/client/options/test.rs b/src/client/options/test.rs index c9b80d998..bf300bd7d 100644 --- a/src/client/options/test.rs +++ b/src/client/options/test.rs @@ -10,6 +10,7 @@ use crate::{ bson_util::get_int, client::options::{ClientOptions, ConnectionString, ServerAddress}, error::ErrorKind, + options::Tls, test::spec::deserialize_spec_tests, Client, }; @@ -395,3 +396,55 @@ async fn tls_cert_key_password_connect() { .await .unwrap(); } + +#[tokio::test] +async fn tls_insecure() { + let uri = "mongodb://localhost:27017/?tls=true&tlsInsecure=true"; + let options = ClientOptions::parse(uri).await.unwrap(); + let Some(Tls::Enabled(tls_options)) = options.tls else { + panic!("expected tls options to be set"); + }; + assert_eq!(tls_options.allow_invalid_certificates, Some(true)); + #[cfg(feature = "openssl-tls")] + assert_eq!(tls_options.allow_invalid_hostnames, Some(true)); + + let uri = "mongodb://localhost:27017/?tls=true&tlsInsecure=false"; + let options = ClientOptions::parse(uri).await.unwrap(); + let Some(Tls::Enabled(tls_options)) = options.tls else { + panic!("expected tls options to be set"); + }; + assert_eq!(tls_options.allow_invalid_certificates, Some(false)); + #[cfg(feature = "openssl-tls")] + assert_eq!(tls_options.allow_invalid_hostnames, Some(false)); + + let uri = "mongodb://localhost:27017/?tls=false&tlsInsecure=true"; + let error = ClientOptions::parse(uri).await.unwrap_err(); + assert!(error.message().unwrap().contains("TLS is disabled")); + + let uri = "mongodb://localhost:27017/?tlsInsecure=true&tls=false"; + let error = ClientOptions::parse(uri).await.unwrap_err(); + assert!(error + .message() + .unwrap() + .contains("other TLS options are set")); + + let uri = "mongodb://localhost:27017/?tlsInsecure=true&tlsAllowInvalidCertificates=true"; + let error = ClientOptions::parse(uri).await.unwrap_err(); + assert!(error.message().unwrap().contains("cannot set both")); + + let uri = "mongodb://localhost:27017/?tlsInsecure=true&tlsAllowInvalidCertificates=false"; + let error = ClientOptions::parse(uri).await.unwrap_err(); + assert!(error.message().unwrap().contains("cannot set both")); + + // TODO RUST-1896: uncomment these tests + // #[cfg(feature = "openssl-tls")] + // { + // let uri = "mongodb://localhost:27017/?tlsInsecure=true&tlsAllowInvalidHostnames=true"; + // let error = ClientOptions::parse(uri).await.unwrap_err(); + // assert!(error.message().unwrap().contains("cannot set both")); + + // let uri = "mongodb://localhost:27017/?tlsInsecure=true&tlsAllowInvalidHostnames=false"; + // let error = ClientOptions::parse(uri).await.unwrap_err(); + // assert!(error.message().unwrap().contains("cannot set both")); + // } +}