api: allow_any_dynamic_dns mode in outbound traffic policy#3667
api: allow_any_dynamic_dns mode in outbound traffic policy#3667rudrakhp wants to merge 1 commit intoistio:masterfrom
Conversation
|
😊 Welcome @rudrakhp! 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. |
Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com>
|
@istio/technical-oversight-committee bumping this up. Thanks! |
ramaraochavali
left a comment
There was a problem hiding this comment.
LGTM. @keithmattix WDYT?
| // In `ALLOW_ANY_DYNAMIC_DNS` mode, traffic to unknown destinations will be allowed via dynamic DNS resolution. | ||
| // This mode allows users that do not have all possible egress destinations registered through `ServiceEntry` configurations to still connect | ||
| // to arbitrary destinations. Client TLS settings can be configured for connections to such destinations. | ||
| ALLOW_ANY_DYNAMIC_DNS = 3; |
There was a problem hiding this comment.
I'm kind of confused how this will work; don't you need a service entry for dynamic dns anyway?
There was a problem hiding this comment.
@keithmattix Thanks for the comment!
Today, if you create a ServiceEntry with resolution DYNAMIC_DNS, it results in a DFP cluster for that wildcard hostname.
This proposal extends the same mechanism for a mesh-wide fallback. When outboundTrafficPolicy.mode = ALLOW_ANY_DYNAMIC_DNS, Istio should program a dedicated DFP cluster serving catch all requests (domain name *, analogous to PassthroughCluster in ALLOW_ANY).
There was a problem hiding this comment.
Honestly, I'm not very comfortable with the idea of DFP as a mesh wide fallback. The TLS SNI filter in particular is still alpha and having envoy resolve all DNS in the cluster with this mode just feels like confused deputy waiting to happen
There was a problem hiding this comment.
The TLS SNI filter in particular is still alpha
Are you referring to SNI Dynamic Forward Proxy? I wasn't thinking we will do toSNI Dynamic Forward Proxy here - rather when the mode is ALLOW_ANY_DYNAMIC_DNS and the traffic is TLS - just use PassthroughCluster mode and if it is non TLS - use HTTP Dynamic Forward Proxy with client TLS settings - this works similar to mesh call with cluster + DR defined.
Can you please explain more about confused deputy issue - definitely want to understand more to see if the above resolves or we need to rethink?
There was a problem hiding this comment.
The TLS SNI filter in particular is still alpha and having envoy resolve all DNS in the cluster with this mode just feels like confused deputy waiting to happen
Valid concern on the SNI filter being alpha, but this is an explicit opt-in only mode and the security posture is arguably better than ALLOW_ANY where Envoy blindly forwards to whatever IP the application resolved to. In this mode, DFP independently re-resolves via SNI/Host header, and while that re-resolution could be the "confused deputy" problem you are pointing to (ack on that), isn't it actually more secure?
There was a problem hiding this comment.
Thank you @keithmattix for the detailed explanation. Now I get your point.
So if I put 169.254.169.254 in my host header, envoy will resolve it and send my request to Envoy's metadata server (that's the confused deputy part). And the mesh admin has no way to even stop it in this mode
Is n't the same is true with ALLOW_ANY? The malicious client can directly use the IP and invoke that endpoint? or Am I missing some thing here? Also the Wildcard SE with DYNAMIC_DNS if is made available to all Mesh services - it results in the same? In the ALLOW_ANY_DYNAMIC_DNS mode, the mesh admin has ability to configure ClientTLSSettings and enforce MUTUAL and validate CA to ensure the call goes to known destinations. I think ALLOW_ANY_DYNAMIC_DNS has the same trust model as ALLOW_ANY?
BTW, I completely agree with you on REGISTRY_ONLY - it is not a strong security boundary but a best effort one.
If we want to be more restrictive, possibly we should advocate for NetworkPolicies as documented here or have default Authorization Rule that restrict/allow only certain CIDRs or better enhance envoy DFP to allow configuring some rules.
There was a problem hiding this comment.
Oh wait hmm do gateways even have a passthrough cluster? If not, then there's no issue here because this is a moot concern in sidecars (they're in the same netns as the client application). If this ALLOW_ANY changes the default behavior of e.g egressgateways if there's no ServiceEntry described, then I think the concern is still valid.
I'm also a +1 on enhancing envoy DFP to have a blocklist of ips it will refuse to connect to (e.g. localhost CIDR, link local CIDR, etc.).
There was a problem hiding this comment.
My mental model is to restrict this to Sidecars - I wasn't thinking we will expose this to gateways (We can document that). We can check on how egress gateways work ( haven't used my self)
We can enhance DFP for sure - but if we restrict this to sidecar, it is not a blocker IMO - WDYT?
There was a problem hiding this comment.
Yeah not a blocker if this is restricted to sidecar; my question with that option though is: is MeshConfig the right API for a sidecar only config?
There was a problem hiding this comment.
Right now also the OutboundTrafficPolicy is not honoured/processed for Gateways. The sidecar scope construction does not use it and the CatchAll listener is only added in VirtualOutboundListener - only generated for sidecars. Same with Passthrough Cluster it is not generated for Gateways. So essentially implemented only for sidecar
keithmattix
left a comment
There was a problem hiding this comment.
Need some discussion on security posture re: my last comment /cc @howardjohn
Since DFP is now supported in sidecar mode (see istio/istio#58688) proposal is to introduce a
ALLOW_ANY_DYNAMIC_DNSmode for outbound traffic policy that uses DFP to route unknown traffic. This will enable users to apply TLS config to unknown traffic as well.Related usecase
Related issue: istio/istio#58244
fyi @ramaraochavali