You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2013/12/05 16:36:36 UTC

svn commit: r1548169 - in /tomcat/trunk: java/org/apache/catalina/realm/JNDIRealm.java java/org/apache/catalina/realm/RealmBase.java test/org/apache/catalina/core/TesterContext.java test/org/apache/catalina/realm/TestRealmBase.java

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-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org