Skip to content
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.

Sign up for GitHub

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

Add publisher delete and queryable reply messages to ACL #1259

Merged
merged 14 commits into from
Jul 24, 2024
Merged

Add publisher delete and queryable reply messages to ACL #1259

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
3a78d5d
Expose reply key_expr to interceptors
oteffahi Jul 24, 2024
f45f2dd
Add reply message to ACL logic and config
oteffahi Jul 23, 2024
6a632d9
Update DEFAULT_CONFIG
oteffahi Jul 23, 2024
d2a700b
Update ACL get/queryable tests, add reply tests
oteffahi Jul 23, 2024
b5dcf1b
Improve reply matching
oteffahi Jul 23, 2024
b3c0906
Specify all existing message types in ACL interceptor matching
oteffahi Jul 23, 2024
8e30f55
Add reply to authentication qbl tests configs
oteffahi Jul 24, 2024
99323ad
Add delete message to ACL logic and config
oteffahi Jul 24, 2024
0c2ee09
Add delete message to DEFAULT_CONFIG, format messages
oteffahi Jul 24, 2024
036ac96
Add delete message to ACL pub/sub tests
oteffahi Jul 24, 2024
bf7fb08
Fix clippy errors
oteffahi Jul 24, 2024
8db19ec
Reorder message matching
oteffahi Jul 24, 2024
2ea951e
Revert "Expose reply key_expr to interceptors", use wire_expr for ACL…
oteffahi Jul 24, 2024
26f6ca5
Revert key_expr parsing change to reply ingress
oteffahi Jul 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions DEFAULT_CONFIG.json5
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
{
Comment on lines +364 to +367
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will have to be adressed once #1255 is implemented.

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
Open in desktop
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
Loading