Skip to content

Commit 1df4404

Browse files
committed
refactor!(auth): drop SSH handshake secret in favor of mTLS
The OPENSHELL_SSH_HANDSHAKE_SECRET / x-sandbox-secret mechanism was misnamed: it does not authenticate SSH (which flows over the RelayStream gRPC RPC and is gated by mTLS plus supervisor Unix-socket permissions). It only gated a small set of sandbox-to-gateway control-plane RPCs, and production deployments already enforce mTLS on that channel — so the shared secret was redundant. Replace the secret check with an mTLS-presence marker. Sandbox-class methods (ReportPolicyStatus, PushSandboxLogs, GetSandboxProviderEnvironment, SubmitPolicyAnalysis, GetSandboxConfig, GetInferenceBundle) accept callers without a Bearer token; the gRPC mTLS handshake is the trust boundary. Dual-auth methods treat Bearer-present as full-scope CLI access and Bearer-absent as sandbox-restricted scope via validate_sandbox_caller_update. Drops the secret from all drivers (K8s, Podman, VM), the sandbox gRPC interceptor, the Helm chart (values + pre-install hook + StatefulSet env), the RPM bootstrap script, the man pages, and the debug-openshell-cluster skill. Also removes the never-read ssh_handshake_skew_secs flag and config field. BREAKING CHANGE: --ssh-handshake-secret / OPENSHELL_SSH_HANDSHAKE_SECRET and --ssh-handshake-skew-secs / OPENSHELL_SSH_HANDSHAKE_SKEW_SECS are removed from the gateway, sandbox, and all driver binaries. The openshell-ssh-handshake K8s Secret is no longer managed by the chart; operators may delete the orphan. Deployments using --disable-gateway-auth must enforce caller authentication at the fronting proxy, since the gateway no longer validates a per-request secret on sandbox-class methods. Refs OS-174.
1 parent 6deb1f0 commit 1df4404

35 files changed

Lines changed: 172 additions & 679 deletions

File tree

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,6 @@ Check required Helm deployment secrets:
114114

