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

@@ -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.

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<Utc>,
+    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
 // ============================================================

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<pm_ca::CertAuthority>,
     /// Short-lived cache for approved enrollment PKI bundles.
-    pub approved_enrollments: Arc<DashMap<String, PkiBundle>>,
+    ///
+    /// 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<DashMap<String, ApprovedEntry>>,
 }
 
 #[tokio::main]
@@ -101,7 +104,7 @@ async fn main() -> anyhow::Result<()> {
     let ws_tickets: Arc<DashMap<String, WsTicket>> = Arc::new(DashMap::new());
     let sso_sessions: Arc<DashMap<String, SsoSession>> = Arc::new(DashMap::new());
     let oidc_cache: Arc<Mutex<OidcCache>> = Arc::new(Mutex::new(OidcCache::default()));
-    let approved_enrollments: Arc<DashMap<String, PkiBundle>> = Arc::new(DashMap::new());
+    let approved_enrollments: Arc<DashMap<String, ApprovedEntry>> = 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");
+                }
             }
         });
     }

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<AppState>,
     Path(token): Path<String>,
 ) -> Result<Json<EnrollmentStatusResponse>, (StatusCode, Json<serde_json::Value>)> {
-    // 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)
 }