envoyproxy / envoyproxy/envoy

rbac: support mixing allow/deny policies with precedence

Open
#9,376 16 comments 0 reactions 0 assignees View on GitHub
design proposal no stalebot
Dominant language
C++
Stars
28.9k
Forks
5.6k
Avg merge
1d 22h
Merged PRs (30d)
430

Description

*Title*: support mixing allow/deny policies with precedence

*Description*:
The current [RBAC filter API](https://github.com/envoyproxy/envoy/blob/6d816ad9af0527e63ac58d0ffe7eac29bfe56d62/api/envoy/config/rbac/v2/rbac.proto#L59) has the following limitations:
- It binds a single `action` to a set of policies, in other words, we cannot mix the allow/deny policies in the same config
- It doesn't support policy precedence as all policies are specified in a map

Feature Request:
- Support action per-policy in the filter config
- Support policy with precedence

Use Cases:
- Some system (e.g. Istio) may have complicated layered access control design that uses both deny and allow policy at the same time with different priority.

Strawman design:

- Option 1: Incrementally change the current RBAC filter API
- add the [Action](https://github.com/envoyproxy/envoy/blob/6d816ad9af0527e63ac58d0ffe7eac29bfe56d62/api/envoy/config/rbac/v2/rbac.proto#L61) to the [Policy](https://github.com/envoyproxy/envoy/blob/master/api/envoy/config/rbac/v2/rbac.proto#L84) message
- add a new `repeated Policy list_policies` in the RBAC message and mark the current [policies](https://github.com/envoyproxy/envoy/blob/6d816ad9af0527e63ac58d0ffe7eac29bfe56d62/api/envoy/config/rbac/v2/rbac.proto#L78) as deprecated

- Option 2: Introduce a new top-level config to simplify the RBAC filter API:

```protobuf
message Authorization {
// The filter config is just a list of rules,
// the first matched rule decide the action: ALLOW or DENY
repeated Rule rules = 2;
}

message Rule {
// Optional. A human readable string explaining the purpose of this rule.
string description = 1;

// A single match decide when to trigger the action
Match match = 2;

// action could be either ALLOW or DENY
Action action = 3;
}

message Match {
// Optional. A human readable string explaining the purpose of this match.
string description = 1;

oneof type {
MatchSet and = 2;
MatchSet or = 3;
Match not = 4;

bool always = 5;
HeaderMatcher header = 6;
CidrRange source_ip = 7;
CidrRange destination_ip = 8;
uint32 destination_port = 9;
StringMatcher requested_server_name = 10;
Authenticated authenticated = 11;
MetadataMatcher metadata = 12;
string cel = 13;
}
}

message MatchSet {
repeated Match matches = 1;
}
```

Summary
- Option 1 tries to do everything incrementally, the change is less but I'm worried it's going to make the RBAC filter API more complicated in the long turn. Option 1 is fully backward-compatible and requires minimal changes in user code if they want to use the new features.
- Option 2 tries to do some some optimizations (most notably it has a much simpler and cleaner match/action pair), in the long term, we could rename the `RBAC` filter to just `authz` filter. Option 2 requires slight more but still reasonable changes in user code if they want to use the new feature, Envoy could also auto-translate the config if needed.

@mattklein123 @htuch @lizan would you mind to take a look and let me know what do you think about this? Thank you.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.