Bug 57324 - Change in Expect100Continue behaviour in Tomcat is breaking existing clients with "keepalive" connections
Change in Expect100Continue behaviour in Tomcat is breaking existing clients ...
Status: RESOLVED FIXED
Product: Tomcat 7
Classification: Unclassified
Component: Catalina
7.0.50
PC All
: P2 blocker (vote)
: ---
Assigned To: Tomcat Developers Mailing List
:
Depends on:
Blocks:
  Show dependency tree
 
Reported: 2014-12-08 05:58 UTC by Vamsi Krishna
Modified: 2015-12-03 19:48 UTC (History)
2 users (show)



Attachments
Issue seen with Tomcat 7.0.57 (628.23 KB, image/jpeg)
2014-12-10 04:55 UTC, Vamsi Krishna
Details
Tomcat HTTP traffic (66.08 KB, image/jpeg)
2014-12-10 15:05 UTC, Vamsi Krishna
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Vamsi Krishna 2014-12-08 05:58:16 UTC
1) In the version of Tomcat 7.0.50, the behavior of Coyote handler is modified to handle "Expect: 100-continue" clients. To be specific look at this change in the following line, http://grepcode.com/file/repo1.maven.org/maven2/org.apache.tomcat/tomcat-coyote/7.0.50/org/apache/coyote/http11/AbstractHttp11Processor.java#1088

2) In the above change, the fix was done to handle cases against misbehaving client while POST requests are submitted to Tomcat.

3) While it is done with a "security" intent, the fix only takes into account status codes ranging in 2xx series. It breaks some legitimate cases where other status codes are returned for legitimate usecases. An example is given below.

Example:

   * It is quite normal to have a keepalive connections with different resources having different authorization controls
   * HTTP 401 is typical of webservers to inform clients of unauthorized access to resources and triggering client to resubmit the request. Please refer http://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html for 401 behavior.
   * The change done in Tomcat breaks this legitimate case.

Steps to reproduce:
1) Install a version of Tomcat 7.0.50 or above
2) Write a client that uses Expect100Continue behavior along with KeepAlive
3) Make atleast two requests with the first request succeeding and second request simulating a HTTP 401 scenario. 
4) Notice that the server closes the connection after the second response
5) This will break clients which are in the process of responding to the second request with valid credentials

Why blocker:

1) .NET defaults to Expect 100 behaviour
2) .NET clients optimize the keepalive connections and do not send credentials with every request going out on keepalive connections
3) This induces 401 situation more often in communications between Tomcat and .NET clients
4) Closing of server connections is causing existing connections to break and our customers are complaining this started happening after upgrading Tomcat (7.0.50)
5) Hence, this needs attention.
Comment 1 Konstantin Kolinko 2014-12-08 06:45:34 UTC
Reverting this feature won't help you, as .NET clients misbehave either way.


See recent thread "Receiving HTTP 505 on Expect: 100-continue" on users mailing http://marc.info/?t=141768539900004&r=1&w=2
Comment 2 Vamsi Krishna 2014-12-08 07:37:07 UTC
I suggest to consider giving an option in Tomcat to configure the behavior. Having such a popular Application Server behave only one configured is breaking lot of existing software.

I think we should have a conversation on what is the "right" fix that can satisfy both needs.
Comment 3 Vamsi Krishna 2014-12-08 09:12:00 UTC
I am attaching traces from my wireshark analysis. It captures 3 request response cycles. Please proceed to my notes at the end after the trace.

**************************************************
1) POST /someresource/ HTTP/1.1
User-Agent: Mozilla/4.0 (compatible; MSIE 6.0; MS Web Services Client Protocol 4.0.30319.18444)
Content-Type: text/xml; charset=utf-8
Authorization: Basic VALIDCREDENTIALS=
Host: serveraddress
Content-Length: 327
Expect: 100-continue

HTTP/1.1 100 Continue

<request body sent>

HTTP/1.1 200 OK
Server: Apache-Coyote/1.1
Content-Type: text/xml;charset=utf-8
Content-Length: 424
Date: Mon, 08 Dec 2014 08:47:08 GMT

<first response successful>


