api: Authorization API design

[zetaab]

What type of PR is this?

api: Authorization API design

The below snippet is what an authorization config looks like, the first iteration only supports ClientCIDR as the Principal. JWT Claims, custom headers, and client certs will be supported in the future.

authorization:
  defaultAction: "Deny"
  rules:
    - action: "Allow"
      principal:
        clientCIDR: ["10.0.1.0/24"]

Related: #2462 #2250

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 67.12%. Comparing base (29946b0) to head (3293ec2).
Report is 153 commits behind head on main.

❗ Current head 3293ec2 differs from pull request most recent head 387951b. Consider uploading reports for the commit 387951b to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2652      +/-   ##
==========================================
+ Coverage   66.51%   67.12%   +0.61%     
==========================================
  Files         161      164       +3     
  Lines       22673    23803    +1130     
==========================================
+ Hits        15080    15978     +898     
- Misses       6720     6906     +186     
- Partials      873      919      +46     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[zhaohuabing]

Should another type of client identity be considered in this PR?
For example, EG likely needs to support fine-grained access control based on JWT claims, or peer identity extracted from client certificate attributes.
Under the hood, all types of client identities(IP, JWT, peer identity) will be backed by the envoy RBAC filter. There are a few other implementations that can be referenced:

• Ambassdar: similar to this PR, but I think Ambassdar doesn't support other type of credentials. https://www.getambassador.io/docs/edge-stack/latest/topics/running/ambassador#ip-allow-and-deny
• Istio: a fat authorization policy including all types of client identities https://istio.io/latest/docs/reference/config/security/authorization-policy/#Source

@envoyproxy/gateway-maintainers

[zetaab]

@zhaohuabing yep, I was thinking to take that issue as next. However, in http filter level ACL is going to be handled before JWT filter. So there must be 2 different RBAC filters because of that? Otherwise it is not possible to make RBAC filter based on JWT (or other auth) results. At least I do not like the way that ACL is also handled after authentication etc. That will expose all auth providers to everyone and they can try to hack things before their request is finally denied by ACL filter.

So my idea was to do two separated filters: acl (executed as first in filter chain) and authorisation(not sure about the name). Authorisation will be executed after auth plugins (like jwt + ext auth..etc). In theory we could do ACL things in same filter. However, it will make auth plugins and others open for everybody and acl idea is block these kind of requests?

btw I am not familiar with envoyproxy itself, so if my idea about filter chains are incorrect please enlight me.

[zetaab]

@envoyproxy/gateway-maintainers before implementing anything, could others give opinion also about the format in #2652 (comment) (usecases explained in previous post)

[zetaab]

@arkodg @zhaohuabing updated the PR according the ideas.. but still not sure is everything like should.

[zetaab]

@arkodg I did some tests with the api spec. The authorization commit can be seen in zetaab@65885b5

1. other rbac stuff (like jwt authorization) can be implemented under clientselectors and zetaab@65885b5#diff-39171fec9cb0b38b175c0b1d846990f95dd907518d1d7757857aaa7bca5ea528R109

2. there is perhaps need to define "permissions" as well later. zetaab@65885b5#diff-39171fec9cb0b38b175c0b1d846990f95dd907518d1d7757857aaa7bca5ea528R129 but its not mandatory (yet) for ip allow/deny apis. So as I see it: this PR could proceed and I could start with implementing ip deny/allow part.

Then cases that I tested with this (ips are modified little bit, not real ips for me):

case 1:

  authorization:
    rules:
    - clientSelector:
        - clientCIDR:
          - 44.94.0.0/16
          - 44.65.0.0/16
      action: Allow

result: both cidrs are allowed and other traffic outside of these are denied.

case 2:

  authorization:
    rules:
    - clientSelector:
        - clientCIDR:
          - 44.94.0.0/16
        - clientCIDR:
          - 44.65.0.0/16
      action: Allow

result: both cidrs are allowed and other traffic outside of these are denied.

case 3:

  authorization:
    rules:
    - clientSelector:
        - clientCIDR:
          - 44.94.0.0/16
      action: Allow
    - clientSelector:
        - clientCIDR:
          - 44.65.0.0/16
      action: Allow

result: none of the CIDRs are working. (difference to case 2 is that instead of having two objects of clientCIDR under same clientselector, this use separated clientselectors. Which is creating two RBAC filters (instead of one that was the case with 1 and 2) in envoy level with the example commit).

case 4:

  authorization:
    rules:
    - clientSelector:
        - clientCIDR:
          - 44.94.0.0/16
          - 44.65.0.0/16
      action: Allow
    - clientSelector:
        - clientCIDR:
          - 44.94.107.109/32
      action: Log
    - clientSelector:
        - clientCIDR:
          - 44.94.107.109/32
      action: Deny

result: both cidrs are allowed and the specific ip is denied. Requests from that single ip is logged.

as we can see multiple allow rules (multiple envoy rbac filters, instead of single) might be interesting (case 3), not sure is there really use-case for that. Maybe there is if we combine other components like user information. In case of IPs it might be not that useful.

[arkodg]

LGTM thanks @zhaohuabing & @zetaab for building this out !

[zirain]

I'm not sure why this pass limiter, prefer to remove instead of commnet.

[zirain]

LGTM, can you clean the comment code in following PR?