Update ACL config format to support AND/OR logic between subjects

[oteffahi]

This PR reworks the ACL config to support boolean combinations of subjects. An examples of the new config format is the following:

{
  "access_control":
    {
      "enabled": true,
      "default_permission": "deny",

      "rules":
      [
        {
          "id": "allow pub/sub ingress on test/demo",
          "permission": "allow",
          "flows": ["ingress"],
          "messages": [
            "put",
            "declare_subscriber"
          ],
          "key_exprs": [
            "test/demo"
          ]
        },
        {
          "id": "allow get/queryable test/demo ingress/egress",
          "permission": "allow",
          "flows": ["ingress", "egress"],
          "messages": [
            "query",
            "declare_queryable"
          ],
          "key_exprs": [
            "test/demo"
          ]
        },
      ],

      "subjects":
      [
        {
          "id": "client1 or client2 on domain1 through en0/en1",
          "interfaces": [
            "en0",
            "en1",
          ],
          "cert_common_names": [
            "domain1.local"
          ],
          "usernames": [
            "client1name",
            "client2name"
          ]
        },
        {
          "id": "domain2",
          "cert_common_names": [
            "domain2.local"
          ]
        },
        {
          "id": "all"
          // no fields defined = wildcard (all messages)
        }
      ],

      "policies":
      [
        {
          "rules": ["allow pub/sub ingress on test/demo"],
          "subjects": [
            "client1 or client2 on domain1 through en0/en1",
            "domain2"
          ]
        },
        {
          "rules": ["allow get/queryable test/demo ingress/egress"],
          "subjects": [
            "all",
          ]
        },
      ]
    }
  }

Within each subject, a cartesian product is performed to produce the (interface, cert_common_name, username) combinations. Each combination is a logical AND between its components, and different combinations within the same subject in the subjects list represent a logical OR between them.

Rules are declared seperately, and applied to these subject logical combinations in the policies list. Unique identifiers (id fields) are used to represent the subjects and rules in the policies.

[oteffahi]

@Mallets This is ready for review.

[oteffahi]

It is an array of policy , so policies

In SOTA, the access control policy (singular) is the set of rules applied to subjects, so calling it policies would be confusing in that regard. The whole configuration creates the ACL policy.
EDIT: policies will be added to align the naming in the config to all plural.

[oteffahi]

Also, queryable and get actions are not clear, like a queryable must have the permissions to receive get and send reply, in the current implementation there is no such reply action.

I would expect rules like (pseudocode, all allows), from the PoV of a router

queryable: 
   - declare_queryable: ingress
   - get: egress
   - reply: ingress
getter:
  - declare_queryable: egress
  - get: ingress
  - reply: egress 

in current implementation we have

queryable:
 - declare_queryable: ingress
 - get: egress
 getter:
  - declare_queryable: egress
  - get: ingress

@gabrik Reply actions were excluded from the initial ACL design because we assumed that allowing/denying a get means allowing/denying its associated Reply even if it is not on the same key_expr. This can be changed in a future PR if deemed useful for certain applications. @OlivierHecart what do you think about this?
EDIT: Reply messages will be added to ACL in a future PR, following the change of perspective from actions to messages.

[gabrik]

@gabrik Reply actions were excluded from the initial ACL design because we assumed that allowing/denying a get means allowing/denying its associated Reply even if it is not on the same key_expr

I'm against implicit things, because if that applies for get then also a declare_subscribe should imply the put

But I'm fine that to have this behavior for 1.0.0 and change for the next one.

[fuzzypixelz]

I would rename "declare_subscriber" to "subscriber_declaration", "declare_queryable" to "queryable_declaration", "put" to "publication" and "get" to "query".

The idea is to use nouns instead of verbs now that we think of these as data and not actions.

This is more a matter of opinion so overall this LGTM 🚀

[Mallets]

I would rename "declare_subscriber" to "subscriber_declaration", "declare_queryable" to "queryable_declaration", "put" to "publication" and "get" to "query".

The idea is to use nouns instead of verbs now that we think of these as data and not actions.

This is more a matter of opinion so overall this LGTM 🚀

