Extend DestinationRule with tunneling settings #2283
Conversation
|
😊 Welcome @jewertow! 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 contributing guidelines Thanks for contributing! Courtesy of your friendly welcome wagon. |
|
Hi @jewertow. Thanks for your PR. I'm waiting for a istio member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
cf83a1f to
d7d8f5e
Compare
|
/ok-to-test |
| } | ||
|
|
||
| // Configuration for tunneling TCP over HTTP. | ||
| TunnelingSettings tunneling = 6; |
There was a problem hiding this comment.
same question :
IUUC, hosts will be used as tunneling_config.hostname
- Then what if this is wildcard hosts?
- What's the expected behavior if dr has subsets
There was a problem hiding this comment.
- That's one of the limitations that I mentioned in the RFC. Tunneling cannot be applied when a hostname contains a wildcard, because it's not possible to determine what hostname should be set in the
tunneling_config.hostname. I started to work on settingtunneling_config.hostnameautomatically based on SNI, but it might work only for TLS connections. It's also important to note that the same limitation applies to virtual services with multiple hostnames. In such a case, it's also not possible to determine which hostname to choose, so the destination rule must be ignored. - Subsets with tunneling settings are handled like any other traffic policy for a subset.
There was a problem hiding this comment.
@jewertow should there be some validation defined in DR analyzer to ensure all these limitations? Seems to me this can people into trouble quite easily, it will be super nice to reflect the logic/limitation in a specific filter analyzer.
There was a problem hiding this comment.
I included relevant validations in Istio. Should it be done somehow in the API repository? Could you share an example of such a validation?
| // Specifies whether to use CONNECT or POST http method for the upstream tunnel request. | ||
| // If set to true, then POST is used, otherwise CONNECT. | ||
| bool use_post = 2; |
There was a problem hiding this comment.
Not a big fan of use_post as a boolean. What is the default? CONNECT?
I think i prefer a simple method string with CONNECT as the default.
There was a problem hiding this comment.
Thank you for your feedback.
At the beginning I assumed that it will be method = CONNECT/POST, but then I thought that probably CONNECT is the most common case, so if it would be a default setting then the tunneling configuration might be less verbose.
What's more, I was considering to make this API as similar to Envoy API as possible, but it seems that it was not the right idea.
What is the default? CONNECT?
Yes, the CONNECT method is used by default.
There was a problem hiding this comment.
Maybe not expose to users
Since CONNECT is probably the most common use case, it sounds reasonable. We might not expose this setting for now and consider enabling it in the future if necessary.
There was a problem hiding this comment.
@howardjohn @costinm could you share your opinions?
There was a problem hiding this comment.
string "protocol" ( but we already have that in the Service, so not clear why would we need it in DR as well ).
And we should define some protocol names - I think "hbone" can be the normal CONNECT and the default, and we can use hbone-post and other variants.
I think it is pretty clear that tunneling will not be a boolean, but we may need to support multiple protocols and modes. MASQUE and related standards, etc.
There was a problem hiding this comment.
but we already have that in the Service, so not clear why would we need it in DR as well
In my opinion it would be unclear how to route traffic to a service which has specified a tunnel protocol. Let's say you define a service entry with protocol hbone. Which route type do you use in a virtual service? TCP or TLS? How to determine which one should be matched. I think it's tricky and we can't find an intuitive solution.
I think "hbone" can be the normal CONNECT and the default, and we can use hbone-post and other variants.
In my opinion "hbone" might be confusing and users might have problems to understand what is it and how does it work, because I found only one section in Istio docs with this term, but it's not explained there. I also searched in Google the following terms: "hbone", "hbone connect", "hbone protocol" and I didn't get any related article.
On the other hand, even if we will document in detail what hbone is, we still can't assume that users will find what they need, because it's a new term and I guess that users will search in Google terms like "Istio connect support" or "Istio tunneling" rather than "Istio hbone".
There was a problem hiding this comment.
I see another important problem. Relying only on the service protocol would make it possible to tunnel traffic only directly from a sidecar proxy to the forward proxy, and tunneling traffic through a gateway would be impossible, because it requires to route traffic to a gateway and then to the forward proxy. So there are two steps of routing. With the destination rule we are able to choose on which stage to apply tunneling config. On the other hand, when you only have a service protocol, it's not possible to determine when exactly to enable tunneling.
I was investigating how it might be implemented only with service protocol, but there is more problems to identify and discuss. In my opinion it's much more difficult to implement and much less flexible.
There was a problem hiding this comment.
How about 'h2-connect', 'h2-post' ? BTW, 'hbone' and 'better transport' proposals are inspired from https://datatracker.ietf.org/wg/masque/about/ - we could use 'h2-masque'.
Why would be impossible to tunnel through a gateway ? The same would work, we would set the protocol on the gateway port that supports CONNECT. Same for traffic to another app with a sidecar.
We already rely a lot on Service port identifying the protocol - grpc, tls, h2, http, https, etc.
I was thinking about this a bit more - extending DR with a 'protocol' setting that would override the Service is not
a bad thing, there are cases where the port name/protocol in Service can't be modified ( existing apps installed from helm), or needs to be overridden.
There was a problem hiding this comment.
Why would be impossible to tunnel through a gateway ?
I assumed that intermediate tunneling between sidecar and egress gateway is undesired, so I didn't consider it. Maybe API would be simpler, but on the other hand the underlying implementation would be unnecessarily complicated.
| bool use_post = 2; | ||
| } | ||
|
|
||
| // Configuration for tunneling TCP over HTTP. |
There was a problem hiding this comment.
can we add this is for the host configured in the DR?
2d3616f to
366b826
Compare
| @@ -340,6 +340,18 @@ message TrafficPolicy { | |||
| // overridden by port-level settings, i.e. default values will be applied | |||
| // to fields omitted in port-level traffic policies. | |||
| repeated PortTrafficPolicy port_level_settings = 5; | |||
|
|
|||
| message TunnelingSettings { | |||
There was a problem hiding this comment.
I definitely agree with the high level goal of improving/enabling tunneling in Istio. However, I want to make sure this aligns with our intentions to use HTTP/2 CONNECT ('bts' or 'hbone') as base line transport protocol in Istio, as well as general plans to improve Egress traffic in Istio. Can you sync up with @lambdai (the first topic) and @costinm (second topic) to make sure these are aligned?
Additionally, it could be useful to compare initiating the tunnel in Envoy vs in the app; most users that are going to use an egress proxy probably already have it when they adopt Istio, so their apps already have HTTP(s)_PROXY configured. AFAIK this is broken with Istio today.
There was a problem hiding this comment.
Is there any RFC about "bts" or "hbone"? I need to know more context and plans for the future to figure out how tunneling fits these plans. I found only this package and this repository, but still don't know how Istio will use these things.
There was a problem hiding this comment.
Yes, there is a doc describing "Better Transport Security" - we have been slowly working toward it, the support for CONNECT in Envoy and other changes are in part based on that design.
Support for POST as a fallback is IMO a strong requirement, but I don't think it should be exposed in the API, but be implemented at discovery level, like auto MTLS.
There was a problem hiding this comment.
I commented on the istio PR as well - I personally don't think we should add this to DestinationRule at all, in particular in context of the BTS/Hbone proposal to have all in-mesh TCP streams use CONNECT.
We already have Auto-MTLS as an example - and we know the UX is far easier and better than
before, user not having to manually configure this each time.
The Service already has protocol - and we use that across Istio to determine how to connect
(h2, grpc, https, etc), it is IMO much better UX to just use discovery information and auto-configure the protocol used to tunnel.
|
I am also not completely opposed to have something in DestinationRule - if we find some strong use cases where using discovery info is not possible. But I can't think of a use case where we need DR and can't use the Service protocol name. |
| @@ -340,6 +340,19 @@ message TrafficPolicy { | |||
| // overridden by port-level settings, i.e. default values will be applied | |||
| // to fields omitted in port-level traffic policies. | |||
| repeated PortTrafficPolicy port_level_settings = 5; | |||
|
|
|||
| message TunnelSettings { | |||
There was a problem hiding this comment.
Can we flatten it out ? I don't know if this is exclusive to 'tunnels' - what we would do is use the specified protocol and destination_port to override what the service normally define. It is not specific to egress gateways.
Also not sure if 'which HTTP method' is right - we may want in future to extend this to MASQUE ( over QUIC ), or support HAProxy prefix, or tunnel over WS.
There was a problem hiding this comment.
Also 'protocol' is typically lowercase, and we may need to be more precise - h2-connect, h2-post, http-connect (http-post is not possible since http/1.1 POST is not bi-directional in most implementations).
There was a problem hiding this comment.
Used HTTP version depends on the protocol defined in the ServiceEntry configured for a given proxy, so "h2-" and "http-" prefixes can't be used to choose the protocol.
There was a problem hiding this comment.
Can we flatten it out ? I don't know if this is exclusive to 'tunnels'
If you see other potential use cases for these fields, I'm okay with that.
There was a problem hiding this comment.
I was trying to document these fields without the TunnelingSettings wrapper, but then it becomes difficult to specify requirements and semantics clearly. Then configuration lacks the context and validation will be tricky. So after consideration I think it's not worth to flattening these properties.
There was a problem hiding this comment.
Also not sure if 'which HTTP method' is right - we may want in future to extend this to MASQUE ( over QUIC ), or support HAProxy prefix, or tunnel over WS.
But why to document something that is not implemented? It could be document as well once it's implemented.
|
Yes, envoy only supports tunnel config under tcp proxy now. |
|
cc @lambdai is an expert on this aspect. |
|
Right - and CONNECT is only meant for TCP, plain text HTTP is not supposed to use CONNECT, the proxies will get confused and send it as plain text. We need to document ( and ensure ) that tunnel DR is only used for TCP ( including HTTPS/TLS), at least until envoy has support for proper HTTP_PROXY. |
Why? What is going to be broken?
Why? HTTP is built on top of TCP, so what's wrong with it? If you suggest to not support HTTP, because it's not encrypted, we should not support any other non-TLS traffic, but it doesn't make sense, because for plain traffic users can apply TLS origination.
HTTP is TCP as well, so CONNECT proxy don't care what is this traffic. It accepts HTTP CONNECT, establishes connection and send bytes back and forth. Proxies will not get confused, because it's responsibility is only to forward traffic to a target socket. The whole point of such proxies is to not understand the traffic. So there is no difference in tunneling HTTP or HTTPS. I really don't understand why to complicate so simple feature. I want to keep it as simple as possible and don't see any benefit in restricting which application protocols might be tunneled. As far as a protocol is TCP, it should be allowed to be tunneled. Restrictions will complicate semantics, implementation and debugging. |
|
On Fri, Apr 8, 2022 at 4:11 AM Jacek Ewertowski ***@***.***> wrote:
I think we need to figure out what to do about HTTP - because that's
clearly going to be broken.
Why? What is going to be broken?
The protocol ? The RFC is pretty clear about how HTTP proxy is supposed to
work, and it isn't CONNECT.
Plus telemetry, any RBAC rules that expect http attributes, routing, etc.
We can't treat HTTP as TCP.
Why? HTTP is built on top of TCP, so what's wrong with it? If you suggest
to not support HTTP, because it's not encrypted, we should not support any
other non-TLS traffic, but it doesn't make sense, because for plain traffic
users can apply TLS origination.
I meant: our typical example is upgrading HTTP to HTTPS via egress. User
originates HTTP, egress gateway upgrade to HTTPS after applying various
RBAC rules
or routing. If HTTP is treated as TCP - all this is gone.
Besides - if the goal is to support 'standard proxies' like SQUID - you
must follow the RFC, and that is pretty clear about how http proxy is
handled.
I also doubt many properly configured proxies will allow CONNECT on port
80, usually they are used for some policy enforcement.
Right - and CONNECT is only meant for TCP, plain text HTTP is not supposed
to use CONNECT, the proxies will get confused and send it as plain text.
HTTP is TCP as well, so CONNECT proxy don't care what is this traffic. It
accepts HTTP CONNECT, establishes connection and send bytes back and forth.
Proxies will not get confused, because it's responsibility is only to
forward traffic to a target socket. The whole point of such proxies is to
not understand the traffic. So there is no difference in tunneling HTTP or
HTTPS
There is a big difference - https is following the RFC semantics.
Costin
… .
—
Reply to this email directly, view it on GitHub
<#2283 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAUR2U2TOTRFEW65RWAQ2DVEAH5NANCNFSM5QZ5Y7TA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
It seems that I didn't understand what you mean by treating HTTP as TCP and we talk about different things. |
|
The examples seem to configure port 8080 as TCP - so a tcp proxy is
created. That's wrong IMO, it means http telemetry and any RBAC in the
egress proxy
that uses http attributes will be gone. It may pass the test, since it's
not including this.
Neither in-cluster nor outbound HTTP should be treated as TCP, and I've
seen many proxies that reject CONNECT traffic that doesn't look encrypted
or even if dest is not 443 (or some small set of ports), in some cases
doing MITM to apply further policies.
…On Fri, Apr 8, 2022 at 7:36 AM Jacek Ewertowski ***@***.***> wrote:
It seems that I didn't understand what you mean by treating HTTP as TCP
and we talk about different things.
I didn't wanted to say that HTTP proxy should be treated as TCP. I was
thinking that you mean that outbound HTTP traffic can't be treat as TCP.
HTTP proxy obviously shouldn't be treated as TCP service. To avoid
misunderstanding please take a look at this pull request:
istio/istio#37968 <istio/istio#37968>. There are
examples in the directory tests/integration/pilot/tunneling.
—
Reply to this email directly, view it on GitHub
<#2283 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAUR2W4EBUWAYP56MFISDLVEA77JANCNFSM5QZ5Y7TA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
| @@ -340,6 +340,26 @@ message TrafficPolicy { | |||
| // overridden by port-level settings, i.e. default values will be applied | |||
| // to fields omitted in port-level traffic policies. | |||
| repeated PortTrafficPolicy port_level_settings = 5; | |||
|
|
|||
There was a problem hiding this comment.
I mentioned this in the WG meeting but will reiterate it here. I think it's critical we have a strategy planned out for how this relates to user initiated CONNECT; not suggesting implementing that is a blocker but i would like to see some discussion of the long term plans around that so we can guide users, or docs, and our future plans.
There was a problem hiding this comment.
We (at RedHat) have no other plans related to tunneling or initiating CONNECT requests. I proposed the simplest API I can see. I know that you had other ideas similar to auto-mtls or service protocol, but both have flaws which I explained above. As far as Envoy is not going to provide nor extend its tunneling capabilities, I can't see alternative solutions.
It's important to note that this API does not block you to provide other approach for this problem in the future - as is in case of mTLS which can be applied both with DestinationRule and PeerAuthentication.
Yes, but for TLS traffic you also can't apply HTTP telemetry, so it's not a matter of whether the traffic is encrypted or not. On the other hand why do we have to assume that CONNECT proxies must reject not encrypted traffic? It's not the problem of Istio. I don't care how users configure external components.
It's not wrong. It's absolutely intentional configuration, because only TcpProxy enables tunneling, so HttpConnectionManager can't be used. |
|
On Fri, Apr 8, 2022 at 8:09 AM Jacek Ewertowski ***@***.***> wrote:
The examples seem to configure port 8080 as TCP - so a tcp proxy is
created. That's wrong IMO, it means http telemetry and any RBAC in the
egress proxy
that uses http attributes will be gone. It may pass the test, since it's
not including this.
Yes, but for TLS traffic you also can't apply HTTP telemetry, so it's not
a matter of whether the traffic is encrypted or not.
Yes, for TLS and HTTPS traffic we can't apply HTTP telemetry or RBAC (
without MITM), and according to the RFC CONNECT is
the proper proxy methid . But for HTTP traffic - we can, and there is a
clear standard on how a HTTP proxy should behave.
Wasn't the purpose of this change to allow users to integrate with proxies
?
On the other hand why do we have to assume that CONNECT proxies must
reject not encrypted traffic? It's not the problem of Istio. I don't care
how users configure external components.
One of the main purposes of HTTP proxies is to enforce enterprise policies.
You would reject plain text traffic on CONNECT because it's a clear
violation of the RFC that the proxy implements, and a sign of likely abuse
( circumventing the http checks - like what hosts you are allowed to
connect).
Advanced proxies to MITM - so typically CONNECT is decrypted (using roots
that get distributed to all machines) and policy enforced.
BTW - AFAIK almost all proxies support 'transparent proxy' mode, so just
forwarding HTTP to the proxy with the original Host: header will work fine
( proxies are also sometimes deployed via interception )
so a tcp proxy is created. That's wrong IMO
It's not wrong. It's absolutely intentional configuration, because only
TcpProxy enables tunneling, so HttpConnectionManager can't be used.
Well - it is wrong because it violates the RFC implemented by proxies, and
the user expectations on how telemetry and RBAC will work for L7 traffic.
Users having to configure HTTP as L4 as a workaround for Envoy
implementation limitations doesn't seem right. I understand you can
intentionally miss-configure to work around envoy - and it may work in some
cases ( if egress doesn't do any Authz or L7 routing for example).
Message ID: ***@***.***>
… |
|
For http, does n't some thing similar will help https://github.com/envoyproxy/envoy/blob/8cb6862fe6099cd8583a64ff037ecdeaf0e939fa/configs/proxy_connect.yaml#L36? |
There was a problem hiding this comment.
Adding this to DR has few confusing semantics i.e. can you specify all other settings in the TrafficPollcy if you want to use the Tunnel protocol? Settings like TLS, connection level settings and load balancer settings?
Since the tunneling happens over HTTP the VS associated with this should be an HTTP block and the service port prefix of the upstream should be http*, is that correct?
| // connect - uses HTTP CONNECT; | ||
| // post - uses HTTP POST. | ||
| // HTTP version for upstream requests is determined by the service protocol defined for the proxy. | ||
| string protocol = 1 [(google.api.field_behavior) = REQUIRED]; |
There was a problem hiding this comment.
Tunneling protocols are much more bounded than application protocols so it made sense to me keep service protocol as string so that we don't change our APIs often. If we expect new tunneling protocols to be supported by Envoy very frequently then I can understand keeping this as string else I would prefer a ENUM here.
|
There is no such thing as "CONNECT tunneling for HTTP" - the standard that
defines CONNECT also defines how to proxy HTTP, using
the absolute URL - you can't change the verb from POST/GET/etc to CONNECT
or use CONNECT and then send the HTTP request as the body.
We need to stop inventing weird protocols and follow the standards.
…On Mon, Apr 11, 2022 at 2:55 AM Neeraj Poddar ***@***.***> wrote:
***@***.**** commented on this pull request.
Adding this to DR has few confusing semantics i.e. can you specify all
other settings in the TrafficPollcy if you want to use the Tunnel protocol?
Settings like TLS, connection level settings and load balancer settings?
Since the tunneling happens over HTTP the VS associated with this should
be an HTTP block and the service port prefix of the upstream should be
http*, is that correct?
------------------------------
In networking/v1alpha3/destination_rule.proto
<#2283 (comment)>:
> @@ -340,6 +340,22 @@ message TrafficPolicy {
// overridden by port-level settings, i.e. default values will be applied
// to fields omitted in port-level traffic policies.
repeated PortTrafficPolicy port_level_settings = 5;
+
+ message TunnelSettings {
+ // Specifies which protocol to use for tunneling the downstream connection.
+ // Supported protocols are:
+ // connect - uses HTTP CONNECT;
+ // post - uses HTTP POST.
+ // HTTP version for upstream requests is determined by the service protocol defined for the proxy.
+ string protocol = 1 [(google.api.field_behavior) = REQUIRED];
Tunneling protocols are much more bounded than application protocols so it
made sense to me keep service protocol as string so that we don't change
our APIs often. If we expect new tunneling protocols to be supported by
Envoy very frequently then I can understand keeping this as string else I
would prefer a ENUM here.
—
Reply to this email directly, view it on GitHub
<#2283 (review)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAUR2UOC5G3HKLWCPBJY5LVEPZJTANCNFSM5QZ5Y7TA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
@nrjpoddar I don't know what's confusing in your opinion.
There is |
I don't know what's weird for you. It's as simple as possible. If you don't agree to support plain TCP traffic, let's support at least TLS traffic. The main purpose of this PR is to provide API which enable users to eliminate configuring HTTP proxies in their apps by setting envs like |
|
I do agree with supporting TLS and TCP traffic, via TcpProxy.
App-originated HTTPS too.
I am also ok with supporting HTTP proxies - equivalent with setting
HTTP_PROXY and HTTPS_PROXY.
But setting HTTP_PROXY is NOT using CONNECT, and all HTTP proxies implement
the RFC, which is to use
the absolute URL. The configs you have - treating HTTP as a TCP connection
- is not the same thing with setting HTTP_PROXY and
is not supported/used by http proxies, and is changing the semantics of
Istio, that's what I don't like.
…On Mon, Apr 11, 2022 at 1:53 PM Jacek Ewertowski ***@***.***> wrote:
We need to stop inventing weird protocols and follow the standards.
I don't know what's weird for you. It's as simple as possible. If you
don't agree to support plain TCP traffic, let's support at least TLS
traffic.
The main purpose of this PR is to provide API which enable users to
eliminate configuring HTTP proxies in their apps by setting envs like
HTTP_PROXY or properties like java -Dhttps.proxyHost=host
-Dhttps.proxyPort=port.
If "tunneling API" is too broad topic and you have other plans that
conflict with this use case and submitted API, what do you think to change
it to HttpsProxy and keep it simple and support only TLS?
—
Reply to this email directly, view it on GitHub
<#2283 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAUR2R263WSK36VWV4J7ADVESGMPANCNFSM5QZ5Y7TA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
I tested multiple commonly used HTTP tools and libraries (Java, NodeJS, curl...) and all of them use HTTP CONNECT when HTTP proxy is configured, so I don't understand why you still say that it's weird protocol and why you suggest that proxies does not support it. I also inspected the traffic with Wireshark and it works as expected. Could you share the RFC which you are referring to?
and I can't see there any information about absolute URL which you talk about. Both these RFCs says that tunnel should be established with the following request and it makes sense, because the absolute URL that you want connect to will be sent once the connection is established. |
|
Quick search:
https://stackoverflow.com/questions/7577917/how-does-a-http-proxy-utilize-the-http-protocol-a-proxy-rfc
Can you share wireshark trace for any of those languages - with HTTP_PROXY
set ( for a http:// request, not https:// ) ?
The go implementation is net/http/request.go is
if usingProxy && r.URL.Scheme != "" && r.URL.Opaque == "" {
ruri = r.URL.Scheme + "://" + host + ruri
....
and transport.go
case cm.targetScheme == "http":
pconn.isProxy = true
if pa := cm.proxyAuth(); pa != "" {
pconn.mutateHeaderFunc = func(h Header) {
h.Set("Proxy-Authorization", pa)
}
}
case cm.targetScheme == "https":
conn := pconn.conn
var hdr Header
if t.GetProxyConnectHeader != nil {
var err error
hdr, err = t.GetProxyConnectHeader(ctx, cm.proxyURL, cm.targetAddr)
if err != nil {
conn.Close()
return nil, err
}
I can probably find the java implementation as well.
…On Tue, Apr 12, 2022 at 4:39 AM Jacek Ewertowski ***@***.***> wrote:
But setting HTTP_PROXY is NOT using CONNECT, and all HTTP proxies
implement the RFC
I tested multiple commonly used HTTP tools and libraries (Java, NodeJS,
curl...) and all of them use HTTP CONNECT when HTTP proxy is configured, so
I don't understand why you still say that it's weird protocol and why you
suggest that proxies does not support it. I also inspected the traffic with
Wireshark and it works as expected.
Could you share the RFC which you are referring to?
I read the following RFCs:
- HTTP CONNECT:
https://datatracker.ietf.org/doc/html/rfc7231#section-4.3.6
- Tunneling TCP based protocols through Web proxy servers:
https://datatracker.ietf.org/doc/html/draft-luotonen-web-proxy-tunneling-01
and I can't see there any information about absolute URL which you talk
about. Both these RFCs says that tunnel should be established with the
following request
CONNECT server.example.com:80 HTTP/1.1
Host: server.example.com:80
and it makes sense, because the absolute URL that you want connect to will
be sent once the connection is established.
—
Reply to this email directly, view it on GitHub
<#2283 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAUR2UVBXMH5LZEGVXWQ3DVEVOG3ANCNFSM5QZ5Y7TA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
Ok, now I understand what you mean and you're right that setting HTTP or HTTPS proxies in most clients don't use HTTP CONNECT for HTTP connection initiated by a client, but for TLS connections proxies use HTTP CONNECT. I think we look at this feature from completely different perspectives. and then at section "2.6. Protocol Versioning": As you can see there is no information how tunnel proxies have to treat HTTP requests, because their purpose is to be blind, to not process received messages and just establish TCP connection between a client and an origin server. As far as this API is named |
|
I am not actually opposed to supporting the CONNECT tunnel for HTTP
connections ( in particular for HTTP/2 plain text), if:
- we find some non-Istio proxies that support this and don't get confused
- we implement it in a way that doesn't break Istio own APIs.
Maybe I was not clear in my comments - the later point is very important
and what started the discussion, in your example the HTTP port is labeled
as TCP to
force the use of the TCP proxy which supports CONNECT. That breaks
telemetry and policies operating on HTTP attributes.
We could either make changes in Envoy, or generate a config that still
treats the request as HTTP, applies all policies/telemetry, including mTLS
- but does an internal forward to
a local Tcp cluster that does the upgrade to CONNECT. This is IMO the
'right' use of CONNECT, for tunneling encrypted traffic and preserving mTLS.
…On Wed, Apr 13, 2022 at 8:52 AM Jacek Ewertowski ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In networking/v1alpha3/destination_rule.proto
<#2283 (comment)>:
> @@ -340,6 +340,26 @@ message TrafficPolicy {
// overridden by port-level settings, i.e. default values will be applied
// to fields omitted in port-level traffic policies.
repeated PortTrafficPolicy port_level_settings = 5;
+
We (at RedHat) have no other plans related to tunneling or initiating
CONNECT requests. I proposed the simplest API I can see. I know that you
had other ideas similar to auto-mtls or service protocol, but both have
flaws which I explained above. As far as Envoy is not going to provide nor
extend its tunneling capabilities, I can't see alternative solutions.
It's important to note that this API does not block you to provide other
approach for this problem in the future - as is in case of mTLS which can
be applied both with DestinationRule and PeerAuthentication.
—
Reply to this email directly, view it on GitHub
<#2283 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAUR2SFTMYUAEVCVJIHAC3VE3USBANCNFSM5QZ5Y7TA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Do you mean to ensure which proxies would it work with? I was testing it mostly with Envoy and there are no restrictions. I was also using squid and nginx with proxy_connect module. I could also test it with apache http. It's definitely possible to support HTTP port without changing anything in Envoy, but it's much more complex solution. I was testing redirecting traffic from HttpConnectionManager to a TcpProxy listening on unix domain socket and it worked fine. So what do you think to start with support only for TLS traffic and then investigate support for plain TCP and HTTP in another pull request? Tunneling TLS is the most important case and I don't want to block it with discussions about plain HTTP. Edit: I also want to remember once again why both
So at the first glance it might seem that specifying these properties is redundant, but because of them there are no limitations for a related virtual service. |
|
On Fri, Apr 15, 2022 at 7:54 AM Jacek Ewertowski ***@***.***> wrote:
we find some non-Istio proxies that support this and don't get confused
Do you mean to ensure which proxies would it work with? I was testing it
mostly with Envoy and there are no restrictions. I was also using squid and
nginx with proxy_connect module. I could also test it with apache http.
I'm sure all will "work" - treating it as a TCP stream. What I mean is that
proxy may have http-level settings/policies - like allow some domains or
block others, or keep track of the domains. It's the main reason proxies
were required. I don't know how much those days with https.
The fundamental problem is treating HTTP as TCP - both in Istio and in the
proxy.
It's definitely possible to support HTTP port without changing anything in
Envoy, but it's much more complex solution. I was testing redirecting
traffic from HttpConnectionManager to a TcpProxy listening on unix domain
socket and it worked fine.
Yes, it is more complex, but I think it is necessary. There are some
changes to use an internal redirection and further optimize.
So what do you think to start with support only for TLS traffic and then
investigate support for plain TCP and HTTP in another pull request?
Tunneling TLS is the most important case and I don't want to block it with
discussions about plain HTTP.
I agree. Let's just document in the API that it will not apply to HTTP or
H2 for now - and not include any of the examples treating http as tcp.
Message ID: ***@***.***>
… |
|
|
||
| // Configuration of tunneling TCP over other transport or application layers | ||
| // for the host configured in the DestinationRule. | ||
| // Tunnel settings can be applied to TCP or TLS routes and can't be applied to HTTP routes. |
There was a problem hiding this comment.
I will remove this note once I submit support for HTTP routes.
47c5a20 to
362698b
Compare
| string target_host = 2 [(google.api.field_behavior) = REQUIRED]; | ||
|
|
||
| // Specifies a port to which the downstream connection is tunneled. | ||
| uint32 target_port = 3 [(google.api.field_behavior) = REQUIRED]; |
There was a problem hiding this comment.
I know we traditionally have done things this way, but virtually everyone is using hostnames:port or URLs, there is no need to be so verbose and ask the user to act as a URL parser. Why not:
// Specify the host to which connection is tunneled, as host:port or host or URL
string target
HTTPS_PROXY is a host:port.
I know using URL is controversial in Istio for some reason, so treat this as a rant, not blocking.
There was a problem hiding this comment.
To be honest, it completely doesn't matter for me. So I would like to know the opinion of API maintainers.
What do you think @linsun?
There was a problem hiding this comment.
I think targetHost and targetPort is pretty simple for user to use, i could be convinced to use below but also felt targetHost and targetPort offers more clarity.
target: {host:port}
| string protocol = 1 [(google.api.field_behavior) = REQUIRED]; | ||
|
|
||
| // Specifies a host to which the downstream connection is tunneled. | ||
| // Target host must be an FQDN. |
There was a problem hiding this comment.
Why not IP ? HTTPS_PROXY does allow this, and it is common to not have a DNS name for proxies (many times
they are used over VPN or hosts where the DNS resolver is hard to change, since setting the proxy can be done by non-root users).
There was a problem hiding this comment.
That's true, thanks. I just forgot to mention about it. I added this information and pushed new commit.
TunnelSettings enables tunneling TCP traffic over other transport or application layers. Istio will initially support tunneling TCP over HTTP or H2 using CONNECT or POST methods, but the supported protocols list might be extended in the future. At the beginning tunnel settings will be applicable to TCP or TLS routes only, but support for HTTP routes is also on the roadmap. Signed-off-by: Jacek Ewertowski <jewertow@redhat.com>
362698b to
793b356
Compare
Signed-off-by: Jacek Ewertowski jewertow@redhat.com
Background context
This change aims to enable tunneling outbound traffic. I described the idea in detail in this RFC.
The API in this pull request differs slightly from that described in the document, but I decided to make it as close to Envoy API as possible. What's more, I didn't mention about field
destination_portin the RFC, because I forgot that port matching is optional in a VirtualService configuration, so without explicitly defined destination port it might be not possible to configureTcpProxy.tunneling_config.hostnamein some cases.This API change is related to this pull request: istio/istio#37968.