From 028e9a2b197d238e249c12de41916d361a65ac82 Mon Sep 17 00:00:00 2001 From: "Zhidong Peng (HE/HIM)" Date: Wed, 7 Oct 2026 14:16:27 -0700 Subject: [PATCH] Refactor ruleId away from local_rules.rs and extend it for enableSoftAudit field. --- proxy_agent/src/key_keeper.rs | 111 +++++++++++++++++++++- proxy_agent/src/key_keeper/key.rs | 79 +++++++++++++++ proxy_agent/src/key_keeper/local_rules.rs | 110 +++++---------------- 3 files changed, 208 insertions(+), 92 deletions(-) diff --git a/proxy_agent/src/key_keeper.rs b/proxy_agent/src/key_keeper.rs index 20426d64..956109cd 100644 --- a/proxy_agent/src/key_keeper.rs +++ b/proxy_agent/src/key_keeper.rs @@ -34,7 +34,7 @@ use self::local_rules::{ use crate::common::error::{Error, KeyErrorType}; use crate::common::result::Result; use crate::common::{constants, helpers, logger}; -use crate::key_keeper::key::{AuthorizationRules, KeyStatus}; +use crate::key_keeper::key::{AuthorizationItem, AuthorizationRules, KeyStatus}; use crate::provision; use crate::proxy::authorization_rules::{AuthorizationRulesForLogging, ComputedAuthorizationRules}; use crate::shared_state::access_control_wrapper::AccessControlSharedState; @@ -107,6 +107,25 @@ enum WakeReason { TimerElapsed, } +/// Determines whether soft audit should be enabled based on the WireServer and IMDS rules and their descriptors. +/// return true when any of the host service is in Audit mode and its “enableSoftAudit” is set to true +/// return false otherwise +fn should_enable_soft_audit( + wireserver_rules: &Option, + wireserver_rule_id_descriptor: &key::RuleIdDescriptor, + imds_rules: &Option, + imds_rule_id_descriptor: &key::RuleIdDescriptor, +) -> bool { + (wireserver_rule_id_descriptor.enable_soft_audit + && wireserver_rules + .as_ref() + .is_some_and(|rules| rules.mode.eq_ignore_ascii_case("audit"))) + || (imds_rule_id_descriptor.enable_soft_audit + && imds_rules + .as_ref() + .is_some_and(|rules| rules.mode.eq_ignore_ascii_case("audit"))) +} + impl KeyKeeper { pub fn new( host: String, @@ -537,23 +556,49 @@ impl KeyKeeper { } } + let remote_wire_server_rules = status.get_wireserver_rules(); + let wire_server_rule_id_descriptor = key::parse_rule_id_descriptor( + remote_wire_server_rules + .as_ref() + .map(|item| item.id.as_str()), + ); let (wireserver_rules, wireserver_local_state_changed) = resolve_effective_rules( &self.rules_dir, - status.get_wireserver_rules(), + remote_wire_server_rules, + &wire_server_rule_id_descriptor, LocalRuleTarget::WireServer, &mut local_rule_state_tracker.wireserver, wireserver_rule_id_changed, ) .await; + + let remote_imds_rules = status.get_imds_rules(); + let imds_rule_id_descriptor = + key::parse_rule_id_descriptor(remote_imds_rules.as_ref().map(|item| item.id.as_str())); let (imds_rules, imds_local_state_changed) = resolve_effective_rules( &self.rules_dir, - status.get_imds_rules(), + remote_imds_rules, + &imds_rule_id_descriptor, LocalRuleTarget::Imds, &mut local_rule_state_tracker.imds, imds_rule_id_changed, ) .await; + let soft_audit_mode_enabled = should_enable_soft_audit( + &wireserver_rules, + &wire_server_rule_id_descriptor, + &imds_rules, + &imds_rule_id_descriptor, + ); + if let Err(e) = self + .agent_status_shared_state + .set_soft_audit_mode(soft_audit_mode_enabled) + .await + { + logger::write_warning(format!("Failed to set soft audit mode: {e}")); + } + if wireserver_rule_id_changed || wireserver_local_state_changed { if let Err(e) = self .access_control_shared_state @@ -1097,10 +1142,10 @@ impl KeyKeeper { #[cfg(test)] mod tests { - use super::key::Key; + use super::key::{AuthorizationItem, Key, RuleIdDescriptor}; use super::local_rules; - use crate::key_keeper; use crate::key_keeper::KeyKeeper; + use crate::key_keeper::{self, should_enable_soft_audit}; use proxy_agent_shared::misc_helpers; use proxy_agent_shared::server_mock; use std::env; @@ -1108,6 +1153,62 @@ mod tests { use std::time::Duration; use tokio_util::sync::CancellationToken; + fn authorization_item(mode: &str) -> Option { + Some(AuthorizationItem { + defaultAccess: "deny".to_string(), + mode: mode.to_string(), + id: "test-rule-id".to_string(), + rules: None, + }) + } + + fn rule_id_descriptor(enable_soft_audit: bool) -> RuleIdDescriptor { + RuleIdDescriptor { + logical_id: "test-rule-id".to_string(), + use_local_file_rules: false, + enable_soft_audit, + } + } + + #[test] + fn should_enable_soft_audit_test() { + let enabled_descriptor = rule_id_descriptor(true); + let disabled_descriptor = rule_id_descriptor(false); + let audit_rules = authorization_item("Audit"); + let enforce_rules = authorization_item("enforce"); + + assert!(should_enable_soft_audit( + &audit_rules, + &enabled_descriptor, + &enforce_rules, + &disabled_descriptor, + )); + assert!(should_enable_soft_audit( + &enforce_rules, + &disabled_descriptor, + &audit_rules, + &enabled_descriptor, + )); + assert!(!should_enable_soft_audit( + &audit_rules, + &disabled_descriptor, + &audit_rules, + &disabled_descriptor, + )); + assert!(!should_enable_soft_audit( + &enforce_rules, + &enabled_descriptor, + &audit_rules, + &disabled_descriptor, + )); + assert!(!should_enable_soft_audit( + &None, + &enabled_descriptor, + &None, + &enabled_descriptor, + )); + } + #[tokio::test] async fn check_local_key_test() { let mut temp_test_path = env::temp_dir(); diff --git a/proxy_agent/src/key_keeper/key.rs b/proxy_agent/src/key_keeper/key.rs index 965e5443..fa129de6 100644 --- a/proxy_agent/src/key_keeper/key.rs +++ b/proxy_agent/src/key_keeper/key.rs @@ -31,6 +31,7 @@ use crate::{ }, proxy::{proxy_connection::ConnectionLogger, Claims}, }; +use base64::{engine::general_purpose, Engine as _}; use http::{Method, StatusCode}; use hyper::Uri; use proxy_agent_shared::hyper_client; @@ -772,6 +773,68 @@ impl Display for KeyAction { } } +#[derive(Default)] +pub(crate) struct RuleIdDescriptor { + pub(crate) logical_id: String, + pub(crate) use_local_file_rules: bool, + pub(crate) enable_soft_audit: bool, +} + +impl RuleIdDescriptor { + pub(crate) fn is_empty(&self) -> bool { + self.logical_id.is_empty() + } + + pub(crate) fn display_id(&self) -> String { + if self.logical_id.is_empty() { + "unknown".to_string() + } else { + format!( + "{}-useLocalFileRules-{}", + self.logical_id, self.use_local_file_rules + ) + } + } +} + +#[derive(Deserialize)] +#[allow(non_snake_case)] +pub(crate) struct EncodedRuleId { + #[serde(default)] + pub(crate) id: String, + #[serde(default)] + pub(crate) useLocalFileRules: bool, + #[serde(default)] + pub(crate) enableSoftAudit: bool, +} + +/// Parse the rule ID descriptor from the raw rule ID string. +/// The raw rule ID can be either a plain logical ID or +/// a base64-encoded JSON string containing the logical ID and whether to use local file rules. +pub(crate) fn parse_rule_id_descriptor(raw_rule_id: Option<&str>) -> RuleIdDescriptor { + let raw_rule_id = raw_rule_id.unwrap_or_default().trim(); + if raw_rule_id.is_empty() { + return RuleIdDescriptor::default(); + } + + if let Ok(decoded) = general_purpose::STANDARD.decode(raw_rule_id) { + if let Ok(contract) = serde_json::from_slice::(&decoded) { + return RuleIdDescriptor { + logical_id: contract.id, + use_local_file_rules: contract.useLocalFileRules, + enable_soft_audit: contract.enableSoftAudit, + }; + } + } + + // If parsing fails, treat the raw rule ID as the logical ID and do not use local file rules. + RuleIdDescriptor { + logical_id: raw_rule_id.to_string(), + use_local_file_rules: false, + enable_soft_audit: false, + } +} + const STATUS_URL: &str = "/secure-channel/status"; const KEY_URL: &str = "/secure-channel/key"; const HOST_DATE_TIME_DRIFT_MAX_AGE: Duration = Duration::from_secs(60 * 15); @@ -914,9 +977,12 @@ mod tests { use super::Key; use super::KeyStatus; + use crate::key_keeper::key::parse_rule_id_descriptor; use crate::key_keeper::key::Identity; use crate::key_keeper::key::Privilege; use crate::proxy::proxy_connection::ConnectionLogger; + use base64::engine::general_purpose; + use base64::Engine; use hyper::Uri; use proxy_agent_shared::hyper_client; use serde_json::json; @@ -1808,4 +1874,17 @@ mod tests { "process name after lossy conversion should be equal to replacement char" ); } + + #[test] + fn parse_rule_id_descriptor_test() { + let legacy = parse_rule_id_descriptor(Some("legacy-id")); + assert_eq!(legacy.logical_id, "legacy-id"); + assert!(!legacy.use_local_file_rules); + + let encoded = general_purpose::STANDARD + .encode(r#"{"id":"sig-resource-id","useLocalFileRules":true}"#); + let descriptor = parse_rule_id_descriptor(Some(&encoded)); + assert_eq!(descriptor.logical_id, "sig-resource-id"); + assert!(descriptor.use_local_file_rules); + } } diff --git a/proxy_agent/src/key_keeper/local_rules.rs b/proxy_agent/src/key_keeper/local_rules.rs index 40451d94..fdca180d 100644 --- a/proxy_agent/src/key_keeper/local_rules.rs +++ b/proxy_agent/src/key_keeper/local_rules.rs @@ -6,8 +6,8 @@ use crate::common::logger; use crate::common::result::Result; use crate::key_keeper::key::{ AccessControlRules, AuthorizationItem, Identity, Privilege, Role, RoleAssignment, + RuleIdDescriptor, }; -use base64::{engine::general_purpose, Engine as _}; use proxy_agent_shared::logger::LoggerLevel; use proxy_agent_shared::misc_helpers; use proxy_agent_shared::telemetry::event_logger; @@ -65,38 +65,6 @@ pub(crate) struct LocalRuleStateTracker { pub(crate) imds: LocalRuleMonitorState, } -#[derive(Default)] -pub(crate) struct RuleIdDescriptor { - pub(crate) logical_id: String, - pub(crate) use_local_file_rules: bool, -} - -impl RuleIdDescriptor { - pub(crate) fn is_empty(&self) -> bool { - self.logical_id.is_empty() - } - - pub(crate) fn display_id(&self) -> String { - if self.logical_id.is_empty() { - "unknown".to_string() - } else { - format!( - "{}-useLocalFileRules-{}", - self.logical_id, self.use_local_file_rules - ) - } - } -} - -#[derive(Deserialize)] -#[allow(non_snake_case)] -struct EncodedRuleId { - #[serde(default)] - id: String, - #[serde(default)] - useLocalFileRules: bool, -} - #[derive(Deserialize)] #[allow(non_snake_case)] pub(crate) struct LocalAuthorizationRulesFile { @@ -132,31 +100,6 @@ fn get_local_rule_file_state(local_rules_file_path: &Path) -> LocalRuleFileState } } -/// Parse the rule ID descriptor from the raw rule ID string. -/// The raw rule ID can be either a plain logical ID or -/// a base64-encoded JSON string containing the logical ID and whether to use local file rules. -fn parse_rule_id_descriptor(raw_rule_id: Option<&str>) -> RuleIdDescriptor { - let raw_rule_id = raw_rule_id.unwrap_or_default().trim(); - if raw_rule_id.is_empty() { - return RuleIdDescriptor::default(); - } - - if let Ok(decoded) = general_purpose::STANDARD.decode(raw_rule_id) { - if let Ok(contract) = serde_json::from_slice::(&decoded) { - return RuleIdDescriptor { - logical_id: contract.id, - use_local_file_rules: contract.useLocalFileRules, - }; - } - } - - // If parsing fails, treat the raw rule ID as the logical ID and do not use local file rules. - RuleIdDescriptor { - logical_id: raw_rule_id.to_string(), - use_local_file_rules: false, - } -} - fn normalize_authorization_item( authorization_item: Option, rule_id_descriptor: &RuleIdDescriptor, @@ -580,18 +523,18 @@ pub(crate) async fn read_local_rules_file( pub(crate) async fn resolve_effective_rules( rules_dir: &Path, remote_rules: Option, + rule_id_descriptor: &RuleIdDescriptor, target: LocalRuleTarget, tracker: &mut LocalRuleMonitorState, remote_rule_changed: bool, ) -> (Option, bool) { - let descriptor = parse_rule_id_descriptor(remote_rules.as_ref().map(|item| item.id.as_str())); - let normalized_remote_rules = normalize_authorization_item(remote_rules, &descriptor); + let normalized_remote_rules = normalize_authorization_item(remote_rules, rule_id_descriptor); let use_local_file_rules_changed = - tracker.use_local_file_rules != descriptor.use_local_file_rules; + tracker.use_local_file_rules != rule_id_descriptor.use_local_file_rules; let previous_parse_failed = tracker.parse_failed; if use_local_file_rules_changed { - let action = if descriptor.use_local_file_rules { + let action = if rule_id_descriptor.use_local_file_rules { "enabled" } else { "disabled" @@ -604,8 +547,8 @@ pub(crate) async fn resolve_effective_rules( ); } - tracker.use_local_file_rules = descriptor.use_local_file_rules; - if !descriptor.use_local_file_rules { + tracker.use_local_file_rules = rule_id_descriptor.use_local_file_rules; + if !rule_id_descriptor.use_local_file_rules { // not using local file rules, return normalized remote rules directly. // also reset the tracker state as we are not monitoring local file changes in this case. tracker.file_state = LocalRuleFileState::Unknown; @@ -716,7 +659,8 @@ pub(crate) async fn resolve_effective_rules( message, ); - let fail_closed_rules = build_fail_closed_rules(normalized_remote_rules, &descriptor); + let fail_closed_rules = + build_fail_closed_rules(normalized_remote_rules, rule_id_descriptor); tracker.parse_failed = true; tracker.effective_rules = fail_closed_rules.clone(); return ( @@ -728,7 +672,7 @@ pub(crate) async fn resolve_effective_rules( match read_local_rules_file(&local_rules_file, target).await { Ok(local_rules) => { let effective_rules = - merge_authorization_item(normalized_remote_rules, local_rules, &descriptor); + merge_authorization_item(normalized_remote_rules, local_rules, rule_id_descriptor); tracker.parse_failed = false; tracker.effective_rules = effective_rules.clone(); ( @@ -750,7 +694,8 @@ pub(crate) async fn resolve_effective_rules( message, ); - let fail_closed_rules = build_fail_closed_rules(normalized_remote_rules, &descriptor); + let fail_closed_rules = + build_fail_closed_rules(normalized_remote_rules, rule_id_descriptor); tracker.parse_failed = true; tracker.effective_rules = fail_closed_rules.clone(); ( @@ -779,14 +724,15 @@ pub(crate) fn write_local_rules_event( #[cfg(test)] mod tests { use super::{ - get_rules_dir_from_key_dir, merge_authorization_item, parse_rule_id_descriptor, - prefix_local_rule_names, read_local_rules_file, resolve_effective_rules, - validate_access_control_rules, validate_identities, validate_privileges, - validate_role_assignments, validate_roles, LocalAuthorizationRulesFile, - LocalRuleMonitorState, LocalRuleTarget, RuleIdDescriptor, LOCAL_RULE_NAME_PREFIX, + get_rules_dir_from_key_dir, merge_authorization_item, prefix_local_rule_names, + read_local_rules_file, resolve_effective_rules, validate_access_control_rules, + validate_identities, validate_privileges, validate_role_assignments, validate_roles, + LocalAuthorizationRulesFile, LocalRuleMonitorState, LocalRuleTarget, RuleIdDescriptor, + LOCAL_RULE_NAME_PREFIX, }; use crate::key_keeper::key::{ - AccessControlRules, AuthorizationItem, Identity, Privilege, Role, RoleAssignment, + parse_rule_id_descriptor, AccessControlRules, AuthorizationItem, Identity, Privilege, Role, + RoleAssignment, }; use base64::{engine::general_purpose, Engine as _}; use proxy_agent_shared::misc_helpers; @@ -908,10 +854,12 @@ mod tests { rules: remote_rules, }); let mut tracker = LocalRuleMonitorState::default(); - + let rule_id_descriptor = + parse_rule_id_descriptor(remote_rules.as_ref().map(|item| item.id.as_str())); let result = resolve_effective_rules( &rules_dir, remote_rules, + &rule_id_descriptor, LocalRuleTarget::WireServer, &mut tracker, remote_rule_changed, @@ -944,19 +892,6 @@ mod tests { fs::write(config_target, config_content).unwrap(); } - #[test] - fn parse_rule_id_descriptor_test() { - let legacy = parse_rule_id_descriptor(Some("legacy-id")); - assert_eq!(legacy.logical_id, "legacy-id"); - assert!(!legacy.use_local_file_rules); - - let encoded = general_purpose::STANDARD - .encode(r#"{"id":"sig-resource-id","useLocalFileRules":true}"#); - let descriptor = parse_rule_id_descriptor(Some(&encoded)); - assert_eq!(descriptor.logical_id, "sig-resource-id"); - assert!(descriptor.use_local_file_rules); - } - #[test] fn get_rules_dir_from_key_dir_test() { #[cfg(windows)] @@ -1004,6 +939,7 @@ mod tests { let descriptor = RuleIdDescriptor { logical_id: "decoded-id".to_string(), use_local_file_rules: true, + enable_soft_audit: false, }; let local_rules = LocalAuthorizationRulesFile { id: Some("local-id".to_string()),