Skip to content
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
0cb59af
router: adding CONNECT support
alyssawilk Apr 1, 2020
7f134d1
decapsulating *is* a word
alyssawilk Apr 2, 2020
f1fe955
sigh
alyssawilk Apr 2, 2020
6ddcb00
reviewer comments
alyssawilk Apr 6, 2020
21986a9
hidden docs
alyssawilk Apr 8, 2020
e8de80b
Merge branch 'master' into upstream_connect
alyssawilk Apr 8, 2020
0729b25
Merge branch 'master' into upstream_connect
alyssawilk Apr 9, 2020
500c4e5
Merge branch 'master' into upstream_connect
alyssawilk Apr 13, 2020
0286031
Matcher API?
alyssawilk Apr 13, 2020
771c1a6
WIP matcher API use
alyssawilk Apr 13, 2020
b83f9bd
API WIP
alyssawilk Apr 15, 2020
f56e2f0
cleanup
alyssawilk Apr 16, 2020
1f2e9e5
Merge branch 'master' into upstream_connect
alyssawilk Apr 16, 2020
9587888
Merge branch 'master' into upstream_connect
alyssawilk Apr 16, 2020
67a753d
cleanup and tests of new API
alyssawilk Apr 16, 2020
795231f
finally got the config impl test working
alyssawilk Apr 16, 2020
a847f91
default return
alyssawilk Apr 20, 2020
ac38de1
fixing build rule
alyssawilk Apr 20, 2020
9a82f12
tidier
alyssawilk Apr 20, 2020
c3193d2
Merge branch 'master' into upstream_connect
alyssawilk Apr 21, 2020
92d833d
integration tests!
alyssawilk Apr 21, 2020
4434593
Merge branch 'refs/heads/master' into upstream_connect
alyssawilk Apr 21, 2020
8363d5a
reviewer comments
alyssawilk Apr 22, 2020
394c387
Merge branch 'master' into upstream_connect
alyssawilk Apr 22, 2020
72b292a
docs rewording
alyssawilk Apr 22, 2020
e44bfbe
Merge branch 'master' into upstream_connect
alyssawilk Apr 27, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 15 additions & 1 deletion api/envoy/api/v2/route/route_components.proto
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ option (udpa.annotations.file_status).package_version_status = FROZEN;
// host header. This allows a single listener to service multiple top level domain path trees. Once
// a virtual host is selected based on the domain, the routes are processed in order to see which
// upstream cluster to route to or whether to perform a redirect.
// [#next-free-field: 21]
// [#next-free-field: 23]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No more changes allowed to v2, it's frozen :)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand that.
I reverted them and my build seg-faulted. I can't get it to build without the changes.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pastebin of

  1. copying v3 changes into a clean build
  2. running fix_proto
  3. showing the expected files are changed
  4. building tests fails

https://pastebin.com/N8vSXHQM

