Merge pull request #41 from mwoodiupui/DS-861

[DS-861] Salt PasswordAuthentication

dspace-api/src/main/java/org/dspace/content/packager/RoleDisseminator.java

@@ -32,6 +32,7 @@ import org.dspace.core.Constants;
 import org.dspace.core.Context;
 import org.dspace.eperson.EPerson;
 import org.dspace.eperson.Group;
+import org.dspace.eperson.PasswordHash;
 
 import org.jdom.Namespace;
 
@@ -70,6 +71,8 @@ public class RoleDisseminator implements PackageDisseminator
     public static final String LAST_NAME = "LastName";
     public static final String LANGUAGE = "Language";
     public static final String PASSWORD_HASH = "PasswordHash";
+    public static final String PASSWORD_DIGEST = "digest";
+    public static final String PASSWORD_SALT = "salt";
     public static final String CAN_LOGIN = "CanLogin";
     public static final String REQUIRE_CERTIFICATE = "RequireCertificate";
     public static final String SELF_REGISTERED = "SelfRegistered";
@@ -475,8 +478,19 @@ public class RoleDisseminator implements PackageDisseminator
         
         if (emitPassword)
         {
+            PasswordHash password = eperson.getPasswordHash();
+
             writer.writeStartElement(PASSWORD_HASH);
-            writer.writeCharacters(eperson.getPasswordHash());
+
+            String algorithm = password.getAlgorithm();
+            if (null != algorithm)
+                writer.writeAttribute(PASSWORD_DIGEST, algorithm);
+
+            String salt = password.getSaltString();
+            if (null != salt)
+                writer.writeAttribute(PASSWORD_SALT, salt);
+
+            writer.writeCharacters(password.getHashString());
             writer.writeEndElement();
         }
 

dspace-api/src/main/java/org/dspace/content/packager/RoleIngester.java

@@ -16,6 +16,7 @@ import java.util.List;
 import javax.xml.parsers.DocumentBuilder;
 import javax.xml.parsers.DocumentBuilderFactory;
 import javax.xml.parsers.ParserConfigurationException;
+import org.apache.commons.codec.DecoderException;
 
 import org.dspace.authorize.AuthorizeException;
 import org.dspace.content.Collection;
@@ -26,11 +27,10 @@ import org.dspace.core.Constants;
 import org.dspace.core.Context;
 import org.dspace.eperson.EPerson;
 import org.dspace.eperson.Group;
+import org.dspace.eperson.PasswordHash;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
-import org.w3c.dom.Document;
+import org.w3c.dom.*;
-import org.w3c.dom.Element;
-import org.w3c.dom.NodeList;
 import org.xml.sax.SAXException;
 
 /**
@@ -171,7 +171,30 @@ public class RoleIngester implements PackageIngester
             data = user.getElementsByTagName(RoleDisseminator.PASSWORD_HASH);
             if (data.getLength() > 0)
             {
-                eperson.setPasswordHash(data.item(0).getTextContent());
+                Node element = data.item(0);
+                NamedNodeMap attributes = element.getAttributes();
+
+                Node algorithm = attributes.getNamedItem(RoleDisseminator.PASSWORD_DIGEST);
+                String algorithmText;
+                if (null != algorithm)
+                    algorithmText = algorithm.getNodeValue();
+                else
+                    algorithmText = null;
+
+                Node salt = attributes.getNamedItem(RoleDisseminator.PASSWORD_SALT);
+                String saltText;
+                if (null != salt)
+                    saltText = salt.getNodeValue();
+                else
+                    saltText = null;
+
+                PasswordHash password;
+                try {
+                    password = new PasswordHash(algorithmText, saltText, element.getTextContent());
+                } catch (DecoderException ex) {
+                    throw new PackageValidationException("Unable to decode hexadecimal password hash or salt", ex);
+                }
+                eperson.setPasswordHash(password);
             }
             else
             {

dspace-api/src/main/java/org/dspace/eperson/EPerson.java

@@ -10,6 +10,7 @@ package org.dspace.eperson;
 import java.sql.SQLException;
 import java.util.ArrayList;
 import java.util.List;
+import org.apache.commons.codec.DecoderException;
 
 import org.apache.log4j.Logger;
 import org.dspace.authorize.AuthorizeException;
@@ -851,43 +852,64 @@ public class EPerson extends DSpaceObject
     }
 
     /**
-     * Set the EPerson's password
+     * Set the EPerson's password.
      * 
      * @param s
-     *            the new email
+     *            the new password.
      */
     public void setPassword(String s)
     {
-        // FIXME: encoding
+        PasswordHash hash = new PasswordHash(s);
-        String encoded = Utils.getMD5(s);
+        myRow.setColumn("password", Utils.toHex(hash.getHash()));
-
+        myRow.setColumn("salt", Utils.toHex(hash.getSalt()));
-        myRow.setColumn("password", encoded);
+        myRow.setColumn("digest_algorithm", hash.getAlgorithm());
         modified = true;
     }
 
     /**
-     * Set the EPerson's password hash
+     * Set the EPerson's password hash.
      * 
-     * @param s
+     * @param password
-     *          hash of the password
+     *          hashed password, or null to set row data to NULL.
      */
-    public void setPasswordHash(String s)
+    public void setPasswordHash(PasswordHash password)
     {
-        myRow.setColumn("password", s);
+        if (null == password)
+        {
+            myRow.setColumnNull("digest_algorithm");
+            myRow.setColumnNull("salt");
+            myRow.setColumnNull("password");
+        }
+        else
+        {
+            myRow.setColumn("digest_algorithm", password.getAlgorithm());
+            myRow.setColumn("salt", password.getSaltString());
+            myRow.setColumn("password", password.getHashString());
+        }
         modified = true;
     }
 
     /**
-     * Return the EPerson's password hash
+     * Return the EPerson's password hash.
+     *
      * @return hash of the password
      */
