Thanks for the effort you have put into this, it is really good stuff.
Comments inline.
James Livingston wrote:
Hi,
A few weeks ago on jboss-as7-dev, Undertow compatibility with Tomcat
valves and -D options was briefly mentioned. Direct compatibility is a
different topic, but I think it is important to make sure that Undertow
can cover the reasons people are using some of those Tomcat options.
I've done some analysis on the JBoss support cases Red Hat deals with
and our knowledge base, and had a bit of a look over the community
forums. I looked at the forums less since it's less structured data but
what people mention there seems consistent with the the RH support cases.
I'm sorry this post is as long as it is, but hopefully what I've found
will help make sure we think about some cases people may run into using
Undertow, and remind us of a few lessons to learn from previous
Tomcat/JBossWeb bugs.
The most common valves we see people add are the RewriteValve,
RequestDumperValve and AccessLogValve. UNDERTOW-12 and UNDERTOW-17 are
already filed for equivalents to the first two. I can't see any code or
open JIRA issues for access logging, which I imagine people will want.
The most commonly used Tomcat -D option is
org.apache.catalina.STRICT_SERVLET_COMPLIANCE=false. When it's
discussed, it always seems to be added for the effect of not enforcing
"any wrapped request or response object passed to an application
dispatcher will be checked to ensure that it has wrapped the original
request or response. (SRV.8.2 / SRV.14.2.5.1)". Undertow enforces that
clause.
I will add this option. I think this should be modifiable per
deployment, as well as per server. For a lot of these options I am
thinking we have a servlet settings option in the config, which can then
be overridden in jboss-web.xml.
I don't really want to have things setup via system properties.
A quick search for "ApplicationDispatcher.checkSameObjects" shows people
running into this all over the place. Including
https://issues.jboss.org/browse/RESTEASY-745 (which has a work-around).
Unfortunately a reasonable number of libraries have had or still have
bugs where their wrappers don't derive from the spec one, and it only
break when someone tries to pass it to the dispatcher.
This is relatively easy to fix, it basically just requires a ThreadLocal
instead of using get(Request/Response). It may have a minor impact on
performance but probably nothing really noticeable.
There are a few uses of
org.apache.tomcat.util.http.Parameters.MAX_COUNT, that adjusts the
arbitrary limit on the parameter count which was added to prevent DoS
attacks using hash collisions. Looking at the code, there appear to be
several places where Undertow could suffer similar DoS attacks where
client data is used as HashMap keys: CookieHandler,
HttpServletRequestImpl.getParameterMap() and so on.
At the moment this limit is hard coded, but that should be fixed up
soon. This can only be modified on a global level, as this happens
before it is dispatched to a servlet application. We also use
SecureHashMap in some places, which will throw an exception if there are
two many hash collisions. Looks like cookie handler is vulnerable at the
moment, I will fix that up tomorrow.
Two important things here: what protection does/will Undertow have
against those attacks (such as a limit on the number of parameters, and
is there a way of changing the arbitrary limit. It's not the best idea
to do that, but I've seen at least two people saying they have>2000
hidden parameters in some of their forms.
There are a few options that get used a bit for session cookies. You can
change the cookie name from JSESSIONID on a per-webapp basis, but Tomcat
has a org.apache.catalina.JSESSIONID option to set it globally. This is
probably more of a WildFly integration thing, but is there a plan to
allow it to be changed globally?
We can do this.
org.apache.tomcat.util.http.ServerCookie.ALLOW_EQUALS_IN_VALUE is used
occasionally to allow un-escaped equals signs in the cookie value.
org.apache.tomcat.util.http.ServerCookie.VERSION_SWITCH=false is used
occasionally to not switch v0 cookies to v1 cookies, to deal with some
broken clients. See JBWEB-196.
We should be able to support these as well.
org.apache.catalina.connector.Response.REWRITE_CONTEXT_CHECK=false is
used primarily in WebLogic migrations. See JBWEB-181, but apparently
when encodeRedirectURL() adds the sessionId to the url, WL will do that
for URLs belonging to other contexts. I'm not sure if this is still
useful.
By default Tomcat un-escapes %2F in a URL to / to prevent security flaws
by something incorrectly doing it later.
org.apache.tomcat.util.buf.UDecoder.ALLOW_ENCODED_SLASH can be used to
disable that, for example if you want to put %2F inside a path component
of a REST service. Should Undertow to the same automatic un-escaping,
and if so have a way not to do it?
I will need to look into this. I am actually working on some decoder
stuff at the moment, so I should get to it tomorrow.
The final option that I've seen used a non-trivial number of times is
org.apache.jasper.runtime.BodyContentImpl.LIMIT_BUFFER. Jasper has
re-sizable buffers which are pooled for performance but may need to grow
arbitrarily large to render some JSPs. By default the enlarged buffers
are kept in the pool like normal ones, which means a bad JSP render
consuming hundreds of mb will not free the extra memory when it
finishes. This option causes it to shrink the buffers back down after
they're finished being used. The moral for Undertow here is if you have
resizable buffers and they get pooled, make sure the is a limit.
Undertow does not re-size buffers, it uses fixed size direct buffers for
the most part. We are still using Jasper for JSP though, so I will look
into this option.
Stuart