diff --git a/dspace-api/src/main/java/org/dspace/content/packager/RoleDisseminator.java b/dspace-api/src/main/java/org/dspace/content/packager/RoleDisseminator.java index 772f298a7d..29169363c7 100644 --- a/dspace-api/src/main/java/org/dspace/content/packager/RoleDisseminator.java +++ b/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(); } diff --git a/dspace-api/src/main/java/org/dspace/content/packager/RoleIngester.java b/dspace-api/src/main/java/org/dspace/content/packager/RoleIngester.java index 0be5710db3..5015433bd0 100644 --- a/dspace-api/src/main/java/org/dspace/content/packager/RoleIngester.java +++ b/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.Element; -import org.w3c.dom.NodeList; +import org.w3c.dom.*; 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 { diff --git a/dspace-api/src/main/java/org/dspace/eperson/EPerson.java b/dspace-api/src/main/java/org/dspace/eperson/EPerson.java index c7cce5c84b..bcd6ae6dcc 100644 --- a/dspace-api/src/main/java/org/dspace/eperson/EPerson.java +++ b/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 - String encoded = Utils.getMD5(s); - - myRow.setColumn("password", encoded); + PasswordHash hash = new PasswordHash(s); + myRow.setColumn("password", Utils.toHex(hash.getHash())); + myRow.setColumn("salt", Utils.toHex(hash.getSalt())); + myRow.setColumn("digest_algorithm", hash.getAlgorithm()); modified = true; } /** - * Set the EPerson's password hash + * Set the EPerson's password hash. * - * @param s - * hash of the password + * @param 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; } /** diff --git a/dspace-api/src/main/java/org/dspace/eperson/Groomer.java b/dspace-api/src/main/java/org/dspace/eperson/Groomer.java new file mode 100644 index 0000000000..435a954fd4 --- /dev/null +++ b/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()); + } +} diff --git a/dspace-api/src/main/java/org/dspace/eperson/PasswordHash.java b/dspace-api/src/main/java/org/dspace/eperson/PasswordHash.java new file mode 100644 index 0000000000..dc9255b817 --- /dev/null +++ b/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(); + } +} diff --git a/dspace-api/src/test/java/org/dspace/eperson/EPersonTest.java b/dspace-api/src/test/java/org/dspace/eperson/EPersonTest.java new file mode 100644 index 0000000000..234cb0f772 --- /dev/null +++ b/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 epersonColumns = new ArrayList(); + 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."); + } +*/ +} diff --git a/dspace-api/src/test/java/org/dspace/eperson/PasswordHashTest.java b/dspace-api/src/test/java/org/dspace/eperson/PasswordHashTest.java new file mode 100644 index 0000000000..1082f51797 --- /dev/null +++ b/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."); + } + */ +} diff --git a/dspace/config/modules/authentication-password.cfg b/dspace/config/modules/authentication-password.cfg index 5b0c1aa00e..db03e09ce4 100644 --- a/dspace/config/modules/authentication-password.cfg +++ b/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 \ No newline at end of file +# 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 diff --git a/dspace/etc/h2/database_schema.sql b/dspace/etc/h2/database_schema.sql index 70babe41ca..7865ba7cc6 100644 --- a/dspace/etc/h2/database_schema.sql +++ b/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, diff --git a/dspace/etc/oracle/database_schema.sql b/dspace/etc/oracle/database_schema.sql index 8efab6b3b7..cb338d2693 100644 --- a/dspace/etc/oracle/database_schema.sql +++ b/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), diff --git a/dspace/etc/oracle/database_schema_18-30.sql b/dspace/etc/oracle/database_schema_18-30.sql new file mode 100644 index 0000000000..ab68041003 --- /dev/null +++ b/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); diff --git a/dspace/etc/postgres/database_schema.sql b/dspace/etc/postgres/database_schema.sql index e88caaab98..cb73d60049 100644 --- a/dspace/etc/postgres/database_schema.sql +++ b/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, diff --git a/dspace/etc/postgres/database_schema_18-30.sql b/dspace/etc/postgres/database_schema_18-30.sql new file mode 100644 index 0000000000..ab68041003 --- /dev/null +++ b/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);