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

Author: lfrancke

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

@@ -1,4 +1,7 @@
-use std::sync::Arc;
+use std::{
+    sync::Arc,
+    time::SystemTime,
+};
 
 use arc_swap::ArcSwap;
 use snafu::{OptionExt, ResultExt, Snafu};
@@ -57,6 +60,9 @@ pub struct CertificateResolver {
     /// Using a [`ArcSwap`] (over e.g. [`tokio::sync::RwLock`]), so that we can easily
     /// (and performant) bridge between async write and sync read.
     current_certified_key: ArcSwap<CertifiedKey>,
+    /// The wall-clock expiry time (`not_after`) of the current certificate.
+    /// Used to detect clock drift between monotonic and wall-clock time.
+    current_not_after: ArcSwap<SystemTime>,
     subject_alterative_dns_names: Arc<Vec<String>>,
 
     certificate_tx: mpsc::Sender<Certificate>,
@@ -68,7 +74,7 @@ impl CertificateResolver {
         certificate_tx: mpsc::Sender<Certificate>,
     ) -> Result<Self> {
         let subject_alterative_dns_names = Arc::new(subject_alterative_dns_names);
-        let certified_key = Self::generate_new_certificate_inner(
+        let (certified_key, not_after) = Self::generate_new_certificate_inner(
             subject_alterative_dns_names.clone(),
             &certificate_tx,
         )
@@ -77,20 +83,33 @@ impl CertificateResolver {
         Ok(Self {
             subject_alterative_dns_names,
             current_certified_key: ArcSwap::new(certified_key),
+            current_not_after: ArcSwap::new(Arc::new(not_after)),
             certificate_tx,
         })
     }
 
     pub async fn rotate_certificate(&self) -> Result<()> {
-        let certified_key = self.generate_new_certificate().await?;
+        let (certified_key, not_after) = self.generate_new_certificate().await?;
 
         // TODO: Sign the new cert somehow with the old cert. See https://github.com/stackabletech/decisions/issues/56
         self.current_certified_key.store(certified_key);
+        self.current_not_after.store(Arc::new(not_after));
 
         Ok(())
     }
 
-    async fn generate_new_certificate(&self) -> Result<Arc<CertifiedKey>> {
+    /// Returns `true` if the current certificate is expired or will expire
+    /// within the given `buffer` duration according to wall-clock time.
+    ///
+    /// This catches cases where the monotonic timer (used by `tokio::time`)
+    /// has drifted from wall-clock time, e.g. due to system hibernation.
+    pub fn needs_rotation(&self, buffer: std::time::Duration) -> bool {
+        let not_after = **self.current_not_after.load();
+        let deadline = not_after.checked_sub(buffer).unwrap_or(not_after);
+        SystemTime::now() >= deadline
+    }
+
+    async fn generate_new_certificate(&self) -> Result<(Arc<CertifiedKey>, SystemTime)> {
         let subject_alterative_dns_names = self.subject_alterative_dns_names.clone();
         Self::generate_new_certificate_inner(subject_alterative_dns_names, &self.certificate_tx)
             .await
@@ -106,7 +125,7 @@ impl CertificateResolver {
     async fn generate_new_certificate_inner(
         subject_alterative_dns_names: Arc<Vec<String>>,
         certificate_tx: &mpsc::Sender<Certificate>,
-    ) -> Result<Arc<CertifiedKey>> {
+    ) -> Result<(Arc<CertifiedKey>, SystemTime)> {
         // The certificate generations can take a while, so we use `spawn_blocking`
         let (cert, certified_key) = tokio::task::spawn_blocking(move || {
             let tls_provider =
@@ -144,12 +163,18 @@ impl CertificateResolver {
         .await
         .context(TokioSpawnBlockingSnafu)??;
 
+        let not_after = cert
+            .tbs_certificate
+            .validity
+            .not_after
+            .to_system_time();
+
         certificate_tx
             .send(cert)
             .await
             .map_err(|_err| CertificateResolverError::SendCertificateToChannel)?;
 
-        Ok(certified_key)
+        Ok((certified_key, not_after))
     }
 }
 

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

@@ -42,6 +42,13 @@ pub const WEBHOOK_CA_LIFETIME: Duration = Duration::from_hours_unchecked(24);
 pub const WEBHOOK_CERTIFICATE_LIFETIME: Duration = Duration::from_hours_unchecked(24);
 pub const WEBHOOK_CERTIFICATE_ROTATION_INTERVAL: Duration = Duration::from_hours_unchecked(20);
 
+/// How often to check wall-clock time for certificate expiry.
+/// This catches clock drift from system hibernation, VM migration, etc.
+const WALL_CLOCK_CHECK_INTERVAL: std::time::Duration = std::time::Duration::from_secs(5 * 60);
+
+/// Rotate the certificate when it is within this buffer of expiry (wall-clock).
+const WALL_CLOCK_EXPIRY_BUFFER: std::time::Duration = std::time::Duration::from_secs(4 * 60 * 60);
+
 pub type Result<T, E = TlsServerError> = std::result::Result<T, E>;
 
 #[derive(Debug, Snafu)]
@@ -156,6 +163,12 @@ impl TlsServer {
         let start = tokio::time::Instant::now() + *WEBHOOK_CERTIFICATE_ROTATION_INTERVAL;
         let mut interval = tokio::time::interval_at(start, *WEBHOOK_CERTIFICATE_ROTATION_INTERVAL);
 
+        // A separate, shorter interval to check wall-clock time against the
+        // certificate's not_after. This catches monotonic/wall-clock drift
+        // caused by hibernation, VM migration, cgroup freezing, etc.
+        let mut wall_clock_check =
+            tokio::time::interval(tokio::time::Duration::from(WALL_CLOCK_CHECK_INTERVAL));
+
         let tls_acceptor = TlsAcceptor::from(Arc::new(config));
         let tcp_listener = TcpListener::bind(socket_addr)
             .await
@@ -207,6 +220,21 @@ impl TlsServer {
                         .context(RotateCertificateSnafu)?
                 }
 
+                // Wall-clock check: detect if the certificate is near expiry
+                // even though the monotonic rotation interval hasn't fired yet.
+                // This handles clock drift from hibernation, VM migration, etc.
+                _ = wall_clock_check.tick() => {
+                    if cert_resolver.needs_rotation(WALL_CLOCK_EXPIRY_BUFFER) {
+                        tracing::info!("wall-clock check detected certificate near expiry, rotating early");
+                        cert_resolver
+                            .rotate_certificate()
+                            .await
+                            .context(RotateCertificateSnafu)?;
+                        // Reset the monotonic interval so we don't double-rotate
+                        interval.reset();
+                    }
+                }
+
                 // This is cancellation-safe. If cancelled, no new connections are accepted.
                 tcp_connection = tcp_listener.accept() => {
                     let (tcp_stream, remote_addr) = match tcp_connection {