Skip to content

Commit

Permalink
Add publisher delete and queryable reply messages to ACL (#1259)
Browse files Browse the repository at this point in the history 
* Expose reply key_expr to interceptors

* Add reply message to ACL logic and config

* Update DEFAULT_CONFIG

* Update ACL get/queryable tests, add reply tests

* Improve reply matching

* Specify all existing message types in ACL interceptor matching

* Add reply to authentication qbl tests configs

* Add delete message to ACL logic and config

* Add delete message to DEFAULT_CONFIG, format messages

* Add delete message to ACL pub/sub tests

* Fix clippy errors

* Reorder message matching

* Revert "Expose reply key_expr to interceptors", use wire_expr for ACL filtering of reply messages

This reverts commit 3a78d5d.

* Revert key_expr parsing change to reply ingress

Ingress reply messages are not affected by the unimplemented key_expr in routing/dispatcher/queries.rs
  • Loading branch information
@oteffahi
oteffahi authored Jul 24, 2024
1 parent aace7f1 commit 7aeb4ad
Show file tree
Hide file tree
Showing 6 changed files with 372 additions and 49 deletions.
6 changes: 4 additions & 2 deletions DEFAULT_CONFIG.json5
View file
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,8 @@
// /// Id has to be unique within the rule set
// "id": "rule1",
// "messages": [
// "put", "query", "declare_subscriber", "declare_queryable"
// "put", "delete", "declare_subscriber",
// "query", "reply", "declare_queryable",
// ],
// "flows":["egress","ingress"],
// "permission": "allow",
Expand All @@ -210,7 +211,8 @@
// {
// "id": "rule2",
// "messages": [
// "put", "query", "declare_subscriber", "declare_queryable"
// "put", "delete", "declare_subscriber",
// "query", "reply", "declare_queryable",
// ],
// "flows":["ingress"],
// "permission": "allow",
Expand Down
2 changes: 2 additions & 0 deletions commons/zenoh-config/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -166,9 +166,11 @@ pub struct PolicyRule {
#[serde(rename_all = "snake_case")]
pub enum AclMessage {
Put,
Delete,
DeclareSubscriber,
Query,
DeclareQueryable,
Reply,
}

#[derive(Clone, Copy, Debug, Serialize, Deserialize, Eq, Hash, PartialEq)]
Expand Down
113 changes: 104 additions & 9 deletions zenoh/src/net/routing/interceptor/access_control.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ use zenoh_config::{
};
use zenoh_protocol::{
core::ZenohIdProto,
network::{Declare, DeclareBody, NetworkBody, NetworkMessage, Push, Request},
network::{Declare, DeclareBody, NetworkBody, NetworkMessage, Push, Request, Response},
zenoh::{PushBody, RequestBody},
};
use zenoh_result::ZResult;
Expand Down Expand Up @@ -235,6 +235,21 @@ impl InterceptorTrait for IngressAclEnforcer {
.or_else(|| ctx.full_expr());

match &ctx.msg.body {
NetworkBody::Request(Request {
payload: RequestBody::Query(_),
..
}) => {
if self.action(AclMessage::Query, "Query (ingress)", key_expr?) == Permission::Deny
{
return None;
}
}
NetworkBody::Response(Response { .. }) => {
if self.action(AclMessage::Reply, "Reply (ingress)", key_expr?) == Permission::Deny
{
return None;
}
}
NetworkBody::Push(Push {
payload: PushBody::Put(_),
..
Expand All @@ -243,11 +258,12 @@ impl InterceptorTrait for IngressAclEnforcer {
return None;
}
}
NetworkBody::Request(Request {
payload: RequestBody::Query(_),
NetworkBody::Push(Push {
payload: PushBody::Del(_),
..
}) => {
if self.action(AclMessage::Query, "Query (ingress)", key_expr?) == Permission::Deny
if self.action(AclMessage::Delete, "Delete (ingress)", key_expr?)
== Permission::Deny
{
return None;
}
Expand Down Expand Up @@ -278,7 +294,38 @@ impl InterceptorTrait for IngressAclEnforcer {
return None;
}
}
_ => {}
// Unfiltered Declare messages
NetworkBody::Declare(Declare {
body: DeclareBody::DeclareKeyExpr(_),
..
})
| NetworkBody::Declare(Declare {
body: DeclareBody::DeclareFinal(_),
..
})
| NetworkBody::Declare(Declare {
body: DeclareBody::DeclareToken(_),
..
}) => {}
// Unfiltered Undeclare messages
NetworkBody::Declare(Declare {
body: DeclareBody::UndeclareKeyExpr(_),
..
})
| 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(_) => {}
}
Some(ctx)
}
Expand All @@ -305,6 +352,22 @@ impl InterceptorTrait for EgressAclEnforcer {
.or_else(|| ctx.full_expr());

match &ctx.msg.body {
NetworkBody::Request(Request {
payload: RequestBody::Query(_),
..
}) => {
if self.action(AclMessage::Query, "Query (egress)", key_expr?) == Permission::Deny {
return None;
}
}
NetworkBody::Response(Response { wire_expr, .. }) => {
// @TODO: Remove wire_expr usage when issue #1255 is implemented
if self.action(AclMessage::Reply, "Reply (egress)", wire_expr.as_str())
== Permission::Deny
{
return None;
}
}
NetworkBody::Push(Push {
payload: PushBody::Put(_),
..
Expand All @@ -313,11 +376,12 @@ impl InterceptorTrait for EgressAclEnforcer {
return None;
}
}
NetworkBody::Request(Request {
payload: RequestBody::Query(_),
NetworkBody::Push(Push {
payload: PushBody::Del(_),
..
}) => {
if self.action(AclMessage::Query, "Query (egress)", key_expr?) == Permission::Deny {
if self.action(AclMessage::Delete, "Delete (egress)", key_expr?) == Permission::Deny
{
return None;
}
}
Expand Down Expand Up @@ -347,7 +411,38 @@ impl InterceptorTrait for EgressAclEnforcer {
return None;
}
}
_ => {}
// Unfiltered Declare messages
NetworkBody::Declare(Declare {
body: DeclareBody::DeclareKeyExpr(_),
..
})
| NetworkBody::Declare(Declare {
body: DeclareBody::DeclareFinal(_),
..
})
| NetworkBody::Declare(Declare {
body: DeclareBody::DeclareToken(_),
..
}) => {}
// Unfiltered Undeclare messages
NetworkBody::Declare(Declare {
body: DeclareBody::UndeclareKeyExpr(_),
..
})
| 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(_) => {}
}
Some(ctx)
}
Expand Down
6 changes: 6 additions & 0 deletions zenoh/src/net/routing/interceptor/authorization.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -184,23 +184,29 @@ impl PermissionPolicy {
struct ActionPolicy {
query: PermissionPolicy,
put: PermissionPolicy,
delete: PermissionPolicy,
declare_subscriber: PermissionPolicy,
declare_queryable: PermissionPolicy,
reply: PermissionPolicy,
}

impl ActionPolicy {
fn action(&self, action: AclMessage) -> &PermissionPolicy {
match action {
AclMessage::Query => &self.query,
AclMessage::Reply => &self.reply,
AclMessage::Put => &self.put,
AclMessage::Delete => &self.delete,
AclMessage::DeclareSubscriber => &self.declare_subscriber,
AclMessage::DeclareQueryable => &self.declare_queryable,
}
}
fn action_mut(&mut self, action: AclMessage) -> &mut PermissionPolicy {
match action {
AclMessage::Query => &mut self.query,
AclMessage::Reply => &mut self.reply,
AclMessage::Put => &mut self.put,
AclMessage::Delete => &mut self.delete,
AclMessage::DeclareSubscriber => &mut self.declare_subscriber,
AclMessage::DeclareQueryable => &mut self.declare_queryable,
}
Expand Down
Loading

0 comments on commit 7aeb4ad

Please sign in to comment.