tomee-users mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Romain Manni-Bucau <rmannibu...@gmail.com>
Subject Re: Incorrect URL decoding in MulticastConnectionFactory$URI::parseParameters
Date Wed, 20 Sep 2017 16:50:23 GMT
Hi Zachary


We have some coverage for it in
https://github.com/apache/tomee/blob/master/server/openejb-client/src/test/java/org/apache/openejb/client/HttpConnectionTest.java

Multicast not being used that often with http (more likely with ejbd) we
didnt test it, do you want to try to add a test and fix?

Le 20 sept. 2017 18:28, "Zachary Bedell" <zbedell@nycourts.gov> a écrit :

Greetings all,

Pretty sure I found a bug in the way
org.apache.openejb.client.MulticastConnectionFactory
decodes URL parameters.  The final result of the issue is that if you use
HTTP basic authentication when calling ServerServlet and
openejb.ejbd.authenticate-with-request=true, you can't login with a
password that contains an ampersand character.

The flow looks something like:

1) Create a new IntitialContext with PROVIDER_URL set to
failover:sticky+random:https://myserver/ejb/invoke?basic.
username=xyz&basic.password=pass%26word
  a) /ejb/invoke is where I have org.apache.openejb.server.httpd.ServerServlet
mapped
  b) web.xml on that mapping requires BASIC auth.
  c) key part of URL is the literal password "pass&word" URL encoded with
ampersand -> %26

2) TomEE internals eventually end up at HttpConnectionFactory$HttpConnection's
constructor where line 76 calls:
        params = MulticastConnectionFactory.URIs.parseParamters(uri);
        By this time, various unwrapping has paired the URL down to:
        https://myserver/ejb/invoke?basic.username=xyz&basic.
password=pass%26word

3) MulticastConnectionFactory...parseParameters, IE line 136:
        return uri.getQuery() == null ? new HashMap<String, String>(0) :
parseQuery(stripPrefix(uri.getQuery(), "?"));

That calls URI.getQuery() which decodes the URI's query string, then passes
that into parseQuery() which splits up the query parameters delimited by
ampersands.  The call to URI.getQuery() is the problem.  For the above URI,
the result is:
        basic.username=xyz&basic.password=pass&word
        The ampersand in the query parameter basic.password is decoded and
then indistinguishable from a query parameter separator.  When passed into
parseQuery, the resulting value for basic.password is just "pass".

Since MulticastConnectionFactory$URIs::parseQuery already calls
URLDecoder.decode() on both name and value pairs, the call in
parseParameters should be to URI::getRawQuery() instead of getQuery().  I
think there's also a possible double decoding issue here which could
corrupt certain values by decoding the value a second time.

For the time being, I think I can work around this by passing the
authorization query parameter with the user:pass already base64 encoded.
Pretty sure this should be a complete & safe fix:


diff --git a/server/openejb-client/src/main/java/org/apache/openejb/client/
MulticastConnectionFactory.java b/server/openejb-client/src/
main/java/org/apache/openejb/client/MulticastConnectionFactory.java
index 22f2f86a6a..eedb54840e 100644
--- a/server/openejb-client/src/main/java/org/apache/openejb/client/
MulticastConnectionFactory.java
+++ b/server/openejb-client/src/main/java/org/apache/openejb/client/
MulticastConnectionFactory.java
@@ -133,7 +133,7 @@ public class MulticastConnectionFactory implements
ConnectionFactory {
         }

         public static Map<String, String> parseParamters(final URI uri)
throws URISyntaxException {
-            return uri.getQuery() == null ? new HashMap<String, String>(0)
: parseQuery(stripPrefix(uri.getQuery(), "?"));
+            return uri.getQuery() == null ? new HashMap<String, String>(0)
: parseQuery(stripPrefix(uri.getRawQuery(), "?"));
         }

         public static String stripPrefix(final String value, final String
prefix) {



Best regards,
Zac Bedell

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message