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

4.x: Initial support for proxy protocol V1 and V2 #7829

Merged
merged 8 commits into from
Nov 20, 2023

Conversation

spericas
Copy link
Member

@spericas spericas commented Oct 18, 2023

Description

This PR includes an implementation of the proxy protocol V1 and V2 as described here https://www.haproxy.org/download/1.8/doc/proxy-protocol.txt. This feature can be enabled on a listener configuration:

    /**
     * Enable support for proxy protocol for this socket.
     * Default is {@code false}.
     *
     * @return proxy support status
     */
    @Option.Default("false")
    boolean enableProxyProtocol();

Once enabled, it is disabled by default, the proxy protocol header MUST be present in the connection (only for the first request if multiple ones are sent over a long-lived connection). The feature is supported for HTTP/1.1 and HTTP2 and can also be used in combination with TLS (proxy protocol header is processed before the SSL handshake).

The proxy protocol version, either V1 or V2, is automatically discovered by inspecting the first few bytes of a connection. The data collected from the protocol header can be obtained from a ServerRequest as shown below:

ServerRequest request = ...
ProxyProtocolData data = request.proxyProtocolData().ifPresent(ppd -> ...);

It addition, a subset of the proxy protocol data is made available as the HTTP headers X-Forwarded-For and X-Forwarded-Port. These headers can also be used in combination with RequestedUriDiscoveryContext when configured to inspect the aforementioned headers.

Other than unit and integration tests, both the V1 and V2 protocol headers have been manually tested to be compatible with Wireshark's decoder.

Documentation

I shall create a new follow-up issue to document this new features, if and when it is merged.

@spericas spericas self-assigned this Oct 18, 2023
@oracle-contributor-agreement oracle-contributor-agreement bot added the OCA Verified All contributors have signed the Oracle Contributor Agreement. label Oct 18, 2023
@spericas spericas marked this pull request as draft October 18, 2023 14:15
@spericas spericas force-pushed the issue-7672 branch 2 times, most recently from 8b465f8 to e2d25ef Compare October 23, 2023 15:31
Signed-off-by: Santiago Pericasgeertsen <santiago.pericasgeertsen@oracle.com>
@spericas spericas marked this pull request as ready for review October 23, 2023 20:12
@spericas spericas changed the title 4.x: Initial support for proxy protocol data V1 and V2 4.x: Initial support for proxy protocol V1 and V2 Oct 23, 2023
@spericas spericas added this to the 4.x milestone Oct 23, 2023
@tjquinno
Copy link
Member

tjquinno commented Oct 24, 2023

The revised Http1Connection and Http2Connection classes set the non-standard (but oft-used) X-Forwarded-For and X-Forwarded-Port headers but not the standard Forwarded header.

Should the code allow users to configure which family of header to use (maybe both??) and then set the corresponding header(s)?

I see that the proxy spec (thanks for the link) uses the X-Forwarded-* headers in the text but also mentions in passing Forwarded. It's not clear from my quick read whether the proxy spec requires X-Forwarded-* or happened to use that as an example.

Given that our requested URI support allows users to configure one or the other or both families of headers for that feature, should the proxy support do the same?

Or, perhaps, just always set both Forwarded and the X-Forwarded-* family?

@spericas
Copy link
Member Author

spericas commented Oct 24, 2023

The revised Http1Connection and Http2Connection classes set the non-standard (but oft-used) X-Forwarded-For and X-Forwarded-Port headers but not the standard Forwarded header.

Should the code allow users to configure which family of header to use (maybe both??) and then set the corresponding header(s)?

I see that the proxy spec (thanks for the link) uses the X-Forwarded-* headers in the text but also mentions in passing Forwarded. It's not clear from my quick read whether the proxy spec requires X-Forwarded-* or happened to use that as an example.

Given that our requested URI support allows users to configure one or the other or both families of headers for that feature, should the proxy support do the same?

Or, perhaps, just always set both Forwarded and the X-Forwarded-* family?

It's obviously possible to support both. I wanted to do the minimum without consulting config and without creating formatted strings, to avoid adding more code to the request processing loop. Using the X headers seems more efficient and it is one of the options supported by the existing URI mechanism. Perhaps we can start with something simple and provide other options if/when customer feedback is received.

@barchetta barchetta linked an issue Nov 13, 2023 that may be closed by this pull request
@spericas spericas merged commit dc4db18 into helidon-io:main Nov 20, 2023
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OCA Verified All contributors have signed the Oracle Contributor Agreement.
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Ability for helidon server to handle ProxyProtocol
3 participants