Skip to content

Commit d94e073

Browse files
committed
fix(policy)!: reject removed tls endpoint values
Signed-off-by: Yuedong Wu <dwcn22@outlook.com>
1 parent 5023061 commit d94e073

23 files changed

Lines changed: 145 additions & 179 deletions

File tree

‎architecture/security-policy.md‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -242,8 +242,11 @@ higher specificity rank deterministically overrides broader request-processing
242242
metadata. Equally specific overlapping endpoints must agree.
243243

244244
Endpoint `tls`, `enforcement`, and `access` use protobuf enums and retain their
245-
named YAML spellings. `protocol` remains a string so the supported protocol set
246-
can evolve, but every ingress validates it before persistence or activation.
245+
named YAML spellings. `tls` admits only an omitted value, meaning auto-detect
246+
and terminate for inspection, or `skip`; every other value, including the
247+
removed `terminate` and `passthrough` spellings, is rejected. `protocol`
248+
remains a string so the supported protocol set can evolve, but every ingress
249+
validates it before persistence or activation.
247250
The supervisor also refuses unknown enum numbers and protocol values
248251
defensively; an unrecognized enforcement value never falls back to audit.
249252

‎crates/openshell-cli/src/commands/provider.rs‎

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2909,13 +2909,7 @@ binaries: [/usr/bin/curl]
29092909
)
29102910
.expect("valid profile fixture");
29112911
let mut descriptions = Vec::new();
2912-
for (tls, allow_uninspected_credentials) in [
2913-
("", false),
2914-
("", true),
2915-
("skip", true),
2916-
("terminate", false),
2917-
("passthrough", false),
2918-
] {
2912+
for (tls, allow_uninspected_credentials) in [("", false), ("", true), ("skip", true)] {
29192913
let mut variant = profile.clone();
29202914
variant.endpoints[0].tls = tls.to_string();
29212915
variant.endpoints[0].allow_uninspected_credentials = allow_uninspected_credentials;
@@ -2947,8 +2941,6 @@ binaries: [/usr/bin/curl]
29472941
.contains("TLS: skip (raw tunnel; no L7 inspection or credential rewrite)\n")
29482942
);
29492943
assert!(descriptions[2].contains("Allow uninspected credentials: true\n"));
2950-
assert!(descriptions[3].contains("TLS: terminate (automatic TLS detection)\n"));
2951-
assert!(descriptions[4].contains("TLS: passthrough (automatic TLS detection)\n"));
29522944
}
29532945

29542946
#[test]

‎crates/openshell-cli/tests/provider_commands_integration.rs‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4863,7 +4863,6 @@ endpoints:
48634863
- host: api.advanced.example
48644864
ports: [443, 8443]
48654865
protocol: rest
4866-
tls: terminate
48674866
enforcement: enforce
48684867
rules:
48694868
- allow:

‎crates/openshell-policy/src/l7_validate.rs‎

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -68,10 +68,17 @@ pub fn network_access_preset_to_str(value: i32) -> Option<&'static str> {
6868
}
6969
}
7070

