Fix(ACL): Apply queryable and subscriber declaration filters on their…

… respective undeclarations (#1573)

* Apply declaration filters to their respective undeclarations

* Populate ext_wire_expr of UndeclareSubscriber and UndeclareQueryable messages to expose their keyexpr in RoutingContext for interceptors

* Revert "Populate ext_wire_expr of UndeclareSubscriber and UndeclareQueryable messages to expose their keyexpr in RoutingContext for interceptors"

This reverts commit cafeaba.

* Add filtering of undeclarations in ingress depending on if key_expr is provided

* Change undeclaration log level to trace

* Tidy up undeclaration logs based on PR comments

* Move log to debug, add clarification comment

* Fix typo

zenoh/src/net/routing/dispatcher/pubsub.rs

@@ -124,7 +124,6 @@ pub(crate) fn undeclare_subscription(
     node_id: NodeId,
     send_declare: &mut SendDeclare,
 ) {
-    tracing::debug!("Undeclare subscription {}", face);
     let res = if expr.is_empty() {
         None
     } else {
@@ -173,7 +172,8 @@ pub(crate) fn undeclare_subscription(
         Resource::clean(&mut res);
         drop(wtables);
     } else {
-        tracing::error!("{} Undeclare unknown subscriber {}", face, id);
+        // NOTE: This is expected behavior if subscriber declarations are denied with ingress ACL interceptor.
+        tracing::debug!("{} Undeclare unknown subscriber {}", face, id);
     }
 }
 

zenoh/src/net/routing/dispatcher/queries.rs

@@ -186,7 +186,8 @@ pub(crate) fn undeclare_queryable(
         Resource::clean(&mut res);
         drop(wtables);
     } else {
-        tracing::error!("{} Undeclare unknown queryable {}", face, id);
+        // NOTE: This is expected behavior if queryable declarations are denied with ingress ACL interceptor.
+        tracing::debug!("{} Undeclare unknown queryable {}", face, id);
     }
 }
 

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

@@ -281,6 +281,26 @@ impl InterceptorTrait for IngressAclEnforcer {
                     return None;
                 }
             }
+            NetworkBody::Declare(Declare {
+                body: DeclareBody::UndeclareSubscriber(_),
+                ..
+            }) => {
+                // Undeclaration filtering diverges between ingress and egress:
+                // Undeclarations in ingress are only filtered if the ext_wire_expr is set.
+                // If it's not set, we let the undeclaration pass, it will be rejected by the routing logic
+                // if its associated declaration was denied.
+                if let Some(key_expr) = key_expr {
+                    if !key_expr.is_empty()
+                        && self.action(
+                            AclMessage::DeclareSubscriber,
+                            "Undeclare Subscriber (ingress)",
+                            key_expr,
+                        ) == Permission::Deny
+                    {
+                        return None;
+                    }
+                }
+            }
             NetworkBody::Declare(Declare {
                 body: DeclareBody::DeclareQueryable(_),
                 ..
@@ -294,6 +314,26 @@ impl InterceptorTrait for IngressAclEnforcer {
                     return None;
                 }
             }
+            NetworkBody::Declare(Declare {
+                body: DeclareBody::UndeclareQueryable(_),
+                ..
+            }) => {
+                // Undeclaration filtering diverges between ingress and egress:
+                // Undeclarations in ingress are only filtered if the ext_wire_expr is set.
+                // If it's not set, we let the undeclaration pass, it will be rejected by the routing logic
+                // if its associated declaration was denied.
+                if let Some(key_expr) = key_expr {
+                    if !key_expr.is_empty()
+                        && self.action(
+                            AclMessage::DeclareQueryable,
+                            "Undeclare Queryable (ingress)",
+                            key_expr,
+                        ) == Permission::Deny
+                    {
+                        return None;
+                    }
+                }
+            }
             // Unfiltered Declare messages
             NetworkBody::Declare(Declare {
                 body: DeclareBody::DeclareKeyExpr(_),
@@ -315,14 +355,6 @@ impl InterceptorTrait for IngressAclEnforcer {
             | NetworkBody::Declare(Declare {
                 body: DeclareBody::UndeclareToken(_),
                 ..
-            })
-            | NetworkBody::Declare(Declare {
-                body: DeclareBody::UndeclareQueryable(_),
-                ..
-            })
-            | NetworkBody::Declare(Declare {
-                body: DeclareBody::UndeclareSubscriber(_),
-                ..
             }) => {}
             // Unfiltered remaining message types
             NetworkBody::Interest(_) | NetworkBody::OAM(_) | NetworkBody::ResponseFinal(_) => {}
@@ -395,6 +427,21 @@ impl InterceptorTrait for EgressAclEnforcer {
                     return None;
                 }
             }
+            NetworkBody::Declare(Declare {
+                body: DeclareBody::UndeclareSubscriber(_),
+                ..
+            }) => {
+                // Undeclaration filtering diverges between ingress and egress:
+                // in egress the keyexpr has to be provided in the RoutingContext
+                if self.action(
+                    AclMessage::DeclareSubscriber,
+                    "Undeclare Subscriber (egress)",
+                    key_expr?,
+                ) == Permission::Deny
+                {
+                    return None;
+                }
+            }
             NetworkBody::Declare(Declare {
                 body: DeclareBody::DeclareQueryable(_),
                 ..
@@ -408,6 +455,21 @@ impl InterceptorTrait for EgressAclEnforcer {
                     return None;
                 }
             }
+            NetworkBody::Declare(Declare {
+                body: DeclareBody::UndeclareQueryable(_),
+                ..
+            }) => {
+                // Undeclaration filtering diverges between ingress and egress:
+                // in egress the keyexpr has to be provided in the RoutingContext
+                if self.action(
+                    AclMessage::DeclareQueryable,
+                    "Undeclare Queryable (egress)",
+                    key_expr?,
+                ) == Permission::Deny
+                {
+                    return None;
+                }
+            }
             // Unfiltered Declare messages
             NetworkBody::Declare(Declare {
                 body: DeclareBody::DeclareKeyExpr(_),
@@ -429,14 +491,6 @@ impl InterceptorTrait for EgressAclEnforcer {
             | NetworkBody::Declare(Declare {
                 body: DeclareBody::UndeclareToken(_),
                 ..
-            })
-            | NetworkBody::Declare(Declare {
-                body: DeclareBody::UndeclareQueryable(_),
-                ..
-            })
-            | NetworkBody::Declare(Declare {
-                body: DeclareBody::UndeclareSubscriber(_),
-                ..
             }) => {}
             // Unfiltered remaining message types
             NetworkBody::Interest(_) | NetworkBody::OAM(_) | NetworkBody::ResponseFinal(_) => {}