-    public String getPasswordHash()
+    public PasswordHash getPasswordHash()
     {
-        return myRow.getStringColumn("password");
+        PasswordHash hash = null;
+        try {
+            hash = new PasswordHash(myRow.getStringColumn("digest_algorithm"),
+                    myRow.getStringColumn("salt"),
+                    myRow.getStringColumn("password"));
+        } catch (DecoderException ex) {
+            log.error("Problem decoding stored salt or hash:  " + ex.getMessage());
+        }
+        return hash;
     }
 
     /**
-     * Check EPerson's password
+     * Check EPerson's password.  Side effect:  original unsalted MD5 hashes are
+     * converted using the current algorithm.
      * 
      * @param attempt
      *            the password attempt
@@ -895,9 +917,38 @@ public class EPerson extends DSpaceObject
      */
     public boolean checkPassword(String attempt)
     {
-        String encoded = Utils.getMD5(attempt);
+        PasswordHash myHash;
+        try
+        {
+            myHash = new PasswordHash(
+                    myRow.getStringColumn("digest_algorithm"),
+                    myRow.getStringColumn("salt"),
+                    myRow.getStringColumn("password"));
+        } catch (DecoderException ex)
+        {
+            log.error(ex.getMessage());
+            return false;
+        }
+        boolean answer = myHash.matches(attempt);
 
-        return (encoded.equals(myRow.getStringColumn("password")));
+        // If using the old unsalted hash, and this password is correct, update to a new hash
+        if (answer && (null == myRow.getStringColumn("digest_algorithm")))
+        {
+            log.info("Upgrading password hash for EPerson " + getID());
+            setPassword(attempt);
+            try {
+                myContext.turnOffAuthorisationSystem();
+                update();
+            } catch (SQLException ex) {
+                log.error("Could not update password hash", ex);
+            } catch (AuthorizeException ex) {
+                log.error("Could not update password hash", ex);
+            } finally {
+                myContext.restoreAuthSystemState();
+            }
+        }
+
+        return answer;
     }
 
     /**

dspace-api/src/main/java/org/dspace/eperson/Groomer.java

@@ -0,0 +1,71 @@
+/**
+ * The contents of this file are subject to the license and copyright
+ * detailed in the LICENSE and NOTICE files at the root of the source
+ * tree and available online at
+ *
+ * http://www.dspace.org/license/
+ */
+
+package org.dspace.eperson;
+
+import java.sql.SQLException;
+import org.apache.commons.cli.*;
+import org.dspace.core.Context;
+import org.dspace.storage.rdbms.DatabaseManager;
+import org.dspace.storage.rdbms.TableRow;
+import org.dspace.storage.rdbms.TableRowIterator;
+
+/**
+ * Tools for manipulating EPersons and Groups.
+ *
+ * @author mwood
+ */
+public class Groomer
+{
+    /**
+     * Command line tool for "grooming" the EPerson collection.
+     */
+    static public void main(String[] argv)
+            throws SQLException
+    {
+        final String USAGE = "EPerson -verb [option...]";
+
+        OptionGroup verbs = new OptionGroup();
+        verbs.setRequired(true);
+        verbs.addOption(new Option("h", "help", false, "explain this tool"));
+        verbs.addOption(new Option("u", "unsalted", false, "list accounts with unsalted password hashes"));
+
+        Options options = new Options();
+        options.addOptionGroup(verbs);
+
+        PosixParser parser = new PosixParser();
+        CommandLine command = null;
+        try {
+            command = parser.parse(options, argv);
+        } catch (ParseException ex) {
+            System.err.println(ex.getMessage());
+            if (! (ex instanceof MissingOptionException))
+                new HelpFormatter().printHelp(USAGE, options);
+            System.exit(1);
+        }
+
+         // Help the user
+        if (command.hasOption('h') || command.hasOption('?'))
+        {
+            new HelpFormatter().printHelp(USAGE, options);
+        }
+        // Scan for unsalted hashes
+        else if (command.hasOption('u'))
+        {
+            Context myContext = new Context();
+            final TableRowIterator tri = DatabaseManager.query(myContext,
+                    "SELECT email FROM EPerson WHERE password IS NOT NULL AND digest_algorithm IS NULL");
+            for (TableRow row = tri.next(); tri.hasNext(); row = tri.next())
+                System.out.println(row.getStringColumn("email"));
+            myContext.abort(); // No changes to commit
+        }
+        // Should not happen:  verb option defined but no code!
+        else
+            System.err.println("Unimplemented verb:  " + verbs.getSelected());
+    }
+}

dspace-api/src/main/java/org/dspace/eperson/PasswordHash.java

