Proxy Protocol Support

[tomciopp]

Motivation

I'm running a phoenix app that needs to support HAProxy's proxy protocol which I'm currently doing with help from https://github.com/heroku/ranch_proxy_protocol However I'm currently running into issues surrounding websocket support for that library and thought it would be better if this was directly built into cowboy with support via a configuration or as some sort of plugin. Based on some quick googling it looks like this has been requested before but no one has really taken the initiative to properly build out the feature. I would like to remedy that.

I wrote a parser for the protocol in elixir which can be found here[0]. The api is pretty basic you feed it a packet and it returns a buffer and a struct containing the relevant information. My idea was that you could place this code at the beginning of your parser, pull out any relevant info and then continue on with the rest of the buffer as though it weren't there. As of right now the project supports both versions of the protocol out of the box and is fully tested.

Caveats

I don't know the architecture of cowboy very well, and while I've read a few erlang programs I would not call myself an erlang programmer. I would likely need some help translating this code unless you wanted to call elixir from erlang[1] What are your thoughts on how to best add support for this feature?

[0] https://github.com/tomciopp/proxy-protocol
[1] https://joearms.github.io/published/2017-12-18-Calling-Elixir-From-Erlang.html

[essen]

Depending on Elixir code is definitely a big no, it's just not practical for non-Elixir developers.

I'm not sure this feature should be in Cowboy, I think the ranch_proxy_protocol approach works well to be honest. Perhaps you could give more details as to what issues you ran into when using it for Websocket? I don't really see what problem you could have encountered considering Websocket is on top of HTTP (you never connect to Websocket directly).

[tomciopp]

I'm going to work on a minimal app that demos the issue that I'm running into but the gist of it is when I swap out the default :ranch_tcp and :ranch_ssl modules for :ranch_proxy and :ranch_proxy_ssl I can properly load a web page but websockets will always timeout. The upgrade handshake still works but any attempt to communicate over the channel fails. Everything works as expected when behind the proxy when we are not sending the protocol. I'm trying to figure out the exact cause but debugging has been fairly difficult.

[essen]

Disable the proxy protocol and try against a plain Cowboy, chances are your problem is unrelated to ranch_proxy_protocol.

[tomciopp]

I have and it works in that case. My original thought was that the issue was not it ranch_proxy_protocol but everything in my testing points to that being the issue.

[essen]

Oh ranch_proxy_protocol is more complex than I remembered. Considering the PROXY stuff is just a connection header I don't think there should be any need to override transports at all. A better solution would be to have a protocol module that will read and parse the PROXY header and then call the real protocol module (similar to what cowboy_clear/cowboy_tls do). Of course it would need to fill in some options that Cowboy can then read from.

Yeah I can see the case for having this built-in I guess. I just won't have free time for that for some time.

[tomciopp]

Yeah that was my original thought as well (why I wrote the elixir PoC). If you could point me in the right direction I believe I should be able to put together a PR for this fairly soon. Translating the code I have previously written shouldn't be that hard. I just would need to know where/how to integrate it into Cowboy.

[essen]

I'll need time to think about it and I got no time right now. Feel free to experiment a little and show what you get, that would help.

[bullno1]

Would a transport wrapper work better? It would "lie" to ranch about the peer.

Whitelisting upstream proxy esp, by cidr notation would be a pain. I had to do something similar recently (whitelisting X-Forwarded-For) and I decided to just use TLS to verify upstream proxy.

[essen]

I don't think that's a good idea, better not lose information.

[bullno1]

It's not really lost, just default to the client's address. ranch_proxy:sockname/1 would return the proxied address according to ranch_transport behaviour then we can add: ranch_proxy:real_sockname/1 for the proxy's address. The idea is that the transport would work transparently. The upper layer would still have to be aware that it is using the the PROXY protocol anyway if it cares about lost information so I don't think making it call real_sockname is a problem.

[essen]

I don't see much point in having a special transport for something that occurs only at the beginning of the connection. Best have a special protocol module that gets the info then switches to h1/h2 (or any other protocol that implements the same interface) passing along the information from the PROXY header. And then in Cowboy you could have something like cowboy_req:forwarded_peer or something that would work for both PROXY and Forwarded headers depending on configuration.

[bullno1]

It's mostly a wrapper, you pass ranch_tcp or ranch_ssl and options into it and all it does is read the header then just pass all calls through. At transport level, it can be use for non-HTTP traffics too.

[essen]

I understand how ranch_proxy_protocol works and I'm mostly fine with it, but it's not the kind of solution I would go for if it was to be in Cowboy or Ranch themselves. Note that the solution I (vaguely) propose would also work for non-HTTP traffic too.

