-
Notifications
You must be signed in to change notification settings - Fork 1.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Ambient policy enforcement #15072
Ambient policy enforcement #15072
Conversation
/test lint |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
May also want to wrap some of the really long lines
| no | Selector | Pod | n/a | DENY destination ztunnel | | ||
| yes | Selector | Pod | n/a | DENY destination ztunnel | | ||
| no | _empty**_ | Namespace | n/a | DENY destination ztunnel | | ||
| yes | _empty**_ | Namespace | n/a | DENY destination ztunnel | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, I don't understand why source identity is n/a
. Wouldnt that be the client or waypoint if it exists?
|
||
* Whether or not there is already a waypoint is in the traffic path. | ||
|
||
** If no attachment is specified the policy will be treated as Namespace scoped. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Namespace scoped but only for L4? This is same as line 127 & 128, correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was stated that having a blank cell in the table was confusing so I was trying to convey that if you don't have a selector or targetRef then you get namespace scope enforced at the ztunnel and this extra note was intended to clarify what empty in the cell means. Maybe it's a bad idea or just not worded correctly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I updated the wording of this line to hopefully clarify it a little. Ultimately maybe we should just change _empty**_
though so that a footnote isn't required
@@ -0,0 +1,70 @@ | |||
--- | |||
title: Policy enforcement in ambient mode |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this only Authz policy?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would like to see this under a page for authz policy vs under usage/policy as if folks don't need authz policy, they don't need to care about this. Or if folks don't need L4 authz policy, they don't need to worry about most of this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My suggestion is:
- this becomes a "Designing authorization policy for ambient mode" guide (or whatever other types of policy are different between L4 and L7 and need consideration)
- it is weighted to come after the initial L4/L7 feature introductions
- most of the content is here, and the L4/L7 feature introductions link out to this doc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, I thought we were attempting to get rid of a dedicated auth pol page for ambient. I'm ok with having it though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We got rid of a dedicated page to move policy into the journey of an adopter:
- here's how you install
- here's how you can do L4 things
- here's how you can add on L7 things
We didn't want to leave policy to after you've learned about L7.
However, a page on how you should think about and design policy is a good thing to have, and by the time you get to reading that, you should have understood L4/L7/etc architecturally.
test: no | ||
--- | ||
|
||
Istio's ambient data plane splits the data plane into two different sets of components. This architecture allows users to only pay for application layer processing if they need it. The trade-off made is the extra complexity of understanding how traffic flows through this architecture and where policy will be enforced. This guide will introduce a number of broad scenarios and explore the permutations therein and the details to understand for each scenario. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
extra complexity
- we need to spin positively. flexibility, pay only what you need
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we just delete this page. Right now it is basically just storing notes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, maybe good to have dedicated page for authz policy.
|
321ef22
to
a5726f1
Compare
Thanks for driving this, Ian! I've had a look through today and here's what I would recommend:
What you have here is basically that, with some rearrangement. The docs would probably also benefit from some "when and why to do X" docs, which I think @keithmattix was going to have a go at. If you're interested, I can have a first pass at moving the things around? |
@@ -0,0 +1,70 @@ | |||
--- | |||
title: Policy enforcement in ambient mode |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My suggestion is:
- this becomes a "Designing authorization policy for ambient mode" guide (or whatever other types of policy are different between L4 and L7 and need consideration)
- it is weighted to come after the initial L4/L7 feature introductions
- most of the content is here, and the L4/L7 feature introductions link out to this doc.
| no | Selector | Pod | n/a | DENY destination ztunnel | | ||
| yes | Selector | Pod | n/a | DENY destination ztunnel | | ||
| no | _empty**_ | Namespace | n/a | DENY destination ztunnel | | ||
| yes | _empty**_ | Namespace | n/a | DENY destination ztunnel | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So why is row 2 a DENY?
also, suggest wording update to 'destination ztunnel (always DENY)' so it starts with the actual column heading, which is where it is enforced
@@ -75,6 +75,31 @@ Even though the identity of the pod is otherwise correct, the presence of a L7 p | |||
command terminated with exit code 56 | |||
{{< /text >}} | |||
|
|||
### Authorization policy attachment | |||
|
|||
In the simplest enforcement scenario, you want to enforce policy against TCP attributes and you have no waypoint proxies in your traffic's path. These policies can be enforced by ztunnel proxies. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am still not comfortable with the hoops we have to jump through to describe "having a waypoint in your traffic's path"
https://istio.io/latest/docs/ambient/architecture/data-plane/ uses "in mesh" and "in mesh, waypoint enabled" which is close but probably still not how people are going to describe workload configuration.
Thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure the ideal way to word this if we don't like "no waypoint proxies in your traffic path" which I think is at least fairly clear, if somewhat clunky. I don't really like "waypoint disabled" personally but if that is the project's preferred wording I can change it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've never heard "waypoint disabled" but I'd never use that; you don't define Stage 1 of something additive as "Without Stage 2" :)
"In the simplest enforcement scenario, you want to enforce policy against TCP attributes and you are only using the secure L4 overlay. These policies can be enforced by ztunnel proxies."
(Still too wordy.)
Diet Ambient™
|
||
In the simplest enforcement scenario, you want to enforce policy against TCP attributes and you have no waypoint proxies in your traffic's path. These policies can be enforced by ztunnel proxies. | ||
|
||
Once you introduce a waypoint proxy, the ideal place to enforce policy shifts. Traffic arriving at the destination ztunnel will be coming from the waypoint's identity because waypoint proxies do not impersonate `src` identity on behalf of the client. This means that even if you only wish to enforce policy against TCP attributes, you should bind that policy to your waypoint proxy. An additional TCP policy may be applied to your workload to request that ztunnel enforce rules like, "in-mesh traffic needs to come from my waypoint in order to reach my application". This type of policy allows you to choose if "bypassing" the waypoint proxy is permissible in your scenario. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
replace 'be coming' with 'come' or 'appear' or 'is presented as' (to make active voice)
you should bind that policy to your waypoint proxy
Is there any performance penalty? I guess the implication is "it will go through the waypoint anyway, if the PEP is the destination ztunnel"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The point I was trying to make is that if you want to base any of your policy on client identity waypoint is your only real choice. Otherwise even a client called evil-do-not-ever-allow
will just look like your waypoint proxy to the dest ztunnel so you can't make a very good decision based on identity anymore.
/easycla |
@craigbox, I invited you as a collaborator on my istio.io fork if you wanna be able to contribute directly to this PR. |
2969715
to
206d2c3
Compare
Thanks, in general as a maintainer of Istio I can do that anyway 😊 |
Do we want to consider getting the L7 and L4 updates from this PR merged and break writing of the "design and use auth policy" out into it's own PR? I think the updates that exist now may provide value even though they are not the end state |
PTAL at these exact links
I am especially concerned as to if the behavior I have written up in the L7 table is true. |
/easycla |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice work on rewording and organizing this. Couple of suggestions but overall I think it's an improvement
Signed-off-by: ilrudie <ian.rudie@solo.io>
Signed-off-by: ilrudie <ian.rudie@solo.io>
Co-authored-by: Ian Rudie <ilrudie@gmail.com>
e7095cc
to
58ed7b8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good overall and likely fine to merge as is if doc maintainers like it. I've added some notes/suggestions mostly for clarity.
|
||
## Layer 4 authorization policies | ||
## Policy enforcement using the secure overlay |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"secure overlay" hasn't really been introduced, folks may not know what it is?
## Policy enforcement using the secure overlay | |
## Policy enforcement using ztunnel |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See https://istio.io/latest/docs/ambient/overview/#ztunnel and a few other places, as well as the bikeshed thread. We need a way to refer to what you get when ambient is installed with no waypoints, where you don't have to care about the implementation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In peer authentication we should probably mention permissive and maybe remove the part about disabling ambient mode all together. "if you need to disable mTLS just don't use Istio" is effectively the last thing we say when practically speaking inbound passthrough via permissive is probably what they would actually want.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure how to put that into words sorry, but could we do that in a different PR? It hasn't changed (except I merged in a fix to the formatting)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, I can try to tackle that as a fast follower. I mostly just don't love the idea of an Istio doc ending with "don't use Istio" if we can avoid it.
|
||
### Attach to the entire waypoint proxy | ||
|
||
To attach a policy or routing rule to the entire waypoint — so that it applies to all traffic enrolled to use it — set `Gateway` as the `parentRefs` or `targetRefs` value, depending on the type. | ||
To attach a route or a policy to the entire waypoint — so that it applies to all traffic enrolled to use it — set `Gateway` as the `parentRefs` or `targetRefs` value, depending on the type. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be helpful to include when/why you would want to do this here - for policies with L7 attributes dependent on the identity of the calling service (as mentioned in the L4 docs under "Choosing enforcement points when waypoints are introduced " - any other specific use cases?) and mention implications for scope and which services are fronted by the waypoint (single service account vs all services in a namespace).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe we intend to write such s guide but this doc was scoped down to be useful until that happened.
I kind of stole it off Ian 😁
{{< /text >}} | ||
|
||
### Attach to a specific service | ||
|
||
You can also attach a policy or routing rule to a specific service within the waypoint. Set `Service` as the `parentRefs` or `targetRefs` value, as appropriate. | ||
You can also attach a route or policy to one or more specific services within the waypoint. Set `Service` as the `parentRefs` or `targetRefs` value, as appropriate. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar to waypoint attachment above, suggest which policies make sense to attach in this way (HTTPRoute commonly), and which may be impractical (would AuthorizationPolicy perhaps not work as may be expected by a user because the calling identity becomes the waypoint?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It pains me I can do this
I am reminded of the StarCraft 2 Legacy of the Void intro where the Archon forms and says, "Power overwhelming". Craig the maintainer and Craig the author together has great powers! |
In response to a cherrypick label: new pull request created: #15202 |
Description
Adding some information about different ways to apply/target policy and the resulting enforcement locations. This is presently in a deleted doc which I suspect we don't want to introduce this page again so @craigbox would appreciate suggestions as to where we may want to add these.
Alternatively these could be depicted as flow diagrams rather than tables if desired.
Reviewers