Skip to content
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

CONNECT support for HTTP/1.1 upstreams #11308

Closed
nhinds opened this issue May 25, 2020 · 35 comments · Fixed by #13293 or #14623
Closed

CONNECT support for HTTP/1.1 upstreams #11308

nhinds opened this issue May 25, 2020 · 35 comments · Fixed by #13293 or #14623
Labels

Comments

@nhinds
Copy link

nhinds commented May 25, 2020

Description:

I attempted to configure the second scenario from #1451, where Envoy accepts a raw TCP connection and uses HTTP CONNECT to establish a tunnel through an upstream proxy. This currently works fine when the upstream proxy is also Envoy (e.g. connecting envoy --config-path configs/encapsulate_in_connect.v3.yaml --base-id 1 to envoy --config-path configs/terminate_connect.v3.yaml using the sample YAML files), as the Envoy proxies talk HTTP/2 to each other, but it does not work when the upstream proxy only supports HTTP/1.1 as mentioned on #1451:

Client --[plain HTTP]--> Envoy --[HTTP CONNECT]--> Upstream Proxy --[HTTPS]--> Server
We use some upstream non-envoy servers that only support HTTP/1 CONNECT, so HTTP/2 CONNECT wouldn't work for our use case.

It appears that removing http2_protocol_options from the cluster configuration is not sufficient to disable HTTP/2 when tunneling_config has been specified in the listener's TCP proxy filter. This matches the current .proto docs for TunnelingConfig:

Currently, only HTTP/2 is supported

Raising this as a separate feature request as it seems like the HTTP/2 support is working fine.

@mattklein123
Copy link
Member

Yeah let's track this as an independent issue. I think either @alyssawilk or @lambdai are planning on implementing this but I'm not sure of the current thinking.

@alyssawilk
Copy link
Contributor

@lambdai do folks on your end need this? AFIK @chradcliffe and co were interested too.

@froismo
Copy link

froismo commented May 29, 2020

I'm interested in this feature.

@irozzo-1A
Copy link
Contributor

I opened this issue #11569, which is related to this feature request. I would gladly propose my help to implement it.
Disclaimer, I have not lot of experience with C++ and Envoy code base so it may take some time, but I've plenty of motivation ;-). WDYT @alyssawilk @mattklein123 ?

@alyssawilk
Copy link
Contributor

Feel free to give it a shot! It shouldn't be too bad with the HTTP/2 code and tests to model off of :-)

@irozzo-1A
Copy link
Contributor

Ok thx I will give it a try. I will let you know if I encounter any issue.

@ggmm-0
Copy link

ggmm-0 commented Sep 2, 2020

@irozzo-1A, is that in progress?

where Envoy accepts a raw TCP connection and uses HTTP CONNECT to establish a tunnel through an upstream proxy

I have a scenario where I'v got a plain HTTP request and I need to apply some path matching (HTTP Connection Manager), then route it to a cluster through upstream HTTPS (not Envoy) proxy.

It is basically a scenario similar to the one described by @chradcliffe:

Client --[plain HTTP]--> Envoy --[HTTP CONNECT]--> Upstream Proxy --[HTTPS]--> Server

It seems that tunneling_config is only for TCP proxy filter, not HTTP Connection Manager.

@alyssawilk
Am I right that the scenario above is currently not supported?

@irozzo-1A
Copy link
Contributor

@irozzo-1A, is that in progress?

@grzegorz-mirek unfortunately I had to reshuffle my priorities and I did not manage to start working on this yet. I'm still interested on doing that but I cannot give an ETA right now. If you have urgency feel free to take it over.

@alyssawilk
Copy link
Contributor

alyssawilk commented Sep 2, 2020

the config is connect_config on HCM, and tunneling config for TCP proxy.

but yeah, while you can "decap" HTTP/1 or HTTP/2 CONNECT requests into TCP, currently Envoy only "encapsulates" (instantiates CONNECT) for HTTP/2 so unless your upstream supports CONNECT-over-H2 (which Envoy does but many other servers do not) I don't think your use case will be supported until this issue is closed off as implemented.

[edits for correctness]

@chradcliffe
Copy link
Contributor

Internally, we had an immediate requirement to inspect CONNECT-tunnelled traffic, but the solution we came up with isn't necessarily the best. We created an HTTP filter that pushes the CONNECT-tunnelled traffic into a Network::FilterManagerImpl that owns an "inner" HCM. We had to provide a Network::Connection implementation that forwards most of the calls to the real connection, which itself (sadly) requires a const_cast of the real connection.

I'm hoping that the work being done towards #11725 will be the better long-term solution -- I think it also means that we can also process TLS traffic carried by the CONNECT tunnel.

@alyssawilk
Copy link
Contributor

I've been talking to @grzegorz-mirek about this and it turns out he needs to handle L7 HTTP, then encapsulate that in CONNECT, where previously we were talking about using the TCP proxy session to encapsulate CONNECT, which is a somewhat different thing.

I think the best way to do this long term would be to support TCP HTTP/1.1 CONNECT, then forward the normal upstream serialized HTTP traffic to a virtual TCP listener using the work @lambdai is doing. We'd have full CONNECT support for raw proxy more and HTTP/1 via L7, and it could do all the Envoy bells and whistles (say take the payload and wrap in TLS before encapsulating in the CONNECT)

Given that work isn't landed though, it may be worth an interim solution using a custom HTTP upstream (like http/http/upstream_request.cc) which serialized synthetic CONNECT headers before forwarding on [headers][body][data] so it would basically do [CONNECT][headers][body][data]. It would only work with raw HTTP/1.1 and we know we'd want to replace it. Just want to lay the options out here so we can weigh pros and cons.

