diff --git a/architecture/security-policy.md b/architecture/security-policy.md index e669ea2fd5..4d6b5a5348 100644 --- a/architecture/security-policy.md +++ b/architecture/security-policy.md @@ -123,6 +123,11 @@ stream, but it does not inspect TLS SNI, HTTP `Host`, or another protocol-level destination. Compatible shared infrastructure can therefore let a client select another tenant, virtual host, or service behind the approved front door. +An endpoint's `tls` field is either omitted, meaning auto-detect and terminate +for inspection, or `skip`. Every other value fails shared L7 endpoint +validation, so a policy cannot name a transport mode the proxy does not +implement. + ## Credentialed Endpoints OpenShell keeps provider credentials on paths it can inspect or rewrite by diff --git a/crates/openshell-cli/tests/provider_commands_integration.rs b/crates/openshell-cli/tests/provider_commands_integration.rs index 2c9b9ba005..20d1fc21f8 100644 --- a/crates/openshell-cli/tests/provider_commands_integration.rs +++ b/crates/openshell-cli/tests/provider_commands_integration.rs @@ -2604,7 +2604,6 @@ endpoints: - host: api.advanced.example ports: [443, 8443] protocol: rest - tls: terminate enforcement: enforce rules: - allow: diff --git a/crates/openshell-driver-mxc/tests/wxc_exec_real.rs b/crates/openshell-driver-mxc/tests/wxc_exec_real.rs index a3bbdc7d07..e1dd545423 100644 --- a/crates/openshell-driver-mxc/tests/wxc_exec_real.rs +++ b/crates/openshell-driver-mxc/tests/wxc_exec_real.rs @@ -732,7 +732,6 @@ async fn pc_https_egress_reads_injected_ca_bundle() { host: "example.com".to_string(), ports: vec![443], protocol: "rest".to_string(), - tls: "terminate".to_string(), enforcement: "enforce".to_string(), access: "read-only".to_string(), ..Default::default() diff --git a/crates/openshell-policy/src/l7_validate.rs b/crates/openshell-policy/src/l7_validate.rs index fe07df123f..8302923417 100644 --- a/crates/openshell-policy/src/l7_validate.rs +++ b/crates/openshell-policy/src/l7_validate.rs @@ -49,6 +49,23 @@ pub fn is_explicit_tcp_protocol(protocol: &str) -> bool { protocol.eq_ignore_ascii_case("tcp") } +/// `skip` is deliberately not offered as an alternative: it is a posture +/// downgrade, not a fix. +const TLS_MODE_REMEDIATION: &str = "remove the tls field to keep automatic TLS termination"; + +/// Reject `tls` values outside the supported set. +/// +/// Omitted means auto-detect and terminate for inspection; `skip` opts out. +pub fn validate_tls_mode(tls: &str) -> Option { + if tls.is_empty() || tls == "skip" { + return None; + } + + Some(format!( + "unsupported tls value '{tls}'; {TLS_MODE_REMEDIATION}" + )) +} + /// Reject transport choices that an in-sandbox agent must not grant itself. /// /// An omitted protocol remains allowed: it uses the established explicit @@ -101,12 +118,33 @@ mod agent_transport_tests { #[test] fn agent_cannot_request_native_tcp_or_skip_tls_inspection() { assert!(agent_authored_transport_rejection("tcp", "").is_some()); - assert!(agent_authored_transport_rejection("TCP", "terminate").is_some()); + assert!(agent_authored_transport_rejection("TCP", "").is_some()); assert!(agent_authored_transport_rejection("", "skip").is_some()); assert!(agent_authored_transport_rejection("rest", "SKIP").is_some()); } } +#[cfg(test)] +mod tls_mode_tests { + use super::validate_tls_mode; + + #[test] + fn omitted_and_skip_are_accepted() { + for tls in ["", "skip"] { + assert_eq!(validate_tls_mode(tls), None, "tls: {tls:?}"); + } + } + + #[test] + fn every_other_value_is_rejected() { + for tls in ["terminate", "passthrough", "bogus", "SKIP"] { + let error = validate_tls_mode(tls).expect("must be rejected"); + assert!(error.contains(&format!("unsupported tls value '{tls}'"))); + assert!(error.contains("remove the tls field")); + } + } +} + /// Fields extracted from an endpoint definition needed for L7 semantic /// validation. Both profile lint and the runtime validator construct this /// from their own data representation. @@ -120,6 +158,9 @@ pub struct L7EndpointFields<'a> { /// means no access preset. pub access: &'a str, + /// TLS handling as authored. Empty string means the default. + pub tls: &'a str, + /// `true` when the endpoint has a non-empty rules list. pub has_rules: bool, @@ -134,8 +175,8 @@ pub struct L7EndpointFields<'a> { pub allow_all_known_mcp_methods: bool, } -/// Validate the semantic consistency of an L7 endpoint's field -/// combination. +/// Validate an L7 endpoint's individual field values and their semantic +/// consistency with each other. /// /// Returns a list of error message strings. An empty list means the /// endpoint passes validation. Messages are bare — callers prepend @@ -226,6 +267,11 @@ pub fn validate_l7_endpoint_semantics(ep: &L7EndpointFields<'_>) -> Vec errors.push("deny_rules require rules or access to define the base allow set".to_string()); } + // 10. Unsupported tls value. + if let Some(reason) = validate_tls_mode(ep.tls) { + errors.push(reason); + } + errors } @@ -237,6 +283,7 @@ mod tests { L7EndpointFields { protocol: "rest", access: "read-only", + tls: "", has_rules: false, has_deny_rules: false, rules_would_deny_all: false, @@ -255,6 +302,7 @@ mod tests { let ep = L7EndpointFields { protocol: "ftp", access: "", + tls: "", has_rules: false, has_deny_rules: false, rules_would_deny_all: false, @@ -269,6 +317,7 @@ mod tests { let ep = L7EndpointFields { protocol: "rest", access: "full", + tls: "", has_rules: true, has_deny_rules: false, rules_would_deny_all: false, @@ -283,6 +332,7 @@ mod tests { let ep = L7EndpointFields { protocol: "json-rpc", access: "full", + tls: "", has_rules: false, has_deny_rules: false, rules_would_deny_all: false, @@ -301,6 +351,7 @@ mod tests { let ep = L7EndpointFields { protocol: "mcp", access: "full", + tls: "", has_rules: false, has_deny_rules: false, rules_would_deny_all: false, @@ -319,6 +370,7 @@ mod tests { let ep = L7EndpointFields { protocol: "json-rpc", access: "", + tls: "", has_rules: false, has_deny_rules: false, rules_would_deny_all: false, @@ -333,6 +385,7 @@ mod tests { let ep = L7EndpointFields { protocol: "rest", access: "", + tls: "", has_rules: false, has_deny_rules: false, rules_would_deny_all: false, @@ -351,6 +404,7 @@ mod tests { let ep = L7EndpointFields { protocol: "mcp", access: "", + tls: "", has_rules: false, has_deny_rules: false, rules_would_deny_all: false, @@ -365,6 +419,7 @@ mod tests { let ep = L7EndpointFields { protocol: "mcp", access: "", + tls: "", has_rules: false, has_deny_rules: false, rules_would_deny_all: false, @@ -379,6 +434,7 @@ mod tests { let ep = L7EndpointFields { protocol: "rest", access: "", + tls: "", has_rules: true, has_deny_rules: false, rules_would_deny_all: true, @@ -393,6 +449,7 @@ mod tests { let ep = L7EndpointFields { protocol: "mcp", access: "", + tls: "", has_rules: true, has_deny_rules: false, rules_would_deny_all: true, @@ -410,6 +467,7 @@ mod tests { let ep = L7EndpointFields { protocol: "", access: "", + tls: "", has_rules: false, has_deny_rules: true, rules_would_deny_all: false, @@ -428,6 +486,7 @@ mod tests { let ep = L7EndpointFields { protocol: "rest", access: "", + tls: "", has_rules: false, has_deny_rules: true, rules_would_deny_all: false, @@ -446,6 +505,7 @@ mod tests { let ep = L7EndpointFields { protocol: "", access: "", + tls: "", has_rules: false, has_deny_rules: false, rules_would_deny_all: false, @@ -460,6 +520,7 @@ mod tests { let ep = L7EndpointFields { protocol: "tcp", access: "", + tls: "", has_rules: false, has_deny_rules: false, rules_would_deny_all: false, @@ -476,6 +537,7 @@ mod tests { let ep = L7EndpointFields { protocol: "tcp", access: "full", + tls: "", has_rules: false, has_deny_rules: false, rules_would_deny_all: false, @@ -520,6 +582,7 @@ mod tests { let ep = L7EndpointFields { protocol: "json-rpc", access: "full", + tls: "", has_rules: false, has_deny_rules: false, rules_would_deny_all: false, @@ -541,6 +604,7 @@ mod tests { let ep = L7EndpointFields { protocol: "json-rpc", access: "", + tls: "", has_rules: false, has_deny_rules: false, rules_would_deny_all: false, diff --git a/crates/openshell-policy/src/lib.rs b/crates/openshell-policy/src/lib.rs index a9d2ab0bbf..a027fe6b5e 100644 --- a/crates/openshell-policy/src/lib.rs +++ b/crates/openshell-policy/src/lib.rs @@ -38,7 +38,7 @@ pub use compose::{ }; pub use l7_validate::{ L7EndpointFields, L7Protocol, agent_authored_transport_rejection, - validate_explicit_tcp_additional_fields, validate_l7_endpoint_semantics, + validate_explicit_tcp_additional_fields, validate_l7_endpoint_semantics, validate_tls_mode, }; pub use merge::{ PolicyMergeError, PolicyMergeOp, PolicyMergeResult, PolicyMergeWarning, @@ -1513,6 +1513,7 @@ fn validate_sandbox_policy_with_mcp_presence( let fields = L7EndpointFields { protocol: &ep.protocol, access: &ep.access, + tls: &ep.tls, has_rules: !ep.rules.is_empty(), has_deny_rules: !ep.deny_rules.is_empty(), rules_would_deny_all, @@ -3871,6 +3872,35 @@ network_policies: assert!(validate_sandbox_policy(&policy).is_ok()); } + #[test] + fn validate_rejects_unknown_tls_mode() { + let mut policy = restrictive_default_policy(); + policy.network_policies.insert( + "api".into(), + NetworkPolicyRule { + name: "api".into(), + endpoints: vec![NetworkEndpoint { + host: "api.example.com".into(), + port: 443, + tls: "terminate".into(), + ..Default::default() + }], + ..Default::default() + }, + ); + + let violations = + validate_sandbox_policy(&policy).expect_err("unsupported tls value must be rejected"); + assert!( + violations.iter().any(|v| matches!( + v, + PolicyViolation::InvalidL7Endpoint { reason, .. } + if reason.contains("unsupported tls value 'terminate'") + )), + "should be rejected: {violations:?}" + ); + } + #[test] fn validate_rejects_sigv4_no_body_without_signing_service() { let mut policy = restrictive_default_policy(); diff --git a/crates/openshell-policy/src/merge.rs b/crates/openshell-policy/src/merge.rs index 59b8bc7605..66b8ad2ad1 100644 --- a/crates/openshell-policy/src/merge.rs +++ b/crates/openshell-policy/src/merge.rs @@ -697,7 +697,7 @@ fn endpoint_attributes_cover(loaded: &NetworkEndpoint, proposed: &NetworkEndpoin if !proposed.protocol.is_empty() && !protocols_match(&loaded.protocol, &proposed.protocol) { return false; } - if !proposed.tls.is_empty() && effective_tls(&loaded.tls) != effective_tls(&proposed.tls) { + if !proposed.tls.is_empty() && loaded.tls != proposed.tls { return false; } if !proposed.enforcement.is_empty() @@ -801,13 +801,6 @@ fn protocols_match(left: &str, right: &str) -> bool { } } -fn effective_tls(value: &str) -> &str { - match value { - "" | "terminate" | "passthrough" => "auto", - value => value, - } -} - fn effective_enforcement(value: &str) -> &str { if value.is_empty() { "audit" } else { value } } @@ -3537,7 +3530,6 @@ mod tests { assert!(!policy_covers_rule(&loaded, &different_body)); let mut explicit_defaults = loaded_endpoint; - explicit_defaults.tls = "passthrough".to_string(); explicit_defaults.enforcement = "audit".to_string(); let runtime_defaults = rule_with_authorizations( "proposed", @@ -3546,13 +3538,17 @@ mod tests { ); assert!(policy_covers_rule(&loaded, &runtime_defaults)); - explicit_defaults.tls = "terminate".to_string(); - let legacy_terminate = rule_with_authorizations( - "proposed", - vec![explicit_defaults.clone()], - &["/usr/bin/client"], - ); - assert!(policy_covers_rule(&loaded, &legacy_terminate)); + // Coverage validates both sides, so a legacy spelling fails closed + // rather than comparing equal to the automatic default. + for legacy in ["terminate", "passthrough"] { + explicit_defaults.tls = legacy.to_string(); + let legacy_rule = rule_with_authorizations( + "proposed", + vec![explicit_defaults.clone()], + &["/usr/bin/client"], + ); + assert!(!policy_covers_rule(&loaded, &legacy_rule), "tls: {legacy}"); + } explicit_defaults.tls = "skip".to_string(); let skip_tls = rule_with_authorizations( diff --git a/crates/openshell-providers/src/profiles.rs b/crates/openshell-providers/src/profiles.rs index 1ef224bd09..a7511d2825 100644 --- a/crates/openshell-providers/src/profiles.rs +++ b/crates/openshell-providers/src/profiles.rs @@ -2535,6 +2535,7 @@ pub fn validate_profile_set( let l7_fields = L7EndpointFields { protocol: &endpoint.protocol, access: &endpoint.access, + tls: &endpoint.tls, has_rules: endpoint.rules.as_ref().is_some_and(|r| !r.is_empty()), has_deny_rules: endpoint.deny_rules.as_ref().is_some_and(|r| !r.is_empty()), rules_would_deny_all: endpoint.rules.as_ref().is_some_and(|r| { @@ -3625,6 +3626,34 @@ credentials: })); } + #[test] + fn profile_lint_rejects_unsupported_tls_value() { + let profile = parse_profile_yaml( + r" +id: legacy-tls +display_name: Legacy TLS +category: other +endpoints: + - host: api.example.com + port: 443 + protocol: rest + access: read-only + tls: terminate +", + ) + .expect("profile should parse"); + let diagnostics = validate_profile_set(&[("legacy.yaml".to_string(), profile)]); + assert!( + diagnostics.iter().any(|diagnostic| { + diagnostic.field == "endpoints[0]" + && diagnostic + .message + .contains("unsupported tls value 'terminate'") + }), + "expected a tls diagnostic: {diagnostics:?}" + ); + } + #[test] fn builtin_agent_conversation_defaults_preserve_own_and_foreign_body_text() { use openshell_core::proto::{StaticCredentialBinding, StaticCredentialEndpointBinding}; @@ -5481,7 +5510,6 @@ endpoints: - host: api.example.com ports: [443, 8443] protocol: rest - tls: terminate enforcement: enforce rules: - allow: @@ -5525,7 +5553,7 @@ binaries: let rest_ep = &proto.endpoints[1]; assert_eq!(rest_ep.port, 0); assert_eq!(rest_ep.ports, vec![443, 8443]); - assert_eq!(rest_ep.tls, "terminate"); + assert_eq!(rest_ep.tls, ""); assert_eq!(rest_ep.allowed_ips, vec!["10.0.0.0/24"]); assert!(rest_ep.allow_encoded_slash); assert!(rest_ep.allow_uninspected_credentials); diff --git a/crates/openshell-server/src/grpc/policy.rs b/crates/openshell-server/src/grpc/policy.rs index d986c5cba9..47004e84bd 100644 --- a/crates/openshell-server/src/grpc/policy.rs +++ b/crates/openshell-server/src/grpc/policy.rs @@ -9822,7 +9822,6 @@ mod tests { let mut policy = test_policy_with_rule("aws", host); let endpoint = &mut policy.network_policies.get_mut("aws").unwrap().endpoints[0]; endpoint.protocol = "rest".to_string(); - endpoint.tls = "terminate".to_string(); endpoint.access = "full".to_string(); endpoint.credential_signing = "sigv4".to_string(); endpoint.signing_service = "s3".to_string(); @@ -11791,7 +11790,6 @@ mod tests { .endpoints[0]; bound_endpoint.protocol = "rest".to_string(); bound_endpoint.access = "full".to_string(); - bound_endpoint.tls = "terminate".to_string(); openshell_policy::ensure_sandbox_process_identity(&mut policy); state .store diff --git a/crates/openshell-server/src/grpc/validation.rs b/crates/openshell-server/src/grpc/validation.rs index 8a81e2648c..891d38b0b4 100644 --- a/crates/openshell-server/src/grpc/validation.rs +++ b/crates/openshell-server/src/grpc/validation.rs @@ -2088,6 +2088,30 @@ mod tests { assert!(err.message().contains("root")); } + #[test] + fn validate_policy_safety_rejects_unsupported_endpoint_tls() { + use openshell_core::proto::{NetworkEndpoint, NetworkPolicyRule}; + + let mut policy = openshell_policy::restrictive_default_policy(); + policy.network_policies.insert( + "api".into(), + NetworkPolicyRule { + name: "api".into(), + endpoints: vec![NetworkEndpoint { + host: "api.example.com".into(), + port: 443, + tls: "terminate".into(), + ..Default::default() + }], + ..Default::default() + }, + ); + + let err = validate_policy_safety(&policy).unwrap_err(); + assert_eq!(err.code(), Code::InvalidArgument); + assert!(err.message().contains("unsupported tls value 'terminate'")); + } + #[test] fn validate_policy_safety_rejects_path_traversal() { use openshell_core::proto::FilesystemPolicy; diff --git a/crates/openshell-supervisor-network/src/l7/mod.rs b/crates/openshell-supervisor-network/src/l7/mod.rs index 2085b46681..6ce3c2fc35 100644 --- a/crates/openshell-supervisor-network/src/l7/mod.rs +++ b/crates/openshell-supervisor-network/src/l7/mod.rs @@ -297,34 +297,7 @@ pub fn parse_l7_config(val: ®orus::Value) -> Option { let protocol_val = get_object_str(val, "protocol")?; let protocol = L7Protocol::parse(&protocol_val)?; - let tls = match get_object_str(val, "tls").as_deref() { - Some("skip") => TlsMode::Skip, - Some("terminate") => { - let event = openshell_ocsf::NetworkActivityBuilder::new(openshell_ocsf::ctx::ctx()) - .activity(openshell_ocsf::ActivityId::Other) - .severity(openshell_ocsf::SeverityId::Medium) - .message( - "'tls: terminate' is deprecated; TLS termination is now automatic. \ - Use 'tls: skip' to explicitly disable. This field will be removed in a future version.", - ) - .build(); - openshell_ocsf::ocsf_emit!(event); - TlsMode::Auto - } - Some("passthrough") => { - let event = openshell_ocsf::NetworkActivityBuilder::new(openshell_ocsf::ctx::ctx()) - .activity(openshell_ocsf::ActivityId::Other) - .severity(openshell_ocsf::SeverityId::Medium) - .message( - "'tls: passthrough' is deprecated; TLS termination is now automatic. \ - Use 'tls: skip' to explicitly disable. This field will be removed in a future version.", - ) - .build(); - openshell_ocsf::ocsf_emit!(event); - TlsMode::Auto - } - _ => TlsMode::Auto, - }; + let tls = parse_tls_mode(val); let enforcement = match get_object_str(val, "enforcement").as_deref() { Some("enforce") => EnforcementMode::Enforce, @@ -457,10 +430,12 @@ pub fn endpoint_path_matches(pattern: &str, path: &str) -> bool { /// /// Used to check for `tls: skip` even on L4-only endpoints (no `protocol` /// field) that explicitly opt out of TLS auto-detection. +/// +/// Unsupported values fall through to `Auto`, which inspects, so an +/// unvalidated path still fails closed. pub fn parse_tls_mode(val: ®orus::Value) -> TlsMode { match get_object_str(val, "tls").as_deref() { Some("skip") => TlsMode::Skip, - // "terminate" and "passthrough" are deprecated aliases (logged by parse_l7_config); fall through to Auto. _ => TlsMode::Auto, } } @@ -1340,6 +1315,7 @@ pub fn validate_l7_policies(data_json: &serde_json::Value) -> (Vec, Vec< let l7_fields = L7EndpointFields { protocol, access, + tls, has_rules, has_deny_rules, rules_would_deny_all, @@ -1477,13 +1453,6 @@ pub fn validate_l7_policies(data_json: &serde_json::Value) -> (Vec, Vec< } } - // Deprecated tls values: warn but don't error - if tls == "terminate" || tls == "passthrough" { - warnings.push(format!( - "{loc}: 'tls: {tls}' is deprecated; TLS termination is now automatic. Use 'tls: skip' to disable." - )); - } - // tls: skip with L7 on port 443 won't work if tls == "skip" && !protocol.is_empty() && ports.contains(&443) { warnings.push(format!( @@ -1498,10 +1467,6 @@ pub fn validate_l7_policies(data_json: &serde_json::Value) -> (Vec, Vec< )); } - // port 443 + rest + tls: skip — L7 won't work (already handled above) - // The old warning about missing `tls: terminate` is no longer needed - // because TLS termination is now automatic. - // Per-rule deny_rules validation (semantic checks handled by // shared validator above). if has_deny_rules { @@ -1941,12 +1906,11 @@ mod tests { #[test] fn parse_l7_config_rest_enforce() { let val = regorus::Value::from_json_str( - r#"{"protocol": "rest", "tls": "terminate", "enforcement": "enforce", "host": "api.example.com", "port": 443}"#, + r#"{"protocol": "rest", "enforcement": "enforce", "host": "api.example.com", "port": 443}"#, ) .unwrap(); let config = parse_l7_config(&val).unwrap(); assert_eq!(config.protocol, L7Protocol::Rest); - // "terminate" is deprecated and treated as Auto. assert_eq!(config.tls, TlsMode::Auto); assert_eq!(config.enforcement, EnforcementMode::Enforce); } @@ -3398,33 +3362,53 @@ mod tests { assert!(errors.iter().any(|e| e.contains("SQL enforcement"))); } - #[test] - fn validate_tls_terminate_deprecated_warning() { - let data = serde_json::json!({ + fn policy_data_with_tls(tls: &str) -> serde_json::Value { + serde_json::json!({ "network_policies": { "test": { "endpoints": [{ "host": "api.example.com", "port": 443, - "tls": "terminate", + "tls": tls, "protocol": "rest", "access": "full" }], "binaries": [] } } - }); - let (errors, warnings) = validate_l7_policies(&data); + }) + } + + /// The issue's acceptance criterion: a legacy value is an error naming the + /// endpoint, and no deprecation warning survives anywhere. + #[test] + fn validate_rejects_legacy_tls_without_warning() { + let (errors, warnings) = validate_l7_policies(&policy_data_with_tls("terminate")); assert!( - errors.is_empty(), - "deprecated tls should not error: {errors:?}" + errors.iter().any(|e| e.contains("test.endpoints[0]") + && e.contains("unsupported tls value 'terminate'")), + "should be rejected: {errors:?}" ); assert!( - warnings.iter().any(|w| w.contains("deprecated")), - "should warn about deprecated tls: {warnings:?}" + !warnings.iter().any(|w| w.contains("deprecated")), + "no deprecation warning should survive: {warnings:?}" ); } + #[test] + fn accepted_tls_values_reach_their_intended_mode() { + for (tls, expected) in [("", TlsMode::Auto), ("skip", TlsMode::Skip)] { + let (errors, _) = validate_l7_policies(&policy_data_with_tls(tls)); + assert!( + !errors.iter().any(|e| e.contains("unsupported tls value")), + "tls: {tls:?} must validate: {errors:?}" + ); + + let val = regorus::Value::from_json_str(&format!(r#"{{"tls": "{tls}"}}"#)).unwrap(); + assert_eq!(parse_tls_mode(&val), expected, "tls: {tls:?}"); + } + } + #[test] fn validate_tls_skip_with_l7_on_443_warns() { let data = serde_json::json!({ diff --git a/crates/openshell-supervisor-network/src/l7/rest.rs b/crates/openshell-supervisor-network/src/l7/rest.rs index bf5c99e140..16b42b24c1 100644 --- a/crates/openshell-supervisor-network/src/l7/rest.rs +++ b/crates/openshell-supervisor-network/src/l7/rest.rs @@ -7427,12 +7427,12 @@ mod tests { /// placeholders in request headers before forwarding to upstream. /// /// This is the code path exercised when an endpoint has `protocol: rest` - /// and `tls: terminate` — the proxy terminates TLS, sees plaintext HTTP, - /// and replaces placeholder tokens with real secrets. + /// and terminated TLS — the proxy sees plaintext HTTP and replaces + /// placeholder tokens with real secrets. /// - /// Without this test, a misconfigured endpoint (missing `tls: terminate`) - /// silently leaks placeholder strings like `openshell:resolve:env:NVIDIA_API_KEY` - /// to the upstream API, causing 401 Unauthorized errors. + /// A regression here forwards placeholder strings like + /// `openshell:resolve:env:NVIDIA_API_KEY` to the upstream API verbatim, + /// causing 401 Unauthorized errors. #[tokio::test] async fn relay_request_with_resolver_rewrites_credential_placeholders() { let provider_env: HashMap = [( diff --git a/crates/openshell-supervisor-network/src/opa.rs b/crates/openshell-supervisor-network/src/opa.rs index 34fc81a6e2..455ee79224 100644 --- a/crates/openshell-supervisor-network/src/opa.rs +++ b/crates/openshell-supervisor-network/src/opa.rs @@ -8290,6 +8290,48 @@ network_policies: ); } + #[test] + fn unsupported_tls_value_fails_policy_load() { + for tls in ["terminate", "passthrough", "bogus"] { + let data = format!( + r#" +network_policies: + legacy_tls: + name: legacy_tls + endpoints: + - host: api.example.com + port: 443 + protocol: rest + access: read-only + tls: {tls} + binaries: + - {{ path: /usr/bin/curl }} +filesystem_policy: + include_workdir: true + read_only: [] + read_write: [] +landlock: + compatibility: best_effort +process: + run_as_user: sandbox + run_as_group: sandbox +"# + ); + let Err(error) = OpaEngine::from_strings(TEST_POLICY, &data) else { + panic!("tls: {tls} must reject the policy"); + }; + let error = error.to_string(); + assert!( + error.contains("L7 policy validation failed"), + "tls: {tls} should fail policy load: {error}" + ); + assert!( + error.contains("legacy_tls.endpoints[0]") && error.contains("remove the tls field"), + "tls: {tls} error should locate the endpoint and state the fix: {error}" + ); + } + } + #[test] fn wildcard_host_l7_rules_apply() { let data = r#" @@ -8301,7 +8343,6 @@ network_policies: port: 8080 protocol: rest enforcement: enforce - tls: terminate rules: - allow: method: GET @@ -8344,7 +8385,6 @@ network_policies: port: 8080 protocol: rest enforcement: enforce - tls: terminate rules: - allow: method: GET @@ -8392,7 +8432,6 @@ network_policies: ports: [8080, 9090] protocol: rest enforcement: enforce - tls: terminate rules: - allow: method: GET diff --git a/crates/openshell-supervisor-network/src/policy_local.rs b/crates/openshell-supervisor-network/src/policy_local.rs index 77ed3e14da..25e0594200 100644 --- a/crates/openshell-supervisor-network/src/policy_local.rs +++ b/crates/openshell-supervisor-network/src/policy_local.rs @@ -1131,6 +1131,9 @@ fn network_endpoint_from_json( { return Err(reason.to_string()); } + if let Some(reason) = openshell_policy::validate_tls_mode(&endpoint.tls) { + return Err(reason); + } let mut ports = endpoint.ports; if ports.is_empty() && endpoint.port > 0 { @@ -1424,7 +1427,6 @@ mod tests { "host": "api.github.com", "port": 443, "protocol": "rest", - "tls": "terminate", "enforcement": "enforce", "rules": [ { @@ -1522,6 +1524,30 @@ mod tests { } } + #[test] + fn proposal_chunks_from_body_rejects_unsupported_tls_values() { + for tls in ["terminate", "passthrough", "bogus"] { + let body = format!( + r#"{{ + "operations": [{{ + "addRule": {{ + "ruleName": "legacy_tls", + "rule": {{ + "endpoints": [{{"host":"api.example.com","port":443,"tls":"{tls}"}}] + }} + }} + }}] + }}"# + ); + + let error = proposal_chunks_from_body(body.as_bytes()).unwrap_err(); + assert!( + error.contains(&format!("unsupported tls value '{tls}'")), + "tls: {tls} unexpected error: {error}" + ); + } + } + #[test] fn proposal_chunks_from_body_accepts_omitted_protocol_with_default_tls() { let body = br#"{ diff --git a/crates/openshell-tui/src/lib.rs b/crates/openshell-tui/src/lib.rs index 3d451e13c8..30a2cb6673 100644 --- a/crates/openshell-tui/src/lib.rs +++ b/crates/openshell-tui/src/lib.rs @@ -1313,7 +1313,6 @@ fn render_policy_lines( // Rule header — include L7/TLS/allowed_ips annotation if any endpoint has it. let has_l7 = rule.endpoints.iter().any(|e| !e.protocol.is_empty()); - let has_tls_term = rule.endpoints.iter().any(|e| e.tls == "terminate"); let has_allowed_ips = rule.endpoints.iter().any(|e| !e.allowed_ips.is_empty()); let mut annotations = Vec::new(); if has_l7 { @@ -1327,9 +1326,6 @@ fn render_policy_lines( annotations.push(format!("L7 {proto}")); } } - if has_tls_term { - annotations.push("TLS terminate".to_string()); - } if has_allowed_ips { annotations.push("private IP".to_string()); } diff --git a/docs/about/release-notes.mdx b/docs/about/release-notes.mdx index c5f518fd79..dea65a669d 100644 --- a/docs/about/release-notes.mdx +++ b/docs/about/release-notes.mdx @@ -18,6 +18,16 @@ OpenShell 0.1.0 removes the deprecated `NetworkBinary.harness` protobuf field an Remove `harness` from sandbox policies and provider profiles before upgrading. Policy and profile YAML now reject the property. Provider profiles should list binaries as scalar paths, such as `- /usr/bin/curl`; the transitional object form `- path: /usr/bin/curl` remains accepted and is exported as a scalar. +### Network endpoint TLS handling + +OpenShell 0.1.0 closes the network endpoint `tls` field to two forms: omit it to auto-detect and terminate TLS for inspection, or set `skip` to stop inspecting that endpoint. The gateway, the sandbox supervisor, provider profile lint, and agent-authored policy proposals all reject any other value, including the former `terminate` and `passthrough` spellings. Those values previously logged a deprecation warning on every sandbox start and had no effect on behavior. + +Remove the `tls` field from affected endpoints before upgrading. Removal preserves behavior, because `terminate` and `passthrough` already selected automatic termination. + +Do not substitute `tls: skip` for a removed `terminate`. `skip` stops TLS inspection, credential placeholder rewriting, dynamic token grant injection, and L7 rule enforcement for that endpoint, so it widens what the endpoint allows. + +A stored global or sandbox policy that carries a removed `tls` value fails validation on read, so `GetSandboxConfig` returns `FAILED_PRECONDITION` and affected sandboxes cannot start. Run `openshell policy list --global` to see which revision is invalid and why, then re-apply a corrected policy with `openshell policy set --global`. + | Resource | Description | |---|---| | [Releases](https://github.com/NVIDIA/OpenShell/releases) | Versioned release notes and downloadable assets. | diff --git a/docs/reference/policy-schema.mdx b/docs/reference/policy-schema.mdx index a721718e2e..468769476d 100644 --- a/docs/reference/policy-schema.mdx +++ b/docs/reference/policy-schema.mdx @@ -194,7 +194,7 @@ Each endpoint defines a reachable destination and optional inspection rules. | `port` | integer | Yes | TCP port number. | | `path` | string | No | Optional HTTP path glob used to select between L7 endpoints that share the same host and port. Empty means all paths. Use this when REST and GraphQL live under the same host, such as `/repos/**` and `/graphql`. | | `protocol` | string | No | Set to `tcp` with a valid DNS hostname to allow native TCP clients through policy DNS and transparent capture without payload inspection. Omit the field for L4 passthrough through an explicit proxy, including legacy hostless `allowed_ips` endpoints. Set to `rest` for HTTP method/path inspection, `websocket` for RFC 6455 upgrade and client text-message inspection, `graphql` for GraphQL-over-HTTP operation inspection, `mcp` for MCP Streamable HTTP request inspection, or `json-rpc` for generic JSON-RPC-over-HTTP method inspection. WebSocket endpoints can also use GraphQL operation rules for GraphQL-over-WebSocket traffic. Provider-credentialed endpoints require an inspected protocol unless `allow_uninspected_credentials` is explicitly set. | -| `tls` | string | No | TLS handling mode. The proxy auto-detects TLS by peeking the first bytes of each connection and terminates it for inspected HTTPS traffic, so this field is optional in most cases. Set to `skip` to disable auto-detection for edge cases such as client-certificate mTLS or non-standard protocols. Provider-credentialed endpoints reject `tls: skip` unless `allow_uninspected_credentials` is explicitly set. The values `terminate` and `passthrough` are deprecated and log a warning; they are still accepted for backward compatibility but have no effect on behavior. | +| `tls` | string | No | TLS handling mode. Omit the field to auto-detect TLS by peeking the first bytes of each connection and terminate it for inspected HTTPS traffic. Set to `skip` to disable auto-detection for edge cases such as client-certificate mTLS or non-standard protocols. Provider-credentialed endpoints reject `tls: skip` unless `allow_uninspected_credentials` is explicitly set. `skip` is the only accepted non-empty value; any other value, including the removed `terminate` and `passthrough` spellings, fails validation. | | `enforcement` | string | No | `enforce` actively blocks disallowed requests. `audit` logs violations but allows traffic through. | | `access` | string | No | Access preset. One of `read-only`, `read-write`, or `full`. Mutually exclusive with `rules`. Not valid on `protocol: mcp` or `protocol: json-rpc`; MCP uses explicit rules unless `mcp.allow_all_known_mcp_methods: true` enables the endpoint method profile, and JSON-RPC always uses explicit rules. | | `rules` | list of allow rule objects | No | Fine-grained protocol-specific allow rules. Mutually exclusive with `access`. | diff --git a/e2e/python/test_sandbox_providers.py b/e2e/python/test_sandbox_providers.py index 1f2a7e4aa8..beee2e5d2b 100644 --- a/e2e/python/test_sandbox_providers.py +++ b/e2e/python/test_sandbox_providers.py @@ -185,7 +185,6 @@ def _native_inference_profile( host="host.openshell.internal", port=port, protocol="rest", - tls="none", enforcement="enforce", rules=rules, allowed_ips=[ diff --git a/e2e/rust/tests/provider_token_exchange.rs b/e2e/rust/tests/provider_token_exchange.rs index c4e06f029f..c2af5090a9 100644 --- a/e2e/rust/tests/provider_token_exchange.rs +++ b/e2e/rust/tests/provider_token_exchange.rs @@ -583,7 +583,6 @@ endpoints: - host: host.openshell.internal port: {target_port} protocol: rest - tls: none access: read-write enforcement: enforce allowed_ips: diff --git a/examples/sandbox-policy-quickstart/README.md b/examples/sandbox-policy-quickstart/README.md index ce6b16bfb3..dd19bc14e2 100644 --- a/examples/sandbox-policy-quickstart/README.md +++ b/examples/sandbox-policy-quickstart/README.md @@ -98,7 +98,6 @@ network_policies: - host: api.github.com port: 443 protocol: rest - tls: terminate enforcement: enforce access: read-only binaries: @@ -111,7 +110,7 @@ select it. These settings are required because `policy set` replaces the entire policy. The `network_policies` section is the interesting part: **curl may make GET, HEAD, and OPTIONS requests to `api.github.com` over HTTPS. -Everything else is denied.** The proxy terminates TLS (`tls: terminate`) +Everything else is denied.** The proxy auto-detects and terminates TLS to inspect each HTTP request and enforce the `read-only` access preset at the method level. diff --git a/examples/spiffe-token-exchange-demo/podman/provider-profile.yaml b/examples/spiffe-token-exchange-demo/podman/provider-profile.yaml index b7d1cf30e5..92f130fb92 100644 --- a/examples/spiffe-token-exchange-demo/podman/provider-profile.yaml +++ b/examples/spiffe-token-exchange-demo/podman/provider-profile.yaml @@ -44,13 +44,11 @@ endpoints: - host: alpha-exchange port: 8080 protocol: rest - tls: none access: read-write enforcement: enforce - host: beta-exchange port: 8080 protocol: rest - tls: none access: read-write enforcement: enforce binaries: diff --git a/examples/spiffe-token-exchange-demo/provider-profile.yaml b/examples/spiffe-token-exchange-demo/provider-profile.yaml index 48e5dbe6c8..41b2d76406 100644 --- a/examples/spiffe-token-exchange-demo/provider-profile.yaml +++ b/examples/spiffe-token-exchange-demo/provider-profile.yaml @@ -39,13 +39,11 @@ endpoints: - host: alpha-exchange.default.svc.cluster.local port: 80 protocol: rest - tls: none access: read-write enforcement: enforce - host: beta-exchange.default.svc.cluster.local port: 80 protocol: rest - tls: none access: read-write enforcement: enforce binaries: diff --git a/examples/spiffe-token-grant-demo/provider-profile.yaml b/examples/spiffe-token-grant-demo/provider-profile.yaml index caadfe2b3a..8950d600aa 100644 --- a/examples/spiffe-token-grant-demo/provider-profile.yaml +++ b/examples/spiffe-token-grant-demo/provider-profile.yaml @@ -31,7 +31,6 @@ endpoints: - host: alpha.default.svc.cluster.local port: 80 protocol: rest - tls: none access: read-write enforcement: enforce allowed_ips: @@ -39,7 +38,6 @@ endpoints: - host: beta.default.svc.cluster.local port: 80 protocol: rest - tls: none access: read-write enforcement: enforce allowed_ips: diff --git a/proto/sandbox.proto b/proto/sandbox.proto index 600f4a750b..470e68a4d4 100644 --- a/proto/sandbox.proto +++ b/proto/sandbox.proto @@ -113,7 +113,9 @@ message NetworkEndpoint { // Endpoint protocol. "tcp" and "" select L4-only handling; "rest", // "websocket", "graphql", "sql", "json-rpc", and "mcp" select L7 inspection. string protocol = 3; - // TLS handling: "terminate" or "passthrough" (default). + // TLS handling. Empty (default) auto-detects and terminates TLS for + // inspection; "skip" disables detection for this endpoint. Any other value + // is rejected at validation. string tls = 4; // Enforcement mode: "enforce" or "audit" (default). string enforcement = 5; diff --git a/providers/aws-s3.yaml b/providers/aws-s3.yaml index f51ffedb9a..ef828126b4 100644 --- a/providers/aws-s3.yaml +++ b/providers/aws-s3.yaml @@ -66,7 +66,6 @@ endpoints: - host: "*.s3.amazonaws.com" port: 443 protocol: rest - tls: terminate access: read-write enforcement: enforce credential_signing: sigv4 @@ -74,7 +73,6 @@ endpoints: - host: "s3.amazonaws.com" port: 443 protocol: rest - tls: terminate access: read-write enforcement: enforce credential_signing: sigv4 @@ -82,7 +80,6 @@ endpoints: - host: "*.s3.*.amazonaws.com" port: 443 protocol: rest - tls: terminate access: read-write enforcement: enforce credential_signing: sigv4 @@ -90,7 +87,6 @@ endpoints: - host: "s3.*.amazonaws.com" port: 443 protocol: rest - tls: terminate access: read-write enforcement: enforce credential_signing: sigv4 @@ -98,7 +94,6 @@ endpoints: - host: "*.s3.dualstack.*.amazonaws.com" port: 443 protocol: rest - tls: terminate access: read-write enforcement: enforce credential_signing: sigv4 @@ -106,7 +101,6 @@ endpoints: - host: "s3.dualstack.*.amazonaws.com" port: 443 protocol: rest - tls: terminate access: read-write enforcement: enforce credential_signing: sigv4 diff --git a/sdk/go/proto/sandboxv1/sandbox.pb.go b/sdk/go/proto/sandboxv1/sandbox.pb.go index 6cbbd5fbda..40375e0acd 100644 --- a/sdk/go/proto/sandboxv1/sandbox.pb.go +++ b/sdk/go/proto/sandboxv1/sandbox.pb.go @@ -656,7 +656,9 @@ type NetworkEndpoint struct { // Endpoint protocol. "tcp" and "" select L4-only handling; "rest", // "websocket", "graphql", "sql", "json-rpc", and "mcp" select L7 inspection. Protocol string `protobuf:"bytes,3,opt,name=protocol,proto3" json:"protocol,omitempty"` - // TLS handling: "terminate" or "passthrough" (default). + // TLS handling. Empty (default) auto-detects and terminates TLS for + // inspection; "skip" disables detection for this endpoint. Any other value + // is rejected at validation. Tls string `protobuf:"bytes,4,opt,name=tls,proto3" json:"tls,omitempty"` // Enforcement mode: "enforce" or "audit" (default). Enforcement string `protobuf:"bytes,5,opt,name=enforcement,proto3" json:"enforcement,omitempty"` diff --git a/skills/generate-sandbox-policy/SKILL.md b/skills/generate-sandbox-policy/SKILL.md index 73c0863df7..31afb8c233 100644 --- a/skills/generate-sandbox-policy/SKILL.md +++ b/skills/generate-sandbox-policy/SKILL.md @@ -199,13 +199,9 @@ Is L7 inspection needed? ### TLS Decision -| API host port | TLS setting | -|--------------|-------------| -| Port 443 (HTTPS) and L7 rules/preset needed | `tls: terminate` (required for inspection) | -| Port 443 (HTTPS) and L4-only | Omit `tls` (passthrough, no L7); choose omitted protocol or explicit TCP based on client/runtime as above | -| Non-443 (HTTP) | Omit `tls` | +Omit `tls` on every endpoint, regardless of port: the proxy auto-detects TLS and terminates it for inspection. `skip` is the only accepted non-empty value, reserved for upstreams requiring client-certificate mTLS or a non-HTTP protocol. -**Critical**: `protocol: rest` on port 443 without `tls: terminate` will not work — the proxy cannot inspect encrypted traffic. Always set `tls: terminate` when combining port 443 with L7 rules. +Any other value — including the removed `terminate` and `passthrough` spellings — fails policy validation, and the gateway rejects such a policy outright. Do not "fix" a rejected value by changing it to `skip`; remove the field instead. `skip` stops inspection, credential injection, and L7 rule enforcement for that endpoint, so it silently widens what the endpoint allows. ### Middleware Decision @@ -269,7 +265,6 @@ network_policies: - host: port: protocol: rest # Required for L7 inspection - tls: terminate # Required for HTTPS + L7 enforcement: enforce # or audit # Use ONE of: access OR rules (never both) access: # read-only | read-write | full @@ -373,7 +368,7 @@ Before presenting the policy to the user, verify correctness **and** flag breadt `protocol: tcp` is L4-only and must not contain either field - [ ] Every `protocol: tcp` endpoint has a valid DNS hostname; it is not hostless, an IP literal, a trailing-dot name, or a malformed DNS selector -- [ ] If `tls: terminate` is set, `protocol` is also set +- [ ] `tls` is either omitted or set to `skip`; no other value is accepted - [ ] `rules` list is not empty when present - [ ] If `protocol: sql`, `enforcement` is not `enforce` - [ ] Every middleware config has a non-empty `middleware` name and non-empty `endpoints.include` @@ -384,7 +379,7 @@ Before presenting the policy to the user, verify correctness **and** flag breadt ### Schema Warnings (log-only, but should be fixed) -- [ ] `protocol: rest` on port 443 should have `tls: terminate` +- [ ] `tls: skip` is not combined with L7 rules on port 443; inspection cannot work on encrypted traffic - [ ] HTTP methods are standard: GET, HEAD, POST, PUT, DELETE, PATCH, OPTIONS, or `*` - [ ] Credentialed destinations are also covered by the attached provider profile endpoint; policy admission alone does not authorize credential @@ -551,7 +546,6 @@ my_api_readonly: - host: api.example.com port: 443 protocol: rest - tls: terminate enforcement: enforce access: read-only binaries: @@ -567,7 +561,6 @@ my_api_custom: - host: api.example.com port: 443 protocol: rest - tls: terminate enforcement: enforce rules: - allow: diff --git a/skills/generate-sandbox-policy/examples.md b/skills/generate-sandbox-policy/examples.md index 2cbc21b6b6..f5475a6c2a 100644 --- a/skills/generate-sandbox-policy/examples.md +++ b/skills/generate-sandbox-policy/examples.md @@ -3,11 +3,10 @@ Examples organized by detail tier — from minimal (just host + intent) to full (complete API docs). > **TLS note:** TLS termination is automatic. The proxy auto-detects TLS by -> peeking the first bytes of each connection, so there is no need to specify -> `tls: terminate` in policies. The `tls: terminate` and `tls: passthrough` -> values are deprecated. If you have an edge case where auto-detection must -> be bypassed, you can set `tls: skip` to disable TLS interception for that -> endpoint. +> peeking the first bytes of each connection, so omit `tls` on inspected +> endpoints. `skip` is the only accepted value and disables TLS interception +> for that endpoint; the removed `terminate` and `passthrough` spellings fail +> policy validation. ---