[bullno1]

Sry, I misread what you posted. You are saying providing something like ranch_proxy:parse(Sock, Transport, Opts) -> {Addr, Port} and then protocol module can call it before passing it on?

[essen]

No I'm saying a full fledged protocol module that receives the socket/options/etc. normally and eventually passes on to another protocol. Something like cowboy_clear/cowboy_tls except it'd have a receive loop to parse the PROXY header and then on completion call another protocol module.

[tomciopp]

I translated my original elixir version of the proxy protocol into erlang.[0] This is my first time just writing Erlang directly so any pointers on idiomatic Erlang would be appreciated. The library is fully tested and everything should work as you would expect.

My next goal would be to integrate this code into Cowboy. The easiest way is to likely publish this as a package and then write a small module that extends cowboy in order to parse the protocol where necessary. I'm not as familiar with the internals of Cowboy so I will defer to your expertise of the best way to handle this. Let me know what you think.

[0] https://github.com/tomciopp/proxy_protocol

[essen]

Basically what you want is something that will run between cowboy_clear/cowboy_tls and cowboy_http/cowboy_http2. It'll start reading from the socket to get the PROXY header and when it's done will run cowboy_http/cowboy_http2 and pass the extracted info to them.

[jimdigriz]

If anyone is curious, the patch to cowboy was not that intrusive to get this to work master...jimdigriz:proxy

...didn't say it was not offensive though! :)

[essen]

Note that I am starting work on adding PROXY support to Ranch directly, and after that Cowboy. The plan is to have a special protocol module that then gives control to the normal protocol module along with the proxy information in the options. Cowboy will pass that information forward via the Req.

[jimdigriz]

Noted and much oblig'd, thanks!

I guess we can follow this work in ninenines/ranch#204

[essen]

Yes, thanks for linking.

[tomciopp]

@essen Feel free to use the following code as a guide to parse the proxy_protocol as it should already be spec compliant and agnostic for v1 and v2. https://github.com/tomciopp/proxy_protocol

[essen]

Thanks. I'm probably not going to make it this far today, I'm mostly figuring out how to integrate this into Ranch, Cowboy and RabbitMQ. That said, I probably wouldn't be able to reuse it as-is, because your code expects the full proxy header to be given when trying to parse it, which might not necessarily be the case. I'll definitely steal some test cases though.

[essen]

Actually my assumptions might be wrong there because of:

Both formats are designed to fit in the smallest TCP segment that any TCP/IP
host is required to support (576 - 40 = 536 bytes). This ensures that the whole
header will always be delivered at once when the socket buffers are still empty
at the beginning of a connection.

That however says nothing of the Erlang implementation on top of that. I wonder if we can safely expect the first read to actually contain everything.

[tomciopp]

Based on my understanding of the protocol it should contain everything since it prepends the info before the rest of the request. You should be good to use it as-is, but I'm not as familiar with all of the edge cases, so I will defer to your knowledge.

[essen]

My lack of knowledge rather. A quick experiment shows that it's likely true for Erlang as well. I'll therefore retract my initial observation.

[jimdigriz]

From my experience, it has not been the receiver that causes this per-say, it is because of the SSL framing by the sender. For example, old Opera sends one byte (error codes on socket writes are easier to reason about), followed by the rest of the payload behind it. OpenSSL decodes on frames and so when you read, you see the first frame, which is one byte, and the second frame is the rest.

I would be surprised if there was a proxy implementing this protocol out there that pushed a single byte rather than the whole header though.

[essen]

The SSL example was misguided because the PROXY header precedes the TLS part (and I edited it out of my comment because it was not a good example).

The protocol seems to require sending the whole header in one packet, for example:

A receiver may reject an incomplete line which does not
contain the CRLF sequence in the first atomic read operation

And:

The sender must ensure that all the protocol header is sent at once. This block
is always smaller than an MSS, so there is no reason for it to be segmented at
the beginning of the connection.

So there's no problem doing a single gen_tcp:recv/3 call and expecting that to be the full proxy header.

[ferd]

Yeah the risky bit though is that there's no guarantee that something on the network did not bundle part of the TLS headers/hello along with the proxy header in the same packet. If you do a recv, either be sure to properly deal with the re-buffering of TLS data, or make sure to use line packet parsing for the headers and switch back to raw after that.

[essen]

