Skip to content

Commit 476ce29

Browse files
committed
fix: add wall-clock certificate expiry check to webhook TLS rotation
The rotation interval uses tokio's monotonic clock, but certificate validity uses wall-clock time. When these diverge (hibernation, VM migration, cgroup freezing), the certificate can expire before rotation. Add a periodic wall-clock check (every 5 minutes) that compares SystemTime::now() against the certificate's not_after field and triggers early rotation if the cert is within 4 hours of expiry. Fixes: #1174
1 parent 7486017 commit 476ce29

2 files changed

Lines changed: 58 additions & 6 deletions

File tree

crates/stackable-webhook/src/tls/cert_resolver.rs

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use std::sync::Arc;
1+
use std::{sync::Arc, time::SystemTime};
22

33
use arc_swap::ArcSwap;
44
use snafu::{OptionExt, ResultExt, Snafu};
@@ -57,6 +57,9 @@ pub struct CertificateResolver {
5757
/// Using a [`ArcSwap`] (over e.g. [`tokio::sync::RwLock`]), so that we can easily
5858
/// (and performant) bridge between async write and sync read.
5959
current_certified_key: ArcSwap<CertifiedKey>,
60+
/// The wall-clock expiry time (`not_after`) of the current certificate.
61+
/// Used to detect clock drift between monotonic and wall-clock time.
62+
current_not_after: ArcSwap<SystemTime>,
6063
subject_alterative_dns_names: Arc<Vec<String>>,
6164

6265
certificate_tx: mpsc::Sender<Certificate>,
@@ -68,7 +71,7 @@ impl CertificateResolver {
6871
certificate_tx: mpsc::Sender<Certificate>,
6972
) -> Result<Self> {
7073
let subject_alterative_dns_names = Arc::new(subject_alterative_dns_names);
71-
let certified_key = Self::generate_new_certificate_inner(
74+
let (certified_key, not_after) = Self::generate_new_certificate_inner(
7275
subject_alterative_dns_names.clone(),
7376
&certificate_tx,
7477
)
@@ -77,20 +80,37 @@ impl CertificateResolver {
7780
Ok(Self {
7881
subject_alterative_dns_names,
7982
current_certified_key: ArcSwap::new(certified_key),
83+
current_not_after: ArcSwap::new(Arc::new(not_after)),
8084
certificate_tx,
8185
})
8286
}
8387

8488
pub async fn rotate_certificate(&self) -> Result<()> {
85-
let certified_key = self.generate_new_certificate().await?;
89+
let (certified_key, not_after) = self.generate_new_certificate().await?;
8690

8791
// TODO: Sign the new cert somehow with the old cert. See https://github.com/stackabletech/decisions/issues/56
8892
self.current_certified_key.store(certified_key);
93+
self.current_not_after.store(Arc::new(not_after));
8994

9095
Ok(())
9196
}
9297

93-
async fn generate_new_certificate(&self) -> Result<Arc<CertifiedKey>> {
98+
/// Returns `true` if the current certificate is expired or will expire
99+
/// within the given `buffer` duration according to wall-clock time.
100+
///
101+
/// This catches cases where the monotonic timer (used by `tokio::time`)
102+
/// has drifted from wall-clock time, e.g. due to system hibernation.
103+
pub fn needs_rotation(&self, buffer: std::time::Duration) -> bool {
104+
let not_after = **self.current_not_after.load();
105+
// If subtraction underflows (buffer > time since epoch), fall back to
106+
// UNIX_EPOCH so that the comparison always triggers rotation.
107+
let deadline = not_after
108+
.checked_sub(buffer)
109+
.unwrap_or(SystemTime::UNIX_EPOCH);
110+
SystemTime::now() >= deadline
111+
}
112+
113+
async fn generate_new_certificate(&self) -> Result<(Arc<CertifiedKey>, SystemTime)> {
94114
let subject_alterative_dns_names = self.subject_alterative_dns_names.clone();
95115
Self::generate_new_certificate_inner(subject_alterative_dns_names, &self.certificate_tx)
96116
.await
@@ -106,7 +126,7 @@ impl CertificateResolver {
106126
async fn generate_new_certificate_inner(
107127
subject_alterative_dns_names: Arc<Vec<String>>,
108128
certificate_tx: &mpsc::Sender<Certificate>,
109-
) -> Result<Arc<CertifiedKey>> {
129+
) -> Result<(Arc<CertifiedKey>, SystemTime)> {
110130
// The certificate generations can take a while, so we use `spawn_blocking`
111131
let (cert, certified_key) = tokio::task::spawn_blocking(move || {
112132
let tls_provider =
@@ -144,12 +164,14 @@ impl CertificateResolver {
144164
.await
145165
.context(TokioSpawnBlockingSnafu)??;
146166

167+
let not_after = cert.tbs_certificate.validity.not_after.to_system_time();
168+
147169
certificate_tx
148170
.send(cert)
149171
.await
150172
.map_err(|_err| CertificateResolverError::SendCertificateToChannel)?;
151173

152-
Ok(certified_key)
174+
Ok((certified_key, not_after))
153175
}
154176
}
155177

crates/stackable-webhook/src/tls/mod.rs

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,14 @@ pub const WEBHOOK_CA_LIFETIME: Duration = Duration::from_hours_unchecked(24);
4242
pub const WEBHOOK_CERTIFICATE_LIFETIME: Duration = Duration::from_hours_unchecked(24);
4343
pub const WEBHOOK_CERTIFICATE_ROTATION_INTERVAL: Duration = Duration::from_hours_unchecked(20);
4444

45+
/// How often to check wall-clock time for certificate expiry (5 minutes).
46+
/// This catches clock drift from system hibernation, VM migration, etc.
47+
const WALL_CLOCK_CHECK_INTERVAL: Duration = Duration::from_minutes_unchecked(5);
48+
49+
/// Rotate the certificate when it is within this buffer of expiry according
50+
/// to wall-clock time (4 hours before the 24h certificate expires).
51+
const WALL_CLOCK_EXPIRY_BUFFER: Duration = Duration::from_hours_unchecked(4);
52+
4553
pub type Result<T, E = TlsServerError> = std::result::Result<T, E>;
4654

4755
#[derive(Debug, Snafu)]
@@ -156,6 +164,13 @@ impl TlsServer {
156164
let start = tokio::time::Instant::now() + *WEBHOOK_CERTIFICATE_ROTATION_INTERVAL;
157165
let mut interval = tokio::time::interval_at(start, *WEBHOOK_CERTIFICATE_ROTATION_INTERVAL);
158166

167+
// A separate, shorter interval to check wall-clock time against the
168+
// certificate's not_after. This catches monotonic/wall-clock drift
169+
// caused by hibernation, VM migration, cgroup freezing, etc.
170+
let wall_clock_check_start = tokio::time::Instant::now() + *WALL_CLOCK_CHECK_INTERVAL;
171+
let mut wall_clock_check =
172+
tokio::time::interval_at(wall_clock_check_start, *WALL_CLOCK_CHECK_INTERVAL);
173+
159174
let tls_acceptor = TlsAcceptor::from(Arc::new(config));
160175
let tcp_listener = TcpListener::bind(socket_addr)
161176
.await
@@ -207,6 +222,21 @@ impl TlsServer {
207222
.context(RotateCertificateSnafu)?
208223
}
209224

225+
// Wall-clock check: detect if the certificate is near expiry
226+
// even though the monotonic rotation interval hasn't fired yet.
227+
// This handles clock drift from hibernation, VM migration, etc.
228+
_ = wall_clock_check.tick() => {
229+
if cert_resolver.needs_rotation(*WALL_CLOCK_EXPIRY_BUFFER) {
230+
tracing::info!("wall-clock check detected certificate near expiry, rotating early");
231+
cert_resolver
232+
.rotate_certificate()
233+
.await
234+
.context(RotateCertificateSnafu)?;
235+
// Reset the monotonic interval so we don't double-rotate
236+
interval.reset();
237+
}
238+
}
239+
210240
// This is cancellation-safe. If cancelled, no new connections are accepted.
211241
tcp_connection = tcp_listener.accept() => {
212242
let (tcp_stream, remote_addr) = match tcp_connection {

0 commit comments

Comments
 (0)