waffle
waffle copied to clipboard
Waffle
Java SPNego bug
I think there is a bug introduced in waffle 1.7.
AuthorizationHeader.isNtlmType1PostAuthorizationHeader() returns true for both SPNego message types (NegTokenInit or NegTokenArg), what causes call of auth.resetSecurityToken() inside NegotiateSecurityFilterProvider.doFilter().
Negotiation looses context when NegotiateSecurityFilterProvider receives NegTokenArg message.
This might be problem inside my code since I am implementing my own HttpServletRequest and HttpServletResponse interfaces, but it works in waffle 1.5 and 1.6.
WS
You should be able to turn this into a spec, which will make it easy to fix. Thanks for reporting it.
Reviewing changes logs.
- Waffle 1.5 did not support SPNego.
- Support was added in 1.6 as PR #40.
- In version 1.7, PR #76 was added. That seems to be the clue for this issue.
Support for SPNego was added by @AriSuutariST. Possibly this user can assist on this.
Will try to write test for this issue, but I need some time to do it since I am no too experienced with Java projects.
Hi,
We are using waffle.apache.NegotiateAuthenticator valve at several sites and the SPNego stuff seems to work ok with it. However, I must admit that code I added only detects that those packets are related to authentication and should not be passed to application (for example a servlet). I'm not familiar how Waffle processes the packet after it has been classfied as authentication data, but it makes sense that NegTokenArg might require different processing as it is related to previous NegTokenInit.
@ WiktorS is it possible to verify if this still exists and based on your last comment I'm hoping last year has seen great growth for you with java. If so, we would love to see a PR for this issue if it still exists.
We are observing the same issue and I have a reproduction procedure using ajax request and Internet Explorer :
The symptoms :
- an SPNEGO authentication is properly performed on first nonxhr request
- subsequent nonxhr request are properly handled and authenticated
- then an ajax request occurs :
using
POST, with aContent-Length: 0, and IE behaviors is to resend theAuthorization: Negotiatefor this request
This leads to an invalid 401 Unauthorized being sent.
IE may output the following error in its console :
SCRIPT7002: XMLHttpRequest: Network Error 0x2ef3, Could not complete the operation due to error 00002ef3
This problem did not occur when we were using waffle 1.5
To reproduce :
You can easily reproduce this problem with the following HTML test page:
<!DOCTYPE html>
<html>
<head><script src="//code.jquery.com/jquery-1.12.0.min.js"></script></head>
<body>
<button onclick="jQuery.ajax({ url:document.location.href, type:'POST' });">
Click me to send empty ajax request
</button>
</body>
</html>
Put this page on any site for which authentication is handled by waffle. Use wireshark or any real sniffer to observe the 401 (not the IE11 network panel in which the ajax request will appear has abandonned)
With waffle 1.5 messages with SPNEGO auth ended up into application servlet calls, that's why I first spotted that some functionality was missing. I think that stuff added in 1.6 should be quite ok, but NegTokenArg stuff added in 1.7 might be the problem as the method to detect the authentication packet is not really solid (there is no signature that could be used for detection).
Could you try with 1.6, just to see if it works in this situation ?
We have 1.7 in use in production sites, but there might be no zero-length ajax requests in our application.
@AriSuutariST I'll do a test with 1.6 during the hours to come.
In the mean time, you can also test an ajax request on your site with the simple html file I gave above which I guarantee will not modify anything:) And if you cannot deploy this file, you can also test in the IE developper console by running this code :
var jq = document.createElement('script');
jq.src = "https://code.jquery.com/jquery-1.12.0.min.js";
document.getElementsByTagName('head')[0].appendChild(jq);
// ... give time for script to load, then type.
jQuery.noConflict();
jQuery.ajax({ url:document.location.href, type:'POST' });
Just tested on production site, your javascript example works there (HTTP 200 response) with Waffle 1.7.4, Win 2012 R2, Tomcat 7, JDK 1.8.
Yes, Win 10 + IE11. I'm using Valve in tomcat like this:
<Valve className="waffle.apache.NegotiateAuthenticator" principalFormat="fqn" roleFormat="both" />
But please check with 1.6 if you can.
I'll do a test as soon as I can today. but I have other pending tasks to do before. (your valve configuration did not appear in your comment)
@dblock To avoid resetSecurityToken() mentioned in first comment, I think waffle needs extra method to AuthorizationHeader, maybe something like isNtlmType1PostAuthorizationHeaderContinuation() as original design does not (to my understanding) handle cases where multi-packet negotiation is used for authorization.
That new method could return true when we have NegTokenArg header, false for all other cases. NegotiateSecurityFilterProvider could then check that.
I just tested my test.html file on previous version :
- 1.6 -> same bug
- 1.5 -> no problem
One important difference regarding your setup @AriSuutariST , I'm not using waffle-tomcat but the NegotiateSecurityFilter available in waffle-jna.
I am really willing to help you on this matter, for example by modifying the source code (specially waffle.util.AuthorizationHeader) to test whatever implementation you might have, but I am not familiar enough with the protocol to perfom any modification without risking other regression.
@hazendaz said in in first comment that support was added in 1.6 as PR #40... wouldn't it be logical that this is the source of problem ? As I understand, the problem seems to be in the behavior verified by unittest testIsSPNegoPostAuthorizationHeader which should not be true for ajax request (though I understand my use case differs from the initial report of @WiktorS )
I just configured by development system with filter instead of valve. Works ok... I wonder what is the difference between our environments.
For authentication there is no difference if the request is ajax or not. The problems is somewhere else.
Maybe the zero Content-Length confuses some layer in Waffle to think that request is authentication when it is not.
@AriSuutariST since you can reproduce I think you're in the best place to fix it, looking forward to a pull request.
When a PR comes along...please submit to master and 1.7 branch. I'll cut a fix for both so java 6 users are not left out.
Sent from Outlook Mobile
On Thu, Mar 24, 2016 at 5:38 AM -0700, "Daniel Doubrovkine (dB.) @dblockdotorg" [email protected] wrote:
@AriSuutariST since you can reproduce I think you're in the best place to fix it, looking forward to a pull request.
You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/dblock/waffle/issues/167#issuecomment-200813348
I'm currently debugging it and trying to obtain better understanding what is really going on. Anyway, this is going to take some days so don't hold your breath yet :-)
I see why this occurs with zero-length POST now. AuthorizationHeader.isNtlmType1PostAuthorizationHeader() returns false if contentLength() != 0. This causes bug not to be visible if POST contains any data.
This is what I was trying to tell when i was reffering to testIsSPNegoPostAuthorizationHeader, I'm sorry I did not made myself clear enough.
Tell me if I can help you in any way ! As I have a working environement with an easy reproduction I can quickly verify any lead you might have.
I just did a debugging session with my reproduction step, and it seems to me the problem starts in waffle.servlet.NegotiateSecurityFilter.doFilterPrincipal with the following code :
https://github.com/dblock/waffle/blob/5cba5c6b2ef7821f2f6c9134f9965c9164304a01/Source/JNA/waffle-jna/src/main/java/waffle/servlet/NegotiateSecurityFilter.java#L225-L228
if (this.providers.isPrincipalException(request)) {
// the providers signal to authenticate despite an existing principal, eg. NTLM post
return false;
}
The implementation of waffle.servlet.spi.NegotiateSecurityFilterProvider.isPrincipalException relies on method AuthorizationHeader.isNtlmType1PostAuthorizationHeader to consider that a new authentication must be performed even though the existing one is perfectly valid and as far I know there is no reason to attempt a new one :
https://github.com/dblock/waffle/blob/5cba5c6b2ef7821f2f6c9134f9965c9164304a01/Source/JNA/waffle-jna/src/main/java/waffle/servlet/spi/NegotiateSecurityFilterProvider.java#L111-L117
public boolean isPrincipalException(final HttpServletRequest request) {
final AuthorizationHeader authorizationHeader = new AuthorizationHeader(request);
final boolean ntlmPost = authorizationHeader.isNtlmType1PostAuthorizationHeader();
LOGGER.debug("authorization: {}, ntlm post: {}", authorizationHeader, Boolean.valueOf(ntlmPost));
return ntlmPost;
}
It is important to detect which post as only for authentication - otherwise your application starts receiving spurious zero-length post-requests. I think that it was to reason why I started digging into SPNEGO headers in first place.
I think that original idea in isNtlmType1PostAuthorizationHeader() is this filtering, but I'm not sure. I just mimicked same design for SPNEGO headers. But now the method is used for controlling in resetSecurityToken() calls also, which might be wrong. If the token is bound to connection is there ever a situation where it should be reset (as it is 'reset' for new connections, I suppose).
So would the correct fix be to:
- never reset token like done now
- filter those post requests from application which have no data and are just authentication
The last one is actually not that simple. It seems that browser sends post data with last NegTokenArg packet (there can be multiple). And data can be empty or not.
@OlivierJaquemet I'll be away for a few days, but I'll continue digging into this (unless it has been solved before I get back).
Thank for your explanation of all aspects of this subject @AriSuutariST. There is no hurry as far as I am concerned, I have implemented a (kludge) workaround above the waffle filter for our use case. An official fix in waffle would definitely be better so everybody can benefit from it, including us. So If I can help your later on this matter, i'll be there.
I have been thinking how to get this working in more robust way. Currently isNtlmType1PostAuthorizationHeader() logic is used for two things (based on browsing the source code):
- resetSecurityToken is called for connectionId
- if POST/PUT doesn't contain data (ie. browser is sending only authentication information) request doesn't go to application, it is processed internally in waffle code only.
The later was the original reason for me to start working with SPNEGO stuff. I didn't realize the first case at the time.
I think too that @OlivierJaquemet is correct here, resetSecurityToken call is completely unnecessary. This is because authentication is per each connection. For new connection, no resetting is needed as it is already in "reset" state. For old connection, server / browser will not renegotiate it. So that part of current code could just be removed without any harm.
This leaves filtering out POST/PUTs which are only for authentication (see rfc4559 chapter 6 for details). Maybe instead of trying to decode data in Authorization header isNtlmType1PostAuthorizationHeader() could be implemented by just checking if Authorization header starts with "Negotiate" word and content-length is zero.
Please comment, I'll make the changes if this makes sense to others.
@AriSuutariST I'm not sure I completely understood the modification you plan to make. Do you have a PR/commit that I could look at ? (not that I want to review, but just to better understand it)
Is there any part in the spec that mandates Authorization header to be read and used if a user is already authenticated ?
Because if after your commit, Waffle continues to consider request (POST/PUT && Content-Length==0 && authorization.startsWith "Negotiate") as a trigger to request authentication, it seems to me that some requests will still get caught by an authentication request leading to 401 and preventing proper client side handling, even though there is not reason for that (since a user is already authenticated)