feat(agent): implement certificate renewal (#1775)

irvingoujAtDevolution · web-flow · commit 6f692e75a7eb · 2026-05-21T02:52:13.000+09:00
diff --git a/crates/agent-tunnel/src/listener.rs b/crates/agent-tunnel/src/listener.rs
@@ -105,6 +105,7 @@ pub struct AgentTunnelListener {
     endpoint: quinn::Endpoint,
     registry: Arc<AgentRegistry>,
     agent_connections: Arc<RwLock<HashMap<Uuid, quinn::Connection>>>,
+    ca_manager: Arc<CaManager>,
 }
 
 impl AgentTunnelListener {
@@ -158,6 +159,7 @@ impl AgentTunnelListener {
             endpoint,
             registry,
             agent_connections,
+            ca_manager,
         };
 
         Ok((listener, handle))
@@ -198,8 +200,9 @@ impl devolutions_gateway_task::Task for AgentTunnelListener {
 
                     let registry = Arc::clone(&self.registry);
                     let agent_connections = Arc::clone(&self.agent_connections);
+                    let ca_manager = Arc::clone(&self.ca_manager);
 
-                    conn_handles.spawn(run_agent_connection(registry, agent_connections, incoming));
+                    conn_handles.spawn(run_agent_connection(registry, agent_connections, ca_manager, incoming));
                 }
 
                 // Reap completed connection tasks to prevent unbounded growth.
@@ -220,6 +223,7 @@ impl devolutions_gateway_task::Task for AgentTunnelListener {
 async fn run_agent_connection(
     registry: Arc<AgentRegistry>,
     agent_connections: Arc<RwLock<HashMap<Uuid, quinn::Connection>>>,
+    ca_manager: Arc<CaManager>,
     incoming: quinn::Incoming,
 ) {
     let peer_addr = incoming.remote_address();
@@ -253,7 +257,7 @@ async fn run_agent_connection(
         agent_connections.write().await.insert(agent_id, conn.clone());
 
         // Accept the first bidirectional stream as the control stream.
-        let control_result = run_control_loop(&conn, agent_id, &registry).await;
+        let control_result = run_control_loop(&conn, agent_id, &agent_name, &registry, &ca_manager).await;
 
         // Agent disconnected — clean up.
         info!(%agent_id, "Agent QUIC connection closed");
@@ -269,7 +273,13 @@ async fn run_agent_connection(
     }
 }
 
-async fn run_control_loop(conn: &quinn::Connection, agent_id: Uuid, registry: &AgentRegistry) -> anyhow::Result<()> {
+async fn run_control_loop(
+    conn: &quinn::Connection,
+    agent_id: Uuid,
+    agent_name: &str,
+    registry: &AgentRegistry,
+    ca_manager: &CaManager,
+) -> anyhow::Result<()> {
     let mut ctrl: ControlStream<_, _> = conn.accept_bi().await.context("accept control stream")?.into();
 
     info!(%agent_id, "Control stream accepted");
@@ -290,7 +300,7 @@ async fn run_control_loop(conn: &quinn::Connection, agent_id: Uuid, registry: &A
                     }
                 };
 
-                handle_control_message(registry, agent_id, &mut ctrl, msg).await;
+                handle_control_message(registry, ca_manager, agent_id, agent_name, &mut ctrl, msg).await;
             }
 
             // Detect connection close.
@@ -306,7 +316,9 @@ async fn run_control_loop(conn: &quinn::Connection, agent_id: Uuid, registry: &A
 
 async fn handle_control_message<S: tokio::io::AsyncWrite + Unpin, R: tokio::io::AsyncRead + Unpin>(
     registry: &AgentRegistry,
+    ca_manager: &CaManager,
     agent_id: Uuid,
+    agent_name: &str,
     ctrl: &mut ControlStream<S, R>,
     msg: ControlMessage,
 ) {
@@ -358,8 +370,36 @@ async fn handle_control_message<S: tokio::io::AsyncWrite + Unpin, R: tokio::io::
         ControlMessage::HeartbeatAck { .. } => {
             debug!(%agent_id, "Unexpected HeartbeatAck from agent");
         }
-        ControlMessage::CertRenewalRequest { .. } | ControlMessage::CertRenewalResponse { .. } => {
-            debug!(%agent_id, "Certificate renewal not supported in this build");
+        ControlMessage::CertRenewalRequest { csr_pem, .. } => {
+            info!(%agent_id, "Agent requested certificate renewal");
+
+            // Reuse the agent_id and agent_name authenticated by mTLS — never
+            // trust the CSR's subject. The CA only re-signs the public key the
+            // agent put in its CSR; identity stays whatever the existing cert
+            // already proved during the handshake.
+            let result = match ca_manager.sign_agent_csr(agent_id, agent_name, &csr_pem, None) {
+                Ok(signed) => {
+                    info!(%agent_id, %agent_name, "Renewed agent certificate");
+                    agent_tunnel_proto::CertRenewalResult::Success {
+                        client_cert_pem: signed.client_cert_pem,
+                        gateway_ca_cert_pem: signed.ca_cert_pem,
+                    }
+                }
+                Err(error) => {
+                    warn!(%agent_id, error = %format!("{error:#}"), "Certificate renewal failed");
+                    agent_tunnel_proto::CertRenewalResult::Error {
+                        reason: format!("{error:#}"),
+                    }
+                }
+            };
+
+            let response = ControlMessage::cert_renewal_response(result);
+            let _ = ctrl.send(&response).await.inspect_err(|e| {
+                warn!(%agent_id, error = %e, "Failed to send CertRenewalResponse");
+            });
+        }
+        ControlMessage::CertRenewalResponse { .. } => {
+            debug!(%agent_id, "Unexpected CertRenewalResponse from agent");
         }
     }
 }
diff --git a/devolutions-agent/src/enrollment.rs b/devolutions-agent/src/enrollment.rs
@@ -245,6 +245,78 @@ fn persist_enrollment_response(
     })
 }
 
+// ---------------------------------------------------------------------------
+// Certificate renewal helpers
+// ---------------------------------------------------------------------------
+
+/// Check whether the PEM-encoded certificate at `cert_path` expires within
+/// `threshold_days`. The agent uses this on every reconnect to decide whether
+/// to ask the gateway for a new certificate before opening real traffic.
+pub fn is_cert_expiring(cert_path: &Utf8Path, threshold_days: u32) -> Result<bool> {
+    use std::io::BufReader;
+
+    let pem_str = std::fs::read_to_string(cert_path).with_context(|| format!("read certificate from {cert_path}"))?;
+    let der = rustls_pemfile::certs(&mut BufReader::new(pem_str.as_bytes()))
+        .next()
+        .context("empty PEM input")?
+        .context("parse certificate PEM")?;
+    let (_, cert) =
+        x509_parser::parse_x509_certificate(&der).map_err(|e| anyhow::anyhow!("parse X.509 certificate: {e}"))?;
+
+    let not_after_epoch = cert.validity().not_after.timestamp();
+    let now_epoch = i64::try_from(
+        std::time::SystemTime::now()
+            .duration_since(std::time::UNIX_EPOCH)
+            .context("system clock before UNIX epoch")?
+            .as_secs(),
+    )
+    .context("unix timestamp exceeds i64::MAX")?;
+
+    let threshold_secs = i64::from(threshold_days) * 86_400;
+    Ok(not_after_epoch - now_epoch <= threshold_secs)
+}
+
+/// Extract the `CommonName` from an existing PEM certificate. The renewal CSR
+/// must reuse the agent's name across renewals — the gateway looks the agent
+/// up in its registry by that name, and the most authoritative source for it
+/// is the cert the gateway itself signed last time.
+pub fn read_agent_name_from_cert(cert_path: &Utf8Path) -> Result<String> {
+    use std::io::BufReader;
+
+    let pem_str = std::fs::read_to_string(cert_path).with_context(|| format!("read certificate from {cert_path}"))?;
+    let der = rustls_pemfile::certs(&mut BufReader::new(pem_str.as_bytes()))
+        .next()
+        .context("empty PEM input")?
+        .context("parse certificate PEM")?;
+    let (_, cert) =
+        x509_parser::parse_x509_certificate(&der).map_err(|e| anyhow::anyhow!("parse X.509 certificate: {e}"))?;
+
+    let cn = cert
+        .subject()
+        .iter_common_name()
+        .next()
+        .context("certificate subject has no Common Name")?
+        .as_str()
+        .context("certificate Common Name is not valid UTF-8")?;
+
+    Ok(cn.to_owned())
+}
+
+/// Build a renewal CSR using the agent's existing private key. Reusing the key
+/// across renewals matches the design that says the private key never leaves
+/// the agent — the gateway only ever sees CSRs.
+pub fn generate_csr_from_existing_key(key_path: &Utf8Path, agent_name: &str) -> Result<String> {
+    let key_pem = std::fs::read_to_string(key_path).with_context(|| format!("read private key from {key_path}"))?;
+    let key_pair = rcgen::KeyPair::from_pem(&key_pem).context("parse private key PEM")?;
+
+    let mut params = rcgen::CertificateParams::default();
+    params.distinguished_name.push(rcgen::DnType::CommonName, agent_name);
+
+    let csr = params.serialize_request(&key_pair).context("serialize renewal CSR")?;
+
+    csr.pem().context("encode CSR to PEM")
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
diff --git a/devolutions-agent/src/tunnel.rs b/devolutions-agent/src/tunnel.rs
@@ -133,10 +133,17 @@ impl Task for TunnelTask {
             let start = std::time::Instant::now();
 
             match run_single_connection(&self.conf_handle, &mut shutdown_signal).await {
-                Ok(()) => {
+                Ok(ConnectionOutcome::Shutdown) => {
                     info!("Tunnel task stopped");
                     return Ok(());
                 }
+                Ok(ConnectionOutcome::CertRenewed) => {
+                    // Renewal is a successful "completion", not a failure — skip
+                    // the backoff and reconnect immediately with the new cert.
+                    info!("Certificate renewed; reconnecting with new cert immediately");
+                    backoff.reset();
+                    continue;
+                }
                 Err(error) => {
                     warn!(error = %format!("{error:#}"), "Tunnel connection lost");
                 }
@@ -175,11 +182,23 @@ impl Task for TunnelTask {
 // Single connection lifetime
 // ---------------------------------------------------------------------------
 
+/// Outcome of a single connection lifetime, telling the outer loop what to do next.
+enum ConnectionOutcome {
+    /// Shutdown signal received — exit the tunnel task cleanly.
+    Shutdown,
+    /// Certificate was renewed successfully; reconnect immediately with the new cert.
+    CertRenewed,
+}
+
 /// Run a single QUIC tunnel connection lifetime: config → connect → event loop.
 ///
-/// Returns `Ok(())` on graceful shutdown (shutdown signal received).
-/// Returns `Err(...)` on any failure — the caller should retry with backoff.
-async fn run_single_connection(conf_handle: &ConfHandle, shutdown_signal: &mut ShutdownSignal) -> anyhow::Result<()> {
+/// - `Ok(Shutdown)`: graceful shutdown, exit the task.
+/// - `Ok(CertRenewed)`: certificate renewed; caller should reconnect immediately.
+/// - `Err(...)`: connection lost or handshake failed — caller should retry with backoff.
+async fn run_single_connection(
+    conf_handle: &ConfHandle,
+    shutdown_signal: &mut ShutdownSignal,
+) -> anyhow::Result<ConnectionOutcome> {
     // Ensure rustls crypto provider is installed (ring).
     let _ = rustls::crypto::ring::default_provider().install_default();
 
@@ -356,6 +375,18 @@ async fn run_single_connection(conf_handle: &ConfHandle, shutdown_signal: &mut S
 
     info!(epoch, "Sent initial RouteAdvertise");
 
+    // -- Certificate renewal (post-connect, pre-traffic) --
+    //
+    // Run once per reconnect rather than on a periodic timer: the QUIC session
+    // has a 120s idle timeout and 15s keep-alive, so any blip / VPN reconnect
+    // / host sleep / gateway restart drops the connection within minutes and
+    // sends us back through this path. With a 1-year cert and a 15-day
+    // threshold, the renewal window will be hit on the first reconnect after
+    // T-15d, which is more than often enough in any real deployment.
+    if let Some(outcome) = try_renew_certificate(&mut ctrl, &connection, cert_path, key_path, ca_path).await? {
+        return Ok(outcome);
+    }
+
     // Split: recv half goes to a reader task, send half stays for periodic messages.
     let (mut ctrl_send, ctrl_recv) = ctrl.into_split();
     let mut task_handles = tokio::task::JoinSet::new();
@@ -409,7 +440,102 @@ async fn run_single_connection(conf_handle: &ConfHandle, shutdown_signal: &mut S
 
     task_handles.shutdown().await;
 
-    Ok(())
+    Ok(ConnectionOutcome::Shutdown)
+}
+
+// ---------------------------------------------------------------------------
+// Certificate renewal
+// ---------------------------------------------------------------------------
+
+/// Check if the client cert is near expiry; if so, renew it via the control
+/// stream before opening real traffic.
+///
+/// Returns:
+/// - `Ok(Some(CertRenewed))` — renewed successfully; outer loop must reconnect
+///   so the new cert takes effect on the next mTLS handshake.
+/// - `Ok(None)` — no renewal needed (or attempted renewal failed in a recoverable
+///   way, e.g. the gateway said no); proceed with the existing cert.
+/// - `Err(_)` — IO / protocol error on the control stream itself; treat as
+///   connection lost.
+async fn try_renew_certificate<S, R>(
+    ctrl: &mut ControlStream<S, R>,
+    connection: &quinn::Connection,
+    cert_path: &camino::Utf8Path,
+    key_path: &camino::Utf8Path,
+    ca_path: &camino::Utf8Path,
+) -> anyhow::Result<Option<ConnectionOutcome>>
+where
+    S: tokio::io::AsyncWrite + Unpin,
+    R: tokio::io::AsyncRead + Unpin,
+{
+    const RENEWAL_THRESHOLD_DAYS: u32 = 15;
+    const RENEWAL_TIMEOUT: Duration = Duration::from_secs(30);
+
+    match crate::enrollment::is_cert_expiring(cert_path, RENEWAL_THRESHOLD_DAYS) {
+        Ok(false) => {
+            debug!("Client certificate not in renewal window");
+            return Ok(None);
+        }
+        Err(error) => {
+            warn!(error = %format!("{error:#}"), "Failed to check certificate expiry; skipping renewal");
+            return Ok(None);
+        }
+        Ok(true) => {}
+    }
+
+    info!(
+        threshold_days = RENEWAL_THRESHOLD_DAYS,
+        "Certificate within renewal window; requesting renewal"
+    );
+
+    // Reuse the agent name from the existing cert as the renewal CSR's
+    // CommonName. The gateway ignores CSR subject and trusts the
+    // mTLS-authenticated identity, but matching the existing CN keeps the
+    // CSR semantically correct in case validation tightens later.
+    let agent_name = crate::enrollment::read_agent_name_from_cert(cert_path)
+        .context("read agent name from existing certificate for renewal")?;
+    let csr_pem =
+        crate::enrollment::generate_csr_from_existing_key(key_path, &agent_name).context("generate renewal CSR")?;
+
+    ctrl.send(&ControlMessage::cert_renewal_request(csr_pem))
+        .await
+        .context("send CertRenewalRequest")?;
+
+    let response = tokio::time::timeout(RENEWAL_TIMEOUT, ctrl.recv())
+        .await
+        .context("timeout waiting for CertRenewalResponse")?
+        .context("receive CertRenewalResponse")?;
+
+    match response {
+        ControlMessage::CertRenewalResponse {
+            result:
+                agent_tunnel_proto::CertRenewalResult::Success {
+                    client_cert_pem,
+                    gateway_ca_cert_pem,
+                },
+            ..
+        } => {
+            std::fs::write(cert_path.as_str(), &client_cert_pem).context("write renewed certificate")?;
+            std::fs::write(ca_path.as_str(), &gateway_ca_cert_pem).context("write renewed CA certificate")?;
+            info!("Certificate renewed; closing connection so new cert takes effect on reconnect");
+            connection.close(0u32.into(), b"cert-renewed");
+            Ok(Some(ConnectionOutcome::CertRenewed))
+        }
+        ControlMessage::CertRenewalResponse {
+            result: agent_tunnel_proto::CertRenewalResult::Error { reason },
+            ..
+        } => {
+            warn!(%reason, "Gateway refused certificate renewal; continuing with existing cert");
+            Ok(None)
+        }
+        other => {
+            warn!(
+                ?other,
+                "Unexpected response to renewal request; continuing with existing cert"
+            );
+            Ok(None)
+        }
+    }
 }
 
 // ---------------------------------------------------------------------------
