-
Notifications
You must be signed in to change notification settings - Fork 86
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
DIGEST-MD5 wrong hash #56
Comments
I think the problem here is the encoding of the string before it is passed to the hashing function. Looks like only fixed-width encoding is supported, defaulting to 8-bit (ASCII) characters. I'm surprised that upper-case characters aren't working, but not surprised by accented characters. After skimming parts of RFC 6120/6122, I believe that the JID should be UTF-8 encoded. Looks like there is a function in crypt.js for UTF-8 encoding, but another option is to move to v2.2 of the MD5 hashing library (http://pajhome.org.uk/crypt/md5/instructions.html). |
@gavllew Thanks for your help! I tried v2.2 of MD5 tools, updated it and applied it to JSJaC and... it does not work, even for "normal" usernames. I get the challenge not matching error everywhere now. Thus, even if I when I compare the v2.2 hex_md5() function with PHP's one and luckily hex_md5('éé') gives the same hash as PHP's md5('éé') - where v2.1 gives a completely different hash (it's broken, then). |
Just a quick check: what did you do with the str_md5 on line 899 of JSJaCConnection? This has to be handled differently to the hex_md5 calls. I suspect what you would need is: rstr_md5(str2rstr_utf8(...)) This makes sure the MD5 output is used in binary form (not hex encoded into ASCII). |
Can you try using.https://raw.github.com/dhruvbird/wikipins.org/master/static/md5.js instead? |
Mhh doesn't work either. I had some console tests done, and here's what I get: With md5.js v2.2:
Connecting with jappix@jappix.com doesn't work. It breaks everything. With md5.js v2.1:
Connecting with jappix@jappix.com works fine, as expected. Now the problem is with rstr_md5(), I'll try to patch it with the old one to see what I get then. |
OK, I think I get it now. In v2.2, hex_md5 expects UTF-8 input, not binary/raw string input. rstr_md5 expects raw string input, and provides raw string output, so the two functions cannot be combined as above. Instead you do the following: rstr2hex(rstr_md5(rstr_md5(str2rstr_utf8('jappix')))); So I was correct in what should happen on line 899, but when hashing the A1 variable you need to use rstr_md5 followed by rstr2hex (instead of just using hex_md5). |
Ok. I tested what I thought I understood but still not working. Here's my not-working patch: var A1 = rstr_md5(str2rstr_utf8(this.username+':'+this.domain+':'+this.pass))+
':'+this._nonce+':'+this._cnonce;
var HA1 = rstr2hex(rstr_md5(A1));
var A2 = 'AUTHENTICATE:'+this._digest_uri;
var HA2 = rstr2hex(rstr_md5(rstr_md5(str2rstr_utf8(A2))));
var response = hex_md5(HA1+':'+this._nonce+':'+this._nc+':'+
this._cnonce+':auth:'+HA2);
var rPlain = 'username="'+this.username+'",realm="'+this.domain+
'",nonce="'+this._nonce+'",cnonce="'+this._cnonce+'",nc="'+this._nc+
'",qop=auth,digest-uri="'+this._digest_uri+'",response="'+response+
'",charset="utf-8"';
this._sendRaw("<response xmlns='urn:ietf:params:xml:ns:xmpp-sasl'>"+
b64encode(rPlain)+"</response>",
this._doSASLAuthDigestMd5S2); |
You don't need to hash A2 twice to get HA2; just a single hex_md5 call will do there. |
Yep, still not working. I now have this code, which follows the spec: var X = this.username+':'+this.domain+':'+this.pass;
var Y = rstr_md5(X);
var A1 = Y+':'+this._nonce+':'+this._cnonce;
var HA1 = hex_md5(A1);
var A2 = 'AUTHENTICATE:'+this._digest_uri;
var HA2 = hex_md5(A2);
var response = hex_md5(HA1+':'+this._nonce+':'+this._nc+':'+
this._cnonce+':auth:'+HA2); Using:
|
At a glace I can't see any difference between that and your previous code snippet - you're still hashing A2 twice. |
Yep, because it's in the spec. Have a look to points 6 and 8. |
I'm not sure what spec you are looking at. I'm looking at RFC 2831 section 2.1.2.1., where the calculation of the response value only hashes A2 once, but uses the hex representation of the hash output. |
Yep so following RFC 2831 this gives: /* A1 = { H( { username-value, ":", realm-value, ":", passwd } ),
":", nonce-value, ":", cnonce-value, ":", authzid-value } */
var A1 = rstr_md5(this.username+':'+this.domain+':'+this.pass)+':'+this._nonce+':'+this._cnonce;
/* A2 = { "AUTHENTICATE:", digest-uri-value } */
var A2 = 'AUTHENTICATE:'+this._digest_uri;
/* HEX( KD ( HEX(H(A1)),
{ nonce-value, ":" nc-value, ":",
cnonce-value, ":", qop-value, ":", HEX(H(A2)) })) */
var response = hex_md5(hex_md5(A1)+':'+this._nonce+':'+this._nc+':'+this._cnonce+':auth:'+hex_md5(A2)); And still doesn't work! |
No, it won't - I tried to explain previously that you cannot use the output from rstr_md5 as input to hex_md5, as it will misinterpret it as UTF8. Your previous code snippet was nearly correct, apart from the double hash of A2. var A1 = rstr_md5(str2rstr_utf8(this.username+':'+this.domain+':'+this.pass))+
':'+this._nonce+':'+this._cnonce;
var HA1 = rstr2hex(rstr_md5(A1));
var A2 = 'AUTHENTICATE:'+this._digest_uri;
var HA2 = hex_md5(A2);
var response = hex_md5(HA1+':'+this._nonce+':'+this._nc+':'+
this._cnonce+':auth:'+HA2); This code assumes that nonce and cnonce are plain ASCII, and thus don't need the str2rstr_utf8 conversion. To be even safer, do the following: var A1 = rstr_md5(str2rstr_utf8(this.username+':'+this.domain+':'+this.pass));
A1 += str2rstr_utf8(':'+this._nonce+':'+this._cnonce);
var HA1 = rstr2hex(rstr_md5(A1)); |
Nope, still doesn't work. Response hash doesn't match with the server calculated one, even with a non-accented username. I tried all possible way on my end, I'm afraid the rstr_md5() concatenation with ':'+this._nonce+':'+this._cnonce in A1 var does not work as expected. |
Found out. This is because the hex_md5 function also make an UTF-8 conversion. Working on this to get a nice patch out, I'll provide a pull request in a few minutes. Thanks for your help @gavllew and sorry for taking time to understand. I'm not very familiar to all this cryptographic stuff and all those low-level programming stuff ;) |
No problems - glad you got it working in the end! |
Can be considered as resolved after pull. |
Hey,
It seems the DIGEST-MD5 processing function creates a Base64 hash which is not matching any of the available XMPP server verification once they got this hash.
This happens when username (maybe server domain too?) contains UTF-8 chars such as accents or even upper case chars.
As a result:
All those account succeeded using Psi+ DIGEST-MD5 auth method.
I spent some hours debugging and trying to fix & patch JSJaC but the reason of this issue is very obscure to me. I only know something is fucked up on the DIGEST-MD5 processing algorithm.
I checked hidden NULL chars (\0) but did not found any.
This bug impacts a wide range of Jappix users, that's why it's critical to us all ;)
The text was updated successfully, but these errors were encountered: