QPID-1536: modify the B64MD5 PD to take plain text input and perform the required hashing itself in order to present a consistent interface for user management. Alter management console to use mbean versioning to detect this and send plaintext to v2+ user management mbeans. Update RMIPasswordAuthenticator to make use of the new PD input consistency

git-svn-id: https://svn.apache.org/repos/asf/qpid/trunk/qpid@748680 13f79535-47bb-0310-9956-ffa450edef68

5 files changed, 229 insertions, 106 deletions

diff --git a/java/broker/src/main/java/org/apache/qpid/server/security/auth/database/Base64MD5PasswordFilePrincipalDatabase.java b/java/broker/src/main/java/org/apache/qpid/server/security/auth/database/Base64MD5PasswordFilePrincipalDatabase.java
index 69ad9014db..3c211746e3 100644
--- a/java/broker/src/main/java/org/apache/qpid/server/security/auth/database/Base64MD5PasswordFilePrincipalDatabase.java
+++ b/java/broker/src/main/java/org/apache/qpid/server/security/auth/database/Base64MD5PasswordFilePrincipalDatabase.java
@@ -152,8 +152,39 @@ public class Base64MD5PasswordFilePrincipalDatabase implements PrincipalDatabase
     public boolean verifyPassword(String principal, char[] password) throws AccountNotFoundException
     {
         char[] pwd = lookupPassword(principal);
+        
+        if (pwd == null)
+        {
+            throw new AccountNotFoundException("Unable to lookup the specfied users password");
+        }
+        
+        byte[] byteArray = new byte[password.length];
+        int index = 0;
+        for (char c : password)
+        {
+            byteArray[index++] = (byte) c;
+        }
+        
+        byte[] MD5byteArray;
+        try
+        {
+            MD5byteArray = HashedUser.getMD5(byteArray);
+        }
+        catch (Exception e1)
+        {
+            _logger.warn("Unable to hash password for user '" + principal + "' for comparison");
+            return false;
+        }
+        
+        char[] hashedPassword = new char[MD5byteArray.length];
 
-        return compareCharArray(pwd, password);
+        index = 0;
+        for (byte c : MD5byteArray)
+        {
+            hashedPassword[index++] = (char) c;
+        }
+
+        return compareCharArray(pwd, hashedPassword);
     }
     
     private boolean compareCharArray(char[] a, char[] b)
@@ -193,7 +224,7 @@ public class Base64MD5PasswordFilePrincipalDatabase implements PrincipalDatabase
             {
                 _userUpdate.lock();
                 char[] orig = user.getPassword();
-                user.setPassword(password);
+                user.setPassword(password,false);
 
                 try
                 {
@@ -204,7 +235,7 @@ public class Base64MD5PasswordFilePrincipalDatabase implements PrincipalDatabase
                     _logger.error("Unable to save password file, password change for user'"
                                   + principal + "' will revert at restart");
                     //revert the password change
-                    user.setPassword(orig);
+                    user.setPassword(orig,true);
                     return false;
                 }
                 return true;
@@ -230,7 +261,17 @@ public class Base64MD5PasswordFilePrincipalDatabase implements PrincipalDatabase
             return false;
         }
 
-        HashedUser user = new HashedUser(principal.getName(), password);
+        HashedUser user;
+        try
+        {
+            user = new HashedUser(principal.getName(), password);
+        }
+        catch (Exception e1)
+        {
+            _logger.warn("Unable to create new user '" + principal.getName() + "'");
+            return false;
+        }
+
 
         try
         {
diff --git a/java/broker/src/main/java/org/apache/qpid/server/security/auth/database/HashedUser.java b/java/broker/src/main/java/org/apache/qpid/server/security/auth/database/HashedUser.java
index 4d92e3fb4c..3690e7f92a 100644
--- a/java/broker/src/main/java/org/apache/qpid/server/security/auth/database/HashedUser.java
+++ b/java/broker/src/main/java/org/apache/qpid/server/security/auth/database/HashedUser.java
@@ -25,6 +25,7 @@ import org.apache.commons.codec.binary.Base64;
 import org.apache.log4j.Logger;
 
 import java.io.UnsupportedEncodingException;
+import java.security.MessageDigest;
 import java.security.NoSuchAlgorithmException;
 import java.security.Principal;
 
@@ -63,10 +64,22 @@ public class HashedUser implements Principal
         }
     }
 