Right. We don't have peek[1] but we do have unrecv[2] so all that's really necessary is that unrecv be documented and/or peek gets added (the NIF rewrite might be a good opportunity to request this). Then you just unrecv what remains from the buffer before initiating the TLS handshake (or read the consumed data if you peeked).

[1] erlang/otp#335
[2] https://github.com/erlang/otp/blob/master/lib/kernel/src/gen_tcp.erl#L326

[essen]

There's at least one custom TLV: https://docs.aws.amazon.com/elasticloadbalancing/latest/network/load-balancer-target-groups.html#proxy-protocol

Don't want to support parsing it, but definitely should make the data available.

[essen]

I got most of the parser done now, about to add tests. I reused some of your code, though most of it is probably unrecognizable by now. I've carefully reimplemented with a close eye on the spec and the haproxy implementation. I'll probably push that part on Monday.

[essen]

@tomciopp Your v2 implementation seems to be missing one byte: The next byte (the 13th one) is the protocol version and command.

Noticed while running your tests against my implementation.

EDIT: do the tests even pass? Some lengths in the tests are incorrect. Doesn't matter, correcting and reusing.

[tomciopp]

Yep, I must have glossed over that section while implementing. Luckily the fix for this should be quite simple by inserting the correct byte for each case. I will make a note to fix it while I have some time later today.

[essen]

Just pushed my initial parser implementation: ninenines/ranch@1d6d695

It supports everything from the sections 2.1 and 2.2 of the specification, although I need to write many more tests especially for the TLVs. Please review and provide feedback!

[essen]

I forgot to say in the commit: @tomciopp I reused your find_ip/port functions and the tests. I looked into ranch_proxy_protocol to reuse some tests too but they're complicated.

[essen]

Afraid I'll drop OTP-18 since it doesn't have map type syntax. It's old anyway.

[tomciopp]

@essen I will take a look later today and give my thoughts

[essen]

The ipv6 address parsing is incorrect, it expects the full size but that's not what the spec says (it allows double colon for example) and that's not what the haproxy code says either. So getting until $\s seems to be the only reasonable solution in that case.

[tomciopp]

@essen Are you referring to the human readable (version 1) protocol? If so you should be correct I am only handling the naive case. Unless I am mistaken, I believe that the ipv6 parser for v2 should handle all cases since you can't represent an address more efficiently without losing information.

[essen]

Yes. Afraid I imported that flaw into my first implementation. But what I will push later today fixes it.

[essen]

I've just pushed ninenines/ranch@1cc7de1 which contains functions for building the PROXY header and tests for the TLVs.

I'll add the ranch_tcp:recv_proxy_header function tomorrow.

[essen]

I've just pushed ninenines/ranch@4b9970b which adds the function ranch_tcp:recv_proxy_header/2.

[essen]

Reviews and feedback welcome! I'll do the Cowboy side of things in a week or two.

[tomciopp]

Ok, the only question I have is a hypothetical.

Say I'm using another cloud provider that has defined their own custom TLV (other than AWS) and I need the information defined within? How should I proceed? My general thoughts are that you should copy the current ranch_proxy_header:parse code and extend it with your own. (e.g. custom_ranch_proxy_header:parse) How would I go about calling this code in practice? What are your policies going to be on accepting patches to handling these new cases?

[essen]

See raw_tlvs => [{0..255, binary()}]. Ranch doesn't parse them.

