fix bug in acl config validation (#982)

* fix bug in acl config validation

* Update ACL config error messages

* Apply suggestions from code review

Co-authored-by: oteffahi <[email protected]>

* Update zenoh/src/net/routing/interceptor/authorization.rs

Co-authored-by: oteffahi <[email protected]>

* Fix mistake in ACL config error message

* Allow empty interfaces and flows in config

* Add warning logs for implicit wildcard ACL config

* Fix linting

* Add support for undefined interfaces and flows lists

This reverts commits a2db2d7 5008e38 e204f2d.

* Error out when ACL cannot be enabled

---------

Co-authored-by: oteffahi <[email protected]>
Co-authored-by: Luca Cominardi <[email protected]>
Co-authored-by: Oussama Teffahi <[email protected]>

commons/zenoh-config/src/lib.rs

@@ -102,10 +102,10 @@ pub struct DownsamplingItemConf {
 
 #[derive(Serialize, Debug, Deserialize, Clone)]
 pub struct AclConfigRules {
-    pub interfaces: Vec<String>,
+    pub interfaces: Option<Vec<String>>,
     pub key_exprs: Vec<String>,
     pub actions: Vec<Action>,
-    pub flows: Vec<InterceptorFlow>,
+    pub flows: Option<Vec<InterceptorFlow>>,
     pub permission: Permission,
 }
 

zenoh/src/net/routing/interceptor/access_control.rs

@@ -66,7 +66,7 @@ pub(crate) fn acl_interceptor_factories(
                     enforcer: Arc::new(policy_enforcer),
                 }))
             }
-            Err(e) => tracing::error!("Access control inizialization error: {}", e),
+            Err(e) => bail!("Access control not enabled due to: {}", e),
         }
     } else {
         tracing::debug!("Access control is disabled");

zenoh/src/net/routing/interceptor/authorization.rs

@@ -19,12 +19,14 @@
 //! [Click here for Zenoh's documentation](../zenoh/index.html)
 use ahash::RandomState;
 use std::collections::HashMap;
+use std::net::Ipv4Addr;
 use zenoh_config::{
     AclConfig, AclConfigRules, Action, InterceptorFlow, Permission, PolicyRule, Subject,
 };
 use zenoh_keyexpr::keyexpr;
 use zenoh_keyexpr::keyexpr_tree::{IKeyExprTree, IKeyExprTreeMut, KeBoxTree};
 use zenoh_result::ZResult;
+use zenoh_util::net::get_interface_names_by_addr;
 type PolicyForSubject = FlowPolicy;
 
 type PolicyMap = HashMap<usize, PolicyForSubject, RandomState>;
@@ -135,10 +137,11 @@ impl PolicyEnforcer {
        initializes the policy_enforcer
     */
     pub fn init(&mut self, acl_config: &AclConfig) -> ZResult<()> {
-        self.acl_enabled = acl_config.enabled;
-        self.default_permission = acl_config.default_permission;
+        let mut_acl_config = acl_config.clone();
+        self.acl_enabled = mut_acl_config.enabled;
+        self.default_permission = mut_acl_config.default_permission;
         if self.acl_enabled {
-            if let Some(rules) = &acl_config.rules {
+            if let Some(mut rules) = mut_acl_config.rules {
                 if rules.is_empty() {
                     tracing::warn!("Access control rules are empty in config file");
                     self.policy_map = PolicyMap::default();
@@ -150,7 +153,30 @@ impl PolicyEnforcer {
                         };
                     }
                 } else {
-                    let policy_information = self.policy_information_point(rules)?;
+                    // check for undefined values in rules and initialize them to defaults
+                    for (rule_offset, rule) in rules.iter_mut().enumerate() {
+                        match rule.interfaces {
+                            Some(_) => (),
+                            None => {
+                                tracing::warn!("ACL config interfaces list is empty. Applying rule #{} to all network interfaces", rule_offset);
+                                if let Ok(all_interfaces) =
+                                    get_interface_names_by_addr(Ipv4Addr::UNSPECIFIED.into())
+                                {
+                                    rule.interfaces = Some(all_interfaces);
+                                }
+                            }
+                        }
+                        match rule.flows {
+                            Some(_) => (),
+                            None => {
+                                tracing::warn!("ACL config flows list is empty. Applying rule #{} to both Ingress and Egress flows", rule_offset);
+                                rule.flows = Some(
+                                    [InterceptorFlow::Ingress, InterceptorFlow::Egress].into(),
+                                );
+                            }
+                        }
+                    }
+                    let policy_information = self.policy_information_point(&rules)?;
                     let subject_map = policy_information.subject_map;
                     let mut main_policy: PolicyMap = PolicyMap::default();
 
@@ -199,10 +225,33 @@ impl PolicyEnforcer {
     ) -> ZResult<PolicyInformation> {
         let mut policy_rules: Vec<PolicyRule> = Vec::new();
         for config_rule in config_rule_set {
-            for subject in &config_rule.interfaces {
-                for flow in &config_rule.flows {
+            // config validation
+            let mut validation_err = String::new();
+            if config_rule.interfaces.as_ref().unwrap().is_empty() {
+                validation_err.push_str("ACL config interfaces list is empty. ");
+            }
+            if config_rule.actions.is_empty() {
+                validation_err.push_str("ACL config actions list is empty. ");
+            }
+            if config_rule.flows.as_ref().unwrap().is_empty() {
+                validation_err.push_str("ACL config flows list is empty. ");
+            }
+            if config_rule.key_exprs.is_empty() {
+                validation_err.push_str("ACL config key_exprs list is empty. ");
+            }
+            if !validation_err.is_empty() {
+                bail!("{}", validation_err);
+            }
+            for subject in config_rule.interfaces.as_ref().unwrap() {
+                if subject.trim().is_empty() {
+                    bail!("found an empty interface value in interfaces list");
+                }
+                for flow in config_rule.flows.as_ref().unwrap() {
                     for action in &config_rule.actions {
                         for key_expr in &config_rule.key_exprs {
+                            if key_expr.trim().is_empty() {
+                                bail!("found an empty key-expression value in key_exprs list");
+                            }
                             policy_rules.push(PolicyRule {
                                 subject: Subject::Interface(subject.clone()),
                                 key_expr: key_expr.clone(),