message VirtualHost {
enum TlsRequirementType {
// No TLS requirement for the virtual host.
Expand All @@ -49,6 +49,13 @@ message VirtualHost {
ALL = 2;
}

// [#not-implemented-hide:]
// Configuration for sending data upstream as a raw data payload. This is used for
// CONNECT requests, when forwarding CONNECT payload as raw TCP.
message ProxyConfig {
// TODO(alyssawilk) add proxy proto configuration here.
}

reserved 9;

// The logical name of the virtual host. This is used when emitting certain
Expand Down Expand Up @@ -180,6 +187,13 @@ message VirtualHost {
// If set and a route-specific limit is not set, the bytes actually buffered will be the minimum
// value of this and the listener per_connection_buffer_limit_bytes.
google.protobuf.UInt32Value per_request_buffer_limit_bytes = 18;

// [#not-implemented-hide:]
// Configuration for sending data upstream as a raw data payload. This is used for terminating
Comment thread
alyssawilk marked this conversation as resolved.
Outdated
// CONNECT requests. If this message is present, the HTTP body of incoming CONNECT request
// will be proxied upstream as raw TCP. Upstream TCP payload will result in Envoy synthesizing
// response headers and forwarding the TCP response payload in the HTTP response body.
ProxyConfig proxying_config = 22;
}

// A filter-defined action type.
Expand Down
19 changes: 18 additions & 1 deletion api/envoy/config/route/v3/route_components.proto
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE;
// host header. This allows a single listener to service multiple top level domain path trees. Once
// a virtual host is selected based on the domain, the routes are processed in order to see which
// upstream cluster to route to or whether to perform a redirect.
// [#next-free-field: 21]
// [#next-free-field: 23]
message VirtualHost {
option (udpa.annotations.versioning).previous_message_type = "envoy.api.v2.route.VirtualHost";

Expand All @@ -50,6 +50,16 @@ message VirtualHost {
ALL = 2;
}

// [#not-implemented-hide:]
// Configuration for sending data upstream as a raw data payload. This is used for
// CONNECT requests, when forwarding CONNECT payload as raw TCP.
message ProxyConfig {
// TODO(alyssawilk) add proxy proto configuration here.

option (udpa.annotations.versioning).previous_message_type =
"envoy.api.v2.route.VirtualHost.ProxyConfig";
}

reserved 9, 12;

reserved "per_filter_config";
Expand Down Expand Up @@ -176,6 +186,13 @@ message VirtualHost {
// If set and a route-specific limit is not set, the bytes actually buffered will be the minimum
// value of this and the listener per_connection_buffer_limit_bytes.
google.protobuf.UInt32Value per_request_buffer_limit_bytes = 18;

// [#not-implemented-hide:]
// Configuration for sending data upstream as a raw data payload. This is used for terminating
// CONNECT requests. If this message is present, the HTTP body of incoming CONNECT request
// will be proxied upstream as raw TCP. Upstream TCP payload will result in Envoy synthesizing
// response headers and forwarding the TCP response payload in the HTTP response body.
ProxyConfig proxying_config = 22;
}

// A filter-defined action type.
Expand Down
19 changes: 18 additions & 1 deletion api/envoy/config/route/v4alpha/route_components.proto
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ option (udpa.annotations.file_status).package_version_status = NEXT_MAJOR_VERSIO
// host header. This allows a single listener to service multiple top level domain path trees. Once
// a virtual host is selected based on the domain, the routes are processed in order to see which
// upstream cluster to route to or whether to perform a redirect.
// [#next-free-field: 21]
// [#next-free-field: 23]
message VirtualHost {
option (udpa.annotations.versioning).previous_message_type = "envoy.config.route.v3.VirtualHost";

Expand All @@ -50,6 +50,16 @@ message VirtualHost {
ALL = 2;
}

// [#not-implemented-hide:]
// Configuration for sending data upstream as a raw data payload. This is used for
// CONNECT requests, when forwarding CONNECT payload as raw TCP.
message ProxyConfig {
// TODO(alyssawilk) add proxy proto configuration here.

option (udpa.annotations.versioning).previous_message_type =
"envoy.config.route.v3.VirtualHost.ProxyConfig";
}

reserved 9, 12;

reserved "per_filter_config";
Expand Down Expand Up @@ -176,6 +186,13 @@ message VirtualHost {
// If set and a route-specific limit is not set, the bytes actually buffered will be the minimum
// value of this and the listener per_connection_buffer_limit_bytes.
google.protobuf.UInt32Value per_request_buffer_limit_bytes = 18;

// [#not-implemented-hide:]
// Configuration for sending data upstream as a raw data payload. This is used for terminating
// CONNECT requests. If this message is present, the HTTP body of incoming CONNECT request
// will be proxied upstream as raw TCP. Upstream TCP payload will result in Envoy synthesizing
// response headers and forwarding the TCP response payload in the HTTP response body.
ProxyConfig proxying_config = 22;
}

// A filter-defined action type.
Expand Down
2 changes: 1 addition & 1 deletion docs/root/intro/arch_overview/http/http.rst
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,5 @@ HTTP
http_connection_management
http_filters
http_routing
websocket
upgrades
http_proxy
Original file line number Diff line number Diff line change
@@ -1,19 +1,19 @@
.. _arch_overview_websocket:

WebSocket and HTTP upgrades
HTTP upgrades
===========================

Envoy Upgrade support is intended mainly for WebSocket but may be used for non-WebSocket
upgrades as well. Upgrades pass both the HTTP headers and the upgrade payload
Envoy Upgrade support is intended mainly for WebSocket and CONNECT support, but may be used for
arbitrary upgrades as well. Upgrades pass both the HTTP headers and the upgrade payload
through an HTTP filter chain. One may configure the
:ref:`upgrade_configs <envoy_api_field_config.filter.network.http_connection_manager.v2.HttpConnectionManager.upgrade_configs>`
with or without custom filter chains. If only the
:ref:`upgrade_type <envoy_api_field_config.filter.network.http_connection_manager.v2.HttpConnectionManager.UpgradeConfig.upgrade_type>`
is specified, both the upgrade headers, any request and response body, and WebSocket payload will
is specified, both the upgrade headers, any request and response body, and HTTP data payload will
pass through the default HTTP filter chain. To avoid the use of HTTP-only filters for upgrade payload,
one can set up custom
:ref:`filters <envoy_api_field_config.filter.network.http_connection_manager.v2.HttpConnectionManager.UpgradeConfig.filters>`
for the given upgrade type, up to and including only using the router filter to send the WebSocket
for the given upgrade type, up to and including only using the router filter to send the HTTP
data upstream.

Upgrades can be enabled or disabled on a :ref:`per-route <envoy_api_field_route.RouteAction.upgrade_configs>` basis.
Expand All @@ -32,12 +32,12 @@ laid out below, but custom filter chains can only be configured on a per-HttpCon
| F | F | F |
+-----------------------+-------------------------+-------------------+

Note that the statistics for upgrades are all bundled together so WebSocket
Note that the statistics for upgrades are all bundled together so WebSocket and other upgrades
:ref:`statistics <config_http_conn_man_stats>` are tracked by stats such as
downstream_cx_upgrades_total and downstream_cx_upgrades_active

Handling HTTP/2 hops
^^^^^^^^^^^^^^^^^^^^
Websocket over HTTP/2 hops
^^^^^^^^^^^^^^^^^^^^^^^^^^

While HTTP/2 support for WebSockets is off by default, Envoy does support tunneling WebSockets over
HTTP/2 streams for deployments that prefer a uniform HTTP/2 mesh throughout; this enables, for example,
Expand All @@ -61,3 +61,27 @@ a GET method on the final Envoy-Upstream hop.

Note that the HTTP/2 upgrade path has very strict HTTP/1.1 compliance, so will not proxy WebSocket
upgrade requests or responses with bodies.

.. TODO(alyssawilk) unhide this when unhiding config
.. CONNECT support
.. ^^^^^^^^^^^^^^^

.. Envoy CONNECT support is off by default (Envoy will send an internall generated 403 in response to

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo: internall -> internally

.. CONNECT requests). CONNECT support can be enabled via the upgrade options described above, setting
.. the upgrade value to the special keyword "CONNECT".

.. Envoy can handle CONNECT in one of two ways, either proxying the CONNECT headers through as if they

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For my own understanding, if we proxy CONNECT through, this is really no different from WebSocket, is that right?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, in that case it's just a blind upgrade where we proxy payload. Want me to comment something to that effect?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From reading this it sounds to me like if we proxy CONNECT we don't do anything special and just treat it like any other request? I think it could be clearer about what happens after the CONNECT is proxied/established.

I think it might also be useful to explain whether we support terminating the CONNECT and dropping down to L4 proxying. I don't think we do (?) but I think it's a use case people might be interested in and it might be worth calling out what our support is there if we don't already.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I think some clarifications would be helpful to explain the differences between WS/CONNECT and termination vs. pass through. With this change terminating WS would be pretty trivial so that could be a future enhancement for someone (might be worth tracking in an issue).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I may have been steeped in the code too long but I'm not sure how I could make this more clear, but I think it's clear from Snow's comment that it's not as clear as I want it to be.

For connect we can basically treat it like normal L7 request, forwarding request headers and payload through, or we can terminate, strip connect headers, and forward the CONNECT payload on a raw TCP connection. I don't know how to explain that more clearly than I am doing so - I'll try for a little rewording in my next push but I'd totally appreciate any help making it clear what the options are.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is fine and we can ship and iterate.

.. were any other request, and letting the upstream handle the CONNECT request, or by terminating the
.. CONNECT request, and forwarding the payload as raw TCP data. When CONNECT upgrade configuration is
.. set up, the default behavior is to proxy the CONNECT request. If termination is desired, this can
.. be accomplished by setting
.. the :ref:`per-route proxying_config <envoy_api_field_route.Route.proxying_config>` or
.. :ref:`per-virtual-host proxying_config <envoy_api_field_route.VirtualHost.proxying_config>`
.. If either is present for CONNECT requests, the router filter will strip the request headers,
.. and forward the HTTP payload upstream. On receipt of initial TCP data from upstream, the router
.. will synthesize 200 response headers, and then forward the TCP data as the HTTP response body.

.. .. warning::
.. This mode of CONNECT support can create major security holes if configured correctly, as the upstream
.. will be forwarded *unsanitized* headers if they are in the body payload. Please use with caution

16 changes: 15 additions & 1 deletion generated_api_shadow/envoy/api/v2/route/route_components.proto

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions include/envoy/router/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ envoy_cc_library(
"//source/common/protobuf",
"//source/common/protobuf:utility_lib",
"@envoy_api//envoy/config/core/v3:pkg_cc_proto",
"@envoy_api//envoy/config/route/v3:pkg_cc_proto",
"@envoy_api//envoy/type/v3:pkg_cc_proto",
],
)
Expand Down
19 changes: 19 additions & 0 deletions include/envoy/router/router.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "envoy/access_log/access_log.h"
#include "envoy/common/matchers.h"
#include "envoy/config/core/v3/base.pb.h"
#include "envoy/config/route/v3/route_components.pb.h"
#include "envoy/config/typed_metadata.h"
#include "envoy/http/codec.h"
#include "envoy/http/codes.h"
Expand Down Expand Up @@ -477,6 +478,15 @@ class VirtualHost {
*/
virtual bool includeAttemptCountInResponse() const PURE;

/**
* If present, this indicates that CONNECT requests to this host should be converted
* to TCP proxying, i.e. the payload of the HTTP body should be sent as raw
* TCP upstream.
* @return configuration for TCP proxying, if present.
*/
using ProxyConfig = envoy::config::route::v3::VirtualHost::ProxyConfig;
virtual const absl::optional<ProxyConfig>& proxyConfig() const PURE;

/**
* @return uint32_t any route cap on bytes which should be buffered for shadowing or retries.
* This is an upper bound so does not necessarily reflect the bytes which will be buffered
Expand Down Expand Up @@ -819,6 +829,15 @@ class RouteEntry : public ResponseEntry {
*/
virtual bool includeAttemptCountInResponse() const PURE;

/**
* If present, this indicates that CONNECT requests on this route should be converted
* to TCP proxying, i.e. the payload of the HTTP body should be sent as raw
* TCP upstream.
* @return configuration for TCP proxying, if present.
*/
using ProxyConfig = envoy::config::route::v3::VirtualHost::ProxyConfig;
virtual const absl::optional<ProxyConfig>& proxyConfig() const PURE;

using UpgradeMap = std::map<std::string, bool>;
/**
* @return a map of route-specific upgrades to their enabled/disabled status.
Expand Down
4 changes: 4 additions & 0 deletions source/common/http/async_client_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,16 @@ const std::vector<Router::ShadowPolicyPtr> AsyncStreamImpl::RouteEntryImpl::shad
const AsyncStreamImpl::NullVirtualHost AsyncStreamImpl::RouteEntryImpl::virtual_host_;
const AsyncStreamImpl::NullRateLimitPolicy AsyncStreamImpl::NullVirtualHost::rate_limit_policy_;
const AsyncStreamImpl::NullConfig AsyncStreamImpl::NullVirtualHost::route_configuration_;
const absl::optional<AsyncStreamImpl::NullVirtualHost::ProxyConfig>
AsyncStreamImpl::NullVirtualHost::proxy_config_nullopt_;
const std::multimap<std::string, std::string> AsyncStreamImpl::RouteEntryImpl::opaque_config_;
const envoy::config::core::v3::Metadata AsyncStreamImpl::RouteEntryImpl::metadata_;
const Config::TypedMetadataImpl<Envoy::Config::TypedMetadataFactory>
AsyncStreamImpl::RouteEntryImpl::typed_metadata_({});
const AsyncStreamImpl::NullPathMatchCriterion
AsyncStreamImpl::RouteEntryImpl::path_match_criterion_;
const absl::optional<AsyncStreamImpl::RouteEntryImpl::ProxyConfig>
AsyncStreamImpl::RouteEntryImpl::proxy_config_nullopt_;
const std::list<LowerCaseString> AsyncStreamImpl::NullConfig::internal_only_headers_;

AsyncClientImpl::AsyncClientImpl(Upstream::ClusterInfoConstSharedPtr cluster,
Expand Down
Loading