Proxy protocol support does not handle proxy payload arriving before TLS

[kylepl]

I'm new to the Proxy protocol, but from what I can tell:

• When using a TLS-passthrough for a load balancer (mine happens to be DigitalOcean), Proxy protocol is added before the TLS connection starts (since it has no way to add it just after it starts, of course, if it is passthrough).
• The ProxyParser in uWS is called in HttpParser::getHeaders, which is after TLS parsing.

Thus, even with Proxy parsing enabled, the TLS connection fails since it is trying to parse the Proxy protocol.

I confirmed that if I hacked ssl_on_data in openssl.c, that I can see the proxy packets as expected.

So the open questions I have are:

• Is it important to support parsing the Proxy protocol both before and after TLS, since it could come either time? My specific case is before, but it could be either in theory. The current logic is implemented to support after.
• Open to suggestions on a reasonable approach for implementing it that I could submit as a PR. My brief investigation makes me consider sticking the parsed proxy data into loop_ssl_data, and then still exposing it similarly as today through at least HttpResponse.

[uNetworkingAB]

This is exactly the issue I'm having myself. Do you know a simple config to a local haproxy that triggers the issue?

[uNetworkingAB]

sticking the parsed proxy data into loop_ssl_data

It can't be per-loop data since many connections share the same loop. It must be per-socket.

Is it important to support parsing the Proxy protocol both before and after TLS

I guess it kind of needs to support both, because if you do TLS <-> TLS it would come after but if you do TCP <-> TLS it would come before like you detailed. With TCP <-> TCP it essentially is before, so it's only really TLS <-> TLS that is after, so maybe low prio for that one but it sounds like it could happen

[uNetworkingAB]

Sounds pretty much that proxy protocol must be implemented in uSockets entirely, not in uWS.

[uNetworkingAB]

Can you test this library and see if it is compatible with uWS as is: https://github.com/kosmas-valianos/libproxyprotocol

[kylepl]

This is exactly the issue I'm having myself. Do you know a simple config to a local haproxy that triggers the issue?

I have never used haproxy before, I've only triggered the situation with DigitalOcean loadbalancers (which are closed source, and thus, unfortunately, not good for testing).

It can't be per-loop data since many connections share the same loop. It must be per-socket.

Ah, yes, makes sense.

And sounds reasonable to support both.

Can you test this library and see if it is compatible with uWS as is: https://github.com/kosmas-valianos/libproxyprotocol

This is GPL/LGPL, which I believe is incompatible with this Apache license.

[kylepl]

An Apache'd license library is: https://github.com/s8sg/proxy_proto_c/tree/master.

[kylepl]

Ok, I took a swing at it, using https://github.com/s8sg/proxy_proto_c - and managed to get it working. I need to clean it up before sharing, but a few notes to speed up the process:

• As you mentioned, I think it makes sense to move it all into uSockets.
  • Thus I would delete ProxyParser.h from uWebSockets
• To implement it in uSockets, use proxy_proto_c, and expose us_socket_proxy_remote_address, in addition to us_socket_remote_address.
• The data is stored on us_socket_t.
• The extraction is being done in us_internal_dispatch_ready_poll, checking to see if it's been done yet, before calling s->context->on_data (thus it covers both TLS and non-TLS).
• It currently only supports getting the proxy at the start (before TLS if there is any). I think this can be made more flexible by just calling the same function that us_internal_dispatch_ready_poll uses before calling on_data when the TLS calls the inner on_data.

Beyond any comments on the above, I'm wondering if you have any concerns about using https://github.com/s8sg/proxy_proto_c, and the method for inclusion. In my local hacking, I just vendored it in, not a git submodule (or whatever the others are).

[kylepl]

From a bit more digging of the protocol specification, my belief it the protocol requires it only be sent first, and thus it should not support receiving it in within the TLS connection:

The receiver MUST NOT start processing the connection before it receives a
complete and valid PROXY protocol header. This is particularly important for
protocols where the receiver is expected to speak first (eg: SMTP, FTP or SSH).
The receiver may apply a short timeout and decide to abort the connection if
the protocol header is not seen within a few seconds (at least 3 seconds to
cover a TCP retransmit).

This simplifies the implementation. The downside is this is a change in behaviour to the current uWebSockets implementation, but it's not clear to me how to see if this is actually being depended on - and the likelihood is reduced since it is non-standard. Thoughts?