Skip to content

Commit b918cd6

Browse files
isabelatkinsonisabelatkinson
authored
RUST-2264 Fix handling of tlsInsecure in the URI (#1453)
1 parent 007894c commit b918cd6

File tree

3 files changed

+140
-75
lines changed

3 files changed

+140
-75
lines changed

‎src/action/client_options.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -51,10 +51,10 @@ impl ClientOptions {
5151
/// * `socketTimeoutMS`: unsupported, does not map to any field
5252
/// * `ssl`: an alias of the `tls` option
5353
/// * `tls`: maps to the TLS variant of the `tls` field`.
54-
/// * `tlsInsecure`: relaxes the TLS constraints on connections being made; currently is just
55-
/// an alias of `tlsAllowInvalidCertificates`, but more behavior may be added to this option
56-
/// in the future
57-
/// * `tlsAllowInvalidCertificates`: maps to the `allow_invalidCertificates` field of the
54+
/// * `tlsInsecure`: relaxes the TLS constraints on connections being made; if set, the
55+
/// `allow_invalid_certificates` and `allow_invalid_hostnames` fields of the `tls` field are
56+
/// set to its value
57+
/// * `tlsAllowInvalidCertificates`: maps to the `allow_invalid_certificates` field of the
5858
/// `tls` field
5959
/// * `tlsCAFile`: maps to the `ca_file_path` field of the `tls` field
6060
/// * `tlsCertificateKeyFile`: maps to the `cert_key_file_path` field of the `tls` field

‎src/client/options.rs

Lines changed: 83 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,10 @@ pub(crate) use resolver_config::ResolverConfig;
5555

5656
pub(crate) const DEFAULT_PORT: u16 = 27017;
5757

58+
const TLS_INSECURE: &str = "tlsinsecure";
59+
const TLS_ALLOW_INVALID_CERTIFICATES: &str = "tlsallowinvalidcertificates";
60+
#[cfg(feature = "openssl-tls")]
61+
const TLS_ALLOW_INVALID_HOSTNAMES: &str = "tlsallowinvalidhostnames";
5862
const URI_OPTIONS: &[&str] = &[
5963
"appname",
6064
"authmechanism",
@@ -82,8 +86,8 @@ const URI_OPTIONS: &[&str] = &[
8286
"sockettimeoutms",
8387
"tls",
8488
"ssl",
85-
"tlsinsecure",
86-
"tlsallowinvalidcertificates",
89+
TLS_INSECURE,
90+
TLS_ALLOW_INVALID_CERTIFICATES,
8791
"tlscafile",
8892
"tlscertificatekeyfile",
8993
"uuidRepresentation",
@@ -1647,7 +1651,9 @@ impl ConnectionString {
16471651
}
16481652

16491653
/// Relax TLS constraints as much as possible (e.g. allowing invalid certificates or hostname
1650-
/// mismatches). Not supported by the Rust driver.
1654+
/// mismatches). This option can only be set in a URI. If it is set in a URI provided to
1655+
/// [`ConnectionString::parse`], [`TlsOptions::allow_invalid_certificates`] and
1656+
/// [`TlsOptions::allow_invalid_hostnames`] are set to its value.
16511657
pub fn tls_insecure(&self) -> Option<bool> {
16521658
self.tls_insecure
16531659
}
@@ -1662,41 +1668,47 @@ impl ConnectionString {
16621668
return Ok(parts);
16631669
}
16641670

1665-
let mut keys: Vec<&str> = Vec::new();
1671+
let mut keys = HashSet::new();
16661672

16671673
for option_pair in options.split('&') {
1668-
let (key, value) = match option_pair.find('=') {
1669-
Some(index) => option_pair.split_at(index),
1674+
let (key, value) = match option_pair.split_once('=') {
1675+
Some((key, value)) => (key.to_lowercase(), value),
16701676
None => {
1671-
return Err(ErrorKind::InvalidArgument {
1672-
message: format!(
1673-
"connection string options is not a `key=value` pair: {option_pair}",
1674-
),
1675-
}
1676-
.into())
1677+
return Err(Error::invalid_argument(format!(
1678+
"connection string option is not a 'key=value' pair: {option_pair}"
1679+
)))
16771680
}
16781681
};
16791682

1680-
if key.to_lowercase() != "readpreferencetags" && keys.contains(&key) {
1681-
return Err(ErrorKind::InvalidArgument {
1682-
message: "repeated options are not allowed in the connection string"
1683-
.to_string(),
1684-
}
1685-
.into());
1686-
} else {
1687-
keys.push(key);
1683+
if !keys.insert(key.clone()) && key != "readpreferencetags" {
1684+
return Err(Error::invalid_argument(
1685+
"repeated options are not allowed in the connection string",
1686+
));
16881687
}
16891688

1690-
// Skip leading '=' in value.
16911689
self.parse_option_pair(
16921690
&mut parts,
1693-
&key.to_lowercase(),
1694-
percent_encoding::percent_decode(&value.as_bytes()[1..])
1691+
&key,
1692+
percent_encoding::percent_decode(value.as_bytes())
16951693
.decode_utf8_lossy()
16961694
.as_ref(),
16971695
)?;
16981696
}
16991697

1698+
if keys.contains(TLS_INSECURE) {
1699+
#[cfg(feature = "openssl-tls")]
1700+
let disallowed = [TLS_ALLOW_INVALID_CERTIFICATES, TLS_ALLOW_INVALID_HOSTNAMES];
1701+
#[cfg(not(feature = "openssl-tls"))]
1702+
let disallowed = [TLS_ALLOW_INVALID_CERTIFICATES];
1703+
for option in disallowed {
1704+
if keys.contains(option) {
1705+
return Err(Error::invalid_argument(format!(
1706+
"cannot set both {TLS_INSECURE} and {option} in the connection string"
1707+
)));
1708+
}
1709+
}
1710+
}
1711+
17001712
if let Some(tags) = parts.read_preference_tags.take() {
17011713
self.read_preference = match self.read_preference.take() {
17021714
Some(read_pref) => Some(read_pref.with_tags(tags)?),
@@ -2017,63 +2029,63 @@ impl ConnectionString {
20172029
k @ "tls" | k @ "ssl" => {
20182030
let tls = get_bool!(value, k);
20192031

2020-
match (self.tls.as_ref(), tls) {
2021-
(Some(Tls::Disabled), true) | (Some(Tls::Enabled(..)), false) => {
2022-
return Err(ErrorKind::InvalidArgument {
2023-
message: "All instances of `tls` and `ssl` must have the same
2024-
value"
2025-
.to_string(),
2032+
match self.tls {
2033+
Some(Tls::Enabled(_)) if !tls => {
2034+
return Err(Error::invalid_argument(
2035+
"cannot set {key}={tls} if other TLS options are set",
2036+
))
2037+
}
2038+
Some(Tls::Disabled) if tls => {
2039+
return Err(Error::invalid_argument(
2040+
"cannot set {key}={tls} if TLS is disabled",
2041+
))
2042+
}
2043+
None => {
2044+
if tls {
2045+
self.tls = Some(Tls::Enabled(Default::default()))
2046+
} else {
2047+
self.tls = Some(Tls::Disabled)
20262048
}
2027-
.into());
20282049
}
20292050
_ => {}
2030-
};
2031-
2032-
if self.tls.is_none() {
2033-
let tls = if tls {
2034-
Tls::Enabled(Default::default())
2035-
} else {
2036-
Tls::Disabled
2037-
};
2038-
2039-
self.tls = Some(tls);
20402051
}
20412052
}
2042-
k @ "tlsinsecure" | k @ "tlsallowinvalidcertificates" => {
2043-
let val = get_bool!(value, k);
2044-
2045-
let allow_invalid_certificates = if k == "tlsinsecure" { !val } else { val };
2046-
2047-
match self.tls {
2048-
Some(Tls::Disabled) => {
2049-
return Err(ErrorKind::InvalidArgument {
2050-
message: "'tlsInsecure' can't be set if tls=false".into(),
2053+
TLS_INSECURE => {
2054+
let val = get_bool!(value, key);
2055+
self.tls_insecure = Some(val);
2056+
2057+
match self
2058+
.tls
2059+
.get_or_insert_with(|| Tls::Enabled(Default::default()))
2060+
{
2061+
Tls::Enabled(ref mut options) => {
2062+
options.allow_invalid_certificates = Some(val);
2063+
#[cfg(feature = "openssl-tls")]
2064+
{
2065+
options.allow_invalid_hostnames = Some(val);
20512066
}
2052-
.into())
20532067
}
2054-
Some(Tls::Enabled(ref options))
2055-
if options.allow_invalid_certificates.is_some()
2056-
&& options.allow_invalid_certificates
2057-
!= Some(allow_invalid_certificates) =>
2058-
{
2059-
return Err(ErrorKind::InvalidArgument {
2060-
message: "all instances of 'tlsInsecure' and \
2061-
'tlsAllowInvalidCertificates' must be consistent (e.g. \
2062-
'tlsInsecure' cannot be true when \
2063-
'tlsAllowInvalidCertificates' is false, or vice-versa)"
2064-
.into(),
2065-
}
2066-
.into());
2068+
Tls::Disabled => {
2069+
return Err(Error::invalid_argument(format!(
2070+
"cannot set {key} when TLS is disabled"
2071+
)));
20672072
}
2068-
Some(Tls::Enabled(ref mut options)) => {
2069-
options.allow_invalid_certificates = Some(allow_invalid_certificates);
2073+
}
2074+
}
2075+
TLS_ALLOW_INVALID_CERTIFICATES => {
2076+
let val = get_bool!(value, key);
2077+
2078+
match self
2079+
.tls
2080+
.get_or_insert_with(|| Tls::Enabled(Default::default()))
2081+
{
2082+
Tls::Enabled(ref mut options) => {
2083+
options.allow_invalid_certificates = Some(val);
20702084
}
2071-
None => {
2072-
self.tls = Some(Tls::Enabled(
2073-
TlsOptions::builder()
2074-
.allow_invalid_certificates(allow_invalid_certificates)
2075-
.build(),
2076-
))
2085+
Tls::Disabled => {
2086+
return Err(Error::invalid_argument(format!(
2087+
"cannot set {key} when TLS is disabled"
2088+
)))
20772089
}
20782090
}
20792091
}

‎src/client/options/test.rs

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ use crate::{
1010
bson_util::get_int,
1111
client::options::{ClientOptions, ConnectionString, ServerAddress},
1212
error::ErrorKind,
13+
options::Tls,
1314
test::spec::deserialize_spec_tests,
1415
Client,
1516
};
@@ -428,3 +429,55 @@ async fn tls_cert_key_password_connect() {
428429
.await
429430
.unwrap();
430431
}
432+
433+
#[tokio::test]
434+
async fn tls_insecure() {
435+
let uri = "mongodb://localhost:27017?tls=true&tlsInsecure=true";
436+
let options = ClientOptions::parse(uri).await.unwrap();
437+
let Some(Tls::Enabled(tls_options)) = options.tls else {
438+
panic!("expected tls options to be set");
439+
};
440+
assert_eq!(tls_options.allow_invalid_certificates, Some(true));
441+
#[cfg(feature = "openssl-tls")]
442+
assert_eq!(tls_options.allow_invalid_hostnames, Some(true));
443+
444+
let uri = "mongodb://localhost:27017?tls=true&tlsInsecure=false";
445+
let options = ClientOptions::parse(uri).await.unwrap();
446+
let Some(Tls::Enabled(tls_options)) = options.tls else {
447+
panic!("expected tls options to be set");
448+
};
449+
assert_eq!(tls_options.allow_invalid_certificates, Some(false));
450+
#[cfg(feature = "openssl-tls")]
451+
assert_eq!(tls_options.allow_invalid_hostnames, Some(false));
452+
453+
let uri = "mongodb://localhost:27017?tls=false&tlsInsecure=true";
454+
let error = ClientOptions::parse(uri).await.unwrap_err();
455+
assert!(error.message().unwrap().contains("TLS is disabled"));
456+
457+
let uri = "mongodb://localhost:27017?tlsInsecure=true&tls=false";
458+
let error = ClientOptions::parse(uri).await.unwrap_err();
459+
assert!(error
460+
.message()
461+
.unwrap()
462+
.contains("other TLS options are set"));
463+
464+
let uri = "mongodb://localhost:27017?tlsInsecure=true&tlsAllowInvalidCertificates=true";
465+
let error = ClientOptions::parse(uri).await.unwrap_err();
466+
assert!(error.message().unwrap().contains("cannot set both"));
467+
468+
let uri = "mongodb://localhost:27017?tlsInsecure=true&tlsAllowInvalidCertificates=false";
469+
let error = ClientOptions::parse(uri).await.unwrap_err();
470+
assert!(error.message().unwrap().contains("cannot set both"));
471+
472+
// TODO RUST-1896: uncomment these tests
473+
// #[cfg(feature = "openssl-tls")]
474+
// {
475+
// let uri = "mongodb://localhost:27017?tlsInsecure=true&tlsAllowInvalidHostnames=true";
476+
// let error = ClientOptions::parse(uri).await.unwrap_err();
477+
// assert!(error.message().unwrap().contains("cannot set both"));
478+
479+
// let uri = "mongodb://localhost:27017?tlsInsecure=true&tlsAllowInvalidHostnames=false";
480+
// let error = ClientOptions::parse(uri).await.unwrap_err();
481+
// assert!(error.message().unwrap().contains("cannot set both"));
482+
// }
483+
}

0 commit comments

Comments
 (0)