@@ -0,0 +1,283 @@
+/**
+ * The contents of this file are subject to the license and copyright
+ * detailed in the LICENSE and NOTICE files at the root of the source
+ * tree and available online at
+ *
+ * http://www.dspace.org/license/
+ */
+
+package org.dspace.eperson;
+
+import java.nio.charset.Charset;
+import java.security.MessageDigest;
+import java.security.NoSuchAlgorithmException;
+import java.security.SecureRandom;
+import java.util.Arrays;
+import org.apache.commons.codec.DecoderException;
+import org.apache.commons.codec.binary.Hex;
+import org.dspace.services.ConfigurationService;
+import org.dspace.utils.DSpace;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * For handling digested secrets (such as passwords).
+ * Use {@link PasswordHash(String, byte[], byte[])} to package and manipulate
+ * secrets that have already been hashed, and {@link PasswordHash(String)} for
+ * plaintext secrets.  Compare a plaintext candidate to a hashed secret with
+ * {@link matches(String)}.
+ *
+ * @author mwood
+ */
+public class PasswordHash
+{
+    private static final Logger log = LoggerFactory.getLogger(PasswordHash.class);
+    private static final ConfigurationService config
+            = new DSpace().getConfigurationService();
+    private static final Charset UTF_8 = Charset.forName("UTF-8"); // Should always succeed:  UTF-8 is required
+
+    private static final String DEFAULT_DIGEST_ALGORITHM = "SHA-512"; // XXX magic
+    private static final String ALGORITHM_PROPERTY = "authentication-password.digestAlgorithm";
+    private static final int SALT_BYTES = 128/8; // XXX magic we want 128 bits
+    private static final int HASH_ROUNDS = 1024; // XXX magic 1024 rounds
+    private static final int SEED_BYTES = 64; // XXX magic
+    private static final int RESEED_INTERVAL = 100; // XXX magic
+
+    /** A secure random number generator instance. */
+    private static SecureRandom rng = null;
+
+    /** How many times has the RNG been called without re-seeding? */
+    private static int rngUses;
+
+    private String algorithm;
+    private byte[] salt;
+    private byte[] hash;
+
+    /** Don't allow empty instances. */
+    private PasswordHash() {}
+
+    /**
+     * Construct a hash structure from existing data, just for passing around.
+     *
+     * @param algorithm the digest algorithm used in producing {@code hash}.
+     *          If empty, set to null.  Other methods will treat this as unsalted MD5.
+     *          If you want salted multi-round MD5, specify "MD5".
+     * @param salt the salt hashed with the secret, or null.
+     * @param hash the hashed secret.
+     */
+    public PasswordHash(String algorithm, byte[] salt, byte[] hash)
+    {
+        if ((null != algorithm) && algorithm.isEmpty())
+            this.algorithm = null;
+        else
+            this.algorithm = algorithm;
+
+        this.salt = salt;
+
+        this.hash = hash;
+    }
+
+    /**
+     * Convenience:  like {@link PasswordHash(String, byte[], byte[])} but with
+     *          hexadecimal-encoded {@code String}s.
+     * @param algorithm the digest algorithm used in producing {@code hash}.
+     *          If empty, set to null.  Other methods will treat this as unsalted MD5.
+     *          If you want salted multi-round MD5, specify "MD5".
+     * @param salt hexadecimal digits encoding the bytes of the salt, or null.
+     * @param hash hexadecimal digits encoding the bytes of the hash.
+     * @throws DecoderException if salt or hash is not proper hexadecimal.
+     */
+    public PasswordHash(String algorithm, String salt, String hash)
+            throws DecoderException
+    {
+        if ((null != algorithm) && algorithm.isEmpty())
+            this.algorithm = null;
+        else
+            this.algorithm = algorithm;
+
+        if (null == salt)
+            this.salt = null;
+        else
+            this.salt = Hex.decodeHex(salt.toCharArray());
+
+        if (null == hash)
+            this.hash = null;
+        else
+            this.hash = Hex.decodeHex(hash.toCharArray());
+    }
+
+    /**
+     * Construct a hash structure from a cleartext password using the configured
+     * digest algorithm.
+     *
+     * @param password the secret to be hashed.
+     */
+    public PasswordHash(String password)
+    {
+        // Generate some salt
+        salt = generateSalt();
+
+        // What digest algorithm to use?
+        algorithm = config.getPropertyAsType(ALGORITHM_PROPERTY, DEFAULT_DIGEST_ALGORITHM);
+
+        // Hash it!
+        try {
+            hash = digest(salt, algorithm, password);
+        } catch (NoSuchAlgorithmException e) {
+            log.error(e.getMessage());
+            hash = new byte[] { 0 };
+        }
+    }
+
+    /**
+     * Is this the string whose hash I hold?
+     *
+     * @param secret string to be hashed and compared to this hash.
+     * @return true if secret hashes to the value held by this instance.
+     */
+    public boolean matches(String secret)
+    {
+        byte[] candidate;
+        try {
+            candidate = digest(salt, algorithm, secret);
+        } catch (NoSuchAlgorithmException e) {
+            log.error(e.getMessage());
+            return false;
+        }
+        return Arrays.equals(candidate, hash);
+    }
+
+    /**
+     * Get the hash.
+     *
+     * @return the value of hash
+     */
+    public byte[] getHash()
+    {
+        return hash;
+    }
+
+    /**
+     * Get the hash, as a String.
+     *
+     * @return hash encoded as hexadecimal digits, or null if none.
+     */
+    public String getHashString()
+    {
+        if (null != hash)
+            return new String(Hex.encodeHex(hash));
+        else
+            return null;
+    }
+
+    /**
+     * Get the salt.
+     *
+     * @return the value of salt
+     */
+    public byte[] getSalt()
+    {
+        return salt;
+    }
+
+    /**
+     * Get the salt, as a String.
+     *
+     * @return salt encoded as hexadecimal digits, or null if none.
+     */
+    public String getSaltString()
+    {
+        if (null != salt)
+            return new String(Hex.encodeHex(salt));
+        else
+            return null;
+    }
+
+    /**
+     * Get the value of algorithm
+     *
+     * @return the value of algorithm
+     */
+    public String getAlgorithm()
+    {
+        return algorithm;
+    }
+
+    /**
+     * The digest algorithm used if none is configured.
+     * 
+     * @return name of the default digest.
+     */
+    static public String getDefaultAlgorithm()
+    {
+        return DEFAULT_DIGEST_ALGORITHM;
+    }
+
+    /** Generate an array of random bytes. */
+    private synchronized byte[] generateSalt()
+    {
+        // Initialize a random-number generator
+        if (null == rng)
+        {
+            rng = new SecureRandom();
+            log.info("Initialized a random number stream using {} provided by {}",
+                    rng.getAlgorithm(), rng.getProvider());
+            rngUses = 0;
+        }
+
+        if (rngUses++ > RESEED_INTERVAL)
+        { // re-seed the generator periodically to break up possible patterns
+            log.debug("Re-seeding the RNG");
+            rng.setSeed(rng.generateSeed(SEED_BYTES));
+            rngUses = 0;
+        }
+
+        salt = new byte[SALT_BYTES];
+        rng.nextBytes(salt);
+        return salt;
+    }
+
+    /**
+     * Generate a salted hash of a string using a given algorithm.
+     *
+     * @param salt random bytes to salt the hash.
+     * @param algorithm name of the digest algorithm to use.  Assume unsalted MD5 if null.
+     * @param secret the string to be hashed.  Null is treated as an empty string ("").
+     * @return hash bytes.
+     * @throws NoSuchAlgorithmException if algorithm is unknown.
+     */
+    private byte[] digest(byte[] salt, String algorithm, String secret)
+            throws NoSuchAlgorithmException
+    {
+        MessageDigest digester;
+
+        if (null == secret)
+            secret = "";
+
+        // Special case:  old unsalted one-trip MD5 hash.
+        if (null == algorithm)
+        {
+            digester = MessageDigest.getInstance("MD5");
+            digester.update(secret.getBytes(UTF_8));
+            return digester.digest();
+        }
+
+        // Set up a digest
+        digester =  MessageDigest.getInstance(algorithm);
+
+        // Grind up the salt with the password, yielding a hash
+        if (null != salt)
+            digester.update(salt);
+
+        digester.update(secret.getBytes(UTF_8)); // Round 0
+
+        for (int round = 1; round < HASH_ROUNDS; round++)
+        {
+            byte[] lastRound = digester.digest();
+            digester.reset();
+            digester.update(lastRound);
+        }
+
+        return digester.digest();
+    }
+}

dspace-api/src/test/java/org/dspace/eperson/EPersonTest.java

@@ -0,0 +1,776 @@
+/**
+ * The contents of this file are subject to the license and copyright
+ * detailed in the LICENSE and NOTICE files at the root of the source
+ * tree and available online at
+ *
+ * http://www.dspace.org/license/
+ */
+
+package org.dspace.eperson;
+
+import java.sql.SQLException;
+import java.util.ArrayList;
+import mockit.UsingMocksAndStubs;
+import org.apache.commons.codec.DecoderException;
+import org.dspace.MockConfigurationManager;
+import org.dspace.core.Constants;
+import org.dspace.core.Context;
+import org.dspace.servicemanager.DSpaceKernelImpl;
+import org.dspace.servicemanager.DSpaceKernelInit;
+import org.dspace.services.ConfigurationService;
+import org.dspace.storage.rdbms.MockDatabaseManager;
+import org.dspace.storage.rdbms.TableRow;
+import org.junit.*;
+import static org.junit.Assert.*;
+
+/**
+ *
+ * @author mwood
+ */
+@UsingMocksAndStubs(value={MockDatabaseManager.class, MockConfigurationManager.class})
+public class EPersonTest
+{
+    private static TableRow row1;
+
+    private static DSpaceKernelImpl kernel;
+
+    private static ConfigurationService config;
+
+    public EPersonTest()
+    {
+    }
+
+    @BeforeClass
+    public static void setUpClass()
+            throws Exception
+    {
+        // Build a TableRow for an EPerson to wrap
+        final ArrayList<String> epersonColumns = new ArrayList<String>();
+        epersonColumns.add("eperson_id");
+        epersonColumns.add("password");
+        epersonColumns.add("salt");
+        epersonColumns.add("digest_algorithm");
+
+        row1 = new TableRow("EPerson", epersonColumns);
+
+        // Make certain that a default DSpaceKernel is started.
+        kernel = DSpaceKernelInit.getKernel(null);
+        kernel.start();
+
+        // Configure the kernel
+        config = kernel.getConfigurationService();
+        config.setProperty("db.name", "H2");
+        config.setProperty("db.driver", "org.h2.Driver");
+    }
+
+    @AfterClass
+    public static void tearDownClass()
+            throws Exception
+    {
+    }
+    
+    @Before
+    public void setUp()
+    {
+    }
+    
+    @After
+    public void tearDown()
+    {
+    }
+
+    /**
+     * Test of equals method, of class EPerson.
+     */
+/*
+    @Test
+    public void testEquals()
+    {
+        System.out.println("equals");
+        Object obj = null;
+        EPerson instance = null;
+        boolean expResult = false;
+        boolean result = instance.equals(obj);
+        assertEquals(expResult, result);
+        // TODO review the generated test code and remove the default call to fail.
+        fail("The test case is a prototype.");
+    }
+*/
+
+    /**
+     * Test of hashCode method, of class EPerson.
+     */
+/*
+    @Test
+    public void testHashCode()
+    {
+        System.out.println("hashCode");
+        EPerson instance = null;
+        int expResult = 0;
+        int result = instance.hashCode();
+        assertEquals(expResult, result);
+        // TODO review the generated test code and remove the default call to fail.
+        fail("The test case is a prototype.");
+    }
+*/
+
+    /**
+     * Test of find method, of class EPerson.
+     */
+/*
+    @Test
+    public void testFind()
+            throws Exception
+    {
+        System.out.println("find");
+        Context context = null;
+        int id = 0;
+        EPerson expResult = null;
+        EPerson result = EPerson.find(context, id);
+        assertEquals(expResult, result);
+        // TODO review the generated test code and remove the default call to fail.
+        fail("The test case is a prototype.");
+    }
+*/
+
+    /**
+     * Test of findByEmail method, of class EPerson.
+     */
+/*
+    @Test
+    public void testFindByEmail()
+            throws Exception
+    {
+        System.out.println("findByEmail");
+        Context context = null;
+        String email = "";
+        EPerson expResult = null;
+        EPerson result = EPerson.findByEmail(context, email);
+        assertEquals(expResult, result);
+        // TODO review the generated test code and remove the default call to fail.
+        fail("The test case is a prototype.");
+    }
+*/
+
+    /**
+     * Test of findByNetid method, of class EPerson.
+     */
+/*
+    @Test
+    public void testFindByNetid()
+            throws Exception
+    {
+        System.out.println("findByNetid");
+        Context context = null;
+        String netid = "";
+        EPerson expResult = null;
+        EPerson result = EPerson.findByNetid(context, netid);
+        assertEquals(expResult, result);
+        // TODO review the generated test code and remove the default call to fail.
+        fail("The test case is a prototype.");
+    }
+*/
+
+    /**
+     * Test of search method, of class EPerson.
+     */
+/*
+    @Test
+    public void testSearch_Context_String()
+            throws Exception
+    {
+        System.out.println("search");
+        Context context = null;
+        String query = "";
+        EPerson[] expResult = null;
+        EPerson[] result = EPerson.search(context, query);
+        assertEquals(expResult, result);
+        // TODO review the generated test code and remove the default call to fail.
+        fail("The test case is a prototype.");
+    }
+*/
+
+    /**
+     * Test of search method, of class EPerson.
+     */
+/*
+    @Test
+    public void testSearch_4args()
+            throws Exception
+    {
+        System.out.println("search");
+        Context context = null;
+        String query = "";
+        int offset = 0;
+        int limit = 0;
+        EPerson[] expResult = null;
+        EPerson[] result = EPerson.search(context, query, offset, limit);
+        assertEquals(expResult, result);
+        // TODO review the generated test code and remove the default call to fail.
+        fail("The test case is a prototype.");
+    }
+*/
+
+    /**
+     * Test of searchResultCount method, of class EPerson.
+     */
+/*
+    @Test
+    public void testSearchResultCount()
+            throws Exception
+    {
+        System.out.println("searchResultCount");
+        Context context = null;
+        String query = "";
+        int expResult = 0;
+        int result = EPerson.searchResultCount(context, query);
+        assertEquals(expResult, result);
+        // TODO review the generated test code and remove the default call to fail.
+        fail("The test case is a prototype.");
+    }
+*/
+
+    /**
+     * Test of findAll method, of class EPerson.
+     */
+/*
+    @Test
+    public void testFindAll()
+            throws Exception
+    {
+        System.out.println("findAll");
+        Context context = null;
+        int sortField = 0;
+        EPerson[] expResult = null;
+        EPerson[] result = EPerson.findAll(context, sortField);
+        assertEquals(expResult, result);
+        // TODO review the generated test code and remove the default call to fail.
+        fail("The test case is a prototype.");
+    }
+*/
+
+    /**
+     * Test of create method, of class EPerson.
+     */
+/*
+    @Test
+    public void testCreate()
+            throws Exception
+    {
+        System.out.println("create");
+        Context context = null;
+        EPerson expResult = null;
+        EPerson result = EPerson.create(context);
+        assertEquals(expResult, result);
+        // TODO review the generated test code and remove the default call to fail.
+        fail("The test case is a prototype.");
+    }
+*/
+
+    /**
+     * Test of delete method, of class EPerson.
+     */
+/*
+    @Test
+    public void testDelete()
+            throws Exception
+    {
+        System.out.println("delete");
+        EPerson instance = null;
+        instance.delete();
+        // TODO review the generated test code and remove the default call to fail.
+        fail("The test case is a prototype.");
+    }
+*/
+
+    /**
+     * Test of getID method, of class EPerson.
+     */
+/*
+    @Test
+    public void testGetID()
+    {
+        System.out.println("getID");
+        EPerson instance = null;
+        int expResult = 0;
+        int result = instance.getID();
+        assertEquals(expResult, result);
+        // TODO review the generated test code and remove the default call to fail.
+        fail("The test case is a prototype.");
+    }
+*/
+
+    /**
+     * Test of getLanguage method, of class EPerson.
+     */
+/*
+    @Test
+    public void testGetLanguage()
+    {
+        System.out.println("getLanguage");
+        EPerson instance = null;
+        String expResult = "";
+        String result = instance.getLanguage();
+        assertEquals(expResult, result);
+        // TODO review the generated test code and remove the default call to fail.
+        fail("The test case is a prototype.");
+    }
+*/
+
+    /**
+     * Test of setLanguage method, of class EPerson.
+     */
+/*
+    @Test
+    public void testSetLanguage()
+    {
+        System.out.println("setLanguage");
+        String language = "";
+        EPerson instance = null;
+        instance.setLanguage(language);
+        // TODO review the generated test code and remove the default call to fail.
+        fail("The test case is a prototype.");
+    }
+*/
+
+    /**
+     * Test of getHandle method, of class EPerson.
+     */
+/*
+    @Test
+    public void testGetHandle()
+    {
+        System.out.println("getHandle");
+        EPerson instance = null;
+        String expResult = "";
+        String result = instance.getHandle();
+        assertEquals(expResult, result);
+        // TODO review the generated test code and remove the default call to fail.
+        fail("The test case is a prototype.");
+    }
+*/
+
+    /**
+     * Test of getEmail method, of class EPerson.
+     */
+/*
+    @Test
+    public void testGetEmail()
+    {
+        System.out.println("getEmail");
+        EPerson instance = null;
+        String expResult = "";
+        String result = instance.getEmail();
+        assertEquals(expResult, result);
+        // TODO review the generated test code and remove the default call to fail.
+        fail("The test case is a prototype.");
+    }
+*/
+
+    /**
+     * Test of setEmail method, of class EPerson.
+     */
+/*
+    @Test
+    public void testSetEmail()
+    {
+        System.out.println("setEmail");
+        String s = "";
+        EPerson instance = null;
+        instance.setEmail(s);
+        // TODO review the generated test code and remove the default call to fail.
+        fail("The test case is a prototype.");
+    }
+*/
+
+    /**
+     * Test of getNetid method, of class EPerson.
+     */
+/*
+    @Test
+    public void testGetNetid()
+    {
+        System.out.println("getNetid");
+        EPerson instance = null;
+        String expResult = "";
+        String result = instance.getNetid();
+        assertEquals(expResult, result);
+        // TODO review the generated test code and remove the default call to fail.
+        fail("The test case is a prototype.");
+    }
+*/
+
+    /**
+     * Test of setNetid method, of class EPerson.
+     */
+/*
+    @Test
+    public void testSetNetid()
+    {
+        System.out.println("setNetid");
+        String s = "";
+        EPerson instance = null;
+        instance.setNetid(s);
+        // TODO review the generated test code and remove the default call to fail.
+        fail("The test case is a prototype.");
+    }
+*/
+
+    /**
+     * Test of getFullName method, of class EPerson.
+     */
+/*
+    @Test
+    public void testGetFullName()
+    {
+        System.out.println("getFullName");
+        EPerson instance = null;
+        String expResult = "";
+        String result = instance.getFullName();
+        assertEquals(expResult, result);
+        // TODO review the generated test code and remove the default call to fail.
+        fail("The test case is a prototype.");
+    }
+*/
+
+    /**
+     * Test of getFirstName method, of class EPerson.
+     */
+/*
+    @Test
+    public void testGetFirstName()
+    {
+        System.out.println("getFirstName");
+        EPerson instance = null;
+        String expResult = "";
+        String result = instance.getFirstName();
+        assertEquals(expResult, result);
+        // TODO review the generated test code and remove the default call to fail.
+        fail("The test case is a prototype.");
+    }
+*/
+
+    /**
+     * Test of setFirstName method, of class EPerson.
+     */
+/*
+    @Test
+    public void testSetFirstName()
+    {
+        System.out.println("setFirstName");
+        String firstname = "";
+        EPerson instance = null;
+        instance.setFirstName(firstname);
+        // TODO review the generated test code and remove the default call to fail.
+        fail("The test case is a prototype.");
+    }
+*/
+
+    /**
+     * Test of getLastName method, of class EPerson.
+     */
+/*
+    @Test
+    public void testGetLastName()
+    {
+        System.out.println("getLastName");
+        EPerson instance = null;
+        String expResult = "";
+        String result = instance.getLastName();
+        assertEquals(expResult, result);
+        // TODO review the generated test code and remove the default call to fail.
+        fail("The test case is a prototype.");
+    }
+*/
+
+    /**
+     * Test of setLastName method, of class EPerson.
+     */
+/*
+    @Test
+    public void testSetLastName()
+    {
+        System.out.println("setLastName");
+        String lastname = "";
+        EPerson instance = null;
+        instance.setLastName(lastname);
+        // TODO review the generated test code and remove the default call to fail.
+        fail("The test case is a prototype.");
+    }
+*/
+
+    /**
+     * Test of setCanLogIn method, of class EPerson.
+     */
+/*
+    @Test
+    public void testSetCanLogIn()
+    {
+        System.out.println("setCanLogIn");
+        boolean login = false;
+        EPerson instance = null;
+        instance.setCanLogIn(login);
+        // TODO review the generated test code and remove the default call to fail.
+        fail("The test case is a prototype.");
+    }
+*/
+
+    /**
+     * Test of canLogIn method, of class EPerson.
+     */
+/*
+    @Test
+    public void testCanLogIn()
+    {
+        System.out.println("canLogIn");
+        EPerson instance = null;
+        boolean expResult = false;
+        boolean result = instance.canLogIn();
+        assertEquals(expResult, result);
+        // TODO review the generated test code and remove the default call to fail.
+        fail("The test case is a prototype.");
+    }
+*/
+
+    /**
+     * Test of setRequireCertificate method, of class EPerson.
+     */
+/*
+    @Test
+    public void testSetRequireCertificate()
+    {
+        System.out.println("setRequireCertificate");
+        boolean isrequired = false;
+        EPerson instance = null;
+        instance.setRequireCertificate(isrequired);
+        // TODO review the generated test code and remove the default call to fail.
+        fail("The test case is a prototype.");
+    }
+*/
+
+    /**
+     * Test of getRequireCertificate method, of class EPerson.
+     */
+/*
+    @Test
+    public void testGetRequireCertificate()
+    {
+        System.out.println("getRequireCertificate");
+        EPerson instance = null;
+        boolean expResult = false;
+        boolean result = instance.getRequireCertificate();
+        assertEquals(expResult, result);
+        // TODO review the generated test code and remove the default call to fail.
+        fail("The test case is a prototype.");
+    }
+*/
+
+    /**
+     * Test of setSelfRegistered method, of class EPerson.
+     */
+/*
+    @Test
+    public void testSetSelfRegistered()
+    {
+        System.out.println("setSelfRegistered");
+        boolean sr = false;
+        EPerson instance = null;
+        instance.setSelfRegistered(sr);
+        // TODO review the generated test code and remove the default call to fail.
+        fail("The test case is a prototype.");
+    }
+*/
+
+    /**
+     * Test of getSelfRegistered method, of class EPerson.
+     */
+/*
+    @Test
+    public void testGetSelfRegistered()
+    {
+        System.out.println("getSelfRegistered");
+        EPerson instance = null;
+        boolean expResult = false;
+        boolean result = instance.getSelfRegistered();
+        assertEquals(expResult, result);
+        // TODO review the generated test code and remove the default call to fail.
+        fail("The test case is a prototype.");
+    }
+*/
+
+    /**
+     * Test of getMetadata method, of class EPerson.
+     */
+/*
+    @Test
+    public void testGetMetadata()
+    {
+        System.out.println("getMetadata");
+        String field = "";
+        EPerson instance = null;
+        String expResult = "";
+        String result = instance.getMetadata(field);
+        assertEquals(expResult, result);
+        // TODO review the generated test code and remove the default call to fail.
+        fail("The test case is a prototype.");
+    }
+*/
+
+    /**
+     * Test of setMetadata method, of class EPerson.
+     */
+/*
+    @Test
+    public void testSetMetadata()
+    {
+        System.out.println("setMetadata");
+        String field = "";
+        String value = "";
+        EPerson instance = null;
+        instance.setMetadata(field, value);
+        // TODO review the generated test code and remove the default call to fail.
+        fail("The test case is a prototype.");
+    }
+*/
+
+    /**
+     * Test of setPassword method, of class EPerson.
+     */
+/*
+    @Test
+    public void testSetPassword()
+    {
+        System.out.println("setPassword");
+        String s = "";
+        EPerson instance = null;
+        instance.setPassword(s);
+        // TODO review the generated test code and remove the default call to fail.
+        fail("The test case is a prototype.");
+    }
+*/
+
+    /**
+     * Test of setPasswordHash method, of class EPerson.
+     */
+/*
+    @Test
+    public void testSetPasswordHash()
+    {
+        System.out.println("setPasswordHash");
+        PasswordHash password = null;
+        EPerson instance = null;
+        instance.setPasswordHash(password);
+        // TODO review the generated test code and remove the default call to fail.
+        fail("The test case is a prototype.");
+    }
+*/
+
+    /**
+     * Test of getPasswordHash method, of class EPerson.
+     */
+/*
+    @Test
+    public void testGetPasswordHash()
+    {
+        System.out.println("getPasswordHash");
+        EPerson instance = null;
+        PasswordHash expResult = null;
+        PasswordHash result = instance.getPasswordHash();
+        assertEquals(expResult, result);
+        // TODO review the generated test code and remove the default call to fail.
+        fail("The test case is a prototype.");
+    }
+*/
+
+    /**
+     * Test of checkPassword method, of class EPerson.
+     */
+    @Test
+    public void testCheckPassword()
+            throws SQLException, DecoderException
+    {
+        System.out.println("checkPassword");
+        final String attempt = "secret";
+        Context ctx = new Context();
+        EPerson instance = new EPerson(ctx, row1);
+
+        // Test old unsalted MD5 hash
+        final String hash = "5ebe2294ecd0e0f08eab7690d2a6ee69"; // MD5("secret");
+        instance.setPasswordHash(new PasswordHash(null, null, hash));
+        boolean result = instance.checkPassword(attempt);
+        assertTrue("check string with matching MD5 hash", result);
+        // It should have converted the password to the new hash
+        assertEquals("should have upgraded algorithm",
+                PasswordHash.getDefaultAlgorithm(),
+                instance.getPasswordHash().getAlgorithm());
+        assertTrue("upgraded hash should still match",
+                instance.checkPassword(attempt));
+
+        // TODO test a salted multiround hash
+    }
+
+    /**
+     * Test of update method, of class EPerson.
+     */
+/*
+    @Test
+    public void testUpdate()
+            throws Exception
+    {
+        System.out.println("update");
+        EPerson instance = null;
+        instance.update();
+        // TODO review the generated test code and remove the default call to fail.
+        fail("The test case is a prototype.");
+    }
+*/
+
+    /**
+     * Test of getType method, of class EPerson.
+     */
+    @Test
+    public void testGetType()
+            throws SQLException
+    {
+        System.out.println("getType");
+        EPerson instance = new EPerson(new Context(), row1);
+        int expResult = Constants.EPERSON;
+        int result = instance.getType();
+        assertEquals("Should return Constants.EPERSON", expResult, result);
+    }
+
+    /**
+     * Test of getDeleteConstraints method, of class EPerson.
+     */
+/*
+    @Test
+    public void testGetDeleteConstraints()
+            throws Exception
+    {
+        System.out.println("getDeleteConstraints");
+        EPerson instance = null;
+        List expResult = null;
+        List result = instance.getDeleteConstraints();
+        assertEquals(expResult, result);
+        // TODO review the generated test code and remove the default call to fail.
+        fail("The test case is a prototype.");
+    }
+*/
+
+    /**
+     * Test of getName method, of class EPerson.
+     */
+/*
+    @Test
+    public void testGetName()
+    {
+        System.out.println("getName");
+        EPerson instance = null;
+        String expResult = "";
+        String result = instance.getName();
+        assertEquals(expResult, result);
+        // TODO review the generated test code and remove the default call to fail.
+        fail("The test case is a prototype.");
+    }
+*/
+}

