Bug 64210 - parsing request headers fail
Summary: parsing request headers fail
Status: RESOLVED FIXED
Alias: None
Product: Tomcat 9
Classification: Unclassified
Component: Catalina (show other bugs)
Version: 9.0.31
Hardware: All All
: P2 critical (vote)
Target Milestone: -----
Assignee: Tomcat Developers Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-03-10 07:11 UTC by Christian H.
Modified: 2020-03-23 14:47 UTC (History)
1 user (show)



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Christian H. 2020-03-10 07:11:56 UTC
Since we switched to tomcat 9.0.31, a lot of responses fail with HTTP status 400 Bad Request.

org.apache.coyote.http11.Http11Processor tells us, that the requests fail due to "The HTTP/1.1 request did not provide a host header". But the affected requests all have a host header.

We found out that this problem occurs when a request is send in multiple TCP-packets and one packet ends with (CR) an the next starts with (LF).

e.g.
TCP packet 1: "Accept: image/webp,*/*(CR)",
TCP packet 2: "(LF)Host: www.myhost.com(CR)(LF)(CR)(LF)"

As far as I can see, there was a change in header parsing between 9.0.30 and 9.0.31:
https://github.com/apache/tomcat/commit/ae8c82eff96990878e79691819ae941538ee62fd#diff-6d40d03c206f9d0696e5fa2c3b015c2e

