Skip to content

Commit 5250e2e

Browse files
oteffahioteffahi
authored
Add policy id to ACL config (#1781)
* Add optional ACL Policy id * Update DEFAULT_CONFIG * Add acl config format tests * Fix typo
1 parent e210f9d commit 5250e2e

File tree

4 files changed

+232
-1
lines changed

4 files changed

+232
-1
lines changed

‎DEFAULT_CONFIG.json5

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -365,6 +365,8 @@
365365
// [
366366
// /// Each policy associates one or multiple rules to one or multiple subject combinations
367367
// {
368+
// /// Id is optional. If provided, it has to be unique within the policies list
369+
// "id": "policy1",
368370
// /// Rules and Subjects are identified with their unique IDs declared above
369371
// "rules": ["rule1"],
370372
// "subjects": ["subject1", "subject2"],

‎commons/zenoh-config/src/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,7 @@ impl std::fmt::Display for Username {
153153

154154
#[derive(Serialize, Debug, Deserialize, Clone, PartialEq, Eq, Hash)]
155155
pub struct AclConfigPolicyEntry {
156+
pub id: Option<String>,
156157
pub rules: Vec<String>,
157158
pub subjects: Vec<String>,
158159
}

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

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
//! This module is intended for Zenoh's internal use.
1818
//!
1919
//! [Click here for Zenoh's documentation](https://docs.rs/zenoh/latest/zenoh)
20-
use std::collections::HashMap;
20+
use std::collections::{HashMap, HashSet};
2121

2222
use ahash::RandomState;
2323
use itertools::Itertools;
@@ -389,6 +389,7 @@ impl PolicyEnforcer {
389389
let mut policy_rules: Vec<PolicyRule> = Vec::new();
390390
let mut rule_map = HashMap::new();
391391
let mut subject_id_map = HashMap::<String, Vec<usize>>::new();
392+
let mut policy_id_set = HashSet::<String>::new();
392393
let mut subject_map_builder = SubjectMapBuilder::new();
393394

394395
// validate rules config and insert them in hashmaps
@@ -511,6 +512,14 @@ impl PolicyEnforcer {
511512
}
512513
// finally, handle policy content
513514
for (entry_id, entry) in policies.iter().enumerate() {
515+
if let Some(policy_custom_id) = &entry.id {
516+
if !policy_id_set.insert(policy_custom_id.clone()) {
517+
bail!(
518+
"Policy id must be unique: id '{}' is repeated",
519+
policy_custom_id
520+
);
521+
}
522+
}
514523
// validate policy config lists
515524
if entry.rules.is_empty() || entry.subjects.is_empty() {
516525
bail!(

‎zenoh/tests/acl.rs

Lines changed: 219 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ const VALUE: &str = "zenoh";
3232
#[tokio::test(flavor = "multi_thread", worker_threads = 4)]
3333
async fn test_acl_pub_sub() {
3434
zenoh::init_log_from_env_or("error");
35+
test_acl_config_format(27447).await;
3536
test_pub_sub_deny(27447).await;
3637
test_pub_sub_allow(27447).await;
3738
test_pub_sub_deny_then_allow(27447).await;
@@ -124,6 +125,224 @@ async fn close_sessions(s01: Session, s02: Session) {
124125
ztimeout!(s02.close()).unwrap();
125126
}
126127

128+
async fn test_acl_config_format(port: u16) {
129+
println!("test_acl_config_format");
130+
let mut config_router = get_basic_router_config(port).await;
131+
132+
// missing lists
133+
config_router
134+
.insert_json5(
135+
"access_control",
136+
r#"{
137+
"enabled": true,
138+
"default_permission": "deny"
139+
}"#,
140+
)
141+
.unwrap();
142+
assert!(ztimeout!(zenoh::open(config_router.clone()))
143+
.is_err_and(|e| e.to_string().contains("config lists must be provided")));
144+
145+
// repeated rule id
146+
config_router
147+
.insert_json5(
148+
"access_control",
149+
r#"{
150+
"enabled": true,
151+
"default_permission": "deny",
152+
"rules": [
153+
{
154+
"id": "r1",
155+
"permission": "allow",
156+
"flows": ["egress", "ingress"],
157+
"messages": ["put"],
158+
"key_exprs": ["foo"],
159+
},
160+
{
161+
"id": "r1",
162+
"permission": "allow",
163+
"flows": ["egress", "ingress"],
164+
"messages": ["put"],
165+
"key_exprs": ["bar"],
166+
},
167+
],
168+
"subjects": [{id: "all"}],
169+
"policies": [
170+
{
171+
rules: ["r1"],
172+
subjects: ["all"],
173+
}
174+
],
175+
}"#,
176+
)
177+
.unwrap();
178+
assert!(ztimeout!(zenoh::open(config_router.clone()))
179+
.is_err_and(|e| e.to_string().contains("Rule id must be unique")));
180+
181+
// repeated subject id
182+
config_router
183+
.insert_json5(
184+
"access_control",
185+
r#"{
186+
"enabled": true,
187+
"default_permission": "deny",
188+
"rules": [
189+
{
190+
"id": "r1",
191+
"permission": "allow",
192+
"flows": ["egress", "ingress"],
193+
"messages": ["put"],
194+
"key_exprs": ["foo"],
195+
},
196+
],
197+
"subjects": [
198+
{
199+
id: "s1",
200+
interfaces: ["lo"],
201+
},
202+
{
203+
id: "s1",
204+
interfaces: ["lo0"],
205+
},
206+
],
207+
"policies": [
208+
{
209+
rules: ["r1"],
210+
subjects: ["s1"],
211+
}
212+
],
213+
}"#,
214+
)
215+
.unwrap();
216+
assert!(ztimeout!(zenoh::open(config_router.clone()))
217+
.is_err_and(|e| e.to_string().contains("Subject id must be unique")));
218+
219+
// repeated policy id
220+
config_router
221+
.insert_json5(
222+
"access_control",
223+
r#"{
224+
"enabled": true,
225+
"default_permission": "deny",
226+
"rules": [
227+
{
228+
"id": "r1",
229+
"permission": "allow",
230+
"flows": ["egress", "ingress"],
231+
"messages": ["put"],
232+
"key_exprs": ["foo"],
233+
},
234+
{
235+
"id": "r2",
236+
"permission": "allow",
237+
"flows": ["egress", "ingress"],
238+
"messages": ["put"],
239+
"key_exprs": ["bar"],
240+
},
241+
],
242+
"subjects": [{id: "all"}],
243+
"policies": [
244+
{
245+
id: "p1",
246+
rules: ["r1"],
247+
subjects: ["all"],
248+
},
249+
{
250+
id: "p1",
251+
rules: ["r2"],
252+
subjects: ["all"],
253+
}
254+
],
255+
}"#,
256+
)
257+
.unwrap();
258+
assert!(ztimeout!(zenoh::open(config_router.clone()))
259+
.is_err_and(|e| e.to_string().contains("Policy id must be unique")));
260+
261+
// non-existent rule in policy
262+
config_router
263+
.insert_json5(
264+
"access_control",
265+
r#"{
266+
"enabled": true,
267+
"default_permission": "deny",
268+
"rules": [
269+
{
270+
"id": "r1",
271+
"permission": "allow",
272+
"flows": ["egress", "ingress"],
273+
"messages": ["put"],
274+
"key_exprs": ["foo"],
275+
},
276+
{
277+
"id": "r2",
278+
"permission": "allow",
279+
"flows": ["egress", "ingress"],
280+
"messages": ["put"],
281+
"key_exprs": ["bar"],
282+
},
283+
],
284+
"subjects": [{id: "all"}],
285+
"policies": [
286+
{
287+
id: "p1",
288+
rules: ["r1"],
289+
subjects: ["all"],
290+
},
291+
{
292+
id: "p2",
293+
rules: ["NON-EXISTENT"],
294+
subjects: ["all"],
295+
}
296+
],
297+
}"#,
298+
)
299+
.unwrap();
300+
assert!(ztimeout!(zenoh::open(config_router.clone()))
301+
.is_err_and(|e| e.to_string().contains("does not exist in rules list")));
302+
303+
// non-existent subject in policy
304+
config_router
305+
.insert_json5(
306+
"access_control",
307+
r#"{
308+
"enabled": true,
309+
"default_permission": "deny",
310+
"rules": [
311+
{
312+
"id": "r1",
313+
"permission": "allow",
314+
"flows": ["egress", "ingress"],
315+
"messages": ["put"],
316+
"key_exprs": ["foo"],
317+
},
318+
{
319+
"id": "r2",
320+
"permission": "allow",
321+
"flows": ["egress", "ingress"],
322+
"messages": ["put"],
323+
"key_exprs": ["bar"],
324+
},
325+
],
326+
"subjects": [{id: "all"}],
327+
"policies": [
328+
{
329+
id: "p1",
330+
rules: ["r1"],
331+
subjects: ["all"],
332+
},
333+
{
334+
id: "p2",
335+
rules: ["r2"],
336+
subjects: ["NON-EXISTENT"],
337+
}
338+
],
339+
}"#,
340+
)
341+
.unwrap();
342+
assert!(ztimeout!(zenoh::open(config_router.clone()))
343+
.is_err_and(|e| e.to_string().contains("does not exist in subjects list")));
344+
}
345+
127346
async fn test_pub_sub_deny(port: u16) {
128347
println!("test_pub_sub_deny");
129348

0 commit comments

Comments
 (0)