-    public HashedUser(String name, char[] password)
+    public HashedUser(String name, char[] password) throws UnsupportedEncodingException, NoSuchAlgorithmException
     {
         _name = name;
-        setPassword(password);
+        setPassword(password,false);
+    }
+
+    public static byte[] getMD5(byte[] data) throws NoSuchAlgorithmException, UnsupportedEncodingException
+    {
+        MessageDigest md = MessageDigest.getInstance("MD5");
+
+        for (byte b : data)
+        {
+            md.update(b);
+        }
+
+        return md.digest();
     }
 
     public String getName()
@@ -84,9 +97,31 @@ public class HashedUser implements Principal
         return _password;
     }
 
-    void setPassword(char[] password)
+    void setPassword(char[] password, boolean alreadyHashed) throws UnsupportedEncodingException,  NoSuchAlgorithmException
     {
-        _password = password;
+        if(alreadyHashed){
+            _password = password;
+        }
+        else
+        {
+            byte[] byteArray = new byte[password.length];
+            int index = 0;
+            for (char c : password)
+            {
+                byteArray[index++] = (byte) c;
+            }
+            
+            byte[] MD5byteArray = getMD5(byteArray);
+            
+            _password = new char[MD5byteArray.length];
+
+            index = 0;
+            for (byte c : MD5byteArray)
+            {
+                _password[index++] = (char) c;
+            }
+        }
+        
         _modified = true;
         _encodedPassword = null;
     }
diff --git a/java/broker/src/main/java/org/apache/qpid/server/security/auth/rmi/RMIPasswordAuthenticator.java b/java/broker/src/main/java/org/apache/qpid/server/security/auth/rmi/RMIPasswordAuthenticator.java
index 378b17e733..77040e896c 100644
--- a/java/broker/src/main/java/org/apache/qpid/server/security/auth/rmi/RMIPasswordAuthenticator.java
+++ b/java/broker/src/main/java/org/apache/qpid/server/security/auth/rmi/RMIPasswordAuthenticator.java
@@ -20,23 +20,14 @@
  */
 package org.apache.qpid.server.security.auth.rmi;
 
-import java.io.IOException;
-import java.io.UnsupportedEncodingException;
-import java.security.MessageDigest;
-import java.security.NoSuchAlgorithmException;
-import java.util.Arrays;
 import java.util.Collections;
 
 import javax.management.remote.JMXAuthenticator;
 import javax.management.remote.JMXPrincipal;
 import javax.security.auth.Subject;
-import javax.security.auth.callback.PasswordCallback;
 import javax.security.auth.login.AccountNotFoundException;
 
-import org.apache.qpid.server.security.auth.database.Base64MD5PasswordFilePrincipalDatabase;
-import org.apache.qpid.server.security.auth.database.PlainPasswordFilePrincipalDatabase;
 import org.apache.qpid.server.security.auth.database.PrincipalDatabase;
-import org.apache.qpid.server.security.auth.sasl.UsernamePrincipal;
 
 public class RMIPasswordAuthenticator implements JMXAuthenticator
 {
@@ -48,7 +39,6 @@ public class RMIPasswordAuthenticator implements JMXAuthenticator
     static final String CREDENTIALS_REQUIRED = "User details are required. " +
     		            "Please ensure you are using an up to date management console to connect.";
     
-    public static final String DEFAULT_ENCODING = "utf-8";
     private PrincipalDatabase _db = null;
 
     public RMIPasswordAuthenticator()
@@ -91,56 +81,26 @@ public class RMIPasswordAuthenticator implements JMXAuthenticator
             throw new SecurityException(SHOULD_BE_NON_NULL);
         }
         