@mattklein123
Copy link
Member

I just synced up with @alyssawilk on this offline so I think I understand all the permutations now.

It's pretty ugly, but as a stopgap would looping back to a local TCP listener work for your use case? You would essentially use connect_config to loop back to local Envoy. On that listener, you can do whatever you want, and then initiate a CONNECT upstream. We would still need to implement HTTP/1.1 CONNECT for tunneling_config, but that seems like it wouldn't be that bad.

Later, the ugly loop back can be replaced with the internal listener work that @lambdai is doing as a perf optimization?

@lambdai
Copy link
Contributor

lambdai commented Sep 9, 2020

Also see #13001 for a concrete example of looping back to local listener configs. Thanks @wlhee

@ggmm-0
Copy link

ggmm-0 commented Sep 10, 2020

Thanks for all the replies. @mattklein123, do you mean something like that?

client --> HCM --> TcpProxy --CONNECT over HTTP/1.1--> upstream proxy

where HCM and TcpProxy are on the same Envoy instance.

@lambdai, that's a very good example. Thank you.

@alyssawilk
Copy link
Contributor

Yes, that's exactly what we'd suggest for now. It gets you L7 routing, HTTP/1.1 connect with full Envoy filter chain (in case you want to say encrypt the inner payload or outer CONNECT) and hopefully soon you can bypass the loopback listener and save some CPU with @lambdai 's virtual listener route.

@irozzo-1A
Copy link
Contributor

irozzo-1A commented Sep 10, 2020

@alyssawilk Would using UDS instead of a loopback listener to chain the two stages make sense as a small optimisation? I tried a similar approach here

@alyssawilk
Copy link
Contributor

Yeah, using a pipe is fine too. Either way it's basically using the kernel to do 2 passes through Envoy filter chains (one L4, one L7) and should work great. Avoiding the kernel step just helps on performance.

@irozzo-1A
Copy link
Contributor

Probably I will be able to start working on this during this we.

Yesterday I tried to simply remove those lines:

if ((cluster->info()->features() & Upstream::ClusterInfo::Features::HTTP2) == 0) {
ENVOY_LOG(error, "Attempted to tunnel over HTTP/1.1, this is not supported. Set "
"http2_protocol_options on the cluster.");
return false;
}

and I was happily surprised to see that the HTTP/1.1 tunneling seemed to work out of the box. I did a manual test terminating the CONNECT with squid proxy and everything went fine.

@alyssawilk Please let me know if I'm missing something important here and if there are some cases where I should put particular care while writing the tests, but my current understanding (maybe I'm naive :-)) is that my PR should consist of:

@alyssawilk
Copy link
Contributor

Sweet, I was hoping it would be pretty simple!

This is one of the cases where we just want to be really careful about the details.
Offhand I expected tunneling to mostly work but we'll definitely want to

  1. change HttpUpstream::setRequestEncoder to not send host/scheme/path for HTTP/1.1, but send
    CONNECT [hostname]
    which I think can be done setting {Http::Headers::get().Path, hostname} if the configured codec is HTTP/1.1
  2. change HttpUpstream::isValidBytestreamResponse to make sure we only accept well formed HTTP connect responses. I assume this means no content length and no chunked encoding (which might have to be validated in the codec?) but you'll want to check the RFCs and not rely on my memory ;-)
  3. test the heck out of it, making sure we've got integration tests for T-E chunked, whacky content lengths, and tests that the raw HTTP/1.1 headers are what we expect.

And then yeah, update docs :-)

@ggmm-0
Copy link

ggmm-0 commented Sep 23, 2020

@irozzo-1A, I also played with that change and manually tested some scenarios with different proxies that support CONNECT over HTTP/1.1 only. The only issue I found so far is in this scenario (though I'm not fully sure it's related to HTTP/1.1):

client -HTTP-> envoy -CONNECT-over-HTTP/1.1-then-HTTPS-> upstream proxy

Specifying :443 in tunneling_config doesn't help - the upstream proxy tunnels plain HTTP (http://hostname:443) instead of https://hostname. As a workaround, I used loop back to local Envoy suggested above:

client -HTTP-> envoy -HTTPS-> envoy -CONNECT-over-HTTP/1.1-then-HTTPS-> upstream proxy

@irozzo-1A
Copy link
Contributor

@grzegorz-mirek Thx for the info!

I also played with that change and manually tested some scenarios with different proxies that support CONNECT over HTTP/1.1 only. The only issue I found so far is in this scenario (though I'm not fully sure it's related to HTTP/1.1):

client -HTTP-> envoy -CONNECT-over-HTTP/1.1-then-HTTPS-> upstream proxy

From my current understanding I think this cannot be achieved without the two stages, but my opinion is not authoritative ;-)

@alyssawilk
Copy link
Contributor

It depends - if you want to do L7 routing and encapsulate in L4 connect you do the double pass through Envoy. If you just want to take incoming traffic (HTTP or otherwise) and wrap it in CONNECT you only need the one pass.

@irozzo-1A
Copy link
Contributor

It depends - if you want to do L7 routing and encapsulate in L4 connect you do the double pass through Envoy. If you just want to take incoming traffic (HTTP or otherwise) and wrap it in CONNECT you only need the one pass.

Ok, my understanding was good I think.

@grzegorz-mirek, in the scenario you depicted you don't just want to tunnel your incoming plain HTTP traffic with HTTP/1.1 CONNECT, but also using TLS with the end destination (not the upstream proxy). Did I get it right?

@ggmm-0
Copy link

ggmm-0 commented Sep 24, 2020

@irozzo-1A, yes - that's true. Even without L7 routing (for now).

@govale
Copy link