Just find the right TLV data by using its number, and parse it yourself. Note that Ranch does not implement the Amazon TLV, you have to parse it yourself (it has a fake one in one of the tests but it doesn't parse its contents).

That's all we can do to support custom TLVs, different providers could reuse the same codes for different things, it's not workable otherwise.

[essen]

For TLS connections I'm considering the possibility of grabbing the TCP socket from the sslsocket() before doing the TLS handshake and receiving the PROXY header. It's bad practice because the sslsocket() type is undocumented and might change in the future, but doing this gives us the advantage that we still have the various option checks done early on instead of at each connection's handshake, and can therefore fail early. That said I doubt the sslsocket() type will change much in the future.

The cb_info callback module can be used also but it looks even more awkward to use it for this purpose, especially since users may want to use it as well.

I'm open to suggestions or warnings that this would be a huge mistake, and otherwise will probably go down that path as this gives us a lot of advantages and simplifies the code that needs to read the PROXY protocol header immensely.

If Ranch 2.0 ends up removing the socket from the protocol start_link call, a separate function will be necessary to receive the socket before doing the handshake, but I suppose that's OK. But we can figure that out when the time comes and this extra function may be useful regardless of the PROXY protocol concerns.

[essen]

The other bonus is that the implementation ends up tiny, outside of course of the parsing code (which is big but only because it supports the full spec).

[essen]

How about having something like this in the protocol code for when you want to get proxy headers (forward compatible with planned Ranch 2.0 changes):

{ok, Socket} = ranch:wait_for_socket(Ref),
{ok, ProxyInfo} = Transport:recv_proxy_header(Socket, 1000),
{ok, Socket} = ranch:handshake(Ref),

In this example the sockets returned are the same, but they don't have to be when using custom transports. The first line just waits for the socket to be received (implying controlling_process was updated) and returns it. It then queues the message again. The second line reads the proxy header and returns the proxy info (using the TCP socket, unrecv if necessary) and the third line does the TLS handshake.

The code would be compatible with both TCP and TLS so no need to do anything special to have a protocol support both TCP and TLS.

Ultimately this uses two undocumented features: the unrecv (there will be a peek feature in OTP 22 to replace it) and retrieving the TCP socket from the sslsocket(), for which I think I will send a PR adding a function to do it directly in OTP.

[essen]

Only the second line can return error tuples so even if you need to handle errors it's not a terrifying prospect.

[tomciopp]

For TLS connections I'm considering the possibility of grabbing the TCP socket from the sslsocket() before doing the TLS handshake and receiving the PROXY header. It's bad practice because the sslsocket() type is undocumented and might change in the future, but doing this gives us the advantage that we still have the various option checks done early on instead of at each connection's handshake, and can therefore fail early. That said I doubt the sslsocket() type will change much in the future.

The cb_info callback module can be used also but it looks even more awkward to use it for this purpose, especially since users may want to use it as well.

I'm open to suggestions or warnings that this would be a huge mistake, and otherwise will probably go down that path as this gives us a lot of advantages and simplifies the code that needs to read the PROXY protocol header immensely.

If Ranch 2.0 ends up removing the socket from the protocol start_link call, a separate function will be necessary to receive the socket before doing the handshake, but I suppose that's OK. But we can figure that out when the time comes and this extra function may be useful regardless of the PROXY protocol concerns.

I think this is risky for the reasons that you've already listed. I don't necessarily understand why we're optimizing for the failure condition when we shouldn't expect it to happen that often. I get that the implementation would be smaller, but shouldn't we be optimizing for robustness? What is the worst case for the alternative?

[essen]

The failure condition in question is due to misconfiguration, which for projects like RabbitMQ is happening often enough to warrant guarding the input as much as possible as early as possible.

I don't think the solution I propose is less robust, it should be equivalent. It might have a slightly higher maintenance cost if the ssl application changes drastically. It's also more consistent since you can just configure your TLS listener as normal and not have to worry about this at all, just call the transport module's recv_proxy_header function and Ranch hides the details from you.

[essen]

I have opened https://bugs.erlang.org/browse/ERL-757 with regard to the hack necessary for doing the recv on the ssl socket.

[essen]

OK so I changed things a bit and went for this:

{ok, ProxyInfo} = ranch:recv_proxy_header(Ref, 1000),
{ok, Socket} = ranch:handshake(Ref),

This way there's no confusion about having two Socket variables, since there's only one. And there's no confusing function name.

That'll become 1.7.0 once I finish porting RabbitMQ and Cowboy to it.

[essen]

As far as RabbitMQ goes, the switch from ranch_proxy_protocol to this is trivial:

• Use the built-in Ranch PROXY protocol support rabbitmq/rabbitmq-server#1747
• Use the built-in Ranch PROXY protocol support rabbitmq/rabbitmq-common#279
• Use the built-in Ranch PROXY protocol support rabbitmq/rabbitmq-mqtt#167
• Use the built-in Ranch PROXY protocol support rabbitmq/rabbitmq-stomp#127

Cowboy's implementation is also expected to be short. Though for some projects switching will probably be a little more complex than this.

[essen]

I've got the start of a Cowboy implementation done. I will push it tomorrow.

As far as configuration goes, you enable it via the proxy_header => true protocol option, and then it is present in the Req in the proxy_header key. I don't think anymore than that is necessary. Thoughts?

[essen]

I've pushed PROXY support in Cowboy in 122faed

See previous comment for instructions. Please try it and provide feedback, if possible within one week as then I could release Ranch 1.7 next week at the very least.

[essen]

Hello, has anyone done any experiments with this? Any negative comments? I'll wait a few more days and if no feedback assume it's all good and start releasing new versions with this feature.

[essen]

I've added some short documentation locally for Cowboy, and will release Ranch 1.7.0 later today. Considering this fixed. Thanks!