* chore(deps): replace ring with AWS-LC
Signed-off-by: Simon Scatton <sscatton@nvidia.com>
* fix(lint): address warnings after dependency upgrades
Signed-off-by: Simon Scatton <sscatton@nvidia.com>
* fix(tls): limit provider initialization to reqwest clients
Signed-off-by: Simon Scatton <sscatton@nvidia.com>
---------
Signed-off-by: Simon Scatton <sscatton@nvidia.com>
* 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>
Remove three groups of copy-pasted code in openshell-server:
1. grpc/mod.rs had a private current_time_ms() wrapper identical to the
one already exported from persistence/mod.rs. Remove the duplicate
and update the three grpc sub-modules (policy, sandbox, service) to
import directly from crate::persistence.
2. test_store() was repeated verbatim in seven #[cfg(test)] blocks.
Promote a single canonical version to persistence/mod.rs (cfg-gated)
and replace all copies with crate::persistence::test_store() calls or
a thin Arc wrapper in supervisor_session.
3. grpc_client_mtls() and build_tls_root() were copy-pasted across
edge_tunnel_auth.rs and multiplex_tls_integration.rs. Move both into
the existing tests/common/mod.rs shared module and import from there.
Remove ~280 lines of duplicated code across 30 files in 5 areas:
- centered_rect: consolidate 5 identical TUI layout helpers into a
single pub fn in openshell-tui/src/ui/mod.rs
- server test helpers: replace ~100 inline Store::connect() calls
with local test_store() helpers; deduplicate test_server_state()
in grpc/service.rs to use the shared test_support version
- rogue PKI: extract 20-line rogue CA+client cert generation block
(duplicated in two integration tests) into generate_rogue_pki()
in tests/common/mod.rs
- provider tests: replace 8 identical 28-line test modules with a
single macro_rules! test_discovers_env_credential! invocation
- label constants: centralize openshell.ai/ container label keys
in openshell-core::driver_utils; update Docker and Kubernetes
drivers to import from there instead of redefining them locally
Extract four categories of copy-pasted test code into single
canonical locations:
- POLICY_OBJECT_TYPE / DRAFT_CHUNK_OBJECT_TYPE constants that were
defined identically in both persistence/sqlite.rs and
persistence/postgres.rs are now owned by persistence/mod.rs.
- test_server_state() was duplicated across grpc/provider.rs,
grpc/policy.rs, and grpc/sandbox.rs. A shared grpc::test_support
module in grpc/mod.rs now owns the single implementation; each
submodule imports it.
- The TestOpenShell stub (full OpenShell trait impl), install_rustls_provider(),
PkiBundle, generate_pki(), and start_test_server() were copied across
up to five openshell-server integration test files. A new
tests/common/mod.rs module owns them; each test file uses mod common.
- StubResponse, unique_socket_path(), and spawn_podman_stub() were
duplicated between openshell-driver-podman/src/driver.rs and
openshell-driver-podman/src/grpc.rs. A new src/test_utils.rs
(cfg(test)-gated) owns the shared helpers.
* feat(exec): add bidirectional streaming for interactive TTY sessions
The existing ExecSandbox RPC sends stdin upfront in the request body,
making interactive programs (bash, top, vim) unusable with --tty. This
adds an ExecSandboxInteractive bidirectional streaming RPC that forwards
live keystrokes, terminal resize events, and EOF to the sandbox.
- Add ExecSandboxInteractive RPC, ExecSandboxInput and
ExecSandboxWindowResize messages to the proto definition
- Implement server handler with relay bridging, PTY allocation via
russh channel.split(), and timeout support
- Implement CLI client with raw mode, spawn_blocking stdin reader,
SIGWINCH resize forwarding, and process::exit for clean shutdown
- Route to interactive path only when --tty is explicitly passed
Signed-off-by: Florent Benoit <fbenoit@redhat.com>
* fix(exec): address interactive TTY streaming review feedback
Server-side (sandbox.rs):
- Close SSH channel after EOF so PTY-backed programs (bash, vim, top)
terminate on client disconnect instead of leaking the session
- Break output loop when tx.send() fails (client gone) to tear down
the streaming pipeline promptly
- Default zero cols/rows to 80x24 before request_pty, matching the
proto "0 = use default" contract and preventing 1x1 terminals for
non-CLI clients
Client-side (run.rs):
- Replace spawn_blocking stdin reader with a detached std::thread so
the tokio runtime shutdown does not hang on a thread blocked in
stdin.read(). This removes the need for std::process::exit(),
allowing save_last_sandbox() in the caller to execute normally
- Switch from futures::channel::mpsc with try_send() to
tokio::sync::mpsc with blocking_send() for proper backpressure —
try_send silently stopped forwarding all input when the channel
filled during paste or slow network
- Add TaskGuard (abort-on-drop) for the resize task to ensure cleanup
on all return paths including early errors
---------
Signed-off-by: Florent Benoit <fbenoit@redhat.com>
Make --tls-client-ca optional and make client certificates always
optional when a CA is configured. This decouples HTTPS encryption
from mTLS authentication, allowing mTLS and OIDC bearer tokens to
coexist as parallel authentication mechanisms.
When --tls-client-ca is provided, client certificates are validated
against the CA when presented but never required. Clients may connect
with or without a certificate — authentication is handled at the
application layer (e.g. OIDC).
Two TLS modes are now supported:
- HTTPS with optional mTLS (--tls-client-ca provided)
- HTTPS-only (--tls-client-ca omitted)
The --disable-gateway-auth flag is preserved for backward
compatibility but is now a no-op. The allow_unauthenticated field
has been removed from TlsConfig. The Helm chart conditionally
includes the client-ca volume and env var based on whether
clientCaSecretName is configured.
Varsha