Skip to content

Commit

Permalink
Add policy id to ACL config (#1781)
Browse files Browse the repository at this point in the history
* Add optional ACL Policy id

* Update DEFAULT_CONFIG

* Add acl config format tests

* Fix typo
  • Loading branch information
oteffahi authored Feb 19, 2025
1 parent e210f9d commit 5250e2e
Show file tree
Hide file tree
Showing 4 changed files with 232 additions and 1 deletion.
2 changes: 2 additions & 0 deletions DEFAULT_CONFIG.json5
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,8 @@
// [
// /// Each policy associates one or multiple rules to one or multiple subject combinations
// {
// /// Id is optional. If provided, it has to be unique within the policies list
// "id": "policy1",
// /// Rules and Subjects are identified with their unique IDs declared above
// "rules": ["rule1"],
// "subjects": ["subject1", "subject2"],
Expand Down
1 change: 1 addition & 0 deletions commons/zenoh-config/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@ impl std::fmt::Display for Username {

#[derive(Serialize, Debug, Deserialize, Clone, PartialEq, Eq, Hash)]
pub struct AclConfigPolicyEntry {
pub id: Option<String>,
pub rules: Vec<String>,
pub subjects: Vec<String>,
}
Expand Down
11 changes: 10 additions & 1 deletion zenoh/src/net/routing/interceptor/authorization.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
//! This module is intended for Zenoh's internal use.
//!
//! [Click here for Zenoh's documentation](https://docs.rs/zenoh/latest/zenoh)
use std::collections::HashMap;
use std::collections::{HashMap, HashSet};

use ahash::RandomState;
use itertools::Itertools;
Expand Down Expand Up @@ -389,6 +389,7 @@ impl PolicyEnforcer {
let mut policy_rules: Vec<PolicyRule> = Vec::new();
let mut rule_map = HashMap::new();
let mut subject_id_map = HashMap::<String, Vec<usize>>::new();
let mut policy_id_set = HashSet::<String>::new();
let mut subject_map_builder = SubjectMapBuilder::new();

// validate rules config and insert them in hashmaps
Expand Down Expand Up @@ -511,6 +512,14 @@ impl PolicyEnforcer {
}
// finally, handle policy content
for (entry_id, entry) in policies.iter().enumerate() {
if let Some(policy_custom_id) = &entry.id {
if !policy_id_set.insert(policy_custom_id.clone()) {
bail!(
"Policy id must be unique: id '{}' is repeated",
policy_custom_id
);
}
}
// validate policy config lists
if entry.rules.is_empty() || entry.subjects.is_empty() {
bail!(
Expand Down
219 changes: 219 additions & 0 deletions zenoh/tests/acl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ const VALUE: &str = "zenoh";
#[tokio::test(flavor = "multi_thread", worker_threads = 4)]
async fn test_acl_pub_sub() {
zenoh::init_log_from_env_or("error");
test_acl_config_format(27447).await;
test_pub_sub_deny(27447).await;
test_pub_sub_allow(27447).await;
test_pub_sub_deny_then_allow(27447).await;
Expand Down Expand Up @@ -124,6 +125,224 @@ async fn close_sessions(s01: Session, s02: Session) {
ztimeout!(s02.close()).unwrap();
}

async fn test_acl_config_format(port: u16) {
println!("test_acl_config_format");
let mut config_router = get_basic_router_config(port).await;

// missing lists
config_router
.insert_json5(
"access_control",
r#"{
"enabled": true,
"default_permission": "deny"
}"#,
)
.unwrap();
assert!(ztimeout!(zenoh::open(config_router.clone()))
.is_err_and(|e| e.to_string().contains("config lists must be provided")));

// repeated rule id
config_router
.insert_json5(
"access_control",
r#"{
"enabled": true,
"default_permission": "deny",
"rules": [
{
"id": "r1",
"permission": "allow",
"flows": ["egress", "ingress"],
"messages": ["put"],
"key_exprs": ["foo"],
},
{
"id": "r1",
"permission": "allow",
"flows": ["egress", "ingress"],
"messages": ["put"],
"key_exprs": ["bar"],
},
],
"subjects": [{id: "all"}],
"policies": [
{
rules: ["r1"],
subjects: ["all"],
}
],
}"#,
)
.unwrap();
assert!(ztimeout!(zenoh::open(config_router.clone()))
.is_err_and(|e| e.to_string().contains("Rule id must be unique")));

// repeated subject id
config_router
.insert_json5(
"access_control",
r#"{
"enabled": true,
"default_permission": "deny",
"rules": [
{
"id": "r1",
"permission": "allow",
"flows": ["egress", "ingress"],
"messages": ["put"],
"key_exprs": ["foo"],
},
],
"subjects": [
{
id: "s1",
interfaces: ["lo"],
},
{
id: "s1",
interfaces: ["lo0"],
},
],
"policies": [
{
rules: ["r1"],
subjects: ["s1"],
}
],
}"#,
)
.unwrap();
assert!(ztimeout!(zenoh::open(config_router.clone()))
.is_err_and(|e| e.to_string().contains("Subject id must be unique")));

// repeated policy id
config_router
.insert_json5(
"access_control",
r#"{
"enabled": true,
"default_permission": "deny",
"rules": [
{
"id": "r1",
"permission": "allow",
"flows": ["egress", "ingress"],
"messages": ["put"],
"key_exprs": ["foo"],
},
{
"id": "r2",
"permission": "allow",
"flows": ["egress", "ingress"],
"messages": ["put"],
"key_exprs": ["bar"],
},
],
"subjects": [{id: "all"}],
"policies": [
{
id: "p1",
rules: ["r1"],
subjects: ["all"],
},
{
id: "p1",
rules: ["r2"],
subjects: ["all"],
}
],
}"#,
)
.unwrap();
assert!(ztimeout!(zenoh::open(config_router.clone()))
.is_err_and(|e| e.to_string().contains("Policy id must be unique")));

// non-existent rule in policy
config_router
.insert_json5(
"access_control",
r#"{
"enabled": true,
"default_permission": "deny",
"rules": [
{
"id": "r1",
"permission": "allow",
"flows": ["egress", "ingress"],
"messages": ["put"],
"key_exprs": ["foo"],
},
{
"id": "r2",
"permission": "allow",
"flows": ["egress", "ingress"],
"messages": ["put"],
"key_exprs": ["bar"],
},
],
"subjects": [{id: "all"}],
"policies": [
{
id: "p1",
rules: ["r1"],
subjects: ["all"],
},
{
id: "p2",
rules: ["NON-EXISTENT"],
subjects: ["all"],
}
],
}"#,
)
.unwrap();
assert!(ztimeout!(zenoh::open(config_router.clone()))
.is_err_and(|e| e.to_string().contains("does not exist in rules list")));

// non-existent subject in policy
config_router
.insert_json5(
"access_control",
r#"{
"enabled": true,
"default_permission": "deny",
"rules": [
{
"id": "r1",
"permission": "allow",
"flows": ["egress", "ingress"],
"messages": ["put"],
"key_exprs": ["foo"],
},
{
"id": "r2",
"permission": "allow",
"flows": ["egress", "ingress"],
"messages": ["put"],
"key_exprs": ["bar"],
},
],
"subjects": [{id: "all"}],
"policies": [
{
id: "p1",
rules: ["r1"],
subjects: ["all"],
},
{
id: "p2",
rules: ["r2"],
subjects: ["NON-EXISTENT"],
}
],
}"#,
)
.unwrap();
assert!(ztimeout!(zenoh::open(config_router.clone()))
.is_err_and(|e| e.to_string().contains("does not exist in subjects list")));
}

async fn test_pub_sub_deny(port: u16) {
println!("test_pub_sub_deny");

Expand Down

0 comments on commit 5250e2e

Please sign in to comment.