mirror of
https://github.com/NVIDIA/OpenShell.git
synced 2026-10-22 06:19:47 +08:00
* fix(core): trust public root CAs alongside the sandbox mTLS CA The supervisor gRPC client only trusted the CA configured via OPENSHELL_TLS_CA, since tonic ClientTlsConfig starts with an empty root store unless with_native_roots()/with_webpki_roots() is also enabled. Deployments where the gateway server certificate is issued by a public CA (e.g. cert-manager against an ACME issuer) caused every supervisor connection to fail the TLS handshake with "UnknownCA", since the sandbox mTLS CA and the server cert issuer were no longer the same. Enable both native and webpki roots in addition to the configured CA. tonic root store is a union of all configured sources, so this does not weaken verification for existing self-signed deployments. webpki-roots (compiled in) is enabled alongside native-roots since the supervisor binary may run in minimal sandbox images without a populated system CA bundle. Signed-off-by: Jesse Jaggars <jjaggars@redhat.com> * feat(helm): support external cert-manager issuers and OpenShift Route passthrough Add certManager.serverIssuerRef/clientIssuerRef so the gateway and mTLS client certificates can be issued by a real Issuer/ClusterIssuer (e.g. ACME) instead of only the chart built-in self-signed CA. Add openshiftRoute template for exposing the gateway via a TLS passthrough Route so the gateway keeps terminating its own TLS/mTLS. The server Certificate excludes internal-only SANs (cluster-local, localhost, loopback) when an external issuer is configured, since ACME issuers reject those per CA/Browser Forum baseline requirements. A template-time fail guard catches the misconfiguration at helm install time rather than asynchronously at cert-manager issuance time. Includes Helm unittest coverage for both issuerRef overrides and Route rendering, plus a CI values overlay for lint coverage. Signed-off-by: Jesse Jaggars <jjaggars@redhat.com> * docs: document cert-manager external issuer and OpenShift Route Update managing-certificates.mdx with the serverIssuerRef workflow and install-time validation behavior. Add a production section to the OpenShift guide covering passthrough Route with a real certificate. Regenerate Helm README for new certManager and openshiftRoute values. Sync debug-openshell-cluster skill with new troubleshooting steps for ACME issuance failures and supervisor UnknownCA from mismatched CAs. Signed-off-by: Jesse Jaggars <jjaggars@redhat.com> * fix(helm,core): address PR review feedback on cert-manager external issuer Addresses all five blocking review items from #2468: 1. Remove .with_native_roots() from supervisor gRPC client -- the supervisor runs inside the user-selected sandbox image, so the image CA bundle is not operator-controlled. Keep .with_webpki_roots() (compiled-in, not user-controlled) alongside the configured CA. 2. Fail at render time when serverIssuerRef.name is set but clientCaFromServerTlsSecret is still true. Add negative Helm test. 3. Remove clientIssuerRef -- changing only clientIssuerRef breaks both directions because trust bundles are not modeled separately. Change serverIssuerRef.kind default from ClusterIssuer to Issuer. 4. Add server.oidc.issuer and server.oidc.audience to the documented OpenShift production Helm command. Add Access Control prerequisite. 5. Fail at render time when openshiftRoute.enabled and disableTls are both true. Add negative Helm test. Signed-off-by: Jesse Jaggars <jjaggars@redhat.com> * fix(drivers): strip GATEWAY_TLS_SERVER_NAME from Docker and Podman env Signed-off-by: Jesse Jaggars <jjaggars@redhat.com> * fix(helm,drivers): guard default clientCaSecretName and add env-strip tests Signed-off-by: Jesse Jaggars <jjaggars@redhat.com> * feat(tls): SNI-based dual certificate for internal and external server TLS Split the gateway server certificate into two: an internal cert issued by the chart's own CA (for supervisor connections via cluster-local SANs) and an external cert issued by an operator-configured Issuer such as ACME/Let's Encrypt (for CLI and Route access via public SANs). The gateway uses SNI-based certificate selection: connections whose SNI hostname matches external_server_names receive the external cert; all others (including those with no SNI) receive the internal cert. Security improvement: remove .with_webpki_roots() from the supervisor gRPC client so supervisors trust only the chart CA, closing a MITM vector via publicly-trusted certificates in user-supplied container images. Key changes: - Add DualCertResolver with SNI-based cert selection and full test coverage - Add external_cert_path, external_key_path, external_server_names to TlsConfig - Validate partial external cert config (error on cert-without-key or vice versa) - Validate empty external_server_names when external cert is configured - Split cert-manager templates into internal + external Certificate resources - Add Helm guards for misconfigured external issuer (empty serverDnsNames, internal-only SANs with external issuer, conflicting clientCaFromServerTlsSecret) - Update gateway-config.mdx, managing-certificates.mdx, openshift.mdx docs - Update debug-openshell-cluster skill for dual-cert troubleshooting Signed-off-by: Pi Agent <agent@openshell.local> * fix(drivers): strip GATEWAY_TLS_SERVER_NAME in VM driver and correct comments Add the same GATEWAY_TLS_SERVER_NAME environment stripping to the VM compute driver that Docker, Podman, and Kubernetes drivers already perform. Without this, a sandbox user on the VM driver could override the TLS server name the supervisor verifies. Fix stale comments in Docker and Podman drivers that referenced 'with WebPKI roots trusted' — WebPKI roots are explicitly not trusted after the tls-webpki-roots removal. Use tls-ring instead of bare channel for tonic in openshell-core so the TLS API (ClientTlsConfig, Endpoint::tls_config) is available without pulling in any root certificate store. Signed-off-by: Pi Agent <agent@openshell.local> * fix(tls,helm): wildcard SNI matching and Route host validation Add RFC 6125 single-level wildcard matching to DualCertResolver so external_server_names entries like *.example.com correctly match SNI hostnames like gw.example.com. Previously only exact matches worked, silently falling back to the internal cert for wildcard configurations. Add a Helm fail guard in route.yaml that rejects openshiftRoute.host values not listed in certManager.serverDnsNames when an external issuer is configured — catches cert/route hostname mismatches at install time instead of at TLS connect time. Quote the host field in route.yaml for robustness. Signed-off-by: Pi Agent <agent@openshell.local> * fix(helm): address blocking review items — client-CA guard, wildcard Route, serverIssuerRef gate 1. Remove the obsolete guard rejecting serverIssuerRef + clientCaFromServerTlsSecret=true. The internal server certificate is always signed by the chart CA (the same CA that signs the client cert), so clientCaFromServerTlsSecret=true is correct — its filtered ca.crt is exactly the right trust anchor. The old workaround (mounting openshell-ca-tls directly) unnecessarily exposed the CA private key to the gateway container. Remove the client-CA overrides from docs, CI overlay, and production examples. 2. Route host validation now supports wildcard certificates per RFC 6125: single-level wildcards like *.example.com match gateway.example.com but not deep.sub.example.com. Require an explicit openshiftRoute.host when an external issuer is configured — without one, OpenShift generates a hostname absent from serverDnsNames. 3. Reject serverIssuerRef.name when certManager.enabled is false — the external certificate, its Secret mount, and the gateway TLS config all require cert-manager to be enabled. Validated on ROSA (dev.dyee.p3) with branch-built images: - Fresh install with letsencrypt-prod ClusterIssuer - SNI dual-cert: external hostname served Let's Encrypt cert - Supervisor mTLS via internal cert path: ConnectSupervisor accepted - Client CA volume: filtered ca.crt from internal server secret (no key) - CLI connected via Route + OIDC Helm tests: 81 pass across 7 suites. Signed-off-by: Jesse Jaggars <jjaggars@redhat.com> --------- Signed-off-by: Jesse Jaggars <jjaggars@redhat.com> Signed-off-by: Pi Agent <agent@openshell.local>
253 lines
7.8 KiB
Rust
253 lines
7.8 KiB
Rust
// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
|
|
// SPDX-License-Identifier: Apache-2.0
|
|
|
|
mod common;
|
|
|
|
use bytes::Bytes;
|
|
use common::{
|
|
PkiBundle, build_tls_root, generate_pki, generate_rogue_pki, grpc_client_mtls,
|
|
install_rustls_provider, start_test_server,
|
|
};
|
|
use http_body_util::Empty;
|
|
use hyper::Request;
|
|
use hyper::StatusCode;
|
|
use hyper_rustls::HttpsConnectorBuilder;
|
|
use hyper_util::{client::legacy::Client, rt::TokioExecutor};
|
|
use openshell_core::proto::{HealthRequest, ServiceStatus, open_shell_client::OpenShellClient};
|
|
use rustls::pki_types::CertificateDer;
|
|
use rustls_pemfile::certs;
|
|
use tonic::transport::{ClientTlsConfig, Endpoint};
|
|
|
|
/// Build an HTTPS client with mTLS (CA trust + client cert/key).
|
|
fn https_client_mtls(
|
|
pki: &PkiBundle,
|
|
) -> Client<
|
|
hyper_rustls::HttpsConnector<hyper_util::client::legacy::connect::HttpConnector>,
|
|
Empty<Bytes>,
|
|
> {
|
|
let roots = build_tls_root(&pki.ca_cert_pem);
|
|
|
|
let client_certs = {
|
|
let mut cursor = std::io::Cursor::new(&pki.client_cert_pem);
|
|
certs(&mut cursor)
|
|
.collect::<Result<Vec<CertificateDer<'static>>, _>>()
|
|
.expect("failed to parse client cert pem")
|
|
};
|
|
let client_key = {
|
|
let mut cursor = std::io::Cursor::new(&pki.client_key_pem);
|
|
rustls_pemfile::private_key(&mut cursor)
|
|
.expect("failed to parse client key pem")
|
|
.expect("no private key found")
|
|
};
|
|
|
|
let tls_config = rustls::ClientConfig::builder()
|
|
.with_root_certificates(roots)
|
|
.with_client_auth_cert(client_certs, client_key)
|
|
.expect("failed to build client TLS config with client cert");
|
|
let https = HttpsConnectorBuilder::new()
|
|
.with_tls_config(tls_config)
|
|
.https_only()
|
|
.enable_http1()
|
|
.build();
|
|
Client::builder(TokioExecutor::new()).build(https)
|
|
}
|
|
|
|
#[tokio::test]
|
|
async fn serves_grpc_and_http_over_tls_on_same_port() {
|
|
install_rustls_provider();
|
|
let (temp, pki) = generate_pki();
|
|
|
|
let tls_acceptor = openshell_server::TlsAcceptor::from_files(
|
|
&temp.path().join("server-cert.pem"),
|
|
&temp.path().join("server-key.pem"),
|
|
Some(temp.path().join("ca.pem").as_path()),
|
|
false,
|
|
None,
|
|
None,
|
|
Vec::new(),
|
|
)
|
|
.unwrap();
|
|
|
|
let (addr, server) = start_test_server(tls_acceptor).await;
|
|
|
|
// gRPC with mTLS
|
|
let mut grpc = grpc_client_mtls(
|
|
addr,
|
|
pki.ca_cert_pem.clone(),
|
|
pki.client_cert_pem.clone(),
|
|
pki.client_key_pem.clone(),
|
|
)
|
|
.await;
|
|
let response = grpc.health(HealthRequest {}).await.unwrap();
|
|
assert_eq!(response.get_ref().status, ServiceStatus::Healthy as i32);
|
|
|
|
// HTTP with mTLS
|
|
let client = https_client_mtls(&pki);
|
|
let req = Request::builder()
|
|
.method("GET")
|
|
.uri(format!("https://localhost:{}/healthz", addr.port()))
|
|
.body(Empty::<Bytes>::new())
|
|
.unwrap();
|
|
let resp = client.request(req).await.unwrap();
|
|
assert_eq!(resp.status(), StatusCode::OK);
|
|
|
|
server.abort();
|
|
}
|
|
|
|
#[tokio::test]
|
|
async fn mtls_valid_client_cert_accepted() {
|
|
install_rustls_provider();
|
|
let (temp, pki) = generate_pki();
|
|
|
|
let tls_acceptor = openshell_server::TlsAcceptor::from_files(
|
|
&temp.path().join("server-cert.pem"),
|
|
&temp.path().join("server-key.pem"),
|
|
Some(temp.path().join("ca.pem").as_path()),
|
|
false,
|
|
None,
|
|
None,
|
|
Vec::new(),
|
|
)
|
|
.unwrap();
|
|
|
|
let (addr, server) = start_test_server(tls_acceptor).await;
|
|
|
|
let mut grpc = grpc_client_mtls(
|
|
addr,
|
|
pki.ca_cert_pem.clone(),
|
|
pki.client_cert_pem.clone(),
|
|
pki.client_key_pem.clone(),
|
|
)
|
|
.await;
|
|
let response = grpc.health(HealthRequest {}).await.unwrap();
|
|
assert_eq!(response.get_ref().status, ServiceStatus::Healthy as i32);
|
|
|
|
server.abort();
|
|
}
|
|
|
|
#[tokio::test]
|
|
async fn no_client_cert_accepted_with_ca() {
|
|
install_rustls_provider();
|
|
let (temp, pki) = generate_pki();
|
|
|
|
let tls_acceptor = openshell_server::TlsAcceptor::from_files(
|
|
&temp.path().join("server-cert.pem"),
|
|
&temp.path().join("server-key.pem"),
|
|
Some(temp.path().join("ca.pem").as_path()),
|
|
false,
|
|
None,
|
|
None,
|
|
Vec::new(),
|
|
)
|
|
.unwrap();
|
|
|
|
let (addr, server) = start_test_server(tls_acceptor).await;
|
|
|
|
// Connect with CA trust but no client cert — should succeed (certs are optional).
|
|
let ca_cert = tonic::transport::Certificate::from_pem(pki.ca_cert_pem.clone());
|
|
let tls = ClientTlsConfig::new()
|
|
.ca_certificate(ca_cert)
|
|
.domain_name("localhost");
|
|
let endpoint = Endpoint::from_shared(format!("https://localhost:{}", addr.port()))
|
|
.expect("invalid endpoint")
|
|
.tls_config(tls)
|
|
.expect("failed to set tls");
|
|
|
|
let channel = endpoint
|
|
.connect()
|
|
.await
|
|
.expect("should connect without client cert");
|
|
let mut client = OpenShellClient::new(channel);
|
|
let response = client.health(HealthRequest {}).await.unwrap();
|
|
assert_eq!(response.get_ref().status, ServiceStatus::Healthy as i32);
|
|
|
|
server.abort();
|
|
}
|
|
|
|
#[tokio::test]
|
|
async fn no_client_cert_rejected_when_required() {
|
|
install_rustls_provider();
|
|
let (temp, pki) = generate_pki();
|
|
|
|
let tls_acceptor = openshell_server::TlsAcceptor::from_files(
|
|
&temp.path().join("server-cert.pem"),
|
|
&temp.path().join("server-key.pem"),
|
|
Some(temp.path().join("ca.pem").as_path()),
|
|
true,
|
|
None,
|
|
None,
|
|
Vec::new(),
|
|
)
|
|
.unwrap();
|
|
|
|
let (addr, server) = start_test_server(tls_acceptor).await;
|
|
|
|
let ca_cert = tonic::transport::Certificate::from_pem(pki.ca_cert_pem.clone());
|
|
let tls = ClientTlsConfig::new()
|
|
.ca_certificate(ca_cert)
|
|
.domain_name("localhost");
|
|
let endpoint = Endpoint::from_shared(format!("https://localhost:{}", addr.port()))
|
|
.expect("invalid endpoint")
|
|
.tls_config(tls)
|
|
.expect("failed to set tls");
|
|
|
|
let result = endpoint.connect().await;
|
|
if let Ok(channel) = result {
|
|
let mut client = OpenShellClient::new(channel);
|
|
let rpc_result = client.health(HealthRequest {}).await;
|
|
assert!(
|
|
rpc_result.is_err(),
|
|
"expected RPC to fail without client cert when mTLS is required"
|
|
);
|
|
}
|
|
|
|
server.abort();
|
|
}
|
|
|
|
#[tokio::test]
|
|
async fn mtls_wrong_ca_client_cert_rejected() {
|
|
install_rustls_provider();
|
|
let (temp, pki) = generate_pki();
|
|
|
|
let tls_acceptor = openshell_server::TlsAcceptor::from_files(
|
|
&temp.path().join("server-cert.pem"),
|
|
&temp.path().join("server-key.pem"),
|
|
Some(temp.path().join("ca.pem").as_path()),
|
|
false,
|
|
None,
|
|
None,
|
|
Vec::new(),
|
|
)
|
|
.unwrap();
|
|
|
|
let (addr, server) = start_test_server(tls_acceptor).await;
|
|
|
|
// Generate a rogue CA + client cert not signed by the server's CA.
|
|
let rogue = generate_rogue_pki();
|
|
|
|
// Connect with rogue client cert -- server should reject it.
|
|
let ca_cert = tonic::transport::Certificate::from_pem(pki.ca_cert_pem.clone());
|
|
let identity =
|
|
tonic::transport::Identity::from_pem(rogue.client_cert_pem, rogue.client_key_pem);
|
|
let tls = ClientTlsConfig::new()
|
|
.ca_certificate(ca_cert)
|
|
.identity(identity)
|
|
.domain_name("localhost");
|
|
let endpoint = Endpoint::from_shared(format!("https://localhost:{}", addr.port()))
|
|
.expect("invalid endpoint")
|
|
.tls_config(tls)
|
|
.expect("failed to set tls");
|
|
|
|
let result = endpoint.connect().await;
|
|
if let Ok(channel) = result {
|
|
let mut client = OpenShellClient::new(channel);
|
|
let rpc_result = client.health(HealthRequest {}).await;
|
|
assert!(
|
|
rpc_result.is_err(),
|
|
"expected RPC to fail with rogue client cert"
|
|
);
|
|
}
|
|
|
|
server.abort();
|
|
}
|