You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by cs...@apache.org on 2017/02/07 18:13:40 UTC

svn commit: r1782037 - in /tomcat/tc8.5.x/trunk: conf/catalina.properties java/org/apache/tomcat/util/http/parser/HttpParser.java webapps/docs/changelog.xml webapps/docs/config/systemprops.xml

Author: csutherl
Date: Tue Feb  7 18:13:40 2017
New Revision: 1782037

URL: http://svn.apache.org/viewvc?rev=1782037&view=rev
Log:
Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=60594
Adding implementation of whitelist patch

Modified:
    tomcat/tc8.5.x/trunk/conf/catalina.properties
    tomcat/tc8.5.x/trunk/java/org/apache/tomcat/util/http/parser/HttpParser.java
    tomcat/tc8.5.x/trunk/webapps/docs/changelog.xml
    tomcat/tc8.5.x/trunk/webapps/docs/config/systemprops.xml

Modified: tomcat/tc8.5.x/trunk/conf/catalina.properties
URL: http://svn.apache.org/viewvc/tomcat/tc8.5.x/trunk/conf/catalina.properties?rev=1782037&r1=1782036&r2=1782037&view=diff
==============================================================================
--- tomcat/tc8.5.x/trunk/conf/catalina.properties (original)
+++ tomcat/tc8.5.x/trunk/conf/catalina.properties Tue Feb  7 18:13:40 2017
@@ -146,3 +146,6 @@ tomcat.util.buf.StringCache.byte.enabled
 #tomcat.util.buf.StringCache.char.enabled=true
 #tomcat.util.buf.StringCache.trainThreshold=500000
 #tomcat.util.buf.StringCache.cacheSize=5000
+
+# Allow for changes to HTTP request validation
+#tomcat.util.http.parser.HttpParser.requestTargetAllow=|

Modified: tomcat/tc8.5.x/trunk/java/org/apache/tomcat/util/http/parser/HttpParser.java
URL: http://svn.apache.org/viewvc/tomcat/tc8.5.x/trunk/java/org/apache/tomcat/util/http/parser/HttpParser.java?rev=1782037&r1=1782036&r2=1782037&view=diff
==============================================================================
--- tomcat/tc8.5.x/trunk/java/org/apache/tomcat/util/http/parser/HttpParser.java (original)
+++ tomcat/tc8.5.x/trunk/java/org/apache/tomcat/util/http/parser/HttpParser.java Tue Feb  7 18:13:40 2017
@@ -19,6 +19,9 @@ package org.apache.tomcat.util.http.pars
 import java.io.IOException;
 import java.io.StringReader;
 