govale commented Oct 29, 2020

Are there any optimistic plans to support this in the near future? 🙂

alyssawilk pushed a commit that referenced this issue Nov 23, 2020
Commit Message:
Additional Description:
Risk Level: Low
Testing: unit test, integration, manual testing
Docs Changes: Added documentation on how to configure Envoy for tunneling TCP over HTTP/1
Release Notes: n/a (still hidden)
Part of #11308

Signed-off-by: Iacopo Rozzo <[email protected]>
@alyssawilk alyssawilk reopened this Nov 23, 2020
@alyssawilk
Copy link
Contributor

oops - leaving this open until the last TODOs are done and config is not hidden.

@irozzo-1A
Copy link
Contributor

thx @alyssawilk, I will try to address the open points ASAP

qqustc pushed a commit to qqustc/envoy that referenced this issue Nov 24, 2020
Commit Message:
Additional Description:
Risk Level: Low
Testing: unit test, integration, manual testing
Docs Changes: Added documentation on how to configure Envoy for tunneling TCP over HTTP/1
Release Notes: n/a (still hidden)
Part of envoyproxy#11308

Signed-off-by: Iacopo Rozzo <[email protected]>
Signed-off-by: Qin Qin <[email protected]>
alyssawilk pushed a commit that referenced this issue Jan 12, 2021
…4623)

Commit Message: Ignore the transfer encoding header in CONNECT responses
Additional Description: NONE
Risk Level: low
Testing: integration test
Docs Changes: NONE
Release Notes: https://github.com/irozzo-1A/envoy/blob/ignore-transfer-encoding/docs/root/version_history/current.rst#new-features
Platform Specific Features: NONE
Fixes #11308

Signed-off-by: Iacopo Rozzo <[email protected]>
qinggniq added a commit to qinggniq/envoy that referenced this issue Jan 25, 2021
Signed-off-by: qinggniq <[email protected]>

add auth helper

Signed-off-by: qinggniq <[email protected]>

refactor mysql server greeting codec

Signed-off-by: qinggniq <[email protected]>

codec encode

Signed-off-by: qinggniq <[email protected]>

add unit test for mysql greeting codec

Signed-off-by: qinggniq <[email protected]>

complete login resp codec

Signed-off-by: qinggniq <[email protected]>

add login message test

Signed-off-by: qinggniq <[email protected]>

feat link error

Signed-off-by: qinggniq <[email protected]>

refactor mysql login resp

Signed-off-by: qinggniq <[email protected]>

pass all codec test

Signed-off-by: qinggniq <[email protected]>

only contain codec change

Signed-off-by: qinggniq <[email protected]>

remove header

Signed-off-by: qinggniq <[email protected]>

