ACS-4759 Replace default password hashing algorithm. (#1789)

ACS-4759 Replace default password hashing algorithm with bcrypt10.

Update tests to use sha256 hashing and fix tests that relied on md4 being the default hashing algorithm.

Remove test for default password hashing algorithm since this is being overridden for the tests anyway.
This commit is contained in:
Tom Page
2023-03-15 14:11:25 +00:00
committed by GitHub
parent a96e805d52
commit acc5425d68
3 changed files with 163 additions and 156 deletions

View File

@@ -1149,7 +1149,7 @@ smart.folders.config.type.templates.path=${spaces.dictionary.childname}/${spaces
smart.folders.config.type.templates.qname.filter=none
# Preferred password encoding, md4, sha256, bcrypt10
system.preferred.password.encoding=md4
system.preferred.password.encoding=bcrypt10
# Upgrade Password Hash Job
system.upgradePasswordHash.jobBatchSize=100

View File

@@ -26,15 +26,12 @@
package org.alfresco.repo.security.authentication;
import java.io.Serializable;
import java.io.UnsupportedEncodingException;
import java.security.NoSuchAlgorithmException;
import java.util.Arrays;
import java.util.Date;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Set;
import javax.transaction.Status;
import javax.transaction.UserTransaction;
@@ -48,7 +45,6 @@ import net.sf.acegisecurity.DisabledException;
import net.sf.acegisecurity.LockedException;
import net.sf.acegisecurity.UserDetails;
import net.sf.acegisecurity.providers.UsernamePasswordAuthenticationToken;
import org.alfresco.error.AlfrescoRuntimeException;
import org.alfresco.model.ContentModel;
import org.alfresco.repo.admin.SysAdminParamsImpl;
@@ -519,50 +515,48 @@ public class AuthenticationTest extends TestCase
assertTrue("The user should exist", dao.userExists(userName));
}
public void testCreateAndyUserAndOtherCRUD() throws NoSuchAlgorithmException, UnsupportedEncodingException
public void testCreateAndyUserAndUpdatePassword()
{
RepositoryAuthenticationDao dao = createRepositoryAuthenticationDao();
dao.createUser("Andy", "cabbage".toCharArray());
assertNotNull(dao.getUserOrNull("Andy"));
UserDetails AndyDetails = (UserDetails) dao.loadUserByUsername("Andy");
assertNotNull(AndyDetails);
assertEquals("Andy", AndyDetails.getUsername());
// assertNotNull(dao.getSalt(AndyDetails));
assertTrue(AndyDetails.isAccountNonExpired());
assertTrue(AndyDetails.isAccountNonLocked());
assertTrue(AndyDetails.isCredentialsNonExpired());
assertTrue(AndyDetails.isEnabled());
assertNotSame("cabbage", AndyDetails.getPassword());
assertTrue(compositePasswordEncoder.matches(compositePasswordEncoder.getPreferredEncoding(),"cabbage", AndyDetails.getPassword(), null));
assertEquals(1, AndyDetails.getAuthorities().length);
RepositoryAuthenticatedUser andyDetails = (RepositoryAuthenticatedUser) dao.loadUserByUsername("Andy");
assertNotNull("User unexpectedly null", andyDetails);
assertEquals("Unexpected username", "Andy", andyDetails.getUsername());
Object originalSalt = andyDetails.getSalt();
assertNotNull("Salt was not generated", originalSalt);
assertTrue("Account unexpectedly expired", andyDetails.isAccountNonExpired());
assertTrue("Account unexpectedly locked", andyDetails.isAccountNonLocked());
assertTrue("Credentials unexpectedly expired", andyDetails.isCredentialsNonExpired());
assertTrue("User unexpectedly disabled", andyDetails.isEnabled());
assertNotSame("Password was not hashed", "cabbage", andyDetails.getPassword());
assertTrue("Failed to recalculate same password hash", compositePasswordEncoder.matches(compositePasswordEncoder.getPreferredEncoding(),"cabbage", andyDetails.getPassword(), originalSalt));
assertEquals("User does not have a single authority", 1, andyDetails.getAuthorities().length);
// Object oldSalt = dao.getSalt(AndyDetails);
dao.updateUser("Andy", "carrot".toCharArray());
UserDetails newDetails = (UserDetails) dao.loadUserByUsername("Andy");
assertNotNull(newDetails);
assertEquals("Andy", newDetails.getUsername());
// assertNotNull(dao.getSalt(newDetails));
assertTrue(newDetails.isAccountNonExpired());
assertTrue(newDetails.isAccountNonLocked());
assertTrue(newDetails.isCredentialsNonExpired());
assertTrue(newDetails.isEnabled());
assertNotSame("carrot", newDetails.getPassword());
assertEquals(1, newDetails.getAuthorities().length);
RepositoryAuthenticatedUser newDetails = (RepositoryAuthenticatedUser) dao.loadUserByUsername("Andy");
assertNotNull("New details were null", newDetails);
assertEquals("New details contain wrong username", "Andy", newDetails.getUsername());
Object updatedSalt = newDetails.getSalt();
assertNotNull("New details contain null salt", updatedSalt);
assertTrue("Updated account is expired", newDetails.isAccountNonExpired());
assertTrue("Updated account is locked", newDetails.isAccountNonLocked());
assertTrue("Updated account has expired credentials", newDetails.isCredentialsNonExpired());
assertTrue("Updated account is not enabled", newDetails.isEnabled());
assertNotSame("Updated account contains unhashed password", "carrot", newDetails.getPassword());
assertEquals("Updated account should have a single authority", 1, newDetails.getAuthorities().length);
assertTrue("Failed to validate updated password hash", compositePasswordEncoder.matches(compositePasswordEncoder.getPreferredEncoding(),"carrot", newDetails.getPassword(), updatedSalt));
assertNotSame("Expected salt to be replaced when password was updated", originalSalt, updatedSalt);
assertNotSame(AndyDetails.getPassword(), newDetails.getPassword());
RepositoryAuthenticatedUser rau = (RepositoryAuthenticatedUser) newDetails;
assertTrue(compositePasswordEncoder.matchesPassword("carrot", newDetails.getPassword(), null, rau.getHashIndicator()));
// assertNotSame(oldSalt, dao.getSalt(newDetails));
//Update again
dao.updateUser("Andy", "potato".toCharArray());
newDetails = (UserDetails) dao.loadUserByUsername("Andy");
assertNotNull(newDetails);
assertEquals("Andy", newDetails.getUsername());
rau = (RepositoryAuthenticatedUser) newDetails;
assertTrue(compositePasswordEncoder.matchesPassword("potato", newDetails.getPassword(), null, rau.getHashIndicator()));
// Update back to first password again.
dao.updateUser("Andy", "cabbage".toCharArray());
RepositoryAuthenticatedUser thirdDetails = (RepositoryAuthenticatedUser) dao.loadUserByUsername("Andy");
Object thirdSalt = thirdDetails.getSalt();
assertNotSame("New salt should not match original salt", thirdSalt, originalSalt);
assertNotSame("New salt should not match previous salt", thirdSalt, updatedSalt);
assertTrue("New password hash was not reproducible", compositePasswordEncoder.matches(compositePasswordEncoder.getPreferredEncoding(), "cabbage", thirdDetails.getPassword(), thirdSalt));
dao.deleteUser("Andy");
assertFalse("Should not be a cache entry for 'Andy'.", authenticationCache.contains("Andy"));
@@ -1989,14 +1983,20 @@ public class AuthenticationTest extends TestCase
* Tests the scenario where a user logs in after the system has been upgraded.
* Their password should get re-hashed using the preferred encoding.
*/
public void testRehashedPasswordOnAuthentication() throws Exception
public void testRehashedPasswordOnAuthentication()
{
// This test requires upgrading from md4 to sha256 hashing.
String defaultPreferredEncoding = compositePasswordEncoder.getPreferredEncoding();
compositePasswordEncoder.setPreferredEncoding("md4");
try
{
// create the Andy authentication
assertNull(authenticationComponent.getCurrentAuthentication());
authenticationComponent.setSystemUserAsCurrentUser();
pubAuthenticationService.createAuthentication("Andy", "auth1".toCharArray());
// find the node representing the Andy user and it's properties
// find the node representing the Andy user and its properties
NodeRef andyUserNodeRef = getRepositoryAuthenticationDao().getUserOrNull("Andy");
assertNotNull(andyUserNodeRef);
@@ -2065,7 +2065,7 @@ public class AuthenticationTest extends TestCase
assertNotNull("Expected the hashIndicator property to be populated", hashIndicatorProp);
assertTrue("Expected there to be a single hash indicator entry", (hashIndicatorProp.size() == 1));
String preferredEncoding = compositePasswordEncoder.getPreferredEncoding();
String hashEncoding = (String)hashIndicatorProp.get(0);
String hashEncoding = hashIndicatorProp.get(0);
assertEquals("Expected hash indicator to be '" + preferredEncoding + "' but it was: " + hashEncoding,
preferredEncoding, hashEncoding);
@@ -2073,28 +2073,28 @@ public class AuthenticationTest extends TestCase
this.deleteAndy();
authenticationComponent.clearCurrentSecurityContext();
}
/**
* For on premise the default is MD4, for cloud BCRYPT10
*
* @throws Exception
*/
public void testDefaultEncodingIsMD4() throws Exception
catch (Exception e)
{
assertNotNull(compositePasswordEncoder);
assertEquals("md4", compositePasswordEncoder.getPreferredEncoding());
throw new RuntimeException(e);
}
finally
{
compositePasswordEncoder.setPreferredEncoding(defaultPreferredEncoding);
}
}
/**
* For on premise the default is MD4, get it
*
* @throws Exception
* Test password encoding with MD4 without a salt.
*/
public void testGetsMD4Password() throws Exception
public void testGetsMD4Password()
{
String defaultPreferredEncoding = compositePasswordEncoder.getPreferredEncoding();
compositePasswordEncoder.setPreferredEncoding("md4");
try
{
String user = "mduzer";
String rawPass = "roarPazzw0rd";
assertEquals("md4", compositePasswordEncoder.getPreferredEncoding());
dao.createUser(user, null, rawPass.toCharArray());
NodeRef userNodeRef = getRepositoryAuthenticationDao().getUserOrNull(user);
assertNotNull(userNodeRef);
@@ -2107,7 +2107,7 @@ public class AuthenticationTest extends TestCase
properties.remove(ContentModel.PROP_HASH_INDICATOR);
properties.remove(ContentModel.PROP_PASSWORD);
properties.remove(ContentModel.PROP_PASSWORD_SHA256);
String encoded = compositePasswordEncoder.encode("md4",new String(rawPass), null);
String encoded = compositePasswordEncoder.encodePassword("md4", rawPass, List.of("md4"));
properties.put(ContentModel.PROP_PASSWORD, encoded);
nodeService.setProperties(userNodeRef, properties);
pass = dao.getMD4HashedPassword(user);
@@ -2115,6 +2115,11 @@ public class AuthenticationTest extends TestCase
assertEquals(encoded, pass);
dao.deleteUser(user);
}
finally
{
compositePasswordEncoder.setPreferredEncoding(defaultPreferredEncoding);
}
}
/**
* Tests creating a user with a Hashed password

View File

@@ -66,3 +66,5 @@ encryption.cipherAlgorithm=DESede/CBC/PKCS5Padding
encryption.keystore.type=JCEKS
encryption.keystore.backup.type=JCEKS
# For CI override the default hashing algorithm for password storage to save build time.
system.preferred.password.encoding=sha256