+        // Verify that a PD has been set.
+        if (_db == null)
+        {
+            throw new SecurityException(UNABLE_TO_LOOKUP);
+        }
+        
         boolean authenticated = false;
 
         // Perform authentication
         try
         {
-            PasswordCallback pwCallback = new PasswordCallback("prompt",false);
-            UsernamePrincipal uname = new UsernamePrincipal(username);
-            
-            if (_db instanceof Base64MD5PasswordFilePrincipalDatabase)
-            {
-                //retrieve the stored password for the given user
-                _db.setPassword(uname, pwCallback);
-                
-                //compare the MD5Hash of the given password with the stored value
-                if (Arrays.equals(getMD5Hash(password), pwCallback.getPassword()))
-                {                  
-                    authenticated = true;
-                }
-            }
-            else if (_db instanceof PlainPasswordFilePrincipalDatabase)
-            {
-                //retrieve the users stored password and compare with given value
-                _db.setPassword(uname, pwCallback);
-                
-                if (password.equals(new String(pwCallback.getPassword())))
-                {
-                    authenticated = true;
-                }
-            }
-            else
-            {
-                throw new SecurityException(UNABLE_TO_LOOKUP);
+            if (_db.verifyPassword(username, password.toCharArray()))
+            {            
+                authenticated = true;
             }
         }
         catch (AccountNotFoundException e)
         {
             throw new SecurityException(INVALID_CREDENTIALS);
         }
-        catch (UnsupportedEncodingException e)
-        {
-            throw new SecurityException(UNABLE_TO_LOOKUP);
-        }
-        catch (NoSuchAlgorithmException e)
-        {
-            throw new SecurityException(UNABLE_TO_LOOKUP);
-        }
-        catch (IOException e)
-        {
-            throw new SecurityException(UNABLE_TO_LOOKUP);
-        }
 
         if (authenticated)
         {
@@ -155,28 +115,5 @@ public class RMIPasswordAuthenticator implements JMXAuthenticator
             throw new SecurityException(INVALID_CREDENTIALS);
         }
     }
-    
-    public static char[] getMD5Hash(String text) throws NoSuchAlgorithmException, UnsupportedEncodingException
-    {
-        byte[] data = text.getBytes(DEFAULT_ENCODING);
 
-        MessageDigest md = MessageDigest.getInstance("MD5");
-
-        for (byte b : data)
-        {
-            md.update(b);
-        }
-
-        byte[] digest = md.digest();
-
-        char[] hash = new char[digest.length ];
-
-        int index = 0;
-        for (byte b : digest)
-        {            
-            hash[index++] = (char) b;
-        }
-
-        return hash;
-    }
 }
\ No newline at end of file
diff --git a/java/broker/src/test/java/org/apache/qpid/server/security/auth/database/Base64MD5PasswordFilePrincipalDatabaseTest.java b/java/broker/src/test/java/org/apache/qpid/server/security/auth/database/Base64MD5PasswordFilePrincipalDatabaseTest.java
index b5034d9f5d..413b974986 100644
--- a/java/broker/src/test/java/org/apache/qpid/server/security/auth/database/Base64MD5PasswordFilePrincipalDatabaseTest.java
+++ b/java/broker/src/test/java/org/apache/qpid/server/security/auth/database/Base64MD5PasswordFilePrincipalDatabaseTest.java
@@ -22,8 +22,10 @@ package org.apache.qpid.server.security.auth.database;
 
 import junit.framework.TestCase;
 
+import javax.security.auth.callback.PasswordCallback;
 import javax.security.auth.login.AccountNotFoundException;
 
+import org.apache.commons.codec.binary.Base64;
 import org.apache.qpid.server.security.auth.sasl.UsernamePrincipal;
 
 import java.io.BufferedReader;
@@ -33,7 +35,9 @@ import java.io.FileNotFoundException;
 import java.io.FileReader;
 import java.io.FileWriter;
 import java.io.IOException;
+import java.io.UnsupportedEncodingException;
 import java.security.Principal;
+import java.util.Arrays;
 import java.util.List;
 import java.util.regex.Pattern;
 
