-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Forward Proxy support #1214
Comments
What actually has to change here beyond just handling absolute URLs and inferring :authority out of it? Once that is done, it seems like everything else should just work? This seems like a very reasonable first step to do.
We essentially already have this via wildcard domains and the catch-all '/' route. What else is needed?
I would recommend tackling this one last as it needs a bunch of thinking, especially related to potential inline DNS resolution which is not supported today. We should probably just get the first part working and then focus on CONNECT as a separate issue if needed. |
In general, handling absolute uris seemed sufficient to get something working, but there are some other details that envoy can easily handle and would make this mode more conformant.Specifically there are Connection and Via headers. Now that I've thought about it a bit more concretely, I don't think I'd want the ability to send to upstreams that envoy doesn't know about. That would probably just add a lot of danger. I think there's probably nothing else I'd want to add for #2 at this time. I intend to address CONNECT support separately. In the long run I think I'll want CONNECT but I think some forward support is useful even without it. Also this set of changes probably needs an 'allow_forward_proxy' flag somewhere in the config or there is a potential for users to accidentally configure themselves into being an open proxy. On the connection manager? |
AFAIK Envoy already properly handles connection headers. If not that should be fixed. Via is tracked here: #1030 (and needs to be configurable). |
I didn't see where envoy was cleaning the hop by hop headers referenced in the Connection? Did I miss it? |
Thats what I thought you were referring to, but it's missing part of the handling from: https://tools.ietf.org/html/rfc7230#section-6.1. Connection can contain a list of other headers and if it does, those headers should be removed from the request/response as well, with a special case handling of Connection: close. |
BTW - thats a complete nitpick, its not widely implemented. If you want me to put something in for that then I'll be happy to otherwise I'll sort out the failing tests and add a config option somewhere and send a PR in the next couple of days. |
I've never implemented that in any proxy server I've dealt with, but feel free. In general sounds like we are on the same page so feel free to start sending PRs. |
Here's sort of an initial cut: master...mattwoodyard:forward_proxy. I'd like to add a config option to enable/disable this. There are some codec options for http2 should I do the same with this, or add a new option on the connection manager? |
@mattwoodyard at a high level this looks good to be. Yeah, I would add an http1_settings object that parallels that http2_settings object and them plumb that through in the same way. |
I'm going to call this done. We are tracking CONNECT and generic outbound DNS resolution in different issues. |
…auth (envoyproxy#1216) Automatic merge from submit-queue. Validate JWTs in Istio authentication policy using the output of jwt-auth **What this PR does / why we need it**: POC: validate JWTs in Istio authentication policy using the output of jwt-auth **Which issue this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes envoyproxy#1214 **Special notes for your reviewer**: **Release note**: ```release-note ```
…1214) Description: Expose the histogram stat in `envoy/include/envoy/stats/` to envoy-mobile engine. This will be leveraged in subsequent PRs to support a new set of stats collection in the iOS and Android layers. Risk Level: Medium (due to changes in the core engine impl, but should be Low given no one depends on these APIs yet) Testing: Unit tests, CI, Build Envoy Mobile Library and compile into mobile apps to ensure that changes do not break the build. Docs Changes: Done in https://github.com/envoyproxy/envoy-mobile/blob/main/docs/root/api/stats.rst Release Notes: Expose histogram as a stat exposed via Pulse Signed-off-by: Don Yu <donyu@lyft.com> Signed-off-by: JP Simard <jp@jpsim.com>
…1214) Description: Expose the histogram stat in `envoy/include/envoy/stats/` to envoy-mobile engine. This will be leveraged in subsequent PRs to support a new set of stats collection in the iOS and Android layers. Risk Level: Medium (due to changes in the core engine impl, but should be Low given no one depends on these APIs yet) Testing: Unit tests, CI, Build Envoy Mobile Library and compile into mobile apps to ensure that changes do not break the build. Docs Changes: Done in https://github.com/envoyproxy/envoy-mobile/blob/main/docs/root/api/stats.rst Release Notes: Expose histogram as a stat exposed via Pulse Signed-off-by: Don Yu <donyu@lyft.com> Signed-off-by: JP Simard <jp@jpsim.com>
Envoy currently doesn't support being used as a forward proxy. Forward proxy support would mean that any standard web browser could set an envoy as their proxy and send requests into a service mesh. I've hashed out some initial support in an internal build. I propose to add forward proxy support to envoy in 3 stages. I think that what roughly needs to happen is:
The text was updated successfully, but these errors were encountered: