Skip to content

Commit 257d941

Browse files
authored
feat(dgw): new TlsVerifyStrict option (#1415)
This adds a `TlsVerifyStrict` option for controlling the new stricter checks on TLS certificates. When enabled (`true`), the client performs additional checks on the server certificate, including: - Ensuring the presence of the **Subject Alternative Name (SAN)** extension. - Verifying that the **Extended Key Usage (EKU)** extension includes `serverAuth`. Certificates that do not meet these requirements are increasingly rejected by modern clients (e.g., Chrome, macOS). Therefore, we strongly recommend using certificates that comply with these standards. The default configuration for fresh installs will include the `TlsVerifyStrict` key set to `true`. Issue: DGW-293
1 parent 78028a6 commit 257d941

File tree

4 files changed

+83
-26
lines changed

4 files changed

+83
-26
lines changed

README.md

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,18 @@ Stable options are:
122122
additional measures like securing access to the files or using the system certificate store (see
123123
**TlsCertificateSource** option).
124124

125+
- **TlsVerifyStrict** (_Boolean_): Enables strict TLS certificate verification (default is `true`).
126+
127+
When enabled (`true`), the client performs additional checks on the server certificate,
128+
including:
129+
130+
- Ensuring the presence of the **Subject Alternative Name (SAN)** extension.
131+
- Verifying that the **Extended Key Usage (EKU)** extension includes `serverAuth`.
132+
133+
Certificates that do not meet these requirements are increasingly rejected by modern clients
134+
(e.g., Chrome, macOS). Therefore, we strongly recommend using certificates that comply with
135+
these standards.
136+
125137
- **Listeners** (_Array_): Array of listener URLs.
126138

127139
Each element has the following schema:

devolutions-gateway/src/config.rs

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,9 @@ impl fmt::Debug for Tls {
5858
}
5959

6060
impl Tls {
61-
fn init(cert_source: crate::tls::CertificateSource) -> anyhow::Result<Self> {
62-
let tls_server_config = crate::tls::build_server_config(cert_source).context("failed to build TLS config")?;
61+
fn init(cert_source: crate::tls::CertificateSource, strict_checks: bool) -> anyhow::Result<Self> {
62+
let tls_server_config =
63+
crate::tls::build_server_config(cert_source, strict_checks).context("failed to build TLS config")?;
6364

6465
let acceptor = tokio_rustls::TlsAcceptor::from(Arc::new(tls_server_config));
6566

@@ -151,6 +152,8 @@ impl Conf {
151152
.iter()
152153
.any(|l| matches!(l.internal_url.scheme(), "https" | "wss"));
153154

155+
let strict_checks = conf_file.tls_verify_strict.unwrap_or(true);
156+
154157
let tls = match conf_file.tls_certificate_source.unwrap_or_default() {
155158
dto::CertSource::External => match conf_file.tls_certificate_file.as_ref() {
156159
None if requires_tls => anyhow::bail!("TLS usage implied, but TLS certificate file is missing"),
@@ -181,7 +184,9 @@ impl Conf {
181184
private_key,
182185
};
183186

184-
Tls::init(cert_source).context("failed to init TLS config")?.pipe(Some)
187+
Tls::init(cert_source, strict_checks)
188+
.context("failed to init TLS config")?
189+
.pipe(Some)
185190
}
186191
},
187192
dto::CertSource::System => match conf_file.tls_certificate_subject_name.clone() {
@@ -202,7 +207,9 @@ impl Conf {
202207
store_name,
203208
};
204209

205-
Tls::init(cert_source).context("failed to init TLS config")?.pipe(Some)
210+
Tls::init(cert_source, strict_checks)
211+
.context("failed to init TLS config")?
212+
.pipe(Some)
206213
}
207214
},
208215
};
@@ -963,6 +970,20 @@ pub mod dto {
963970
/// Location of the Windows Certificate Store to use
964971
#[serde(skip_serializing_if = "Option::is_none")]
965972
pub tls_certificate_store_location: Option<CertStoreLocation>,
973+
/// Enables strict TLS certificate verification.
974+
///
975+
/// When enabled (`true`), the client performs additional checks on the server certificate,
976+
/// including:
977+
/// - Ensuring the presence of the **Subject Alternative Name (SAN)** extension.
978+
/// - Verifying that the **Extended Key Usage (EKU)** extension includes `serverAuth`.
979+
///
980+
/// Certificates that do not meet these requirements are increasingly rejected by modern clients
981+
/// (e.g., Chrome, macOS). Therefore, we strongly recommend using certificates that comply with
982+
/// these standards.
983+
///
984+
/// If unset, the default is `true`.
985+
#[serde(skip_serializing_if = "Option::is_none")]
986+
pub tls_verify_strict: Option<bool>,
966987

967988
/// Listeners to launch at startup
968989
#[serde(default, skip_serializing_if = "Vec::is_empty")]
@@ -1038,6 +1059,7 @@ pub mod dto {
10381059
tls_certificate_subject_name: None,
10391060
tls_certificate_store_name: None,
10401061
tls_certificate_store_location: None,
1062+
tls_verify_strict: Some(true),
10411063
listeners: vec![
10421064
ListenerConf {
10431065
internal_url: "tcp://*:8181".to_owned(),

devolutions-gateway/src/tls.rs

Lines changed: 39 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,10 @@ pub enum CertificateSource {
6666
},
6767
}
6868

69-
pub fn build_server_config(cert_source: CertificateSource) -> anyhow::Result<rustls::ServerConfig> {
69+
pub fn build_server_config(
70+
cert_source: CertificateSource,
71+
strict_checks: bool,
72+
) -> anyhow::Result<rustls::ServerConfig> {
7073
let builder = rustls::ServerConfig::builder().with_no_client_auth();
7174

7275
match cert_source {
@@ -76,18 +79,20 @@ pub fn build_server_config(cert_source: CertificateSource) -> anyhow::Result<rus
7679
} => {
7780
let first_certificate = certificates.first().context("empty certificate list")?;
7881

79-
if let Ok(report) = check_certificate_now(&first_certificate) {
80-
if report.issues.intersects(
81-
CertIssues::MISSING_SERVER_AUTH_EXTENDED_KEY_USAGE | CertIssues::MISSING_SUBJECT_ALT_NAME,
82-
) {
83-
let serial_number = report.serial_number;
84-
let subject = report.subject;
85-
let issuer = report.issuer;
86-
let not_before = report.not_before;
87-
let not_after = report.not_after;
88-
let issues = report.issues;
89-
90-
anyhow::bail!("found significant issues with the certificate: serial_number = {serial_number}, subject = {subject}, issuer = {issuer}, not_before = {not_before}, not_after = {not_after}, issues = {issues}");
82+
if strict_checks {
83+
if let Ok(report) = check_certificate_now(&first_certificate) {
84+
if report.issues.intersects(
85+
CertIssues::MISSING_SERVER_AUTH_EXTENDED_KEY_USAGE | CertIssues::MISSING_SUBJECT_ALT_NAME,
86+
) {
87+
let serial_number = report.serial_number;
88+
let subject = report.subject;
89+
let issuer = report.issuer;
90+
let not_before = report.not_before;
91+
let not_after = report.not_after;
92+
let issues = report.issues;
93+
94+
anyhow::bail!("found significant issues with the certificate: serial_number = {serial_number}, subject = {subject}, issuer = {issuer}, not_before = {not_before}, not_after = {not_after}, issues = {issues}");
95+
}
9196
}
9297
}
9398

@@ -103,9 +108,14 @@ pub fn build_server_config(cert_source: CertificateSource) -> anyhow::Result<rus
103108
store_location,
104109
store_name,
105110
} => {
106-
let resolver =
107-
windows::ServerCertResolver::new(machine_hostname, cert_subject_name, store_location, store_name)
108-
.context("create ServerCertResolver")?;
111+
let resolver = windows::ServerCertResolver::new(
112+
machine_hostname,
113+
cert_subject_name,
114+
store_location,
115+
store_name,
116+
strict_checks,
117+
)
118+
.context("create ServerCertResolver")?;
109119
Ok(builder.with_cert_resolver(Arc::new(resolver)))
110120
}
111121
#[cfg(not(windows))]
@@ -146,6 +156,7 @@ pub mod windows {
146156
store_type: CertStoreType,
147157
store_name: String,
148158
cached_key: Mutex<Option<KeyCache>>,
159+
strict_checks: bool,
149160
}
150161

151162
#[derive(Debug)]
@@ -160,6 +171,7 @@ pub mod windows {
160171
cert_subject_name: String,
161172
store_type: dto::CertStoreLocation,
162173
store_name: String,
174+
strict_checks: bool,
163175
) -> anyhow::Result<Self> {
164176
let store_type = match store_type {
165177
dto::CertStoreLocation::LocalMachine => CertStoreType::LocalMachine,
@@ -173,6 +185,7 @@ pub mod windows {
173185
store_type,
174186
store_name,
175187
cached_key: Mutex::new(None),
188+
strict_checks,
176189
})
177190
}
178191

@@ -255,14 +268,18 @@ pub mod windows {
255268
cert_issues |= report.issues;
256269

257270
// Skip the certificate if any of the following is true:
258-
// - The certificate is not yet valid.
259-
// - The certificate is missing the server auth extended key usage.
260-
// - The certificate is missing a subject alternative name (SAN) extension.
261-
let skip = report.issues.intersects(
271+
// - the certificate is not yet valid,
272+
// - (if strict) the certificate is missing the server auth extended key usage,
273+
// - (if strict) the certificate is missing a subject alternative name (SAN) extension.
274+
let issues_to_check = if self.strict_checks {
262275
CertIssues::NOT_YET_VALID
263276
| CertIssues::MISSING_SERVER_AUTH_EXTENDED_KEY_USAGE
264-
| CertIssues::MISSING_SUBJECT_ALT_NAME,
265-
);
277+
| CertIssues::MISSING_SUBJECT_ALT_NAME
278+
} else {
279+
CertIssues::NOT_YET_VALID
280+
};
281+
282+
let skip = report.issues.intersects(issues_to_check);
266283

267284
if skip {
268285
debug!(

devolutions-gateway/tests/config.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ fn hub_sample() -> Sample {
1919
"Hostname": "hostname.example.io",
2020
"TlsPrivateKeyFile": "/path/to/tls-private.key",
2121
"TlsCertificateFile": "/path/to/tls-certificate.pem",
22+
"TlsVerifyStrict": true,
2223
"ProvisionerPublicKeyFile": "/path/to/provisioner.pub.key",
2324
"SubProvisionerPublicKey": {
2425
"Id": "subkey-id",
@@ -69,6 +70,7 @@ fn hub_sample() -> Sample {
6970
tls_certificate_subject_name: None,
7071
tls_certificate_store_location: None,
7172
tls_certificate_store_name: None,
73+
tls_verify_strict: Some(true),
7274
listeners: vec![
7375
ListenerConf {
7476
internal_url: "tcp://*:8080".to_owned(),
@@ -121,6 +123,7 @@ fn legacy_sample() -> Sample {
121123
tls_certificate_subject_name: None,
122124
tls_certificate_store_location: None,
123125
tls_certificate_store_name: None,
126+
tls_verify_strict: None,
124127
listeners: vec![],
125128
subscriber: None,
126129
log_file: Some("/path/to/log/file.log".into()),
@@ -163,6 +166,7 @@ fn system_store_sample() -> Sample {
163166
tls_certificate_subject_name: Some("localhost".to_owned()),
164167
tls_certificate_store_location: Some(CertStoreLocation::LocalMachine),
165168
tls_certificate_store_name: Some("My".to_owned()),
169+
tls_verify_strict: None,
166170
listeners: vec![],
167171
subscriber: None,
168172
log_file: None,
@@ -221,6 +225,7 @@ fn standalone_custom_auth_sample() -> Sample {
221225
tls_certificate_subject_name: None,
222226
tls_certificate_store_location: None,
223227
tls_certificate_store_name: None,
228+
tls_verify_strict: None,
224229
listeners: vec![
225230
ListenerConf {
226231
internal_url: "tcp://*:8080".to_owned(),
@@ -295,6 +300,7 @@ fn standalone_no_auth_sample() -> Sample {
295300
tls_certificate_subject_name: None,
296301
tls_certificate_store_location: None,
297302
tls_certificate_store_name: None,
303+
tls_verify_strict: None,
298304
listeners: vec![
299305
ListenerConf {
300306
internal_url: "tcp://*:8080".to_owned(),

0 commit comments

Comments
 (0)