@@ -41,12 +45,38 @@ public class Base64MD5PasswordFilePrincipalDatabaseTest extends TestCase
 {
 
     private static final String TEST_COMMENT = "# Test Comment";
-    private String USERNAME = "testUser";
-    private String _username = this.getClass().getName()+"username";
-    private char[] _password = "password".toCharArray();
-    private Principal _principal = new UsernamePrincipal(_username);
+
+    private static final String USERNAME = "testUser";
+    private static final String PASSWORD = "guest";
+    private static final String PASSWORD_B64MD5HASHED = "CE4DQ6BIb/BVMN9scFyLtA==";
+    private static char[] PASSWORD_MD5_CHARS;
+    private static final String PRINCIPAL_USERNAME = "testUserPrincipal";
+    private static final Principal PRINCIPAL = new UsernamePrincipal(PRINCIPAL_USERNAME);
     private Base64MD5PasswordFilePrincipalDatabase _database;
     private File _pwdFile;
+    
+    static
+    {
+        try
+        {
+            Base64 b64 = new Base64();
+            byte[] md5passBytes = PASSWORD_B64MD5HASHED.getBytes(Base64MD5PasswordFilePrincipalDatabase.DEFAULT_ENCODING);
+            byte[] decoded = b64.decode(md5passBytes);
+
+            PASSWORD_MD5_CHARS = new char[decoded.length];
+
+            int index = 0;
+            for (byte c : decoded)
+            {
+                PASSWORD_MD5_CHARS[index++] = (char) c;
+            }
+        }
+        catch (UnsupportedEncodingException e)
+        {
+            fail("Unable to perform B64 decode to get the md5 char[] password");
+        }
+    }
+    
 
     public void setUp() throws Exception
     {
@@ -111,7 +141,56 @@ public class Base64MD5PasswordFilePrincipalDatabaseTest extends TestCase
 
         loadPasswordFile(testFile);
 
-        final String CREATED_PASSWORD = "createdPassword";
+
+        Principal principal = new Principal()
+        {
+            public String getName()
+            {
+                return USERNAME;
+            }
+        };
+
+        assertTrue("New user not created.", _database.createPrincipal(principal, PASSWORD.toCharArray()));
+        
+        PasswordCallback callback = new PasswordCallback("prompt",false);
+        try
+        {
+            _database.setPassword(principal, callback);
+        }
+        catch (AccountNotFoundException e)
+        {
+            fail("user account did not exist");
+        }
+        assertTrue("Password returned was incorrect.", Arrays.equals(PASSWORD_MD5_CHARS, callback.getPassword()));
+
+        loadPasswordFile(testFile);
+
+        try
+        {
+            _database.setPassword(principal, callback);
+        }
+        catch (AccountNotFoundException e)
+        {
+            fail("user account did not exist");
+        }
+        assertTrue("Password returned was incorrect.", Arrays.equals(PASSWORD_MD5_CHARS, callback.getPassword()));
+        
+        assertNotNull("Created User was not saved", _database.getUser(USERNAME));
+
+        assertFalse("Duplicate user created.", _database.createPrincipal(principal, PASSWORD.toCharArray()));
+
+        testFile.delete();
+    }
+    
+    public void testCreatePrincipalIsSavedToFile()
+    {
+
+        File testFile = createPasswordFile(1, 0);
+
+        loadPasswordFile(testFile);
+        
+        final String CREATED_PASSWORD = "guest";
+        final String CREATED_B64MD5HASHED_PASSWORD = "CE4DQ6BIb/BVMN9scFyLtA==";
         final String CREATED_USERNAME = "createdUser";
 
         Principal principal = new Principal()
@@ -122,16 +201,37 @@ public class Base64MD5PasswordFilePrincipalDatabaseTest extends TestCase
             }
         };
 
-        assertTrue("New user not created.", _database.createPrincipal(principal, CREATED_PASSWORD.toCharArray()));
+        _database.createPrincipal(principal, CREATED_PASSWORD.toCharArray());
 
-        loadPasswordFile(testFile);
+        try
+        {
+            BufferedReader reader = new BufferedReader(new FileReader(testFile));
+
+            assertTrue("File has no content", reader.ready());
+
+            assertEquals("Comment line has been corrupted.", TEST_COMMENT, reader.readLine());
 
-        assertNotNull("Created User was not saved", _database.getUser(CREATED_USERNAME));
+            assertTrue("File is missing user data.", reader.ready());
 
-        assertFalse("Duplicate user created.", _database.createPrincipal(principal, CREATED_PASSWORD.toCharArray()));
+            String userLine = reader.readLine();
+
+            String[] result = Pattern.compile(":").split(userLine);
 
+            assertEquals("User line not complete '" + userLine + "'", 2, result.length);
+
+            assertEquals("Username not correct,", CREATED_USERNAME, result[0]);
+            assertEquals("Password not correct,", CREATED_B64MD5HASHED_PASSWORD, result[1]);
+
+            assertFalse("File has more content", reader.ready());
+
+        }
+        catch (IOException e)
+        {
+            fail("Unable to valdate file contents due to:" + e.getMessage());
+        }
         testFile.delete();
     }
+    
 
     public void testDeletePrincipal()
     {
@@ -228,8 +328,8 @@ public class Base64MD5PasswordFilePrincipalDatabaseTest extends TestCase
 
         assertNotNull(testUser);
 
-        String NEW_PASSWORD = "NewPassword";
-        String NEW_PASSWORD_HASH = "TmV3UGFzc3dvcmQ=";
+        String NEW_PASSWORD = "guest";
+        String NEW_PASSWORD_HASH = "CE4DQ6BIb/BVMN9scFyLtA==";
         try
         {
             _database.updatePassword(testUser, NEW_PASSWORD.toCharArray());
@@ -268,7 +368,7 @@ public class Base64MD5PasswordFilePrincipalDatabaseTest extends TestCase
         testFile.delete();
     }
 
-    public void testSetPasswordWithMissingFile()
+    public void testSetPasswordFileWithMissingFile()
     {
         try
         {
@@ -285,7 +385,7 @@ public class Base64MD5PasswordFilePrincipalDatabaseTest extends TestCase
 
     }
 
-    public void testSetPasswordWithReadOnlyFile()
+    public void testSetPasswordFileWithReadOnlyFile()
     {
 
         File testFile = createPasswordFile(0, 0);
@@ -310,28 +410,38 @@ public class Base64MD5PasswordFilePrincipalDatabaseTest extends TestCase
     
     public void testCreateUserPrincipal() throws IOException
     {
-        _database.createPrincipal(_principal, _password);
-        Principal newPrincipal = _database.getUser(_username);
+        _database.createPrincipal(PRINCIPAL, PASSWORD.toCharArray());
+        Principal newPrincipal = _database.getUser(PRINCIPAL_USERNAME);
         assertNotNull(newPrincipal);
-        assertEquals(_principal.getName(), newPrincipal.getName());
+        assertEquals(PRINCIPAL.getName(), newPrincipal.getName());
     }
     
     public void testVerifyPassword() throws IOException, AccountNotFoundException
     {
         testCreateUserPrincipal();
         //assertFalse(_pwdDB.verifyPassword(_username, null));
-        assertFalse(_database.verifyPassword(_username, new char[]{}));
-        assertFalse(_database.verifyPassword(_username, "massword".toCharArray()));
-        assertTrue(_database.verifyPassword(_username, _password));
+        assertFalse(_database.verifyPassword(PRINCIPAL_USERNAME, new char[]{}));
+        assertFalse(_database.verifyPassword(PRINCIPAL_USERNAME, (PASSWORD+"z").toCharArray()));
+        assertTrue(_database.verifyPassword(PRINCIPAL_USERNAME, PASSWORD.toCharArray()));
+        
+        try
+        {
+            _database.verifyPassword("made.up.username", PASSWORD.toCharArray());
+            fail("Should not have been able to verify this non-existant users password.");
+        }
+        catch (AccountNotFoundException e)
+        {
+            // pass
+        }
     }
     
     public void testUpdatePassword() throws IOException, AccountNotFoundException 
     {
         testCreateUserPrincipal();
         char[] newPwd = "newpassword".toCharArray();
-        _database.updatePassword(_principal, newPwd);
-        assertFalse(_database.verifyPassword(_username, _password));
-        assertTrue(_database.verifyPassword(_username, newPwd));
+        _database.updatePassword(PRINCIPAL, newPwd);
+        assertFalse(_database.verifyPassword(PRINCIPAL_USERNAME, PASSWORD.toCharArray()));
+        assertTrue(_database.verifyPassword(PRINCIPAL_USERNAME, newPwd));
     }
-    
+
 }
diff --git a/java/broker/src/test/java/org/apache/qpid/server/security/auth/database/HashedUserTest.java b/java/broker/src/test/java/org/apache/qpid/server/security/auth/database/HashedUserTest.java
index a7d951cb5b..aa85cac758 100644
--- a/java/broker/src/test/java/org/apache/qpid/server/security/auth/database/HashedUserTest.java
+++ b/java/broker/src/test/java/org/apache/qpid/server/security/auth/database/HashedUserTest.java
@@ -34,7 +34,7 @@ public class HashedUserTest extends TestCase
 
     String USERNAME = "username";
     String PASSWORD = "password";
-    String HASHED_PASSWORD = "cGFzc3dvcmQ=";
+    String B64_ENCODED_PASSWORD = "cGFzc3dvcmQ=";
 
     public void testToLongArrayConstructor()
     {
@@ -57,11 +57,11 @@ public class HashedUserTest extends TestCase
     {
         try
         {
-            HashedUser user = new HashedUser(new String[]{USERNAME, HASHED_PASSWORD});
+            HashedUser user = new HashedUser(new String[]{USERNAME, B64_ENCODED_PASSWORD});
             assertEquals("Username incorrect", USERNAME, user.getName());
             int index = 0;
 
-            char[] hash = HASHED_PASSWORD.toCharArray();
+            char[] hash = B64_ENCODED_PASSWORD.toCharArray();
 
             try
             {