http: expose encoded headers/trailers via callbacks (envoyproxy#14544)

In order to support upstream filters passing ownership of headers and
then being able to reference them after the fact, expose a HTTP filter
function that allows reading the header maps back.

Signed-off-by: Snow Pettersen <[email protected]>

Implement request header processing in ext_proc (envoyproxy#14385)

Send request headers to the server and apply header mutations based
on the response. The rest of the protocol is still ignored.

Signed-off-by: Gregory Brail <[email protected]>

1.17.0 release (envoyproxy#14624)

Signed-off-by: Matt Klein <[email protected]>

kick off v1.18.0 (envoyproxy#14637)

Signed-off-by: Matt Klein <[email protected]>

filter manager: drop assert (envoyproxy#14633)

Signed-off-by: Raul Gutierrez Segales <[email protected]>

docs: update ext_proc docs to reflect implementation status (envoyproxy#14636)

Signed-off-by: Gregory Brail <[email protected]>

[tls] Expose ServerContextImpl::selectTlsContext (envoyproxy#14592)

Signed-off-by: Chad Retz <[email protected]>

http: support creating filters with match tree (envoyproxy#14430)

Adds support for wrapping a HTTP filter with an ExtensionWithMatcher proto to create the filters with an associated match tree.

Under the hood this makes use of a wrapper filter factory that manages creating the match tree and adding it to the FM
alongside the associated filter.

Also includes some code to register factories for input/actions, allowing them to be referenced in the proto configuration.

Signed-off-by: Snow Pettersen <[email protected]>

fix empty connection debug logs (envoyproxy#14666)

Fixes envoyproxy#14661

Signed-off-by: Rama Chavali <[email protected]>

tcp_proxy: ignore transfer encoding in HTTP/1.1 CONNECT responses (envoyproxy#14623)

Commit Message: Ignore the transfer encoding header in CONNECT responses
Additional Description: NONE
Risk Level: low
Testing: integration test
Docs Changes: NONE
Release Notes: https://github.com/irozzo-1A/envoy/blob/ignore-transfer-encoding/docs/root/version_history/current.rst#new-features
Platform Specific Features: NONE
Fixes envoyproxy#11308

Signed-off-by: Iacopo Rozzo <[email protected]>

ci: fix docs tag build (envoyproxy#14653)

Signed-off-by: Lizan Zhou <[email protected]>

HTTP health checker: handle GOAWAY from HTTP2 upstreams (envoyproxy#13599)

Makes the HTTP health checker handle GOAWAY properly. When the NO_ERROR
code is received, any in flight request will be allowed to complete, at
which time the connection will be closed and a new connection created on
the next interval.

GOAWAY frames with codes other than NO_ERROR are treated as a health
check failure, and immediately close the connection.

Signed-off-by: Michael Puncel <[email protected]>

upstream: clean up feature parsing code (envoyproxy#14629)

Fixing a perfectly safe and fairly terrible version merge in the ALPN pr the "refactor all upstream config" PRs.
the original code created the new options for new config, and parseFeatures handled parsing features from either the new options, or the old config.  I decided that was too complicated, changed the code to always create the new options struct and forgot to clean up parseFeatures to assume the presence of the new options struct and remove handling things the old style way.

Risk Level: low (clean up inaccessible code)
Testing: added one extra unit test just because
Docs Changes: n/a
Release Notes:  n/a

Signed-off-by: Alyssa Wilk <[email protected]>

upstream: force a full rebuild on host weight changes (envoyproxy#14569)

This will allow us to build load balancers that pre-compute data
structures based on host weights (for example using weighted queues),
to work around some of the deficiencies of EDF scheduling.

This behavior can be temporarily disabled by setting the
envoy.reloadable_features.upstream_host_weight_change_causes_rebuild
feature flag to false.

Fixes envoyproxy#14360

Signed-off-by: Matt Klein <[email protected]>

access log: add support for command formatter extensions (envoyproxy#14512)

Signed-off-by: Raul Gutierrez Segales <[email protected]>

test: improving dynamic_forward_proxy coverage (envoyproxy#14672)

Risk Level: n/a (test only)

Signed-off-by: Alyssa Wilk <[email protected]>

access_logs: removing disallow_unbounded_access_logs (envoyproxy#14677)

Signed-off-by: Alyssa Wilk <[email protected]>

wasm: replace the obsolete contents in wasm-cc's README with docs link (envoyproxy#14628)

Signed-off-by: Kenjiro Nakayama <[email protected]>

grpc-json-transcoder: support root path (envoyproxy#14585)

Signed-off-by: Xuyang Tao <[email protected]>

ecds: add config source for network filter configs (envoyproxy#14674)

Signed-off-by: Kuat Yessenov <[email protected]>

fix comment for parameters end_stream of decodeData/encodeData. (envoyproxy#14620)

Signed-off-by: wangfakang <[email protected]>

[fuzz] Fix bugs in HPACK fuzz test (envoyproxy#14638)

- Use after free because nghttp2_nv object has pointers to the underlying strings and copying them resulted in a use after free when the copy was used after the original was destroyed
- Fixed sorting issues and tested leading/trailing whitespace headers (I can no longer reproduce an issue I saw where a null byte appeared after decoding whitespace, maybe the former fix fixed this)

Risk Level: Low
Testing: Added regression tests and cases for whitespace headers

Fixes
https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=28880
https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=28869

Signed-off-by: Asra Ali <[email protected]>

v3 packages updates for omit_canary_hosts proto (envoyproxy#14117)

Risk Level: LOW
Testing: unit ( proto_format and docs )

part of envoyproxy#12841

Signed-off-by: Abhay Narayan Katare <[email protected]>

streaminfo/mocks: delay filter_state_ dereference (envoyproxy#14612)

By dereferencing filter_state_ in the constructor, any test that sets
filter_state_ will dereference an invalid pointer. This may not be a
common use-case, but it came up when writing some microbenchmarks for
a custom filter where I needed to reset the FilterState on each
iteration of the benchmark.

Signed-off-by: Brian Wolfe <[email protected]>

http: support passing match result action to filter (envoyproxy#14462)

Adds support for passing through a match action from a match tree to the associated HTTP filter.

Some care has to be taken here around dual filters, so we introduce an abstraction that moves handling HttpMatchingData
updates and applying the match result into a FilterMatchState object that is shared between all filter wrappers for a given filter.

This should also avoid having to match twice for dual filters: the match result is shared for both filters, instead of both of
them having to independently arrive at it with the same data.

Signed-off-by: Snow Pettersen <[email protected]>

refactor: use unitfloat in more places (envoyproxy#14396)

Commit Message: Use UnitFloat in place of float in more locations
Additional Description:
UnitFloat represents a floating point value that is guaranteed to be in the range [0, 1]. Use
it in place of floats that also have the same expectation in OverloadActionState and
connection listeners.

This PR introduces no functional changes.

Risk Level: low
Testing: ran affected tests
Docs Changes: n/a
Release Notes: n/a
Platform Specific Features: n/a

Signed-off-by: Alex Konradi <[email protected]>

[tls] add missing built in cipher stat names (envoyproxy#14676)

* add missing ciphers

Signed-off-by: Asra Ali <[email protected]>

docs: adding coverage walkthroguh (envoyproxy#14688)

Risk Level: n/a
Testing: n/a
Docs Changes: adding developer docs
Release Notes: n/a

Signed-off-by: Alyssa Wilk <[email protected]>

local ratelimit: Add descriptor support in HTTP Local Rate Limiting (envoyproxy#14588)

Signed-off-by: Kuat Yessenov <[email protected]>
Co-authored-by: gargnupur <[email protected]>

http: prefetch for upstreams (envoyproxy#14143)

Commit Message: Adding predictive prefetch (useful mainly for HTTP/1.1 and TCP proxying) and uncommenting prefetch config.
Additional Description:
Risk Level: low (mostly config guarded)
Testing: unit, integration tests
Docs Changes: APIs unhidden
Release Notes: inline
Fixes envoyproxy#2755

Signed-off-by: Alyssa Wilk <[email protected]>

docs: Give a hint to specify type_url instead (envoyproxy#14562)

Signed-off-by: Dhi Aurrahman <[email protected]>

Remove flaky_on_windows tag from proxy_filter_integration_test (envoyproxy#14680)

Testing: Ran proxy_filter_integration_test thousands of times

Signed-off-by: Randy Miller <[email protected]>

upstream: Fix moving EDS hosts between priorities. (envoyproxy#14483)

At present if health checks are enabled and passing then moving an EDS host from P0->P1 is a NOOP, and P1->P0 results in an abort.

In the first case:
* P0 processing treats A as being removed because it's not in P0's list of endpoints anymore.
* P0 marks A's existing Host as PENDING_DYNAMIC_REMOVAL. It marks A as having been updated in this config apply.
* P1 skips over A because it is marked as updated in this update cycle already.

In the second case:
* P0 updates the priority on the existing Host. It is appended to the vector of added hosts.
* P1 marks A's existing Host as PENDING_DYNAMIC_REMOVAL. It does adjust the removed host vector as the host is still pending removal.
* A's Host is now in both priorities and is PENDING_DYNAMIC_REMOVAL. This is wrong, and would cause problems later but doesn't have a chance to because:
* onClusterMemberUpdate fires with A's existing Host in the added vector (remember it wasn't removed from P1!)
* HealthChecker attempts to create a new health check session on A, which results in an abort from the destructor of the already existing one.

This was masked in tests by the tests enabling ignore_health_on_host_removal.

We fix this by passing in the set of addresses that appear in the endpoint update. If a host being considered for removal appears in this set,
and it isn't being duplicated into the current priority as a result of a health check address change, then we assume it's being moved and will
immediately remove it.

To simplify the case where a host's health check address is being changed AND it is being moved between priorities we always apply priority moves in
place before we attempt any other modifications. This means such a case is treated as a separate priority move, followed by the health check address change.

fixes envoyproxy#11517

Signed-off-by: Jonathan Oddy <[email protected]>

examples: Add TLS SNI sandbox (envoyproxy#13975)

Signed-off-by: Ryan Northey <[email protected]>

[utility]: Change behavior of main thread verification utility (envoyproxy#14660)

Currently, the isMainThread function can only be called during the lifetime of thread local instance because the singleton that store main thread id is initialized in the constructor of tls instance and cleared in the destructor of tls instance. Change the utility so that outside the lifetime of tls instance, the function return true by default because everything is in main thread when threading is off.

Risk Level: low
Testing: change unit to reflect change of behavior.

Signed-off-by: chaoqin-li1123 <[email protected]>

Windows build: Add repository cache to CI (envoyproxy#14678)

Signed-off-by: Sunjay Bhatia <[email protected]>

ext-proc: Support "immediate_response" options for request headers  (envoyproxy#14652)

This lets ext_proc servers return an immediate HTTP response (such as to indicate an error) in response to a request_headers message.

Signed-off-by: Gregory Brail <[email protected]>

stats: convert tag extractor regexs to Re2 (envoyproxy#14519)

Risk Level: high, the regexes are updated to match more specific patterns.
Testing: unit tests

Fixes envoyproxy#14439

Signed-off-by: Dmitry Rozhkov <[email protected]>

hcm: removing envoy.reloadable_features.early_errors_via_hcm envoyproxy#14641 (envoyproxy#14684)

Risk Level: Low (removal of deprecated disabled code)
Testing: n/a
Docs Changes: n/a
Release Notes: inline
Fixes envoyproxy#14641

Signed-off-by: Alyssa Wilk <[email protected]>

test: Fix O(1/32k) flakiness in H2 flood tests that disable writes based on source port of outgoing connections. (envoyproxy#14695)

It is possible for the kernel to assign the same source port to both the client connection used by
the test framework to connect to the Envoy and the Envoy's client connection to the upstream. When
the source port is reused by both connections, the test client times out while trying to send the
request because disabling write on the upstream connection also disabled writes on the test's client
connection.

Signed-off-by: Antonio Vicente <[email protected]>

tls: add missing stats for signature algorithms. (envoyproxy#14703)

While there, refresh supported cipher suites and add more warnings.

Signed-off-by: Piotr Sikora <[email protected]>

connection: tighten network connection buffer limits (envoyproxy#14333)

Signed-off-by: Antonio Vicente <[email protected]>

xdstp: LDS glob collection support. (envoyproxy#14311)

This patch introduces support for LDS xdstp:// collection URLs for glob collections over ADS. Context
parameters are currently computed from node and resource URLs. Followup PRs will add support for
other collection types (CDS, SRDS), non-ADS, provide dynamic context parameter update, extend support to
singleton resources and then other xdstp:// features (list collections, redirects, alternatives,
etc.)

Part of envoyproxy#11264.

Risk level: Low (opt-in)
Testing: ADS integration test added. Various unit tests following implementation.

Signed-off-by: Harvey Tuch <[email protected]>

listener manager: avoid unique -> shared conversion (envoyproxy#14693)

buildFilterChainInternal() returns a shared_ptr, so let's make
that instead of unique_ptr.

Signed-off-by: Raul Gutierrez Segales <[email protected]>

proto: re-implement RepeatedPtrUtil::hash(). (envoyproxy#14701)

This changes RepeatedPtrUtil::hash() implementation to match
MessageUtil::hash(), which was re-implemented in envoyproxy#8231.

Reported by Tomoaki Fujii (Google).

Signed-off-by: Piotr Sikora <[email protected]>

tcp: setting nodelay on all connections (envoyproxy#14574)

This should have minimal effect, new server side connections had no-delay, codecs set no-delay, and upstream pools set no-delay. Traffic not using the tcp connection pool may be affected as well as raw use of the TCP client.

Risk Level: Medium (data plane)
Testing: new unit tests
Docs Changes: n/a
Release Notes: inline
Runtime guard: envoy.reloadable_features.always_nodelay

Signed-off-by: Alyssa Wilk <[email protected]>

test: Add multiheader TE + Content-Length test (envoyproxy#14686)

Signed-off-by: Yan Avlasov <[email protected]>

http2: Flip the upstream H2 frame flood and abuse checks to ON by default (envoyproxy#14443)

Signed-off-by: Yan Avlasov <[email protected]>

Fix the emsdk patching. (envoyproxy#14673)

If the patch fails, because of `|| true`, bazel continues
the build.

Signed-off-by: Jonh Wendell <[email protected]>

test: print test parameters meaningfully (envoyproxy#14604)

Signed-off-by: Alex Konradi <[email protected]>

Migrate v2 thrift_filter to v3 api and corresponding docs changes. (envoyproxy#13885)

part of envoyproxy#12841

Signed-off-by: Abhay Narayan Katare <[email protected]>

http: reinstating prior connect timeout behavior (envoyproxy#14685)

Signed-off-by: Alyssa Wilk <[email protected]>

Fix typo (envoyproxy#14716)

Signed-off-by: Hu Shuai <[email protected]>

master -> main (envoyproxy#14729)

Various fixes

Signed-off-by: Matt Klein <[email protected]>

readme: fix logo URL (envoyproxy#14733)

Signed-off-by: Matt Klein <[email protected]>

Bump nghttp2 to 1.42.0 (envoyproxy#14730)

- Drops nghttp2 PR1468 patch
- Requires bazel_external_cmake to support copts, defines to drop the rest

Risk Level: low
Testing: CI

Fixes envoyproxy#1417

Signed-off-by: William A Rowe Jr <[email protected]>

Pick up current bazel-build-tools tag (envoyproxy#14734)

Signed-off-by: William A Rowe Jr <[email protected]>

access-logger: support request/response headers size (envoyproxy#14692)

Add following command operator in access logger

%REQUEST_HEADER_BYTES%
%RESPONSE_HEADER_BYTES%
%RESPONSE_TRAILER_BYTES%
Risk Level: Low
Testing: unit test
Docs Changes: done
Release Notes: done

Signed-off-by: Xuyang Tao <[email protected]>

dynamic_forward_proxy: envoy.reloadable_features.enable_dns_cache_circuit_breakers deprecation (envoyproxy#14683)

* dynamic_forward_proxy:  deprecation

Signed-off-by: Shikugawa <[email protected]>

Add support for google::protobuf::ListValue formatting (envoyproxy#14518)

Signed-off-by: Itamar Kaminski <[email protected]>

tls: improve TLS handshake/read/write error log (envoyproxy#14600)

Signed-off-by: Shikugawa <[email protected]>

config: switch from std::set to absl::flat_hash_set for resource names. (envoyproxy#14739)

This was a cleanup deferred from the review of envoyproxy#14311. The idea is to switch to the more efficient
unordered absl::flat_hash_set across the resource subscription code base. Internally, we still use
std::set (and even explicitly sort in the http_subscription_impl) to avoid changing any wire
ordering. It seems desirable to preserve this for two reasons: (1) this derisks this PR as an
internal-only change and (2) having deterministic wire ordering makes debug of xDS issues somewhat
easier.

Risk level: Low
Testing: Updated tests.

Signed-off-by: Harvey Tuch <[email protected]>

network filters: avoid unnecessary std::shared_ptrs (envoyproxy#14711)

While debugging a crash in:

envoyproxy#13592

I ended up discussing with @lambdai and @mattklein123 whether
network filters can hold references to things owned by their
corresponding FactoryFilterCb. The answer is yes and the HCM
and some other notable filters already use references instead
of std::shared_ptrs.

So let's consistently do this everywhere to avoid someone
else asking this same question in the future. Plus, it's
always nice to create fewer std::shared_ptrs.

Follow-up on: envoyproxy#8633

Signed-off-by: Raul Gutierrez Segales <[email protected]>

docs: Updated version history with 1.13.8 release notes. (envoyproxy#14742)

Signed-off-by: Christoph Pakulski <[email protected]>

Dispatcher: keeps a stack of tracked objects. (envoyproxy#14573)

Dispatcher will now keep a stack of tracked objects; on crash it'll "unwind" and have those objects dump their state. Moreover, it'll invoke fatal actions with the tracked objects. This allows us to dump more information during crash.

See related PR: envoyproxy#14509

Will follow up with another PR dumping information at the codec/parser level.

Signed-off-by: Kevin Baichoo <[email protected]>

bootstrap-extensions: fix a crash on http callout (envoyproxy#14478)

Currently when the ServerFactoryContext is passed to bootstrap extensions, it is only partially initialized. Specifically, attempting to access the cluster manager will cause a nullptr access (and hence a crash)

This PR splits the creation and initialized to 2 seperate fucntions. Early creation is required to not break the `default_socket_interface` feature. Once created, the extension will receive the ServerFactoryContext in a different callback (the newly added `serverInitialized`), once they are fully initialized.

Commit Message:
Fix a crash that happens when bootstrap extensions perform http calls.

Additional Description:
Risk Level: Low (small bug-fix)
Testing: Unit tests updated; tested manually with the changes as well.
Docs Changes: N/A
Release Notes: N/A

Fixes envoyproxy#14420

Signed-off-by: Yuval Kohavi <[email protected]>

overload: create scaled timers via the dispatcher (envoyproxy#14679)

Refactor the existing pathway for creating scaled Timer objects away from the
ThreadLocalOverloadState and into the Dispatcher interface. This allows scaled
timers to be created without plumbing through a bunch of extra state.

Signed-off-by: Alex Konradi <[email protected]>

http: removing nvoy.reloadable_features.fix_upgrade_response  envoyproxy#14643 (envoyproxy#14706)

Risk Level: Low (removing deprecated disabled code)
Testing: n/a
Docs Changes: n/a
Release Notes: inline
Fixes envoyproxy#14643

Signed-off-by: Alyssa Wilk <[email protected]>

http: removing envoy.reloadable_features.fixed_connection_close (envoyproxy#14705)

Risk Level: Low (removing deprecated guarded code)
Testing: n/a
Docs Changes: n/a
Release Notes: inline
Fixes envoyproxy#14645

Signed-off-by: Alyssa Wilk <[email protected]>

Revert "network filters: avoid unnecessary std::shared_ptrs (envoyproxy#14711)" (envoyproxy#14755)

This reverts commit 72db81d.

Per discussion in envoyproxy#14717 and via Slack, we'll come up with a different
approach since using a std::function to keep state presents a few
challenges.

Signed-off-by: Raul Gutierrez Segales <[email protected]>

Clarify Consecutive Gateway Failure docs (envoyproxy#14738)

It was initially unclear to me that when
split_external_local_origin_errors is in the default setting of false
that local origin failures will be counted as Consecutive Gateway
Failures. It is clear above that they are counted by the Consecutive 5xx
detection type but since I had that disabled I was surprised to find
them counted in Consecutive Gateway Failure. I think the logic makes
sense though so just attempting to clarify the docs here.

Signed-off-by: Matthew Mead-Briggs <[email protected]>

thrift proxy: fix crash when using payload_passthrough (envoyproxy#14723)

We started seeing crashes triggered by ConnectionManager::passthroughEnabled()
once we enabled `payload_passthrough`. That code assumes that there will
_always_ be an active RPC. However, this is not true after a local response
has been sent (e.g.: no healthy upstream, no cluster, no route, etc.).

Risk Level: low
Testing: unit tests added
Doc Changes: n/a
Release Notes: n/a
Signed-off-by: Raul Gutierrez Segales <[email protected]>

thrift proxy: add comments explaining local replies (envoyproxy#14754)

Risk Level: low
Testing: n/a
Docs Changes: n/a
Release Notes: n/a
Signed-off-by: Raul Gutierrez Segales <[email protected]>

wasm: update V8 to v8.9.255.6. (envoyproxy#14764)

Signed-off-by: Piotr Sikora <[email protected]>

Request headers to add (envoyproxy#14747)

Being consistent about treating Host: and :authority the same way in Envoy header modification.

Risk Level: Medium (changes allowed modifiable headers)
Testing: new unit tests
Docs Changes: yes
Release Notes: inline

Signed-off-by: Alyssa Wilk <[email protected]>

tls: update BoringSSL to fbbf8781 (4324). (envoyproxy#14763)

Signed-off-by: Piotr Sikora <[email protected]>

docs: change getting started order (envoyproxy#14774)

Sandboxes are more relevant to new users than the other sections.

Signed-off-by: Matt Klein <[email protected]>

oauth2: set accept header on access_token request (envoyproxy#14538)

Co-authored-by: Dhi Aurrahman <[email protected]>
Signed-off-by: Richard Patel <[email protected]>

router: Remove envoy.reloadable_features.consume_all_retry_headers (envoyproxy#14662)

This patch removes the
envoy.reloadable_features.consume_all_retry_headers runtime flag.

Signed-off-by: Martin Matusiak <[email protected]>

docs: API review checklist (envoyproxy#14399)

* API review checklist

Signed-off-by: Mark D. Roth <[email protected]>

[docs] Add guidance on ENVOY_BUG in STYLE.md (envoyproxy#14575)

* Add guidanceon ENVOY_BUG and macro usage to STYLE.md

Signed-off-by: Asra Ali <[email protected]>

filters: Add test/server:filter_config_test (envoyproxy#14746)

As part of envoyproxy#14470, I'll be modifying the base filter interface to include an overridable dependencies() function. This is prep work.

Risk Level: Low (test only)
Doc Change: n/a
Release Notes: n/a
Signed-off-by: Auni Ahsan <[email protected]>

dns: removing envoy.reloadable_features.fix_wildcard_matching envoyproxy#14644 (envoyproxy#14768)

Signed-off-by: Alyssa Wilk <[email protected]>

test: FakeUpstream threading fixes (envoyproxy#14526)

Signed-off-by: Antonio Vicente <[email protected]>

server: add FIPS mode statistic indicating FIPS compliance (envoyproxy#14719)

Signed-off-by: Ravindra Akella <[email protected]>

Add error_state to all config dump resources (envoyproxy#14689)

Store the NACKed resource in each resources

Risk Level: None

Fixes: envoyproxy#14431

Signed-off-by: Lidi Zheng <[email protected]>

docs: fix two typos in jwt_authn_filter (envoyproxy#14796)

Signed-off-by: Lukasz Jernas <[email protected]>

 tcp: adding logs and debug checks (envoyproxy#14771)

Adding some logs and one ENVOY_BUG around the new TCP pool.

Signed-off-by: Alyssa Wilk <[email protected]>

google_grpc: attempt to reduce lock contention between completionThread() and onCompletedOps() (envoyproxy#14777)

Holding a stream's lock while running handleOpCompletion can result in the completion queue having to wait until the lock is released before adding messages on that stream to completed_ops_. In cases where the completion queue is shared across multiple gRPC streams, delivery of new messages on all streams is blocked until the lock held by the first stream while executing onCompletedOps.

Signed-off-by: Antonio Vicente <[email protected]>

filters: Add dependencies.proto (envoyproxy#14750)

Introduces the FilterDependency proto. This isn't quite an extension, but it's a common proto to be used by all filter extensions.

Risk Level: Low (proto addition only)

Signed-off-by: Auni Ahsan <[email protected]>

tools: Syncing api/BUILD file to generated_api_shadow (envoyproxy#14792)

After chatting with @akonradi on Slack, it seems the generated_api_shadow/BUILD file was not being updated by proto_format since PR envoyproxy#9719. This PR copies the api/BUILD file to generated_api_shadow.

Risk Level: Low (relevant for development)

Signed-off-by: Adi Suissa-Peleg <[email protected]>

Add debug log for slow config updates for GRPC subscriptions (envoyproxy#14343)

Risk Level: Low
Testing:
Docs Changes: N/A
Release Notes: N/A

Signed-off-by: Adam Schaub <[email protected]>

oauth2 filter: Make OAuth scopes configurable.  (envoyproxy#14168)

New optional parameter 'auth_scopes' added to the filter. The default value is 'user' (if not provided) to avoid breaking changes to users updating to the latest version.

Signed-off-by: andreyprezotto <[email protected]>

Co-authored-by: Nitin Goyal <[email protected]>

upstream: Optimize LoadStatsReporter::startLoadReportPeriod implementation (envoyproxy#14803)

cm_.clusters() is not O(1) in part due to it creating maps and returning by value. This means that startLoadReportPeriod was effectively O(n**2) on number of clusters since cm_.clusters() is called for every active cluster.

Risk Level: low, functional no-op
Testing: Existing tests. We may want a benchmark.

Signed-off-by: Antonio Vicente <[email protected]>

ext_proc: Implement response path for headers only (envoyproxy#14713)

Implement header processing on the response path by sending the
response_headers message to the processor and handling the result.

Also update the docs in the .proto file.

Signed-off-by: Gregory Brail <[email protected]>

reformat code

Signed-off-by: qinggniq <[email protected]>
@cortex93
Copy link

@irozzo-1A, I also played with that change and manually tested some scenarios with different proxies that support CONNECT over HTTP/1.1 only. The only issue I found so far is in this scenario (though I'm not fully sure it's related to HTTP/1.1):

client -HTTP-> envoy -CONNECT-over-HTTP/1.1-then-HTTPS-> upstream proxy

Specifying :443 in tunneling_config doesn't help - the upstream proxy tunnels plain HTTP (http://hostname:443) instead of https://hostname. As a workaround, I used loop back to local Envoy suggested above:

client -HTTP-> envoy -HTTPS-> envoy -CONNECT-over-HTTP/1.1-then-HTTPS-> upstream proxy

Would you mind sharing your configuration ?

@alyssawilk
Copy link
Contributor

Can you share config for

"client -HTTP-> envoy -CONNECT-over-HTTP/1.1-then-HTTPS-> upstream proxy"

that wasn't doing TLS? You'd need to configure TLS on the uptream cluster*, I wouldn't expect 443 on tunneling config to be helpful.

*https://www.envoyproxy.io/docs/envoy/latest/intro/arch_overview/security/ssl

@ggmm-0
Copy link

ggmm-0 commented Mar 2, 2021

@alyssawilk, sorry - I don't have this config anymore. I don't remember exactly, but perhaps the issue was that specifying TLS on the upstream cluster caused CONNECT to the proxy to be encrypted (which my proxy does not support). My goal was to have plaintext HTTP/1.1 CONNECT to the proxy and then TLS in the tunnel to the end server. I achieved that with the loop back to local Envoy as described above.

@cortex93, sorry for the delay, do you want the loop back config?

@alyssawilk
Copy link
Contributor

Ahh, I think (offhand) Envoy today will forward encrypted tcp over a plaintext CONNECT, or encrypt both the payload and the CONNECT request, but won't take in plaintext tcp data, encrypt, send CONNECT in the clear and ciphertext payload in a single pass.

You could accomplish that with the loopback hack, doing tcp proxy adding TLS to a loopback address, taking that now encrypted payload and sending it out as CONNECT. If there's areas we could update the docs to make this more clear, let me know!

@cortex93
Copy link

cortex93 commented Mar 2, 2021

@grzegorz-mirek

Yes, that would be kind

@ggmm-0
Copy link

ggmm-0 commented Mar 3, 2021

@cortex93, this should work: https://gist.github.com/grzegorz-mirek/c028a24ec2b32dfa47b1fdb904ed3baa

@cortex93
Copy link

cortex93 commented Mar 5, 2021

@askquestions
Copy link

I want to ask a related question on dynamic configuration of forwarding raw TCP traffic.

Does anyone have experience in using lds.yaml and cds.yaml to make raw TCP packet to be matched to the filter:
- name: envoy.filters.network.tcp_proxy
typed_config:
"@type": type.googleapis.com/envoy.extensions.filters.network.tcp_proxy.v3.TcpProxy
stat_prefix: tcp
cluster: tcp_proxy_cluster

Can anyone tell me any error in my lds.yaml file shown below? Why envoy proxy just won't identify raw TCP packets by using this lds.yaml in filesystem-based dynamic configuration?

resources:

  • "@type": type.googleapis.com/envoy.config.listener.v3.Listener
    name: listener_0
    address:
    socket_address:
    address: 0.0.0.0
    port_value: 10000
    filter_chains:
    • filters:
      • name: envoy.http_connection_manager
        typed_config:
        "@type": type.googleapis.com/envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager
        stat_prefix: ingress_http
        http_filters:
        • name: envoy.router
          route_config:
          name: local_route
          virtual_hosts:
          • name: local_service
            domains:
            • "*"
              routes:
            • match:
              prefix: "/"
              route:
              cluster: http_proxy_cluster
      • name: envoy.filters.network.tcp_proxy
        typed_config:
        "@type": type.googleapis.com/envoy.extensions.filters.network.tcp_proxy.v3.TcpProxy
        stat_prefix: tcp
        cluster: tcp_proxy_cluster

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet