From 8873b2c70ce81dea800cf802bb89aba833b66d16 Mon Sep 17 00:00:00 2001 From: Draco-Lunaris-Echo Date: Tue, 2 Jun 2026 15:16:44 -0500 Subject: [PATCH] fix(security): harden enrollment PKI bundle retrieval (#12) - Add single-retrieval semantics: approved PKI bundles are atomically removed from the in-memory cache on first retrieval via DashMap::remove(), preventing concurrent requests from obtaining the private key - Add TTL expiry: ApprovedEntry wraps PkiBundle with approved_at and ttl fields; bundles expire after ENROLLMENT_BUNDLE_TTL_SECS (600s / 10 min) - Replace brute-force clear() purge with TTL-based retain() in background task, running every 60s instead of every 600s - Audit tracing calls: confirm no raw polling token is logged; add security comment documenting this policy - Document CSR-based enrollment as future enhancement in both enrollment.rs and SECURITY.md, explaining why server-generated keys are used currently --- SECURITY.md | 22 ++++++++++++ crates/pm-core/src/models.rs | 38 +++++++++++++++++++++ crates/pm-web/src/main.rs | 22 ++++++++---- crates/pm-web/src/routes/enrollment.rs | 46 ++++++++++++++++++++------ 4 files changed, 112 insertions(+), 16 deletions(-) mode change 100755 => 100644 crates/pm-core/src/models.rs diff --git a/SECURITY.md b/SECURITY.md index 52106ad..1d01bbd 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -41,6 +41,28 @@ This project is a security tool — we hold ourselves to a high standard: - **CI enforcement**: All PRs require passing CI checks (fmt, clippy, test, audit, build) - **Dependency auditing**: `cargo audit` runs in CI to catch known vulnerabilities +## Enrollment PKI Design Decisions + +### Server-Generated Keys vs CSR-Based Enrollment + +Currently, the server generates the agent's private key during enrollment approval and +transmits it over the mTLS-secured polling endpoint. This approach was chosen for +initial implementation simplicity — the agent polls a single endpoint and receives a +complete PKI bundle without an extra round-trip. + +**Mitigations in place:** +- The PKI bundle is stored in an in-memory cache with single-retrieval semantics — + it can only be fetched once and is atomically removed on retrieval. +- A 10-minute TTL ensures the bundle expires even if never retrieved. +- The raw polling token is never logged; only its SHA-256 hash is stored. + +**Future direction:** A CSR-based enrollment flow should replace server-generated keys. +Under that model, the agent generates its own key pair locally and submits a Certificate +Signing Request, eliminating the need for the server to ever hold or transmit the agent's +private key. This significantly reduces the attack surface. + +See: [Issue #9](https://github.com/Draco-Lunaris/Linux-Patch-Manager/issues/9) + ## Credit Contributors who responsibly report vulnerabilities will be credited in the corresponding GitHub Security Advisory. diff --git a/crates/pm-core/src/models.rs b/crates/pm-core/src/models.rs old mode 100755 new mode 100644 index e70f960..2eb9c05 --- a/crates/pm-core/src/models.rs +++ b/crates/pm-core/src/models.rs @@ -180,6 +180,44 @@ pub struct PkiBundle { pub server_key: String, } +/// Time-to-live for approved enrollment PKI bundles (10 minutes). +/// +/// After approval, the agent has this duration to retrieve its PKI bundle +/// via the polling endpoint. Once retrieved (single-use) or expired, +/// the bundle is permanently removed from the in-memory cache. +/// +/// This TTL balances security (limiting private key exposure in memory) +/// against reliability (giving agents enough time to poll after approval). +pub const ENROLLMENT_BUNDLE_TTL_SECS: u32 = 600; // 10 minutes + +/// An approved enrollment PKI bundle awaiting single-use retrieval. +/// +/// Stored in the in-memory cache between admin approval and agent pickup. +/// The entry is removed atomically on first retrieval and expires after +/// the configured TTL, whichever comes first. +#[derive(Debug, Clone)] +pub struct ApprovedEntry { + pub pki: PkiBundle, + pub approved_at: chrono::DateTime, + pub ttl: chrono::Duration, +} + +impl ApprovedEntry { + /// Create a new entry with the current timestamp and default TTL. + pub fn new(pki: PkiBundle) -> Self { + Self { + pki, + approved_at: Utc::now(), + ttl: chrono::Duration::seconds(ENROLLMENT_BUNDLE_TTL_SECS as i64), + } + } + + /// Returns true if this entry has exceeded its TTL. + pub fn is_expired(&self) -> bool { + Utc::now() > self.approved_at + self.ttl + } +} + // ============================================================ // Health Checks // ============================================================ diff --git a/crates/pm-web/src/main.rs b/crates/pm-web/src/main.rs index 99722db..af4c537 100644 --- a/crates/pm-web/src/main.rs +++ b/crates/pm-web/src/main.rs @@ -10,7 +10,7 @@ use pm_auth::{ rbac::{require_auth, AuthConfig}, }; use pm_core::{ - config::AppConfig, db, logging, models::PkiBundle, request_id::request_id_middleware, + config::AppConfig, db, logging, models::ApprovedEntry, request_id::request_id_middleware, }; use routes::sso::{OidcCache, SsoSession}; use routes::ws::WsTicket; @@ -41,7 +41,10 @@ pub struct AppState { /// Internal certificate authority for mTLS client cert issuance. pub ca: Arc, /// Short-lived cache for approved enrollment PKI bundles. - pub approved_enrollments: Arc>, + /// + /// Entries are single-use (removed on retrieval) and expire after + /// [`ENROLLMENT_BUNDLE_TTL_SECS`](pm_core::models::ENROLLMENT_BUNDLE_TTL_SECS). + pub approved_enrollments: Arc>, } #[tokio::main] @@ -101,7 +104,7 @@ async fn main() -> anyhow::Result<()> { let ws_tickets: Arc> = Arc::new(DashMap::new()); let sso_sessions: Arc> = Arc::new(DashMap::new()); let oidc_cache: Arc> = Arc::new(Mutex::new(OidcCache::default())); - let approved_enrollments: Arc> = Arc::new(DashMap::new()); + let approved_enrollments: Arc> = Arc::new(DashMap::new()); // Background task: purge expired WS tickets every 30 seconds. { @@ -140,14 +143,21 @@ async fn main() -> anyhow::Result<()> { }); } - // Background task: purge approved enrollment PKI bundles every 10 minutes. + // Background task: purge expired approved enrollment PKI bundles. + // Entries are also removed on first retrieval (single-use), so this + // task only cleans up bundles that were never picked up by the agent. { let approved = approved_enrollments.clone(); tokio::spawn(async move { - let mut interval = tokio::time::interval(Duration::from_secs(600)); + let mut interval = tokio::time::interval(Duration::from_secs(60)); loop { interval.tick().await; - approved.clear(); + let before = approved.len(); + approved.retain(|_, entry| !entry.is_expired()); + let removed = before.saturating_sub(approved.len()); + if removed > 0 { + tracing::debug!(removed, "Purged expired enrollment PKI bundles"); + } } }); } diff --git a/crates/pm-web/src/routes/enrollment.rs b/crates/pm-web/src/routes/enrollment.rs index 710fcc7..eef0a5b 100644 --- a/crates/pm-web/src/routes/enrollment.rs +++ b/crates/pm-web/src/routes/enrollment.rs @@ -11,7 +11,8 @@ use pm_auth::AuthUser; use pm_core::{ db, models::{ - CreateEnrollmentRequest, EnrollmentRequest, EnrollmentStatusResponse, Host, PkiBundle, + ApprovedEntry, CreateEnrollmentRequest, EnrollmentRequest, EnrollmentStatusResponse, Host, + PkiBundle, }, }; use rand::{distributions::Alphanumeric, Rng}; @@ -76,7 +77,10 @@ async fn enroll_status( State(state): State, Path(token): Path, ) -> Result, (StatusCode, Json)> { - // Hash the provided token to match DB + // Hash the provided token to match DB. + // Security note: the raw polling token is intentionally never logged. + // Only the SHA-256 hash is stored and compared; all tracing calls in + // this module log error contexts only, never the token itself. use sha2::{Digest, Sha256}; let mut hasher = Sha256::new(); hasher.update(token.as_bytes()); @@ -98,11 +102,17 @@ async fn enroll_status( } // 2. If not in pending, check if it was recently approved. - if let Some(pki) = state.approved_enrollments.get(&token_hash) { + // Single-retrieval: remove() atomically consumes the entry, ensuring + // the private key can only be fetched once regardless of concurrent requests. + if let Some((_, entry)) = state.approved_enrollments.remove(&token_hash) { + if entry.is_expired() { + // Bundle TTL expired — treat as not found. Entry is already removed. + return Ok(Json(EnrollmentStatusResponse::NotFound)); + } return Ok(Json(EnrollmentStatusResponse::Approved { - ca_crt: pki.ca_crt.clone(), - server_crt: pki.server_crt.clone(), - server_key: pki.server_key.clone(), + ca_crt: entry.pki.ca_crt.clone(), + server_crt: entry.pki.server_crt.clone(), + server_key: entry.pki.server_key.clone(), })); } @@ -277,15 +287,31 @@ async fn approve_enrollment( ) })?; - // Store PKI bundle in cache for client retrieval + // Store PKI bundle in cache for single-use client retrieval. + // + // Design decision — server-generated keys vs CSR-based enrollment: + // Currently the server generates the agent's private key and transmits it + // over the (already mTLS-secured) polling endpoint. This approach was chosen + // for initial implementation simplicity: the agent only needs to poll one + // endpoint and receives a complete PKI bundle without an extra round-trip. + // + // A future enhancement should adopt CSR-based enrollment where the agent + // generates its own key pair locally and submits a Certificate Signing + // Request, eliminating the need for the server to ever hold or transmit + // the agent's private key. This reduces the attack surface significantly + // — the private key never traverses the network and never resides in + // server memory beyond the signing operation. + // + // See: https://github.com/Draco-Lunaris/Linux-Patch-Manager/issues/9 let pki = PkiBundle { ca_crt: issued.ca_root_pem, server_crt: issued.server_cert_pem, server_key: issued.server_key_pem, }; - state - .approved_enrollments - .insert(enrollment_request.polling_token.clone(), pki); + state.approved_enrollments.insert( + enrollment_request.polling_token.clone(), + ApprovedEntry::new(pki), + ); Ok(StatusCode::OK) }