tomee-users mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Zachary Bedell <zbed...@nycourts.gov>
Subject Re: Incorrect URL decoding in MulticastConnectionFactory$URI::parseParameters
Date Wed, 20 Sep 2017 19:17:27 GMT
I'll give it a shot.  Running into some trouble getting stock tests to pass on Mac OS and separate
problems behind a corporate proxy.  Spinning a Linux VM on the open WiFi to try.

FWIW, we're not using multicast for this.  I just looks like HttpConnectionFactory calls the
URI utility methods physically located in the static URI class on MulticastConnectionFactory.
 Otherwise this is https with sticky failover.

-Zac

> On Sep 20, 2017, at 12:50, Romain Manni-Bucau <rmannibucau@gmail.com> wrote:
> 
> 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
View raw message