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, csuth...@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-unsubscr...@tomcat.apache.org >> For additional commands, e-mail: dev-h...@tomcat.apache.org >> > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org > For additional commands, e-mail: dev-h...@tomcat.apache.org > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
