Error parsing base64 (for non-WAFFLE authentication header)

[hakanai]

I was trying out Milton which appears to have its own authentication enabled by default (something I will be desperately trying to rip out). So my browser was sending a header which looks like this:

Authorization:Digest username="admin", realm="milton", nonce="YjNjZDgxNDYtOGIwMS00NDk0LTlkMTItYzExMGJkNTcxZjli", uri="/case-user-data/431b971d9e1441d381adb277de4f39f8/test", response="ef5d94e786175aa9707d7b73e94298c1", qop=auth, nc=00000023, cnonce="cc84cadcfa74bf45"

On WAFFLE 1.5, this gives the error:

2014-11-05 16:52:06,606 [qtp987834065-67] 23773 WARN  org.eclipse.jetty.servlet.ServletHandler - /case-user-data/431b971d9e1441d381adb277de4f39f8/test
java.lang.StringIndexOutOfBoundsException: String index out of range: 246
    at java.lang.String.charAt(String.java:646)
    at waffle.util.Base64.decode(Unknown Source)
    at waffle.util.AuthorizationHeader.getTokenBytes(Unknown Source)
... omitting jetty stuff

On WAFFLE 1.7, you get a different error:

2014-11-05 17:03:42,177 [qtp1500151620-59] 11405 WARN  org.eclipse.jetty.servlet.ServletHandler - /case-user-data/431b971d9e1441d381adb277de4f39f8/test
java.lang.IllegalArgumentException: com.google.common.io.BaseEncoding$DecodingException: Expected padding character but found '"' at index 10
    at com.google.common.io.BaseEncoding.decode(BaseEncoding.java:228)
    at waffle.util.AuthorizationHeader.getTokenBytes(AuthorizationHeader.java:71)
    ... omitting jetty stuff
Caused by: com.google.common.io.BaseEncoding$DecodingException: Expected padding character but found '"' at index 10
    at com.google.common.io.BaseEncoding$StandardBaseEncoding$2.read(BaseEncoding.java:671)
    at com.google.common.io.BaseEncoding.decodeChecked(BaseEncoding.java:245)
    at com.google.common.io.BaseEncoding.decode(BaseEncoding.java:226)
    ... 35 more

I assume that WAFFLE is making some assumption that the string it receives will be in a particular format and then when it isn't, is just barfing on it when it should be ignoring it and letting a later filter or servlet handle the header.

[hazendaz]

Internal base64 was switched to guava. I'll take a deep look at this tonight. Do you have the entire data stream that was being processed that you can add here?

--- Original Message ---