dspace-api/src/test/java/org/dspace/eperson/PasswordHashTest.java

@@ -0,0 +1,158 @@
+/**
+ * The contents of this file are subject to the license and copyright
+ * detailed in the LICENSE and NOTICE files at the root of the source
+ * tree and available online at
+ *
+ * http://www.dspace.org/license/
+ */
+package org.dspace.eperson;
+
+import java.security.MessageDigest;
+import java.security.NoSuchAlgorithmException;
+import org.apache.commons.codec.DecoderException;
+import org.dspace.servicemanager.DSpaceKernelInit;
+import org.junit.*;
+import static org.junit.Assert.*;
+
+/**
+ *
+ * @author mwood
+ */
+public class PasswordHashTest
+{
+    public PasswordHashTest()
+    {
+    }
+
+    @BeforeClass
+    public static void setUpClass()
+            throws Exception
+    {
+        // Make certain that a default DSpaceKernel is started.
+        DSpaceKernelInit.getKernel(null).start();
+    }
+
+    @AfterClass
+    public static void tearDownClass()
+            throws Exception
+    {
+    }
+    
+    @Before
+    public void setUp()
+    {
+    }
+    
+    @After
+    public void tearDown()
+    {
+    }
+
+    /**
+     * Test the constructors.
+     */
+    @Test
+    public void testConstructors()
+            throws DecoderException
+    {
+        PasswordHash h1, h3;
+
+        // Test null inputs, as from NULL database columns (old EPerson using
+        // unsalted hash, for example).
+        h3 = new PasswordHash(null, (byte[])null, (byte[])null);
+        assertNull("Null algorithm", h3.getAlgorithm());
+        assertNull("Null salt", h3.getSalt());
+        assertNull("Null hash", h3.getHash());
+        assertFalse("Match null string?", h3.matches(null));
+        assertFalse("Match non-null string?", h3.matches("not null"));
+
+        // Test 3-argument constructor with null string arguments
+        h3 = new PasswordHash(null, (String)null, (String)null);
+        assertNull("Null algorithm", h3.getAlgorithm());
+        assertNull("Null salt", h3.getSalt());
+        assertNull("Null hash", h3.getHash());
+        assertFalse("Match null string?", h3.matches(null));
+        assertFalse("Match non-null string?", h3.matches("not null"));
+
+        // Test single-argument constructor, which does the hashing.
+        String password = "I've got a secret.";
+        h1 = new PasswordHash(password);
+        assertEquals("SHA-512", h1.getAlgorithm());
+        assertFalse("Match against a different string", h1.matches("random rubbish"));
+        assertTrue("Match against the correct string", h1.matches(password));
+
+        // Test 3-argument constructor with non-null data.
+        h3 = new PasswordHash(h1.getAlgorithm(), h1.getSalt(), h1.getHash());
+        assertTrue("Match a duplicate original made from getter values", h3.matches(password));
+    }
+
+    /**
+     * Test of matches method, of class PasswordHash.
+     */
+    @Test
+    public void testMatches()
+            throws NoSuchAlgorithmException
+    {
+        System.out.println("matches");
+        final String secret = "Clark Kent is Superman";
+
+        // Test old 1-trip MD5 hash
+        MessageDigest digest = MessageDigest.getInstance("MD5");
+        PasswordHash hash = new PasswordHash(null, null, digest.digest(secret.getBytes()));
+        boolean result = hash.matches(secret);
+        assertTrue("Old unsalted 1-trip MD5 hash", result);
+
+        // 3-argument form:  see constructor tests
+    }
+
+    /**
+     * Test of getHash method, of class PasswordHash.
+     */
+    /*
+    @Test
+    public void testGetHash()
+    {
+        System.out.println("getHash");
+        PasswordHash instance = null;
+        byte[] expResult = null;
+        byte[] result = instance.getHash();
+        assertEquals(expResult, result);
+        // TODO review the generated test code and remove the default call to fail.
+        fail("The test case is a prototype.");
+    }
+    */
+
+    /**
+     * Test of getSalt method, of class PasswordHash.
+     */
+    /*
+    @Test
+    public void testGetSalt()
+    {
+        System.out.println("getSalt");
+        PasswordHash instance = null;
+        byte[] expResult = null;
+        byte[] result = instance.getSalt();
+        assertEquals(expResult, result);
+        // TODO review the generated test code and remove the default call to fail.
+        fail("The test case is a prototype.");
+    }
+    */
+
+    /**
+     * Test of getAlgorithm method, of class PasswordHash.
+     */
+    /*
+    @Test
+    public void testGetAlgorithm()
+    {
+        System.out.println("getAlgorithm");
+        PasswordHash instance = null;
+        String expResult = "";
+        String result = instance.getAlgorithm();
+        assertEquals(expResult, result);
+        // TODO review the generated test code and remove the default call to fail.
+        fail("The test case is a prototype.");
+    }
+    */
+}

dspace/config/modules/authentication-password.cfg

@@ -22,4 +22,12 @@
 # using the DSpace password system will automatically become members of
 # this group. This is useful if you want a group made up of all internal
 # authenticated users.
-# login.specialgroup = group-name
+# login.specialgroup = group-name
+
+##### Password hashing algorithm #####
+
+# You may select any digest algorithm available through 
+# java.security.MessageDigest on your system.  At least MD2, MD5, SHA-1, 
+# SHA-256, SHA-384, and SHA-512 should be available, but you may have 
+# installed others.  If not set, SHA-512 will be used.
+# digestAlgorithm = SHA-512

dspace/etc/h2/database_schema.sql

@@ -172,7 +172,9 @@ CREATE TABLE EPerson
 (
   eperson_id          INTEGER PRIMARY KEY,
   email               VARCHAR(64),
-  password            VARCHAR(64),
+  password            VARCHAR(128),
+  salt                VARCHAR(32),
+  digest_algorithm    VARCHAR(16),
   firstname           VARCHAR(64),
   lastname            VARCHAR(64),
   can_log_in          BOOL,

dspace/etc/oracle/database_schema.sql

@@ -126,7 +126,9 @@ CREATE TABLE EPerson
 (
   eperson_id          INTEGER PRIMARY KEY,
   email               VARCHAR2(64) UNIQUE,
-  password            VARCHAR2(64),
+  password            VARCHAR2(128),
+  salt                VARCHAR2(32),
+  digest_algorithm    VARCHAR2(16),
   firstname           VARCHAR2(64),
   lastname            VARCHAR2(64),
   can_log_in          NUMBER(1),

dspace/etc/oracle/database_schema_18-30.sql

@@ -0,0 +1,28 @@
+--
+-- database_schema_18-30.sql
+--
+-- Version: $Revision$
+--
+-- Date:    $Date$
+--
+-- The contents of this file are subject to the license and copyright
+-- detailed in the LICENSE and NOTICE files at the root of the source
+-- tree and available online at
+--
+-- http://www.dspace.org/license/
+--
+
+--
+-- SQL commands to upgrade the database schema of a live DSpace 1.8 or 1.8.x
+-- to the DSpace 3.0 database schema
+--
+-- DUMP YOUR DATABASE FIRST. DUMP YOUR DATABASE FIRST. DUMP YOUR DATABASE FIRST. DUMP YOUR DATABASE FIRST.
+-- DUMP YOUR DATABASE FIRST. DUMP YOUR DATABASE FIRST. DUMP YOUR DATABASE FIRST. DUMP YOUR DATABASE FIRST.
+-- DUMP YOUR DATABASE FIRST. DUMP YOUR DATABASE FIRST. DUMP YOUR DATABASE FIRST. DUMP YOUR DATABASE FIRST.
+--
+-------------------------------------------
+-- New columns and longer hash for salted password hashing DS-861 --
+-------------------------------------------
+ALTER TABLE EPerson ALTER COLUMN password TYPE VARCHAR(128);
+ALTER TABLE EPerson ADD salt VARCHAR(32);
+ALTER TABLE EPerson ADD digest_algorithm VARCHAR(16);

dspace/etc/postgres/database_schema.sql

@@ -163,7 +163,9 @@ CREATE TABLE EPerson
 (
   eperson_id          INTEGER PRIMARY KEY,
   email               VARCHAR(64) UNIQUE,
-  password            VARCHAR(64),
+  password            VARCHAR(128),
+  salt                VARCHAR(32),
+  digest_algorithm    VARCHAR(16),
   firstname           VARCHAR(64),
   lastname            VARCHAR(64),
   can_log_in          BOOL,

dspace/etc/postgres/database_schema_18-30.sql

@@ -0,0 +1,28 @@
+--
+-- database_schema_18-30.sql
+--
+-- Version: $Revision$
+--
+-- Date:    $Date$
+--
+-- The contents of this file are subject to the license and copyright
+-- detailed in the LICENSE and NOTICE files at the root of the source
+-- tree and available online at
+--
+-- http://www.dspace.org/license/
+--
+
+--
+-- SQL commands to upgrade the database schema of a live DSpace 1.8 or 1.8.x
+-- to the DSpace 3.0 database schema
+--
+-- DUMP YOUR DATABASE FIRST. DUMP YOUR DATABASE FIRST. DUMP YOUR DATABASE FIRST. DUMP YOUR DATABASE FIRST.
+-- DUMP YOUR DATABASE FIRST. DUMP YOUR DATABASE FIRST. DUMP YOUR DATABASE FIRST. DUMP YOUR DATABASE FIRST.
+-- DUMP YOUR DATABASE FIRST. DUMP YOUR DATABASE FIRST. DUMP YOUR DATABASE FIRST. DUMP YOUR DATABASE FIRST.
+--
+-------------------------------------------
+-- New columns and longer hash for salted password hashing DS-861 --
+-------------------------------------------
+ALTER TABLE EPerson ALTER COLUMN password TYPE VARCHAR(128);
+ALTER TABLE EPerson ADD salt VARCHAR(32);
+ALTER TABLE EPerson ADD digest_algorithm VARCHAR(16);