Detect bad framing

[olix0r]

From https://www.mnot.net/blog/2011/07/11/what_proxies_must_do

Proxies also need to be on the lookout for Content-Length headers that are duplicates, as well as ones that conflict with the use of Transfer-Encoding, and either reject the message or remove the bad headers.

This is because there are entire classes of attacks that exploit the differences between how implementations frame messages.

For example, this response:

HTTP/1.1 200 OK
Content-Type: text/html; charset=utf-8
Content-Length: 45
Content-Length: 20

has an ambiguous length. If a proxy treats it differently than a client, an attacker can inject a response. Likewise, this one:

HTTP/1.1 200 OK
Content-Type: text/html; charset=utf-8
Content-Length: 200
Transfer-Encoding: chunked

has both a Content-Length and chunked encoding. The chunked encoding has precedence, and the Content-Length has to be removed before forwarding the message.

See the spec for how to do it well.

[olix0r]

I suspect that finagle does most of this for us, but we should verify in e2e tests

[hawkw]

I'm working on a tool to automate this, and so far, I've found at least one case we're not handling correctly.

According to the spec:

If a message is received without Transfer-Encoding and with
either multiple Content-Length header fields having differing
field-values or a single Content-Length header field having an
invalid value, then the message framing is invalid and MUST be
treated as an error to prevent request or response smuggling.
...
If this is a response message received by a proxy, the proxy MUST
discard the received response, send a 502 (Bad Gateway) status
code as its downstream response, and then close the connection.

We are, as you might have guessed, Not Doing This.

Testing with flossy, where the upstream server sends first bad response message in the blog post @olix0r quoted above (with multiple Content-Length headers), linkerd happily forwards the bad response, multiple headers intact.

(N.B. for potential users of flossy – it's currently a minimum working proof-of-concept. you probably don't want to use this yet)

[hawkw]

I did a manual telnet session to confirm (I don't entirely trust flossy yet; there's probably several lurking bugs in there):

GET https://127.0.0.1:7777/test1 HTTP/1.1

HTTP/1.1 200 OK
Server: Example
Content-Length: 31
Date: Mon, 03 Jul 2017 15:15:07
Content-Length: 45
Content-Length: 20
l5d-success-class: 1.0
Via: 1.1 linkerd

aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa

So yeah. This is real.

[hawkw]

Additional testing with the latest flossy reveals that we're also mishandling the case of a request with multiple Content-Length headers – I'll open an issue for that too.

[hawkw]

Still need to add similar tests in flossy for chunked encodings (BuoyantIO/flossy#2).