-
Notifications
You must be signed in to change notification settings - Fork 1.3k
fix(network): normalize Windows binary paths (NVBug 6782969) #3482
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: windows
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -448,6 +448,7 @@ impl OpaEngine { | |
| let mut data: serde_json::Value = serde_json::from_str(&data_json_str) | ||
| .map_err(|e| miette::miette!("internal: failed to parse proto JSON: {e}"))?; | ||
| inject_runtime_policy_data(&mut data, require_binary_identity); | ||
| normalize_network_binary_paths(&mut data); | ||
| normalize_endpoint_protocols(&mut data); | ||
|
|
||
| // Validate BEFORE expanding presets | ||
|
|
@@ -1141,7 +1142,7 @@ fn network_input_json(input: &NetworkInput) -> serde_json::Value { | |
| let ancestor_strs: Vec<String> = input | ||
| .ancestors | ||
| .iter() | ||
| .map(|p| p.to_string_lossy().into_owned()) | ||
| .map(|p| network_binary_match_path(p)) | ||
| .collect(); | ||
| let cmdline_strs: Vec<String> = input | ||
| .cmdline_paths | ||
|
|
@@ -1150,7 +1151,7 @@ fn network_input_json(input: &NetworkInput) -> serde_json::Value { | |
| .collect(); | ||
| serde_json::json!({ | ||
| "exec": { | ||
| "path": input.binary_path.to_string_lossy(), | ||
| "path": network_binary_match_path(&input.binary_path), | ||
| "ancestors": ancestor_strs, | ||
| "cmdline_paths": cmdline_strs, | ||
| }, | ||
|
|
@@ -1161,6 +1162,29 @@ fn network_input_json(input: &NetworkInput) -> serde_json::Value { | |
| }) | ||
| } | ||
|
|
||
| /// Return the stable representation used only for network-policy path matching. | ||
| /// | ||
| /// Windows paths are case-insensitive by default and accept either path separator. | ||
| /// Normalizing both policy data and runtime input prevents equivalent spellings from | ||
| /// being denied while leaving the original path intact for filesystem access and | ||
| /// executable hashing. Other platforms retain exact path matching. | ||
| pub(crate) fn network_binary_match_path(path: &Path) -> String { | ||
| let path = path.to_string_lossy(); | ||
| #[cfg(target_os = "windows")] | ||
| { | ||
| windows_network_binary_match_path(&path) | ||
| } | ||
| #[cfg(not(target_os = "windows"))] | ||
| { | ||
| path.into_owned() | ||
| } | ||
| } | ||
|
|
||
| #[cfg(any(target_os = "windows", test))] | ||
| fn windows_network_binary_match_path(path: &str) -> String { | ||
| path.replace('\\', "/").to_ascii_lowercase() | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This comparison is unsafe for Windows directories with case sensitivity enabled. NTFS can treat |
||
| } | ||
|
|
||
| /// Sets an already-built JSON value as Regorus input without encoding and reparsing JSON text. | ||
| /// | ||
| /// The explicit fallible conversion preserves evaluator errors because Regorus's infallible | ||
|
|
@@ -1463,6 +1487,7 @@ fn preprocess_yaml_data( | |
| .map_err(|e| miette::miette!("failed to parse YAML data: {e}"))?; | ||
| validate_opa_data_structure(&data)?; | ||
| inject_runtime_policy_data(&mut data, require_binary_identity); | ||
| normalize_network_binary_paths(&mut data); | ||
| normalize_endpoint_protocols(&mut data); | ||
|
|
||
| // Normalize port → ports for all endpoints so Rego always sees "ports" array. | ||
|
|
@@ -1564,6 +1589,35 @@ fn normalize_endpoint_protocols(data: &mut serde_json::Value) { | |
| } | ||
| } | ||
|
|
||
| /// Normalize configured binary paths to the same platform-specific representation | ||
| /// used for runtime process identity before any Rego evaluation. | ||
| fn normalize_network_binary_paths(data: &mut serde_json::Value) { | ||
| let Some(policies) = data | ||
| .get_mut("network_policies") | ||
| .and_then(serde_json::Value::as_object_mut) | ||
| else { | ||
| return; | ||
| }; | ||
|
|
||
| for policy in policies.values_mut() { | ||
| let Some(binaries) = policy | ||
| .get_mut("binaries") | ||
| .and_then(serde_json::Value::as_array_mut) | ||
| else { | ||
| continue; | ||
| }; | ||
| for binary in binaries { | ||
| let Some(path) = binary.get_mut("path") else { | ||
| continue; | ||
| }; | ||
| let Some(value) = path.as_str() else { | ||
| continue; | ||
| }; | ||
| *path = network_binary_match_path(Path::new(value)).into(); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /// Normalize endpoint port/ports in JSON data. | ||
| /// | ||
| /// YAML policies may use `port: N` (single) or `ports: [N, M]` (multi). | ||
|
|
@@ -2397,6 +2451,16 @@ mod tests { | |
| OpaEngine::from_strings(TEST_POLICY, TEST_DATA_YAML).expect("Failed to load test policy") | ||
| } | ||
|
|
||
| #[test] | ||
| fn windows_binary_match_path_normalizes_case_and_separators() { | ||
| let normalized = windows_network_binary_match_path(r"C:\WINDOWS\SYSTEM32\CURL.EXE"); | ||
| assert_eq!(normalized, "c:/windows/system32/curl.exe"); | ||
| assert_ne!( | ||
| normalized, | ||
| windows_network_binary_match_path(r"C:\Windows\System32\powershell.exe") | ||
| ); | ||
| } | ||
|
|
||
| fn opa_container_policy() -> serde_json::Value { | ||
| serde_json::json!({ | ||
| "filesystem_policy": {}, | ||
|
|
@@ -3420,6 +3484,84 @@ network_policies: | |
| assert_eq!(decision.matched_policy.as_deref(), Some("claude_code")); | ||
| } | ||
|
|
||
| #[cfg(target_os = "windows")] | ||
| #[test] | ||
| fn from_proto_matches_windows_equivalent_binary_path() { | ||
| let mut proto = openshell_policy::restrictive_default_policy(); | ||
| proto.network_policies.insert( | ||
| "windows_binary".to_string(), | ||
| NetworkPolicyRule { | ||
| name: "windows_binary".to_string(), | ||
| endpoints: vec![NetworkEndpoint { | ||
| host: "example.com".to_string(), | ||
| port: 443, | ||
| ..Default::default() | ||
| }], | ||
| binaries: vec![NetworkBinary { | ||
| path: r"C:\WINDOWS\SYSTEM32\CURL.EXE".to_string(), | ||
| }], | ||
| }, | ||
| ); | ||
| let engine = OpaEngine::from_proto(&proto).expect("Failed to create engine from proto"); | ||
|
|
||
| let equivalent = NetworkInput { | ||
| host: "example.com".into(), | ||
| port: 443, | ||
| binary_path: PathBuf::from("c:/windows/system32/curl.exe"), | ||
| binary_sha256: "unused".into(), | ||
| ancestors: vec![], | ||
| cmdline_paths: vec![], | ||
| }; | ||
| let decision = engine.evaluate_network(&equivalent).unwrap(); | ||
| assert!( | ||
| decision.allowed, | ||
| "Windows-equivalent binary path should be allowed: {}", | ||
| decision.reason | ||
| ); | ||
|
|
||
| let different_binary = NetworkInput { | ||
| binary_path: PathBuf::from("c:/windows/system32/powershell.exe"), | ||
| ..equivalent | ||
| }; | ||
| let decision = engine.evaluate_network(&different_binary).unwrap(); | ||
| assert!( | ||
| !decision.allowed, | ||
| "normalization must not allow a different binary" | ||
| ); | ||
| } | ||
|
|
||
| #[cfg(target_os = "windows")] | ||
| #[test] | ||
| fn from_strings_matches_windows_equivalent_binary_path() { | ||
| let engine = OpaEngine::from_strings( | ||
| TEST_POLICY, | ||
| r#" | ||
| network_policies: | ||
| windows_binary: | ||
| endpoints: | ||
| - { host: example.com, port: 443 } | ||
| binaries: | ||
| - { path: 'C:\WINDOWS\SYSTEM32\CURL.EXE' } | ||
| "#, | ||
| ) | ||
| .expect("Failed to create engine from YAML"); | ||
| let input = NetworkInput { | ||
| host: "example.com".into(), | ||
| port: 443, | ||
| binary_path: PathBuf::from("c:/windows/system32/curl.exe"), | ||
| binary_sha256: "unused".into(), | ||
| ancestors: vec![], | ||
| cmdline_paths: vec![], | ||
| }; | ||
|
|
||
| let decision = engine.evaluate_network(&input).unwrap(); | ||
| assert!( | ||
| decision.allowed, | ||
| "Windows-equivalent YAML binary path should be allowed: {}", | ||
| decision.reason | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn from_proto_denies_unmatched_request() { | ||
| let proto = test_proto(); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -76,12 +76,12 @@ pub(super) fn http_context( | |
| binary_path: decision | ||
| .binary | ||
| .as_ref() | ||
| .map(|path| path.to_string_lossy().into_owned()) | ||
| .map(|path| crate::opa::network_binary_match_path(path)) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please validate this L7 path directly. The new tests exercise |
||
| .unwrap_or_default(), | ||
| ancestors: decision | ||
| .ancestors | ||
| .iter() | ||
| .map(|path| path.to_string_lossy().into_owned()) | ||
| .map(|path| crate::opa::network_binary_match_path(path)) | ||
| .collect(), | ||
| cmdline_paths: decision | ||
| .cmdline_paths | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This introduces an externally observable policy-matching contract, but the schema and security guidance remain unchanged. Please document the Windows normalization behavior—including ASCII-only case folding, separator handling, and the chosen behavior for case-sensitive/verbatim paths—in
docs/reference/policy-schema.mdxanddocs/security/best-practices.mdx. Users and policy-authoring agents rely on the published schema as the source of truth.