2) POST /someotherresource/ HTTP/1.1
User-Agent: Mozilla/4.0 (compatible; MSIE 6.0; MS Web Services Client Protocol 4.0.30319.18444)
Content-Type: text/xml; charset=utf-8
Host: serveraddress
Content-Length: 331
Expect: 100-continue

HTTP/1.1 100 Continue

<request body sent>

HTTP/1.1 401 Unauthorized
Server: Apache-Coyote/1.1
WWW-Authenticate: Basic realm="blah"
WWW-Authenticate: blah realm="blah"
Content-Type: text/html;charset=utf-8
Content-Language: en
Content-Length: 951
Date: Mon, 08 Dec 2014 08:47:09 GMT

<response with possible error message>

3) POST /someotherresource/ HTTP/1.1
User-Agent: Mozilla/4.0 (compatible; MSIE 6.0; MS Web Services Client Protocol 4.0.30319.18444)
Content-Type: text/xml; charset=utf-8
Authorization: Basic VALIDCREDENTIALS=
Host: 10.71.65.156
Content-Length: 331
Expect: 100-continue


***************************************************

* In the above trace, my client made two webservice calls in a total of 3 reply-request cycles. The first request marked with 1) was straight forward and completed successfully with Expect100continue behavior.

* The second webservice call was split into two cycles by .NET and its unnecessary optimization. Without the Application Developers clear intent to do so, .NET sent the second request without any credentials triggering a 401 response from server. However, according to RFC http://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html#sec10.4.2 

"401" response does give a chance for client to resubmit its request with proper credentials.

* The third request number 3) in the trace is .NET trial at doing the same i.e resubmitting the second request with credentials.

Because of the change done by Tomcat, the client does not get a chance and the socket is closed by Tomcat. 

The problem is severe because , in all of this, Application Developer does not get any control from .NET and Client code simply breaks with an exception because .NET did not check the health of socket before making request 3) in the above trace.

Agreed there is a lot of problem here in .NET and we can reach out to Microsoft, but going and patching each and every client out there is going to be really costly. Also, it can be argued that with this new change, Tomcat is not giving opportunity to client to resubmit the 401 challenged response, so it may be a violation of RFC. 

Also, my traces are not consistent with the original issue reported so I need someone to investigate further. The original issue may be something happening only when there is an intermediate node involved. 

I may not be an expert in all of this, but feel there is something Tomcat can do about all of this. I have also reopened the bug only to demonstrate my point, but not to violate the process in the forum. If it is still not convincing, please go ahead and close the case.
Comment 4 Mark Thomas 2014-12-09 21:05:43 UTC
Upgrade to the latest 7.0.x release. A 401 should not trigger a connection drop in that case.
Comment 5 Vamsi Krishna 2014-12-10 04:55:36 UTC
Created attachment 32279 [details]
Issue seen with Tomcat 7.0.57

Wireshark trace with Tomcat 7.0.57 using my test code.
Comment 6 Vamsi Krishna 2014-12-10 05:00:49 UTC
Mark,

I have upgraded to Tomcat 7.0.57 and the problem is still reproducible. I have attached the screenshot of my wireshark trace which clearly demonstrates a connection closure after a 401 response from Tomcat. Note that Tomcat 7.0.39 does not have this behavior. 

In the trace, you will find the packets highlighted in red that demonstrates the connection closure from Tomcat.

I have also checked the code and in AbstractHttp11Processor from Tomcat 7.0.57, I do not see any different handling for 401 

**************************Snippet *****************************

 if (!isAsync() && !comet) {
                if (getErrorState().isError()) {
                    // If we know we are closing the connection, don't drain
                    // input. This way uploading a 100GB file doesn't tie up the
                    // thread if the servlet has rejected it.
                    getInputBuffer().setSwallowInput(false);
                } else if (expectation &&
                        (response.getStatus() < 200 || response.getStatus() > 299)) {
                    // Client sent Expect: 100-continue but received a
                    // non-2xx final response. Disable keep-alive (if enabled)
                    // to ensure that the connection is closed. Some clients may
                    // still send the body, some may send the next request.
                    // No way to differentiate, so close the connection to
                    // force the client to send the next request.
                    getInputBuffer().setSwallowInput(false);
                    keepAlive = false;
                }
                endRequest();
            }

