Add trustDomains and notTrustDomains to AuthzPolicy source#3665
Add trustDomains and notTrustDomains to AuthzPolicy source#3665istio-testing merged 2 commits intoistio:masterfrom
Conversation
|
😊 Welcome @ymesika! This is either your first contribution to the Istio api repo, or it's been You can learn more about the Istio working groups, Code of Conduct, and contribution guidelines Thanks for contributing! Courtesy of your friendly welcome wagon. |
| // +cue-gen:AuthorizationPolicy:releaseChannel:extended | ||
| repeated string trust_domains = 13; | ||
|
|
||
| // Optional. A list of negative match of trust domains. |
There was a problem hiding this comment.
What are the semantics of notTrustDomains is set but the connection doesn't have mTLS enabled?
There was a problem hiding this comment.
I think it's the same trap as notPrinicipals/notNamespaces.
For those, in positive values we have a comment that it requires mTLS and I also added it (https://github.com/ymesika/api/blob/a23a5bc7d3b28a0ca57056462867817b380479c7/security/v1beta1/authorization_policy.proto#L512) but we lack this comment on the negative ones.
I tried to be consistent with them.
There was a problem hiding this comment.
BTW, for all those negative forms when mTLS is absent, not_* will trivially pass (empty string doesn't match any specified value), potentially allowing unauthenticated traffic through.
| repeated string not_remote_ip_blocks = 10; | ||
|
|
||
| // Optional. A list of trust domains derived from the peer certificate. | ||
| // Can be exact, prefix, suffix and presence. |
There was a problem hiding this comment.
Based on the example, it seems like it's more global based with * vs an envoy string matcher type of API. Does it only match whole components of a SPIFFE URI? Can you do, eg "fo*-bar"?
There was a problem hiding this comment.
The matching semantics are clearly documented above. The trustdomains field supports the same pattern matching as all other string fields in the rule (suffix, prefix, exact or presence).
So no, you cannot do fo*-bar. But that's something to keep in mind to validate in the impl.
There was a problem hiding this comment.
Ack, thanks for the pointer
This PR introduces
trustDomainsandnotTrustDomainsfields to theAuthorizationPolicySource rule.This allows users to accurately and safely match against the identity's trust domain without resorting to potentially error-prone glob-style matching on the full principal URI.
This proposal is fully backward compatible. The addition of optional fields will not impact existing AuthorizationPolicy resources that do not use the new trustDomains fields.
Example:
Or: