diff --git a/apps/gateway/Cargo.lock b/apps/gateway/Cargo.lock index 6e62c693..94993dc4 100644 --- a/apps/gateway/Cargo.lock +++ b/apps/gateway/Cargo.lock @@ -207,6 +207,45 @@ dependencies = [ "zeroize", ] +[[package]] +name = "asn1-rs" +version = "0.6.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5493c3bedbacf7fd7382c6346bbd66687d12bbaad3a89a2d2c303ee6cf20b048" +dependencies = [ + "asn1-rs-derive", + "asn1-rs-impl", + "displaydoc", + "nom", + "num-traits", + "rusticata-macros", + "thiserror 1.0.69", + "time", +] + +[[package]] +name = "asn1-rs-derive" +version = "0.5.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "965c2d33e53cb6b267e148a4cb0760bc01f4904c1cd4bb4002a085bb016d1490" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.117", + "synstructure", +] + +[[package]] +name = "asn1-rs-impl" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7b18050c2cd6fe86c3a76584ef5e0baf286d038cda203eb6223df2cc413565f7" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.117", +] + [[package]] name = "async-lock" version = "3.4.2" @@ -1174,6 +1213,20 @@ dependencies = [ "zeroize", ] +[[package]] +name = "der-parser" +version = "9.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5cd0a5c643689626bec213c4d8bd4d96acc8ffdb4ad4bb6bc16abf27d5f4b553" +dependencies = [ + "asn1-rs", + "displaydoc", + "nom", + "num-bigint", + "num-traits", + "rusticata-macros", +] + [[package]] name = "deranged" version = "0.5.8" @@ -2179,6 +2232,12 @@ version = "0.3.17" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6877bb514081ee2a7ff5ef9de3281f14a4dd4bceac4c09388074a6b5df8a139a" +[[package]] +name = "minimal-lexical" +version = "0.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "68354c5c6bd36d73ff3feceb05efa59b6acb7626617f4962be322a825e61f79a" + [[package]] name = "mio" version = "1.1.1" @@ -2245,6 +2304,16 @@ dependencies = [ "tempfile", ] +[[package]] +name = "nom" +version = "7.1.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d273983c5a657a70a3e8f2a01329822f3b8c8172b73826411a55751e404a0a4a" +dependencies = [ + "memchr", + "minimal-lexical", +] + [[package]] name = "nu-ansi-term" version = "0.50.3" @@ -2316,6 +2385,15 @@ dependencies = [ "libm", ] +[[package]] +name = "oid-registry" +version = "0.7.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a8d8034d9489cdaf79228eb9f6a3b8d7bb32ba00d6645ebd48eef4077ceb5bd9" +dependencies = [ + "asn1-rs", +] + [[package]] name = "once_cell" version = "1.21.4" @@ -2376,6 +2454,7 @@ dependencies = [ "tracing-subscriber", "uuid", "webpki-roots 0.26.11", + "x509-parser", ] [[package]] @@ -2925,6 +3004,15 @@ dependencies = [ "semver", ] +[[package]] +name = "rusticata-macros" +version = "4.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "faf0c4a6ece9950b9abdb62b1cfcf2a68b3b67a10ba445b3bb85be2a293d0632" +dependencies = [ + "nom", +] + [[package]] name = "rustix" version = "1.1.4" @@ -4613,6 +4701,23 @@ dependencies = [ "zeroize", ] +[[package]] +name = "x509-parser" +version = "0.16.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fcbc162f30700d6f3f82a24bf7cc62ffe7caea42c0b2cba8bf7f3ae50cf51f69" +dependencies = [ + "asn1-rs", + "data-encoding", + "der-parser", + "lazy_static", + "nom", + "oid-registry", + "rusticata-macros", + "thiserror 1.0.69", + "time", +] + [[package]] name = "xmlparser" version = "0.13.6" diff --git a/apps/gateway/Cargo.toml b/apps/gateway/Cargo.toml index ba91eeb1..3d2c37c4 100644 --- a/apps/gateway/Cargo.toml +++ b/apps/gateway/Cargo.toml @@ -26,6 +26,9 @@ tokio-rustls = "0.26" rustls = { version = "0.23", features = ["ring"] } rustls-pemfile = "2" +# X.509 parsing (mTLS client-certificate identity extraction — CN/URI SANs) +x509-parser = "0.16" + # WebSocket upstream TLS root certificates webpki-roots = "0.26" diff --git a/apps/gateway/src/ca.rs b/apps/gateway/src/ca.rs index 19872003..496a8e67 100644 --- a/apps/gateway/src/ca.rs +++ b/apps/gateway/src/ca.rs @@ -125,6 +125,14 @@ impl CertificateAuthority { der_to_pem(self.ca_cert_der.as_ref()) } + /// Return the raw CA certificate DER. + /// Used by `client_ca::MtlsConfig::from_env` to reject a `GATEWAY_CLIENT_CA` + /// that is (accidentally or maliciously) this same CA — see the SECURITY + /// note in `client_ca.rs`. + pub(crate) fn ca_cert_der(&self) -> &CertificateDer<'static> { + &self.ca_cert_der + } + /// Load CA from PEM strings (key + certificate). /// Used when CA is provided via environment variables (cloud mode). fn load_from_pem(key_pem: &str, cert_pem: &str) -> Result { @@ -333,9 +341,11 @@ mod tests { fn ensure_crypto_provider() { INIT_CRYPTO.call_once(|| { - rustls::crypto::ring::default_provider() - .install_default() - .expect("install CryptoProvider"); + // Ignore the error: it just means another test module (e.g. + // `client_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. + let _ = rustls::crypto::ring::default_provider().install_default(); }); } diff --git a/apps/gateway/src/client_ca.rs b/apps/gateway/src/client_ca.rs new file mode 100644 index 00000000..2800ae57 --- /dev/null +++ b/apps/gateway/src/client_ca.rs @@ -0,0 +1,1138 @@ +//! mTLS client-certificate support: config assembly, TLS server config +//! construction, and identity extraction from the client certificate chain. +//! +//! Phase 1 (this module): the gateway can *require* a client certificate on a +//! dedicated port and *extract* an identity from it (CN / URI SAN), but never +//! compares that identity to anything — it's threaded onto [`crate::gateway::ProxyContext`] +//! and logged, nothing more. Phase 2 wires the actual enforcement (comparing +//! `client_identity` against `agent_token`) and CRL support (see the +//! `with_crls` hook noted in `build_server_config`). +//! +//! mTLS is entirely opt-in: unset `GATEWAY_MTLS_PORT` and this module never +//! touches a socket. That keeps the OSS build (and every existing deployment) +//! byte-for-byte backward compatible. + +use std::net::{IpAddr, Ipv4Addr}; +use std::sync::Arc; + +use anyhow::{bail, Context, Result}; +use rustls::pki_types::{CertificateDer, PrivateKeyDer}; +use rustls::server::WebPkiClientVerifier; +use rustls::{RootCertStore, ServerConfig}; + +// ── Identity ───────────────────────────────────────────────────────────── + +/// Identity extracted from a client certificate that already passed the TLS +/// verifier's chain-of-trust and expiry checks. +/// +/// Phase 1 only threads and logs this value — see the module doc. `primary()` +/// is the field Phase 2 will compare against the agent token. +#[derive(Debug, Clone, PartialEq)] +pub(crate) struct ClientIdentity { + pub(crate) cn: Option, + pub(crate) uri_sans: Vec, + pub(crate) serial_hex: String, + pub(crate) not_after_unix: i64, +} + +impl ClientIdentity { + /// The identity used for logging (and, in Phase 2, matching): the first + /// URI SAN if present — agents are expected to mint `spiffe://`-style + /// URIs — else the Common Name. + pub(crate) fn primary(&self) -> Option<&str> { + self.uri_sans + .first() + .map(String::as_str) + .or(self.cn.as_deref()) + } +} + +impl std::fmt::Display for ClientIdentity { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str(self.primary().unwrap_or("unknown")) + } +} + +/// Extract a [`ClientIdentity`] from the peer certificate chain presented +/// during the TLS handshake. `certs[0]` is the end-entity leaf — rustls +/// presents the chain leaf-first — intermediates are ignored. +/// +/// Never panics: an empty slice, a leaf that fails to parse, or a hostile CN/ +/// SAN (see [`sanitize_identity_component`]) all just fall through to `None` +/// pieces rather than a panic. The TLS verifier has already rejected chains +/// that don't verify by the time this runs, so there's nothing to fail closed +/// on here — Phase 1 doesn't enforce, it only reports what it saw. +pub(crate) fn identity_from_peer_certs(certs: &[CertificateDer<'_>]) -> Option { + let leaf = certs.first()?; + let (_, cert) = x509_parser::parse_x509_certificate(leaf.as_ref()).ok()?; + + let cn = cert + .subject() + .iter_common_name() + .next() + .and_then(|attr| attr.as_str().ok()) + .and_then(sanitize_identity_component); + + let uri_sans = cert + .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, + }) + .filter_map(sanitize_identity_component) + .collect() + }) + .unwrap_or_default(); + + let serial_hex = hex::encode(cert.raw_serial()); + let not_after_unix = cert.validity().not_after.timestamp(); + + Some(ClientIdentity { + cn, + uri_sans, + serial_hex, + not_after_unix, + }) +} + +/// Validate a single identity component (a CN or a URI SAN) pulled from an +/// otherwise-trusted certificate. The certificate chains to a trust anchor, +/// but its *content* is still attacker-controlled (anyone who can get a cert +/// signed by the configured client CA picks their own CN/SAN) — this becomes +/// a log field and, in Phase 2, a lookup key, so control characters and +/// oversized values are dropped rather than "cleaned up": a component that +/// fails validation contributes nothing rather than a mangled value. +fn sanitize_identity_component(s: &str) -> Option { + if s.is_empty() || s.len() > 253 { + return None; + } + // Printable ASCII only — this also excludes '\n'/'\r' (0x0A/0x0D), which + // fall outside 0x20..=0x7E; '"' and '\\' are inside that range and need + // an explicit check. + if !s.chars().all(|c| matches!(c, '\u{20}'..='\u{7E}')) { + return None; + } + if s.contains('"') || s.contains('\\') { + return None; + } + Some(s.to_string()) +} + +// ── PEM / root store loading ───────────────────────────────────────────── + +/// Resolve a PEM value from a raw string: a value starting with `-----BEGIN` +/// is treated as inline PEM (cloud injects CA/cert/key material this way, +/// from Secrets Manager); anything else is treated as a filesystem path (OSS +/// mounts files). Empty or unset input is `Ok(None)` — the caller decides +/// whether that's fatal. +fn pem_from_value(var_name: &str, value: &str) -> Result> { + let trimmed = value.trim(); + if trimmed.is_empty() { + return Ok(None); + } + if trimmed.starts_with("-----BEGIN") { + return Ok(Some(trimmed.to_string())); + } + std::fs::read_to_string(trimmed) + .map(Some) + .with_context(|| format!("reading {var_name} from path {trimmed}")) +} + +/// Same as [`pem_from_value`], reading the raw value from the environment. +#[cfg_attr(not(test), allow(dead_code))] +fn pem_from_env(var_name: &str) -> Result> { + match std::env::var(var_name) { + Ok(value) => pem_from_value(var_name, &value), + Err(_) => Ok(None), + } +} + +/// Parse every certificate out of a PEM bundle. Errors if the PEM is +/// malformed or contains zero certificates — a client CA file that parses to +/// nothing is a misconfiguration, not "no CAs trusted". +fn pem_to_der_certs(pem: &str) -> Result>> { + let mut reader = pem.as_bytes(); + let certs: Vec> = rustls_pemfile::certs(&mut reader) + .collect::>() + .context("parsing PEM certificate(s)")?; + if certs.is_empty() { + bail!("no certificates found in PEM"); + } + Ok(certs) +} + +/// Extract the DER-encoded SubjectPublicKeyInfo (SPKI) from a certificate — +/// "is this the same key", not "is this byte-identical certificate". Used by +/// the MITM-CA-reuse guard in `from_parts`: a certificate can be re-issued or +/// re-encoded (different serial, validity window, or DN) while wrapping the +/// exact same key pair, which a whole-certificate DER comparison would miss. +fn spki_der(cert_der: &CertificateDer<'_>) -> Result> { + let (_, cert) = x509_parser::parse_x509_certificate(cert_der.as_ref()) + .context("parsing certificate to extract its public key")?; + Ok(cert.public_key().raw.to_vec()) +} + +/// Build a [`RootCertStore`] from a PEM bundle of one or more CA certificates. +/// Reused by later phases (e.g. reloading the client CA bundle on rotation). +pub(crate) fn load_client_ca_roots(pem: &str) -> Result> { + let certs = pem_to_der_certs(pem)?; + let mut store = RootCertStore::empty(); + for cert in certs { + store + .add(cert) + .context("adding client CA certificate to root store")?; + } + Ok(Arc::new(store)) +} + +// ── Server config ───────────────────────────────────────────────────────── + +/// Build the mTLS `ServerConfig`: the gateway's own cert/key for the TLS +/// server side, plus `roots` as the trust anchor(s) for verifying client +/// certificates. +fn build_server_config( + cert_pem: &str, + key_pem: &str, + roots: Arc, +) -> Result> { + let cert_chain = pem_to_der_certs(cert_pem).context("parsing GATEWAY_TLS_CERT")?; + + let mut key_reader = key_pem.as_bytes(); + let key: PrivateKeyDer<'static> = rustls_pemfile::private_key(&mut key_reader) + .context("parsing GATEWAY_TLS_KEY")? + .context("no private key found in GATEWAY_TLS_KEY")?; + + // SECURITY: no `.allow_unauthenticated()` on this builder. The builder's + // default policy — reject any handshake that doesn't present a certificate + // verifiable against `roots` — IS the "no cert -> rejected" guarantee this + // whole module exists to provide. Do not add it, even for a "convenience" + // fallback: that would silently reopen the plaintext-equivalent hole this + // port is meant to close. + let verifier = WebPkiClientVerifier::builder(roots) + .build() + .context("building client certificate verifier")?; + + let mut config = ServerConfig::builder() + .with_client_cert_verifier(verifier) + .with_single_cert(cert_chain, key) + .context("building mTLS ServerConfig")?; + + // Force HTTP/1.1 — same rationale as the MITM leaf configs (ca.rs): + // prevent HTTP/2 negotiation via ALPN, since the gateway's connection + // handling assumes HTTP/1.1 semantics (CONNECT interception, upgrades). + config.alpn_protocols = vec![b"http/1.1".to_vec()]; + + // Phase 2 hook: revocation lists would be wired in here via + // `WebPkiClientVerifier::builder(roots).with_crls(...)`. Certificate + // expiry is already validated by the webpki verifier itself — no manual + // `not_after` check is needed (or added) on top of it. + + Ok(Arc::new(config)) +} + +// ── Config ──────────────────────────────────────────────────────────────── + +/// Resolved mTLS listener configuration. `None` (via [`MtlsConfig::from_env`]) +/// means mTLS is off — the gateway runs exactly as it did before this module +/// existed. +#[derive(Debug)] +pub(crate) struct MtlsConfig { + pub(crate) port: u16, + pub(crate) bind: IpAddr, + pub(crate) server_config: Arc, +} + +impl MtlsConfig { + /// Build the mTLS config from already-resolved parts — no environment + /// access, so this is the unit-testable core. `from_env` is a thin + /// wrapper that reads the four env vars and forwards here. + /// + /// `port`/`cert`/`key`/`ca` are the raw `GATEWAY_MTLS_PORT` / + /// `GATEWAY_TLS_CERT` / `GATEWAY_TLS_KEY` / `GATEWAY_CLIENT_CA` values + /// (or `None` if unset) — inline PEM or filesystem path, resolved here via + /// [`pem_from_value`]. `mitm_ca_der` is the gateway's own MITM CA + /// certificate (see `ca.rs`); `plain_port` is the plaintext listener port. + /// + /// `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. + pub(crate) fn from_parts( + port: Option<&str>, + cert: Option<&str>, + key: Option<&str>, + ca: Option<&str>, + mitm_ca_der: &CertificateDer<'static>, + plain_port: u16, + ) -> Result> { + let Some(port_str) = port else { + // GATEWAY_MTLS_PORT unset: mTLS is off. Full backward compatibility. + return Ok(None); + }; + + let port: u16 = port_str.parse().with_context(|| { + format!("GATEWAY_MTLS_PORT {port_str:?} is not a valid port number") + })?; + if port == 0 { + bail!("GATEWAY_MTLS_PORT must not be 0"); + } + if port == plain_port { + bail!( + "GATEWAY_MTLS_PORT ({port}) must differ from the plaintext gateway port ({plain_port})" + ); + } + + let cert_pem = cert + .context("GATEWAY_TLS_CERT is required when GATEWAY_MTLS_PORT is set") + .and_then(|v| pem_from_value("GATEWAY_TLS_CERT", v))? + .context("GATEWAY_TLS_CERT is required when GATEWAY_MTLS_PORT is set")?; + let key_pem = key + .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")?; + + // SECURITY: reject a client CA bundle that carries the same public + // key as the gateway's own MITM CA. Compared on the DER-encoded + // SubjectPublicKeyInfo (SPKI), not the whole-certificate DER: the real + // risk is the MITM CA's *private key* being host-resident (it signs a + // fresh leaf for every intercepted domain), and a re-issued or + // re-encoded certificate wrapping that SAME key would byte-differ + // from the original cert while remaining exactly as dangerous to + // trust — a whole-cert comparison would miss it. If ANY certificate + // carrying that key were trusted as a client-cert anchor, anyone able + // to mint a MITM leaf could just as easily mint a "valid" client cert + // and impersonate any agent. + let mitm_spki = + spki_der(mitm_ca_der).context("parsing the gateway's own MITM CA certificate")?; + let ca_certs = pem_to_der_certs(&ca_pem).context("parsing GATEWAY_CLIENT_CA")?; + for cert in &ca_certs { + let spki = spki_der(cert) + .context("GATEWAY_CLIENT_CA contains a certificate that failed to parse")?; + if spki == mitm_spki { + bail!( + "GATEWAY_CLIENT_CA must not include a certificate carrying the same public \ + key as the gateway's own MITM CA (its private key lives on this host, so \ + trusting that key as a client anchor would let anyone mint their own \ + client certificate)" + ); + } + } + + let roots = load_client_ca_roots(&ca_pem).context("GATEWAY_CLIENT_CA")?; + let server_config = build_server_config(&cert_pem, &key_pem, roots)?; + + Ok(Some(MtlsConfig { + port, + bind: IpAddr::V4(Ipv4Addr::UNSPECIFIED), + server_config, + })) + } + + /// Read `GATEWAY_MTLS_PORT` / `GATEWAY_TLS_CERT` / `GATEWAY_TLS_KEY` / + /// `GATEWAY_CLIENT_CA` from the environment and forward to [`Self::from_parts`]. + pub(crate) fn from_env( + mitm_ca_der: &CertificateDer<'static>, + plain_port: u16, + ) -> Result> { + let port = std::env::var("GATEWAY_MTLS_PORT").ok(); + let cert = std::env::var("GATEWAY_TLS_CERT").ok(); + let key = std::env::var("GATEWAY_TLS_KEY").ok(); + let ca = std::env::var("GATEWAY_CLIENT_CA").ok(); + Self::from_parts( + port.as_deref(), + cert.as_deref(), + key.as_deref(), + ca.as_deref(), + mitm_ca_der, + plain_port, + ) + } +} + +#[cfg(test)] +mod tests { + 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, + PKCS_ECDSA_P256_SHA256, + }; + use rustls::pki_types::ServerName; + use time::OffsetDateTime; + use tokio::net::{TcpListener, TcpStream}; + use tokio_rustls::{TlsAcceptor, TlsConnector}; + + static INIT_CRYPTO: Once = Once::new(); + + 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. + 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>, + } + + 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); + params.distinguished_name.push(DnType::CommonName, cn); + params.key_usages = vec![KeyUsagePurpose::KeyCertSign, KeyUsagePurpose::CrlSign]; + params.not_before = OffsetDateTime::now_utc() - time::Duration::hours(1); + params.not_after = OffsetDateTime::now_utc() + time::Duration::days(3650); + let cert = params.self_signed(&key).expect("self-sign CA"); + let der = cert.der().clone(); + TestCa { cert, key, der } + } + + /// 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( + ca: &TestCa, + cn: Option<&str>, + uri_sans: &[&str], + not_before_h: i64, + not_after_h: i64, + ) -> (String, String) { + let leaf_key = KeyPair::generate_for(&PKCS_ECDSA_P256_SHA256).expect("leaf key"); + let mut params = CertificateParams::default(); + // `CertificateParams::new(strings)` only ever infers IP or DNS SANs — + // a "spiffe://..."-shaped string comes out as a (nonsensical) DNS + // name, not a URI SAN. Push `SanType::URI` directly instead. + params.subject_alt_names = uri_sans + .iter() + .map(|s| { + rcgen::SanType::URI( + rcgen::Ia5String::try_from(s.to_string()).expect("valid IA5 URI"), + ) + }) + .collect(); + if let Some(cn) = cn { + params.distinguished_name.push(DnType::CommonName, cn); + } + params.key_usages = vec![KeyUsagePurpose::DigitalSignature]; + params.extended_key_usages = vec![rcgen::ExtendedKeyUsagePurpose::ClientAuth]; + params.not_before = OffsetDateTime::now_utc() + time::Duration::hours(not_before_h); + params.not_after = OffsetDateTime::now_utc() + time::Duration::hours(not_after_h); + let leaf_cert = params + .signed_by(&leaf_key, &ca.cert, &ca.key) + .expect("sign leaf"); + (leaf_cert.pem(), leaf_key.serialize_pem()) + } + + /// 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>) { + let key = KeyPair::generate_for(&PKCS_ECDSA_P256_SHA256).expect("server key"); + let mut params = CertificateParams::new(vec!["localhost".to_string()]).expect("params"); + params + .distinguished_name + .push(DnType::CommonName, "localhost"); + params.extended_key_usages = vec![rcgen::ExtendedKeyUsagePurpose::ServerAuth]; + params.not_before = OffsetDateTime::now_utc() - time::Duration::hours(1); + params.not_after = OffsetDateTime::now_utc() + time::Duration::days(1); + let cert = params.self_signed(&key).expect("self-sign server cert"); + let der = cert.der().clone(); + (cert.pem(), key.serialize_pem(), der) + } + + /// 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>) { + 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 = + build_server_config(&server_cert_pem, &server_key_pem, roots).expect("server config"); + (config, server_der) + } + + fn test_client_config( + server_der: &CertificateDer<'static>, + client_cert_pem: Option<&str>, + client_key_pem: Option<&str>, + ) -> Arc { + let mut roots = RootCertStore::empty(); + roots.add(server_der.clone()).expect("trust server cert"); + + let builder = rustls::ClientConfig::builder().with_root_certificates(roots); + let config = match (client_cert_pem, client_key_pem) { + (Some(cert_pem), Some(key_pem)) => { + let chain = pem_to_der_certs(cert_pem).expect("client cert chain"); + let mut key_reader = key_pem.as_bytes(); + let key = rustls_pemfile::private_key(&mut key_reader) + .expect("parse client key") + .expect("client key present"); + builder + .with_client_auth_cert(chain, key) + .expect("client auth cert") + } + _ => builder.with_no_client_auth(), + }; + Arc::new(config) + } + + /// Run one TLS handshake end to end over a real loopback socket (same + /// pattern as the plain-TCP tests in `gateway.rs`/`ca.rs`, just over TLS). + /// 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( + server_config: Arc, + client_config: Arc, + ) -> std::io::Result> { + let listener = TcpListener::bind("127.0.0.1:0").await.expect("bind"); + let addr: SocketAddr = listener.local_addr().expect("local addr"); + + let server = async move { + let (stream, _) = listener.accept().await?; + TlsAcceptor::from(server_config).accept(stream).await + }; + let client = async move { + let stream = TcpStream::connect(addr).await?; + let name = ServerName::try_from("localhost").expect("server name"); + TlsConnector::from(client_config) + .connect(name, stream) + .await + }; + + let (server_result, _client_result) = tokio::join!(server, client); + server_result + } + + fn err_debug_contains(err: &std::io::Error, needle: &str) -> bool { + format!("{err:?}").contains(needle) + } + + // ── Handshake behavior ──────────────────────────────────────────────── + + #[tokio::test] + async fn handshake_rejects_missing_client_cert() { + ensure_crypto_provider(); + let ca = new_test_ca("Test Client CA"); + let ca_pem = ca.cert.pem(); + let (server_config, server_der) = test_server_setup(&ca_pem); + let client_config = test_client_config(&server_der, None, None); + + let result = attempt_handshake(server_config, client_config).await; + let err = result.expect_err("handshake without a client cert must fail"); + assert!( + err_debug_contains(&err, "NoCertificatesPresented") + || err_debug_contains(&err, "CertificateRequired"), + "unexpected error: {err:?}" + ); + } + + #[tokio::test] + async fn handshake_rejects_wrong_ca() { + ensure_crypto_provider(); + let trusted_ca = new_test_ca("Trusted Client CA"); + let other_ca = new_test_ca("Some Other CA"); + let (cert_pem, key_pem) = sign_client_leaf(&other_ca, Some("agent-1"), &[], -1, 24); + + let (server_config, server_der) = test_server_setup(&trusted_ca.cert.pem()); + let client_config = test_client_config(&server_der, Some(&cert_pem), Some(&key_pem)); + + let result = attempt_handshake(server_config, client_config).await; + let err = result.expect_err("handshake signed by an untrusted CA must fail"); + assert!( + err_debug_contains(&err, "UnknownIssuer"), + "unexpected error: {err:?}" + ); + } + + #[tokio::test] + async fn handshake_rejects_expired_cert() { + ensure_crypto_provider(); + let ca = new_test_ca("Trusted Client CA"); + // Valid window entirely in the past. + let (cert_pem, key_pem) = sign_client_leaf(&ca, Some("agent-1"), &[], -48, -24); + + let (server_config, server_der) = test_server_setup(&ca.cert.pem()); + let client_config = test_client_config(&server_der, Some(&cert_pem), Some(&key_pem)); + + let result = attempt_handshake(server_config, client_config).await; + let err = result.expect_err("handshake with an expired client cert must fail"); + assert!( + err_debug_contains(&err, "Expired"), + "unexpected error: {err:?}" + ); + } + + #[tokio::test] + async fn handshake_accepts_valid_cert_with_uri_san() { + ensure_crypto_provider(); + let ca = new_test_ca("Trusted Client CA"); + let (cert_pem, key_pem) = sign_client_leaf( + &ca, + Some("fallback-cn"), + &["spiffe://onecli/agent/1"], + -1, + 24, + ); + + let (server_config, server_der) = test_server_setup(&ca.cert.pem()); + let client_config = test_client_config(&server_der, Some(&cert_pem), Some(&key_pem)); + + let mut tls_stream = attempt_handshake(server_config, client_config) + .await + .expect("valid client cert must be accepted"); + + let identity = tls_stream + .get_ref() + .1 + .peer_certificates() + .and_then(identity_from_peer_certs) + .expect("identity must be extracted"); + assert_eq!(identity.primary(), Some("spiffe://onecli/agent/1")); + assert_eq!(identity.cn.as_deref(), Some("fallback-cn")); + + // Drain so the client side's write half doesn't hang the test. + use tokio::io::AsyncWriteExt; + let _ = tls_stream.shutdown().await; + } + + #[tokio::test] + async fn handshake_accepts_valid_cert_cn_only() { + ensure_crypto_provider(); + let ca = new_test_ca("Trusted Client CA"); + let (cert_pem, key_pem) = sign_client_leaf(&ca, Some("cn-only-agent"), &[], -1, 24); + + let (server_config, server_der) = test_server_setup(&ca.cert.pem()); + let client_config = test_client_config(&server_der, Some(&cert_pem), Some(&key_pem)); + + let mut tls_stream = attempt_handshake(server_config, client_config) + .await + .expect("valid client cert must be accepted"); + + let identity = tls_stream + .get_ref() + .1 + .peer_certificates() + .and_then(identity_from_peer_certs) + .expect("identity must be extracted"); + assert_eq!(identity.primary(), Some("cn-only-agent")); + assert!(identity.uri_sans.is_empty()); + + use tokio::io::AsyncWriteExt; + let _ = tls_stream.shutdown().await; + } + + #[tokio::test] + async fn server_config_pins_http11_alpn() { + ensure_crypto_provider(); + let ca = new_test_ca("Trusted Client CA"); + let (config, _server_der) = test_server_setup(&ca.cert.pem()); + assert_eq!(config.alpn_protocols, vec![b"http/1.1".to_vec()]); + } + + // ── from_parts: no env access ──────────────────────────────────────── + + fn dummy_mitm_ca_der() -> CertificateDer<'static> { + new_test_ca("Dummy MITM CA").der + } + + #[test] + 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(); + assert!(result.is_none()); + } + + /// Stands in for "this field is present" in `from_parts` tests that only + /// care about a *different* field. Must start with `-----BEGIN` so + /// `pem_from_value` takes the inline-PEM branch rather than trying (and + /// failing) to read it as a filesystem path — its content is never + /// actually parsed in these tests, since the function under test returns + /// before reaching that point. + const PRESENT_PLACEHOLDER_PEM: &str = + "-----BEGIN CERTIFICATE-----\nplaceholder\n-----END CERTIFICATE-----\n"; + + #[test] + fn from_parts_missing_cert_errs_naming_it() { + ensure_crypto_provider(); + let mitm_der = dummy_mitm_ca_der(); + let err = MtlsConfig::from_parts( + Some("10256"), + None, + Some("key"), + Some("ca"), + &mitm_der, + 10255, + ) + .unwrap_err(); + assert!(format!("{err:#}").contains("GATEWAY_TLS_CERT")); + } + + #[test] + fn from_parts_missing_key_errs_naming_it() { + ensure_crypto_provider(); + let mitm_der = dummy_mitm_ca_der(); + let err = MtlsConfig::from_parts( + Some("10256"), + Some(PRESENT_PLACEHOLDER_PEM), + None, + Some(PRESENT_PLACEHOLDER_PEM), + &mitm_der, + 10255, + ) + .unwrap_err(); + assert!(format!("{err:#}").contains("GATEWAY_TLS_KEY")); + } + + #[test] + fn from_parts_missing_ca_errs_naming_it() { + 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, + &mitm_der, + 10255, + ) + .unwrap_err(); + assert!(format!("{err:#}").contains("GATEWAY_CLIENT_CA")); + } + + #[test] + 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(); + assert!(format!("{err:#}").contains("must not be 0")); + } + + #[test] + fn from_parts_garbage_port_errs() { + ensure_crypto_provider(); + let mitm_der = dummy_mitm_ca_der(); + let err = MtlsConfig::from_parts( + Some("not-a-port"), + Some("c"), + Some("k"), + Some("a"), + &mitm_der, + 10255, + ) + .unwrap_err(); + assert!(format!("{err:#}").contains("not a valid port")); + } + + #[test] + fn from_parts_port_equals_plain_port_errs() { + ensure_crypto_provider(); + let mitm_der = dummy_mitm_ca_der(); + let err = MtlsConfig::from_parts( + Some("10255"), + Some("c"), + Some("k"), + Some("a"), + &mitm_der, + 10255, + ) + .unwrap_err(); + assert!(format!("{err:#}").contains("must differ")); + } + + #[test] + fn from_parts_loads_inline_pem_and_path_forms() { + ensure_crypto_provider(); + let ca = new_test_ca("Trusted Client CA"); + let (server_cert_pem, server_key_pem, _) = self_signed_server_cert(); + let mitm_der = dummy_mitm_ca_der(); + let ca_pem = ca.cert.pem(); + + // Inline PEM form for all three. + let result = MtlsConfig::from_parts( + Some("10256"), + Some(&server_cert_pem), + Some(&server_key_pem), + Some(&ca_pem), + &mitm_der, + 10255, + ) + .expect("inline PEM must load"); + assert!(result.is_some()); + + // Path form: write each to a tempfile and pass the path. + let dir = tempfile::tempdir().expect("tempdir"); + let cert_path = dir.path().join("cert.pem"); + let key_path = dir.path().join("key.pem"); + let ca_path = dir.path().join("ca.pem"); + std::fs::write(&cert_path, &server_cert_pem).expect("write cert"); + std::fs::write(&key_path, &server_key_pem).expect("write key"); + std::fs::write(&ca_path, &ca_pem).expect("write ca"); + + let result = MtlsConfig::from_parts( + Some("10256"), + Some(cert_path.to_str().unwrap()), + Some(key_path.to_str().unwrap()), + Some(ca_path.to_str().unwrap()), + &mitm_der, + 10255, + ) + .expect("path form must load"); + assert!(result.is_some()); + } + + #[test] + fn from_parts_bad_path_errs() { + ensure_crypto_provider(); + let mitm_der = dummy_mitm_ca_der(); + let err = MtlsConfig::from_parts( + Some("10256"), + Some("/nonexistent/path/cert.pem"), + Some("/nonexistent/path/key.pem"), + Some("/nonexistent/path/ca.pem"), + &mitm_der, + 10255, + ) + .unwrap_err(); + assert!(format!("{err:#}").contains("GATEWAY_TLS_CERT")); + } + + #[test] + fn from_parts_garbage_pem_errs() { + ensure_crypto_provider(); + let mitm_der = dummy_mitm_ca_der(); + let (server_cert_pem, server_key_pem, _) = self_signed_server_cert(); + let err = MtlsConfig::from_parts( + Some("10256"), + Some(&server_cert_pem), + Some(&server_key_pem), + Some("-----BEGIN CERTIFICATE-----\nbm90IGEgY2VydA==\n-----END CERTIFICATE-----\n"), + &mitm_der, + 10255, + ) + .unwrap_err(); + assert!(format!("{err:#}").contains("GATEWAY_CLIENT_CA")); + } + + #[test] + fn from_parts_empty_client_ca_errs() { + ensure_crypto_provider(); + let mitm_der = dummy_mitm_ca_der(); + let (server_cert_pem, server_key_pem, _) = self_signed_server_cert(); + // Empty value resolves to None via pem_from_value, which is then the + // "missing" case for a mandatory var. + let err = MtlsConfig::from_parts( + Some("10256"), + Some(&server_cert_pem), + Some(&server_key_pem), + Some(""), + &mitm_der, + 10255, + ) + .unwrap_err(); + assert!(format!("{err:#}").contains("GATEWAY_CLIENT_CA")); + } + + #[test] + fn from_parts_rejects_client_ca_matching_mitm_ca() { + ensure_crypto_provider(); + let mitm_ca = new_test_ca("Gateway MITM CA"); + let (server_cert_pem, server_key_pem, _) = self_signed_server_cert(); + + // GATEWAY_CLIENT_CA is (accidentally) the same cert as the MITM CA. + let err = MtlsConfig::from_parts( + Some("10256"), + Some(&server_cert_pem), + Some(&server_key_pem), + Some(&mitm_ca.cert.pem()), + &mitm_ca.der, + 10255, + ) + .unwrap_err(); + assert!(format!("{err:#}").contains("MITM CA")); + } + + /// FIX 3: the guard compares public keys (SPKI), not whole-certificate + /// DER — a certificate carrying the SAME key as the MITM CA must still be + /// rejected even though it's a byte-different certificate (different CN, + /// serial, and validity window — e.g. a re-issued or re-encoded cert). + #[test] + fn from_parts_rejects_client_ca_with_same_public_key_as_mitm_ca_even_if_cert_differs() { + ensure_crypto_provider(); + + let mitm_key = KeyPair::generate_for(&PKCS_ECDSA_P256_SHA256).expect("mitm key"); + let mut mitm_params = CertificateParams::default(); + mitm_params.is_ca = IsCa::Ca(BasicConstraints::Unconstrained); + mitm_params + .distinguished_name + .push(DnType::CommonName, "Gateway MITM CA"); + mitm_params.not_before = OffsetDateTime::now_utc() - time::Duration::hours(1); + mitm_params.not_after = OffsetDateTime::now_utc() + time::Duration::days(3650); + let mitm_cert = mitm_params.self_signed(&mitm_key).expect("self-sign mitm"); + let mitm_der = mitm_cert.der().clone(); + + // A DIFFERENT certificate — different CN, serial, and validity window + // (as a re-issued cert would be) — but signed with the SAME key pair. + let mut reissued_params = CertificateParams::default(); + reissued_params.is_ca = IsCa::Ca(BasicConstraints::Unconstrained); + reissued_params + .distinguished_name + .push(DnType::CommonName, "Totally Unrelated Client CA"); + reissued_params.not_before = OffsetDateTime::now_utc() - time::Duration::hours(2); + reissued_params.not_after = OffsetDateTime::now_utc() + time::Duration::days(30); + let reissued_cert = reissued_params + .self_signed(&mitm_key) + .expect("self-sign reissued cert with the same key"); + + // Sanity check: the two certs must NOT be byte-identical — otherwise + // this test would exercise the same path as the whole-DER case above + // and prove nothing new. + assert_ne!(reissued_cert.der().as_ref(), mitm_der.as_ref()); + + let (server_cert_pem, server_key_pem, _) = self_signed_server_cert(); + let err = MtlsConfig::from_parts( + Some("10256"), + Some(&server_cert_pem), + Some(&server_key_pem), + Some(&reissued_cert.pem()), + &mitm_der, + 10255, + ) + .unwrap_err(); + assert!(format!("{err:#}").contains("GATEWAY_CLIENT_CA")); + assert!(format!("{err:#}").contains("public")); + } + + // ── pem_from_env / pem_from_value ──────────────────────────────────── + + #[test] + fn pem_from_value_empty_is_none() { + assert_eq!(pem_from_value("X", "").unwrap(), None); + assert_eq!(pem_from_value("X", " ").unwrap(), None); + } + + #[test] + fn pem_from_value_inline_pem_passthrough() { + // Leading/trailing whitespace around the value is trimmed (matching + // ca.rs's `load_from_pem`), so assert against the trimmed form. + let pem = "-----BEGIN CERTIFICATE-----\nabc\n-----END CERTIFICATE-----\n"; + assert_eq!( + pem_from_value("X", pem).unwrap(), + Some(pem.trim().to_string()) + ); + } + + #[test] + fn pem_from_value_reads_path() { + let mut file = tempfile::NamedTempFile::new().expect("tempfile"); + write!(file, "file-contents").expect("write"); + let path = file.path().to_str().unwrap(); + assert_eq!( + pem_from_value("X", path).unwrap(), + Some("file-contents".to_string()) + ); + } + + #[test] + fn pem_from_value_bad_path_errs() { + let err = pem_from_value("GATEWAY_TLS_CERT", "/no/such/file.pem").unwrap_err(); + assert!(format!("{err:#}").contains("GATEWAY_TLS_CERT")); + assert!(format!("{err:#}").contains("/no/such/file.pem")); + } + + #[test] + fn pem_from_env_unset_is_none() { + // A var name essentially guaranteed not to be set. + assert_eq!( + pem_from_env("GATEWAY_CA_TEST_DOES_NOT_EXIST_XYZ").unwrap(), + None + ); + } + + // ── load_client_ca_roots ────────────────────────────────────────────── + + #[test] + fn load_client_ca_roots_empty_pem_errs() { + assert!(load_client_ca_roots("").is_err()); + } + + #[test] + fn load_client_ca_roots_garbage_errs() { + assert!(load_client_ca_roots("not pem at all").is_err()); + } + + #[test] + fn load_client_ca_roots_valid_pem_ok() { + let ca = new_test_ca("Trusted Client CA"); + assert!(load_client_ca_roots(&ca.cert.pem()).is_ok()); + } + + // Sanity: not_after_unix reflects the certificate's actual expiry, so a + // "certificate expires in ~1 day" leaf really does report a timestamp + // roughly a day in the future (the verifier — not this field — is what + // rejects expired certs; this just confirms the value is meaningful for + // Phase 2 to eventually build on). + #[tokio::test] + async fn identity_not_after_matches_leaf_validity() { + ensure_crypto_provider(); + let ca = new_test_ca("Trusted Client CA"); + let (cert_pem, key_pem) = sign_client_leaf(&ca, Some("agent-1"), &[], -1, 24); + let (server_config, server_der) = test_server_setup(&ca.cert.pem()); + let client_config = test_client_config(&server_der, Some(&cert_pem), Some(&key_pem)); + + let mut tls_stream = attempt_handshake(server_config, client_config) + .await + .expect("valid cert accepted"); + let identity = tls_stream + .get_ref() + .1 + .peer_certificates() + .and_then(identity_from_peer_certs) + .expect("identity extracted"); + + let now = SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap() + .as_secs() as i64; + // Leaf expires ~24h from now; allow generous slack for test runtime. + assert!(identity.not_after_unix > now); + assert!(identity.not_after_unix < now + 25 * 3600); + + use tokio::io::AsyncWriteExt; + let _ = tls_stream.shutdown().await; + } +} diff --git a/apps/gateway/src/gateway.rs b/apps/gateway/src/gateway.rs index d4d83fd3..e5bbf76e 100644 --- a/apps/gateway/src/gateway.rs +++ b/apps/gateway/src/gateway.rs @@ -36,8 +36,9 @@ mod transforms; mod tunnel; mod websocket; -use std::net::SocketAddr; +use std::net::{IpAddr, Ipv4Addr, SocketAddr}; use std::sync::Arc; +use std::time::Duration; use anyhow::{Context, Result}; use axum::extract::State; @@ -47,8 +48,10 @@ use hyper::server::conn::http1; use hyper::service::service_fn; use hyper::{Method, Request, Response, StatusCode}; use hyper_util::rt::TokioIo; -use tokio::net::{TcpListener, TcpStream}; -use tokio_rustls::TlsConnector; +use tokio::io::{AsyncRead, AsyncWrite}; +use tokio::net::TcpListener; +use tokio::time::timeout; +use tokio_rustls::{TlsAcceptor, TlsConnector}; use tower::ServiceExt; use tower_http::cors::CorsLayer; use tracing::{debug, info, info_span, warn, Instrument}; @@ -57,20 +60,29 @@ use crate::approval::{ApprovalDecision, ApprovalStore, APPROVAL_TIMEOUT_SECS}; use crate::auth::AuthUser; use crate::ca::CertificateAuthority; use crate::cache::CacheStore; +use crate::client_ca::{self, ClientIdentity, MtlsConfig}; use crate::connect::{self, AppConnectionResult, ConnectError, PolicyEngine}; use crate::db; use crate::inject; use crate::vault; -/// Pause before retrying a failed `accept`, so a persistent error (a truly -/// exhausted fd table) cannot spin the loop at full tilt. -const ACCEPT_RETRY_DELAY: std::time::Duration = std::time::Duration::from_millis(100); +/// Cap the client TLS handshake on the mTLS listener so a stalled or hostile +/// ClientHello can't hold a connection task (and, in effect, the socket) open +/// indefinitely. +const TLS_HANDSHAKE_TIMEOUT: Duration = Duration::from_secs(10); + +/// Backoff after a recoverable `accept()` error (e.g. EMFILE under fd +/// pressure) before retrying. Both listeners run under the same +/// `tokio::try_join!` in `run()`, so an unhandled error from one accept loop +/// would cancel the other — this keeps a transient error confined to its own +/// listener instead. +const ACCEPT_RETRY_DELAY: Duration = Duration::from_millis(100); // ── GatewayState ─────────────────────────────────────────────────────── /// Context for a proxied request, resolved at CONNECT time. /// Wrapped in `Arc` and shared across all requests within a MITM session. -#[derive(Debug)] +#[derive(Debug, Default)] pub(crate) struct ProxyContext { pub project_id: Option, pub organization_id: Option, @@ -78,6 +90,13 @@ pub(crate) struct ProxyContext { pub agent_name: Option, pub agent_identifier: Option, pub agent_token: Option, + /// Identity extracted from the client's mTLS certificate, when the + /// connection came in on the mTLS listener. Phase 1 only threads and logs + /// this (the log statements read the identity before it's moved in here) + /// — it is never compared against `agent_token`; Phase 2 is the first + /// reader of the field itself, hence the lint allowance below. + #[allow(dead_code)] + pub client_identity: Option>, } /// Shared state for the gateway, passed to all request handlers. @@ -113,6 +132,13 @@ pub(crate) struct GatewayState { pub struct GatewayServer { state: GatewayState, port: u16, + /// `None` when `GATEWAY_MTLS_PORT` is unset — the gateway then runs + /// exactly as it did before mTLS support existed. + mtls: Option, + /// Bind address for the plaintext listener. Defaults to `0.0.0.0` + /// (`GATEWAY_PLAIN_BIND`) — see the warning in `new()` about what + /// narrowing it costs when mTLS is also enabled. + plain_bind: IpAddr, } /// Build the HTTP client used for upstream requests. @@ -247,6 +273,35 @@ fn parse_skip_verify_hosts() -> Vec { .collect() } +/// Parse an already-read `GATEWAY_PLAIN_BIND` value (`None` when the var is +/// unset) into a bind address, defaulting to `0.0.0.0` (unrestricted — +/// today's behavior, unchanged unless the operator opts into narrowing it). +/// +/// Fails closed: unset/empty stays the default, but a SET-and-unparseable +/// value (a typo like `127.0.0.q`, or `localhost`, which isn't an IP literal) +/// is an `Err`, not a silent fallback to the wide-open default. This is the +/// one operator knob for restricting the always-open plaintext listener, so +/// silently widening it on a typo would defeat the whole point of the knob. +/// +/// No env access — that's `parse_plain_bind`'s job — so this is directly +/// unit-testable, mirroring the `from_parts`/`from_env` split in `client_ca.rs`. +fn parse_plain_bind_value(value: Option<&str>) -> Result { + match value { + None => Ok(IpAddr::V4(Ipv4Addr::UNSPECIFIED)), + Some(s) if s.trim().is_empty() => Ok(IpAddr::V4(Ipv4Addr::UNSPECIFIED)), + Some(s) => s + .trim() + .parse() + .with_context(|| format!("GATEWAY_PLAIN_BIND {s:?} is not a valid IP address")), + } +} + +/// Read `GATEWAY_PLAIN_BIND` from the environment and parse it via +/// [`parse_plain_bind_value`]. +fn parse_plain_bind() -> Result { + parse_plain_bind_value(std::env::var("GATEWAY_PLAIN_BIND").ok().as_deref()) +} + /// Returns true if `host` matches any pattern in `patterns`. /// /// - `*.example.com` matches `sub.example.com` but NOT `example.com` itself. @@ -266,6 +321,7 @@ fn host_matches_skip_verify(host: &str, patterns: &[String]) -> bool { } impl GatewayServer { + #[allow(clippy::too_many_arguments)] pub fn new( ca: CertificateAuthority, port: u16, @@ -273,7 +329,8 @@ impl GatewayServer { vault_service: Arc, cache: Arc, approval_store: Arc, - ) -> Self { + mtls: 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()); @@ -283,6 +340,17 @@ impl GatewayServer { info!(hosts = ?skip_verify_hosts.as_ref(), "TLS verification disabled for matched hosts (GATEWAY_SKIP_VERIFY_HOSTS)"); } + let plain_bind = parse_plain_bind()?; + if mtls.is_some() && plain_bind.is_unspecified() { + warn!( + "GATEWAY_MTLS_PORT is set but the plaintext listener is still bound to \ + 0.0.0.0 — anyone who can reach that port bypasses certificate \ + authentication entirely. Set GATEWAY_PLAIN_BIND=127.0.0.1 to restrict it, \ + but note that loopback also breaks Docker-published browser -> gateway \ + vault/approval/cache calls, which arrive on the plaintext listener." + ); + } + let state = GatewayState { ca: Arc::new(ca), http_client: build_http_client(global_skip), @@ -296,23 +364,18 @@ impl GatewayServer { approval_store, }; - Self { state, port } + Ok(Self { + state, + port, + mtls, + plain_bind, + }) } - /// Start the gateway TCP listener. Runs forever. - pub async fn run(&self) -> Result<()> { - let addr = SocketAddr::from(([0, 0, 0, 0], self.port)); - let listener = TcpListener::bind(addr) - .await - .context("binding TCP listener")?; - - // Report what we actually bound rather than what we asked for: with - // `--port 0` the OS assigns the port, and the requested address would - // report `:0` — leaving no way to discover where the gateway is listening. - let bound_addr = listener.local_addr().context("reading bound address")?; - - info!(addr = %bound_addr, "listening for connections"); - + /// Build the Axum router for non-CONNECT routes (healthz, vault API, + /// approvals, org routes, ...). Shared by both listeners — the plaintext + /// one and, when configured, the mTLS one. + fn build_router(&self) -> Router { // CORS configuration for browser → gateway requests. // credentials: true requires explicit headers/methods (not wildcard *). let cors_layer = CorsLayer::new() @@ -420,53 +483,169 @@ impl GatewayServer { // Org-scoped routes are mounted via an edition-swapped seam // (`ee/org_routes.rs` for cloud + onprem, an identity stub for OSS — see // `main.rs`), so the org handler never reaches the OSS build. - let axum_router = crate::org_routes::mount(axum_router) + crate::org_routes::mount(axum_router) .layer(cors_layer) .fallback(fallback) - .with_state(self.state.clone()); - - let mut shutdown_signal = crate::shutdown::subscribe(); - - loop { - let (stream, peer_addr) = tokio::select! { - accepted = listener.accept() => match accepted { - Ok(conn) => conn, - Err(e) => { - // Accept failures are almost always transient and - // self-healing (EMFILE clears as connections close, - // ECONNABORTED is a client that gave up mid-handshake). - // Propagating one would tear down every healthy - // connection this proxy is carrying. - warn!(error = %e, "accept failed; retrying"); - tokio::time::sleep(ACCEPT_RETRY_DELAY).await; - continue; - } - }, - _ = shutdown_signal.wait() => break, - }; + .with_state(self.state.clone()) + } - let state = self.state.clone(); - let router = axum_router.clone(); - let guard = crate::shutdown::task_guard(); + /// Start the gateway's TCP listener(s). Runs forever. + /// + /// Always binds the plaintext listener. When mTLS is configured, ALSO + /// binds the mTLS listener before either accept loop starts — so a bind + /// failure on either port aborts startup instead of leaving one listener + /// silently running without the other — then drives both accept loops + /// concurrently for the life of the process. + pub async fn run(&self) -> Result<()> { + let plain_addr = SocketAddr::new(self.plain_bind, self.port); + let plain_listener = TcpListener::bind(plain_addr) + .await + .context("binding plaintext TCP listener")?; + // Report what we actually bound rather than what we asked for: with + // `--port 0` the OS assigns the port, and the requested address would + // report `:0` — leaving no way to discover where the gateway is listening. + let plain_bound = plain_listener + .local_addr() + .context("reading bound plaintext address")?; + info!(addr = %plain_bound, "listening for plaintext connections"); + + let mtls_listener = match &self.mtls { + Some(mtls) => { + let mtls_addr = SocketAddr::new(mtls.bind, mtls.port); + let listener = TcpListener::bind(mtls_addr) + .await + .context("binding mTLS TCP listener")?; + let mtls_bound = listener + .local_addr() + .context("reading bound mTLS address")?; + info!(addr = %mtls_bound, "listening for mTLS connections"); + Some((listener, TlsAcceptor::from(Arc::clone(&mtls.server_config)))) + } + None => None, + }; - tokio::spawn(async move { - let _guard = guard; - if let Err(e) = handle_connection(stream, peer_addr, state, router).await { - warn!(peer = %peer_addr, error = ?e, "connection error"); - } - }); + let router = self.build_router(); + let plain_loop = accept_loop( + plain_listener, + "plaintext", + router.clone(), + self.state.clone(), + None, + ); + + match mtls_listener { + Some((listener, acceptor)) => { + let mtls_loop = + accept_loop(listener, "mTLS", router, self.state.clone(), Some(acceptor)); + tokio::try_join!(plain_loop, mtls_loop)?; + } + None => plain_loop.await?, } - // Closing the port is what stops new work: anything that connects from - // here on is refused rather than accepted into a process on its way - // out. Dropped explicitly rather than at the end of the scope so the - // port is provably shut before the line below claims it is. - drop(listener); - info!("listener closed — draining connections"); Ok(()) } } +/// Accept connections from `listener` forever, spawning a task per connection. +/// `name` labels this listener in logs (`"plaintext"` or `"mTLS"`) so the two +/// concurrent accept loops are distinguishable. +/// +/// Stops accepting NEW connections once a shutdown signal arrives (breaking +/// out of the loop below) — in-flight connections are tracked via a shutdown +/// task guard and drained by `main`'s shutdown sequence, not by this loop. +/// +/// When `tls` is set, the TLS handshake happens *inside* the spawned task — +/// never in this loop — so one slow or hostile `ClientHello` can only stall +/// its own connection, not every other pending accept. +async fn accept_loop( + listener: TcpListener, + name: &'static str, + router: Router, + state: GatewayState, + tls: Option, +) -> Result<()> { + let mut shutdown_signal = crate::shutdown::subscribe(); + + loop { + let (stream, peer_addr) = tokio::select! { + accepted = listener.accept() => match accepted { + Ok(pair) => pair, + Err(e) => { + // Log-and-continue: a recoverable accept() error (EMFILE + // under fd pressure, ECONNABORTED from a client that gave + // up mid-handshake) must not propagate. Both listeners' + // loops are driven by the same `tokio::try_join!` in + // `run()`, so an `Err` here would cancel the OTHER loop + // too, taking down a perfectly healthy listener over a + // transient blip on this one. + warn!(listener = name, error = %e, "accept() failed, retrying"); + tokio::time::sleep(ACCEPT_RETRY_DELAY).await; + continue; + } + }, + _ = shutdown_signal.wait() => break, + }; + + let state = state.clone(); + let router = router.clone(); + let tls = tls.clone(); + let guard = crate::shutdown::task_guard(); + + tokio::spawn(async move { + let _guard = guard; + match tls { + Some(acceptor) => { + let handshake: Result<_, anyhow::Error> = async { + let tls_stream = + timeout(TLS_HANDSHAKE_TIMEOUT, acceptor.accept(stream)).await??; + Ok(tls_stream) + } + .await; + + let tls_stream = match handshake { + Ok(s) => s, + Err(e) => { + warn!(peer = %peer_addr, error = ?e, "mTLS handshake rejected"); + return; + } + }; + + // Read the peer's certificate chain before moving the + // stream into `TokioIo` — `peer_certificates()` is only + // reachable through the raw rustls connection. + let client_identity = tls_stream + .get_ref() + .1 + .peer_certificates() + .and_then(client_ca::identity_from_peer_certs) + .map(Arc::new); + + if let Err(e) = + handle_connection(tls_stream, peer_addr, state, router, client_identity) + .await + { + warn!(peer = %peer_addr, error = ?e, "connection error"); + } + } + None => { + if let Err(e) = handle_connection(stream, peer_addr, state, router, None).await + { + warn!(peer = %peer_addr, error = ?e, "connection error"); + } + } + } + }); + } + + // Closing the port is what stops new work: anything that connects from + // here on is refused rather than accepted into a process on its way out. + // Dropped explicitly rather than at the end of the scope so the port is + // provably shut before the line below claims it is. + drop(listener); + info!(listener = name, "listener closed — draining connections"); + Ok(()) +} + // ── Axum route handlers ───────────────────────────────────────────────── async fn healthz() -> axum::Json { @@ -715,12 +894,16 @@ fn is_http_proxy_request(req: &Request) -> bool { /// Uses a `service_fn` wrapper that intercepts CONNECT requests before they reach /// the Axum router (CONNECT URIs like `host:port` don't match Axum's path-based routing). /// All other HTTP routes (vault API, healthz, etc.) go through the Axum router. -async fn handle_connection( - stream: TcpStream, +async fn handle_connection( + stream: S, peer_addr: SocketAddr, state: GatewayState, router: Router, -) -> Result<()> { + client_identity: Option>, +) -> Result<()> +where + S: AsyncRead + AsyncWrite + Unpin + Send + 'static, +{ let io = TokioIo::new(stream); let conn = http1::Builder::new() @@ -731,11 +914,12 @@ async fn handle_connection( service_fn(move |req: Request| { let state = state.clone(); let router = router.clone(); + let client_identity = client_identity.clone(); async move { if req.method() == Method::CONNECT { - handle_connect(req, peer_addr, state).await + handle_connect(req, peer_addr, state, client_identity).await } else if is_http_proxy_request(&req) { - handle_http_proxy(req, peer_addr, state).await + handle_http_proxy(req, peer_addr, state, client_identity).await } else { // Axum handles all non-proxy routes (healthz, vault API, fallback) let resp: Response = router @@ -773,6 +957,7 @@ async fn handle_connect( req: Request, peer_addr: SocketAddr, state: GatewayState, + client_identity: Option>, ) -> Result, anyhow::Error> { let host = req .uri() @@ -848,6 +1033,7 @@ async fn handle_connect( org_id = organization_id.as_deref().unwrap_or("-"), agent = agent_name.as_deref().unwrap_or("-"), agent_id = agent_id.as_deref().unwrap_or("-"), + client_identity = client_identity.as_deref().and_then(ClientIdentity::primary).unwrap_or("-"), ); info!( @@ -880,6 +1066,7 @@ async fn handle_connect( agent_name, agent_identifier, agent_token: agent_token.clone(), + client_identity, }); // Taken here, before the spawn, so the session is tracked from the moment @@ -943,6 +1130,7 @@ async fn handle_http_proxy( req: Request, peer_addr: SocketAddr, state: GatewayState, + client_identity: Option>, ) -> Result, anyhow::Error> { let authority = req .uri() @@ -1074,6 +1262,7 @@ async fn handle_http_proxy( org_id = resolved.organization_id.as_deref().unwrap_or("-"), agent = resolved.agent_name.as_deref().unwrap_or("-"), agent_id = resolved.agent_id.as_deref().unwrap_or("-"), + client_identity = client_identity.as_deref().and_then(ClientIdentity::primary).unwrap_or("-"), ); info!( @@ -1090,6 +1279,7 @@ async fn handle_http_proxy( agent_name: resolved.agent_name, agent_identifier: resolved.agent_identifier, agent_token, + client_identity, }; let rules = mitm::ResolvedRules { @@ -1268,6 +1458,215 @@ mod tests { ); } + /// Build a `GatewayState` cheap enough for tests: a lazily-connected pool + /// (`connect_lazy` never actually dials — nothing exercised by these + /// tests touches the DB), an in-memory cache/approval store, and a local + /// `CryptoService` key. None of this is real credential material; it + /// exists only so the type checker is satisfied and `/healthz` (which + /// touches none of these fields) can be routed to through the real + /// `handle_connection` path. + async fn test_gateway_state() -> GatewayState { + use base64::Engine; + + let pool = sqlx::postgres::PgPoolOptions::new() + .connect_lazy("postgres://test:test@127.0.0.1/test") + .expect("lazy pool"); + let crypto = Arc::new( + crate::crypto::CryptoService::from_base64_key( + &base64::engine::general_purpose::STANDARD.encode([0u8; 32]), + ) + .expect("crypto"), + ); + let onepassword = Arc::new(crate::vault::onepassword::OnePasswordVaultProvider::new( + pool.clone(), + Arc::clone(&crypto), + )); + let policy_engine = Arc::new(PolicyEngine { + pool: pool.clone(), + crypto: Arc::clone(&crypto), + onepassword: Arc::clone(&onepassword), + }); + let bitwarden = crate::vault::bitwarden::BitwardenVaultProvider::new( + crate::vault::bitwarden::BitwardenConfig { + proxy_url: "wss://example.invalid".to_string(), + }, + pool.clone(), + Arc::clone(&crypto), + ); + let providers: Vec> = vec![Arc::new(bitwarden), onepassword]; + let vault_service = Arc::new(vault::VaultService::new(providers, pool.clone())); + let cache = crate::cache::create_store().await.expect("cache store"); + let approval_store = crate::approval::create_store() + .await + .expect("approval store"); + + let tmp = tempfile::tempdir().expect("tempdir"); + let ca = crate::ca::CertificateAuthority::load_or_generate(tmp.path()) + .await + .expect("test ca"); + + GatewayState { + ca: Arc::new(ca), + http_client: build_http_client(false), + http_client_no_verify: build_http_client(true), + skip_verify_hosts: Arc::new(vec![]), + ws_connector: TlsConnector::from(build_ws_tls_config(false)), + ws_connector_no_verify: TlsConnector::from(build_ws_tls_config(true)), + policy_engine, + cache, + vault_service, + approval_store, + } + } + + /// End-to-end proof that Phase 1 mTLS *threads* identity but does not + /// *enforce* on it: a client cert that verifies successfully is extracted + /// into a `ClientIdentity` (asserted below), but a plain `GET /healthz` + /// over that same connection still gets a normal 200 through the real + /// `handle_connection` path — nothing gates on the identity yet. + #[tokio::test] + async fn mtls_valid_cert_reaches_healthz_with_no_enforcement() { + static INIT: std::sync::Once = std::sync::Once::new(); + INIT.call_once(|| { + let _ = rustls::crypto::ring::default_provider().install_default(); + }); + + // Client CA + a leaf it signs. + let ca_key = rcgen::KeyPair::generate_for(&rcgen::PKCS_ECDSA_P256_SHA256).expect("ca key"); + let mut ca_params = rcgen::CertificateParams::default(); + ca_params.is_ca = rcgen::IsCa::Ca(rcgen::BasicConstraints::Unconstrained); + ca_params + .distinguished_name + .push(rcgen::DnType::CommonName, "Test Client CA"); + ca_params.not_before = time::OffsetDateTime::now_utc() - time::Duration::hours(1); + ca_params.not_after = time::OffsetDateTime::now_utc() + time::Duration::days(1); + let ca_cert = ca_params.self_signed(&ca_key).expect("self-sign ca"); + let ca_pem = ca_cert.pem(); + + let leaf_key = + rcgen::KeyPair::generate_for(&rcgen::PKCS_ECDSA_P256_SHA256).expect("leaf key"); + let mut leaf_params = rcgen::CertificateParams::default(); + leaf_params + .distinguished_name + .push(rcgen::DnType::CommonName, "agent-1"); + leaf_params.not_before = time::OffsetDateTime::now_utc() - time::Duration::hours(1); + leaf_params.not_after = time::OffsetDateTime::now_utc() + time::Duration::hours(24); + let leaf_cert = leaf_params + .signed_by(&leaf_key, &ca_cert, &ca_key) + .expect("sign leaf"); + let leaf_pem = leaf_cert.pem(); + let leaf_key_pem = leaf_key.serialize_pem(); + + // The gateway's own mTLS listener cert (self-signed, "localhost"). + let server_key = + rcgen::KeyPair::generate_for(&rcgen::PKCS_ECDSA_P256_SHA256).expect("server key"); + let mut server_params = + rcgen::CertificateParams::new(vec!["localhost".to_string()]).expect("params"); + server_params.not_before = time::OffsetDateTime::now_utc() - time::Duration::hours(1); + server_params.not_after = time::OffsetDateTime::now_utc() + time::Duration::days(1); + let server_cert = server_params + .self_signed(&server_key) + .expect("self-sign server"); + let server_pem = server_cert.pem(); + let server_key_pem = server_key.serialize_pem(); + let server_der = server_cert.der().clone(); + + // A distinct CA standing in for "the gateway's MITM CA" — only used + // to satisfy `from_parts`'s signature; unrelated to the client CA above. + let mitm_key = + rcgen::KeyPair::generate_for(&rcgen::PKCS_ECDSA_P256_SHA256).expect("mitm key"); + let mitm_cert = rcgen::CertificateParams::default() + .self_signed(&mitm_key) + .expect("self-sign mitm"); + let mitm_der = mitm_cert.der().clone(); + + // Build the real ServerConfig through the same path main.rs uses. + let mtls = client_ca::MtlsConfig::from_parts( + Some("10256"), + Some(&server_pem), + Some(&server_key_pem), + Some(&ca_pem), + &mitm_der, + 10255, + ) + .expect("from_parts") + .expect("mtls configured"); + + // Client trusts the server's self-signed cert directly and presents + // the CA-signed leaf. + let mut roots = rustls::RootCertStore::empty(); + roots.add(server_der).expect("trust server cert"); + let mut key_reader = leaf_key_pem.as_bytes(); + let client_key = rustls_pemfile::private_key(&mut key_reader) + .expect("parse client key") + .expect("client key present"); + let mut cert_reader = leaf_pem.as_bytes(); + let client_chain: Vec<_> = rustls_pemfile::certs(&mut cert_reader) + .collect::>() + .expect("client chain"); + let client_config = Arc::new( + rustls::ClientConfig::builder() + .with_root_certificates(roots) + .with_client_auth_cert(client_chain, client_key) + .expect("client auth cert"), + ); + + let listener = tokio::net::TcpListener::bind("127.0.0.1:0") + .await + .expect("bind"); + let addr = listener.local_addr().expect("addr"); + + let router = Router::new() + .route("/healthz", axum::routing::get(healthz)) + .fallback(fallback); + let state = test_gateway_state().await; + + let server_task = tokio::spawn(async move { + let (stream, peer_addr) = listener.accept().await.expect("accept"); + let acceptor = TlsAcceptor::from(Arc::clone(&mtls.server_config)); + let tls_stream = acceptor.accept(stream).await.expect("server handshake"); + + let client_identity = tls_stream + .get_ref() + .1 + .peer_certificates() + .and_then(client_ca::identity_from_peer_certs) + .map(Arc::new); + assert!(client_identity.is_some(), "identity must be extracted"); + + handle_connection(tls_stream, peer_addr, state, router, client_identity).await + }); + + let client_stream = tokio::net::TcpStream::connect(addr).await.expect("connect"); + let server_name = rustls::pki_types::ServerName::try_from("localhost").expect("name"); + let mut tls_client = tokio_rustls::TlsConnector::from(client_config) + .connect(server_name, client_stream) + .await + .expect("client handshake"); + + use tokio::io::{AsyncReadExt, AsyncWriteExt}; + tls_client + .write_all(b"GET /healthz HTTP/1.1\r\nHost: localhost\r\nConnection: close\r\n\r\n") + .await + .expect("write request"); + + let mut response = Vec::new(); + tls_client + .read_to_end(&mut response) + .await + .expect("read response"); + let response_str = String::from_utf8_lossy(&response); + assert!( + response_str.starts_with("HTTP/1.1 200"), + "expected 200 OK (no enforcement in Phase 1), got: {response_str}" + ); + + server_task + .await + .expect("server task panicked") + .expect("connection handled"); + } + // ── strip_port ────────────────────────────────────────────────────── #[test] @@ -1352,6 +1751,54 @@ mod tests { assert!(parse_patterns("").is_empty()); } + // ── parse_plain_bind_value ─────────────────────────────────────────── + + #[test] + fn plain_bind_defaults_to_unspecified_when_unset() { + assert_eq!( + parse_plain_bind_value(None).unwrap(), + IpAddr::V4(Ipv4Addr::UNSPECIFIED) + ); + } + + #[test] + fn plain_bind_defaults_to_unspecified_when_empty() { + assert_eq!( + parse_plain_bind_value(Some("")).unwrap(), + IpAddr::V4(Ipv4Addr::UNSPECIFIED) + ); + assert_eq!( + parse_plain_bind_value(Some(" ")).unwrap(), + IpAddr::V4(Ipv4Addr::UNSPECIFIED) + ); + } + + #[test] + fn plain_bind_parses_valid_ip() { + assert_eq!( + parse_plain_bind_value(Some("127.0.0.1")).unwrap(), + IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)) + ); + } + + /// FIX 1: a set-but-unparseable value must fail closed (`Err`), not + /// silently fall back to the wide-open `0.0.0.0` default — that default + /// is exactly what this knob exists to let an operator narrow. + #[test] + fn plain_bind_unparseable_value_errs_naming_var_and_value() { + let err = parse_plain_bind_value(Some("127.0.0.q")).unwrap_err(); + let msg = format!("{err:#}"); + assert!(msg.contains("GATEWAY_PLAIN_BIND"), "message: {msg}"); + assert!(msg.contains("127.0.0.q"), "message: {msg}"); + } + + #[test] + fn plain_bind_hostname_is_not_an_ip_literal_errs() { + // "localhost" is a valid hostname but not an IP literal — parsing it + // as an IpAddr must fail rather than silently resolve or default. + assert!(parse_plain_bind_value(Some("localhost")).is_err()); + } + // ── is_http_proxy_request ────────────────────────────────────────── #[test] diff --git a/apps/gateway/src/gateway/mitm.rs b/apps/gateway/src/gateway/mitm.rs index 47507143..0e0dea6f 100644 --- a/apps/gateway/src/gateway/mitm.rs +++ b/apps/gateway/src/gateway/mitm.rs @@ -479,10 +479,8 @@ mod tests { ProxyContext { project_id: Some("p1".to_string()), organization_id: Some("o1".to_string()), - agent_id: None, - agent_name: None, - agent_identifier: None, agent_token: Some("tok".to_string()), + ..Default::default() } } diff --git a/apps/gateway/src/main.rs b/apps/gateway/src/main.rs index d66b9f19..edca7c79 100644 --- a/apps/gateway/src/main.rs +++ b/apps/gateway/src/main.rs @@ -10,6 +10,7 @@ mod auth; mod auth; mod ca; +mod client_ca; #[cfg(not(edition_cloud))] mod cache; @@ -239,6 +240,16 @@ async fn main() -> Result<()> { let ca = CertificateAuthority::load_or_generate(&data_dir).await?; info!("CA certificate loaded"); + // 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)?; + match &mtls { + Some(m) => info!(port = m.port, "mTLS client-certificate listener configured"), + None => info!("mTLS disabled (GATEWAY_MTLS_PORT not set)"), + } + // Connect to PostgreSQL // Support both DATABASE_URL (OSS) and individual DB_* vars (cloud ECS from Secrets Manager) let database_url = match std::env::var("DATABASE_URL") { @@ -317,7 +328,8 @@ async fn main() -> Result<()> { vault_service, cache, approval_store, - ); + mtls, + )?; let result = server.run().await; // The drain, in the one order that does not lose data: connections first diff --git a/apps/gateway/src/policy_engine/enforce.rs b/apps/gateway/src/policy_engine/enforce.rs index 0af4c190..8ff8cf8d 100644 --- a/apps/gateway/src/policy_engine/enforce.rs +++ b/apps/gateway/src/policy_engine/enforce.rs @@ -242,9 +242,8 @@ mod tests { project_id: Some("p1".to_string()), organization_id: Some("o1".to_string()), agent_id: Some("a1".to_string()), - agent_name: None, - agent_identifier: None, agent_token: Some("t".to_string()), + ..Default::default() } } diff --git a/docker/Dockerfile b/docker/Dockerfile index f86fed37..6c706fd9 100644 --- a/docker/Dockerfile +++ b/docker/Dockerfile @@ -117,7 +117,7 @@ VOLUME ["/app/data"] USER node -EXPOSE 10254 10255 +EXPOSE 10254 10255 10256 HEALTHCHECK --interval=10s --timeout=5s --start-period=60s --retries=3 \ CMD wget -qO- http://127.0.0.1:10254/v1/health && wget -qO- http://127.0.0.1:10255/healthz || exit 1 diff --git a/docker/docker-compose.yml b/docker/docker-compose.yml index f7365003..220e6c39 100644 --- a/docker/docker-compose.yml +++ b/docker/docker-compose.yml @@ -31,6 +31,9 @@ services: ports: - "${ONECLI_BIND_HOST:-127.0.0.1}:${ONECLI_APP_PORT:-10254}:10254" - "${ONECLI_BIND_HOST:-127.0.0.1}:${ONECLI_GATEWAY_PORT:-10255}:10255" + # mTLS client-certificate listener — off by default (GATEWAY_MTLS_PORT + # unset). Uncomment to publish it, and set the env block below. + # - "${ONECLI_BIND_HOST:-127.0.0.1}:${ONECLI_GATEWAY_MTLS_PORT:-10256}:10256" environment: DATABASE_URL: postgresql://${POSTGRES_USER:-onecli}:${POSTGRES_PASSWORD:-onecli}@postgres:5432/${POSTGRES_DB:-onecli} NEXTAUTH_SECRET: ${NEXTAUTH_SECRET:-} @@ -53,6 +56,14 @@ services: # sending internal vault calls out over the internet, whatever version the # `image:` above pins. INTERNAL_API_URL: http://localhost:10254 + # mTLS is OFF unless GATEWAY_MTLS_PORT is set — uncomment all four to + # enable it. GATEWAY_TLS_CERT/KEY/CLIENT_CA accept either an inline PEM + # value (starts with "-----BEGIN") or a filesystem path mounted into the + # container. GATEWAY_CLIENT_CA must NOT be the gateway's own MITM CA. + # GATEWAY_MTLS_PORT: "10256" + # GATEWAY_TLS_CERT: /run/secrets/gateway-tls-cert.pem + # GATEWAY_TLS_KEY: /run/secrets/gateway-tls-key.pem + # GATEWAY_CLIENT_CA: /run/secrets/gateway-client-ca.pem volumes: - app-data:/app/data env_file: