Author: markt Date: Thu Dec 5 15:36:36 2013 New Revision: 1548169 URL: http://svn.apache.org/r1548169 Log: Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=55839 Extend support for digest prefixes {MD5}, {SHA} and {SSHA} to all Realms rather than just the JNDIRealm.
Modified: tomcat/trunk/java/org/apache/catalina/realm/JNDIRealm.java tomcat/trunk/java/org/apache/catalina/realm/RealmBase.java tomcat/trunk/test/org/apache/catalina/core/TesterContext.java tomcat/trunk/test/org/apache/catalina/realm/TestRealmBase.java Modified: tomcat/trunk/java/org/apache/catalina/realm/JNDIRealm.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/realm/JNDIRealm.java?rev=1548169&r1=1548168&r2=1548169&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/catalina/realm/JNDIRealm.java (original) +++ tomcat/trunk/java/org/apache/catalina/realm/JNDIRealm.java Thu Dec 5 15:36:36 2013 @@ -19,11 +19,9 @@ package org.apache.catalina.realm; import java.net.URI; import java.net.URISyntaxException; -import java.nio.charset.StandardCharsets; import java.security.Principal; import java.text.MessageFormat; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collections; import java.util.HashMap; import java.util.Hashtable; @@ -53,7 +51,6 @@ import javax.naming.directory.SearchCont import javax.naming.directory.SearchResult; import org.apache.catalina.LifecycleException; -import org.apache.tomcat.util.codec.binary.Base64; import org.ietf.jgss.GSSCredential; /** @@ -1547,64 +1544,16 @@ public class JNDIRealm extends RealmBase String credentials) throws NamingException { - if (info == null || credentials == null) - return (false); - - String password = info.getPassword(); - if (password == null) - return (false); - // Validate the credentials specified by the user if (containerLog.isTraceEnabled()) containerLog.trace(" validating credentials"); - boolean validated = false; - if (hasMessageDigest()) { - // Some directories prefix the password with the hash type - // The string is in a format compatible with Base64.encode not - // the Hex encoding of the parent class. - if (password.startsWith("{MD5}") || password.startsWith("{SHA}")) { - /* sync since super.digest() does this same thing */ - synchronized (this) { - password = password.substring(5); - md.reset(); - md.update(credentials.getBytes(StandardCharsets.ISO_8859_1)); - byte[] encoded = Base64.encodeBase64(md.digest()); - String digestedPassword = - new String(encoded, StandardCharsets.ISO_8859_1); - validated = password.equals(digestedPassword); - } - } else if (password.startsWith("{SSHA}")) { - // Bugzilla 32938 - /* sync since super.digest() does this same thing */ - synchronized (this) { - password = password.substring(6); - - md.reset(); - md.update(credentials.getBytes(StandardCharsets.ISO_8859_1)); - - // Decode stored password. - byte[] decoded = Base64.decodeBase64(password); - - // Split decoded password into hash and salt. - final int saltpos = 20; - byte[] hash = new byte[saltpos]; - System.arraycopy(decoded, 0, hash, 0, saltpos); - - md.update(decoded, saltpos, decoded.length - saltpos); - - byte[] dp = md.digest(); + if (info == null || credentials == null) + return (false); - validated = Arrays.equals(dp, hash); - } // End synchronized(this) block - } else { - // Hex hashes should be compared case-insensitive - validated = (digest(credentials).equalsIgnoreCase(password)); - } - } else - validated = (digest(credentials).equals(password)); - return (validated); + String password = info.getPassword(); + return compareCredentials(credentials, password); } Modified: tomcat/trunk/java/org/apache/catalina/realm/RealmBase.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/realm/RealmBase.java?rev=1548169&r1=1548168&r2=1548169&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/catalina/realm/RealmBase.java (original) +++ tomcat/trunk/java/org/apache/catalina/realm/RealmBase.java Thu Dec 5 15:36:36 2013 @@ -28,6 +28,7 @@ import java.security.NoSuchAlgorithmExce import java.security.Principal; import java.security.cert.X509Certificate; import java.util.ArrayList; +import java.util.Arrays; import java.util.Locale; import javax.servlet.http.HttpServletResponse; @@ -51,6 +52,7 @@ import org.apache.juli.logging.Log; import org.apache.juli.logging.LogFactory; import org.apache.tomcat.util.buf.B2CConverter; import org.apache.tomcat.util.buf.HexUtils; +import org.apache.tomcat.util.codec.binary.Base64; import org.apache.tomcat.util.descriptor.web.SecurityCollection; import org.apache.tomcat.util.descriptor.web.SecurityConstraint; import org.apache.tomcat.util.res.StringManager; @@ -333,15 +335,8 @@ public abstract class RealmBase extends String serverCredentials = getPassword(username); - boolean validated ; - if ( serverCredentials == null ) { - validated = false; - } else if(hasMessageDigest()) { - validated = serverCredentials.equalsIgnoreCase(digest(credentials)); - } else { - validated = serverCredentials.equals(credentials); - } - if(! validated ) { + boolean validated = compareCredentials(credentials, serverCredentials); + if (!validated) { if (containerLog.isTraceEnabled()) { containerLog.trace(sm.getString("realmBase.authenticateFailure", username)); @@ -500,6 +495,72 @@ public abstract class RealmBase extends } + protected boolean compareCredentials(String userCredentials, + String serverCredentials) { + + if (serverCredentials == null) { + return false; + } + + if (hasMessageDigest()) { + // Some directories and databases prefix the password with the hash + // type. The string is in a format compatible with Base64.encode not + // the normal hex encoding of the digest + if (serverCredentials.startsWith("{MD5}") || + serverCredentials.startsWith("{SHA}")) { + // Server is storing digested passwords with a prefix indicating + // the digest type + String serverDigest = serverCredentials.substring(5); + String userDigest; + synchronized (this) { + md.reset(); + md.update(userCredentials.getBytes(StandardCharsets.ISO_8859_1)); + userDigest = Base64.encodeBase64String(md.digest()); + } + return userDigest.equals(serverDigest); + + } else if (serverCredentials.startsWith("{SSHA}")) { + // Server is storing digested passwords with a prefix indicating + // the digest type and the salt used when creating that digest + + String serverDigestPlusSalt = serverCredentials.substring(6); + + // Need to convert the salt to bytes to apply it to the user's + // digested password. + byte[] serverDigestPlusSaltBytes = + Base64.decodeBase64(serverDigestPlusSalt); + final int saltPos = 20; + byte[] serverDigestBytes = new byte[saltPos]; + System.arraycopy(serverDigestPlusSaltBytes, 0, + serverDigestBytes, 0, saltPos); + + // Generate the digested form of the user provided password + // using the salt + byte[] userDigestBytes; + synchronized (this) { + md.reset(); + // User provided password + md.update(userCredentials.getBytes(StandardCharsets.ISO_8859_1)); + // Add the salt + md.update(serverDigestPlusSaltBytes, saltPos, + serverDigestPlusSaltBytes.length - saltPos); + userDigestBytes = md.digest(); + } + + return Arrays.equals(userDigestBytes, serverDigestBytes); + + } else { + // Hex hashes should be compared case-insensitively + String userDigest = digest(userCredentials); + return serverCredentials.equalsIgnoreCase(userDigest); + } + } else { + // No digests, compare directly + return serverCredentials.equals(userCredentials); + } + } + + /** * Execute a periodic task, such as reloading, etc. This method will be * invoked inside the classloading context of this container. Unexpected Modified: tomcat/trunk/test/org/apache/catalina/core/TesterContext.java URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/core/TesterContext.java?rev=1548169&r1=1548168&r2=1548169&view=diff ============================================================================== --- tomcat/trunk/test/org/apache/catalina/core/TesterContext.java (original) +++ tomcat/trunk/test/org/apache/catalina/core/TesterContext.java Thu Dec 5 15:36:36 2013 @@ -52,6 +52,7 @@ import org.apache.catalina.connector.Req import org.apache.catalina.connector.Response; import org.apache.catalina.deploy.NamingResourcesImpl; import org.apache.juli.logging.Log; +import org.apache.juli.logging.LogFactory; import org.apache.tomcat.InstanceManager; import org.apache.tomcat.JarScanner; import org.apache.tomcat.util.descriptor.web.ApplicationListener; @@ -67,6 +68,8 @@ import org.apache.tomcat.util.descriptor */ public class TesterContext implements Context { + private static final Log log = LogFactory.getLog(TesterContext.class); + private List<String> securityRoles = new ArrayList<>(); @Override public void addSecurityRole(String role) { @@ -108,7 +111,7 @@ public class TesterContext implements Co @Override public Log getLogger() { - return null; + return log; } @Override Modified: tomcat/trunk/test/org/apache/catalina/realm/TestRealmBase.java URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/realm/TestRealmBase.java?rev=1548169&r1=1548168&r2=1548169&view=diff ============================================================================== --- tomcat/trunk/test/org/apache/catalina/realm/TestRealmBase.java (original) +++ tomcat/trunk/test/org/apache/catalina/realm/TestRealmBase.java Thu Dec 5 15:36:36 2013 @@ -17,6 +17,7 @@ package org.apache.catalina.realm; import java.io.IOException; +import java.security.Principal; import java.util.ArrayList; import java.util.List; @@ -47,6 +48,58 @@ public class TestRealmBase { private static final String ROLE3 = "role3"; private static final String ROLE99 = "role99"; + // All digested passwords are the digested form of "password" + private static final String PWD_MD5 = "5f4dcc3b5aa765d61d8327deb882cf99"; + private static final String PWD_SHA = "5baa61e4c9b93f3f0682250b6cf8331b7ee68fd8"; + private static final String PWD_MD5_PREFIX = + "{MD5}X03MO1qnZdYdgyfeuILPmQ=="; + private static final String PWD_SHA_PREFIX = + "{SHA}W6ph5Mm5Pz8GgiULbPgzG37mj9g="; + // Salt added to "password" is "salttoprotectpassword" + private static final String PWD_SSHA_PREFIX = + "{SSHA}oFLhvfQVqFykEWu8v1pPE6nN0QRzYWx0dG9wcm90ZWN0cGFzc3dvcmQ="; + + @Test + public void testDigestMD5() throws Exception { + doTestDigestDigestPasswords(PWD, "MD5", PWD_MD5); + } + + @Test + public void testDigestSHA() throws Exception { + doTestDigestDigestPasswords(PWD, "SHA", PWD_SHA); + } + + @Test + public void testDigestMD5Prefix() throws Exception { + doTestDigestDigestPasswords(PWD, "MD5", PWD_MD5_PREFIX); + } + + @Test + public void testDigestSHAPrefix() throws Exception { + doTestDigestDigestPasswords(PWD, "SHA", PWD_SHA_PREFIX); + } + + @Test + public void testDigestSSHAPrefix() throws Exception { + doTestDigestDigestPasswords(PWD, "SHA", PWD_SSHA_PREFIX); + } + + private void doTestDigestDigestPasswords(String password, + String digest, String digestedPassword) throws Exception { + Context context = new TesterContext(); + TesterMapRealm realm = new TesterMapRealm(); + realm.setContainer(context); + realm.setDigest(digest); + realm.start(); + + realm.addUser(USER1, digestedPassword); + + Principal p = realm.authenticate(USER1, password); + + Assert.assertNotNull(p); + Assert.assertEquals(USER1, p.getName()); + } + @Test public void testUserWithSingleRole() throws IOException { List<String> userRoles = new ArrayList<>(); --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org