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

Establishment of SSL over HTTP CONNECT tunnel (sometimes) fails with {error,{tls_alert,"record overflow"}} #536

Open
cirka opened this issue Sep 27, 2018 · 14 comments · May be fixed by #537

Comments

@cirka
Copy link

cirka commented Sep 27, 2018

A friend of mine has a really interesting case of hackney unable to reliably establish ssl session over HTTP proxy. I have an access to packet traces on the wire which unfortunately is something I can not freely share, but the traces let us to narrow down the difference between cases when connection establishment is successful and when it is not. In case whole HTTP response to CONNECT request returned by the proxy was contained in a single TCP segment the ssl connection was established successfully and if HTTP response was split in 2 TCP segments the ssl_connect/2 was returning terminal {error,{tls_alert,"record overflow"}}. The error usually means a garbage was read by ssl_connect from the socket.

We spend some time investigating how hackney check if a tunnel has been successfully established and the criteria defined by RFCs .. from https://tools.ietf.org/html/rfc7231#section-4.3.6 - Any 2xx (Successful) response indicates that the sender (and all inbound proxies) will switch to tunnel mode immediately after the blank line that concludes the successful response's header section; The hackney https://github.com/benoitc/hackney/blob/master/src/hackney_http_connect.erl has a function check_response(Socket) which basically waits for any size segment to arrive to the socket and then uses function check_status/1 to pattern match the response code and protocol version supported and discards the rest of what was read from the socket. The way hackney checks the response is prone to break in real life scenarios because single read from the socket does not guarantee to have complete HTTP response (including new line) and may leave the rest of HTTP response on the socket buffer ( because it was not read in one gen_tcp:recv or i just came later in time). The ssl_connect then gets called to upgrade session to TLS: it sends client hello and tries to read server hello records but gets to read the rest of HTTP response still lingering in the socket buffer instead and errors out.

I am not committed to providing a patch yet as it directly does not hurt me yet ;-) but a complete implementation of HTTP response parsing would be appreciated from anyone who stepped on this bug.

@benoitc
Copy link
Owner

benoitc commented Sep 28, 2018

can you test the patch above?

@cirka
Copy link
Author

cirka commented Oct 1, 2018

I had something like this in mind: master...cirka:fix_536
In error report i wrote '(including new line)' having in mind 'empty line' ... basically <<\r\n\r\n>>.
My patch tries to match end of line from response/headers + empty line.

@bartima3us
Copy link

I have tested both your fixes but @benoitc still fails. @cirka request freezes. I will make another tcpdump and send it to @cirka

@benoitc
Copy link
Owner

benoitc commented Oct 2, 2018

which kind of proxy are we talking abou? Please not that generally speaking a proxy should send the response during the first packet. Is there a way I can reproduce it myself?

@bartima3us
Copy link

HTTP proxy type. Hmm, it's a bit difficult because I can't send a tcp dump.

@benoitc
Copy link
Owner

benoitc commented Oct 2, 2018

Is this a known proxy server?

@bartima3us
Copy link

bartima3us commented Oct 2, 2018

If you ask what kind of proxy server, so it's Apache. If you ask is it a public or a private, so it's a private, behind VPN.

Edit: I will specify Apache version soon.

@benoitc
Copy link
Owner

benoitc commented Oct 2, 2018

If can you share some bits of the apache configuration you're using, I will test it :)

@bartima3us
Copy link

Apache/2.2.12. What exactly options from configuration do you need?

@benoitc
Copy link
Owner

benoitc commented Oct 2, 2018

@bartima3us the settings of mod_proxy you are using. It should be enough to reproduce it.

@bartima3us
Copy link

@cirka congrats! Problem solved. Make a PR. :)

@benoitc
Copy link
Owner

benoitc commented Oct 3, 2018

@bartima3us what error did you have with #537 though? A proxy must send 1 line and no other headers which should be handled by my patch. Can you send a trace?

@cirka
Copy link
Author

cirka commented Oct 3, 2018

The proxy responds with 200 and additional header:
'
HTTP/1.0 200 Connection Established
Proxy-agent: Apache/2.2.12 (Linux/SUSE)
'
followed by empty line.
My proposed patch gathers all input until empty line is detected ( complete with \r\n matching).
Why do you believe headers can not be present ?
From same RFC regarding limitations for allowed headers: "
A server MUST NOT send any Transfer-Encoding or Content-Length header fields in a 2xx (Successful) response to CONNECT. A client MUST ignore any Content-Length or Transfer-Encoding header fields received in a successful response to CONNECT."

No further limitations are defined, and the server is free to add version header field.

@benoitc
Copy link
Owner

benoitc commented Oct 8, 2018

ok. I would prefer to have a version using hackney_http but let see how I can work around that one. Also a test would be useful.

@benoitc benoitc added the proxy label Dec 9, 2019
@benoitc benoitc self-assigned this May 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants