diff --git a/crates/openshell-core/src/config.rs b/crates/openshell-core/src/config.rs index 56e314127d..aa873dab48 100644 --- a/crates/openshell-core/src/config.rs +++ b/crates/openshell-core/src/config.rs @@ -203,6 +203,13 @@ pub struct Config { /// When `None`, the dedicated metrics listener is disabled. pub metrics_bind_address: Option, + /// TLS configuration for the dedicated metrics listener. + /// + /// When `None`, the metrics listener retains its plaintext HTTP behavior. + /// This configuration is intentionally separate from [`Self::tls`] so + /// metrics clients can use their own client CA trust boundary. + pub metrics_tls: Option, + /// Log level (trace, debug, info, warn, error). pub log_level: String, @@ -351,6 +358,27 @@ pub struct TlsConfig { pub external_server_names: Vec, } +/// TLS configuration for the dedicated metrics listener. +/// +/// Unlike [`TlsConfig`], metrics TLS does not support external certificates +/// or SNI routing. The metrics listener has one narrow HTTPS/mTLS contract +/// with an optional, metrics-specific client CA. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct MetricsTlsConfig { + /// Path to the TLS certificate file served by the metrics listener. + pub cert_path: PathBuf, + + /// Path to the TLS private key file served by the metrics listener. + pub key_path: PathBuf, + + /// Path to the CA certificate file used to verify metrics clients. + pub client_ca_path: Option, + + /// Whether metrics clients must present a certificate trusted by + /// [`Self::client_ca_path`]. + pub require_client_auth: bool, +} + /// OIDC (`OpenID` Connect) configuration for JWT-based authentication. /// /// When configured, the server validates `authorization: Bearer ` @@ -877,6 +905,7 @@ impl Config { bind_address: default_bind_address(), health_bind_address: None, metrics_bind_address: None, + metrics_tls: None, log_level: default_log_level(), policy_validation_failure_mode: PolicyValidationFailureMode::default(), tls, @@ -926,6 +955,13 @@ impl Config { self } + /// Create a new configuration with TLS for the dedicated metrics listener. + #[must_use] + pub fn with_metrics_tls(mut self, tls: MetricsTlsConfig) -> Self { + self.metrics_tls = Some(tls); + self + } + /// Create a new configuration with the given log level. #[must_use] pub fn with_log_level(mut self, level: impl Into) -> Self { @@ -1147,11 +1183,12 @@ mod tests { use super::{ AppArmorProfile, Config, DEFAULT_SERVICE_ROUTING_DOMAIN, GatewayInterceptorBindingPolicy, GatewayInterceptorConfig, GatewayInterceptorFailurePolicy, GatewayJwtConfig, - GatewayProviderProfileSourceConfig, ImagePullPolicy, PolicyValidationFailureMode, - UpstreamProxyConfig, default_sandbox_pids_limit, normalize_compute_driver_name, + GatewayProviderProfileSourceConfig, ImagePullPolicy, MetricsTlsConfig, + PolicyValidationFailureMode, UpstreamProxyConfig, default_sandbox_pids_limit, + normalize_compute_driver_name, }; - use std::net::SocketAddr; use std::time::Duration; + use std::{net::SocketAddr, path::PathBuf}; #[test] fn policy_validation_failure_mode_is_secure_by_default() { @@ -1637,6 +1674,23 @@ mod tests { assert_eq!(cfg.health_bind_address, Some(addr)); } + #[test] + fn metrics_tls_is_disabled_by_default_and_can_be_configured() { + assert!(Config::new(None).metrics_tls.is_none()); + + let tls = MetricsTlsConfig { + cert_path: PathBuf::from("/etc/openshell-metrics/tls.crt"), + key_path: PathBuf::from("/etc/openshell-metrics/tls.key"), + client_ca_path: Some(PathBuf::from("/etc/openshell-metrics/ca.crt")), + require_client_auth: true, + }; + + assert_eq!( + Config::new(None).with_metrics_tls(tls.clone()).metrics_tls, + Some(tls) + ); + } + #[test] fn supervisor_image_tag_prefers_explicit_build_tags() { use super::resolve_supervisor_image_tag; diff --git a/crates/openshell-core/src/lib.rs b/crates/openshell-core/src/lib.rs index 6f7a050f1c..69a00f4454 100644 --- a/crates/openshell-core/src/lib.rs +++ b/crates/openshell-core/src/lib.rs @@ -64,8 +64,8 @@ pub use config::{ AppArmorProfile, Config, GatewayAuthConfig, GatewayInterceptorBindingOverride, GatewayInterceptorBindingPolicy, GatewayInterceptorConfig, GatewayInterceptorFailurePolicy, GatewayInterceptorPhaseConfig, GatewayJwtConfig, GatewayProviderProfileSourceConfig, - ImagePullPolicy, MtlsAuthConfig, OidcConfig, PolicyValidationFailureMode, TlsConfig, - UpstreamProxyConfig, + ImagePullPolicy, MetricsTlsConfig, MtlsAuthConfig, OidcConfig, PolicyValidationFailureMode, + TlsConfig, UpstreamProxyConfig, }; pub use dynamic_string_allowlist::DynamicStringAllowlist; pub use error::{ComputeDriverError, Error, Result}; diff --git a/crates/openshell-server/src/cli.rs b/crates/openshell-server/src/cli.rs index bfe237af39..367beeb499 100644 --- a/crates/openshell-server/src/cli.rs +++ b/crates/openshell-server/src/cli.rs @@ -104,6 +104,29 @@ struct RunArgs { #[arg(long, default_value_t = 0, env = "OPENSHELL_METRICS_PORT")] metrics_port: u16, + /// Path to the TLS certificate served by the metrics listener. + #[arg(long, env = "OPENSHELL_METRICS_TLS_CERT")] + metrics_tls_cert: Option, + + /// Path to the TLS private key served by the metrics listener. + #[arg(long, env = "OPENSHELL_METRICS_TLS_KEY")] + metrics_tls_key: Option, + + /// Path to the CA certificate used to verify metrics clients. + #[arg(long, env = "OPENSHELL_METRICS_TLS_CLIENT_CA")] + metrics_tls_client_ca: Option, + + /// Require metrics clients to present a certificate trusted by the metrics client CA. + #[arg( + long, + env = "OPENSHELL_METRICS_REQUIRE_CLIENT_AUTH", + num_args = 0..=1, + default_missing_value = "true", + require_equals = true, + value_parser = clap::value_parser!(bool) + )] + metrics_require_client_auth: Option, + /// Log level (trace, debug, info, warn, error). #[arg(long, default_value = "info", env = "OPENSHELL_LOG_LEVEL")] log_level: String, @@ -477,6 +500,12 @@ fn prepare_server_config_with_drivers( "metrics_port", || file_gateway.and_then(|g| g.metrics_bind_address), ); + let metrics_tls = resolve_metrics_tls(args)?; + if metrics_tls.is_some() && metrics_bind.is_none() { + return Err(miette::miette!( + "metrics TLS requires an enabled metrics listener; set --metrics-port or metrics_bind_address" + )); + } if let Some(addr) = health_bind { if args.port == addr.port() { @@ -505,6 +534,9 @@ fn prepare_server_config_with_drivers( } config = config.with_metrics_bind_address(addr); } + if let Some(tls) = metrics_tls { + config = config.with_metrics_tls(tls); + } config = config.with_database_url(db_url); if let Some(driver) = &args.compute_driver { @@ -989,6 +1021,7 @@ fn validate_preflight_semantics( "an explicit --tls-client-ca requires --tls-cert and --tls-key" )); } + let metrics_tls = resolve_metrics_tls(args)?; if !args.disable_tls && let Some(tls) = gateway.tls.as_ref() { @@ -1030,6 +1063,11 @@ fn validate_preflight_semantics( "metrics_port", || gateway.metrics_bind_address, ); + if metrics_tls.is_some() && metrics_bind.is_none() { + return Err(miette::miette!( + "metrics TLS requires an enabled metrics listener" + )); + } if health_bind.is_some_and(|address| address.port() == args.port) || metrics_bind.is_some_and(|address| address.port() == args.port) || health_bind @@ -1124,6 +1162,43 @@ fn resolve_aux_listener( } } +/// Resolve the optional TLS configuration for the dedicated metrics listener. +/// +/// A metrics TLS configuration is activated by any metrics TLS input. It is +/// deliberately independent from the gateway listener TLS settings so the +/// metrics client CA remains a separate trust boundary. +fn resolve_metrics_tls(args: &RunArgs) -> Result> { + let configured = args.metrics_tls_cert.is_some() + || args.metrics_tls_key.is_some() + || args.metrics_tls_client_ca.is_some() + || args.metrics_require_client_auth == Some(true); + if !configured { + return Ok(None); + } + + let cert_path = args + .metrics_tls_cert + .clone() + .ok_or_else(|| miette::miette!("metrics TLS requires --metrics-tls-cert"))?; + let key_path = args + .metrics_tls_key + .clone() + .ok_or_else(|| miette::miette!("metrics TLS requires --metrics-tls-key"))?; + let require_client_auth = args.metrics_require_client_auth.unwrap_or(false); + if require_client_auth && args.metrics_tls_client_ca.is_none() { + return Err(miette::miette!( + "--metrics-require-client-auth requires --metrics-tls-client-ca" + )); + } + + Ok(Some(openshell_core::MetricsTlsConfig { + cert_path, + key_path, + client_ca_path: args.metrics_tls_client_ca.clone(), + require_client_auth, + })) +} + /// Apply gateway-wide values from `[openshell.gateway]` onto `RunArgs` for /// every argument that is still sourced from clap's built-in default. /// @@ -1196,6 +1271,21 @@ fn merge_file_into_args(args: &mut RunArgs, file: &GatewayFileSection, matches: args.tls_client_ca.clone_from(&tls.client_ca_path); } } + // Dedicated metrics listener TLS fields. + if let Some(tls) = &file.metrics_tls { + if args.metrics_tls_cert.is_none() && arg_defaulted(matches, "metrics_tls_cert") { + args.metrics_tls_cert = Some(tls.cert_path.clone()); + } + if args.metrics_tls_key.is_none() && arg_defaulted(matches, "metrics_tls_key") { + args.metrics_tls_key = Some(tls.key_path.clone()); + } + if args.metrics_tls_client_ca.is_none() && arg_defaulted(matches, "metrics_tls_client_ca") { + args.metrics_tls_client_ca.clone_from(&tls.client_ca_path); + } + if arg_defaulted(matches, "metrics_require_client_auth") { + args.metrics_require_client_auth = Some(tls.require_client_auth); + } + } // OIDC fields if let Some(oidc) = &file.oidc { if args.oidc_issuer.is_none() && arg_defaulted(matches, "oidc_issuer") { @@ -1313,6 +1403,7 @@ mod tests { use crate::TEST_ENV_LOCK as ENV_LOCK; use clap::Parser; use std::net::{IpAddr, Ipv4Addr}; + use std::path::PathBuf; use std::sync::atomic::{AtomicUsize, Ordering}; static REGISTRY_DETECTION_CALLS: AtomicUsize = AtomicUsize::new(0); @@ -2539,7 +2630,7 @@ mod tests { // // by exercising each combination on representative gateway fields. - use super::{ConfigFile, merge_file_into_args}; + use super::{ConfigFile, merge_file_into_args, resolve_metrics_tls}; use clap::FromArgMatches; fn parse_with_args(argv: &[&str]) -> (super::RunArgs, clap::ArgMatches) { @@ -2552,6 +2643,104 @@ mod tests { toml::from_str(toml).expect("valid TOML in test fixture") } + #[test] + fn metrics_tls_file_values_merge_into_runtime_configuration() { + let _lock = ENV_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let _cert = EnvVarGuard::remove("OPENSHELL_METRICS_TLS_CERT"); + let _key = EnvVarGuard::remove("OPENSHELL_METRICS_TLS_KEY"); + let _ca = EnvVarGuard::remove("OPENSHELL_METRICS_TLS_CLIENT_CA"); + let _require = EnvVarGuard::remove("OPENSHELL_METRICS_REQUIRE_CLIENT_AUTH"); + let (mut args, matches) = parse_with_args(&[ + "openshell-gateway", + "--db-url", + "sqlite::memory:", + "--metrics-port", + "9090", + ]); + let file = config_file_from_toml( + r#" +[openshell.gateway.metrics_tls] +cert_path = "/metrics/tls.crt" +key_path = "/metrics/tls.key" +client_ca_path = "/metrics/ca.crt" +require_client_auth = true +"#, + ); + + merge_file_into_args(&mut args, &file.openshell.gateway, &matches); + let tls = resolve_metrics_tls(&args) + .expect("valid metrics TLS configuration") + .expect("metrics TLS is configured"); + + assert_eq!(tls.cert_path, PathBuf::from("/metrics/tls.crt")); + assert_eq!(tls.key_path, PathBuf::from("/metrics/tls.key")); + assert_eq!(tls.client_ca_path, Some(PathBuf::from("/metrics/ca.crt"))); + assert!(tls.require_client_auth); + } + + #[test] + fn explicit_metrics_client_auth_value_overrides_file_value() { + let _lock = ENV_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let _cert = EnvVarGuard::remove("OPENSHELL_METRICS_TLS_CERT"); + let _key = EnvVarGuard::remove("OPENSHELL_METRICS_TLS_KEY"); + let _ca = EnvVarGuard::remove("OPENSHELL_METRICS_TLS_CLIENT_CA"); + let _require = EnvVarGuard::remove("OPENSHELL_METRICS_REQUIRE_CLIENT_AUTH"); + let (mut args, matches) = parse_with_args(&[ + "openshell-gateway", + "--db-url", + "sqlite::memory:", + "--metrics-port", + "9090", + "--metrics-require-client-auth=false", + ]); + let file = config_file_from_toml( + r#" +[openshell.gateway.metrics_tls] +cert_path = "/metrics/tls.crt" +key_path = "/metrics/tls.key" +client_ca_path = "/metrics/ca.crt" +require_client_auth = true +"#, + ); + + merge_file_into_args(&mut args, &file.openshell.gateway, &matches); + let tls = resolve_metrics_tls(&args) + .expect("valid metrics TLS configuration") + .expect("metrics TLS is configured"); + + assert!(!tls.require_client_auth); + } + + #[test] + fn metrics_client_auth_requires_a_metrics_client_ca() { + let _lock = ENV_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let _cert = EnvVarGuard::remove("OPENSHELL_METRICS_TLS_CERT"); + let _key = EnvVarGuard::remove("OPENSHELL_METRICS_TLS_KEY"); + let _ca = EnvVarGuard::remove("OPENSHELL_METRICS_TLS_CLIENT_CA"); + let _require = EnvVarGuard::remove("OPENSHELL_METRICS_REQUIRE_CLIENT_AUTH"); + let (args, _) = parse_with_args(&[ + "openshell-gateway", + "--metrics-tls-cert", + "/metrics/tls.crt", + "--metrics-tls-key", + "/metrics/tls.key", + "--metrics-require-client-auth", + ]); + + let error = resolve_metrics_tls(&args).expect_err("client auth without a CA must fail"); + assert!( + error + .to_string() + .contains("--metrics-require-client-auth requires --metrics-tls-client-ca") + ); + } + #[test] fn rejects_legacy_drivers_flag() { let error = command() @@ -2590,7 +2779,7 @@ mod tests { assert_eq!( super::resolve_config_path(&args).unwrap(), - Some(std::path::PathBuf::from("/tmp/missing.toml")) + Some(PathBuf::from("/tmp/missing.toml")) ); } diff --git a/crates/openshell-server/src/config_file.rs b/crates/openshell-server/src/config_file.rs index efa0182967..88e7b34896 100644 --- a/crates/openshell-server/src/config_file.rs +++ b/crates/openshell-server/src/config_file.rs @@ -97,6 +97,8 @@ pub struct GatewayFileSection { pub health_bind_address: Option, #[serde(default)] pub metrics_bind_address: Option, + #[serde(default)] + pub metrics_tls: Option, // ── Logging ────────────────────────────────────────────────────────── #[serde(default)] @@ -276,6 +278,21 @@ impl TryFrom for OcsfLogConfig { }) } } +/// TLS fields for the dedicated metrics listener. +/// +/// This is separate from [`GatewayTlsFileConfig`] so metrics clients can +/// trust a dedicated client CA and cannot inherit the gateway listener's +/// wider certificate and SNI configuration. +#[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct MetricsTlsFileConfig { + pub cert_path: PathBuf, + pub key_path: PathBuf, + #[serde(default)] + pub client_ca_path: Option, + #[serde(default)] + pub require_client_auth: bool, +} /// `[openshell.gateway.otlp]` section. /// /// Presence of this table enables OTLP export; there is no `enabled` flag. @@ -824,6 +841,7 @@ version = 2 [openshell.gateway] bind_address = "0.0.0.0:8080" health_bind_address = "0.0.0.0:8081" +metrics_bind_address = "0.0.0.0:9090" log_level = "info" compute_driver = "kubernetes" credential_drivers = ["kubernetes-secrets"] @@ -836,6 +854,12 @@ cert_path = "/etc/openshell/certs/gateway.pem" key_path = "/etc/openshell/certs/gateway-key.pem" client_ca_path = "/etc/openshell/certs/client-ca.pem" +[openshell.gateway.metrics_tls] +cert_path = "/etc/openshell/metrics/server/tls.crt" +key_path = "/etc/openshell/metrics/server/tls.key" +client_ca_path = "/etc/openshell/metrics/client-ca/ca.crt" +require_client_auth = true + [openshell.gateway.oidc] issuer = "https://idp.example.com/realms/openshell" audience = "openshell-cli" @@ -864,6 +888,16 @@ namespace = "agents" Some(openshell_core::PolicyValidationFailureMode::RetainLastValid) ); assert!(gw.tls.is_some()); + let metrics_tls = gw.metrics_tls.as_ref().expect("metrics TLS config parses"); + assert_eq!( + metrics_tls.cert_path, + Path::new("/etc/openshell/metrics/server/tls.crt") + ); + assert_eq!( + metrics_tls.client_ca_path.as_deref(), + Some(Path::new("/etc/openshell/metrics/client-ca/ca.crt")) + ); + assert!(metrics_tls.require_client_auth); let oidc = gw.oidc.as_ref().expect("OIDC config parses"); assert!(!oidc.dangerously_allow_insecure_http); assert_eq!( diff --git a/crates/openshell-server/src/lib.rs b/crates/openshell-server/src/lib.rs index 27dda635d8..01365d78bc 100644 --- a/crates/openshell-server/src/lib.rs +++ b/crates/openshell-server/src/lib.rs @@ -54,6 +54,12 @@ mod tracing_setup; mod watch_cursor; mod ws_tunnel; +use axum::Router; +use hyper_util::{ + rt::{TokioExecutor, TokioIo}, + server::conn::auto::Builder, + service::TowerToHyperService, +}; use openshell_core::net::set_tcp_nodelay_best_effort; use openshell_core::telemetry::TelemetryComputeDriver; use openshell_core::{Config, Error, ObjectLabels, Result}; @@ -365,6 +371,15 @@ fn is_benign_tls_handshake_failure(error: &std::io::Error) -> bool { ) } +fn log_metrics_tls_handshake_rejection(error: &std::io::Error, client: SocketAddr) { + warn!( + event = "metrics_tls_handshake_rejected", + error = %error, + client = %client, + "Rejected metrics TLS handshake; verify that the client uses HTTPS and, when client authentication is enabled, a certificate trusted by the metrics client CA" + ); +} + fn is_benign_connection_close(error: &(dyn std::error::Error + 'static)) -> bool { openshell_core::transport_errors::is_expected_transport_close_error(error) } @@ -859,17 +874,38 @@ pub(crate) async fn run_server( "failed to bind metrics port {metrics_bind_address}: {e}", )) })?; - info!(address = %metrics_bind_address, "Metrics server listening"); - tokio::spawn(async move { - if let Err(e) = axum::serve( + if let Some(metrics_tls) = &config.metrics_tls { + let metrics_tls_acceptor = TlsAcceptor::from_files( + &metrics_tls.cert_path, + &metrics_tls.key_path, + metrics_tls.client_ca_path.as_deref(), + metrics_tls.require_client_auth, + None, + None, + Vec::new(), + )?; + metrics_tls_acceptor.spawn_reload_worker_for_listener(shutdown_rx.clone(), "metrics"); + + info!(address = %metrics_bind_address, "Metrics TLS server listening"); + tokio::spawn(serve_tls_metrics_listener( metrics_listener, - metrics_router(prometheus_handle).into_make_service(), - ) - .await - { - error!("Metrics server error: {e}"); - } - }); + metrics_router(prometheus_handle), + metrics_tls_acceptor, + shutdown_rx.clone(), + )); + } else { + info!(address = %metrics_bind_address, "Metrics server listening"); + tokio::spawn(async move { + if let Err(e) = axum::serve( + metrics_listener, + metrics_router(prometheus_handle).into_make_service(), + ) + .await + { + error!("Metrics server error: {e}"); + } + }); + } } else { info!("Metrics server disabled"); } @@ -982,6 +1018,68 @@ pub(crate) async fn run_server( Ok(()) } +/// Serve the metrics endpoint through a dedicated TLS listener. +/// +/// This listener exists only when `metrics_tls` is configured. The default +/// metrics listener remains plaintext for backwards compatibility. +async fn serve_tls_metrics_listener( + listener: TcpListener, + router: Router, + tls_acceptor: TlsAcceptor, + mut shutdown: watch::Receiver, +) { + loop { + let accepted = tokio::select! { + changed = shutdown.changed() => { + if changed.is_err() || *shutdown.borrow() { + break; + } + continue; + } + accepted = listener.accept() => accepted, + }; + + let (stream, addr) = match accepted { + Ok(connection) => connection, + Err(error) => { + error!(error = %error, "Failed to accept metrics connection"); + continue; + } + }; + set_tcp_nodelay_best_effort(&stream); + + let acceptor = tls_acceptor.clone(); + let service = TowerToHyperService::new(router.clone()); + tokio::spawn(async move { + match tokio::time::timeout(Duration::from_secs(10), acceptor.acceptor().accept(stream)) + .await + { + Ok(Ok(tls_stream)) => { + if let Err(error) = Builder::new(TokioExecutor::new()) + .serve_connection_with_upgrades(TokioIo::new(tls_stream), service) + .await + { + if is_benign_connection_close(error.as_ref()) { + debug!(error = %error, client = %addr, "Metrics connection closed"); + } else { + error!(error = %error, client = %addr, "Metrics connection error"); + } + } + } + Ok(Err(error)) if is_benign_tls_handshake_failure(&error) => { + debug!(error = %error, client = %addr, "Metrics TLS handshake closed early"); + } + Ok(Err(error)) => { + log_metrics_tls_handshake_rejection(&error, addr); + } + Err(_) => { + warn!(client = %addr, "Metrics TLS handshake timed out"); + } + } + }); + } +} + async fn serve_gateway_listener( bound_listener: BoundGatewayListener, service: MultiplexService, @@ -1851,15 +1949,20 @@ mod tests { BoundGatewayListener, ConfiguredComputeDriver, ConnectionProtocol, ExtensionKind, MultiplexService, ServerState, TlsAcceptor, allow_plaintext_service_http, bind_gateway_listener, classify_initial_bytes, configured_compute_driver, - extension_token_ttl, is_benign_tls_handshake_failure, mint_gateway_extension_credential, - serve_gateway_listener, validate_peer_endpoint_scheme, + extension_token_ttl, is_benign_tls_handshake_failure, log_metrics_tls_handshake_rejection, + mint_gateway_extension_credential, serve_gateway_listener, serve_tls_metrics_listener, + validate_peer_endpoint_scheme, }; + use axum::{Router, routing::get}; use openshell_core::{ Config, proto::{HealthRequest, open_shell_client::OpenShellClient}, }; - use std::io::{Error, ErrorKind}; + use rcgen::{CertificateParams, KeyPair, KeyUsagePurpose}; + use std::fs::{self, File}; + use std::io::{BufReader, Error, ErrorKind}; use std::net::SocketAddr; + use std::path::{Path, PathBuf}; use std::sync::{ Arc, LazyLock, Mutex, atomic::{AtomicBool, Ordering}, @@ -1869,6 +1972,7 @@ mod tests { use tokio::io::{AsyncReadExt, AsyncWriteExt}; use tokio::net::{TcpListener, TcpStream}; use tokio::sync::watch; + use tokio_rustls::TlsConnector; use crate::tls_test_utils::generate_test_certs_with_ca; use axum::body::Body; @@ -1887,6 +1991,69 @@ mod tests { })) } + fn test_tls_connector(ca_path: &Path, client_auth: Option<(&Path, &Path)>) -> TlsConnector { + let mut roots = rustls::RootCertStore::empty(); + let file = File::open(ca_path).expect("failed to open test CA certificate"); + for certificate in rustls_pemfile::certs(&mut BufReader::new(file)) { + roots + .add(certificate.expect("failed to parse test CA certificate")) + .expect("failed to add test CA certificate"); + } + + let config = if let Some((cert_path, key_path)) = client_auth { + let cert_file = File::open(cert_path).expect("failed to open test client certificate"); + let cert_chain = rustls_pemfile::certs(&mut BufReader::new(cert_file)) + .collect::, _>>() + .expect("failed to parse test client certificate"); + let key_file = File::open(key_path).expect("failed to open test client key"); + let private_key = rustls_pemfile::private_key(&mut BufReader::new(key_file)) + .expect("failed to parse test client key") + .expect("test client key is missing"); + + Arc::new( + rustls::ClientConfig::builder() + .with_root_certificates(roots) + .with_client_auth_cert(cert_chain, private_key) + .expect("failed to configure test client certificate"), + ) + } else { + Arc::new( + rustls::ClientConfig::builder() + .with_root_certificates(roots) + .with_no_client_auth(), + ) + }; + + TlsConnector::from(config) + } + + fn write_test_client_certificate( + directory: &Path, + name: &str, + ca_cert: &rcgen::Certificate, + ca_key: &KeyPair, + ) -> (PathBuf, PathBuf) { + let client_key = KeyPair::generate().expect("failed to generate test client key"); + let mut client_params = + CertificateParams::new(Vec::::new()).expect("failed to create client params"); + client_params + .distinguished_name + .push(rcgen::DnType::CommonName, name); + client_params.key_usages = vec![ + KeyUsagePurpose::DigitalSignature, + KeyUsagePurpose::KeyEncipherment, + ]; + let client_cert = client_params + .signed_by(&client_key, ca_cert, ca_key) + .expect("failed to sign test client certificate"); + + let cert_path = directory.join(format!("{name}-cert.pem")); + let key_path = directory.join(format!("{name}-key.pem")); + fs::write(&cert_path, client_cert.pem()).expect("failed to write test client certificate"); + fs::write(&key_path, client_key.serialize_pem()).expect("failed to write test client key"); + (cert_path, key_path) + } + #[test] fn plaintext_peer_endpoint_is_rejected_on_a_tls_gateway() { let error = validate_peer_endpoint_scheme(&tls_enabled_config(), "http://10.0.0.1:8080") @@ -2231,6 +2398,282 @@ mod tests { let _ = tokio::time::timeout(Duration::from_secs(2), handle).await; } + #[derive(Clone)] + struct TraceBuffer(Arc>>); + + impl std::io::Write for TraceBuffer { + fn write(&mut self, bytes: &[u8]) -> std::io::Result { + self.0 + .lock() + .expect("trace buffer lock") + .extend_from_slice(bytes); + Ok(bytes.len()) + } + + fn flush(&mut self) -> std::io::Result<()> { + Ok(()) + } + } + + #[test] + fn metrics_tls_handshake_rejection_logs_actionable_context() { + use tracing_subscriber::layer::SubscriberExt as _; + + let buffer = Arc::new(Mutex::new(Vec::new())); + let writer = TraceBuffer(buffer.clone()); + let subscriber = tracing_subscriber::registry().with( + tracing_subscriber::fmt::layer() + .with_writer(move || writer.clone()) + .with_ansi(false), + ); + let _traced = crate::otel_tracing::test_exporter::install_scoped(subscriber); + + log_metrics_tls_handshake_rejection( + &Error::new(ErrorKind::InvalidData, "untrusted client certificate"), + "192.0.2.10:443".parse().expect("valid test client address"), + ); + + let output = String::from_utf8(buffer.lock().expect("trace buffer lock").clone()) + .expect("tracing output is UTF-8"); + assert!(output.contains("WARN"), "log output: {output}"); + assert!( + output.contains("metrics_tls_handshake_rejected"), + "log output: {output}" + ); + assert!( + output.contains("untrusted client certificate"), + "log output: {output}" + ); + assert!(output.contains("192.0.2.10:443"), "log output: {output}"); + assert!( + output.contains("Rejected metrics TLS handshake"), + "log output: {output}" + ); + } + + #[tokio::test] + async fn tls_metrics_listener_serves_https_and_rejects_plaintext() { + let listener = TcpListener::bind("127.0.0.1:0") + .await + .expect("failed to bind metrics listener"); + let address = listener + .local_addr() + .expect("failed to read metrics listener address"); + let (tls_dir, tls_acceptor) = test_tls_acceptor(); + let (shutdown_tx, shutdown_rx) = watch::channel(false); + let listener_task = tokio::spawn(serve_tls_metrics_listener( + listener, + Router::new().route("/metrics", get(|| async { "metrics" })), + tls_acceptor, + shutdown_rx, + )); + + let connector = test_tls_connector(&tls_dir.path().join("ca.pem"), None); + let stream = TcpStream::connect(address) + .await + .expect("failed to connect to metrics listener"); + let mut tls_stream = connector + .connect( + rustls::pki_types::ServerName::try_from("localhost") + .expect("invalid test server name") + .to_owned(), + stream, + ) + .await + .expect("metrics TLS handshake failed"); + tls_stream + .write_all(b"GET /metrics HTTP/1.1\r\nHost: localhost\r\nConnection: close\r\n\r\n") + .await + .expect("failed to request metrics over TLS"); + let mut response = Vec::new(); + tls_stream + .read_to_end(&mut response) + .await + .expect("failed to read metrics TLS response"); + assert!( + String::from_utf8_lossy(&response).starts_with("HTTP/1.1 200"), + "expected a successful HTTPS metrics response" + ); + + let mut plaintext = TcpStream::connect(address) + .await + .expect("failed to connect for plaintext request"); + plaintext + .write_all(b"GET /metrics HTTP/1.1\r\nHost: localhost\r\nConnection: close\r\n\r\n") + .await + .expect("failed to write plaintext metrics request"); + let mut plaintext_response = Vec::new(); + let plaintext_result = tokio::time::timeout( + Duration::from_secs(2), + plaintext.read_to_end(&mut plaintext_response), + ) + .await + .expect("timed out waiting for plaintext metrics connection to close"); + if let Err(error) = plaintext_result { + assert_eq!(error.kind(), ErrorKind::ConnectionReset); + } + assert!( + !String::from_utf8_lossy(&plaintext_response).starts_with("HTTP/"), + "TLS metrics listener must not serve plaintext HTTP" + ); + + stop_listener(shutdown_tx, listener_task).await; + } + + #[tokio::test] + async fn metrics_mtls_trusts_only_the_metrics_client_ca() { + let directory = tempdir().expect("failed to create test directory"); + let (_server_ca_cert, _server_ca_key) = generate_test_certs_with_ca(directory.path()); + + let metrics_ca_directory = directory.path().join("metrics-client-ca"); + fs::create_dir(&metrics_ca_directory).expect("failed to create metrics CA directory"); + let (metrics_ca_cert, metrics_ca_key) = generate_test_certs_with_ca(&metrics_ca_directory); + + let gateway_ca_directory = directory.path().join("gateway-client-ca"); + fs::create_dir(&gateway_ca_directory).expect("failed to create gateway CA directory"); + let (gateway_ca_cert, gateway_ca_key) = generate_test_certs_with_ca(&gateway_ca_directory); + + let (metrics_client_cert, metrics_client_key) = write_test_client_certificate( + directory.path(), + "metrics-client", + &metrics_ca_cert, + &metrics_ca_key, + ); + let (gateway_client_cert, gateway_client_key) = write_test_client_certificate( + directory.path(), + "gateway-client", + &gateway_ca_cert, + &gateway_ca_key, + ); + let metrics_tls_acceptor = TlsAcceptor::from_files( + &directory.path().join("server-cert.pem"), + &directory.path().join("server-key.pem"), + Some(&metrics_ca_directory.join("ca.pem")), + true, + None, + None, + Vec::new(), + ) + .expect("failed to configure metrics mTLS acceptor"); + + let listener = TcpListener::bind("127.0.0.1:0") + .await + .expect("failed to bind metrics listener"); + let address = listener + .local_addr() + .expect("failed to read metrics listener address"); + let (shutdown_tx, shutdown_rx) = watch::channel(false); + let listener_task = tokio::spawn(serve_tls_metrics_listener( + listener, + Router::new().route("/metrics", get(|| async { "metrics" })), + metrics_tls_acceptor, + shutdown_rx, + )); + + let metrics_connector = test_tls_connector( + &directory.path().join("ca.pem"), + Some((&metrics_client_cert, &metrics_client_key)), + ); + let stream = TcpStream::connect(address) + .await + .expect("failed to connect with metrics client certificate"); + let mut metrics_client = metrics_connector + .connect( + rustls::pki_types::ServerName::try_from("localhost") + .expect("invalid test server name") + .to_owned(), + stream, + ) + .await + .expect("metrics client TLS handshake failed"); + metrics_client + .write_all(b"GET /metrics HTTP/1.1\r\nHost: localhost\r\nConnection: close\r\n\r\n") + .await + .expect("failed to request metrics with metrics client certificate"); + let mut authorized_response = Vec::new(); + metrics_client + .read_to_end(&mut authorized_response) + .await + .expect("failed to read authorized metrics response"); + assert!( + String::from_utf8_lossy(&authorized_response).starts_with("HTTP/1.1 200"), + "metrics client certificate should be authorized" + ); + + let gateway_connector = test_tls_connector( + &directory.path().join("ca.pem"), + Some((&gateway_client_cert, &gateway_client_key)), + ); + let stream = TcpStream::connect(address) + .await + .expect("failed to connect with gateway client certificate"); + let rejected_response = match gateway_connector + .connect( + rustls::pki_types::ServerName::try_from("localhost") + .expect("invalid test server name") + .to_owned(), + stream, + ) + .await + { + Ok(mut gateway_client) => { + let _ = gateway_client + .write_all( + b"GET /metrics HTTP/1.1\r\nHost: localhost\r\nConnection: close\r\n\r\n", + ) + .await; + let mut response = Vec::new(); + let _ = tokio::time::timeout( + Duration::from_secs(2), + gateway_client.read_to_end(&mut response), + ) + .await; + response + } + Err(_) => Vec::new(), + }; + assert!( + !String::from_utf8_lossy(&rejected_response).starts_with("HTTP/"), + "gateway client certificate must not authorize metrics scraping" + ); + + let anonymous_connector = test_tls_connector(&directory.path().join("ca.pem"), None); + let stream = TcpStream::connect(address) + .await + .expect("failed to connect without a client certificate"); + let anonymous_response = match anonymous_connector + .connect( + rustls::pki_types::ServerName::try_from("localhost") + .expect("invalid test server name") + .to_owned(), + stream, + ) + .await + { + Ok(mut anonymous_client) => { + let _ = anonymous_client + .write_all( + b"GET /metrics HTTP/1.1\r\nHost: localhost\r\nConnection: close\r\n\r\n", + ) + .await; + let mut response = Vec::new(); + let _ = tokio::time::timeout( + Duration::from_secs(2), + anonymous_client.read_to_end(&mut response), + ) + .await; + response + } + Err(_) => Vec::new(), + }; + assert!( + !String::from_utf8_lossy(&anonymous_response).starts_with("HTTP/"), + "metrics mTLS must reject clients without a certificate" + ); + + stop_listener(shutdown_tx, listener_task).await; + } + #[test] fn classifies_probe_style_tls_disconnects_as_benign() { for kind in [ErrorKind::UnexpectedEof, ErrorKind::ConnectionReset] { diff --git a/crates/openshell-server/src/tls.rs b/crates/openshell-server/src/tls.rs index 90372b9dad..92f8652cc5 100644 --- a/crates/openshell-server/src/tls.rs +++ b/crates/openshell-server/src/tls.rs @@ -98,6 +98,10 @@ impl TlsAcceptor { /// Returns `Ok(())` when the new config was built and swapped successfully. /// Returns `Err(...)` if cert/key loading fails — the old config is preserved. pub fn reload(&self) -> Result<()> { + self.reload_for_listener("gateway") + } + + fn reload_for_listener(&self, listener: &str) -> Result<()> { let new_config = build_server_config( &self.cert_path, &self.key_path, @@ -113,7 +117,9 @@ impl TlsAcceptor { .severity(SeverityId::Informational) .status(StatusId::Success) .state(StateId::Enabled, "reloaded") - .message("TLS certificate config reloaded successfully") + .message(format!( + "{listener} TLS certificate config reloaded successfully" + )) .build(); openshell_ocsf::ocsf_emit!(event); @@ -140,11 +146,26 @@ impl TlsAcceptor { /// gateway to perform a graceful shutdown without orphaned reload /// tasks. pub fn spawn_reload_worker( + &self, + shutdown: watch::Receiver, + ) -> tokio::task::JoinHandle<()> { + self.spawn_reload_worker_for_listener(shutdown, "gateway") + } + + /// Spawn a certificate reload worker identified by the listener it serves. + /// + /// This is used when a process owns more than one TLS listener, so reload + /// logs and OCSF events identify the certificate trust boundary affected. + pub fn spawn_reload_worker_for_listener( &self, mut shutdown: watch::Receiver, + listener: &'static str, ) -> tokio::task::JoinHandle<()> { if self.reload_spawned.swap(true, Ordering::Relaxed) { - warn!("TLS certificate reload worker already spawned, ignoring duplicate call"); + warn!( + listener, + "TLS certificate reload worker already spawned, ignoring duplicate call" + ); return tokio::spawn(async {}); } @@ -194,19 +215,19 @@ impl TlsAcceptor { ) { Ok(w) => w, Err(e) => { - warn!(error = %e, "Failed to start TLS cert file watcher, hot-reload disabled"); + warn!(listener, error = %e, "Failed to start TLS cert file watcher, hot-reload disabled"); return; } }; for dir in &dirs { if let Err(e) = watcher.watch(dir, RecursiveMode::NonRecursive) { - warn!(error = %e, dir = %dir.display(), "Failed to watch TLS cert directory, hot-reload disabled"); + warn!(listener, error = %e, dir = %dir.display(), "Failed to watch TLS cert directory, hot-reload disabled"); return; } } - info!(?dirs, "TLS certificate file watcher started"); + info!(listener, ?dirs, "TLS certificate file watcher started"); // Event loop with manual debounce. // When the watcher fires, we drain any follow-up events that @@ -223,7 +244,10 @@ impl TlsAcceptor { }; if !got_event { - warn!("TLS cert file watcher disconnected, hot-reload stopping"); + warn!( + listener, + "TLS cert file watcher disconnected, hot-reload stopping" + ); break 'outer; } @@ -232,17 +256,17 @@ impl TlsAcceptor { tokio::select! { () = tokio::time::sleep(debounce) => { // Debounce window elapsed — reload now. - if let Err(e) = this.reload() { + if let Err(e) = this.reload_for_listener(listener) { let event = ConfigStateChangeBuilder::new(&tls_ocsf_ctx()) .severity(SeverityId::Medium) .status(StatusId::Failure) .state(StateId::Enabled, "reload_failed") .message(format!( - "TLS certificate reload failed: {e}" + "{listener} TLS certificate reload failed: {e}" )) .build(); openshell_ocsf::ocsf_emit!(event); - warn!(error = %e, "TLS certificate reload failed, keeping existing config"); + warn!(listener, error = %e, "TLS certificate reload failed, keeping existing config"); } break; } @@ -251,11 +275,11 @@ impl TlsAcceptor { // Another event arrived — reset debounce. continue; } - warn!("TLS cert file watcher disconnected, hot-reload stopping"); + warn!(listener, "TLS cert file watcher disconnected, hot-reload stopping"); break 'outer; } _ = shutdown.changed() => { - debug!("TLS certificate reload worker stopped"); + debug!(listener, "TLS certificate reload worker stopped"); break 'outer; } } diff --git a/deploy/helm/openshell/README.md b/deploy/helm/openshell/README.md index 4b534bc0be..23f0562c5c 100644 --- a/deploy/helm/openshell/README.md +++ b/deploy/helm/openshell/README.md @@ -302,6 +302,41 @@ Disabling autoscaling renders `spec.replicas` from `replicaCount` again, which defaults to 1, so set `replicaCount` to the replica count you want in that upgrade. +## Secure metrics scraping + +The dedicated metrics endpoint is plaintext by default for compatibility with +existing Prometheus deployments. Enable `metrics.tls.enabled` to serve it over +HTTPS. The initial implementation uses operator-provided Kubernetes Secrets; +it does not generate metrics PKI or create a `ServiceMonitor`. + +```yaml +metrics: + tls: + enabled: true + # Empty uses server.tls.certSecretName instead. + certSecretName: openshell-metrics-server-tls + clientCaSecretName: openshell-metrics-scraper-ca + requireClientCert: true +``` + +The server Secret must provide `tls.crt` and `tls.key`; the scraper CA Secret +must provide `ca.crt`. When `requireClientCert` is true, the chart rejects a +render without `clientCaSecretName`. The metrics client CA must be separate +from `server.tls.clientCaSecretName`: sandbox workloads receive gateway client +credentials, and trusting that CA on the metrics listener would grant those +workloads scraping access. + +The metrics server certificate may reuse the gateway server TLS Secret when its +SANs cover the Service DNS name used by the scraper. The gateway reloads valid +certificate changes in place and preserves the last known-good configuration on +an invalid replacement. Logs and OCSF events identify these reloads with +`listener=metrics`. + +Configure your monitoring resource separately. A Prometheus Operator +`ServiceMonitor` or `PodMonitor` that uses mTLS must reference its client +certificate, key, and CA from Secrets in the monitoring resource's namespace. +Do not disable server verification with `insecureSkipVerify`. + ## Secret bootstrap By default, a pre-install/pre-upgrade hook Job runs `openshell-gateway generate-certs` @@ -376,6 +411,10 @@ discovery endpoint or its TLS CA. | grpcRoute.gateway.namespace | string | `""` | Namespace of the Gateway referenced by the GRPCRoute parentRef. Defaults to the release namespace. | | grpcRoute.hostnames | list | `[]` | Hostnames the GRPCRoute matches on. Leave empty to match all hosts. | | imagePullSecrets | list | `[]` | Image pull secrets attached to gateway and helper pods. | +| metrics.tls.certSecretName | string | `""` | Kubernetes TLS Secret with tls.crt and tls.key. Empty reuses server.tls.certSecretName. | +| metrics.tls.clientCaSecretName | string | `""` | Secret with ca.crt for verifying metrics scraper client certificates. It must not contain the gateway client CA. Required when requireClientCert is true. | +| metrics.tls.enabled | bool | `false` | Enable TLS for the dedicated metrics listener. | +| metrics.tls.requireClientCert | bool | `false` | Require a client certificate signed by clientCaSecretName to scrape metrics. | | nameOverride | string | `"openshell"` | Override the chart name used in generated resource names. | | networkPolicy.enabled | bool | `true` | Restrict SSH ingress on sandbox pods to the gateway. In managed mode, the driver applies the equivalent policy to each workspace namespace. | | nodeSelector | object | `{}` | Node selector for the gateway pod. | diff --git a/deploy/helm/openshell/README.md.gotmpl b/deploy/helm/openshell/README.md.gotmpl index 59d6fb06b6..5683ad5f56 100644 --- a/deploy/helm/openshell/README.md.gotmpl +++ b/deploy/helm/openshell/README.md.gotmpl @@ -303,6 +303,41 @@ Disabling autoscaling renders `spec.replicas` from `replicaCount` again, which defaults to 1, so set `replicaCount` to the replica count you want in that upgrade. +## Secure metrics scraping + +The dedicated metrics endpoint is plaintext by default for compatibility with +existing Prometheus deployments. Enable `metrics.tls.enabled` to serve it over +HTTPS. The initial implementation uses operator-provided Kubernetes Secrets; +it does not generate metrics PKI or create a `ServiceMonitor`. + +```yaml +metrics: + tls: + enabled: true + # Empty uses server.tls.certSecretName instead. + certSecretName: openshell-metrics-server-tls + clientCaSecretName: openshell-metrics-scraper-ca + requireClientCert: true +``` + +The server Secret must provide `tls.crt` and `tls.key`; the scraper CA Secret +must provide `ca.crt`. When `requireClientCert` is true, the chart rejects a +render without `clientCaSecretName`. The metrics client CA must be separate +from `server.tls.clientCaSecretName`: sandbox workloads receive gateway client +credentials, and trusting that CA on the metrics listener would grant those +workloads scraping access. + +The metrics server certificate may reuse the gateway server TLS Secret when its +SANs cover the Service DNS name used by the scraper. The gateway reloads valid +certificate changes in place and preserves the last known-good configuration on +an invalid replacement. Logs and OCSF events identify these reloads with +`listener=metrics`. + +Configure your monitoring resource separately. A Prometheus Operator +`ServiceMonitor` or `PodMonitor` that uses mTLS must reference its client +certificate, key, and CA from Secrets in the monitoring resource's namespace. +Do not disable server verification with `insecureSkipVerify`. + ## Secret bootstrap By default, a pre-install/pre-upgrade hook Job runs `openshell-gateway generate-certs` diff --git a/deploy/helm/openshell/templates/_gateway-workload.tpl b/deploy/helm/openshell/templates/_gateway-workload.tpl index 919d3a4313..0763f2eb98 100644 --- a/deploy/helm/openshell/templates/_gateway-workload.tpl +++ b/deploy/helm/openshell/templates/_gateway-workload.tpl @@ -164,6 +164,16 @@ spec: readOnly: true {{- end }} {{- end }} + {{- if .Values.metrics.tls.enabled }} + - name: metrics-tls-cert + mountPath: /etc/openshell-metrics/server + readOnly: true + {{- if .Values.metrics.tls.clientCaSecretName }} + - name: metrics-tls-client-ca + mountPath: /etc/openshell-metrics/client-ca + readOnly: true + {{- end }} + {{- end }} {{- if and .Values.server.oidc.issuer .Values.server.oidc.caConfigMapName }} - name: oidc-ca mountPath: /etc/openshell-tls/oidc-ca @@ -265,6 +275,19 @@ spec: {{- end }} {{- end }} {{- end }} + {{- if .Values.metrics.tls.enabled }} + - name: metrics-tls-cert + secret: + secretName: {{ .Values.metrics.tls.certSecretName | default .Values.server.tls.certSecretName }} + {{- if .Values.metrics.tls.clientCaSecretName }} + - name: metrics-tls-client-ca + secret: + secretName: {{ .Values.metrics.tls.clientCaSecretName }} + items: + - key: ca.crt + path: ca.crt + {{- end }} + {{- end }} {{- if and .Values.server.oidc.issuer .Values.server.oidc.caConfigMapName }} - name: oidc-ca configMap: diff --git a/deploy/helm/openshell/templates/gateway-config.yaml b/deploy/helm/openshell/templates/gateway-config.yaml index c6eea2b6a1..b1cfe4662d 100644 --- a/deploy/helm/openshell/templates/gateway-config.yaml +++ b/deploy/helm/openshell/templates/gateway-config.yaml @@ -15,6 +15,25 @@ One value is intentionally NOT rendered here: {{- $credentialDrivers := list -}} {{- $otlp := .Values.server.otlp | default dict -}} {{- $ocsfLog := .Values.server.ocsfLog | default dict -}} +{{- $metrics := .Values.metrics | default dict -}} +{{- $metricsTls := $metrics.tls | default dict -}} +{{- if and $metricsTls.enabled (not .Values.service.metricsPort) }} +{{- fail "metrics.tls.enabled requires service.metricsPort to be non-zero" }} +{{- end }} +{{- if and $metricsTls.enabled $metricsTls.requireClientCert (not $metricsTls.clientCaSecretName) }} +{{- fail "metrics.tls.requireClientCert requires metrics.tls.clientCaSecretName" }} +{{- end }} +{{- $gatewayClientCaSecretName := "" }} +{{- if eq (include "openshell.gatewayClientCaEnabled" .) "true" }} +{{- if or (and .Values.pkiInitJob.enabled (not .Values.certManager.enabled)) (and .Values.certManager.enabled .Values.certManager.clientCaFromServerTlsSecret) }} +{{- $gatewayClientCaSecretName = .Values.server.tls.certSecretName }} +{{- else }} +{{- $gatewayClientCaSecretName = .Values.server.tls.clientCaSecretName }} +{{- end }} +{{- end }} +{{- if and $metricsTls.enabled $metricsTls.clientCaSecretName (eq $metricsTls.clientCaSecretName $gatewayClientCaSecretName) }} +{{- fail "metrics.tls.clientCaSecretName must not reuse the gateway client CA Secret" }} +{{- end }} {{- if .Values.server.credentialDrivers.kubernetesSecrets.enabled -}} {{- $credentialDrivers = append $credentialDrivers "kubernetes-secrets" -}} {{- end -}} @@ -139,6 +158,17 @@ data: {{- end }} {{- end }} + {{- if $metricsTls.enabled }} + + [openshell.gateway.metrics_tls] + cert_path = "/etc/openshell-metrics/server/tls.crt" + key_path = "/etc/openshell-metrics/server/tls.key" + {{- if $metricsTls.clientCaSecretName }} + client_ca_path = "/etc/openshell-metrics/client-ca/ca.crt" + {{- end }} + require_client_auth = {{ $metricsTls.requireClientCert }} + {{- end }} + {{- if .Values.server.auth.allowUnauthenticatedUsers }} [openshell.gateway.auth] diff --git a/deploy/helm/openshell/tests/gateway_config_test.yaml b/deploy/helm/openshell/tests/gateway_config_test.yaml index 16ccfaee6b..b37d34d759 100644 --- a/deploy/helm/openshell/tests/gateway_config_test.yaml +++ b/deploy/helm/openshell/tests/gateway_config_test.yaml @@ -29,6 +29,113 @@ tests: path: data["gateway.toml"] pattern: '(?m)^enable_websocket_tunnel\s*=\s*true$' + - it: leaves metrics TLS disabled by default + template: templates/gateway-config.yaml + asserts: + - notMatchRegex: + path: data["gateway.toml"] + pattern: '\[openshell\.gateway\.metrics_tls\]' + + - it: renders metrics mTLS configuration from operator-provided Secret values + template: templates/gateway-config.yaml + set: + metrics.tls.enabled: true + metrics.tls.certSecretName: metrics-server-tls + metrics.tls.clientCaSecretName: metrics-scraper-ca + metrics.tls.requireClientCert: true + asserts: + - matchRegex: + path: data["gateway.toml"] + pattern: '(?ms)\[openshell\.gateway\.metrics_tls\].*?cert_path\s*=\s*"/etc/openshell-metrics/server/tls\.crt".*?key_path\s*=\s*"/etc/openshell-metrics/server/tls\.key".*?client_ca_path\s*=\s*"/etc/openshell-metrics/client-ca/ca\.crt".*?require_client_auth\s*=\s*true' + + - it: mounts operator-provided metrics mTLS Secrets read-only + template: templates/statefulset.yaml + set: + metrics.tls.enabled: true + metrics.tls.certSecretName: metrics-server-tls + metrics.tls.clientCaSecretName: metrics-scraper-ca + metrics.tls.requireClientCert: true + asserts: + - contains: + path: spec.template.spec.containers[0].volumeMounts + content: + name: metrics-tls-cert + mountPath: /etc/openshell-metrics/server + readOnly: true + - contains: + path: spec.template.spec.containers[0].volumeMounts + content: + name: metrics-tls-client-ca + mountPath: /etc/openshell-metrics/client-ca + readOnly: true + - contains: + path: spec.template.spec.volumes + content: + name: metrics-tls-cert + secret: + secretName: metrics-server-tls + - contains: + path: spec.template.spec.volumes + content: + name: metrics-tls-client-ca + secret: + secretName: metrics-scraper-ca + items: + - key: ca.crt + path: ca.crt + + - it: reuses the gateway server Secret for metrics TLS when no separate Secret is set + template: templates/statefulset.yaml + set: + metrics.tls.enabled: true + asserts: + - contains: + path: spec.template.spec.volumes + content: + name: metrics-tls-cert + secret: + secretName: openshell-server-tls + + - it: rejects metrics mTLS without a dedicated client CA + template: templates/statefulset.yaml + set: + metrics.tls.enabled: true + metrics.tls.requireClientCert: true + asserts: + - failedTemplate: + errorMessage: 'metrics.tls.requireClientCert requires metrics.tls.clientCaSecretName' + + - it: rejects a metrics client CA that reuses the gateway client CA Secret + template: templates/statefulset.yaml + set: + metrics.tls.enabled: true + metrics.tls.clientCaSecretName: openshell-server-tls + metrics.tls.requireClientCert: true + asserts: + - failedTemplate: + errorMessage: 'metrics.tls.clientCaSecretName must not reuse the gateway client CA Secret' + + - it: rejects a metrics client CA that reuses a custom gateway client CA Secret + template: templates/statefulset.yaml + set: + pkiInitJob.enabled: false + server.tls.clientCaSecretName: gateway-client-ca + metrics.tls.enabled: true + metrics.tls.clientCaSecretName: gateway-client-ca + metrics.tls.requireClientCert: true + asserts: + - failedTemplate: + errorMessage: 'metrics.tls.clientCaSecretName must not reuse the gateway client CA Secret' + + - it: rejects metrics TLS when the metrics port is disabled + template: templates/statefulset.yaml + set: + service.metricsPort: 0 + metrics.tls.enabled: true + asserts: + - failedTemplate: + errorMessage: 'metrics.tls.enabled requires service.metricsPort to be non-zero' + - it: defaults to label admission with caller driver config disabled template: templates/gateway-config.yaml asserts: diff --git a/deploy/helm/openshell/values.yaml b/deploy/helm/openshell/values.yaml index d0612cd563..466319f04f 100644 --- a/deploy/helm/openshell/values.yaml +++ b/deploy/helm/openshell/values.yaml @@ -200,6 +200,21 @@ service: # -- Gateway metrics service port. metricsPort: 9090 +# Optional TLS and mTLS configuration for the dedicated metrics listener. +# Plaintext metrics remain the default for compatibility with existing +# Prometheus deployments. Secrets are supplied by the operator; the chart does +# not generate metrics-specific PKI in this initial integration. +metrics: + tls: + # -- Enable TLS for the dedicated metrics listener. + enabled: false + # -- Kubernetes TLS Secret with tls.crt and tls.key. Empty reuses server.tls.certSecretName. + certSecretName: "" + # -- Secret with ca.crt for verifying metrics scraper client certificates. It must not contain the gateway client CA. Required when requireClientCert is true. + clientCaSecretName: "" + # -- Require a client certificate signed by clientCaSecretName to scrape metrics. + requireClientCert: false + # Agent Sandbox is a cluster-scoped prerequisite for the Kubernetes compute # driver. OpenShell deliberately does not install its CRDs or controller. # Enable this check for live Helm installs to fail before creating gateway diff --git a/deploy/man/openshell-gateway.8.md b/deploy/man/openshell-gateway.8.md index 0d217c8a46..28947d470c 100644 --- a/deploy/man/openshell-gateway.8.md +++ b/deploy/man/openshell-gateway.8.md @@ -51,6 +51,27 @@ TLS. : Port for Prometheus metrics (/metrics). Set to 0 to disable. Default: **0**. Environment: **OPENSHELL_METRICS_PORT**. +**--metrics-tls-cert** *PATH* +: Path to the TLS certificate served by the metrics listener. Pair with + **--metrics-tls-key** and enable a metrics port. + Environment: **OPENSHELL_METRICS_TLS_CERT**. + +**--metrics-tls-key** *PATH* +: Path to the TLS private key served by the metrics listener. Pair with + **--metrics-tls-cert** and enable a metrics port. + Environment: **OPENSHELL_METRICS_TLS_KEY**. + +**--metrics-tls-client-ca** *PATH* +: Path to the CA certificate used to verify metrics scraper client + certificates. Use a CA distinct from the gateway client CA. + Environment: **OPENSHELL_METRICS_TLS_CLIENT_CA**. + +**--metrics-require-client-auth** *BOOL* +: Require metrics clients to present a certificate trusted by + **--metrics-tls-client-ca**. Defaults to **false**. When enabled, a + metrics client CA is required. + Environment: **OPENSHELL_METRICS_REQUIRE_CLIENT_AUTH**. + **--log-level** *LEVEL* : Log level: trace, debug, info, warn, error. Default: **info**. Environment: **OPENSHELL_LOG_LEVEL**. diff --git a/docs/how-it-works/gateways/configuration.mdx b/docs/how-it-works/gateways/configuration.mdx index a2b72381a1..28ae3f7b56 100644 --- a/docs/how-it-works/gateways/configuration.mdx +++ b/docs/how-it-works/gateways/configuration.mdx @@ -84,6 +84,9 @@ version = 2 [openshell.gateway.tls] # ... gateway listener TLS ... +[openshell.gateway.metrics_tls] +# ... dedicated metrics listener TLS ... + [openshell.gateway.oidc] # ... JWT bearer auth ... @@ -229,6 +232,14 @@ client_ca_path = "/etc/openshell/certs/client-ca.pem" # external_key_path = "/etc/openshell/certs/external-key.pem" # external_server_names = ["gateway.example.com"] +# Dedicated metrics-listener TLS. Omit this table to retain the plaintext +# metrics listener. Use a CA that is distinct from the gateway client CA. +[openshell.gateway.metrics_tls] +cert_path = "/etc/openshell/metrics/server/tls.crt" +key_path = "/etc/openshell/metrics/server/tls.key" +client_ca_path = "/etc/openshell/metrics/client-ca/ca.crt" +require_client_auth = true + [openshell.gateway.gateway_jwt] signing_key_path = "/etc/openshell/jwt/signing.pem" public_key_path = "/etc/openshell/jwt/public.pem" @@ -302,6 +313,49 @@ The client-certificate handshake policy has no `require_client_auth` TOML field. `[openshell.gateway.tls]` supports optional SNI-based dual-certificate mode for deployments that need separate internal and external server certificates. Set `external_cert_path` and `external_key_path` to point at the external (e.g. ACME/publicly-trusted) certificate and key. List the hostnames that should be served with the external certificate in `external_server_names`. Connections whose TLS SNI hostname matches one of those names receive the external certificate; all other connections (including those with no SNI) receive the primary internal certificate from `cert_path`/`key_path`. Both fields must be set together — providing only one is a configuration error. On Kubernetes with the Helm chart, the external certificate is managed automatically when `certManager.serverIssuerRef.name` is set; the chart populates these fields from the cert-manager-issued external server certificate. +## Metrics Listener TLS + +The dedicated metrics listener is configured with `metrics_bind_address`. It +remains plaintext unless `[openshell.gateway.metrics_tls]` is configured, which +preserves compatibility with existing Prometheus deployments. TLS for this +listener is independent from `[openshell.gateway.tls]`: it has its own server +certificate, optional client CA, and reload worker. + +```toml +[openshell.gateway] +metrics_bind_address = "0.0.0.0:9090" + +[openshell.gateway.metrics_tls] +cert_path = "/etc/openshell/metrics/server/tls.crt" +key_path = "/etc/openshell/metrics/server/tls.key" +client_ca_path = "/etc/openshell/metrics/client-ca/ca.crt" +require_client_auth = true +``` + +`cert_path` and `key_path` are required whenever the table is present. Omit +`client_ca_path` for HTTPS with anonymous scraping. Set `client_ca_path` and +`require_client_auth = true` for mTLS; a client without a certificate, or with +a certificate from another CA, cannot scrape `/metrics`. When +`require_client_auth = false`, anonymous clients remain allowed, but any client +certificate they present must validate against `client_ca_path`. + +The metrics client CA must be distinct from the gateway listener client CA. In +Kubernetes, sandbox workloads receive gateway client credentials for their +control-plane connection. Reusing that CA for metrics would let those workload +identities access the metrics endpoint. The metrics server certificate may be +the same certificate used by the gateway listener when its SANs cover the +metrics Service DNS name. + +The listener watches its certificate, key, and optional client CA files. A +valid change is applied without restarting the gateway; an invalid replacement +keeps the last known-good configuration active. Reload logs and OCSF events +include `listener=metrics` to distinguish this boundary from gateway TLS. + +For a Helm installation, set `metrics.tls.enabled=true` and provide the +operator-managed Secret names through `metrics.tls.certSecretName` and, for +mTLS, `metrics.tls.clientCaSecretName`. The chart does not create +metrics-specific PKI or monitoring resources. + `[openshell.gateway] policy_validation_failure_mode` controls what sandbox supervisors do when a complete candidate policy fails runtime validation. The default, `fail_closed`, deactivates the previous network policy, closes relays pinned to it, and denies new egress until a valid generation loads. `retain_last_valid` leaves the previous valid generation active. Both modes reject the candidate atomically; startup keeps the workload unstarted until the effective policy and matching provider configuration pass admission. A rejected startup exposes `ConfigurationInvalid` and remains available for policy/provider repair in either mode. Gateway mutation paths that can preflight a known effective scope reject invalid candidates before persistence and leave the active policy unchanged regardless of this setting. Changing the value requires restarting the gateway so it can reload `gateway.toml` and distribute the new posture to sandbox supervisors. `[openshell.gateway.gateway_jwt] ttl_secs` controls generation-bound gateway-facing and Sandbox Protocol credentials minted for a sandbox session, plus typed extension JWTs. Omit it for non-expiring local sandbox session credentials: both session tokens carry `exp = 0`, supervisors skip periodic session-token renewal, and refresh responses omit their expiration timestamps. Typed extension JWTs retain a 900-second default when the field is omitted. Use omission only for local single-player Docker, Podman, or VM gateways. Explicit `0` is invalid. Kubernetes and other shared deployments should set a positive TTL; Helm renders `3600` seconds by default, and the gateway logs a warning when a Kubernetes gateway omits the field. diff --git a/skills/debug-openshell-cluster/SKILL.md b/skills/debug-openshell-cluster/SKILL.md index 4d98d1d61a..857c98ffeb 100644 --- a/skills/debug-openshell-cluster/SKILL.md +++ b/skills/debug-openshell-cluster/SKILL.md @@ -626,6 +626,31 @@ kubectl -n openshell get statefulset openshell -o jsonpath='{.spec.template.spec # Should show items filter for ca.crt from openshell-server-tls ``` +#### Secure metrics TLS + +When `metrics.tls.enabled=true`, the dedicated metrics listener uses its own +TLS configuration and, when requested, an independent scraper client CA. Do +not use the gateway client CA: sandbox workloads receive gateway client +credentials, so trusting that CA here would grant them metrics access. Inspect +the rendered configuration and the read-only Secret mounts before debugging the +scraper: + +```bash +kubectl -n openshell get configmap openshell-config \ + -o jsonpath='{.data.gateway\.toml}' | grep -A6 '^\[openshell\.gateway\.metrics_tls\]' +kubectl -n openshell get statefulset openshell \ + -o jsonpath='{.spec.template.spec.volumes[?(@.name=="metrics-tls-cert")]}{"\n"}{.spec.template.spec.volumes[?(@.name=="metrics-tls-client-ca")]}' | jq . +kubectl -n openshell logs statefulset/openshell -c openshell-gateway --tail=200 | grep -E 'metrics.*TLS|listener=metrics' +``` + +`metrics-tls-cert` must reference a Secret with `tls.crt` and `tls.key`; when +mTLS is enabled, `metrics-tls-client-ca` must reference a Secret with `ca.crt`. +An absent mount means the release was rendered without metrics TLS. A failed +certificate reload retains the previous working listener; correct the Secret +contents rather than disabling verification. Configure the scraper with the +metrics server CA and, for mTLS, its dedicated client certificate and key. Do +not use `insecureSkipVerify`. + If `server.providerTokenGrants.spiffe.enabled=true`, the gateway should still render `[openshell.gateway.gateway_jwt]` and mount the `sandbox-jwt` Secret. SPIRE is used by both the gateway and sandbox supervisors for dynamic provider