On 27/06/13 18:51, Konstantin Kolinko wrote:
2013/6/27 Brian Burch <br...@pingtoo.com>:
I eventually got round to integration testing of 7.0.41 yesterday and was
baffled to find I couldn't logon!
To cut a long debugging story short, revision 1491394 has a bug that was
introduced as part of the standardisation of our Base64 handling. I wasn't
sure whether I ought to open a new bug...
Your numbering is wrong, that revision is not ours. It was this one:
http://svn.apache.org/viewvc?diff_format=l&view=revision&revision=1459346
Yes, I see what you mean and agree. I'm puzzled that my svn diff
reported the wrong revision, which is a pity because I put it in the new
thread subject and we will have to live with it unless you can
retrospectively edit the entries?
Thanks for finding the correct revision.
Here is the diff that works for me:
Index: java/org/apache/catalina/realm/JNDIRealm.java
===================================================================
--- java/org/apache/catalina/realm/JNDIRealm.java (revision 1491394)
+++ java/org/apache/catalina/realm/JNDIRealm.java (working copy)
@@ -1573,9 +1573,10 @@
password = password.substring(5);
md.reset();
md.update(credentials.getBytes(Charset.defaultCharset()));
- byte[] decoded = Base64.decodeBase64(md.digest());
+ byte[] digest = md.digest();
+ byte[] base64 = Base64.encodeBase64(digest);
String digestedPassword =
- new String(decoded, B2CConverter.ISO_8859_1);
+ new String(base64, B2CConverter.ISO_8859_1);
validated = password.equals(digestedPassword);
}
} else if (password.startsWith("{SSHA}")) {
In short, s/ decodeBase64 / encodeBase64 /.
Yes. I wanted to break the logic up because I needed to encode/decode my
test cases by hand!
I didn't trust my own external cribs and suspected I was digging into a
difference between the sun and openjdk digest engines.
Pencil/paper/hexadecimal calculator...
I didn't want to go round the loop of another source
change/build/deploy/test/debug/diff. You got the idea, just as I expected.
Imagine my surprise when I got 80% through the exercise and spotted the
"backwards" choice of Base64 method!
It is fun that {MD5}&{SHA} branch and {SSHA} branch there use
different approaches there
(encoding the user-supplied password vs. decoding the stored one).
Actually, I can sort-of understand the person adding SSHA doing his/her
own thing and not wanting to change logic that is stable and works
(well, it did in 7.0.28!).
If we are looking for a vote, I prefer the way MD5/SHA does it. It takes
the user-supplied cleartext and digest/encodes it in a forward
direction, such that the directory hash is never exposed in situations
where the supplied value is incorrect.
BTW. The code is identical in trunk, so this patch works there too.
Thinks... pity some of this stuff doesn't have some lightweight unit tests.
Sorry to be a informal with this notification, but I thought timeliness was
more important than style!
So.... will you deal with it from now on, or would you like me to open
bugs on tc7 and tc8?
Brian
Best regards,
Konstantin Kolinko
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