My suspicion is that - if the bytebuffer used in org.apache.coyote.http11.Http11InputBuffer.parseHeader() starts with an (LF), parsing fails.
In previous version a LF lead to parse status DONE:

            } else if (chr == Constants.LF) {
                return HeaderParseStatus.DONE;

But now the previous char has to be a (CR), which may be not the case, if the ByteBuffer just got filled with the part that starts with the (LF) 

            } else if (prevChr == Constants.CR && chr == Constants.LF) {
                return HeaderParseStatus.DONE;

(with the assumption to the new behaviour I'm not 100% sure though)

After having a first look at the parseHeader()-method, it may be useful to initialize prevChr with the char at ByteBuffer's previous position.
Comment 1 Mark Thomas 2020-03-10 09:06:06 UTC
I'll take a look.

What is triggering the headers to split across multiple TCP packets? That seems a little unusual.
Comment 2 Christian H. 2020-03-10 09:56:22 UTC
It's just some large requests (a lot of cookies).

I have done some wireshark traffic recordings. For example one request was split  into a 2948 bytes TCP packet (ending with CR) and a 539 bytes packet (starting with LF)
Comment 3 Mark Thomas 2020-03-10 10:14:27 UTC
Thanks. I'm able to reproduce this. I'm working on some additional test cases and a fix.
Comment 4 Mark Thomas 2020-03-10 21:33:17 UTC
Fixed in:
- master for 10.0.0-M3 onwards
- 9.0.x for 9.0.33 onwards
- 8.5.x for 8.5.53 onwards
- 7.0.x for 7.0.102 onwards
Comment 5 Christian H. 2020-03-10 21:37:50 UTC
Thanks for the fast reaction and fix (y).
Comment 6 Em Domingues 2020-03-19 22:18:10 UTC
I assume this was intentional, but in the event it wasn't, the combination of the patch for this issue and the previous "strict header value parsing" commit have resulted in Tomcat rejecting all requests that use a single LF as a delimiter between HTTP request lines as opposed to the correct delimiter of CRLF.

Per RFC 2616 Section 19.3 (https://tools.ietf.org/html/rfc2616#section-19.3) it is recommended that applications be tolerant of malformed requests, with HTTP header delimiters called out as a particular area of note:
> The line terminator for message-header fields is the sequence CRLF.
> However, we recommend that applications, when parsing such headers,
> recognize a single LF as a line terminator and ignore the leading CR.

After deploying Tomcat 8.5.53 in our environment, we noticed that our hardware load balancers were sending malformed requests of the following format to perform host liveness checks against our app servers:
GET /foo HTTP/1.0\nHost: host.example.com \nConnection: Close\r\n\r\n

We are able to correct these requests (thankfully) but this behavior wasn't called out in the Tomcat release notes. It also represents a stricter interpretation of RFC 2616 than other major web server software, so I figured I'd at least flag it here.
Comment 7 Michael Osipov 2020-03-19 22:40:44 UTC
(In reply to Em Domingues from comment #6)
> I assume this was intentional, but in the event it wasn't, the combination
> of the patch for this issue and the previous "strict header value parsing"
> commit have resulted in Tomcat rejecting all requests that use a single LF
> as a delimiter between HTTP request lines as opposed to the correct
> delimiter of CRLF.
> 
> Per RFC 2616 Section 19.3 (https://tools.ietf.org/html/rfc2616#section-19.3)
> it is recommended that applications be tolerant of malformed requests, with
> HTTP header delimiters called out as a particular area of note:
> > The line terminator for message-header fields is the sequence CRLF.
> > However, we recommend that applications, when parsing such headers,
> > recognize a single LF as a line terminator and ignore the leading CR.
> 
> After deploying Tomcat 8.5.53 in our environment, we noticed that our
> hardware load balancers were sending malformed requests of the following
> format to perform host liveness checks against our app servers:
> GET /foo HTTP/1.0\nHost: host.example.com \nConnection: Close\r\n\r\n
> 
> We are able to correct these requests (thankfully) but this behavior wasn't
> called out in the Tomcat release notes. It also represents a stricter
> interpretation of RFC 2616 than other major web server software, so I
> figured I'd at least flag it here.

I can't find similar in https://tools.ietf.org/html/rfc7230#section-3.1.1

RFC 2616 is obsolete.
Comment 8 Em Domingues 2020-03-19 23:11:50 UTC
(In reply to Michael Osipov from comment #7)
> (In reply to Em Domingues from comment #6)
> > I assume this was intentional, but in the event it wasn't, the combination
> > of the patch for this issue and the previous "strict header value parsing"
> > commit have resulted in Tomcat rejecting all requests that use a single LF
> > as a delimiter between HTTP request lines as opposed to the correct
> > delimiter of CRLF.
> > 
> > Per RFC 2616 Section 19.3 (https://tools.ietf.org/html/rfc2616#section-19.3)
> > it is recommended that applications be tolerant of malformed requests, with
> > HTTP header delimiters called out as a particular area of note:
> > > The line terminator for message-header fields is the sequence CRLF.
> > > However, we recommend that applications, when parsing such headers,
> > > recognize a single LF as a line terminator and ignore the leading CR.
> > 
> > After deploying Tomcat 8.5.53 in our environment, we noticed that our
> > hardware load balancers were sending malformed requests of the following
> > format to perform host liveness checks against our app servers:
> > GET /foo HTTP/1.0\nHost: host.example.com \nConnection: Close\r\n\r\n
> > 
> > We are able to correct these requests (thankfully) but this behavior wasn't
> > called out in the Tomcat release notes. It also represents a stricter
> > interpretation of RFC 2616 than other major web server software, so I
> > figured I'd at least flag it here.
> 
> I can't find similar in https://tools.ietf.org/html/rfc7230#section-3.1.1
> 
> RFC 2616 is obsolete.

I'm aware. This still runs counter to the robustness principle, no?

For example, the implementation is inconsistent in where it errs on the side of being strict (here) and where it errs on the side of being tolerant (allowing multiple SP or HT between tokens) even when that's similarly against spec: https://github.com/apache/tomcat/blob/ae8c82eff96990878e79691819ae941538ee62fd/java/org/apache/coyote/http11/Http11InputBuffer.java#L404
Comment 9 Xing 2020-03-23 11:07:57 UTC
I don't think the current solution resolved all the header issues.
For example, our project use Socket TCP to monitor Tomcat status like this:
"get /platform HTTP/1.0\n\n" (double \n, no \r) 

Before version 8.5.50 (same for 9.0.30), it response 302 like:
2020-03-09 21:17:12.000 CST DEBUG [18950:140054285395776] childHeartbeat: child reply=HTTP/1.1 302 ^M
Location: http://localhost:8080/platform/^M
Date: Mon, 09 Mar 2020 13:17:12 GMT^M
Connection: close^M
Server: This information has been blocked for security reasons^M
^M

With 8.5.51 (9.0.31), it no response and make my monitor always wrongly restart Tomcat.

With 8.5.53 (9.0.33), it response 400 like:
2020-03-23 18:58:45.000 CST DEBUG [8584:140509820770112] childHeartbeat: child reply=HTTP/1.1 400 ^M
Content-Type: text/html;charset=utf-8^M
Content-Language: en^M
Content-Length: 1823^M
Date: Mon, 23 Mar 2020 10:58:45 GMT^M
Connection: close^M
Server: This information has been blocked for security reasons^M
^M
<!doctype html><html lang="en"><head><title>HTTP Status 400 – Bad Request</title><style type="text/css">body {font-family:Tahoma,Arial,sans-serif;} h1, h2, h3, b {color:white;background-color:#525D76;} h1 {font-size:22px;} h2 {font-size:16px;} h3 {font-size:14px;} p {font-size:12px;} a {c

Can I configure any parameters to ignore such situation?
I tried rejectIllegalHeader="false", and relaxedQueryChars="|{}\r\n", relaxedPathChars="|{}\r\n" but no effect.

Thanks
Xing
Comment 10 Mark Thomas 2020-03-23 12:09:57 UTC
(In reply to Xing from comment #9)
> I don't think the current solution resolved all the header issues.
> For example, our project use Socket TCP to monitor Tomcat status like this:
> "get /platform HTTP/1.0\n\n" (double \n, no \r) 

That request is invalid and is, therefore, rejected with a 400 response.

You can fix the issue by correcting the broken client.
Comment 11 Xing 2020-03-23 13:01:41 UTC
(In reply to Mark Thomas from comment #10)
> (In reply to Xing from comment #9)
> > I don't think the current solution resolved all the header issues.
> > For example, our project use Socket TCP to monitor Tomcat status like this:
> > "get /platform HTTP/1.0\n\n" (double \n, no \r) 
> 
> That request is invalid and is, therefore, rejected with a 400 response.
> 
> You can fix the issue by correcting the broken client.

No configuration to resolve this?
Then should I remove the "\n" in my request?

Thanks
Xing
Comment 12 Mark Thomas 2020-03-23 13:47:17 UTC
The correct line terminator for an HTTP/1.0 request is CRLF ("/r/n"). You should use that.
Comment 13 Xing 2020-03-23 14:47:36 UTC
(In reply to Mark Thomas from comment #12)
> The correct line terminator for an HTTP/1.0 request is CRLF ("/r/n"). You
> should use that.

Thanks a lot, I'm compiling :)

Xing