If we move to messages than the messages are declare_subscriber, declare_queryable, put, and get. Those messages have the same name of the exposed API used by applications, making it quite easy to correlate messages to API for the user.

[fuzzypixelz]

I would rename "declare_subscriber" to "subscriber_declaration", "declare_queryable" to "queryable_declaration", "put" to "publication" and "get" to "query".
The idea is to use nouns instead of verbs now that we think of these as data and not actions.
This is more a matter of opinion so overall this LGTM 🚀

If we move to messages than the messages are declare_subscriber, declare_queryable, put, and get. Those messages have the same name of the exposed API used by applications, making it quite easy to correlate messages to API for the user.

@Mallets If we're talking about the API names then let's not forget that Publisher::put returns a Publication. That a queryable receives Query objects (we don't say it receives a "get"). That Subscriber::undeclare() returns a SubscriberUndeclaration (there is no SubscriberDeclaration type but that's more of an inconsistency in the API naming convention).

It is not enough for the message names to reflect the API of the subject impacted by ingress rules. Subjects impacted by egress rules also ought to be considered.

If I was a user it would be immediately clear to me that a rule with on message get would impact my Session:get but I would very surprised to learn that it also impacts my Queryable::recv_async.

The message names cannot match the API calls of subjects on both ingress and egress sides. If they match either one, then the naming is misleading for the other. Thus the names ought to be abstracted over both.

[Mallets]

I would rename "declare_subscriber" to "subscriber_declaration", "declare_queryable" to "queryable_declaration", "put" to "publication" and "get" to "query".
The idea is to use nouns instead of verbs now that we think of these as data and not actions.
This is more a matter of opinion so overall this LGTM 🚀

If we move to messages than the messages are declare_subscriber, declare_queryable, put, and get. Those messages have the same name of the exposed API used by applications, making it quite easy to correlate messages to API for the user.

@Mallets If we're talking about the API names then let's not forget that Publisher::put returns a Publication. That a queryable receives Query objects (we don't say it receives a "get"). That Subscriber::undeclare() returns a SubscriberUndeclaration (there is no SubscriberDeclaration type but that's more of an inconsistency in the API naming convention).

It is not enough for the message names to reflect the API of the subject impacted by ingress rules. Subjects impacted by egress rules also ought to be considered.

If I was a user it would be immediately clear to me that a rule with on message get would impact my Session:get but I would very surprised to learn that it also impacts my Queryable::recv_async.

The message names cannot match the API calls of subjects on both ingress and egress sides. If they match either one, then the naming is misleading for the other. Thus the names ought to be abstracted over both.

The mapping has nothing to do with Rust types but with the Zenoh API and semantics. Here we are defining a config that can be loaded by a C/C++/Python/Java/Kotlin application. All those languages consistently define declare_subscriber, declare_queryable, etc. operations. So, IMHO restricting the scope to Rust type and driving the discussion around that would be a mistake.

[gabrik]

with reserve to improve the semantic in the future.

i.e, split between get and query reply

[oteffahi]

with reserve to improve the semantic in the future.

i.e, split between get and query

You mean get and reply?

[gabrik]

with reserve to improve the semantic in the future.
i.e, split between get and query

You mean get and reply?

Yup

[fuzzypixelz]

The mapping has nothing to do with Rust types but with the Zenoh API and semantics.

@Mallets My argument wasn't centered around the Rust API names, I was just giving examples. Consider the following ACL rule:

{
  "id": "deny-get-egress",
  "flows": ["egress"],
  "messages": ["get"],
  "permission": "deny",
  "key_exprs": ["test/1"]
}

If deny-get-egress is applied to subject S in a router R for instance, then R would block any queries matching test/1 from reaching S.

What are the semantics of denying "get" messages¹ from reaching S? The word "get" refers to the Zenoh API operation get using your wording.

Here we are trying to configure whether or not queries can reach S, the semantics are more clearly expressed in terms of data in this case, not API operations. Thus "operations" do not capture the semantics of messages in all cases (granted they do for ingress flows).

Footnotes

1. If we're being pedantic here, there is not such thing as a "get" message, it is an operation. ↩