Skip to content

Commit c4b500a

Browse files
authored
feat(helm): cert-manager external issuer + OpenShift passthrough Route (#2468)
* 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>
1 parent c549823 commit c4b500a

28 files changed

Lines changed: 1269 additions & 22 deletions

File tree

.agents/skills/debug-openshell-cluster/SKILL.md

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -284,6 +284,52 @@ If the gateway exits with `failed to read sandbox JWT signing key from
284284
`sandbox-jwt` secret at `/etc/openshell-jwt`. The sandbox JWT mount is required
285285
even when local Helm values disable TLS.
286286

287+
If `certManager.serverIssuerRef` points the server certificate at an external
288+
Issuer or ClusterIssuer (for example an ACME issuer, for a publicly-trusted
289+
cert on an OpenShift `Route` with TLS passthrough — see
290+
`openshiftRoute.enabled`), the chart creates **two** server certificates: an
291+
internal one (chart CA, internal SANs) and an external one (from the configured
292+
issuer, external SANs only). The gateway uses SNI to present the right cert.
293+
294+
Check the external `Certificate`/`CertificateRequest`/`Challenge` resources
295+
directly when the external secret never becomes Ready:
296+
297+
```bash
298+
kubectl -n openshell get certificate,certificaterequest,challenge
299+
kubectl -n openshell describe certificate openshell-server-external
300+
oc -n openshell get route
301+
```
302+
303+
ACME issuers reject certificate requests that include internal-only names
304+
(`*.svc.cluster.local`, `localhost`, loopback IPs) and require the
305+
`commonName` to also be a SAN — the external `Certificate` only requests the
306+
hostnames in `certManager.serverDnsNames`, for exactly this reason.
307+
308+
If sandbox supervisors fail their TLS handshake to the gateway with
309+
`UnknownCA` after configuring `serverIssuerRef`, the most likely cause is
310+
`server.grpcEndpoint` set to the external hostname. This forces supervisors
311+
to connect via the external hostname, receiving the ACME cert (via SNI) which
312+
they cannot verify against the chart CA. Remove `server.grpcEndpoint` or set
313+
it to the internal service name so supervisors receive the internal cert:
314+
315+
```bash
316+
helm -n openshell get values openshell | grep -E 'grpcEndpoint|clientCaFromServerTlsSecret|clientCaSecretName|serverIssuerRef|caSecretName'
317+
# server.grpcEndpoint should be unset or point to internal service name
318+
```
319+
320+
Less commonly, `UnknownCA` can occur if the gateway's client-verification CA
321+
is misconfigured. The default `clientCaFromServerTlsSecret=true` is correct
322+
for all configurations — the internal server certificate is always signed by
323+
the chart CA (the same CA that signs the client cert), so its `ca.crt` is
324+
the right trust anchor. Only override this if you intentionally mount a
325+
separate client CA via `server.tls.clientCaSecretName`. Verify the mounted
326+
client CA matches the CA that signed the client certificate:
327+
328+
```bash
329+
kubectl -n openshell get statefulset openshell -o jsonpath='{.spec.template.spec.volumes[?(@.name=="tls-client-ca")]}' | jq .
330+
# Should show items filter for ca.crt from openshell-server-tls
331+
```
332+
287333
If `server.providerTokenGrants.spiffe.enabled=true`, the gateway should still
288334
render `[openshell.gateway.gateway_jwt]` and mount the `sandbox-jwt` Secret.
289335
SPIRE is used only by sandbox pods for dynamic provider token grants. Verify
@@ -469,6 +515,8 @@ openshell logs <sandbox-name>
469515
| `K8s namespace not ready` with `envoy-gateway-openshell.yaml: the server could not find the requested resource` | Optional Gateway API manifest was applied without Envoy Gateway CRDs, or k3s Helm controller startup exceeded the namespace wait | Apply `deploy/kube/manifests/envoy-gateway-openshell.yaml` manually only after Envoy Gateway is installed and `grpcRoute` is enabled |
470516
| HTTPS ingress (`grpcRoute.gateway.listener.protocol=HTTPS`) connection resets or TLS handshake hangs | Envoy terminates TLS but the gateway pod still expects TLS, so the plaintext backend hop fails | Set `server.disableTls=true` so Envoy forwards plaintext to the pod; verify the listener `certificateRefs` Secret exists in the release namespace and `openshell status` over `https://<host>` |
471517
| HTTPS ingress returns `Unauthenticated` after connecting | TLS terminates at Envoy, so the gateway never sees a client cert; no OIDC issuer is configured for identity | Configure `server.oidc.issuer` and register with `openshell gateway add https://<host> --oidc-issuer <url>`, or set `server.auth.allowUnauthenticatedUsers=true` for a trusted-proxy/dev cluster |
518+
| External server `Certificate` never becomes Ready with `certManager.serverIssuerRef` set | ACME issuer rejected internal-only SANs, a loopback IP, or a `commonName` absent from the SANs | `kubectl -n openshell describe certificate openshell-server-external`; confirm `certManager.serverDnsNames` lists only real, externally-resolvable hostnames |
519+
| Sandbox supervisors fail TLS handshake with `UnknownCA` after configuring `certManager.serverIssuerRef` | `server.grpcEndpoint` is set to the external hostname, forcing supervisors to receive the ACME cert (via SNI) which they can't verify against chart CA | Remove `server.grpcEndpoint` or set it to the internal service name; supervisors should connect via internal service name to receive the internal cert |
472520

473521
## Reporting
474522

crates/openshell-core/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ async-trait = "0.1"
1515
glob = { workspace = true }
1616
prost = { workspace = true }
1717
prost-types = { workspace = true }
18-
tonic = { workspace = true, features = ["channel", "tls-native-roots"] }
18+
tonic = { workspace = true, features = ["channel", "tls-ring"] }
1919
tonic-prost = { workspace = true }
2020
tokio = { workspace = true }
2121
thiserror = { workspace = true }

crates/openshell-core/src/config.rs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -547,6 +547,24 @@ pub struct TlsConfig {
547547
/// When `false`, client certificates are accepted but not required.
548548
#[serde(default)]
549549
pub require_client_auth: bool,
550+
551+
/// Path to an external TLS certificate file (e.g. ACME/publicly-trusted).
552+
/// When set, the server uses SNI-based certificate selection: connections
553+
/// whose SNI hostname matches `external_server_names` receive this cert,
554+
/// all others receive the primary (internal) cert.
555+
#[serde(default)]
556+
pub external_cert_path: Option<PathBuf>,
557+
558+
/// Path to the private key for the external TLS certificate.
559+
#[serde(default)]
560+
pub external_key_path: Option<PathBuf>,
561+
562+
/// Hostnames that should be served with the external certificate.
563+
/// Connections whose SNI matches one of these names receive the external
564+
/// cert; all other connections (including those with no SNI) receive the
565+
/// primary (internal) cert.
566+
#[serde(default)]
567+
pub external_server_names: Vec<String>,
550568
}
551569

552570
/// OIDC (`OpenID` Connect) configuration for JWT-based authentication.

crates/openshell-core/src/grpc_client.rs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,17 @@ async fn build_plain_channel(endpoint: &str) -> Result<Channel> {
167167
.into_diagnostic()
168168
.wrap_err_with(|| format!("failed to read client key from {key_path}"))?;
169169

170+
// Trust only the configured CA — this is the chart's internal CA
171+
// that signs both the gateway's internal server certificate and
172+
// this client's identity certificate. The gateway uses SNI-based
173+
// certificate selection to present this internal cert to supervisor
174+
// connections, so no public root trust is needed here.
175+
//
176+
// Do NOT add `.with_native_roots()` or `.with_webpki_roots()` here:
177+
// the supervisor runs inside the user-selected sandbox image
178+
// (Docker/Podman drivers), and broadening the trust store would let
179+
// an attacker who controls the image + DNS present a publicly valid
180+
// certificate and intercept the supervisor→gateway TLS connection.
170181
let mut tls_config = ClientTlsConfig::new()
171182
.ca_certificate(Certificate::from_pem(ca_pem))
172183
.identity(Identity::from_pem(cert_pem, key_pem));

crates/openshell-driver-docker/src/lib.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2453,6 +2453,11 @@ fn build_environment_for_oci_user(
24532453

24542454
environment.remove(openshell_core::sandbox_env::SANDBOX_TOKEN);
24552455
environment.remove(openshell_core::sandbox_env::SANDBOX_TOKEN_FILE);
2456+
// Prevent user-supplied environment from overriding the TLS server name
2457+
// the supervisor verifies — a sandbox user who can redirect the gateway
2458+
// hostname could otherwise present a certificate for a name they control
2459+
// and intercept the sandbox JWT.
2460+
environment.remove(openshell_core::sandbox_env::GATEWAY_TLS_SERVER_NAME);
24562461
environment.insert(
24572462
openshell_core::sandbox_env::OCI_IMAGE_USER.to_string(),
24582463
oci_user.to_string(),

crates/openshell-driver-docker/src/tests.rs

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -595,6 +595,26 @@ fn build_environment_protects_oci_identity_metadata() {
595595
assert!(!env.iter().any(|entry| entry.ends_with("=9999")));
596596
}
597597

598+
#[test]
599+
fn build_environment_strips_gateway_tls_server_name() {
600+
let mut sandbox = test_sandbox();
601+
let spec = sandbox.spec.as_mut().unwrap();
602+
spec.environment.insert(
603+
openshell_core::sandbox_env::GATEWAY_TLS_SERVER_NAME.to_string(),
604+
"evil.attacker.example.com".to_string(),
605+
);
606+
607+
let env = build_environment(&sandbox, &runtime_config());
608+
609+
assert!(
610+
!env.iter().any(|entry| entry.starts_with(&format!(
611+
"{}=",
612+
openshell_core::sandbox_env::GATEWAY_TLS_SERVER_NAME
613+
))),
614+
"GATEWAY_TLS_SERVER_NAME must be stripped from the supervisor environment"
615+
);
616+
}
617+
598618
#[test]
599619
fn container_creation_uses_inspected_immutable_image() {
600620
let sandbox = test_sandbox();

crates/openshell-driver-podman/src/container.rs

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -483,6 +483,11 @@ fn build_env(
483483

484484
env.remove(openshell_core::sandbox_env::SANDBOX_TOKEN);
485485
env.remove(openshell_core::sandbox_env::SANDBOX_TOKEN_FILE);
486+
// Prevent user-supplied environment from overriding the TLS server name
487+
// the supervisor verifies — a sandbox user who can redirect the gateway
488+
// hostname could otherwise present a certificate for a name they control
489+
// and intercept the sandbox JWT.
490+
env.remove(openshell_core::sandbox_env::GATEWAY_TLS_SERVER_NAME);
486491
env.insert(
487492
openshell_core::sandbox_env::OCI_IMAGE_USER.into(),
488493
oci_user.to_string(),
@@ -1413,6 +1418,24 @@ mod tests {
14131418
);
14141419
}
14151420

1421+
#[test]
1422+
fn build_env_strips_gateway_tls_server_name() {
1423+
let mut sandbox = test_sandbox("test-id", "test-name");
1424+
let spec = sandbox.spec.get_or_insert_default();
1425+
spec.environment.insert(
1426+
openshell_core::sandbox_env::GATEWAY_TLS_SERVER_NAME.to_string(),
1427+
"evil.attacker.example.com".to_string(),
1428+
);
1429+
1430+
let container = build_container_spec(&sandbox, &test_config());
1431+
1432+
assert_eq!(
1433+
container["env"].get(openshell_core::sandbox_env::GATEWAY_TLS_SERVER_NAME),
1434+
None,
1435+
"GATEWAY_TLS_SERVER_NAME must be stripped from the supervisor environment"
1436+
);
1437+
}
1438+
14161439
#[test]
14171440
fn volume_name_uses_id() {
14181441
assert_eq!(

crates/openshell-driver-vm/src/driver.rs

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4447,6 +4447,11 @@ fn build_guest_environment(
44474447
);
44484448
environment.remove(openshell_core::sandbox_env::SANDBOX_TOKEN);
44494449
environment.remove(openshell_core::sandbox_env::SANDBOX_TOKEN_FILE);
4450+
// Prevent user-supplied environment from overriding the TLS server name
4451+
// the supervisor verifies — a sandbox user who can redirect the gateway
4452+
// hostname could otherwise present a certificate for a name they control
4453+
// and intercept the sandbox JWT.
4454+
environment.remove(openshell_core::sandbox_env::GATEWAY_TLS_SERVER_NAME);
44504455
if sandbox
44514456
.spec
44524457
.as_ref()
@@ -6961,6 +6966,36 @@ mod tests {
69616966
)));
69626967
}
69636968

6969+
#[test]
6970+
fn build_guest_environment_strips_gateway_tls_server_name() {
6971+
let config = VmDriverConfig {
6972+
openshell_endpoint: "http://127.0.0.1:8080".to_string(),
6973+
..Default::default()
6974+
};
6975+
let sandbox = Sandbox {
6976+
id: "sandbox-123".to_string(),
6977+
name: "sandbox-123".to_string(),
6978+
spec: Some(SandboxSpec {
6979+
environment: HashMap::from([(
6980+
openshell_core::sandbox_env::GATEWAY_TLS_SERVER_NAME.to_string(),
6981+
"evil.attacker.example.com".to_string(),
6982+
)]),
6983+
..Default::default()
6984+
}),
6985+
..Default::default()
6986+
};
6987+
6988+
let env = build_guest_environment(&sandbox, &config, None);
6989+
6990+
assert!(
6991+
!env.iter().any(|v| v.starts_with(&format!(
6992+
"{}=",
6993+
openshell_core::sandbox_env::GATEWAY_TLS_SERVER_NAME
6994+
))),
6995+
"GATEWAY_TLS_SERVER_NAME must be stripped from the guest environment"
6996+
);
6997+
}
6998+
69646999
#[test]
69657000
fn build_guest_environment_uses_deployment_telemetry_toggle() {
69667001
let _guard = ENV_LOCK.lock().unwrap();

crates/openshell-server/src/cli.rs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -294,11 +294,27 @@ fn prepare_server_config(args: &mut RunArgs, matches: &ArgMatches) -> Result<Ser
294294
let key_path = args.tls_key.clone().ok_or_else(|| {
295295
miette::miette!("--tls-key is required when TLS is enabled (use --disable-tls to skip)")
296296
})?;
297+
// External cert config (SNI-based dual cert) is only configurable
298+
// via the TOML file, not CLI flags — it's a deployment-time setting.
299+
let (ext_cert, ext_key, ext_names) = file
300+
.as_ref()
301+
.and_then(|f| f.openshell.gateway.tls.as_ref())
302+
.map(|tls| {
303+
(
304+
tls.external_cert_path.clone(),
305+
tls.external_key_path.clone(),
306+
tls.external_server_names.clone(),
307+
)
308+
})
309+
.unwrap_or_default();
297310
Some(openshell_core::TlsConfig {
298311
cert_path,
299312
key_path,
300313
require_client_auth: has_client_ca && !has_oidc,
301314
client_ca_path: args.tls_client_ca.clone(),
315+
external_cert_path: ext_cert,
316+
external_key_path: ext_key,
317+
external_server_names: ext_names,
302318
})
303319
};
304320

crates/openshell-server/src/lib.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -564,6 +564,9 @@ pub(crate) async fn run_server(
564564
&tls.key_path,
565565
tls.client_ca_path.as_deref(),
566566
tls.require_client_auth,
567+
tls.external_cert_path.as_deref(),
568+
tls.external_key_path.as_deref(),
569+
tls.external_server_names.clone(),
567570
)?;
568571

569572
// Spawn file-watcher-based TLS certificate reload worker.
@@ -1145,6 +1148,9 @@ mod tests {
11451148
&dir.path().join("server-key.pem"),
11461149
Some(&dir.path().join("ca.pem")),
11471150
false,
1151+
None,
1152+
None,
1153+
Vec::new(),
11481154
)
11491155
.expect("failed to build tls acceptor");
11501156

0 commit comments

Comments
 (0)