Skip to content

Commit 7c2c042

Browse files
authored
fix(dgw): reopen the certificate store for each request (#1274)
When using the system certificate store, we now properly reopen the store for each HTTP request, eliminating the need for restarting the service when renewing the certificate. Issue: DGW-256
1 parent 6e74bcc commit 7c2c042

File tree

1 file changed

+28
-10
lines changed

1 file changed

+28
-10
lines changed

devolutions-gateway/src/tls.rs

Lines changed: 28 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ pub fn build_server_config(cert_source: CertificateSource) -> anyhow::Result<rus
8383
store_name,
8484
} => {
8585
let resolver =
86-
windows::ServerCertResolver::new(machine_hostname, cert_subject_name, store_location, &store_name)
86+
windows::ServerCertResolver::new(machine_hostname, cert_subject_name, store_location, store_name)
8787
.context("create ServerCertResolver")?;
8888
Ok(builder.with_cert_resolver(Arc::new(resolver)))
8989
}
@@ -118,28 +118,28 @@ pub mod windows {
118118
pub struct ServerCertResolver {
119119
machine_hostname: String,
120120
subject_name: String,
121-
store: CertStore,
121+
store_type: CertStoreType,
122+
store_name: String,
122123
}
123124

124125
impl ServerCertResolver {
125126
pub fn new(
126127
machine_hostname: String,
127128
cert_subject_name: String,
128129
store_type: dto::CertStoreLocation,
129-
store_name: &str,
130+
store_name: String,
130131
) -> anyhow::Result<Self> {
131132
let store_type = match store_type {
132133
dto::CertStoreLocation::LocalMachine => CertStoreType::LocalMachine,
133134
dto::CertStoreLocation::CurrentUser => CertStoreType::CurrentUser,
134135
dto::CertStoreLocation::CurrentService => CertStoreType::CurrentService,
135136
};
136137

137-
let store = CertStore::open(store_type, store_name).context("open Windows certificate store")?;
138-
139138
Ok(Self {
140139
machine_hostname,
141140
subject_name: cert_subject_name,
142-
store,
141+
store_type,
142+
store_name,
143143
})
144144
}
145145

@@ -168,12 +168,16 @@ pub mod windows {
168168
)
169169
}
170170

171+
let store = CertStore::open(self.store_type, &self.store_name).context("open Windows certificate store")?;
172+
171173
// Look up certificate by subject.
172174
// TODO(perf): the resolution result could probably be cached.
173-
let contexts = self
174-
.store
175-
.find_by_subject_str(&self.subject_name)
176-
.context("failed to find server certificate from system store")?;
175+
let mut contexts = store.find_by_subject_str(&self.subject_name).with_context(|| {
176+
format!(
177+
"failed to find server certificate for {} from system store",
178+
self.subject_name
179+
)
180+
})?;
177181

178182
anyhow::ensure!(
179183
!contexts.is_empty(),
@@ -183,6 +187,20 @@ pub mod windows {
183187

184188
trace!(subject_name = %self.subject_name, count = contexts.len(), "Found certificate contexts");
185189

190+
// Sort certificates from the furthest to the earliest expiration
191+
// time. Note that it appears the certificates are already returned
192+
// in this order, but it is not a documented behavior. It really
193+
// depends on the internal order maintained by the store, and there
194+
// is no guarantee about what this order is, thus we implement the
195+
// logic here anyway.
196+
contexts.sort_by_cached_key(|ctx| match picky::x509::Cert::from_der(ctx.as_der()) {
197+
Ok(cert) => cert.valid_not_before(),
198+
Err(error) => {
199+
warn!(%error, "Failed to parse store certificate");
200+
picky::x509::date::UtcDate::ymd(1900, 1, 1).expect("hardcoded")
201+
}
202+
});
203+
186204
// Attempt to acquire a private key and construct CngSigningKey.
187205
let (context, key) = contexts
188206
.into_iter()

0 commit comments

Comments
 (0)