diff --git a/apps/gateway/Cargo.lock b/apps/gateway/Cargo.lock index 94993dc4..7f528f49 100644 --- a/apps/gateway/Cargo.lock +++ b/apps/gateway/Cargo.lock @@ -2827,6 +2827,7 @@ dependencies = [ "ring", "rustls-pki-types", "time", + "x509-parser", "yasna", ] @@ -4713,6 +4714,7 @@ dependencies = [ "lazy_static", "nom", "oid-registry", + "ring", "rusticata-macros", "thiserror 1.0.69", "time", diff --git a/apps/gateway/Cargo.toml b/apps/gateway/Cargo.toml index 3d2c37c4..4e59f9f1 100644 --- a/apps/gateway/Cargo.toml +++ b/apps/gateway/Cargo.toml @@ -32,8 +32,10 @@ x509-parser = "0.16" # WebSocket upstream TLS root certificates webpki-roots = "0.26" -# Certificate generation -rcgen = "0.13" +# Certificate generation. `x509-parser` turns on +# `CertificateSigningRequestParams::from_pem` (feature-gated, NOT in +# defaults) — required to parse+verify inbound CSRs in `client_ca_authority.rs`. +rcgen = { version = "0.13", features = ["x509-parser"] } # Cryptography (AES-256-GCM for secret decryption) ring = "0.17" diff --git a/apps/gateway/src/client_ca.rs b/apps/gateway/src/client_ca.rs index 2800ae57..fccccedd 100644 --- a/apps/gateway/src/client_ca.rs +++ b/apps/gateway/src/client_ca.rs @@ -259,16 +259,25 @@ impl MtlsConfig { /// [`pem_from_value`]. `mitm_ca_der` is the gateway's own MITM CA /// certificate (see `ca.rs`); `plain_port` is the plaintext listener port. /// + /// `fallback_client_ca_pem` (Phase 2): when `ca` (`GATEWAY_CLIENT_CA`) is + /// unset, this PEM is used as the client-CA trust anchor instead of + /// erroring — it's `main`'s generated/loaded `client_ca_authority::ClientCa` + /// certificate, so a fresh OSS install trusts (and can mint against) its + /// own client CA with zero configuration. An explicit `GATEWAY_CLIENT_CA` + /// always wins over this fallback: an operator who has configured their + /// own external trust anchor is never silently overridden. + /// /// `Ok(None)` means mTLS is off (port unset). Every other failure mode — - /// unparseable/zero/colliding port, missing material, unreadable/garbage - /// PEM, or a client CA that IS the MITM CA — is `Err`, and the caller - /// (`main`) must fail closed: never fall back to plaintext-only when mTLS - /// was requested but couldn't be built. + /// unparseable/zero/colliding port, missing material (with no fallback to + /// cover it), unreadable/garbage PEM, or a client CA that IS the MITM CA — + /// is `Err`, and the caller (`main`) must fail closed: never fall back to + /// plaintext-only when mTLS was requested but couldn't be built. pub(crate) fn from_parts( port: Option<&str>, cert: Option<&str>, key: Option<&str>, ca: Option<&str>, + fallback_client_ca_pem: Option<&str>, mitm_ca_der: &CertificateDer<'static>, plain_port: u16, ) -> Result> { @@ -297,10 +306,25 @@ impl MtlsConfig { .context("GATEWAY_TLS_KEY is required when GATEWAY_MTLS_PORT is set") .and_then(|v| pem_from_value("GATEWAY_TLS_KEY", v))? .context("GATEWAY_TLS_KEY is required when GATEWAY_MTLS_PORT is set")?; - let ca_pem = ca - .context("GATEWAY_CLIENT_CA is required when GATEWAY_MTLS_PORT is set") - .and_then(|v| pem_from_value("GATEWAY_CLIENT_CA", v))? - .context("GATEWAY_CLIENT_CA is required when GATEWAY_MTLS_PORT is set")?; + + // GATEWAY_CLIENT_CA explicit env value takes precedence; only fall + // back to the generated authority's cert when it's genuinely unset + // (or set-but-empty, which `pem_from_value` already treats as unset). + let ca_from_env = match ca { + Some(v) => pem_from_value("GATEWAY_CLIENT_CA", v)?, + None => None, + }; + let ca_pem = match ca_from_env { + Some(pem) => pem, + None => fallback_client_ca_pem + .map(str::trim) + .filter(|s| !s.is_empty()) + .context( + "GATEWAY_CLIENT_CA is required when GATEWAY_MTLS_PORT is set (and no \ + generated client-CA authority is available as a fallback)", + )? + .to_string(), + }; // SECURITY: reject a client CA bundle that carries the same public // key as the gateway's own MITM CA. Compared on the DER-encoded @@ -340,10 +364,14 @@ impl MtlsConfig { } /// Read `GATEWAY_MTLS_PORT` / `GATEWAY_TLS_CERT` / `GATEWAY_TLS_KEY` / - /// `GATEWAY_CLIENT_CA` from the environment and forward to [`Self::from_parts`]. + /// `GATEWAY_CLIENT_CA` from the environment and forward to + /// [`Self::from_parts`], along with `fallback_client_ca_pem` (see its doc + /// there) which `main` supplies when it holds a generated + /// `client_ca_authority::ClientCa`. pub(crate) fn from_env( mitm_ca_der: &CertificateDer<'static>, plain_port: u16, + fallback_client_ca_pem: Option<&str>, ) -> Result> { let port = std::env::var("GATEWAY_MTLS_PORT").ok(); let cert = std::env::var("GATEWAY_TLS_CERT").ok(); @@ -354,19 +382,27 @@ impl MtlsConfig { cert.as_deref(), key.as_deref(), ca.as_deref(), + fallback_client_ca_pem, mitm_ca_der, plain_port, ) } } +/// Test-only PKI harness shared across this crate's test suites: a minimal +/// CA + leaf-signing helper on rcgen, plus a loopback mTLS handshake runner +/// built on this module's own `build_server_config`/`WebPkiClientVerifier`. +/// +/// `pub(crate)` and NOT nested inside `mod tests` below, so +/// `client_ca_authority`'s test module (Phase 2: minting client certs from a +/// CSR) can drive a real handshake against a freshly minted client +/// certificate through this same verifier, instead of duplicating the +/// harness. #[cfg(test)] -mod tests { +pub(crate) mod test_support { use super::*; - use std::io::Write; use std::net::SocketAddr; use std::sync::Once; - use std::time::{SystemTime, UNIX_EPOCH}; use rcgen::{ BasicConstraints, CertificateParams, DnType, IsCa, KeyPair, KeyUsagePurpose, @@ -379,134 +415,25 @@ mod tests { static INIT_CRYPTO: Once = Once::new(); - fn ensure_crypto_provider() { + pub(crate) fn ensure_crypto_provider() { INIT_CRYPTO.call_once(|| { - // Ignore the error: it just means another test module (e.g. - // `ca`) already installed the process-wide default in this same - // test binary — a no-op for our purposes either way, since it's - // the same `ring` provider. + // Ignore the error: it just means another test suite in this + // same test binary already installed the process-wide default — + // a no-op for our purposes either way, since it's the same + // `ring` provider. let _ = rustls::crypto::ring::default_provider().install_default(); }); } - // ── sanitize_identity_component ───────────────────────────────────── - - #[test] - fn sanitize_accepts_plain_values() { - assert_eq!( - sanitize_identity_component("agent-42"), - Some("agent-42".to_string()) - ); - assert_eq!( - sanitize_identity_component("spiffe://onecli/agent/42"), - Some("spiffe://onecli/agent/42".to_string()) - ); - } - - #[test] - fn sanitize_drops_empty() { - assert_eq!(sanitize_identity_component(""), None); - } - - #[test] - fn sanitize_drops_oversized() { - let long = "a".repeat(254); - assert_eq!(sanitize_identity_component(&long), None); - // 253 bytes is the boundary — still accepted. - let boundary = "a".repeat(253); - assert!(sanitize_identity_component(&boundary).is_some()); - } - - #[test] - fn sanitize_drops_newline_and_cr() { - assert_eq!(sanitize_identity_component("agent\n42"), None); - assert_eq!(sanitize_identity_component("agent\r42"), None); - } - - #[test] - fn sanitize_drops_quote_and_backslash() { - assert_eq!(sanitize_identity_component("agent\"42"), None); - assert_eq!(sanitize_identity_component("agent\\42"), None); - } - - #[test] - fn sanitize_drops_non_ascii() { - assert_eq!(sanitize_identity_component("agenté"), None); - } - - // ── ClientIdentity::primary ────────────────────────────────────────── - - #[test] - fn primary_prefers_uri_san_over_cn() { - let id = ClientIdentity { - cn: Some("fallback-cn".to_string()), - uri_sans: vec!["spiffe://onecli/agent/1".to_string()], - serial_hex: "ab".to_string(), - not_after_unix: 0, - }; - assert_eq!(id.primary(), Some("spiffe://onecli/agent/1")); - } - - #[test] - fn primary_falls_back_to_cn() { - let id = ClientIdentity { - cn: Some("cn-only".to_string()), - uri_sans: vec![], - serial_hex: "ab".to_string(), - not_after_unix: 0, - }; - assert_eq!(id.primary(), Some("cn-only")); - } - - #[test] - fn primary_none_when_both_missing() { - let id = ClientIdentity { - cn: None, - uri_sans: vec![], - serial_hex: "ab".to_string(), - not_after_unix: 0, - }; - assert_eq!(id.primary(), None); - } - - #[test] - fn display_uses_primary() { - let id = ClientIdentity { - cn: Some("cn-only".to_string()), - uri_sans: vec![], - serial_hex: "ab".to_string(), - not_after_unix: 0, - }; - assert_eq!(id.to_string(), "cn-only"); - } - - // ── identity_from_peer_certs: empty/malformed input never panics ──── - - #[test] - fn identity_from_empty_slice_is_none() { - assert_eq!(identity_from_peer_certs(&[]), None); - } - - #[test] - fn identity_from_garbage_der_is_none() { - let garbage = CertificateDer::from(vec![0u8, 1, 2, 3, 4]); - assert_eq!( - identity_from_peer_certs(std::slice::from_ref(&garbage)), - None - ); - } - - // ── test PKI helper ─────────────────────────────────────────────────── - /// A minimal CA + leaf-signing helper built on rcgen, mirroring the /// pattern in `ca.rs`'s own test module. - struct TestCa { - cert: rcgen::Certificate, - key: KeyPair, - der: CertificateDer<'static>, + pub(crate) struct TestCa { + pub(crate) cert: rcgen::Certificate, + pub(crate) key: KeyPair, + pub(crate) der: CertificateDer<'static>, } - fn new_test_ca(cn: &str) -> TestCa { + pub(crate) fn new_test_ca(cn: &str) -> TestCa { let key = KeyPair::generate_for(&PKCS_ECDSA_P256_SHA256).expect("CA key"); let mut params = CertificateParams::default(); params.is_ca = IsCa::Ca(BasicConstraints::Unconstrained); @@ -521,7 +448,7 @@ mod tests { /// Sign a client leaf under `ca`, valid `[not_before_h, not_after_h]` hours /// from now, with the given CN and URI SANs. Returns (cert_pem, key_pem). - fn sign_client_leaf( + pub(crate) fn sign_client_leaf( ca: &TestCa, cn: Option<&str>, uri_sans: &[&str], @@ -557,7 +484,7 @@ mod tests { /// Self-signed "server" cert for `localhost`, used as the mTLS listener's /// own identity in handshake tests. The test client trusts it directly /// (it's its own root), sidestepping the need for a fake server verifier. - fn self_signed_server_cert() -> (String, String, CertificateDer<'static>) { + pub(crate) fn self_signed_server_cert() -> (String, String, CertificateDer<'static>) { let key = KeyPair::generate_for(&PKCS_ECDSA_P256_SHA256).expect("server key"); let mut params = CertificateParams::new(vec!["localhost".to_string()]).expect("params"); params @@ -574,7 +501,9 @@ mod tests { /// Build a server config AND return the matching server cert DER — the /// two must come from the same `self_signed_server_cert()` call, since /// the test client trusts that DER directly as its only root. - fn test_server_setup(trusted_ca_pem: &str) -> (Arc, CertificateDer<'static>) { + pub(crate) fn test_server_setup( + trusted_ca_pem: &str, + ) -> (Arc, CertificateDer<'static>) { let (server_cert_pem, server_key_pem, server_der) = self_signed_server_cert(); let roots = load_client_ca_roots(trusted_ca_pem).expect("roots"); let config = @@ -582,7 +511,7 @@ mod tests { (config, server_der) } - fn test_client_config( + pub(crate) fn test_client_config( server_der: &CertificateDer<'static>, client_cert_pem: Option<&str>, client_key_pem: Option<&str>, @@ -612,7 +541,7 @@ mod tests { /// Returns the server-side accept result — the thing under test — and /// discards the client-side result beyond confirming it also failed when /// the server did (a rejected handshake fails both sides). - async fn attempt_handshake( + pub(crate) async fn attempt_handshake( server_config: Arc, client_config: Arc, ) -> std::io::Result> { @@ -635,11 +564,134 @@ mod tests { server_result } - fn err_debug_contains(err: &std::io::Error, needle: &str) -> bool { + pub(crate) fn err_debug_contains(err: &std::io::Error, needle: &str) -> bool { format!("{err:?}").contains(needle) } +} + +#[cfg(test)] +mod tests { + use super::test_support::*; + use super::*; + use std::io::Write; + use std::time::{SystemTime, UNIX_EPOCH}; + + use rcgen::{ + BasicConstraints, CertificateParams, DnType, IsCa, KeyPair, PKCS_ECDSA_P256_SHA256, + }; + use time::OffsetDateTime; + + // ── sanitize_identity_component ───────────────────────────────────── + + #[test] + fn sanitize_accepts_plain_values() { + assert_eq!( + sanitize_identity_component("agent-42"), + Some("agent-42".to_string()) + ); + assert_eq!( + sanitize_identity_component("spiffe://onecli/agent/42"), + Some("spiffe://onecli/agent/42".to_string()) + ); + } + + #[test] + fn sanitize_drops_empty() { + assert_eq!(sanitize_identity_component(""), None); + } + + #[test] + fn sanitize_drops_oversized() { + let long = "a".repeat(254); + assert_eq!(sanitize_identity_component(&long), None); + // 253 bytes is the boundary — still accepted. + let boundary = "a".repeat(253); + assert!(sanitize_identity_component(&boundary).is_some()); + } + + #[test] + fn sanitize_drops_newline_and_cr() { + assert_eq!(sanitize_identity_component("agent\n42"), None); + assert_eq!(sanitize_identity_component("agent\r42"), None); + } + + #[test] + fn sanitize_drops_quote_and_backslash() { + assert_eq!(sanitize_identity_component("agent\"42"), None); + assert_eq!(sanitize_identity_component("agent\\42"), None); + } + + #[test] + fn sanitize_drops_non_ascii() { + assert_eq!(sanitize_identity_component("agenté"), None); + } + + // ── ClientIdentity::primary ────────────────────────────────────────── + + #[test] + fn primary_prefers_uri_san_over_cn() { + let id = ClientIdentity { + cn: Some("fallback-cn".to_string()), + uri_sans: vec!["spiffe://onecli/agent/1".to_string()], + serial_hex: "ab".to_string(), + not_after_unix: 0, + }; + assert_eq!(id.primary(), Some("spiffe://onecli/agent/1")); + } + + #[test] + fn primary_falls_back_to_cn() { + let id = ClientIdentity { + cn: Some("cn-only".to_string()), + uri_sans: vec![], + serial_hex: "ab".to_string(), + not_after_unix: 0, + }; + assert_eq!(id.primary(), Some("cn-only")); + } + + #[test] + fn primary_none_when_both_missing() { + let id = ClientIdentity { + cn: None, + uri_sans: vec![], + serial_hex: "ab".to_string(), + not_after_unix: 0, + }; + assert_eq!(id.primary(), None); + } + + #[test] + fn display_uses_primary() { + let id = ClientIdentity { + cn: Some("cn-only".to_string()), + uri_sans: vec![], + serial_hex: "ab".to_string(), + not_after_unix: 0, + }; + assert_eq!(id.to_string(), "cn-only"); + } + + // ── identity_from_peer_certs: empty/malformed input never panics ──── + + #[test] + fn identity_from_empty_slice_is_none() { + assert_eq!(identity_from_peer_certs(&[]), None); + } + + #[test] + fn identity_from_garbage_der_is_none() { + let garbage = CertificateDer::from(vec![0u8, 1, 2, 3, 4]); + assert_eq!( + identity_from_peer_certs(std::slice::from_ref(&garbage)), + None + ); + } // ── Handshake behavior ──────────────────────────────────────────────── + // (TestCa, new_test_ca, sign_client_leaf, self_signed_server_cert, + // test_server_setup, test_client_config, attempt_handshake, and + // err_debug_contains now live in `test_support` above, glob-imported.) #[tokio::test] async fn handshake_rejects_missing_client_cert() { @@ -771,7 +823,8 @@ mod tests { fn from_parts_port_none_is_off() { ensure_crypto_provider(); let mitm_der = dummy_mitm_ca_der(); - let result = MtlsConfig::from_parts(None, None, None, None, &mitm_der, 10255).unwrap(); + let result = + MtlsConfig::from_parts(None, None, None, None, None, &mitm_der, 10255).unwrap(); assert!(result.is_none()); } @@ -793,6 +846,7 @@ mod tests { None, Some("key"), Some("ca"), + None, &mitm_der, 10255, ) @@ -809,6 +863,7 @@ mod tests { Some(PRESENT_PLACEHOLDER_PEM), None, Some(PRESENT_PLACEHOLDER_PEM), + None, &mitm_der, 10255, ) @@ -825,6 +880,7 @@ mod tests { Some(PRESENT_PLACEHOLDER_PEM), Some(PRESENT_PLACEHOLDER_PEM), None, + None, &mitm_der, 10255, ) @@ -836,9 +892,16 @@ mod tests { fn from_parts_zero_port_errs() { ensure_crypto_provider(); let mitm_der = dummy_mitm_ca_der(); - let err = - MtlsConfig::from_parts(Some("0"), Some("c"), Some("k"), Some("a"), &mitm_der, 10255) - .unwrap_err(); + let err = MtlsConfig::from_parts( + Some("0"), + Some("c"), + Some("k"), + Some("a"), + None, + &mitm_der, + 10255, + ) + .unwrap_err(); assert!(format!("{err:#}").contains("must not be 0")); } @@ -851,6 +914,7 @@ mod tests { Some("c"), Some("k"), Some("a"), + None, &mitm_der, 10255, ) @@ -867,6 +931,7 @@ mod tests { Some("c"), Some("k"), Some("a"), + None, &mitm_der, 10255, ) @@ -888,6 +953,7 @@ mod tests { Some(&server_cert_pem), Some(&server_key_pem), Some(&ca_pem), + None, &mitm_der, 10255, ) @@ -908,6 +974,7 @@ mod tests { Some(cert_path.to_str().unwrap()), Some(key_path.to_str().unwrap()), Some(ca_path.to_str().unwrap()), + None, &mitm_der, 10255, ) @@ -924,6 +991,7 @@ mod tests { Some("/nonexistent/path/cert.pem"), Some("/nonexistent/path/key.pem"), Some("/nonexistent/path/ca.pem"), + None, &mitm_der, 10255, ) @@ -941,6 +1009,7 @@ mod tests { Some(&server_cert_pem), Some(&server_key_pem), Some("-----BEGIN CERTIFICATE-----\nbm90IGEgY2VydA==\n-----END CERTIFICATE-----\n"), + None, &mitm_der, 10255, ) @@ -960,6 +1029,7 @@ mod tests { Some(&server_cert_pem), Some(&server_key_pem), Some(""), + None, &mitm_der, 10255, ) @@ -979,6 +1049,7 @@ mod tests { Some(&server_cert_pem), Some(&server_key_pem), Some(&mitm_ca.cert.pem()), + None, &mitm_ca.der, 10255, ) @@ -1029,6 +1100,7 @@ mod tests { Some(&server_cert_pem), Some(&server_key_pem), Some(&reissued_cert.pem()), + None, &mitm_der, 10255, ) @@ -1037,6 +1109,105 @@ mod tests { assert!(format!("{err:#}").contains("public")); } + // ── fallback_client_ca_pem (Phase 2: generated client-CA authority) ── + + /// When `GATEWAY_CLIENT_CA` is unset, the caller-supplied fallback (the + /// generated `client_ca_authority::ClientCa`'s own cert) is used instead + /// of erroring — a fresh install trusts its own generated client CA with + /// zero configuration. + #[test] + fn from_parts_uses_fallback_client_ca_when_env_unset() { + ensure_crypto_provider(); + let mitm_der = dummy_mitm_ca_der(); + let (server_cert_pem, server_key_pem, _) = self_signed_server_cert(); + let generated_client_ca = new_test_ca("Generated Client CA"); + let fallback_pem = generated_client_ca.cert.pem(); + + let result = MtlsConfig::from_parts( + Some("10256"), + Some(&server_cert_pem), + Some(&server_key_pem), + None, + Some(&fallback_pem), + &mitm_der, + 10255, + ) + .expect("fallback client CA must be accepted when GATEWAY_CLIENT_CA is unset"); + assert!(result.is_some()); + } + + /// An explicit `GATEWAY_CLIENT_CA` always wins over the fallback — an + /// operator's own configured trust anchor is never silently replaced by + /// the gateway's generated one. Proven end to end: the resolved + /// `server_config` accepts a leaf from the explicit CA and rejects one + /// from the fallback (generated) CA. + #[tokio::test] + async fn from_parts_explicit_client_ca_wins_over_fallback() { + ensure_crypto_provider(); + let mitm_der = dummy_mitm_ca_der(); + let (server_cert_pem, server_key_pem, server_der) = self_signed_server_cert(); + let explicit_ca = new_test_ca("Explicit Operator CA"); + let generated_client_ca = new_test_ca("Generated Client CA"); + + let mtls = MtlsConfig::from_parts( + Some("10256"), + Some(&server_cert_pem), + Some(&server_key_pem), + Some(&explicit_ca.cert.pem()), + Some(&generated_client_ca.cert.pem()), + &mitm_der, + 10255, + ) + .expect("explicit GATEWAY_CLIENT_CA must load") + .expect("mTLS configured"); + + // Rejected: signed by the fallback (generated) CA, which lost. + let (fallback_cert_pem, fallback_key_pem) = + sign_client_leaf(&generated_client_ca, Some("agent-1"), &[], -1, 24); + let fallback_client_config = test_client_config( + &server_der, + Some(&fallback_cert_pem), + Some(&fallback_key_pem), + ); + let rejected = + attempt_handshake(Arc::clone(&mtls.server_config), fallback_client_config).await; + assert!( + rejected.is_err(), + "a leaf from the losing fallback CA must be rejected" + ); + + // Accepted: signed by the explicit CA, which won. + let (explicit_cert_pem, explicit_key_pem) = + sign_client_leaf(&explicit_ca, Some("agent-1"), &[], -1, 24); + let explicit_client_config = test_client_config( + &server_der, + Some(&explicit_cert_pem), + Some(&explicit_key_pem), + ); + attempt_handshake(mtls.server_config, explicit_client_config) + .await + .expect("a leaf from the winning explicit CA must be accepted"); + } + + /// Both `GATEWAY_CLIENT_CA` and the fallback absent is still an error — + /// the fallback is a convenience, not a way to silently skip validation. + #[test] + fn from_parts_errs_when_both_ca_and_fallback_absent() { + ensure_crypto_provider(); + let mitm_der = dummy_mitm_ca_der(); + let err = MtlsConfig::from_parts( + Some("10256"), + Some(PRESENT_PLACEHOLDER_PEM), + Some(PRESENT_PLACEHOLDER_PEM), + None, + None, + &mitm_der, + 10255, + ) + .unwrap_err(); + assert!(format!("{err:#}").contains("GATEWAY_CLIENT_CA")); + } + // ── pem_from_env / pem_from_value ──────────────────────────────────── #[test] diff --git a/apps/gateway/src/client_ca_authority.rs b/apps/gateway/src/client_ca_authority.rs new file mode 100644 index 00000000..953915ec --- /dev/null +++ b/apps/gateway/src/client_ca_authority.rs @@ -0,0 +1,1016 @@ +//! Client-certificate CA: mints short-lived mTLS client certificates from a +//! CSR the Node API forwards on an agent's behalf. +//! +//! Lifecycle mirrors `ca.rs` exactly (env vars → disk → generate+persist, +//! 0600 key permissions, re-self-sign-on-load so `signed_by` has a +//! `Certificate` issuer to reference). What's new here is [`ClientCa::sign_csr`] +//! and the invariant it enforces — see the `SECURITY` note on that function. +//! +//! Declared unconditionally in `main.rs` (like `ca.rs`) so the OSS build +//! always compiles this module; whether it's ever exercised depends on +//! whether `GatewayState.client_ca` is `Some` (see `gateway.rs`). + +use std::path::Path; +use std::time::Duration; + +use anyhow::{Context, Result}; +use rcgen::{ + BasicConstraints, CertificateParams, CertificateSigningRequestParams, DistinguishedName, + DnType, ExtendedKeyUsagePurpose, Ia5String, IsCa, KeyPair, KeyUsagePurpose, SanType, + PKCS_ECDSA_P256_SHA256, +}; +use rustls::pki_types::CertificateDer; +use time::OffsetDateTime; +use tokio::fs; +use tracing::info; + +/// Client-CA certificate validity: 10 years (mirrors `ca.rs::CA_VALIDITY_DAYS`). +const CA_VALIDITY_DAYS: i64 = 3650; + +/// CN for the generated client CA (distinct from `ca.rs`'s MITM CA CN — the +/// two must never be confused, see the SPKI guard in `client_ca.rs`). +const CLIENT_CA_CN: &str = "OneCLI Client CA"; + +/// Default issued-certificate lifetime when the caller doesn't ask for one. +pub(crate) const DEFAULT_LIFETIME: Duration = Duration::from_secs(24 * 3600); + +/// Hard ceiling on issued-certificate lifetime, regardless of what's requested. +pub(crate) const MAX_LIFETIME: Duration = Duration::from_secs(7 * 24 * 3600); + +/// Clamp a caller-requested lifetime (in seconds) to `(0, MAX_LIFETIME]`, +/// defaulting to `DEFAULT_LIFETIME` when unspecified or zero. Shared by the +/// gateway's internal HTTP handler so the policy lives in one place next to +/// the constants it enforces. +pub(crate) fn clamp_lifetime(requested_secs: Option) -> Duration { + match requested_secs { + None | Some(0) => DEFAULT_LIFETIME, + Some(secs) => Duration::from_secs(secs).min(MAX_LIFETIME), + } +} + +/// A minted client certificate. Deliberately carries no private key material — +/// `sign_csr` never generates or sees one; the CSR is proof the caller already +/// holds the key it's requesting a certificate for. +#[derive(Debug)] +pub(crate) struct IssuedCert { + /// PEM chain: the signed leaf, followed by this CA's own certificate. + pub(crate) cert_pem: String, + pub(crate) serial_hex: String, + pub(crate) not_after_unix: i64, +} + +/// Why [`ClientCa::sign_csr`] failed. `BadCsr` is caller error (maps to HTTP +/// 400 in `gateway.rs`); `Sign` is this CA's own failure (maps to a 500-class +/// response) and should not happen against a healthy CA + a CSR that already +/// parsed. +#[derive(Debug)] +pub(crate) enum SignCsrError { + /// The CSR failed to parse, or its embedded signature failed to verify — + /// garbage input, a tampered CSR, or an algorithm rcgen/x509-parser + /// doesn't support. Never a panic: every rcgen parse error lands here. + BadCsr(String), + /// Something failed on our side while building/signing the fresh + /// certificate (e.g. `spiffe_uri` not representable as IA5, or rcgen + /// signing itself failing). + Sign(anyhow::Error), +} + +impl std::fmt::Display for SignCsrError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::BadCsr(msg) => write!(f, "invalid CSR: {msg}"), + Self::Sign(err) => write!(f, "signing failed: {err:#}"), + } + } +} + +impl std::error::Error for SignCsrError {} + +/// The client-certificate CA: holds the key pair that signs every minted +/// client leaf, plus the certificate agents must trust as the mTLS anchor. +pub(crate) struct ClientCa { + /// Re-created (same key + params) so `signed_by` has an issuer to + /// reference — see `load_from_disk`'s doc comment on `ca.rs`'s twin. + ca_cert: rcgen::Certificate, + ca_key: KeyPair, + /// The original CA certificate DER — served to callers via + /// `ca_cert_pem()` and appended to every issued leaf's chain. + ca_cert_der: CertificateDer<'static>, +} + +impl ClientCa { + /// Load an existing client CA from environment variables, disk, or + /// generate a new one. Priority (identical shape to + /// `ca::CertificateAuthority::load_or_generate`): + /// + /// 1. `GATEWAY_CLIENT_CA_KEY` + `GATEWAY_CLIENT_CA_CERT` env vars (cloud: + /// injected from Secrets Manager). + /// 2. Files at `{data_dir}/gateway/client-ca.key` and `client-ca.pem` + /// (OSS: persisted on disk). + /// 3. Generate a new client CA and persist to disk (OSS: first startup). + /// + /// Any failure here must abort startup (see `main.rs`) — never fall back + /// to running with no minting capability while pretending to have one. + pub(crate) async fn load_or_generate(data_dir: &Path) -> Result { + if let (Ok(key_pem), Ok(cert_pem)) = ( + std::env::var("GATEWAY_CLIENT_CA_KEY"), + std::env::var("GATEWAY_CLIENT_CA_CERT"), + ) { + if !key_pem.is_empty() && !cert_pem.is_empty() { + info!("loading client CA from environment variables"); + return Self::load_from_pem(&key_pem, &cert_pem); + } + } + + let gateway_dir = data_dir.join("gateway"); + let key_path = gateway_dir.join("client-ca.key"); + let cert_path = gateway_dir.join("client-ca.pem"); + + if key_path.exists() && cert_path.exists() { + info!( + key = %key_path.display(), + cert = %cert_path.display(), + "loading existing client CA" + ); + Self::load_from_disk(&key_path, &cert_path).await + } else { + info!(dir = %gateway_dir.display(), "generating new client CA"); + fs::create_dir_all(&gateway_dir) + .await + .context("creating gateway data directory")?; + Self::generate_and_persist(&key_path, &cert_path).await + } + } + + /// The client CA certificate as PEM — the trust anchor agents (and, when + /// `GATEWAY_CLIENT_CA` is unset, the gateway's own mTLS listener) need. + pub(crate) fn ca_cert_pem(&self) -> String { + der_to_pem(self.ca_cert_der.as_ref()) + } + + /// The raw CA certificate DER. + #[allow(dead_code)] + pub(crate) fn ca_cert_der(&self) -> &CertificateDer<'static> { + &self.ca_cert_der + } + + /// Mint a client certificate for `host_id`/`spiffe_uri` from a CSR that + /// already proves possession of the private key it names. + /// + /// SECURITY: this function uses ONLY the CSR's public key. + /// `CertificateSigningRequestParams::from_pem` parses AND verifies the + /// CSR's own signature (proof the caller holds the matching private + /// key) — that's the full extent of what the CSR is trusted for. Its + /// `params` field (the CSR's requested subject, SANs, extended/key + /// usage, validity, and CA flag) is 100% attacker-controlled and is + /// discarded in full below, never read again. Every identity-bearing + /// field on the certificate actually issued is set HERE, server-side, + /// from `host_id`/`spiffe_uri` alone. + /// + /// Do NOT replace the `signed_by` call below with + /// `CertificateSigningRequestParams::signed_by` — that method copies the + /// CSR's own subject/SAN/EKU onto the issued certificate (confirmed + /// against rcgen 0.13.2 source), which would let any client mint a + /// certificate for whatever identity it chooses. A client must never be + /// able to choose its own identity. + pub(crate) fn sign_csr( + &self, + host_id: &str, + spiffe_uri: &str, + csr_pem: &str, + lifetime: Duration, + ) -> Result { + // SECURITY: the client's proof of key-possession is entirely + // `from_pem`'s job — it parses the CSR AND verifies its embedded + // self-signature (rcgen 0.13.2, gated behind the `x509-parser` + // feature this crate turns on in Cargo.toml). Garbage, tampered, or + // an unsupported signature algorithm all fail here as a plain `Err`, + // never a panic — see `sign_csr_tampered_csr_body_byte_is_rejected` + // and the malformed/garbage/unsupported-algorithm tests below. That + // suite is the REAL guard against a silent regression here (e.g. an + // rcgen upgrade/downgrade that weakens or drops the signature check) + // — no hard version pin is needed beyond Cargo.lock's normal pinning + // of the resolved version, because these tests would start failing + // immediately if possession verification ever stopped happening. + let CertificateSigningRequestParams { + public_key, + // SECURITY: named (not `_`) and never referenced again, so a + // future edit that starts reading it is a visible diff, not a + // silent regression. This is the CSR's attacker-controlled + // subject/SAN/EKU/key-usage/validity/is_ca — discarded whole. + params: _csr_params_discarded_do_not_use, + } = CertificateSigningRequestParams::from_pem(csr_pem) + .map_err(|e| SignCsrError::BadCsr(e.to_string()))?; + + let uri_san = Ia5String::try_from(spiffe_uri.to_string()) + .map_err(|e| SignCsrError::Sign(anyhow::anyhow!("spiffe_uri is not IA5: {e}")))?; + + let mut distinguished_name = DistinguishedName::new(); + distinguished_name.push(DnType::CommonName, host_id); + + let mut params = CertificateParams::default(); + params.subject_alt_names = vec![SanType::URI(uri_san)]; + params.distinguished_name = distinguished_name; + // Explicit, even though `CertificateParams::default()` already gives + // `IsCa::NoCa` — a client certificate must never be a CA, and that + // must never be something we forward from the CSR (see the SECURITY + // note above). + params.is_ca = IsCa::NoCa; + params.key_usages = vec![KeyUsagePurpose::DigitalSignature]; + params.extended_key_usages = vec![ExtendedKeyUsagePurpose::ClientAuth]; + params.use_authority_key_identifier_extension = true; + // Small backdate for clock skew, mirroring `ca.rs::leaf_params`. + params.not_before = OffsetDateTime::now_utc() - time::Duration::minutes(5); + params.not_after = + OffsetDateTime::now_utc() + time::Duration::seconds(lifetime.as_secs() as i64); + + let leaf_cert = params + .signed_by(&public_key, &self.ca_cert, &self.ca_key) + .map_err(|e| SignCsrError::Sign(anyhow::anyhow!("signing client certificate: {e}")))?; + + let (_, parsed) = x509_parser::parse_x509_certificate(leaf_cert.der().as_ref()) + .map_err(|e| SignCsrError::Sign(anyhow::anyhow!("parsing freshly signed leaf: {e}")))?; + let serial_hex = hex::encode(parsed.raw_serial()); + let not_after_unix = parsed.validity().not_after.timestamp(); + + // Chain: leaf + this CA's certificate, so a peer that only trusts the + // CA (not every intermediate) can still build the path. + let cert_pem = format!("{}{}", leaf_cert.pem(), self.ca_cert_pem()); + + Ok(IssuedCert { + cert_pem, + serial_hex, + not_after_unix, + }) + } + + // ── Private ────────────────────────────────────────────────────────── + + fn build_ca_params() -> CertificateParams { + let mut params = CertificateParams::default(); + params.is_ca = IsCa::Ca(BasicConstraints::Unconstrained); + params + .distinguished_name + .push(DnType::CommonName, CLIENT_CA_CN); + params + .distinguished_name + .push(DnType::OrganizationName, "OneCLI"); + params.key_usages = vec![KeyUsagePurpose::KeyCertSign, KeyUsagePurpose::CrlSign]; + params.not_before = OffsetDateTime::now_utc(); + params.not_after = OffsetDateTime::now_utc() + time::Duration::days(CA_VALIDITY_DAYS); + params + } + + /// Load a client CA supplied by an operator (`GATEWAY_CLIENT_CA_KEY` + + /// `GATEWAY_CLIENT_CA_CERT`), which — unlike our own generated/persisted + /// CA — can carry ANY subject DN the operator's own PKI chose. + /// + /// NIT fix: this used to re-create the issuer via + /// `Self::build_ca_params()` (our own hardcoded `CLIENT_CA_CN`), which + /// only happens to work when the operator's cert's subject DN matches + /// that hardcoded value. `signed_by`'s `Issuer` only reads + /// `distinguished_name`/`key_identifier_method`/`key_usages` off the + /// `Certificate` passed in — it never reads `ca_cert_der` for that — so + /// a mismatch would silently mint leaves whose `issuer` field doesn't + /// match the actually-served trust anchor's `subject`: chain building + /// against the real anchor fails (fails closed, but confusingly, since + /// nothing here would say why). Deriving `CertificateParams` straight + /// from the loaded DER via `from_ca_cert_der` (rcgen's own tool for + /// exactly this "sign with an existing external CA" case) makes the + /// issuer DN always match the served cert, whatever DN it carries. + fn load_from_pem(key_pem: &str, cert_pem: &str) -> Result { + let key_pem = key_pem.trim(); + let cert_pem = cert_pem.trim(); + let ca_key = + KeyPair::from_pem(key_pem).context("parsing client CA private key from env var")?; + + let mut reader = cert_pem.as_bytes(); + let ca_cert_der = rustls_pemfile::certs(&mut reader) + .next() + .context("no certificate found in GATEWAY_CLIENT_CA_CERT env var")? + .context("parsing client CA certificate PEM from env var")?; + + let ca_cert = CertificateParams::from_ca_cert_der(&ca_cert_der) + .context( + "parsing GATEWAY_CLIENT_CA_CERT to extract its subject DN and issuer parameters", + )? + .self_signed(&ca_key) + .context( + "re-creating the loaded client CA certificate for signing (does the private \ + key in GATEWAY_CLIENT_CA_KEY match the certificate in GATEWAY_CLIENT_CA_CERT?)", + )?; + + Ok(Self { + ca_cert, + ca_key, + ca_cert_der, + }) + } + + async fn generate_and_persist(key_path: &Path, cert_path: &Path) -> Result { + let ca_key = KeyPair::generate_for(&PKCS_ECDSA_P256_SHA256) + .context("generating client CA key pair")?; + let params = Self::build_ca_params(); + let ca_cert = params + .self_signed(&ca_key) + .context("self-signing client CA certificate")?; + let ca_cert_der = ca_cert.der().clone(); + + let key_pem = ca_key.serialize_pem(); + fs::write(key_path, key_pem.as_bytes()) + .await + .context("writing client CA private key")?; + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + std::fs::set_permissions(key_path, std::fs::Permissions::from_mode(0o600)).ok(); + } + + let cert_pem = der_to_pem(ca_cert_der.as_ref()); + fs::write(cert_path, cert_pem.as_bytes()) + .await + .context("writing client CA certificate")?; + + info!( + cn = CLIENT_CA_CN, + key = %key_path.display(), + cert = %cert_path.display(), + "generated and persisted new client CA" + ); + + Ok(Self { + ca_cert, + ca_key, + ca_cert_der, + }) + } + + /// Load an existing client CA from disk. Same re-creation trick as + /// `ca::CertificateAuthority::load_from_disk`: `signed_by` needs a + /// `Certificate` issuer reference, and the only way to get one is + /// `self_signed()`/`signed_by()` — so re-self-sign with the same key + + /// params. The issuer DN in minted leaves matches the on-disk cert + /// because the params are identical; the on-disk DER (not this + /// re-creation) is what's served and chained. + async fn load_from_disk(key_path: &Path, cert_path: &Path) -> Result { + let key_pem = fs::read_to_string(key_path) + .await + .context("reading client CA private key")?; + let ca_key = KeyPair::from_pem(&key_pem).context("parsing client CA private key")?; + + let cert_pem = fs::read_to_string(cert_path) + .await + .context("reading client CA certificate")?; + let mut reader = cert_pem.as_bytes(); + let ca_cert_der = rustls_pemfile::certs(&mut reader) + .next() + .context("no certificate found in client CA PEM file")? + .context("parsing client CA certificate PEM")?; + + let ca_cert = Self::build_ca_params() + .self_signed(&ca_key) + .context("re-creating client CA certificate for signing")?; + + info!("loaded existing client CA certificate"); + + Ok(Self { + ca_cert, + ca_key, + ca_cert_der, + }) + } +} + +/// Encode raw DER bytes as a PEM-formatted certificate string. Duplicated +/// from `ca.rs` (private there) rather than shared — each CA module in this +/// crate owns its own small PEM helpers (see also `client_ca.rs`'s +/// `pem_to_der_certs`). +fn der_to_pem(der: &[u8]) -> String { + let b64 = base64::Engine::encode(&base64::engine::general_purpose::STANDARD, der); + let num_lines = b64.len().div_ceil(64); + let capacity = 28 + b64.len() + num_lines + 26; + let mut pem = String::with_capacity(capacity); + pem.push_str("-----BEGIN CERTIFICATE-----\n"); + for chunk in b64.as_bytes().chunks(64) { + pem.push_str(std::str::from_utf8(chunk).expect("base64 is valid utf8")); + pem.push('\n'); + } + pem.push_str("-----END CERTIFICATE-----\n"); + pem +} + +/// Encode raw CSR DER bytes as a PEM-formatted CSR string — the inverse of +/// `rustls_pemfile::csr`, used by `#[cfg(test)]` code that mutates a CSR's +/// DER and needs to feed it back through `sign_csr` as PEM. +#[cfg(test)] +fn csr_der_to_pem(der: &[u8]) -> String { + let b64 = base64::Engine::encode(&base64::engine::general_purpose::STANDARD, der); + let mut pem = String::new(); + pem.push_str("-----BEGIN CERTIFICATE REQUEST-----\n"); + for chunk in b64.as_bytes().chunks(64) { + pem.push_str(std::str::from_utf8(chunk).expect("base64 is valid utf8")); + pem.push('\n'); + } + pem.push_str("-----END CERTIFICATE REQUEST-----\n"); + pem +} + +#[cfg(test)] +mod tests { + use super::*; + + use crate::client_ca::identity_from_peer_certs; + use crate::client_ca::test_support::{ + attempt_handshake, ensure_crypto_provider, err_debug_contains, new_test_ca, + test_client_config, test_server_setup, + }; + + /// Build a CSR PEM for `key`, optionally embedding attacker-requested + /// identity fields (CN/URI SAN) that a correct `sign_csr` must ignore. + /// Mirrors the rcgen 0.13.2 facts: `CertificateParams::serialize_request` + /// produces the CSR PEM directly from params + key. + fn build_csr_pem( + key: &KeyPair, + requested_cn: Option<&str>, + requested_uri: Option<&str>, + ) -> String { + let mut params = CertificateParams::default(); + if let Some(cn) = requested_cn { + params.distinguished_name = DistinguishedName::new(); + params.distinguished_name.push(DnType::CommonName, cn); + } + if let Some(uri) = requested_uri { + params.subject_alt_names = vec![SanType::URI( + Ia5String::try_from(uri.to_string()).expect("ia5"), + )]; + } + let csr = params.serialize_request(key).expect("build CSR"); + csr.pem().expect("CSR PEM") + } + + fn new_client_ca() -> ClientCa { + let ca_key = KeyPair::generate_for(&PKCS_ECDSA_P256_SHA256).expect("ca key"); + let params = ClientCa::build_ca_params(); + let ca_cert = params.self_signed(&ca_key).expect("self-sign client ca"); + let ca_cert_der = ca_cert.der().clone(); + ClientCa { + ca_cert, + ca_key, + ca_cert_der, + } + } + + // ── clamp_lifetime ─────────────────────────────────────────────────── + + #[test] + fn clamp_lifetime_none_is_default() { + assert_eq!(clamp_lifetime(None), DEFAULT_LIFETIME); + } + + #[test] + fn clamp_lifetime_zero_is_default() { + assert_eq!(clamp_lifetime(Some(0)), DEFAULT_LIFETIME); + } + + #[test] + fn clamp_lifetime_within_range_passes_through() { + assert_eq!(clamp_lifetime(Some(3600)), Duration::from_secs(3600)); + } + + #[test] + fn clamp_lifetime_above_max_is_clamped() { + assert_eq!( + clamp_lifetime(Some(MAX_LIFETIME.as_secs() + 1000)), + MAX_LIFETIME + ); + } + + // ── sign_csr: happy path + end-to-end verification ────────────────── + + #[tokio::test] + async fn sign_csr_happy_path_verifies_through_webpki_and_carries_server_identity() { + ensure_crypto_provider(); + let ca = new_client_ca(); + + let client_key = KeyPair::generate_for(&PKCS_ECDSA_P256_SHA256).expect("client key"); + let csr_pem = build_csr_pem(&client_key, None, None); + + let issued = ca + .sign_csr( + "host-42", + "spiffe://onecli/host/42", + &csr_pem, + Duration::from_secs(3600), + ) + .expect("sign_csr succeeds"); + + assert!(!issued.serial_hex.is_empty()); + assert!(issued.not_after_unix > 0); + // Leaf, then CA — both PEM blocks present in the chain. + assert_eq!(issued.cert_pem.matches("BEGIN CERTIFICATE").count(), 2); + + // The chain must actually verify through Phase 1's client verifier — + // reusing the exact harness `client_ca.rs`'s own tests run against. + let (server_config, server_der) = test_server_setup(&ca.ca_cert_pem()); + let leaf_pem = issued + .cert_pem + .split("-----END CERTIFICATE-----\n") + .next() + .map(|s| format!("{s}-----END CERTIFICATE-----\n")) + .expect("leaf pem slice"); + let client_config = test_client_config( + &server_der, + Some(&leaf_pem), + Some(&client_key.serialize_pem()), + ); + + let mut tls_stream = attempt_handshake(server_config, client_config) + .await + .expect("minted client cert must be accepted by the mTLS listener"); + + let identity = tls_stream + .get_ref() + .1 + .peer_certificates() + .and_then(identity_from_peer_certs) + .expect("identity extracted from minted cert"); + assert_eq!(identity.primary(), Some("spiffe://onecli/host/42")); + assert_eq!(identity.cn.as_deref(), Some("host-42")); + + use tokio::io::AsyncWriteExt; + let _ = tls_stream.shutdown().await; + } + + // ── CRITICAL: the CSR's requested identity must never survive ─────── + + #[test] + fn sign_csr_ignores_attacker_requested_identity_entirely() { + ensure_crypto_provider(); + let ca = new_client_ca(); + + let client_key = KeyPair::generate_for(&PKCS_ECDSA_P256_SHA256).expect("client key"); + // The CSR asks to BE someone else entirely. + let csr_pem = build_csr_pem( + &client_key, + Some("attacker-impersonating-admin"), + Some("spiffe://onecli/host/some-other-victim"), + ); + + let issued = ca + .sign_csr( + "real-host-id", + "spiffe://onecli/host/real-host-id", + &csr_pem, + Duration::from_secs(3600), + ) + .expect("sign_csr succeeds"); + + // Parse the actual issued leaf's structured fields — NOT a substring + // search on `cert_pem` (it's base64-encoded DER; the CSR's requested + // strings wouldn't appear as literal text there either way, so that + // would prove nothing). `certs()` on the combined chain yields the + // leaf first. + let mut reader = issued.cert_pem.as_bytes(); + let leaf_der = rustls_pemfile::certs(&mut reader) + .next() + .expect("leaf present") + .expect("leaf parses"); + let (_, parsed) = + x509_parser::parse_x509_certificate(leaf_der.as_ref()).expect("parse leaf"); + + let cn = parsed + .subject() + .iter_common_name() + .next() + .and_then(|a| a.as_str().ok()); + assert_eq!(cn, Some("real-host-id"), "CN must be the server identity"); + assert_ne!(cn, Some("attacker-impersonating-admin")); + + let uri_sans: Vec<&str> = parsed + .subject_alternative_name() + .ok() + .flatten() + .map(|ext| { + ext.value + .general_names + .iter() + .filter_map(|name| match name { + x509_parser::extensions::GeneralName::URI(uri) => Some(*uri), + _ => None, + }) + .collect() + }) + .unwrap_or_default(); + assert_eq!( + uri_sans, + vec!["spiffe://onecli/host/real-host-id"], + "the ONLY SAN must be the server-supplied spiffe URI" + ); + assert!(!uri_sans.contains(&"spiffe://onecli/host/some-other-victim")); + } + + // ── Malformed / tampered / unsupported CSRs → BadCsr, never a panic ─ + + #[test] + fn sign_csr_malformed_csr_is_bad_csr_not_panic() { + ensure_crypto_provider(); + let ca = new_client_ca(); + + let result = ca.sign_csr( + "host", + "spiffe://onecli/host/x", + "not a csr at all", + Duration::from_secs(60), + ); + assert!(matches!(result, Err(SignCsrError::BadCsr(_)))); + } + + #[test] + fn sign_csr_tampered_csr_body_byte_is_rejected() { + ensure_crypto_provider(); + let ca = new_client_ca(); + + let client_key = KeyPair::generate_for(&PKCS_ECDSA_P256_SHA256).expect("client key"); + let csr_pem = build_csr_pem(&client_key, None, None); + + // Flip one base64 character in the PEM body (not the header/footer) — + // this corrupts the DER, which fails to parse or fails signature + // verification, either way a `BadCsr`, never a panic. + let mut lines: Vec<&str> = csr_pem.lines().collect(); + let body_idx = lines + .iter() + .position(|l| !l.starts_with("-----")) + .expect("has a body line"); + let mut chars: Vec = lines[body_idx].chars().collect(); + assert!(!chars.is_empty(), "body line must not be empty"); + chars[0] = if chars[0] == 'A' { 'B' } else { 'A' }; + let tampered_line: String = chars.into_iter().collect(); + lines[body_idx] = &tampered_line; + let tampered_pem = lines.join("\n") + "\n"; + + let result = ca.sign_csr( + "host", + "spiffe://onecli/host/x", + &tampered_pem, + Duration::from_secs(60), + ); + assert!(matches!(result, Err(SignCsrError::BadCsr(_)))); + } + + /// An unrecognized/garbled signature-algorithm OID must fail gracefully + /// (→ `BadCsr` → HTTP 400), never panic. Real-world unsupported-algorithm + /// CSRs are the natural trigger for this path; this test reaches the + /// same rcgen code path deterministically by corrupting the OID's last + /// arc directly, length-preserving so the DER stays syntactically valid. + /// + /// This is a PROXY, not a literal foreign-algorithm CSR: every algorithm + /// this crate's rcgen build can actually GENERATE a CSR with (ECDSA + /// P-256/P-384/P-521, Ed25519 — RSA generation is unavailable without the + /// `aws_lc_rs` feature, which isn't enabled) is one `SignatureAlgorithm:: + /// from_oid` recognizes, so there's no real key we can hand it that's + /// genuinely unsupported. `sign_csr_accepts_a_genuine_ed25519_csr_and_the_leaf_verifies` + /// below covers the one truly-different (non-ECDSA) algorithm we CAN + /// build for real; this OID-mutation test is kept as the deterministic + /// stand-in for "an algorithm neither rcgen nor x509-parser knows at all". + #[test] + fn sign_csr_unsupported_signature_algorithm_is_bad_csr_not_panic() { + ensure_crypto_provider(); + let ca = new_client_ca(); + + let client_key = KeyPair::generate_for(&PKCS_ECDSA_P256_SHA256).expect("client key"); + let csr_pem = build_csr_pem(&client_key, None, None); + + let mut reader = csr_pem.as_bytes(); + let der = rustls_pemfile::csr(&mut reader) + .expect("read csr pem") + .expect("csr present") + .as_ref() + .to_vec(); + + // ecdsa-with-SHA256's OID content bytes (1.2.840.10045.4.3.2). This + // 8-byte pattern is distinct from the 7-byte ecPublicKey OID + // (1.2.840.10045.2.1) that also appears in the CSR's SPKI, so the + // match below can only land on the outer signatureAlgorithm field. + let target: [u8; 8] = [0x2a, 0x86, 0x48, 0xce, 0x3d, 0x04, 0x03, 0x02]; + let mut mutated = der.clone(); + let pos = mutated + .windows(target.len()) + .position(|w| w == target) + .expect("ecdsa-with-SHA256 OID present in CSR DER"); + mutated[pos + target.len() - 1] = 0x7f; + + let mutated_pem = csr_der_to_pem(&mutated); + let result = ca.sign_csr( + "host", + "spiffe://onecli/host/x", + &mutated_pem, + Duration::from_secs(60), + ); + assert!( + matches!(result, Err(SignCsrError::BadCsr(_))), + "an unsupported/garbled signature algorithm must fail as BadCsr, not panic: {result:?}" + ); + } + + /// A GENUINE cross-algorithm CSR (not a proxy): an Ed25519 key, generated + /// for real via `rcgen::PKCS_ED25519` (supported by the `ring` feature + /// without any external key material, unlike RSA — see the comment on + /// `sign_csr_unsupported_signature_algorithm_is_bad_csr_not_panic` + /// above). Pins the ACTUAL behavior: this crate supports minting from a + /// non-ECDSA CSR. The CA's own signing algorithm (ECDSA P-256) is + /// independent of the subject key's algorithm — a CA never needs to + /// share an algorithm with the certificates it signs — so the CSR + /// parses, `signed_by` accepts the Ed25519 public key, and the resulting + /// leaf (Ed25519 subject key + ECDSA-signed by the CA) verifies through + /// the real `WebPkiClientVerifier`, including the TLS handshake's own + /// Ed25519 `CertificateVerify` step. + #[tokio::test] + async fn sign_csr_accepts_a_genuine_ed25519_csr_and_the_leaf_verifies() { + ensure_crypto_provider(); + let ca = new_client_ca(); + + let client_key = + KeyPair::generate_for(&rcgen::PKCS_ED25519).expect("generate Ed25519 client key"); + let csr_pem = build_csr_pem(&client_key, None, None); + + let issued = ca + .sign_csr( + "ed25519-host", + "spiffe://onecli/host/ed25519", + &csr_pem, + Duration::from_secs(3600), + ) + .expect("sign_csr must accept a genuine Ed25519 CSR, not just ECDSA ones"); + + let (server_config, server_der) = test_server_setup(&ca.ca_cert_pem()); + let leaf_pem = issued + .cert_pem + .split("-----END CERTIFICATE-----\n") + .next() + .map(|s| format!("{s}-----END CERTIFICATE-----\n")) + .expect("leaf slice"); + let client_config = test_client_config( + &server_der, + Some(&leaf_pem), + Some(&client_key.serialize_pem()), + ); + + attempt_handshake(server_config, client_config) + .await + .expect("an Ed25519-keyed leaf, signed by our ECDSA CA, must verify end to end"); + } + + #[test] + fn sign_csr_garbage_pem_wrapper_is_bad_csr() { + ensure_crypto_provider(); + let ca = new_client_ca(); + + let garbage_pem = + "-----BEGIN CERTIFICATE REQUEST-----\nbm90IGEgcmVhbCBjc3I=\n-----END CERTIFICATE REQUEST-----\n"; + let result = ca.sign_csr( + "host", + "spiffe://onecli/host/x", + garbage_pem, + Duration::from_secs(60), + ); + assert!(matches!(result, Err(SignCsrError::BadCsr(_)))); + } + + // ── IssuedCert never carries key material (structural guarantee) ──── + // + // `IssuedCert` has exactly three fields — `cert_pem`, `serial_hex`, + // `not_after_unix` — and this constructor is the only place one is built. + // There is no field to leak a key through; this test exists so a future + // edit that adds one has to touch this assertion. + #[test] + fn issued_cert_has_no_key_field() { + ensure_crypto_provider(); + let ca = new_client_ca(); + let client_key = KeyPair::generate_for(&PKCS_ECDSA_P256_SHA256).expect("client key"); + let csr_pem = build_csr_pem(&client_key, None, None); + let issued = ca + .sign_csr( + "host", + "spiffe://onecli/host/x", + &csr_pem, + Duration::from_secs(60), + ) + .expect("sign succeeds"); + let IssuedCert { + cert_pem: _, + serial_hex: _, + not_after_unix: _, + } = issued; + } + + // ── Wrong-CA / expired leaves rejected through the real verifier ──── + + #[tokio::test] + async fn minted_leaf_rejected_when_verifier_trusts_a_different_ca() { + ensure_crypto_provider(); + let ca = new_client_ca(); + let other_ca = new_test_ca("Some Other CA"); + + let client_key = KeyPair::generate_for(&PKCS_ECDSA_P256_SHA256).expect("client key"); + let csr_pem = build_csr_pem(&client_key, None, None); + let issued = ca + .sign_csr( + "host", + "spiffe://onecli/host/x", + &csr_pem, + Duration::from_secs(3600), + ) + .expect("sign succeeds"); + + // The verifier trusts `other_ca`, NOT the CA that actually signed + // this leaf. + let (server_config, server_der) = test_server_setup(&other_ca.cert.pem()); + let leaf_pem = issued + .cert_pem + .split("-----END CERTIFICATE-----\n") + .next() + .map(|s| format!("{s}-----END CERTIFICATE-----\n")) + .expect("leaf slice"); + let client_config = test_client_config( + &server_der, + Some(&leaf_pem), + Some(&client_key.serialize_pem()), + ); + + let result = attempt_handshake(server_config, client_config).await; + let err = result.expect_err("leaf signed by an untrusted CA must be rejected"); + assert!( + err_debug_contains(&err, "UnknownIssuer"), + "unexpected error: {err:?}" + ); + } + + #[tokio::test] + async fn minted_leaf_rejected_once_expired() { + ensure_crypto_provider(); + let ca = new_client_ca(); + + let client_key = KeyPair::generate_for(&PKCS_ECDSA_P256_SHA256).expect("client key"); + let csr_pem = build_csr_pem(&client_key, None, None); + + // `sign_csr` always backdates `not_before` 5 minutes and can't mint + // an already-expired cert through its own API (by design — the + // handler clamps to a positive lifetime). To test the verifier's + // expiry check against a cert issued by THIS CA, build the expired + // leaf directly with the CA's key, mirroring `sign_csr`'s own leaf + // params but with a validity window entirely in the past. + let CertificateSigningRequestParams { public_key, .. } = + CertificateSigningRequestParams::from_pem(&csr_pem).expect("parse csr"); + let mut params = CertificateParams::default(); + params.subject_alt_names = vec![SanType::URI( + Ia5String::try_from("spiffe://onecli/host/x".to_string()).unwrap(), + )]; + params.distinguished_name.push(DnType::CommonName, "host"); + params.key_usages = vec![KeyUsagePurpose::DigitalSignature]; + params.extended_key_usages = vec![ExtendedKeyUsagePurpose::ClientAuth]; + params.not_before = OffsetDateTime::now_utc() - time::Duration::hours(48); + params.not_after = OffsetDateTime::now_utc() - time::Duration::hours(24); + let expired_leaf = params + .signed_by(&public_key, &ca.ca_cert, &ca.ca_key) + .expect("sign expired leaf"); + + let (server_config, server_der) = test_server_setup(&ca.ca_cert_pem()); + let client_config = test_client_config( + &server_der, + Some(&expired_leaf.pem()), + Some(&client_key.serialize_pem()), + ); + + let result = attempt_handshake(server_config, client_config).await; + let err = result.expect_err("expired leaf must be rejected"); + assert!( + err_debug_contains(&err, "Expired"), + "unexpected error: {err:?}" + ); + } + + // ── Persistence: 0600 permissions, persist → reload → sign still works ─ + + #[tokio::test] + async fn client_ca_key_file_has_restricted_permissions() { + let tmp = tempfile::tempdir().expect("tempdir"); + ClientCa::load_or_generate(tmp.path()) + .await + .expect("generate"); + + let key_path = tmp.path().join("gateway").join("client-ca.key"); + assert!(key_path.exists()); + + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let perms = std::fs::metadata(&key_path) + .expect("metadata") + .permissions(); + assert_eq!(perms.mode() & 0o777, 0o600); + } + } + + #[tokio::test] + async fn client_ca_persists_and_reload_can_still_sign() { + ensure_crypto_provider(); + let tmp = tempfile::tempdir().expect("tempdir"); + + let ca1 = ClientCa::load_or_generate(tmp.path()) + .await + .expect("generate"); + let ca2 = ClientCa::load_or_generate(tmp.path()) + .await + .expect("reload"); + assert_eq!(ca1.ca_cert_pem(), ca2.ca_cert_pem()); + + let client_key = KeyPair::generate_for(&PKCS_ECDSA_P256_SHA256).expect("client key"); + let csr_pem = build_csr_pem(&client_key, None, None); + let issued = ca2 + .sign_csr( + "host", + "spiffe://onecli/host/x", + &csr_pem, + Duration::from_secs(3600), + ) + .expect("sign after reload succeeds"); + + let (server_config, server_der) = test_server_setup(&ca2.ca_cert_pem()); + let leaf_pem = issued + .cert_pem + .split("-----END CERTIFICATE-----\n") + .next() + .map(|s| format!("{s}-----END CERTIFICATE-----\n")) + .expect("leaf slice"); + let client_config = test_client_config( + &server_der, + Some(&leaf_pem), + Some(&client_key.serialize_pem()), + ); + + attempt_handshake(server_config, client_config) + .await + .expect("leaf signed after CA reload must verify"); + } + + // ── load_from_pem: issuer DN must come from the LOADED cert ────────── + + /// NIT fix regression guard: an operator-supplied `GATEWAY_CLIENT_CA_CERT` + /// can carry any subject DN — nothing requires it to match this crate's + /// hardcoded `CLIENT_CA_CN`. Before the fix, `load_from_pem` re-created + /// the issuer via the hardcoded `build_ca_params()`, so a minted leaf's + /// `issuer` field would silently mismatch the real trust anchor's + /// `subject` whenever the operator's DN differed — failing chain + /// validation with no clue why. This proves a leaf minted from an + /// externally-supplied CA with a DIFFERENT DN still verifies through + /// that SAME external CA's own certificate. + #[tokio::test] + async fn load_from_pem_derives_issuer_dn_from_the_loaded_cert_not_the_hardcoded_default() { + ensure_crypto_provider(); + + // Deliberately NOT "OneCLI Client CA" (this crate's CLIENT_CA_CN) — + // an operator's own PKI, with its own naming. + let external_ca = new_test_ca("Acme Corp Internal Root CA"); + let key_pem = external_ca.key.serialize_pem(); + let cert_pem = external_ca.cert.pem(); + + let ca = ClientCa::load_from_pem(&key_pem, &cert_pem).expect("load external CA"); + // Sanity: the loaded CA really does serve back the operator's own + // cert (with its own DN), not something we regenerated. + assert_eq!(ca.ca_cert_pem(), external_ca.cert.pem()); + + let client_key = KeyPair::generate_for(&PKCS_ECDSA_P256_SHA256).expect("client key"); + let csr_pem = build_csr_pem(&client_key, None, None); + let issued = ca + .sign_csr( + "host", + "spiffe://onecli/host/x", + &csr_pem, + Duration::from_secs(3600), + ) + .expect("sign succeeds against an externally-supplied CA"); + + // The minted leaf must chain through the ACTUAL external CA + // certificate — this fails if the leaf's issuer DN doesn't match + // that cert's real subject DN. + let (server_config, server_der) = test_server_setup(&external_ca.cert.pem()); + let leaf_pem = issued + .cert_pem + .split("-----END CERTIFICATE-----\n") + .next() + .map(|s| format!("{s}-----END CERTIFICATE-----\n")) + .expect("leaf slice"); + let client_config = test_client_config( + &server_der, + Some(&leaf_pem), + Some(&client_key.serialize_pem()), + ); + + attempt_handshake(server_config, client_config) + .await + .expect( + "a leaf minted against an externally-supplied CA with a non-default DN must still \ + verify through that CA's own certificate", + ); + } +} diff --git a/apps/gateway/src/gateway.rs b/apps/gateway/src/gateway.rs index e5bbf76e..3443544f 100644 --- a/apps/gateway/src/gateway.rs +++ b/apps/gateway/src/gateway.rs @@ -37,11 +37,12 @@ mod tunnel; mod websocket; use std::net::{IpAddr, Ipv4Addr, SocketAddr}; -use std::sync::Arc; +use std::sync::{Arc, OnceLock}; use std::time::Duration; use anyhow::{Context, Result}; use axum::extract::State; +use axum::response::IntoResponse; use axum::Router; use hyper::body::Incoming; use hyper::server::conn::http1; @@ -61,6 +62,7 @@ use crate::auth::AuthUser; use crate::ca::CertificateAuthority; use crate::cache::CacheStore; use crate::client_ca::{self, ClientIdentity, MtlsConfig}; +use crate::client_ca_authority::{self, ClientCa, SignCsrError}; use crate::connect::{self, AppConnectionResult, ConnectError, PolicyEngine}; use crate::db; use crate::inject; @@ -125,6 +127,13 @@ pub(crate) struct GatewayState { pub vault_service: Arc, /// Manual approval store for held requests. pub approval_store: Arc, + /// The client-certificate minting authority (Phase 2), when the gateway + /// is trusting its OWN generated client CA (see `main.rs`). `None` when + /// an operator has configured an externally managed `GATEWAY_CLIENT_CA` + /// trust anchor instead — minting against an unrelated locally-generated + /// CA would produce certificates nobody trusts, so `issue_client_cert` + /// returns 503 rather than silently minting from the wrong CA. + pub client_ca: Option>, } // ── GatewayServer ─────────────────────────────────────────────────────── @@ -330,6 +339,7 @@ impl GatewayServer { cache: Arc, approval_store: Arc, mtls: Option, + client_ca: Option>, ) -> Result { let global_skip = std::env::var("GATEWAY_DANGER_ACCEPT_INVALID_CERTS").is_ok(); let skip_verify_hosts = Arc::new(parse_skip_verify_hosts()); @@ -362,6 +372,7 @@ impl GatewayServer { cache, vault_service, approval_store, + client_ca, }; Ok(Self { @@ -441,6 +452,13 @@ impl GatewayServer { "/v1/approvals/{id}/decision", axum::routing::post(submit_approval_decision), ) + // Internal (Node -> gateway): mint a client certificate from a + // CSR Node forwards. Guarded by X-Gateway-Secret, not session/API + // key auth — see `issue_client_cert`. + .route( + "/v1/internal/client-cert/issue", + axum::routing::post(issue_client_cert), + ) // /api legacy routes (backwards compatibility) .route( "/api/vault/{provider}/pair", @@ -478,6 +496,10 @@ impl GatewayServer { .route( "/api/approvals/{id}/decision", axum::routing::post(submit_approval_decision), + ) + .route( + "/api/internal/client-cert/issue", + axum::routing::post(issue_client_cert), ); // Org-scoped routes are mounted via an edition-swapped seam @@ -705,6 +727,181 @@ async fn invalidate_cache( .await } +// ── Internal (Node -> gateway): client-certificate minting ────────────── + +/// Hard cap on the `POST /v1/internal/client-cert/issue` request body — a +/// CSR (plus `host_id`/`spiffe_uri`/`lifetime_secs`) is a few KB at most even +/// generously padded, nowhere near axum's much larger generic default body +/// limit. Checked explicitly in `issue_client_cert` before the body is +/// parsed. +const MAX_CLIENT_CERT_REQUEST_BODY_BYTES: usize = 16 * 1024; + +/// Shared secret the internal gateway<->Node endpoints authenticate with, +/// presented as the `X-Gateway-Secret` header. Read once. Mirrors +/// `vault/onepassword_api.rs`'s outbound `internal_secret()` (same env var, +/// opposite direction — this module CHECKS a value Node presents TO the +/// gateway, rather than presenting one). +fn internal_secret() -> &'static str { + static SECRET: OnceLock = OnceLock::new(); + SECRET.get_or_init(|| std::env::var("GATEWAY_INTERNAL_SECRET").unwrap_or_default()) +} + +/// Constant-time byte comparison (no data-dependent early exit once lengths +/// match — only the accumulated OR of differences is inspected at the end). +/// `ring::constant_time::verify_slices_are_equal` is deprecated upstream +/// ("internal function, no side-channel promises"), so this is hand-rolled +/// rather than built on it — the same approach Node's `timingSafeEqual` +/// implements, mirrored here for the Rust side of this shared-secret check. +fn constant_time_eq(a: &[u8], b: &[u8]) -> bool { + if a.len() != b.len() { + return false; + } + let mut diff: u8 = 0; + for (x, y) in a.iter().zip(b.iter()) { + diff |= x ^ y; + } + diff == 0 +} + +/// Comparison against an explicit expected value, fail-closed when that +/// value is empty. Mirrors `packages/api/src/middleware/internal-auth.ts`: +/// an empty configured secret must reject EVERY caller (including one +/// presenting an empty header) rather than "matching" on emptiness — an +/// unconfigured secret is a misconfiguration, never an open door. +/// +/// Split from `verify_internal_secret` (which reads the process-wide +/// `OnceLock`-cached env var) purely so this fail-closed logic is directly +/// unit-testable: `internal_secret()`'s cached value can only ever be set +/// once per test binary, which makes it unsuitable for exercising multiple +/// expected-secret scenarios from within the same process. +fn verify_secret(provided: &str, expected: &str) -> bool { + !expected.is_empty() && constant_time_eq(provided.as_bytes(), expected.as_bytes()) +} + +/// Comparison against the configured secret — see `verify_secret` for the +/// fail-closed rule this enforces. +fn verify_internal_secret(provided: &str) -> bool { + verify_secret(provided, internal_secret()) +} + +/// Body of `POST /v1/internal/client-cert/issue`. `lifetime_secs` is +/// optional — [`client_ca_authority::clamp_lifetime`] supplies the default +/// (24h) and ceiling (7d). +#[derive(serde::Deserialize)] +struct IssueClientCertRequest { + host_id: String, + spiffe_uri: String, + csr_pem: String, + #[serde(default)] + lifetime_secs: Option, +} + +#[derive(serde::Serialize)] +struct IssueClientCertResponse { + cert_pem: String, + ca_pem: String, + serial_hex: String, + not_after_unix: i64, +} + +/// `POST /v1/internal/client-cert/issue` (+ `/api` alias): mints a client +/// certificate for `host_id`/`spiffe_uri` from a CSR Node forwards on an +/// agent's behalf. +/// +/// Guarded by a shared secret (`X-Gateway-Secret`), NOT the session/API-key +/// `AuthUser` extractor every other route in this file uses — Node is the +/// only caller, and it has already authenticated the human/agent requesting +/// enrollment before it ever calls here. The secret is checked BEFORE the +/// body is parsed (raw `Bytes`, not an `axum::Json` extractor) so an +/// unauthorized caller always gets 401 regardless of what it sent as a body +/// — never a 400 that leaks "the body shape was wrong" to someone who +/// doesn't hold the shared secret. +/// +/// `sign_csr` re-parses and re-verifies the CSR itself — Node's own +/// well-formedness check (if any) is never trusted as the security boundary; +/// this endpoint is that boundary. +async fn issue_client_cert( + State(state): State, + headers: hyper::HeaderMap, + body: axum::body::Bytes, +) -> impl axum::response::IntoResponse { + let provided = headers + .get("x-gateway-secret") + .and_then(|v| v.to_str().ok()) + .unwrap_or(""); + if !verify_internal_secret(provided) { + return ( + StatusCode::UNAUTHORIZED, + axum::Json(serde_json::json!({ "error": "unauthorized" })), + ) + .into_response(); + } + + // Explicit, tight cap ahead of axum's own (much larger) default body + // limit: a CSR — even a generously padded one, e.g. an RSA-4096 key with + // extra attributes — is a few KB at most. Reject anything wildly outside + // that BEFORE spending effort on it, rather than relying solely on + // axum's generic default limit (which exists for arbitrary request + // bodies, not this specifically small shape). + if body.len() > MAX_CLIENT_CERT_REQUEST_BODY_BYTES { + return ( + StatusCode::PAYLOAD_TOO_LARGE, + axum::Json(serde_json::json!({ "error": "request body too large" })), + ) + .into_response(); + } + + let Some(client_ca) = state.client_ca.as_ref() else { + warn!("client-cert mint requested but no client-CA minting authority is configured"); + return ( + StatusCode::SERVICE_UNAVAILABLE, + axum::Json( + serde_json::json!({ "error": "client certificate minting is not available" }), + ), + ) + .into_response(); + }; + + let req: IssueClientCertRequest = match serde_json::from_slice(&body) { + Ok(r) => r, + Err(e) => { + return ( + StatusCode::BAD_REQUEST, + axum::Json(serde_json::json!({ "error": format!("invalid request body: {e}") })), + ) + .into_response(); + } + }; + + let lifetime = client_ca_authority::clamp_lifetime(req.lifetime_secs); + + match client_ca.sign_csr(&req.host_id, &req.spiffe_uri, &req.csr_pem, lifetime) { + Ok(issued) => ( + StatusCode::OK, + axum::Json(IssueClientCertResponse { + cert_pem: issued.cert_pem, + ca_pem: client_ca.ca_cert_pem(), + serial_hex: issued.serial_hex, + not_after_unix: issued.not_after_unix, + }), + ) + .into_response(), + Err(SignCsrError::BadCsr(msg)) => ( + StatusCode::BAD_REQUEST, + axum::Json(serde_json::json!({ "error": format!("invalid CSR: {msg}") })), + ) + .into_response(), + Err(SignCsrError::Sign(err)) => { + warn!(error = ?err, "client-cert signing failed"); + ( + StatusCode::INTERNAL_SERVER_ERROR, + axum::Json(serde_json::json!({ "error": "signing failed" })), + ) + .into_response() + } + } +} + /// Query parameters for the pending approvals endpoint. /// `pub(crate)` so the org route in `org_routes` can reuse the same shape. #[derive(serde::Deserialize)] @@ -1516,6 +1713,7 @@ mod tests { cache, vault_service, approval_store, + client_ca: None, } } @@ -1586,6 +1784,7 @@ mod tests { Some(&server_pem), Some(&server_key_pem), Some(&ca_pem), + None, &mitm_der, 10255, ) @@ -1836,4 +2035,214 @@ mod tests { .unwrap(); assert!(!is_http_proxy_request(&req)); } + + // ── constant_time_eq / verify_secret ───────────────────────────────── + + #[test] + fn constant_time_eq_equal_bytes_match() { + assert!(constant_time_eq(b"same-value", b"same-value")); + } + + #[test] + fn constant_time_eq_different_bytes_do_not_match() { + assert!(!constant_time_eq(b"same-value", b"other-value")); + } + + #[test] + fn constant_time_eq_different_lengths_do_not_match() { + assert!(!constant_time_eq(b"short", b"a-much-longer-value")); + } + + #[test] + fn constant_time_eq_empty_slices_match() { + assert!(constant_time_eq(b"", b"")); + } + + #[test] + fn verify_secret_matches_the_correct_value() { + assert!(verify_secret("hunter2", "hunter2")); + } + + #[test] + fn verify_secret_rejects_the_wrong_value() { + assert!(!verify_secret("wrong", "hunter2")); + } + + /// An empty configured secret must reject EVERY caller, including one + /// presenting an empty header — never "match on emptiness". This is the + /// property the internal client-cert endpoint's fail-closed posture + /// depends on. + #[test] + fn verify_secret_empty_expected_rejects_everything() { + assert!(!verify_secret("", "")); + assert!(!verify_secret("anything", "")); + } + + /// QA gap: the previous test only covers the empty-CONFIGURED-secret + /// case. The more common real-world bug is the other direction — the + /// server DOES have a real secret configured, and a caller sends an + /// empty-string header (a client that forgot to set it, or a proxy that + /// stripped the value but not the header). `constant_time_eq` already + /// short-circuits on the length mismatch, but that must never be + /// inferred from reading the code — assert it directly. + #[test] + fn verify_secret_empty_provided_against_a_real_configured_secret_is_rejected() { + assert!(!verify_secret("", "a-real-configured-secret")); + } + + // ── issue_client_cert handler ───────────────────────────────────────── + + const TEST_INTERNAL_SECRET: &str = "test-only-gateway-internal-secret"; + static INIT_INTERNAL_SECRET: std::sync::Once = std::sync::Once::new(); + + /// Pin `GATEWAY_INTERNAL_SECRET` to one fixed known value for every test + /// below that exercises `issue_client_cert`'s auth check. + /// + /// Safe under `cargo test`'s default parallelism even though + /// `internal_secret()`'s `OnceLock` can only be initialized once per + /// process: every caller of this function sets the SAME value before + /// touching the handler, so whichever test thread's env write wins the + /// race to initialize that `OnceLock`, the cached value is identical + /// either way. + fn ensure_internal_secret_configured() { + INIT_INTERNAL_SECRET.call_once(|| { + std::env::set_var("GATEWAY_INTERNAL_SECRET", TEST_INTERNAL_SECRET); + }); + } + + fn header_map_with_secret(secret: &str) -> hyper::HeaderMap { + let mut headers = hyper::HeaderMap::new(); + headers.insert( + "x-gateway-secret", + hyper::header::HeaderValue::from_str(secret).expect("valid header value"), + ); + headers + } + + #[tokio::test] + async fn issue_client_cert_401s_without_the_secret_header() { + ensure_internal_secret_configured(); + let state = test_gateway_state().await; + let resp = issue_client_cert( + State(state), + hyper::HeaderMap::new(), + axum::body::Bytes::from_static(b"{}"), + ) + .await + .into_response(); + assert_eq!(resp.status(), StatusCode::UNAUTHORIZED); + } + + #[tokio::test] + async fn issue_client_cert_401s_with_the_wrong_secret() { + ensure_internal_secret_configured(); + let state = test_gateway_state().await; + let resp = issue_client_cert( + State(state), + header_map_with_secret("not-the-right-secret"), + axum::body::Bytes::from_static(b"{}"), + ) + .await + .into_response(); + assert_eq!(resp.status(), StatusCode::UNAUTHORIZED); + } + + /// QA gap: the server has a REAL, non-empty `GATEWAY_INTERNAL_SECRET` + /// configured (`ensure_internal_secret_configured` guarantees this for + /// every test in this file) — the caller is the one sending an + /// empty-string header value (present header, empty value; a caller that + /// forgot to set it, or a proxy that stripped the value but not the + /// header). Must still 401, exercised at the actual handler, not just + /// inferred from `verify_secret`'s pure-function test above. + #[tokio::test] + async fn issue_client_cert_401s_with_an_empty_secret_header_against_a_configured_secret() { + ensure_internal_secret_configured(); + let state = test_gateway_state().await; + let resp = issue_client_cert( + State(state), + header_map_with_secret(""), + axum::body::Bytes::from_static(b"{}"), + ) + .await + .into_response(); + assert_eq!(resp.status(), StatusCode::UNAUTHORIZED); + } + + /// FIX 3 (security review): an explicit cap ahead of axum's own default + /// body limit — the auth check (which never inspects the body) must not + /// mask this: a correctly authenticated caller sending an oversized body + /// still gets rejected, before any JSON parsing is attempted. + #[tokio::test] + async fn issue_client_cert_413s_on_an_oversized_body_even_with_the_right_secret() { + ensure_internal_secret_configured(); + let state = test_gateway_state().await; + let oversized = axum::body::Bytes::from(vec![b'a'; MAX_CLIENT_CERT_REQUEST_BODY_BYTES + 1]); + let resp = issue_client_cert( + State(state), + header_map_with_secret(TEST_INTERNAL_SECRET), + oversized, + ) + .await + .into_response(); + assert_eq!(resp.status(), StatusCode::PAYLOAD_TOO_LARGE); + } + + /// A body right at the cap is NOT rejected by the size check (only + /// `> MAX`, matching the plan's "cap", not an off-by-one). + #[tokio::test] + async fn issue_client_cert_accepts_a_body_exactly_at_the_cap_past_the_size_check() { + ensure_internal_secret_configured(); + let state = test_gateway_state().await; + // Right at the cap, but not valid JSON — proves the size check let + // it through (400 from the JSON parse), not a size rejection (413). + let mut body = vec![b' '; MAX_CLIENT_CERT_REQUEST_BODY_BYTES]; + body[0] = b'{'; // still invalid JSON, deliberately, to isolate the check under test + let resp = issue_client_cert( + State(state), + header_map_with_secret(TEST_INTERNAL_SECRET), + axum::body::Bytes::from(body), + ) + .await + .into_response(); + assert_ne!(resp.status(), StatusCode::PAYLOAD_TOO_LARGE); + } + + #[tokio::test] + async fn issue_client_cert_503s_when_no_minting_authority_is_configured() { + ensure_internal_secret_configured(); + // `test_gateway_state()` leaves `client_ca: None` — the "operator set + // GATEWAY_CLIENT_CA, no matching key" case described on the field's + // doc comment and implemented in `main.rs`. + let state = test_gateway_state().await; + let resp = issue_client_cert( + State(state), + header_map_with_secret(TEST_INTERNAL_SECRET), + axum::body::Bytes::from_static( + br#"{"host_id":"h","spiffe_uri":"spiffe://onecli/host/h","csr_pem":"x"}"#, + ), + ) + .await + .into_response(); + assert_eq!(resp.status(), StatusCode::SERVICE_UNAVAILABLE); + } + + #[tokio::test] + async fn issue_client_cert_400s_on_malformed_json_with_a_valid_secret_and_authority() { + ensure_internal_secret_configured(); + let tmp = tempfile::tempdir().expect("tempdir"); + let authority = client_ca_authority::ClientCa::load_or_generate(tmp.path()) + .await + .expect("client ca authority"); + let mut state = test_gateway_state().await; + state.client_ca = Some(Arc::new(authority)); + + let resp = issue_client_cert( + State(state), + header_map_with_secret(TEST_INTERNAL_SECRET), + axum::body::Bytes::from_static(b"not json"), + ) + .await + .into_response(); + assert_eq!(resp.status(), StatusCode::BAD_REQUEST); + } } diff --git a/apps/gateway/src/main.rs b/apps/gateway/src/main.rs index edca7c79..1499a0e2 100644 --- a/apps/gateway/src/main.rs +++ b/apps/gateway/src/main.rs @@ -11,6 +11,7 @@ mod auth; mod ca; mod client_ca; +mod client_ca_authority; #[cfg(not(edition_cloud))] mod cache; @@ -240,11 +241,41 @@ async fn main() -> Result<()> { let ca = CertificateAuthority::load_or_generate(&data_dir).await?; info!("CA certificate loaded"); + // Client-certificate minting authority (Phase 2). Only meaningful when + // the mTLS trust anchor is the gateway's OWN generated client CA: if an + // operator has configured GATEWAY_CLIENT_CA (Phase 1 — an externally + // managed trust anchor cert, whose matching private key we never hold), + // minting against a locally generated CA would produce certificates + // nobody trusts. In that case, skip generating/loading a client CA + // entirely and leave minting unavailable (the internal endpoint 503s) + // rather than silently minting from an unrelated CA. Any OTHER failure + // here (a corrupt on-disk key, an unwritable data dir, ...) aborts + // startup — fail closed, mirroring `ca::CertificateAuthority`. + let operator_configured_client_ca = std::env::var("GATEWAY_CLIENT_CA") + .ok() + .is_some_and(|v| !v.trim().is_empty()); + let client_ca: Option> = if operator_configured_client_ca { + info!( + "GATEWAY_CLIENT_CA is set — client-certificate minting stays unavailable (the \ + internal endpoint 503s); the trust anchor is externally managed" + ); + None + } else { + let authority = client_ca_authority::ClientCa::load_or_generate(&data_dir).await?; + info!("client-certificate CA loaded"); + Some(Arc::new(authority)) + }; + let fallback_client_ca_pem = client_ca.as_ref().map(|c| c.ca_cert_pem()); + // mTLS is opt-in: unset GATEWAY_MTLS_PORT and this is a no-op (full // backward compatibility). When it IS requested, any load failure here // must abort startup — the gateway must never silently fall back to // plaintext-only when mTLS was asked for. - let mtls = client_ca::MtlsConfig::from_env(ca.ca_cert_der(), cli.port)?; + let mtls = client_ca::MtlsConfig::from_env( + ca.ca_cert_der(), + cli.port, + fallback_client_ca_pem.as_deref(), + )?; match &mtls { Some(m) => info!(port = m.port, "mTLS client-certificate listener configured"), None => info!("mTLS disabled (GATEWAY_MTLS_PORT not set)"), @@ -329,6 +360,7 @@ async fn main() -> Result<()> { cache, approval_store, mtls, + client_ca, )?; let result = server.run().await; diff --git a/packages/api/src/app.ts b/packages/api/src/app.ts index a01fb2e7..00c06496 100644 --- a/packages/api/src/app.ts +++ b/packages/api/src/app.ts @@ -54,7 +54,11 @@ import { userRoutes } from "./routes/user"; import { appRoutes } from "./routes/apps"; import { connectionRoutes } from "./routes/connections"; import { vaultRoutes } from "./routes/vaults"; -import { gatewayUrlRoutes, gatewayCaRoutes } from "./routes/gateway"; +import { + gatewayUrlRoutes, + gatewayCaRoutes, + clientCertRoutes, +} from "./routes/gateway"; import { containerConfigRoutes } from "./routes/container-config"; import { countsRoutes } from "./routes/counts"; import { skillRoutes } from "./routes/skill"; @@ -155,6 +159,7 @@ export const createApiApp = ( app.route("/vaults", vaultRoutes()); app.route("/gateway-url", gatewayUrlRoutes()); app.route("/gateway", gatewayCaRoutes()); + app.route("/gateway", clientCertRoutes()); app.route("/container-config", containerConfigRoutes()); app.route("/counts", countsRoutes()); app.route("/skill", skillRoutes()); diff --git a/packages/api/src/lib/gateway-client-cert.ts b/packages/api/src/lib/gateway-client-cert.ts new file mode 100644 index 00000000..be858160 --- /dev/null +++ b/packages/api/src/lib/gateway-client-cert.ts @@ -0,0 +1,97 @@ +/** + * Outbound call to the gateway's internal client-cert minting endpoint. + * + * Direction: Node -> gateway (the opposite of `gateway-invalidate.ts`'s + * cache-flush calls, which also go Node -> gateway but ride the caller's own + * session/API-key auth). This one authenticates with the shared + * `X-Gateway-Secret` instead — mirroring the gateway's own outbound calls to + * Node's `/v1/internal/onepassword/*` endpoints in + * `apps/gateway/src/vault/onepassword_api.rs`, just in the other direction + * and against the gateway's `/v1/internal/client-cert/issue`. + */ +import { GATEWAY_API_URL, GATEWAY_INTERNAL_SECRET } from "./env"; +import { ServiceError } from "../services/errors"; + +export interface MintClientCertParams { + hostId: string; + spiffeUri: string; + csrPem: string; + lifetimeSecs?: number; +} + +export interface MintClientCertResult { + certPem: string; + caPem: string; + serial: string; + notAfter: number; +} + +interface GatewayIssueResponse { + cert_pem: string; + ca_pem: string; + serial_hex: string; + not_after_unix: number; +} + +/** + * Ask the gateway to mint a client certificate from a CSR. Never sends or + * receives a private key — the CSR is the client's own proof of possession; + * the gateway signs over its public key alone (see the `SECURITY` note on + * `ClientCa::sign_csr` in the gateway). + * + * Maps the gateway's 400 (malformed/tampered CSR) to a `ServiceError` the + * route can translate straight to its own 400; 503 (no minting authority + * configured) and any other non-2xx become distinct `ServiceError`s so the + * route doesn't have to inspect status codes itself. + */ +export const mintClientCert = async ( + params: MintClientCertParams, +): Promise => { + const response = await fetch( + `${GATEWAY_API_URL}/v1/internal/client-cert/issue`, + { + method: "POST", + headers: { + "content-type": "application/json", + "x-gateway-secret": GATEWAY_INTERNAL_SECRET, + }, + body: JSON.stringify({ + host_id: params.hostId, + spiffe_uri: params.spiffeUri, + csr_pem: params.csrPem, + lifetime_secs: params.lifetimeSecs, + }), + }, + ).catch((err) => { + throw new ServiceError( + "SERVICE_UNAVAILABLE", + `gateway client-cert endpoint unreachable: ${err instanceof Error ? err.message : String(err)}`, + ); + }); + + if (response.status === 400) { + const body = await response.json().catch(() => null); + const message = (body as { error?: string } | null)?.error ?? "invalid CSR"; + throw new ServiceError("BAD_REQUEST", message); + } + if (response.status === 503) { + throw new ServiceError( + "SERVICE_UNAVAILABLE", + "client certificate minting is not available", + ); + } + if (!response.ok) { + throw new ServiceError( + "SERVICE_UNAVAILABLE", + `gateway client-cert endpoint returned ${response.status}`, + ); + } + + const body: GatewayIssueResponse = await response.json(); + return { + certPem: body.cert_pem, + caPem: body.ca_pem, + serial: body.serial_hex, + notAfter: body.not_after_unix, + }; +}; diff --git a/packages/api/src/middleware/error-handler.ts b/packages/api/src/middleware/error-handler.ts index fca82ef5..87fc612b 100644 --- a/packages/api/src/middleware/error-handler.ts +++ b/packages/api/src/middleware/error-handler.ts @@ -10,6 +10,7 @@ const STATUS_MAP = { CONFLICT: 409, FORBIDDEN: 403, GONE: 410, + SERVICE_UNAVAILABLE: 503, } as const satisfies Record; const ERROR_TYPE_MAP: Record = { @@ -19,6 +20,7 @@ const ERROR_TYPE_MAP: Record = { CONFLICT: "invalid_request_error", FORBIDDEN: "authentication_error", GONE: "invalid_request_error", + SERVICE_UNAVAILABLE: "api_error", }; const DOCS_URL = "https://onecli.sh/docs/api-reference"; diff --git a/packages/api/src/routes/gateway-client-cert.test.ts b/packages/api/src/routes/gateway-client-cert.test.ts new file mode 100644 index 00000000..ebb5f4bd --- /dev/null +++ b/packages/api/src/routes/gateway-client-cert.test.ts @@ -0,0 +1,318 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +/** + * Route-level tests for `POST /gateway/client-cert` (mounted at + * `/v1/gateway/client-cert` in the full app). Focus: the auth boundary (no + * session/API key -> 401, before any minting work happens), and that + * `withAudit` is called with the right action/service and metadata that + * never carries cert/key material — the property CLAUDE.md's audit-logging + * section requires ("Never include sensitive values"). + * + * `../lib/gateway-client-cert` is mocked so this never makes a real network + * call to a gateway; `../middleware/auth` is replaced with a minimal stub + * gated on a test-only header, so the 401 case exercises a REAL "no auth + * context reaches the handler" path rather than assuming the real + * session/API-key resolution would 401 (that's `middleware/auth.test.ts`'s + * job). + */ + +interface FakeClientHostRow { + id: string; + projectId: string; + organizationId?: string; + label?: string; + spiffeUri: string; +} + +const state = vi.hoisted(() => ({ + auditRows: [] as Record[], + mintCalls: [] as Record[], + clientHosts: [] as FakeClientHostRow[], + clientHostUpdates: [] as Record[], +})); + +vi.mock("@onecli/db", () => ({ + Prisma: { JsonNull: null }, + db: { + clientHost: { + create: async (args: { data: FakeClientHostRow }) => { + state.clientHosts.push({ ...args.data }); + return { id: args.data.id, spiffeUri: args.data.spiffeUri }; + }, + // Tenant-scoped lookup — mirrors the real Prisma query exactly + // (`where: { id, projectId }`): a row belonging to a DIFFERENT + // projectId is indistinguishable from "doesn't exist" here, same as + // in Postgres, which is the property the IDOR guard depends on. + findFirst: async (args: { + where: { id: string; projectId: string }; + select?: unknown; + }) => { + const found = state.clientHosts.find( + (h) => h.id === args.where.id && h.projectId === args.where.projectId, + ); + return found ? { id: found.id, spiffeUri: found.spiffeUri } : null; + }, + update: async (args: Record) => { + state.clientHostUpdates.push(args); + return {}; + }, + }, + auditLog: { + create: async (args: { data: Record }) => { + state.auditRows.push(args.data); + return {}; + }, + }, + apiKey: { + findFirst: async () => null, + findMany: async () => [], + }, + }, +})); + +vi.mock("../lib/gateway-client-cert", () => ({ + mintClientCert: async (params: Record) => { + state.mintCalls.push(params); + return { + certPem: "LEAF-AND-CA-PEM", + caPem: "CA-ONLY-PEM", + serial: "abc123serial", + notAfter: 1893456000, + }; + }, +})); + +vi.mock("../middleware/auth", () => ({ + authMiddleware: async ( + c: { + req: { header: (name: string) => string | undefined }; + set: (key: string, value: unknown) => void; + json: (body: unknown, status: number) => Response; + }, + next: () => Promise, + ) => { + if (c.req.header("x-test-authed") !== "yes") { + return c.json( + { + error: { + message: "Invalid API key or token.", + type: "authentication_error", + }, + }, + 401, + ); + } + c.set("auth", { + userId: "user-1", + userEmail: "guy@acme.com", + organizationId: "org-1", + projectId: "proj-1", + }); + return next(); + }, + requireProjectId: (auth: { projectId?: string }) => { + if (!auth.projectId) throw new Error("no project"); + return auth.projectId; + }, +})); + +import { errorHandler } from "../middleware/error-handler"; +import { clientCertRoutes } from "./gateway"; + +const VALID_CSR = + "-----BEGIN CERTIFICATE REQUEST-----\nMIIBazCB7QIBADAA\n-----END CERTIFICATE REQUEST-----\n"; + +// `clientCertRoutes()` on its own has no error boundary — that's registered +// once on the root app in `app.ts` (`app.onError(errorHandler)`). Attach the +// same handler here so a thrown `ServiceError` (e.g. a validation failure) +// maps to its real status code instead of Hono's generic 500. +const app = clientCertRoutes(); +app.onError(errorHandler); + +const post = (body: unknown, authed: boolean) => + app.request("/client-cert", { + method: "POST", + headers: { + "content-type": "application/json", + ...(authed ? { "x-test-authed": "yes" } : {}), + }, + body: JSON.stringify(body), + }); + +beforeEach(() => { + state.auditRows = []; + state.mintCalls = []; + state.clientHosts = []; + state.clientHostUpdates = []; +}); + +describe("POST /gateway/client-cert", () => { + it("401s without auth, before any minting work happens", async () => { + const res = await post({ csrPem: VALID_CSR }, false); + expect(res.status).toBe(401); + expect(state.mintCalls).toHaveLength(0); + expect(state.clientHosts).toHaveLength(0); + expect(state.auditRows).toHaveLength(0); + }); + + it("400s on a body missing csrPem", async () => { + const res = await post({ label: "no-csr" }, true); + expect(res.status).toBe(400); + expect(state.mintCalls).toHaveLength(0); + }); + + it("400s on a body carrying a key field instead of/alongside a CSR", async () => { + const res = await post( + { + csrPem: VALID_CSR, + keyPem: "-----BEGIN PRIVATE KEY-----\nabc\n-----END PRIVATE KEY-----\n", + }, + true, + ); + expect(res.status).toBe(400); + expect(state.mintCalls).toHaveLength(0); + }); + + it("mints, audits ISSUE/client-cert with no key material, and returns the expected shape", async () => { + const res = await post({ csrPem: VALID_CSR, label: "ci-runner-1" }, true); + expect(res.status).toBe(200); + + const body = (await res.json()) as { + identity: string; + hostId: string; + certPem: string; + caPem: string; + serial: string; + notAfter: number; + }; + expect(body.certPem).toBe("LEAF-AND-CA-PEM"); + expect(body.caPem).toBe("CA-ONLY-PEM"); + expect(body.serial).toBe("abc123serial"); + expect(body.notAfter).toBe(1893456000); + expect(body.identity).toMatch(/^spiffe:\/\/onecli\/host\/.+/); + expect(body.hostId).toBeTruthy(); + // No key field anywhere in the response. + expect(Object.keys(body).sort()).toEqual( + ["caPem", "certPem", "hostId", "identity", "notAfter", "serial"].sort(), + ); + + expect(state.mintCalls).toHaveLength(1); + expect(state.mintCalls[0]).toMatchObject({ + hostId: body.hostId, + spiffeUri: body.identity, + // The schema trims csrPem — assert against the trimmed form. + csrPem: VALID_CSR.trim(), + }); + + expect(state.auditRows).toHaveLength(1); + const audit = state.auditRows[0] as { + action: string; + service: string; + userId: string; + userEmail: string; + projectId: string; + organizationId: string; + metadata: Record; + }; + expect(audit.action).toBe("issue"); + expect(audit.service).toBe("client-cert"); + expect(audit.userId).toBe("user-1"); + expect(audit.userEmail).toBe("guy@acme.com"); + expect(audit.projectId).toBe("proj-1"); + expect(audit.organizationId).toBe("org-1"); + + // The metadata guideline from CLAUDE.md: resource identifiers only, never + // cert/key material. Assert both the exact shape AND (belt-and-braces) + // that no PEM-looking string ever made it into the metadata blob. + expect(audit.metadata).toEqual({ + hostId: body.hostId, + spiffeUri: body.identity, + serial: "abc123serial", + notAfter: 1893456000, + }); + expect(JSON.stringify(audit.metadata)).not.toContain("PEM"); + expect(JSON.stringify(audit.metadata)).not.toContain("PRIVATE KEY"); + + // The ClientHost row was stamped with the mint's serial after issuance. + expect(state.clientHostUpdates).toHaveLength(1); + }); + + // ── FIX 1: ClientHost is per-HOST, not per-enrollment call ──────────── + + it("first enrollment (no hostId) creates a new host", async () => { + const res = await post({ csrPem: VALID_CSR, label: "relay-1" }, true); + expect(res.status).toBe(200); + expect(state.clientHosts).toHaveLength(1); + + const body = (await res.json()) as { hostId: string; identity: string }; + expect(state.clientHosts[0]?.id).toBe(body.hostId); + expect(state.clientHosts[0]?.spiffeUri).toBe(body.identity); + }); + + it("omitting hostId always creates a NEW host, even for the same caller", async () => { + await post({ csrPem: VALID_CSR }, true); + await post({ csrPem: VALID_CSR }, true); + expect(state.clientHosts).toHaveLength(2); + expect(state.clientHosts[0]?.id).not.toBe(state.clientHosts[1]?.id); + }); + + it("renewal with the caller's own hostId reuses the same identity (no new row)", async () => { + const first = await post({ csrPem: VALID_CSR, label: "relay-1" }, true); + const firstBody = (await first.json()) as { + hostId: string; + identity: string; + }; + expect(state.clientHosts).toHaveLength(1); + + const renewed = await post( + { csrPem: VALID_CSR, hostId: firstBody.hostId }, + true, + ); + expect(renewed.status).toBe(200); + const renewedBody = (await renewed.json()) as { + hostId: string; + identity: string; + }; + + expect(renewedBody.hostId).toBe(firstBody.hostId); + expect(renewedBody.identity).toBe(firstBody.identity); + // Still exactly one row — the renewal reused it, it didn't create a + // second one. + expect(state.clientHosts).toHaveLength(1); + expect(state.mintCalls).toHaveLength(2); + expect(state.clientHostUpdates).toHaveLength(2); + }); + + // IDOR guard: an authenticated caller in one project must not be able to + // reuse (or even learn the existence of) a ClientHost row that belongs to + // a DIFFERENT project by guessing/supplying its hostId. + it("renewal with another tenant's hostId is rejected — never falls through to creating a new row", async () => { + state.clientHosts.push({ + id: "11111111-1111-4111-8111-111111111111", + projectId: "some-other-tenants-project", + spiffeUri: "spiffe://onecli/host/11111111-1111-4111-8111-111111111111", + }); + + const res = await post( + { csrPem: VALID_CSR, hostId: "11111111-1111-4111-8111-111111111111" }, + true, + ); + + expect(res.status).toBe(404); + expect(state.mintCalls).toHaveLength(0); + expect(state.auditRows).toHaveLength(0); + // No fallback create happened — still just the one seeded (other + // tenant's) row. + expect(state.clientHosts).toHaveLength(1); + }); + + it("renewal with a hostId that doesn't exist at all is rejected the same way", async () => { + const res = await post( + { csrPem: VALID_CSR, hostId: "22222222-2222-4222-8222-222222222222" }, + true, + ); + expect(res.status).toBe(404); + expect(state.mintCalls).toHaveLength(0); + expect(state.clientHosts).toHaveLength(0); + }); +}); diff --git a/packages/api/src/routes/gateway.ts b/packages/api/src/routes/gateway.ts index cc6379e6..e97fbd7c 100644 --- a/packages/api/src/routes/gateway.ts +++ b/packages/api/src/routes/gateway.ts @@ -1,6 +1,20 @@ import { Hono } from "hono"; import { GATEWAY_API_URL } from "../lib/env"; import { loadCaCertificate } from "../lib/gateway-ca"; +import { mintClientCert } from "../lib/gateway-client-cert"; +import { authMiddleware, requireProjectId } from "../middleware/auth"; +import { + withAudit, + AUDIT_ACTIONS, + AUDIT_SERVICES, +} from "../services/audit-service"; +import { + ensureClientHost, + stampClientHostIssued, +} from "../services/client-host-service"; +import { ServiceError } from "../services/errors"; +import type { ApiEnv } from "../types"; +import { clientCertEnrollSchema } from "../validations/client-cert"; // Public discovery endpoint — no auth, mirroring the `/gateway/ca` sibling // below. It returns only the deployment's gateway proxy URL: a static, @@ -40,3 +54,96 @@ export const gatewayCaRoutes = () => { return app; }; + +/** + * `POST /v1/gateway/client-cert` — enroll (or renew) a host for gateway + * mTLS: mint a short-lived client certificate from a CSR the caller already + * holds the matching private key for. The private key never crosses this + * boundary in either direction — only the CSR (proof of possession) goes out + * to the gateway, and only the signed cert chain comes back (see + * `clientCertEnrollSchema`'s `.strict()` and `MintClientCertResult`, neither + * of which has a key field). + * + * Identity model: `ClientHost` is per-HOST, not per-call. First call omits + * `hostId` and gets back a freshly minted one (`identity`/`hostId` in the + * response); a relay's certificate renewal — re-calling this SAME endpoint + * before/after its short-lived cert expires — passes that `hostId` back so + * `ensureClientHost` re-mints against the SAME row/identity instead of + * accumulating a new one per renewal. `ensureClientHost` scopes that lookup + * to `projectId` (never trusts `hostId` alone) — see its doc comment for the + * IDOR guard this is load-bearing for. + * + * Auth is the same session-or-API-key `authMiddleware` every other + * project-scoped route here uses — this is NOT the internal + * gateway<->Node boundary (`GATEWAY_INTERNAL_SECRET`/`X-Gateway-Secret`), + * which only `mintClientCert` (via `gateway-client-cert.ts`) speaks, one + * layer down. + * + * TODO(phase-followup): rate-limit this route. It's authenticated (session + * or API key), but nothing yet caps how often a caller can re-mint — out of + * scope for this change. + */ +export const clientCertRoutes = () => { + const app = new Hono(); + app.use("*", authMiddleware); + + app.post("/client-cert", async (c) => { + const auth = c.get("auth"); + const projectId = requireProjectId(auth); + + const body = await c.req.json().catch(() => null); + const parsed = clientCertEnrollSchema.safeParse(body); + if (!parsed.success) { + throw new ServiceError( + "BAD_REQUEST", + parsed.error.issues[0]?.message ?? "Invalid request body", + ); + } + + const { id: hostId, spiffeUri } = await ensureClientHost( + projectId, + auth.organizationId, + parsed.data.label, + parsed.data.hostId, + ); + + const minted = await withAudit( + () => + mintClientCert({ + hostId, + spiffeUri, + csrPem: parsed.data.csrPem, + }), + (result) => ({ + projectId, + organizationId: auth.organizationId, + userId: auth.userId, + userEmail: auth.userEmail, + action: AUDIT_ACTIONS.ISSUE, + service: AUDIT_SERVICES.CLIENT_CERT, + // Resource identifiers only — never certPem/caPem (chain material) + // and there is no key to leak in the first place (see the schema + // and `MintClientCertResult` doc comments above). + metadata: { + hostId, + spiffeUri, + serial: result.serial, + notAfter: result.notAfter, + }, + }), + ); + + await stampClientHostIssued(hostId, minted.serial); + + return c.json({ + identity: spiffeUri, + hostId, + certPem: minted.certPem, + caPem: minted.caPem, + serial: minted.serial, + notAfter: minted.notAfter, + }); + }); + + return app; +}; diff --git a/packages/api/src/services/audit-service.ts b/packages/api/src/services/audit-service.ts index 1c4e7486..0b05dee3 100644 --- a/packages/api/src/services/audit-service.ts +++ b/packages/api/src/services/audit-service.ts @@ -20,6 +20,10 @@ export const AUDIT_ACTIONS = { // EE-only (identity): a claimed resource passed its ownership proof // (e.g. an org domain's DNS TXT check). VERIFY: "verify", + // Client-certificate minting (gateway mTLS): a host enrolls and receives a + // signed leaf. Never DELETE/REGENERATE — there is no revocation flow yet + // (see `ClientHost.revokedAt`'s doc comment), so every mint is an ISSUE. + ISSUE: "issue", } as const; export const AUDIT_SERVICES = { @@ -69,6 +73,8 @@ export const AUDIT_SERVICES = { ROLE_MAPPING: "role-mapping", // EE-only (directory): bearer tokens for the org's SCIM endpoint SCIM_TOKEN: "scim-token", + // Gateway mTLS client-certificate issuance (`POST /v1/gateway/client-cert`). + CLIENT_CERT: "client-cert", } as const; export const AUDIT_STATUS = { diff --git a/packages/api/src/services/client-host-service.ts b/packages/api/src/services/client-host-service.ts new file mode 100644 index 00000000..d9da5084 --- /dev/null +++ b/packages/api/src/services/client-host-service.ts @@ -0,0 +1,86 @@ +import { randomUUID } from "crypto"; +import { db } from "@onecli/db"; +import { ServiceError } from "./errors"; + +/** + * Resolve a `ClientHost`'s stable identity for an enrollment request: either + * mints a brand-new one, or reuses an existing one for renewal. + * + * `ClientHost` is per-HOST, not per-enrollment call: this is what makes + * `spiffeUri` a durable identity a relay can keep presenting across + * certificate renewals (re-calling this same endpoint, short-lived certs by + * design) rather than a new row — and identity — minted every time. + * + * - `hostId` omitted (first enrollment): create a fresh row, server-generated + * `id` + `spiffe://onecli/host/`, and return it. + * - `hostId` provided (renewal): look up that row SCOPED TO `projectId` — + * the caller's own tenant, from `AuthContext`, never trust the client's + * `hostId` alone — and reuse its existing `id`/`spiffeUri`. Never falls + * through to creating a new row on a lookup miss. + * + * IDOR guard: `findFirst({ where: { id: hostId, projectId } })` returns + * nothing for BOTH "no such host" and "that host belongs to a different + * project" — the two cases are handled identically (a `NOT_FOUND` thrown + * here, not silently falling through to `create`), so a caller probing + * another tenant's `hostId` cannot distinguish "wrong tenant" from "doesn't + * exist" and never learns anything about a row it doesn't own. + * + * The id is generated client-side (`randomUUID()`, passed explicitly to + * `create` instead of relying on the schema's `@default(uuid())`) + * specifically so `spiffeUri` can be derived from it and written in the same + * insert; deriving it from a DB-assigned id would need a second round-trip + * update, racy on the column's `@unique` constraint if two enrollments ever + * landed between create and update. + * + * `label` has no uniqueness constraint (it's a human-readable hint, not an + * identifier) — a project can enroll multiple unlabeled hosts, each getting + * its own row and its own spiffe URI. + */ +export const ensureClientHost = async ( + projectId: string, + organizationId: string | undefined, + label: string | undefined, + hostId: string | undefined, +): Promise<{ id: string; spiffeUri: string }> => { + if (hostId) { + const existing = await db.clientHost.findFirst({ + where: { id: hostId, projectId }, + select: { id: true, spiffeUri: true }, + }); + if (!existing) { + throw new ServiceError("NOT_FOUND", "Client host not found"); + } + return existing; + } + + const id = randomUUID(); + const spiffeUri = `spiffe://onecli/host/${id}`; + + const host = await db.clientHost.create({ + data: { + id, + projectId, + organizationId, + label, + spiffeUri, + }, + select: { id: true, spiffeUri: true }, + }); + + return host; +}; + +/** + * Stamp `lastSerial`/`lastIssuedAt` on a `ClientHost` row after a successful + * mint. Never touches `revokedAt` — that field has no writer yet (see its + * schema doc comment); this is purely a "when did we last mint" record. + */ +export const stampClientHostIssued = async ( + hostId: string, + serial: string, +): Promise => { + await db.clientHost.update({ + where: { id: hostId }, + data: { lastSerial: serial, lastIssuedAt: new Date() }, + }); +}; diff --git a/packages/api/src/services/errors.ts b/packages/api/src/services/errors.ts index 1f23a8b7..8b787023 100644 --- a/packages/api/src/services/errors.ts +++ b/packages/api/src/services/errors.ts @@ -4,7 +4,13 @@ export type ServiceErrorCode = | "UNPROCESSABLE" | "CONFLICT" | "FORBIDDEN" - | "GONE"; + | "GONE" + // A downstream dependency the request needs is not configured/available — + // e.g. the gateway has no client-CA minting authority (an operator's + // externally managed GATEWAY_CLIENT_CA, no matching key). Distinct from + // BAD_REQUEST: the request itself is fine, retrying it won't help until + // the deployment is (re)configured. + | "SERVICE_UNAVAILABLE"; export class ServiceError extends Error { readonly code: ServiceErrorCode; diff --git a/packages/api/src/validations/client-cert.test.ts b/packages/api/src/validations/client-cert.test.ts new file mode 100644 index 00000000..2ced0b4e --- /dev/null +++ b/packages/api/src/validations/client-cert.test.ts @@ -0,0 +1,128 @@ +import { describe, expect, it } from "vitest"; + +import { clientCertEnrollSchema } from "./client-cert"; + +const VALID_CSR = + "-----BEGIN CERTIFICATE REQUEST-----\nMIIBazCB7QIBADAA\n-----END CERTIFICATE REQUEST-----\n"; + +describe("clientCertEnrollSchema", () => { + it("accepts a well-formed CSR with no label", () => { + const result = clientCertEnrollSchema.safeParse({ csrPem: VALID_CSR }); + expect(result.success).toBe(true); + }); + + it("accepts a well-formed CSR with a label", () => { + const result = clientCertEnrollSchema.safeParse({ + csrPem: VALID_CSR, + label: "ci-runner-1", + }); + expect(result.success).toBe(true); + }); + + it("rejects a body missing csrPem", () => { + const result = clientCertEnrollSchema.safeParse({ label: "no-csr" }); + expect(result.success).toBe(false); + }); + + it("rejects an empty csrPem", () => { + const result = clientCertEnrollSchema.safeParse({ csrPem: "" }); + expect(result.success).toBe(false); + }); + + it("rejects a csrPem that isn't a CERTIFICATE REQUEST PEM", () => { + const notACsr = + "-----BEGIN CERTIFICATE-----\nMIIBazCB7QIBADAA\n-----END CERTIFICATE-----\n"; + const result = clientCertEnrollSchema.safeParse({ csrPem: notACsr }); + expect(result.success).toBe(false); + }); + + it("rejects plain garbage as csrPem", () => { + const result = clientCertEnrollSchema.safeParse({ + csrPem: "not a csr at all", + }); + expect(result.success).toBe(false); + }); + + // The schema is the enforcement point for "no key material in this + // request" — `.strict()` means ANY extra field fails validation, not just + // a specifically-named one, so a client that hands over a key by mistake + // (whatever it calls the field) gets rejected. + it.each(["keyPem", "privateKey", "key"])( + "rejects a body containing a %s field", + (fieldName) => { + const result = clientCertEnrollSchema.safeParse({ + csrPem: VALID_CSR, + [fieldName]: + "-----BEGIN PRIVATE KEY-----\nabc\n-----END PRIVATE KEY-----\n", + }); + expect(result.success).toBe(false); + }, + ); + + it("rejects an empty label", () => { + const result = clientCertEnrollSchema.safeParse({ + csrPem: VALID_CSR, + label: "", + }); + expect(result.success).toBe(false); + }); + + it("rejects an oversized label", () => { + const result = clientCertEnrollSchema.safeParse({ + csrPem: VALID_CSR, + label: "a".repeat(256), + }); + expect(result.success).toBe(false); + }); + + // FIX 1 (per-host identity model): hostId is optional (first enrollment + // omits it) but must be a real uuid when provided (renewal). + it("accepts a well-formed CSR with a valid hostId", () => { + const result = clientCertEnrollSchema.safeParse({ + csrPem: VALID_CSR, + hostId: "11111111-1111-4111-8111-111111111111", + }); + expect(result.success).toBe(true); + }); + + it("rejects a non-uuid hostId", () => { + const result = clientCertEnrollSchema.safeParse({ + csrPem: VALID_CSR, + hostId: "not-a-uuid", + }); + expect(result.success).toBe(false); + }); + + // A CSR-shaped string (has the PEM marker) at an exact total length, with + // no leading/trailing whitespace — so `.trim()` (which runs before + // `.max()` in the chain) is a no-op and doesn't shift the boundary. + const csrPemOfLength = (totalLength: number) => { + const header = "-----BEGIN CERTIFICATE REQUEST-----\n"; + const footer = "\n-----END CERTIFICATE REQUEST-----"; + const padLength = totalLength - header.length - footer.length; + return header + "A".repeat(Math.max(padLength, 0)) + footer; + }; + + // Matches the gateway's MAX_CLIENT_CERT_REQUEST_BODY_BYTES (16KB): an + // oversized CSR must be rejected HERE, at the Node layer, rather than + // being forwarded to the gateway only to be capped there. + it("accepts a csrPem exactly at the 16KB cap", () => { + const csr = csrPemOfLength(16384); + expect(csr.length).toBe(16384); + const result = clientCertEnrollSchema.safeParse({ csrPem: csr }); + expect(result.success).toBe(true); + }); + + it("rejects a csrPem one byte over the 16KB cap", () => { + const csr = csrPemOfLength(16385); + expect(csr.length).toBe(16385); + const result = clientCertEnrollSchema.safeParse({ csrPem: csr }); + expect(result.success).toBe(false); + }); + + it("rejects a wildly oversized csrPem", () => { + const csr = csrPemOfLength(1_000_000); + const result = clientCertEnrollSchema.safeParse({ csrPem: csr }); + expect(result.success).toBe(false); + }); +}); diff --git a/packages/api/src/validations/client-cert.ts b/packages/api/src/validations/client-cert.ts new file mode 100644 index 00000000..e5e70387 --- /dev/null +++ b/packages/api/src/validations/client-cert.ts @@ -0,0 +1,39 @@ +import { z } from "zod"; + +/** + * Body of `POST /v1/gateway/client-cert`. `.strict()` deliberately — this + * schema is also the enforcement point for "no key material in the + * request": any extra field (a `keyPem`, `privateKey`, ...) fails validation + * outright rather than being silently stripped, so a client that + * misunderstands the flow (and tries to hand over a key instead of proving + * possession via CSR) gets a clear 400, not a request that quietly ignores + * what it sent. + * + * `hostId` is optional: omitted on first enrollment (the route mints a new + * `ClientHost`), provided on renewal (the route reuses that host's existing + * identity — see `ensureClientHost`, which is also where the tenant-scoped + * lookup enforcing this lives; this schema only checks the shape). + */ +export const clientCertEnrollSchema = z + .object({ + csrPem: z + .string() + .trim() + .min(1, "csrPem is required") + // Matches the gateway's own MAX_CLIENT_CERT_REQUEST_BODY_BYTES (16KB) + // cap on `POST /v1/internal/client-cert/issue` — a real CSR is a few + // KB at most, so an oversized one is rejected HERE, at the Node layer, + // rather than reaching the gateway (and being forwarded/spent effort + // on) only to be capped there. Keeping the two limits equal means an + // operator reading either file sees the same number. + .max(16384, "csrPem must not exceed 16KB") + .refine( + (v) => v.includes("-----BEGIN CERTIFICATE REQUEST-----"), + "csrPem must be a PEM-encoded CERTIFICATE REQUEST", + ), + label: z.string().trim().min(1).max(255).optional(), + hostId: z.string().uuid("hostId must be a valid host id").optional(), + }) + .strict(); + +export type ClientCertEnrollInput = z.infer; diff --git a/packages/db/prisma/migrations/20260730155821_add_client_hosts/migration.sql b/packages/db/prisma/migrations/20260730155821_add_client_hosts/migration.sql new file mode 100644 index 00000000..1500b9ec --- /dev/null +++ b/packages/db/prisma/migrations/20260730155821_add_client_hosts/migration.sql @@ -0,0 +1,26 @@ +-- CreateTable +CREATE TABLE "client_hosts" ( + "id" TEXT NOT NULL, + "project_id" TEXT NOT NULL, + "organization_id" TEXT, + "label" TEXT, + "spiffe_uri" TEXT NOT NULL, + "last_serial" TEXT, + "last_issued_at" TIMESTAMP(3), + "revoked_at" TIMESTAMP(3), + "created_at" TIMESTAMP(3) NOT NULL DEFAULT CURRENT_TIMESTAMP, + + CONSTRAINT "client_hosts_pkey" PRIMARY KEY ("id") +); + +-- CreateIndex +CREATE UNIQUE INDEX "client_hosts_spiffe_uri_key" ON "client_hosts"("spiffe_uri"); + +-- CreateIndex +CREATE INDEX "client_hosts_project_id_idx" ON "client_hosts"("project_id"); + +-- AddForeignKey +ALTER TABLE "client_hosts" ADD CONSTRAINT "client_hosts_project_id_fkey" FOREIGN KEY ("project_id") REFERENCES "projects"("id") ON DELETE RESTRICT ON UPDATE CASCADE; + +-- AddForeignKey +ALTER TABLE "client_hosts" ADD CONSTRAINT "client_hosts_organization_id_fkey" FOREIGN KEY ("organization_id") REFERENCES "organizations"("id") ON DELETE SET NULL ON UPDATE CASCADE; diff --git a/packages/db/prisma/schema.prisma b/packages/db/prisma/schema.prisma index b7e63767..dfafac12 100644 --- a/packages/db/prisma/schema.prisma +++ b/packages/db/prisma/schema.prisma @@ -69,6 +69,7 @@ model Organization { groups Group[] appAvailabilityRules AppAvailabilityRule[] groupRoleMappings GroupRoleMapping[] + clientHosts ClientHost[] @@index([partnerId]) @@map("organizations") @@ -208,6 +209,7 @@ model Project { appConfigs AppConfig[] budgets Budget[] // cloud-only: per-secret spend budgets for this project (inert in OSS) accessBindings ProjectAccess[] + clientHosts ClientHost[] @@unique([organizationId, slug]) @@index([organizationId]) @@ -318,6 +320,37 @@ model Agent { @@map("agents") } +// Phase 2 (gateway mTLS client-certificate issuance): one row per host that +// has enrolled for a client certificate — NOT per-certificate, NOT per +// enrollment call. `spiffeUri` is this host's stable identity +// (`spiffe://onecli/host/`, derived from `id` in `ensureClientHost`); +// a renewal (the caller passes its existing `id` back as `hostId`, scoped to +// its own project — see `ensureClientHost`'s IDOR guard) re-mints a +// certificate for the SAME row rather than creating a new one, so +// `lastSerial`/`lastIssuedAt` always reflect the most recently minted cert. +// No key material is ever stored here — the gateway signs from a CSR and +// returns only the cert chain. +model ClientHost { + id String @id @default(uuid()) + projectId String @map("project_id") + organizationId String? @map("organization_id") + label String? + spiffeUri String @unique @map("spiffe_uri") + lastSerial String? @map("last_serial") + lastIssuedAt DateTime? @map("last_issued_at") + // Unused hook: no revocation flow exists yet (no CRL/OCSP wiring in the + // gateway's mTLS verifier — see the `with_crls` note in `client_ca.rs`). + // Reserved so a future revocation feature doesn't need a migration. + revokedAt DateTime? @map("revoked_at") + createdAt DateTime @default(now()) @map("created_at") + + project Project @relation(fields: [projectId], references: [id]) + organization Organization? @relation(fields: [organizationId], references: [id]) + + @@index([projectId]) + @@map("client_hosts") +} + model Secret { id String @id @default(uuid()) scope String @default("project") // "project" | "organization" | "partner"