From: "Trejkaz (pen name)" notifications@github.com
Sent: November 5, 2014 1:08 AM
To: "dblock/waffle" waffle@noreply.github.com
Subject: [waffle] Error parsing base64 (for non-WAFFLE authentication header) (#164)

I was trying out Milton which appears to have its own authentication enabled by default (something I will be desperately trying to rip out). So my browser was sending a header which looks like this:

Authorization:Digest username="admin", realm="milton", nonce="YjNjZDgxNDYtOGIwMS00NDk0LTlkMTItYzExMGJkNTcxZjli", uri="/case-user-data/431b971d9e1441d381adb277de4f39f8/test", response="ef5d94e786175aa9707d7b73e94298c1", qop=auth, nc=00000023, cnonce="cc84cadcfa74bf45"

On WAFFLE 1.5, this gives the error:

2014-11-05 16:52:06,606 [qtp987834065-67] 23773 WARN  org.eclipse.jetty.servlet.ServletHandler - /case-user-data/431b971d9e1441d381adb277de4f39f8/test
java.lang.StringIndexOutOfBoundsException: String index out of range: 246
    at java.lang.String.charAt(String.java:646)
    at waffle.util.Base64.decode(Unknown Source)
    at waffle.util.AuthorizationHeader.getTokenBytes(Unknown Source)
    ... omitting jetty stuff

On WAFFLE 1.7, you get a different error:

2014-11-05 17:03:42,177 [qtp1500151620-59] 11405 WARN  org.eclipse.jetty.servlet.ServletHandler - /case-user-data/431b971d9e1441d381adb277de4f39f8/test
java.lang.IllegalArgumentException: com.google.common.io.BaseEncoding$DecodingException: Expected padding character but found '"' at index 10
    at com.google.common.io.BaseEncoding.decode(BaseEncoding.java:228)
    at waffle.util.AuthorizationHeader.getTokenBytes(AuthorizationHeader.java:71)
    ... omitting jetty stuff

Caused by: com.google.common.io.BaseEncoding$DecodingException: Expected padding character but found '"' at index 10
at com.google.common.io.BaseEncoding$StandardBaseEncoding$2.read(BaseEncoding.java:671)
at com.google.common.io.BaseEncoding.decodeChecked(BaseEncoding.java:245)
at com.google.common.io.BaseEncoding.decode(BaseEncoding.java:226)
... 35 more

I assume that WAFFLE is making some assumption that the string it receives will be in a particular format and then when it isn't, is just barfing on it when it should be ignoring it and letting a later filter or servlet handle the header.

---

Reply to this email directly or view it on GitHub:
#164

[hakanai]

Yeah, although I don't think it adds much:

GET /case-user-data/431b971d9e1441d381adb277de4f39f8/test HTTP/1.1
Host: 127.0.0.1:27443
Connection: keep-alive
Cache-Control: max-age=0
Authorization: Digest username="admin", realm="milton", nonce="YjNjZDgxNDYtOGIwMS00NDk0LTlkMTItYzExMGJkNTcxZjli", uri="/case-user-data/431b971d9e1441d381adb277de4f39f8/test", response="30d2d15e89e0b7596325a12852ae6ca5", qop=auth, nc=00000025, cnonce="fb2f97a275d3d9cb"
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,*/*;q=0.8
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_10_0) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/38.0.2125.111 Safari/537.36
Accept-Encoding: gzip,deflate,sdch
Accept-Language: en-US,en;q=0.8
Cookie: JSESSIONID=1w6q1416g102klgrg4978szyt

[hazendaz]

Can you post the entire stack trace as it is given? I think that is going to help out a lot more. I want to walk all the way back. I have taken your use case and setup a junit test which does produce the same error. In this case, guava did produce a more appropriate message so no bug there.

Now as far as waffle just ignoring and moving on that does not seem right to me. If we capture the request, we need to handle it in the filter otherwise it seems like a security defect would occur. So if we don't like the authorization, I think we would want to raise a security error in general. I read up on this type of digest as I'm not familiar with using it. It seems that it might be possible to build out support for it as it is somewhat closely related to ntlm.

@dblock Any thoughts on this?

[dblock]

I think very specifically Digest authorization is not supported by Waffle. It would be helpful to get the entire HTTP conversation because I bet the server is challenging the client with Digest and that's wrong and should be disabled (but it's not coming from Waffle).

[hakanai]

Regarding providing the "entire conversation", there was only one request/response. I omitted the response because it was just the one from WAFFLE throwing an exception back.

If the server has challenged me for Digest auth in the past, and it would have been Milton which did that, which I already mentioned right back at the initial report, then that would have been on an earlier request which I obviously can't provide because I don't have a time machine. If you provide me a time machine, though, I'll happily go back in time and tell myself to start Wireshark before evaluating Milton. (Although, frankly, I would rather use that time to tell myself not to waste the day trying to integrate Milton in the first place.)

Despite the trigger obviously being Milton, it still seems odd to me that WAFFLE is behaving like this. You say that Digest authentication is "not supported". Should WAFFLE not, then, immediately see that it is Digest authentication (it's a simple check on the prefix of the value) and return some sensible 4xx error? Why is it bothering to parse Base64 which isn't Base64? (And if people are going to talk about security defects as an excuse for not fixing this, surely "not validating shit passed to you" is one of the biggest security defects around...)

[dblock]

Fair enough, waffle's AuthorizationHeader class should handle this. Open an
issue and pull requests welcome.

On Sunday, November 9, 2014, Trejkaz (pen name) notifications@github.com
wrote:

Regarding providing the "entire conversation", there was only one
request/response. I omitted the response because it was just the one from
WAFFLE throwing an exception back.

If the server has challenged me for Digest auth in the past, and it would
have been Milton which did that, which I already mentioned right back at
the initial report, then that would have been on an earlier request which I
obviously can't provide because I don't have a time machine. If you provide
me a time machine, though, I'll happily go back in time and tell myself to
start Wireshark before evaluating Milton. (Although, frankly, I would
rather use that time to tell myself not to waste the day trying to
integrate Milton in the first place.)

Despite the trigger obviously being Milton, it still seems odd to me that
WAFFLE is behaving like this. You say that Digest authentication is "not
supported". Should WAFFLE not, then, immediately see that it is Digest
authentication (it's a simple check on the prefix of the value) and return
some sensible 4xx error? Why is it bothering to parse Base64 which isn't
Base64? (And if people are going to talk about security defects as an
excuse for not fixing this, surely "not validating shit passed to you" is
one of the biggest security defects around...)

—
Reply to this email directly or view it on GitHub
#164 (comment).

dB. | Moscow - Geneva - Seattle - New York
code.dblock.org - @dblockdotorg http://twitter.com/#!/dblockdotorg -
artsy.net - github/dblock https://github.com/dblock

[dblock]

I mean this is already an issue :)

[hazendaz]

How might we handle this? 4xx error condition? My thing on this is that we don't support this digest. It is full of base64 and would need parsed differently. By security issue I was referring to ignoring it as that would likely allow it to simply work. I'm not really clear on why waffle is being used in this context. Milton has its own authentication so where is value of using waffle? Plus a full stack trace should show me the full path waffle took as there are multiple ways to get to this. I'd like to make a quick fix and later look at building this out to support this digest. It is closely related to ntlm so in theory should not be too difficult to implement.

--- Original Message ---

From: "Daniel Doubrovkine (dB.) @dblockdotorg" notifications@github.com
Sent: November 9, 2014 9:57 AM
To: "dblock/waffle" waffle@noreply.github.com
Cc: "Jeremy Landis" jeremylandis@hotmail.com
Subject: Re: [waffle] Error parsing base64 (for non-WAFFLE authentication header) (#164)

I mean this is already an issue :)

---

Reply to this email directly or view it on GitHub:
#164 (comment)

[hakanai]

It was never my intent to layer two kinds of authentication with each other. Our application already uses WAFFLE at the top-level, filtering authenticate anything in the server, regardless of what it is. I was trying out Milton as one possible option for providing WebDAV and had no idea that it was trying to implement the full stack... :(

Stack trace (I wasn't at work when making the last reply so I wasn't able to grab it):

This is apparently a slightly different one because the index is different, but oh well. The logs from that day are enormous.

java.lang.StringIndexOutOfBoundsException: String index out of range: 247
        at java.lang.String.charAt(String.java:646)
        at waffle.util.Base64.decode(Unknown Source)
        at waffle.util.AuthorizationHeader.getTokenBytes(Unknown Source)
        at waffle.servlet.spi.NegotiateSecurityFilterProvider.doFilter(Unknown Source)
        at waffle.servlet.spi.SecurityFilterProviderCollection.doFilter(Unknown Source)
        at waffle.servlet.NegotiateSecurityFilter.doFilter(Unknown Source)
        at com.acme.server.web.util.SequentialFilter$PrependFilterChain.doFilter(SequentialFilter.java:99)
        at com.acme.server.web.security.GetUserFromSessionFilter.doFilter(GetUserFromSessionFilter.java:90)
        at com.acme.server.web.util.BaseFilter.doFilter(BaseFilter.java:33)
        at com.acme.server.web.util.SequentialFilter$PrependFilterChain.doFilter(SequentialFilter.java:99)
        at com.acme.server.web.security.UnauthorisedFilter.doFilter(UnauthorisedFilter.java:35)
        at com.acme.server.web.util.BaseFilter.doFilter(BaseFilter.java:33)
        at com.acme.server.web.util.SequentialFilter$PrependFilterChain.doFilter(SequentialFilter.java:99)
        at com.acme.server.web.util.SequentialFilter.doFilter(SequentialFilter.java:62)
        at com.acme.server.web.util.BaseFilter.doFilter(BaseFilter.java:33)
        at com.acme.server.web.security.CombinedSecurityFilter.doFilter(CombinedSecurityFilter.java:124)
        at com.acme.server.web.util.BaseFilter.doFilter(BaseFilter.java:33)
        at org.eclipse.jetty.servlet.ServletHandler$CachedChain.doFilter(ServletHandler.java:1423)
        at com.acme.server.web.legacy.LegacyXmlRpcServerInfoFilter.doFilter(LegacyXmlRpcServerInfoFilter.java:121)
        at com.acme.server.web.util.BaseFilter.doFilter(BaseFilter.java:33)
        at org.eclipse.jetty.servlet.ServletHandler$CachedChain.doFilter(ServletHandler.java:1423)
        at org.eclipse.jetty.servlet.ServletHandler.doHandle(ServletHandler.java:450)
        at org.eclipse.jetty.server.session.SessionHandler.doHandle(SessionHandler.java:211)
        at org.eclipse.jetty.server.handler.ContextHandler.doHandle(ContextHandler.java:1083)
        at org.eclipse.jetty.servlet.ServletHandler.doScope(ServletHandler.java:379)
        at org.eclipse.jetty.server.session.SessionHandler.doScope(SessionHandler.java:175)
        at org.eclipse.jetty.server.handler.ContextHandler.doScope(ContextHandler.java:1017)
        at org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:136)
        at org.eclipse.jetty.server.handler.ContextHandlerCollection.handle(ContextHandlerCollection.java:258)
        at org.eclipse.jetty.server.handler.HandlerCollection.handle(HandlerCollection.java:109)
        at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:97)
        at org.eclipse.jetty.server.Server.handle(Server.java:445)
        at org.eclipse.jetty.server.HttpChannel.handle(HttpChannel.java:260)
        at org.eclipse.jetty.server.HttpConnection.onFillable(HttpConnection.java:225)
        at org.eclipse.jetty.io.AbstractConnection$ReadCallback.run(AbstractConnection.java:358)
        at org.eclipse.jetty.util.thread.QueuedThreadPool.runJob(QueuedThreadPool.java:596)
        at org.eclipse.jetty.util.thread.QueuedThreadPool$3.run(QueuedThreadPool.java:527)
        at java.lang.Thread.run(Thread.java:745)

It's a bit of a tower of nested filters...

There is a second stack trace where we were calling AuthorizationHeader directly as a utility class, which I can probably patch around on our end by checking the content before constructing it.

[hazendaz]

Thanks @trejkaz It will be tomorrow evening before I can get a change to look further at this.

[dblock]

@hazendaz All I am saying is that AuthorizationHeader in this case should not blow up with a "java.lang.StringIndexOutOfBoundsException: String index out of range: 246", but with something like "Unsupported Authorization Header type Digest".

Conceptually, AuthorizationHeader encapsulates all known authorization headers, and those things have some RFC-defined content that it eventually should be able to parse fully.

[hazendaz]

Sorry for the delay here, got busy with the paying job.

The second stack trace provided appears to be from waffle-jna 1.5 but was useful in seeing that the negotiate filter was turned on. I have a quick fix to try around this base encoding and if it fails to throw a runtime exception that states the authentication header is bad. This will not solve the key issue related to how waffle and milton are trying to be used together. It will however do a better job of indicating the core issue which should assist others with debugging authentication issues. I also looked at other locations the base encoding is being performed and do not think a patch is needed elsewhere. This one because of how it is coded is harder to otherwise check before hand easily so just letting the try catch it here should work. I will submit a pull request soon.

[hazendaz]

Please see #166 for the try catch only. If that is sufficient to help on this condition, can we close out this issue?

[hazendaz]

@trejkaz

Snapshot version of waffle-jna 1.7.1 is available here -> https://oss.sonatype.org/content/repositories/snapshots/com/github/dblock/waffle/waffle-jna/1.7.1-SNAPSHOT/

Please give this a try and let me know if it solves the issues you are experiencing. If it does, I expect to have this released tomorrow evening. This fix will only send back a new error condition that better highlights the issue.

[dblock]

Lets close this. Reopen if the issue persists.