+import org.apache.juli.logging.Log;
+import org.apache.juli.logging.LogFactory;
+
 /**
  * HTTP header value parser implementation. Parsing HTTP headers as per RFC2616
  * is not always as simple as it first appears. For headers that only use tokens
@@ -34,6 +37,8 @@ import java.io.StringReader;
  */
 public class HttpParser {
 
+    private static final Log log = LogFactory.getLog(HttpParser.class);
+
     private static final int ARRAY_SIZE = 128;
 
     private static final boolean[] IS_CONTROL = new boolean[ARRAY_SIZE];
@@ -42,8 +47,22 @@ public class HttpParser {
     private static final boolean[] IS_HEX = new boolean[ARRAY_SIZE];
     private static final boolean[] IS_NOT_REQUEST_TARGET = new boolean[ARRAY_SIZE];
     private static final boolean[] IS_HTTP_PROTOCOL = new boolean[ARRAY_SIZE];
+    private static final boolean[] REQUEST_TARGET_ALLOW = new boolean[ARRAY_SIZE];
 
     static {
+        String prop = System.getProperty("tomcat.util.http.parser.HttpParser.requestTargetAllow");
+        if (prop != null) {
+            for (int i = 0; i < prop.length(); i++) {
+                char c = prop.charAt(i);
+                if (c == '{' || c == '}' || c == '|') {
+                    REQUEST_TARGET_ALLOW[c] = true;
+                } else {
+                    log.warn("HttpParser: Character '" + c + "' is not allowed and will continue "
+                        + "being rejected.");
+                }
+            }
+        }
+
         for (int i = 0; i < ARRAY_SIZE; i++) {
             // Control> 0-31, 127
             if (i < 32 || i == 127) {
@@ -74,7 +93,9 @@ public class HttpParser {
             if (IS_CONTROL[i] || i > 127 ||
                     i == ' ' || i == '\"' || i == '#' || i == '<' || i == '>' || i == '\\' ||
                     i == '^' || i == '`'  || i == '{' || i == '|' || i == '}') {
-                IS_NOT_REQUEST_TARGET[i] = true;
+                if (!REQUEST_TARGET_ALLOW[i]) {
+                    IS_NOT_REQUEST_TARGET[i] = true;
+                }
             }
 
             // Not valid for HTTP protocol

Modified: tomcat/tc8.5.x/trunk/webapps/docs/changelog.xml
URL: http://svn.apache.org/viewvc/tomcat/tc8.5.x/trunk/webapps/docs/changelog.xml?rev=1782037&r1=1782036&r2=1782037&view=diff
==============================================================================
--- tomcat/tc8.5.x/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/tc8.5.x/trunk/webapps/docs/changelog.xml Tue Feb  7 18:13:40 2017
@@ -103,6 +103,12 @@
         Ensure that executor thread pools used with connectors, pre-start the
         configured minimum number of idle threads. (markt)
       </fix>
+      <add>
+        <bug>60594</bug>: Allow some invalid characters that were recently
+        restricted to be processed in requests by using the system property
+        <code>tomcat.util.http.parser.HttpParser.requestTargetAllow</code>.
+        (csutherl)
+      </add>
     </changelog>
   </subsection>
   <subsection name="Jasper">

Modified: tomcat/tc8.5.x/trunk/webapps/docs/config/systemprops.xml
URL: http://svn.apache.org/viewvc/tomcat/tc8.5.x/trunk/webapps/docs/config/systemprops.xml?rev=1782037&r1=1782036&r2=1782037&view=diff
==============================================================================
--- tomcat/tc8.5.x/trunk/webapps/docs/config/systemprops.xml (original)
+++ tomcat/tc8.5.x/trunk/webapps/docs/config/systemprops.xml Tue Feb  7 18:13:40 2017
@@ -639,6 +639,14 @@
       <p>If not specified, the default value of <code>3</code> will be used.</p>
     </property>
 
+    <property name="tomcat.util.http.parser.HttpParser.requestTargetAllow">
+      <p>A string comprised of characters the server should allow even when they are not encoded.
+      These characters would normally result in a 400 status.</p>
+      <p>The acceptable characters for this property are: <code>|</code>, <code>{</code>
+      , and <code>}</code></p>
+      <p>If not specified, the default value of <code>null</code> will be used.</p>
+    </property>
+
   </properties>
 
 </section>



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: svn commit: r1782037 - in /tomcat/tc8.5.x/trunk: conf/catalina.properties java/org/apache/tomcat/util/http/parser/HttpParser.java webapps/docs/changelog.xml webapps/docs/config/systemprops.xml

Posted by Coty Sutherland <cs...@redhat.com>.
> LGTM. Thanks for seeing this through.

Awesome, no problem :)

On Wed, Feb 8, 2017 at 2:46 PM, Mark Thomas <ma...@apache.org> wrote:
> On 08/02/17 19:42, Coty Sutherland wrote:
>>
>> Thanks for the suggestions; I implemented them in
>> http://svn.apache.org/r1782240
>
>
> LGTM. Thanks for seeing this through.
>
> Mark
>
>
>
>>
>> On Tue, Feb 7, 2017 at 7:13 PM, Mark Thomas <ma...@apache.org> wrote:
>>>
>>> On 07/02/17 18:13, csutherl@apache.org wrote:
>>>>
>>>>
>>>> Author: csutherl
>>>> Date: Tue Feb  7 18:13:40 2017
>>>> New Revision: 1782037
>>>>
>>>> URL: http://svn.apache.org/viewvc?rev=1782037&view=rev
>>>> Log:
>>>> Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=60594
>>>> Adding implementation of whitelist patch
>>>>
>>>> Modified:
>>>>     tomcat/tc8.5.x/trunk/conf/catalina.properties
>>>>
>>>>
>>>> tomcat/tc8.5.x/trunk/java/org/apache/tomcat/util/http/parser/HttpParser.java
>>>>     tomcat/tc8.5.x/trunk/webapps/docs/changelog.xml
>>>>     tomcat/tc8.5.x/trunk/webapps/docs/config/systemprops.xml
>>>>
>>>> Modified: tomcat/tc8.5.x/trunk/conf/catalina.properties
>>>> URL:
>>>>
>>>> http://svn.apache.org/viewvc/tomcat/tc8.5.x/trunk/conf/catalina.properties?rev=1782037&r1=1782036&r2=1782037&view=diff
>>>>
>>>>
>>>> ==============================================================================
>>>> --- tomcat/tc8.5.x/trunk/conf/catalina.properties (original)
>>>> +++ tomcat/tc8.5.x/trunk/conf/catalina.properties Tue Feb  7 18:13:40
>>>> 2017
>>>> @@ -146,3 +146,6 @@ tomcat.util.buf.StringCache.byte.enabled
>>>>  #tomcat.util.buf.StringCache.char.enabled=true
>>>>  #tomcat.util.buf.StringCache.trainThreshold=500000
>>>>  #tomcat.util.buf.StringCache.cacheSize=5000
>>>> +
>>>> +# Allow for changes to HTTP request validation
>>>
>>>
>>>
>>> I'd add here:
>>>
>>> # WARNING: Using this option will expose the server to CVE-2016-6816
>>>
>>>
>>>> +#tomcat.util.http.parser.HttpParser.requestTargetAllow=|
>>>>
>>>> Modified:
>>>>
>>>> tomcat/tc8.5.x/trunk/java/org/apache/tomcat/util/http/parser/HttpParser.java
>>>> URL:
>>>>
>>>> http://svn.apache.org/viewvc/tomcat/tc8.5.x/trunk/java/org/apache/tomcat/util/http/parser/HttpParser.java?rev=1782037&r1=1782036&r2=1782037&view=diff
>>>>
>>>>
>>>> ==============================================================================
>>>> ---
>>>>
>>>> tomcat/tc8.5.x/trunk/java/org/apache/tomcat/util/http/parser/HttpParser.java
>>>> (original)
>>>> +++
>>>>
>>>> tomcat/tc8.5.x/trunk/java/org/apache/tomcat/util/http/parser/HttpParser.java
>>>> Tue Feb  7 18:13:40 2017
>>>> @@ -19,6 +19,9 @@ package org.apache.tomcat.util.http.pars
>>>>  import java.io.IOException;
>>>>  import java.io.StringReader;
>>>>
>>>> +import org.apache.juli.logging.Log;
>>>> +import org.apache.juli.logging.LogFactory;
>>>> +
>>>>  /**
>>>>   * HTTP header value parser implementation. Parsing HTTP headers as per
>>>> RFC2616
>>>>   * is not always as simple as it first appears. For headers that only
>>>> use
>>>> tokens
>>>> @@ -34,6 +37,8 @@ import java.io.StringReader;
>>>>   */
>>>>  public class HttpParser {
>>>>
>>>> +    private static final Log log = LogFactory.getLog(HttpParser.class);
>>>> +
>>>>      private static final int ARRAY_SIZE = 128;
>>>>
>>>>      private static final boolean[] IS_CONTROL = new
>>>> boolean[ARRAY_SIZE];
>>>> @@ -42,8 +47,22 @@ public class HttpParser {
>>>>      private static final boolean[] IS_HEX = new boolean[ARRAY_SIZE];
>>>>      private static final boolean[] IS_NOT_REQUEST_TARGET = new
>>>> boolean[ARRAY_SIZE];
>>>>      private static final boolean[] IS_HTTP_PROTOCOL = new
>>>> boolean[ARRAY_SIZE];
>>>> +    private static final boolean[] REQUEST_TARGET_ALLOW = new
>>>> boolean[ARRAY_SIZE];
>>>>
>>>>      static {
>>>> +        String prop =
>>>>
>>>> System.getProperty("tomcat.util.http.parser.HttpParser.requestTargetAllow");
>>>> +        if (prop != null) {
>>>> +            for (int i = 0; i < prop.length(); i++) {
>>>> +                char c = prop.charAt(i);
>>>> +                if (c == '{' || c == '}' || c == '|') {
>>>> +                    REQUEST_TARGET_ALLOW[c] = true;
>>>> +                } else {
>>>> +                    log.warn("HttpParser: Character '" + c + "' is not
>>>> allowed and will continue "
>>>> +                        + "being rejected.");
>>>
>>>
>>>
>>> This should use the StringManager for i18n support.
>>>
>>> Also "... will continue to be rejected." sounds better.
>>>
>>>
>>>> +                }
>>>> +            }
>>>> +        }
>>>> +
>>>>          for (int i = 0; i < ARRAY_SIZE; i++) {
>>>>              // Control> 0-31, 127
>>>>              if (i < 32 || i == 127) {
>>>> @@ -74,7 +93,9 @@ public class HttpParser {
>>>>              if (IS_CONTROL[i] || i > 127 ||
>>>>                      i == ' ' || i == '\"' || i == '#' || i == '<' || i
>>>> ==
>>>> '>' || i == '\\' ||
>>>>                      i == '^' || i == '`'  || i == '{' || i == '|' || i
>>>> ==
>>>> '}') {
>>>> -                IS_NOT_REQUEST_TARGET[i] = true;
>>>> +                if (!REQUEST_TARGET_ALLOW[i]) {
>>>> +                    IS_NOT_REQUEST_TARGET[i] = true;
>>>> +                }
>>>>              }
>>>>
>>>>              // Not valid for HTTP protocol
>>>>
>>>> Modified: tomcat/tc8.5.x/trunk/webapps/docs/changelog.xml
>>>> URL:
>>>>
>>>> http://svn.apache.org/viewvc/tomcat/tc8.5.x/trunk/webapps/docs/changelog.xml?rev=1782037&r1=1782036&r2=1782037&view=diff
>>>>
>>>>
>>>> ==============================================================================
>>>> --- tomcat/tc8.5.x/trunk/webapps/docs/changelog.xml (original)
>>>> +++ tomcat/tc8.5.x/trunk/webapps/docs/changelog.xml Tue Feb  7 18:13:40
>>>> 2017
>>>> @@ -103,6 +103,12 @@
>>>>          Ensure that executor thread pools used with connectors,
>>>> pre-start
>>>> the
>>>>          configured minimum number of idle threads. (markt)
>>>>        </fix>
>>>> +      <add>
>>>> +        <bug>60594</bug>: Allow some invalid characters that were
>>>> recently
>>>> +        restricted to be processed in requests by using the system
>>>> property
>>>> +
>>>> <code>tomcat.util.http.parser.HttpParser.requestTargetAllow</code>.
>>>> +        (csutherl)
>>>> +      </add>
>>>>      </changelog>
>>>>    </subsection>
>>>>    <subsection name="Jasper">
>>>>
>>>> Modified: tomcat/tc8.5.x/trunk/webapps/docs/config/systemprops.xml
>>>> URL:
>>>>
>>>> http://svn.apache.org/viewvc/tomcat/tc8.5.x/trunk/webapps/docs/config/systemprops.xml?rev=1782037&r1=1782036&r2=1782037&view=diff
>>>>
>>>>
>>>> ==============================================================================
>>>> --- tomcat/tc8.5.x/trunk/webapps/docs/config/systemprops.xml (original)
>>>> +++ tomcat/tc8.5.x/trunk/webapps/docs/config/systemprops.xml Tue Feb  7
>>>> 18:13:40 2017
>>>> @@ -639,6 +639,14 @@
>>>>        <p>If not specified, the default value of <code>3</code> will be
>>>> used.</p>
>>>>      </property>
>>>>
>>>> +    <property
>>>> name="tomcat.util.http.parser.HttpParser.requestTargetAllow">
>>>> +      <p>A string comprised of characters the server should allow even
>>>> when they are not encoded.
>>>> +      These characters would normally result in a 400 status.</p>
>>>> +      <p>The acceptable characters for this property are:
>>>> <code>|</code>,
>>>> <code>{</code>
>>>> +      , and <code>}</code></p>
>>>> +      <p>If not specified, the default value of <code>null</code> will
>>>> be
>>>> used.</p>
>>>
>>>
>>>
>>> Again, I'd add something along the lines of:
>>>
>>> <strong>WARNING</strong>: Use of this option will expose the server to
>>> CVE-2016-6816.
>>>
>>>
>>>> +    </property>
>>>> +
>>>>    </properties>
>>>>
>>>>  </section>
>>>>
>>>>
>>>>
>>>> ---------------------------------------------------------------------
>>>> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
>>>> For additional commands, e-mail: dev-help@tomcat.apache.org
>>>>
>>>
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
>>> For additional commands, e-mail: dev-help@tomcat.apache.org
>>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
>> For additional commands, e-mail: dev-help@tomcat.apache.org
>>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: svn commit: r1782037 - in /tomcat/tc8.5.x/trunk: conf/catalina.properties java/org/apache/tomcat/util/http/parser/HttpParser.java webapps/docs/changelog.xml webapps/docs/config/systemprops.xml

Posted by Mark Thomas <ma...@apache.org>.
On 08/02/17 19:42, Coty Sutherland wrote:
> Thanks for the suggestions; I implemented them in http://svn.apache.org/r1782240

LGTM. Thanks for seeing this through.

Mark


>
> On Tue, Feb 7, 2017 at 7:13 PM, Mark Thomas <ma...@apache.org> wrote:
>> On 07/02/17 18:13, csutherl@apache.org wrote:
>>>
>>> Author: csutherl
>>> Date: Tue Feb  7 18:13:40 2017
>>> New Revision: 1782037
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1782037&view=rev
>>> Log:
>>> Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=60594
>>> Adding implementation of whitelist patch
>>>
>>> Modified:
>>>     tomcat/tc8.5.x/trunk/conf/catalina.properties
>>>
>>> tomcat/tc8.5.x/trunk/java/org/apache/tomcat/util/http/parser/HttpParser.java
>>>     tomcat/tc8.5.x/trunk/webapps/docs/changelog.xml
>>>     tomcat/tc8.5.x/trunk/webapps/docs/config/systemprops.xml
>>>
>>> Modified: tomcat/tc8.5.x/trunk/conf/catalina.properties
>>> URL:
>>> http://svn.apache.org/viewvc/tomcat/tc8.5.x/trunk/conf/catalina.properties?rev=1782037&r1=1782036&r2=1782037&view=diff
>>>
>>> ==============================================================================
>>> --- tomcat/tc8.5.x/trunk/conf/catalina.properties (original)
>>> +++ tomcat/tc8.5.x/trunk/conf/catalina.properties Tue Feb  7 18:13:40 2017
>>> @@ -146,3 +146,6 @@ tomcat.util.buf.StringCache.byte.enabled
>>>  #tomcat.util.buf.StringCache.char.enabled=true
>>>  #tomcat.util.buf.StringCache.trainThreshold=500000
>>>  #tomcat.util.buf.StringCache.cacheSize=5000
>>> +
>>> +# Allow for changes to HTTP request validation
>>
>>
>> I'd add here:
>>
>> # WARNING: Using this option will expose the server to CVE-2016-6816
>>
>>
>>> +#tomcat.util.http.parser.HttpParser.requestTargetAllow=|
>>>
>>> Modified:
>>> tomcat/tc8.5.x/trunk/java/org/apache/tomcat/util/http/parser/HttpParser.java
>>> URL:
>>> http://svn.apache.org/viewvc/tomcat/tc8.5.x/trunk/java/org/apache/tomcat/util/http/parser/HttpParser.java?rev=1782037&r1=1782036&r2=1782037&view=diff
>>>
>>> ==============================================================================
>>> ---
>>> tomcat/tc8.5.x/trunk/java/org/apache/tomcat/util/http/parser/HttpParser.java
>>> (original)
>>> +++
>>> tomcat/tc8.5.x/trunk/java/org/apache/tomcat/util/http/parser/HttpParser.java
>>> Tue Feb  7 18:13:40 2017
>>> @@ -19,6 +19,9 @@ package org.apache.tomcat.util.http.pars
>>>  import java.io.IOException;
>>>  import java.io.StringReader;
>>>
>>> +import org.apache.juli.logging.Log;
>>> +import org.apache.juli.logging.LogFactory;
>>> +
>>>  /**
>>>   * HTTP header value parser implementation. Parsing HTTP headers as per
>>> RFC2616
>>>   * is not always as simple as it first appears. For headers that only use
>>> tokens
>>> @@ -34,6 +37,8 @@ import java.io.StringReader;
>>>   */
>>>  public class HttpParser {
>>>
>>> +    private static final Log log = LogFactory.getLog(HttpParser.class);
>>> +
>>>      private static final int ARRAY_SIZE = 128;
>>>
>>>      private static final boolean[] IS_CONTROL = new boolean[ARRAY_SIZE];
>>> @@ -42,8 +47,22 @@ public class HttpParser {
>>>      private static final boolean[] IS_HEX = new boolean[ARRAY_SIZE];
>>>      private static final boolean[] IS_NOT_REQUEST_TARGET = new
>>> boolean[ARRAY_SIZE];
>>>      private static final boolean[] IS_HTTP_PROTOCOL = new
>>> boolean[ARRAY_SIZE];
>>> +    private static final boolean[] REQUEST_TARGET_ALLOW = new
>>> boolean[ARRAY_SIZE];
>>>
>>>      static {
>>> +        String prop =
>>> System.getProperty("tomcat.util.http.parser.HttpParser.requestTargetAllow");
>>> +        if (prop != null) {
>>> +            for (int i = 0; i < prop.length(); i++) {
>>> +                char c = prop.charAt(i);
>>> +                if (c == '{' || c == '}' || c == '|') {
>>> +                    REQUEST_TARGET_ALLOW[c] = true;
>>> +                } else {
>>> +                    log.warn("HttpParser: Character '" + c + "' is not
>>> allowed and will continue "
>>> +                        + "being rejected.");
>>
>>
>> This should use the StringManager for i18n support.
>>
>> Also "... will continue to be rejected." sounds better.
>>
>>
>>> +                }
>>> +            }
>>> +        }
>>> +
>>>          for (int i = 0; i < ARRAY_SIZE; i++) {
>>>              // Control> 0-31, 127
>>>              if (i < 32 || i == 127) {
>>> @@ -74,7 +93,9 @@ public class HttpParser {
>>>              if (IS_CONTROL[i] || i > 127 ||
>>>                      i == ' ' || i == '\"' || i == '#' || i == '<' || i ==
>>> '>' || i == '\\' ||
>>>                      i == '^' || i == '`'  || i == '{' || i == '|' || i ==
>>> '}') {
>>> -                IS_NOT_REQUEST_TARGET[i] = true;
>>> +                if (!REQUEST_TARGET_ALLOW[i]) {
>>> +                    IS_NOT_REQUEST_TARGET[i] = true;
>>> +                }
>>>              }
>>>
>>>              // Not valid for HTTP protocol
>>>
>>> Modified: tomcat/tc8.5.x/trunk/webapps/docs/changelog.xml
>>> URL:
>>> http://svn.apache.org/viewvc/tomcat/tc8.5.x/trunk/webapps/docs/changelog.xml?rev=1782037&r1=1782036&r2=1782037&view=diff
>>>
>>> ==============================================================================
>>> --- tomcat/tc8.5.x/trunk/webapps/docs/changelog.xml (original)
>>> +++ tomcat/tc8.5.x/trunk/webapps/docs/changelog.xml Tue Feb  7 18:13:40
>>> 2017
>>> @@ -103,6 +103,12 @@
>>>          Ensure that executor thread pools used with connectors, pre-start
>>> the
>>>          configured minimum number of idle threads. (markt)
>>>        </fix>
>>> +      <add>
>>> +        <bug>60594</bug>: Allow some invalid characters that were
>>> recently
>>> +        restricted to be processed in requests by using the system
>>> property
>>> +
>>> <code>tomcat.util.http.parser.HttpParser.requestTargetAllow</code>.
>>> +        (csutherl)
>>> +      </add>
>>>      </changelog>
>>>    </subsection>
>>>    <subsection name="Jasper">
>>>
>>> Modified: tomcat/tc8.5.x/trunk/webapps/docs/config/systemprops.xml
>>> URL:
>>> http://svn.apache.org/viewvc/tomcat/tc8.5.x/trunk/webapps/docs/config/systemprops.xml?rev=1782037&r1=1782036&r2=1782037&view=diff
>>>
>>> ==============================================================================
>>> --- tomcat/tc8.5.x/trunk/webapps/docs/config/systemprops.xml (original)
>>> +++ tomcat/tc8.5.x/trunk/webapps/docs/config/systemprops.xml Tue Feb  7
>>> 18:13:40 2017
>>> @@ -639,6 +639,14 @@
>>>        <p>If not specified, the default value of <code>3</code> will be
>>> used.</p>
>>>      </property>
>>>
>>> +    <property
>>> name="tomcat.util.http.parser.HttpParser.requestTargetAllow">
>>> +      <p>A string comprised of characters the server should allow even
>>> when they are not encoded.
>>> +      These characters would normally result in a 400 status.</p>
>>> +      <p>The acceptable characters for this property are: <code>|</code>,
>>> <code>{</code>
>>> +      , and <code>}</code></p>
>>> +      <p>If not specified, the default value of <code>null</code> will be
>>> used.</p>
>>
>>
>> Again, I'd add something along the lines of:
>>
>> <strong>WARNING</strong>: Use of this option will expose the server to
>> CVE-2016-6816.
>>
>>
>>> +    </property>
>>> +
>>>    </properties>
>>>
>>>  </section>
>>>
>>>
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
>>> For additional commands, e-mail: dev-help@tomcat.apache.org
>>>
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
>> For additional commands, e-mail: dev-help@tomcat.apache.org
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: svn commit: r1782037 - in /tomcat/tc8.5.x/trunk: conf/catalina.properties java/org/apache/tomcat/util/http/parser/HttpParser.java webapps/docs/changelog.xml webapps/docs/config/systemprops.xml

Posted by Coty Sutherland <cs...@redhat.com>.
Thanks for the suggestions; I implemented them in http://svn.apache.org/r1782240

On Tue, Feb 7, 2017 at 7:13 PM, Mark Thomas <ma...@apache.org> wrote:
> On 07/02/17 18:13, csutherl@apache.org wrote:
>>
>> Author: csutherl
>> Date: Tue Feb  7 18:13:40 2017
>> New Revision: 1782037
>>
>> URL: http://svn.apache.org/viewvc?rev=1782037&view=rev
>> Log:
>> Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=60594
>> Adding implementation of whitelist patch
>>
>> Modified:
>>     tomcat/tc8.5.x/trunk/conf/catalina.properties
>>
>> tomcat/tc8.5.x/trunk/java/org/apache/tomcat/util/http/parser/HttpParser.java
>>     tomcat/tc8.5.x/trunk/webapps/docs/changelog.xml
>>     tomcat/tc8.5.x/trunk/webapps/docs/config/systemprops.xml
>>
>> Modified: tomcat/tc8.5.x/trunk/conf/catalina.properties
>> URL:
>> http://svn.apache.org/viewvc/tomcat/tc8.5.x/trunk/conf/catalina.properties?rev=1782037&r1=1782036&r2=1782037&view=diff
>>
>> ==============================================================================
>> --- tomcat/tc8.5.x/trunk/conf/catalina.properties (original)
>> +++ tomcat/tc8.5.x/trunk/conf/catalina.properties Tue Feb  7 18:13:40 2017
>> @@ -146,3 +146,6 @@ tomcat.util.buf.StringCache.byte.enabled
>>  #tomcat.util.buf.StringCache.char.enabled=true
>>  #tomcat.util.buf.StringCache.trainThreshold=500000
>>  #tomcat.util.buf.StringCache.cacheSize=5000
>> +
>> +# Allow for changes to HTTP request validation
>
>
> I'd add here:
>
> # WARNING: Using this option will expose the server to CVE-2016-6816
>
>
>> +#tomcat.util.http.parser.HttpParser.requestTargetAllow=|
>>
>> Modified:
>> tomcat/tc8.5.x/trunk/java/org/apache/tomcat/util/http/parser/HttpParser.java
>> URL:
>> http://svn.apache.org/viewvc/tomcat/tc8.5.x/trunk/java/org/apache/tomcat/util/http/parser/HttpParser.java?rev=1782037&r1=1782036&r2=1782037&view=diff
>>
>> ==============================================================================
>> ---
>> tomcat/tc8.5.x/trunk/java/org/apache/tomcat/util/http/parser/HttpParser.java
>> (original)
>> +++
>> tomcat/tc8.5.x/trunk/java/org/apache/tomcat/util/http/parser/HttpParser.java
>> Tue Feb  7 18:13:40 2017
>> @@ -19,6 +19,9 @@ package org.apache.tomcat.util.http.pars
>>  import java.io.IOException;
>>  import java.io.StringReader;
>>
>> +import org.apache.juli.logging.Log;
>> +import org.apache.juli.logging.LogFactory;
>> +
>>  /**
>>   * HTTP header value parser implementation. Parsing HTTP headers as per
>> RFC2616
>>   * is not always as simple as it first appears. For headers that only use
>> tokens
>> @@ -34,6 +37,8 @@ import java.io.StringReader;
>>   */
>>  public class HttpParser {
>>
>> +    private static final Log log = LogFactory.getLog(HttpParser.class);
>> +
>>      private static final int ARRAY_SIZE = 128;
>>
>>      private static final boolean[] IS_CONTROL = new boolean[ARRAY_SIZE];
>> @@ -42,8 +47,22 @@ public class HttpParser {
>>      private static final boolean[] IS_HEX = new boolean[ARRAY_SIZE];
>>      private static final boolean[] IS_NOT_REQUEST_TARGET = new
>> boolean[ARRAY_SIZE];
>>      private static final boolean[] IS_HTTP_PROTOCOL = new
>> boolean[ARRAY_SIZE];
>> +    private static final boolean[] REQUEST_TARGET_ALLOW = new
>> boolean[ARRAY_SIZE];
>>
>>      static {
>> +        String prop =
>> System.getProperty("tomcat.util.http.parser.HttpParser.requestTargetAllow");
>> +        if (prop != null) {
>> +            for (int i = 0; i < prop.length(); i++) {
>> +                char c = prop.charAt(i);
>> +                if (c == '{' || c == '}' || c == '|') {
>> +                    REQUEST_TARGET_ALLOW[c] = true;
>> +                } else {
>> +                    log.warn("HttpParser: Character '" + c + "' is not
>> allowed and will continue "
>> +                        + "being rejected.");
>
>
> This should use the StringManager for i18n support.
>
> Also "... will continue to be rejected." sounds better.
>
>
>> +                }
>> +            }
>> +        }
>> +
>>          for (int i = 0; i < ARRAY_SIZE; i++) {
>>              // Control> 0-31, 127
>>              if (i < 32 || i == 127) {
>> @@ -74,7 +93,9 @@ public class HttpParser {
>>              if (IS_CONTROL[i] || i > 127 ||
>>                      i == ' ' || i == '\"' || i == '#' || i == '<' || i ==
>> '>' || i == '\\' ||
>>                      i == '^' || i == '`'  || i == '{' || i == '|' || i ==
>> '}') {
>> -                IS_NOT_REQUEST_TARGET[i] = true;
>> +                if (!REQUEST_TARGET_ALLOW[i]) {
>> +                    IS_NOT_REQUEST_TARGET[i] = true;
>> +                }
>>              }
>>
>>              // Not valid for HTTP protocol
>>
>> Modified: tomcat/tc8.5.x/trunk/webapps/docs/changelog.xml
>> URL:
>> http://svn.apache.org/viewvc/tomcat/tc8.5.x/trunk/webapps/docs/changelog.xml?rev=1782037&r1=1782036&r2=1782037&view=diff
>>
>> ==============================================================================
>> --- tomcat/tc8.5.x/trunk/webapps/docs/changelog.xml (original)
>> +++ tomcat/tc8.5.x/trunk/webapps/docs/changelog.xml Tue Feb  7 18:13:40
>> 2017
>> @@ -103,6 +103,12 @@
>>          Ensure that executor thread pools used with connectors, pre-start
>> the
>>          configured minimum number of idle threads. (markt)
>>        </fix>
>> +      <add>
>> +        <bug>60594</bug>: Allow some invalid characters that were
>> recently
>> +        restricted to be processed in requests by using the system
>> property
>> +
>> <code>tomcat.util.http.parser.HttpParser.requestTargetAllow</code>.
>> +        (csutherl)
>> +      </add>
>>      </changelog>
>>    </subsection>
>>    <subsection name="Jasper">
>>
>> Modified: tomcat/tc8.5.x/trunk/webapps/docs/config/systemprops.xml
>> URL:
>> http://svn.apache.org/viewvc/tomcat/tc8.5.x/trunk/webapps/docs/config/systemprops.xml?rev=1782037&r1=1782036&r2=1782037&view=diff
>>
>> ==============================================================================
>> --- tomcat/tc8.5.x/trunk/webapps/docs/config/systemprops.xml (original)
>> +++ tomcat/tc8.5.x/trunk/webapps/docs/config/systemprops.xml Tue Feb  7
>> 18:13:40 2017
>> @@ -639,6 +639,14 @@
>>        <p>If not specified, the default value of <code>3</code> will be
>> used.</p>
>>      </property>
>>
>> +    <property
>> name="tomcat.util.http.parser.HttpParser.requestTargetAllow">
>> +      <p>A string comprised of characters the server should allow even
>> when they are not encoded.
>> +      These characters would normally result in a 400 status.</p>
>> +      <p>The acceptable characters for this property are: <code>|</code>,
>> <code>{</code>
>> +      , and <code>}</code></p>
>> +      <p>If not specified, the default value of <code>null</code> will be
>> used.</p>
>
>
> Again, I'd add something along the lines of:
>
> <strong>WARNING</strong>: Use of this option will expose the server to
> CVE-2016-6816.
>
>
>> +    </property>
>> +
>>    </properties>
>>
>>  </section>
>>
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
>> For additional commands, e-mail: dev-help@tomcat.apache.org
>>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: svn commit: r1782037 - in /tomcat/tc8.5.x/trunk: conf/catalina.properties java/org/apache/tomcat/util/http/parser/HttpParser.java webapps/docs/changelog.xml webapps/docs/config/systemprops.xml

Posted by Mark Thomas <ma...@apache.org>.
On 07/02/17 18:13, csutherl@apache.org wrote:
> Author: csutherl
> Date: Tue Feb  7 18:13:40 2017
> New Revision: 1782037
>
> URL: http://svn.apache.org/viewvc?rev=1782037&view=rev
> Log:
> Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=60594
> Adding implementation of whitelist patch
>
> Modified:
>     tomcat/tc8.5.x/trunk/conf/catalina.properties
>     tomcat/tc8.5.x/trunk/java/org/apache/tomcat/util/http/parser/HttpParser.java
>     tomcat/tc8.5.x/trunk/webapps/docs/changelog.xml
>     tomcat/tc8.5.x/trunk/webapps/docs/config/systemprops.xml
>
> Modified: tomcat/tc8.5.x/trunk/conf/catalina.properties
> URL: http://svn.apache.org/viewvc/tomcat/tc8.5.x/trunk/conf/catalina.properties?rev=1782037&r1=1782036&r2=1782037&view=diff
> ==============================================================================
> --- tomcat/tc8.5.x/trunk/conf/catalina.properties (original)
> +++ tomcat/tc8.5.x/trunk/conf/catalina.properties Tue Feb  7 18:13:40 2017
> @@ -146,3 +146,6 @@ tomcat.util.buf.StringCache.byte.enabled
>  #tomcat.util.buf.StringCache.char.enabled=true
>  #tomcat.util.buf.StringCache.trainThreshold=500000
>  #tomcat.util.buf.StringCache.cacheSize=5000
> +
> +# Allow for changes to HTTP request validation

I'd add here:

# WARNING: Using this option will expose the server to CVE-2016-6816

> +#tomcat.util.http.parser.HttpParser.requestTargetAllow=|
>
> Modified: tomcat/tc8.5.x/trunk/java/org/apache/tomcat/util/http/parser/HttpParser.java
> URL: http://svn.apache.org/viewvc/tomcat/tc8.5.x/trunk/java/org/apache/tomcat/util/http/parser/HttpParser.java?rev=1782037&r1=1782036&r2=1782037&view=diff
> ==============================================================================
> --- tomcat/tc8.5.x/trunk/java/org/apache/tomcat/util/http/parser/HttpParser.java (original)
> +++ tomcat/tc8.5.x/trunk/java/org/apache/tomcat/util/http/parser/HttpParser.java Tue Feb  7 18:13:40 2017
> @@ -19,6 +19,9 @@ package org.apache.tomcat.util.http.pars
>  import java.io.IOException;
>  import java.io.StringReader;
>
> +import org.apache.juli.logging.Log;
> +import org.apache.juli.logging.LogFactory;
> +
>  /**
>   * HTTP header value parser implementation. Parsing HTTP headers as per RFC2616
>   * is not always as simple as it first appears. For headers that only use tokens
> @@ -34,6 +37,8 @@ import java.io.StringReader;
>   */
>  public class HttpParser {
>
> +    private static final Log log = LogFactory.getLog(HttpParser.class);
> +
>      private static final int ARRAY_SIZE = 128;
>
>      private static final boolean[] IS_CONTROL = new boolean[ARRAY_SIZE];
> @@ -42,8 +47,22 @@ public class HttpParser {
>      private static final boolean[] IS_HEX = new boolean[ARRAY_SIZE];
>      private static final boolean[] IS_NOT_REQUEST_TARGET = new boolean[ARRAY_SIZE];
>      private static final boolean[] IS_HTTP_PROTOCOL = new boolean[ARRAY_SIZE];
> +    private static final boolean[] REQUEST_TARGET_ALLOW = new boolean[ARRAY_SIZE];
>
>      static {
> +        String prop = System.getProperty("tomcat.util.http.parser.HttpParser.requestTargetAllow");
> +        if (prop != null) {
> +            for (int i = 0; i < prop.length(); i++) {
> +                char c = prop.charAt(i);
> +                if (c == '{' || c == '}' || c == '|') {
> +                    REQUEST_TARGET_ALLOW[c] = true;
> +                } else {
> +                    log.warn("HttpParser: Character '" + c + "' is not allowed and will continue "
> +                        + "being rejected.");

This should use the StringManager for i18n support.

Also "... will continue to be rejected." sounds better.

> +                }
> +            }
> +        }
> +
>          for (int i = 0; i < ARRAY_SIZE; i++) {
>              // Control> 0-31, 127
>              if (i < 32 || i == 127) {
> @@ -74,7 +93,9 @@ public class HttpParser {
>              if (IS_CONTROL[i] || i > 127 ||
>                      i == ' ' || i == '\"' || i == '#' || i == '<' || i == '>' || i == '\\' ||
>                      i == '^' || i == '`'  || i == '{' || i == '|' || i == '}') {
> -                IS_NOT_REQUEST_TARGET[i] = true;
> +                if (!REQUEST_TARGET_ALLOW[i]) {
> +                    IS_NOT_REQUEST_TARGET[i] = true;
> +                }
>              }
>
>              // Not valid for HTTP protocol
>
> Modified: tomcat/tc8.5.x/trunk/webapps/docs/changelog.xml
> URL: http://svn.apache.org/viewvc/tomcat/tc8.5.x/trunk/webapps/docs/changelog.xml?rev=1782037&r1=1782036&r2=1782037&view=diff
> ==============================================================================
> --- tomcat/tc8.5.x/trunk/webapps/docs/changelog.xml (original)
> +++ tomcat/tc8.5.x/trunk/webapps/docs/changelog.xml Tue Feb  7 18:13:40 2017
> @@ -103,6 +103,12 @@
>          Ensure that executor thread pools used with connectors, pre-start the
>          configured minimum number of idle threads. (markt)
>        </fix>
> +      <add>
> +        <bug>60594</bug>: Allow some invalid characters that were recently
> +        restricted to be processed in requests by using the system property
> +        <code>tomcat.util.http.parser.HttpParser.requestTargetAllow</code>.
> +        (csutherl)
> +      </add>
>      </changelog>
>    </subsection>
>    <subsection name="Jasper">
>
> Modified: tomcat/tc8.5.x/trunk/webapps/docs/config/systemprops.xml
> URL: http://svn.apache.org/viewvc/tomcat/tc8.5.x/trunk/webapps/docs/config/systemprops.xml?rev=1782037&r1=1782036&r2=1782037&view=diff
> ==============================================================================
> --- tomcat/tc8.5.x/trunk/webapps/docs/config/systemprops.xml (original)
> +++ tomcat/tc8.5.x/trunk/webapps/docs/config/systemprops.xml Tue Feb  7 18:13:40 2017
> @@ -639,6 +639,14 @@
>        <p>If not specified, the default value of <code>3</code> will be used.</p>
>      </property>
>
> +    <property name="tomcat.util.http.parser.HttpParser.requestTargetAllow">
> +      <p>A string comprised of characters the server should allow even when they are not encoded.
> +      These characters would normally result in a 400 status.</p>
> +      <p>The acceptable characters for this property are: <code>|</code>, <code>{</code>
> +      , and <code>}</code></p>
> +      <p>If not specified, the default value of <code>null</code> will be used.</p>

Again, I'd add something along the lines of:

<strong>WARNING</strong>: Use of this option will expose the server to 
CVE-2016-6816.

> +    </property>
> +
>    </properties>
>
>  </section>
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org