71+
fn unknown_tls_value(tls: &str) -> Option<String> {
72+
(!matches!(tls, "" | "skip")).then(|| {
73+
format!("unknown tls value '{tls}'; omit the field to keep automatic TLS termination")
74+
})
75+
}
76+
7177
pub fn validate_endpoint_mode_values(tls: i32, enforcement: i32, access: i32) -> Vec<String> {
7278
let mut errors = Vec::new();
73-
if network_tls_mode_to_str(tls).is_none() {
74-
errors.push(format!("unknown tls enum value {tls}"));
79+
match network_tls_mode_to_str(tls) {
80+
Some(value) => errors.extend(unknown_tls_value(value)),
81+
None => errors.push(format!("unknown tls enum value {tls}")),
7582
}
7683
if network_enforcement_mode_to_str(enforcement).is_none() {
7784
errors.push(format!("unknown enforcement enum value {enforcement}"));
@@ -174,7 +181,7 @@ mod agent_transport_tests {
174181
#[test]
175182
fn agent_cannot_request_native_tcp_or_skip_tls_inspection() {
176183
assert!(agent_authored_transport_rejection("tcp", "").is_some());
177-
assert!(agent_authored_transport_rejection("TCP", "terminate").is_some());
184+
assert!(agent_authored_transport_rejection("TCP", "").is_some());
178185
assert!(agent_authored_transport_rejection("", "skip").is_some());
179186
assert!(agent_authored_transport_rejection("rest", "SKIP").is_some());
180187
}
@@ -185,11 +192,7 @@ mod agent_transport_tests {
185192
pub fn validate_endpoint_modes(tls: &str, enforcement: &str, access: &str) -> Vec<String> {
186193
let mut errors = Vec::new();
187194

188-
if !matches!(tls, "" | "skip" | "terminate" | "passthrough") {
189-
errors.push(format!(
190-
"unknown tls value '{tls}' (expected skip, terminate, or passthrough)"
191-
));
192-
}
195+
errors.extend(unknown_tls_value(tls));
193196
if !matches!(enforcement, "" | "enforce" | "audit") {
194197
errors.push(format!(
195198
"unknown enforcement value '{enforcement}' (expected enforce or audit)"
@@ -357,9 +360,18 @@ mod tests {
357360
assert!(errors[2].contains("unknown access value 'read-wirte'"));
358361
}
359362

363+
#[test]
364+
fn endpoint_mode_values_reject_removed_tls_enums() {
365+
for legacy in [2, 3] {
366+
let errors = validate_endpoint_mode_values(legacy, 0, 0);
367+
assert_eq!(errors.len(), 1, "tls: {legacy}");
368+
assert!(errors[0].contains("unknown tls value"));
369+
}
370+
}
371+
360372
#[test]
361373
fn endpoint_modes_accept_documented_values_and_defaults() {
362-
for tls in ["", "skip", "terminate", "passthrough"] {
374+
for tls in ["", "skip"] {
363375
for enforcement in ["", "enforce", "audit"] {
364376
for access in ["", "read-only", "read-write", "full"] {
365377
assert!(validate_endpoint_modes(tls, enforcement, access).is_empty());

‎crates/openshell-policy/src/merge.rs‎

Lines changed: 1 addition & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -833,9 +833,7 @@ fn endpoint_attributes_cover(loaded: &NetworkEndpoint, proposed: &NetworkEndpoin
833833
if !proposed.protocol.is_empty() && !protocols_match(&loaded.protocol, &proposed.protocol) {
834834
return false;
835835
}
836-
if proposed.tls != NetworkTlsMode::Unspecified as i32
837-
&& effective_tls(loaded.tls) != effective_tls(proposed.tls)
838-
{
836+
if proposed.tls != NetworkTlsMode::Unspecified as i32 && loaded.tls != proposed.tls {
839837
return false;
840838
}
841839
if proposed.enforcement != NetworkEnforcementMode::Unspecified as i32
@@ -940,19 +938,6 @@ fn protocols_match(left: &str, right: &str) -> bool {
940938
}
941939
}
942940

943-
#[allow(deprecated)]
944-
fn effective_tls(value: i32) -> i32 {
945-
match value {
946-
value
947-
if value == NetworkTlsMode::Terminate as i32
948-
|| value == NetworkTlsMode::Passthrough as i32 =>
949-
{
950-
NetworkTlsMode::Unspecified as i32
951-
}
952-
value => value,
953-
}
954-
}
955-
956941
fn effective_enforcement(value: i32) -> i32 {
957942
if value == NetworkEnforcementMode::Unspecified as i32 {
958943
NetworkEnforcementMode::Audit as i32
@@ -3783,7 +3768,6 @@ mod tests {
37833768
assert!(!policy_covers_rule(&loaded, &different_body));
37843769

37853770
let mut explicit_defaults = loaded_endpoint;
3786-
explicit_defaults.tls = 3; // deprecated passthrough compatibility value
37873771
explicit_defaults.enforcement = NetworkEnforcementMode::Audit as i32;
37883772
let runtime_defaults = rule_with_authorizations(
37893773
"proposed",
@@ -3792,14 +3776,6 @@ mod tests {
37923776
);
37933777
assert!(policy_covers_rule(&loaded, &runtime_defaults));
37943778

3795-
explicit_defaults.tls = 2; // deprecated terminate compatibility value
3796-
let legacy_terminate = rule_with_authorizations(
3797-
"proposed",
3798-
vec![explicit_defaults.clone()],
3799-
&["/usr/bin/client"],
3800-
);
3801-
assert!(policy_covers_rule(&loaded, &legacy_terminate));
3802-
38033779
explicit_defaults.tls = NetworkTlsMode::Skip as i32;
38043780
let skip_tls = rule_with_authorizations(
38053781
"proposed",

‎crates/openshell-prover/src/containment.rs‎

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1596,7 +1596,7 @@ fn validate_supported_endpoint_extensions(
15961596
context: &str,
15971597
endpoint: &Endpoint,
15981598
) -> Result<(), UnsupportedFeature> {
1599-
if !matches!(endpoint.tls.as_str(), "" | "terminate" | "passthrough")
1599+
if !endpoint.tls.is_empty()
16001600
|| endpoint.allow_encoded_slash
16011601
|| endpoint.websocket_credential_rewrite
16021602
|| endpoint.request_body_credential_rewrite
@@ -3184,17 +3184,20 @@ network_policies:
31843184
}
31853185

31863186
#[test]
3187-
fn deprecated_tls_spelling_does_not_change_authority() {
3188-
let boundary = parse(
3189-
"version: 1\nnetwork_policies:\n n:\n endpoints:\n - host: api.example.com\n port: 443\n protocol: rest\n tls: terminate\n enforcement: enforce\n access: read-only\n binaries: [{ path: /usr/bin/curl }]\n",
3190-
);
3191-
let candidate = parse(
3192-
"version: 1\nnetwork_policies:\n n:\n endpoints:\n - host: api.example.com\n port: 443\n protocol: rest\n enforcement: enforce\n rules:\n - allow: { method: GET, path: '/v1/**' }\n binaries: [{ path: /usr/bin/curl }]\n",
3193-
);
3194-
assert!(matches!(
3195-
check_within_boundary(&boundary, &candidate, options()),
3196-
CheckResult::Within(_)
3197-
));
3187+
fn removed_tls_spelling_is_outside_the_authority_model() {
3188+
for tls in ["terminate", "passthrough"] {
3189+
let policy = parse(&format!(
3190+
"version: 1\nnetwork_policies:\n n:\n endpoints:\n - host: api.example.com\n port: 443\n protocol: rest\n tls: {tls}\n enforcement: enforce\n access: read-only\n binaries: [{{ path: /usr/bin/curl }}]\n"
3191+
));
3192+
assert!(
3193+
matches!(
3194+
check_within_boundary(&policy, &policy, options()),
3195+
CheckResult::Unsupported(ref evidence)
3196+
if evidence.reason().contains("outside the initial model")
3197+
),
3198+
"tls: {tls}"
3199+
);
3200+
}
31983201
}
31993202

32003203
#[test]

‎crates/openshell-providers/src/profiles.rs‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5516,7 +5516,6 @@ endpoints:
55165516
- host: api.example.com
55175517
ports: [443, 8443]
55185518
protocol: rest
5519-
tls: terminate
55205519
enforcement: enforce
55215520
rules:
55225521
- allow:
@@ -5563,10 +5562,6 @@ binaries:
55635562
let rest_ep = &proto.endpoints[1];
55645563
assert_eq!(rest_ep.port, 0);
55655564
assert_eq!(rest_ep.ports, vec![443, 8443]);
5566-
assert_eq!(
5567-
rest_ep.tls,
5568-
openshell_core::proto::NetworkTlsMode::Terminate as i32
5569-
);
55705565
assert_eq!(rest_ep.allowed_ips, vec!["10.0.0.0/24"]);
55715566
assert!(rest_ep.allow_encoded_slash);
55725567
assert!(rest_ep.allow_uninspected_credentials);

‎crates/openshell-server/src/grpc/policy.rs‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10019,14 +10019,12 @@ mod tests {
1001910019
}
1002010020
}
1002110021

10022-
#[allow(deprecated)]
1002310022
fn l7_scope_policy() -> ProtoSandboxPolicy {
1002410023
let endpoint = NetworkEndpoint {
1002510024
host: "api.example.com".to_string(),
1002610025
port: 443,
1002710026
ports: vec![443, 8443],
1002810027
protocol: "rest".to_string(),
10029-
tls: openshell_core::proto::NetworkTlsMode::Terminate as i32,
1003010028
access: openshell_core::proto::NetworkAccessPreset::ReadOnly as i32,
1003110029
..Default::default()
1003210030
};
@@ -10982,7 +10980,6 @@ mod tests {
1098210980
let mut policy = test_policy_with_rule("aws", host);
1098310981
let endpoint = &mut policy.network_policies.get_mut("aws").unwrap().endpoints[0];
1098410982
endpoint.protocol = "rest".to_string();
10985-
endpoint.tls = 2;
1098610983
endpoint.access = openshell_core::proto::NetworkAccessPreset::Full as i32;
1098710984
endpoint.credential_signing = "sigv4".to_string();
1098810985
endpoint.signing_service = "s3".to_string();
@@ -13047,7 +13044,6 @@ mod tests {
1304713044
.endpoints[0];
1304813045
bound_endpoint.protocol = "rest".to_string();
1304913046
bound_endpoint.access = openshell_core::proto::NetworkAccessPreset::Full as i32;
13050-
bound_endpoint.tls = 2;
1305113047
openshell_policy::ensure_sandbox_process_identity(&mut policy);
1305213048
state
1305313049
.store

0 commit comments

Comments
 (0)