Bug 60697 - TRACE method incorrectly advertised as a supported HTTP method for custom servlet
Summary: TRACE method incorrectly advertised as a supported HTTP method for custom ser...
Status: RESOLVED FIXED
Alias: None
Product: Tomcat 8
Classification: Unclassified
Component: Catalina (show other bugs)
Version: 8.5.x-trunk
Hardware: All All
: P2 normal (vote)
Target Milestone: ----
Assignee: Tomcat Developers Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-02-07 08:31 UTC by Olivier Jaquemet
Modified: 2017-02-16 11:16 UTC (History)
0 users



Attachments
Very simple webapp with one custom servlet to reproduce issue (3.12 KB, application/zip)
2017-02-07 08:31 UTC, Olivier Jaquemet
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Olivier Jaquemet 2017-02-07 08:31:59 UTC
Created attachment 34728 [details]
Very simple webapp with one custom servlet to reproduce issue

OWASP recommends testing HTTP methods of remote servers using nmap "http-methods" script:
https://www.owasp.org/index.php/Test_HTTP_Methods_(OTG-CONFIG-006)
One of the recommandations is to ensure TRACE method is disabled (let's just omit the recommandation on PUT/DELETE in this discussion..)

For this matter, the 'Security Considerations' documentation of Tomcat states the following :
 "The allowTrace attribute may be used to enable TRACE requests which can be useful for debugging. Due to the way some browsers handle the response from a TRACE request (which exposes the browser to an XSS attack), support for TRACE requests is disabled by default."
http://tomcat.apache.org/tomcat-8.5-doc/security-howto.html#Connectors

And indeed, with the default configuration, the TRACE method is always refused with the unsupported 405 HTTP status code.

However there is one case where the configuration does not fully apply : response to OPTIONS request for custom servlet (i.e. any non tomcat servlet extending HttpServlet).
In such case the TRACE methods is incorrectly listed in the Allow header sent back, even though it is correctly handled as not supported when executed.

To reproduce:
1. deploy the attached war (containg all sources) in a tomcat instance listening on port 80 (listing on port 80 is required for proper validation through nmap https-methods script).
2. run the following unix commands :

** Test of custom Servlet :
  $> curl -v -X OPTIONS http://yourIP/test/
  BUG : 200 + Allow GET, HEAD, TRACE, OPTIONS
  Expected : 200 + Allow GET, HEAD, OPTIONS

  $> curl -v -X TRACE http://yourIP/test/
  OK : 405 + Allow: GET, HEAD, OPTIONS

  $> nmap -p 80 --script http-methods --script-args http-methods.url-path='/test/' yourIP
  BUG : nmap reports "Potentially risky methods: TRACE"
(even though it is correctly locked down)

This leads several security products which relies on the same tests to incorrectly report Tomcat as having a potential security risk, even though there is none.


Technical explanation for this behavior on custom servlet :
- executing the TRACE method is correctly refused by CoyoteAdapter,
https://svn.apache.org/repos/asf/tomcat/tc8.5.x/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java
- but executing the OPTIONS methods is handled by the parent class which DOES NOT apply the connector "allowTrace" configuration and always sends the Allow header listing TRACE method
https://svn.apache.org/repos/asf/tomcat/tc8.5.x/trunk/java/javax/servlet/http/HttpServlet.java



To be fully complete on this matter, here is the behavior for JSPs and default servlet :

** Test of JSP :
  $> curl -v -X OPTIONS http://yourIP/test.jsp
  OK : 405 + HTML message indicating "JSPs only permit GET POST or HEAD"
  (unrelated to this bug report, an "Allow: GET, POST, HEAD" header would be expected here, there is none.
  this could be improved while sending "jsp.error.servlet.invalid.method" message, see

https://svn.apache.org/repos/asf/tomcat/tc8.5.x/trunk/java/org/apache/jasper/compiler/Generator.java )

  $> curl -v -X TRACE http://yourIP/test.jsp
  OK : 405 + Allow: OPTIONS
  (unrelated to this bug report, it seems the "Allow" header is incorrrect, "Allow: GET, POST, HEAD" header would be expected here)

  nmap -p 80 --script http-methods --script-args http-methods.url-path='/test.jsp' yourIP
  OK

** Test of default servlet :
  $> curl -v -X OPTIONS http://yourIP/index.html
  OK : 200 + Allow: GET, HEAD, POST, PUT, DELETE, OPTIONS

  $> curl -v -X TRACE http://yourIP/index.html
  OK : 405 + Allow: HEAD, DELETE, POST, GET, OPTIONS, PUT

  $> nmap -p 80 --script http-methods --script-args http-methods.url-path='/index.html' yourIP
  OK (...) : nmap reports "Potentially risky methods: PUT DELETE"
  (this is unrelated to this bug report, but there could be some improvement there too)
Comment 1 Mark Thomas 2017-02-14 10:35:40 UTC
This has been looked at before which is why the DefaultServlet handles this correctly. Requests for "OPTIONS *" are also handled correctly.

It is difficult to do this for custom servlets since the code that handles the OPTIONS request is in javax.servlet.http.HttpServlet. This is a specification class so we can't change the API and neither can it depend on any Tomcat specific classes.

That leaves us with two options. Use reflection in HttpServlet or filter  TRACE from the Allow header when it is disabled.

I don't really like either option. Using reflection in HttpServlet is a fairly ugly hack and filtering the headers is going to add overhead to every request. Of the two, reflection is the least bad option.

A third option is never including TRACE in Allow headers generated by HttpServlet. The problem with that is that it may well break applications that depend on it and it would result in a non-specification compliant response when TRACE was allowed.

I have a test case for this that I'll commit shortly. That test case identified a scenario when the WebDAV servlet included TRACE in the response when it was disabled. That is an easy fix.

I'll take a look at what the reflection code looks like for HttpServlet. If it isn't too ugly I'll go that route.

A completely different option is to always include TRACE in the OPTIONS response but respond with a 403 rather than a 405 which is, arguably, more specification compliant behaviour - although the security scanners might not like it.
Comment 2 Mark Thomas 2017-02-14 11:09:10 UTC
The hack wasn't too bad although I still don't really like having to do it.

I've fixed WebDAV in:
- trunk for 9.0.0.M18 onwards
- 8.5.x for 8.5.12 onwards
- 8.0.x for 8.0.42 onwards
- 7.0.x for 7.0.76 onwards

I've fixed custom servlets in:
- trunk for 9.0.0.M18 onwards
- 8.5.x for 8.5.12 onwards

I don't intend back-porting this to 8.0.x and 7.0.x.
Comment 3 Olivier Jaquemet 2017-02-16 11:16:57 UTC
Awesome, thank you Mark.