115115
```bash
116116
kubectl -n openshell get secret \
117-
openshell-ssh-handshake \
118117
openshell-server-tls \
119118
openshell-server-client-ca \
120119
openshell-client-tls

crates/openshell-core/src/config.rs

Lines changed: 0 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,6 @@ pub const DEFAULT_SERVER_PORT: u16 = 8080;
2727
/// Default container stop timeout in seconds (SIGTERM → SIGKILL).
2828
pub const DEFAULT_STOP_TIMEOUT_SECS: u32 = 10;
2929

30-
/// Default allowed clock skew for SSH handshake validation, in seconds.
31-
pub const DEFAULT_SSH_HANDSHAKE_SKEW_SECS: u64 = 300;
32-
3330
/// Default Podman bridge network name.
3431
pub const DEFAULT_NETWORK_NAME: &str = "openshell";
3532

@@ -272,14 +269,6 @@ pub struct Config {
272269
#[serde(default = "default_sandbox_ssh_socket_path")]
273270
pub sandbox_ssh_socket_path: String,
274271

275-
/// Shared secret for gateway-to-sandbox SSH handshake.
276-
#[serde(default)]
277-
pub ssh_handshake_secret: String,
278-
279-
/// Allowed clock skew for SSH handshake validation, in seconds.
280-
#[serde(default = "default_ssh_handshake_skew_secs")]
281-
pub ssh_handshake_skew_secs: u64,
282-
283272
/// TTL for SSH session tokens, in seconds. 0 disables expiry.
284273
#[serde(default = "default_ssh_session_ttl_secs")]
285274
pub ssh_session_ttl_secs: u64,
@@ -429,8 +418,6 @@ impl Config {
429418
ssh_gateway_port: default_ssh_gateway_port(),
430419
sandbox_ssh_port: default_sandbox_ssh_port(),
431420
sandbox_ssh_socket_path: default_sandbox_ssh_socket_path(),
432-
ssh_handshake_secret: String::new(),
433-
ssh_handshake_skew_secs: default_ssh_handshake_skew_secs(),
434421
ssh_session_ttl_secs: default_ssh_session_ttl_secs(),
435422
client_tls_secret_name: String::new(),
436423
host_gateway_ip: String::new(),
@@ -544,20 +531,6 @@ impl Config {
544531
self
545532
}
546533

547-
/// Create a new configuration with the SSH handshake secret.
548-
#[must_use]
549-
pub fn with_ssh_handshake_secret(mut self, secret: impl Into<String>) -> Self {
550-
self.ssh_handshake_secret = secret.into();
551-
self
552-
}
553-
554-
/// Create a new configuration with SSH handshake skew allowance.
555-
#[must_use]
556-
pub const fn with_ssh_handshake_skew_secs(mut self, secs: u64) -> Self {
557-
self.ssh_handshake_skew_secs = secs;
558-
self
559-
}
560-
561534
/// Create a new configuration with the SSH session TTL.
562535
#[must_use]
563536
pub const fn with_ssh_session_ttl_secs(mut self, secs: u64) -> Self {
@@ -707,10 +680,6 @@ const fn default_sandbox_ssh_port() -> u16 {
707680
DEFAULT_SSH_PORT
708681
}
709682

710-
const fn default_ssh_handshake_skew_secs() -> u64 {
711-
DEFAULT_SSH_HANDSHAKE_SKEW_SECS
712-
}
713-
714683
const fn default_ssh_session_ttl_secs() -> u64 {
715684
86400 // 24 hours
716685
}

crates/openshell-core/src/sandbox_env.rs

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,6 @@ pub const SANDBOX_ID: &str = "OPENSHELL_SANDBOX_ID";
2020
/// Filesystem path to the UNIX socket used for the in-sandbox SSH server.
2121
pub const SSH_SOCKET_PATH: &str = "OPENSHELL_SSH_SOCKET_PATH";
2222

23-
/// Shared secret used for HMAC-based SSH handshake authentication.
24-
pub const SSH_HANDSHAKE_SECRET: &str = "OPENSHELL_SSH_HANDSHAKE_SECRET";
25-
26-
/// Allowed clock-skew tolerance in seconds for the SSH handshake.
27-
pub const SSH_HANDSHAKE_SKEW_SECS: &str = "OPENSHELL_SSH_HANDSHAKE_SKEW_SECS";
28-
2923
/// Log level for the sandbox supervisor (e.g. `"debug"`, `"info"`, `"warn"`).
3024
pub const LOG_LEVEL: &str = "OPENSHELL_LOG_LEVEL";
3125

crates/openshell-driver-kubernetes/src/config.rs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,6 @@ pub struct KubernetesComputeConfig {
5353
pub supervisor_sideload_method: SupervisorSideloadMethod,
5454
pub grpc_endpoint: String,
5555
pub ssh_socket_path: String,
56-
pub ssh_handshake_secret: String,
57-
pub ssh_handshake_skew_secs: u64,
5856
pub client_tls_secret_name: String,
5957
pub host_gateway_ip: String,
6058
pub enable_user_namespaces: bool,

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

Lines changed: 11 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -176,10 +176,6 @@ impl KubernetesComputeDriver {
176176
&self.config.ssh_socket_path
177177
}
178178

179-
pub const fn ssh_handshake_skew_secs(&self) -> u64 {
180-
self.config.ssh_handshake_skew_secs
181-
}
182-
183179
fn watch_api(&self) -> Api<DynamicObject> {
184180
let gvk = GroupVersionKind::gvk(SANDBOX_GROUP, SANDBOX_VERSION, SANDBOX_KIND);
185181
let resource = ApiResource::from_gvk(&gvk);
@@ -296,10 +292,6 @@ impl KubernetesComputeDriver {
296292
}
297293
}
298294

299-
fn ssh_handshake_secret(&self) -> &str {
300-
&self.config.ssh_handshake_secret
301-
}
302-
303295
pub async fn create_sandbox(&self, sandbox: &Sandbox) -> Result<(), KubernetesDriverError> {
304296
let name = sandbox.name.as_str();
305297
info!(
@@ -328,8 +320,6 @@ impl KubernetesComputeDriver {
328320
sandbox_name: &sandbox.name,
329321
grpc_endpoint: &self.config.grpc_endpoint,
330322
ssh_socket_path: self.ssh_socket_path(),
331-
ssh_handshake_secret: self.ssh_handshake_secret(),
332-
ssh_handshake_skew_secs: self.ssh_handshake_skew_secs(),
333323
client_tls_secret_name: &self.config.client_tls_secret_name,
334324
host_gateway_ip: &self.config.host_gateway_ip,
335325
enable_user_namespaces: self.config.enable_user_namespaces,
@@ -984,8 +974,6 @@ struct SandboxPodParams<'a> {
984974
sandbox_name: &'a str,
985975
grpc_endpoint: &'a str,
986976
ssh_socket_path: &'a str,
987-
ssh_handshake_secret: &'a str,
988-
ssh_handshake_skew_secs: u64,
989977
client_tls_secret_name: &'a str,
990978
host_gateway_ip: &'a str,
991979
enable_user_namespaces: bool,
@@ -1146,8 +1134,6 @@ fn sandbox_template_to_k8s(
11461134
params.sandbox_name,
11471135
params.grpc_endpoint,
11481136
params.ssh_socket_path,
1149-
params.ssh_handshake_secret,
1150-
params.ssh_handshake_skew_secs,
11511137
!params.client_tls_secret_name.is_empty(),
11521138
);
11531139

@@ -1310,8 +1296,6 @@ fn build_env_list(
13101296
sandbox_name: &str,
13111297
grpc_endpoint: &str,
13121298
ssh_socket_path: &str,
1313-
ssh_handshake_secret: &str,
1314-
ssh_handshake_skew_secs: u64,
13151299
tls_enabled: bool,
13161300
) -> Vec<serde_json::Value> {
13171301
let mut env = existing_env.cloned().unwrap_or_default();
@@ -1323,8 +1307,6 @@ fn build_env_list(
13231307
sandbox_name,
13241308
grpc_endpoint,
13251309
ssh_socket_path,
1326-
ssh_handshake_secret,
1327-
ssh_handshake_skew_secs,
13281310
tls_enabled,
13291311
);
13301312
env
@@ -1341,15 +1323,12 @@ fn apply_env_map(
13411323

13421324
// Required env vars are passed individually for clarity at call sites; grouping into a struct
13431325
// would not improve readability for this internal helper.
1344-
#[allow(clippy::too_many_arguments)]
13451326
fn apply_required_env(
13461327
env: &mut Vec<serde_json::Value>,
13471328
sandbox_id: &str,
13481329
sandbox_name: &str,
13491330
grpc_endpoint: &str,
13501331
ssh_socket_path: &str,
1351-
ssh_handshake_secret: &str,
1352-
ssh_handshake_skew_secs: u64,
13531332
tls_enabled: bool,
13541333
) {
13551334
upsert_env(env, openshell_core::sandbox_env::SANDBOX_ID, sandbox_id);
@@ -1367,16 +1346,6 @@ fn apply_required_env(
13671346
ssh_socket_path,
13681347
);
13691348
}
1370-
upsert_env(
1371-
env,
1372-
openshell_core::sandbox_env::SSH_HANDSHAKE_SECRET,
1373-
ssh_handshake_secret,
1374-
);
1375-
upsert_env(
1376-
env,
1377-
openshell_core::sandbox_env::SSH_HANDSHAKE_SKEW_SECS,
1378-
&ssh_handshake_skew_secs.to_string(),
1379-
);
13801349
// TLS cert paths for sandbox-to-server mTLS. Only set when TLS is enabled
13811350
// and the client TLS secret is mounted into the sandbox pod.
13821351
if tls_enabled {
@@ -1535,29 +1504,27 @@ mod tests {
15351504
use prost_types::{Struct, Value, value::Kind};
15361505

15371506
#[test]
1538-
fn apply_required_env_always_injects_ssh_handshake_secret() {
1507+
fn apply_required_env_does_not_inject_handshake_env() {
15391508
let mut env = Vec::new();
15401509
apply_required_env(
15411510
&mut env,
15421511
"sandbox-1",
15431512
"my-sandbox",
15441513
"https://endpoint:8080",
15451514
"0.0.0.0:2222",
1546-
"my-secret-value",
1547-
300,
15481515
true,
15491516
);
15501517

1551-
let secret_entry = env
1552-
.iter()
1553-
.find(|e| {
1554-
e.get("name").and_then(|v| v.as_str()) == Some("OPENSHELL_SSH_HANDSHAKE_SECRET")
1555-
})
1556-
.expect("OPENSHELL_SSH_HANDSHAKE_SECRET must be present in env");
1557-
assert_eq!(
1558-
secret_entry.get("value").and_then(|v| v.as_str()),
1559-
Some("my-secret-value")
1560-
);
1518+
for name in [
1519+
"OPENSHELL_SSH_HANDSHAKE_SECRET",
1520+
"OPENSHELL_SSH_HANDSHAKE_SKEW_SECS",
1521+
] {
1522+
assert!(
1523+
!env.iter()
1524+
.any(|e| e.get("name").and_then(|v| v.as_str()) == Some(name)),
1525+
"{name} must not appear in sandbox pod env after handshake-secret removal"
1526+
);
1527+
}
15611528
}
15621529

15631530
#[test]
@@ -1790,8 +1757,6 @@ mod tests {
17901757
"my-sandbox",
17911758
"https://endpoint:8080",
17921759
"0.0.0.0:2222",
1793-
"secret",
1794-
300,
17951760
true, // tls_enabled
17961761
);
17971762

crates/openshell-driver-kubernetes/src/main.rs

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -47,12 +47,6 @@ struct Args {
4747
)]
4848
sandbox_ssh_socket_path: String,
4949

50-
#[arg(long, env = "OPENSHELL_SSH_HANDSHAKE_SECRET")]
51-
ssh_handshake_secret: String,
52-
53-
#[arg(long, env = "OPENSHELL_SSH_HANDSHAKE_SKEW_SECS", default_value_t = 300)]
54-
ssh_handshake_skew_secs: u64,
55-
5650
#[arg(long, env = "OPENSHELL_CLIENT_TLS_SECRET_NAME")]
5751
client_tls_secret_name: Option<String>,
5852

@@ -96,8 +90,6 @@ async fn main() -> Result<()> {
9690
supervisor_sideload_method: args.supervisor_sideload_method,
9791
grpc_endpoint: args.grpc_endpoint.unwrap_or_default(),
9892
ssh_socket_path: args.sandbox_ssh_socket_path,
99-
ssh_handshake_secret: args.ssh_handshake_secret,
100-
ssh_handshake_skew_secs: args.ssh_handshake_skew_secs,
10193
client_tls_secret_name: args.client_tls_secret_name.unwrap_or_default(),
10294
host_gateway_ip: args.host_gateway_ip.unwrap_or_default(),
10395
enable_user_namespaces: args.enable_user_namespaces,

crates/openshell-driver-podman/README.md

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -215,13 +215,9 @@ via sandbox templates:
215215
- `OPENSHELL_SANDBOX_ID`
216216
- `OPENSHELL_ENDPOINT`
217217
- `OPENSHELL_SSH_SOCKET_PATH`
218-
- `OPENSHELL_SSH_HANDSHAKE_SKEW_SECS`
219218
- `OPENSHELL_CONTAINER_IMAGE`
220219
- `OPENSHELL_SANDBOX_COMMAND`
221220

222-
The `PodmanComputeConfig::Debug` implementation redacts the handshake secret as
223-
`[REDACTED]`.
224-
225221
## Sandbox Lifecycle
226222

227223
### Creation Flow
@@ -239,18 +235,15 @@ sequenceDiagram
239235
D->>P: pull_image(supervisor, "missing")
240236
D->>P: pull_image(sandbox_image, policy)
241237
242-
D->>P: create_secret(handshake)
243-
Note over D: On failure below, rollback secret
244-
245238
D->>P: create_volume(workspace)
246-
Note over D: On failure below, rollback volume + secret
239+
Note over D: On failure below, rollback volume
247240
248241
D->>P: create_container(spec)
249242
alt Conflict (409)
250-
D->>P: remove_volume + remove_secret
243+
D->>P: remove_volume
251244
D-->>GW: AlreadyExists
252245
end
253-
Note over D: On failure below, rollback container + volume + secret
246+
Note over D: On failure below, rollback container + volume
254247
255248
D->>P: start_container
256249
D-->>GW: Ok
@@ -300,8 +293,6 @@ Podman resources after out-of-band container removal or label drift.
300293
| `OPENSHELL_GATEWAY_PORT` | `--gateway-port` | `8080` | Gateway port used for endpoint auto-detection by the standalone binary. |
301294
| `OPENSHELL_NETWORK_NAME` | `--network-name` | `openshell` | Podman bridge network name. |
302295
| `OPENSHELL_SANDBOX_SSH_PORT` | `--sandbox-ssh-port` | `2222` | SSH compatibility port inside the container. |
303-
| `OPENSHELL_SSH_HANDSHAKE_SECRET` | `--ssh-handshake-secret` | Required standalone, gateway-generated in-process | Shared secret for the NSSH1 handshake. |
304-
| `OPENSHELL_SSH_HANDSHAKE_SKEW_SECS` | `--ssh-handshake-skew-secs` | `300` | Allowed timestamp skew for SSH handshake validation. |
305296
| `OPENSHELL_SANDBOX_SSH_SOCKET_PATH` | `--sandbox-ssh-socket-path` | `/run/openshell/ssh.sock` | Standalone driver only: supervisor Unix socket path in `PodmanComputeConfig`. In-gateway Podman uses server `config.sandbox_ssh_socket_path`. |
306297
| `OPENSHELL_STOP_TIMEOUT` | `--stop-timeout` | `10` | Container stop timeout in seconds. |
307298
| `OPENSHELL_SUPERVISOR_IMAGE` | `--supervisor-image` | `openshell/supervisor:latest` through the gateway, required standalone | OCI image containing the supervisor binary. |

0 commit comments

Comments
 (0)