****************************End of Snippet **************************

Let me know what is missing link here ?

Thanks for your time.
Comment 7 Michael Osipov 2014-12-10 12:35:08 UTC
As far as I can see, this behavior is fully RFC-compliant:

A server that responds with a final status code before reading the
entire message body SHOULD indicate in that response whether it
intends to close the connection or continue reading and discarding
the request message (see Section 6.6 of [RFC7230]).

http://tools.ietf.org/html/rfc7231#section-5.1.1

This is what Tomcat does, closing the connection.
Comment 8 Mark Thomas 2014-12-10 14:38:39 UTC
Ah yes. I confused two different error condition handling mechanisms.

Ultimately Tomcat has to deal with the problem that after a non 2xx response, Tomcat can't tell if the next bytes received are the request body from the previous request or a brand new request. While this problematic for directly connected clients, it is a security risk when behind a proxy.

Tomcat should send the connection close header to signal that the connection is being closed. If there are cases where it isn't there that is something we could look at.
Comment 9 Vamsi Krishna 2014-12-10 15:05:37 UTC
Created attachment 32280 [details]
Tomcat HTTP traffic
Comment 10 Vamsi Krishna 2014-12-10 15:15:36 UTC
Yes, Tomcat is not sending "Connection: close" after response to 401.

My traces where the client is connected directly demonstrate that 401 is responded by Tomcat only after consuming the previous request body completely.Please refer Tomcat HTTP traffic attachment. I do not see your security point here in my trace. Is the security issue only when a "Proxy" is involved. 

According to RFC, if a server responds with 401 and if client has not sent "credentials" atleast once , then the client MAY request again with credentials. By closing the connection, you have really denied that right of the client. Having said that, the client can always resubmit using a fresh connection, but this adds the overhead of SSL handshake for those repeated calls. In a "Keepalive" scenario this does impact performance.

So, may I request and if you do not mind, can you cross reference the wireshark trace when the original problem is raised with a Proxy in middle so that people referencing this bug in future has the full context.

At the very least, by making sure Tomcat sends a "Connection: close" directive is acceptable, but I do want you to consider the performance aspect and see if something better can be done. I am trying to get to speed on Tomcat code to see if I can suggest a patch, but given my familiarity it may take time. So, it will help if you solve both problems.
Comment 11 Mark Thomas 2014-12-15 12:01:25 UTC
I've added code to set the "Connection: close" header if it is known at the time that the response is committed that the connection is going to be closed. This change has been made to trunk, 8.0.x (for 8.0.16 onwards) and 7.0.x (for 7.0.58 onwards).
Comment 12 Michael Osipov 2014-12-15 13:29:39 UTC
Can we have this complete handling fixed in Tomcat 6 too?
This patch and the previous one by Konstantin.
Comment 13 Michael Osipov 2015-12-03 09:57:32 UTC
Any ambitions to port this back to 6.0.x?
Comment 14 Christopher Schultz 2015-12-03 15:56:53 UTC
I don't intend for this to sound confrontational, but would you care to submit a patch?

Tomcat 7 and higher differ substantially from Tomcat 6 and so a back-port might not be trivial. If you are motivated to get this back-ported to Tomcat 6, you could try your hand at preparing a patch and help both yourself and The Community at large at the same time (not to mention the various committers who would otherwise have to prepare the patch themselves).
Comment 15 Michael Osipov 2015-12-03 19:48:15 UTC
(In reply to Christopher Schultz from comment #14)
> I don't intend for this to sound confrontational, but would you care to
> submit a patch?
>
> Tomcat 7 and higher differ substantially from Tomcat 6 and so a back-port
> might not be trivial. If you are motivated to get this back-ported to Tomcat
> 6, you could try your hand at preparing a patch and help both yourself and
> The Community at large at the same time (not to mention the various
> committers who would otherwise have to prepare the patch themselves).

Hopefully, because I am a long year ASF committer and don't need a lecture on that. I don't mind to provide patches at all.

I can try to backport that but I cannot compile Tomcat. The custom Ant build won't download the dependencies at work. It has either to be Maven or at least Aether Ant Tasks. Nothing else will work. All calls must be routed through Nexus.