From f345bc8daa856bd456bf8127fbcdbf3547a23a71 Mon Sep 17 00:00:00 2001 From: Richard Millet Date: Fri, 16 Aug 2019 15:31:51 -0700 Subject: [PATCH] CC-740: Adding support for salting user passwords. --- .../WEB-INF/applicationContext-security.xml | 6 ++ services/account/pstore/pom.xml | 10 +-- .../storage/csidp/TokenStorageClient.java | 2 +- .../storage/csidp/UserStorageClient.java | 12 ++-- .../authentication_identity_provider.xsd | 19 ++++- services/authentication/pstore/build.xml | 2 +- services/authentication/pstore/pom.xml | 5 +- .../db/postgresql/authentication.sql | 2 + .../test/resources/META-INF/persistence.xml | 1 + services/authentication/service/pom.xml | 5 ++ .../authentication/CSpaceSaltSource.java | 13 ++++ .../authentication/CSpaceUser.java | 14 +++- .../authentication/realm/CSpaceRealm.java | 8 +++ .../realm/db/CSpaceDbRealm.java | 69 +++++++++++++++++++ .../spring/CSpaceUserDetailsService.java | 7 +- services/authorization/pstore/build.xml | 2 +- services/authorization/pstore/pom.xml | 5 +- .../services/authorization/AuthZ.java | 2 +- .../AuthorizationCommon.java | 16 +++-- .../common/security/SecurityUtils.java | 53 +++++++++++--- 20 files changed, 211 insertions(+), 42 deletions(-) create mode 100644 services/authentication/service/src/main/java/org/collectionspace/authentication/CSpaceSaltSource.java diff --git a/services/JaxRsServiceProvider/src/main/webapp/WEB-INF/applicationContext-security.xml b/services/JaxRsServiceProvider/src/main/webapp/WEB-INF/applicationContext-security.xml index bab5d159a..a12b8e497 100644 --- a/services/JaxRsServiceProvider/src/main/webapp/WEB-INF/applicationContext-security.xml +++ b/services/JaxRsServiceProvider/src/main/webapp/WEB-INF/applicationContext-security.xml @@ -90,6 +90,7 @@ + @@ -97,6 +98,10 @@ + + + + @@ -105,6 +110,7 @@ + diff --git a/services/account/pstore/pom.xml b/services/account/pstore/pom.xml index b54900e0f..6e8654a39 100644 --- a/services/account/pstore/pom.xml +++ b/services/account/pstore/pom.xml @@ -26,13 +26,10 @@ property-listener-injector - - mysql - mysql-connector-java - org.postgresql postgresql + ${postgres.driver.version} @@ -120,13 +117,10 @@ - - mysql - mysql-connector-java - org.postgresql postgresql + ${postgres.driver.version} diff --git a/services/account/service/src/main/java/org/collectionspace/services/account/storage/csidp/TokenStorageClient.java b/services/account/service/src/main/java/org/collectionspace/services/account/storage/csidp/TokenStorageClient.java index 747ee40ef..7a69d118a 100644 --- a/services/account/service/src/main/java/org/collectionspace/services/account/storage/csidp/TokenStorageClient.java +++ b/services/account/service/src/main/java/org/collectionspace/services/account/storage/csidp/TokenStorageClient.java @@ -186,7 +186,7 @@ public class TokenStorageClient { throw new BadRequestException(e.getMessage()); } String secEncPasswd = SecurityUtils.createPasswordHash( - userId, new String(password)); + userId, new String(password), null); return secEncPasswd; } } diff --git a/services/account/service/src/main/java/org/collectionspace/services/account/storage/csidp/UserStorageClient.java b/services/account/service/src/main/java/org/collectionspace/services/account/storage/csidp/UserStorageClient.java index b274c21bc..a0b86daa6 100644 --- a/services/account/service/src/main/java/org/collectionspace/services/account/storage/csidp/UserStorageClient.java +++ b/services/account/service/src/main/java/org/collectionspace/services/account/storage/csidp/UserStorageClient.java @@ -28,6 +28,8 @@ package org.collectionspace.services.account.storage.csidp; import java.util.Date; +import java.util.UUID; + import javax.persistence.Query; import org.collectionspace.services.authentication.User; @@ -61,7 +63,9 @@ public class UserStorageClient { public User create(String userId, byte[] password) throws Exception { User user = new User(); user.setUsername(userId); - user.setPasswd(getEncPassword(userId, password)); + String salt = UUID.randomUUID().toString(); + user.setPasswd(getEncPassword(userId, password, salt)); + user.setSalt(salt); user.setCreatedAtItem(new Date()); return user; } @@ -111,7 +115,7 @@ public class UserStorageClient { throws DocumentNotFoundException, Exception { User userFound = get(jpaTransactionContext, userId); if (userFound != null) { - userFound.setPasswd(getEncPassword(userId, password)); + userFound.setPasswd(getEncPassword(userId, password, userFound.getSalt())); userFound.setUpdatedAtItem(new Date()); if (logger.isDebugEnabled()) { logger.debug("updated user=" + JaxbUtils.toString(userFound, User.class)); @@ -144,7 +148,7 @@ public class UserStorageClient { } } - private String getEncPassword(String userId, byte[] password) throws BadRequestException { + private String getEncPassword(String userId, byte[] password, String salt) throws BadRequestException { //jaxb unmarshaller already unmarshal xs:base64Binary, no need to b64 decode //byte[] bpass = Base64.decodeBase64(accountReceived.getPassword()); try { @@ -153,7 +157,7 @@ public class UserStorageClient { throw new BadRequestException(e.getMessage()); } String secEncPasswd = SecurityUtils.createPasswordHash( - userId, new String(password)); + userId, new String(password), salt); return secEncPasswd; } } diff --git a/services/authentication/jaxb/src/main/resources/authentication_identity_provider.xsd b/services/authentication/jaxb/src/main/resources/authentication_identity_provider.xsd index 3ead8c192..6971c2715 100644 --- a/services/authentication/jaxb/src/main/resources/authentication_identity_provider.xsd +++ b/services/authentication/jaxb/src/main/resources/authentication_identity_provider.xsd @@ -66,6 +66,15 @@ + + + + + + + + + @@ -84,7 +93,15 @@ - + + + + + + + + + diff --git a/services/authentication/pstore/build.xml b/services/authentication/pstore/build.xml index 44c6a13f6..3f472893b 100644 --- a/services/authentication/pstore/build.xml +++ b/services/authentication/pstore/build.xml @@ -111,7 +111,7 @@ - + diff --git a/services/authentication/pstore/pom.xml b/services/authentication/pstore/pom.xml index 5e26ecf5c..6451e5058 100644 --- a/services/authentication/pstore/pom.xml +++ b/services/authentication/pstore/pom.xml @@ -112,13 +112,10 @@ - - mysql - mysql-connector-java - org.postgresql postgresql + ${postgres.driver.version} diff --git a/services/authentication/pstore/src/main/resources/db/postgresql/authentication.sql b/services/authentication/pstore/src/main/resources/db/postgresql/authentication.sql index 4760478b5..a8be95c93 100644 --- a/services/authentication/pstore/src/main/resources/db/postgresql/authentication.sql +++ b/services/authentication/pstore/src/main/resources/db/postgresql/authentication.sql @@ -1,7 +1,9 @@ CREATE TABLE IF NOT EXISTS users ( username VARCHAR(128) NOT NULL PRIMARY KEY, created_at TIMESTAMP NOT NULL, + lastLogin TIMESTAMP, passwd VARCHAR(128) NOT NULL, + salt VARCHAR(128) updated_at TIMESTAMP ); diff --git a/services/authentication/pstore/src/test/resources/META-INF/persistence.xml b/services/authentication/pstore/src/test/resources/META-INF/persistence.xml index 59673d1db..284419804 100644 --- a/services/authentication/pstore/src/test/resources/META-INF/persistence.xml +++ b/services/authentication/pstore/src/test/resources/META-INF/persistence.xml @@ -3,6 +3,7 @@ http://java.sun.com/xml/ns/persistence/orm http://java.sun.com/xml/ns/persistence/orm_1_0.xsd" xmlns="http://java.sun.com/xml/ns/persistence" xmlns:orm="http://java.sun.com/xml/ns/persistence/orm" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"> org.collectionspace.services.authentication.User + org.collectionspace.services.authentication.Token diff --git a/services/authentication/service/pom.xml b/services/authentication/service/pom.xml index efd814e57..2fb839b03 100644 --- a/services/authentication/service/pom.xml +++ b/services/authentication/service/pom.xml @@ -100,6 +100,11 @@ ${spring.security.oauth2.version} provided + + org.postgresql + postgresql + provided + diff --git a/services/authentication/service/src/main/java/org/collectionspace/authentication/CSpaceSaltSource.java b/services/authentication/service/src/main/java/org/collectionspace/authentication/CSpaceSaltSource.java new file mode 100644 index 000000000..34c3471df --- /dev/null +++ b/services/authentication/service/src/main/java/org/collectionspace/authentication/CSpaceSaltSource.java @@ -0,0 +1,13 @@ +package org.collectionspace.authentication; + +import org.springframework.security.authentication.dao.ReflectionSaltSource; +import org.springframework.security.core.userdetails.UserDetails; + +public class CSpaceSaltSource extends ReflectionSaltSource { + + @Override + public Object getSalt(UserDetails user) { + return super.getSalt(user); + } + +} diff --git a/services/authentication/service/src/main/java/org/collectionspace/authentication/CSpaceUser.java b/services/authentication/service/src/main/java/org/collectionspace/authentication/CSpaceUser.java index 8cdd002c6..ed0931210 100644 --- a/services/authentication/service/src/main/java/org/collectionspace/authentication/CSpaceUser.java +++ b/services/authentication/service/src/main/java/org/collectionspace/authentication/CSpaceUser.java @@ -20,6 +20,7 @@ public class CSpaceUser extends User { private Set tenants; private CSpaceTenant primaryTenant; + private String salt; /** * Creates a CSpaceUser with the given username, hashed password, associated @@ -30,7 +31,7 @@ public class CSpaceUser extends User { * @param tenants the tenants associated with the user * @param authorities the authorities that have been granted to the user */ - public CSpaceUser(String username, String password, + public CSpaceUser(String username, String password, String salt, Set tenants, Set authorities) { @@ -42,12 +43,13 @@ public class CSpaceUser extends User { authorities); this.tenants = tenants; + this.salt = salt; if (!tenants.isEmpty()) { primaryTenant = tenants.iterator().next(); } } - + /** * Retrieves the tenants associated with the user. * @@ -66,4 +68,12 @@ public class CSpaceUser extends User { return primaryTenant; } + /** + * Returns a "salt" string to use when encrypting a user's password + * @return + */ + public String getSalt() { + return salt != null ? salt : ""; + } + } diff --git a/services/authentication/service/src/main/java/org/collectionspace/authentication/realm/CSpaceRealm.java b/services/authentication/service/src/main/java/org/collectionspace/authentication/realm/CSpaceRealm.java index 0b55b88a0..c70a14ccc 100644 --- a/services/authentication/service/src/main/java/org/collectionspace/authentication/realm/CSpaceRealm.java +++ b/services/authentication/service/src/main/java/org/collectionspace/authentication/realm/CSpaceRealm.java @@ -38,6 +38,14 @@ import org.collectionspace.authentication.CSpaceTenant; * Interface for the CollectionSpace realm. */ public interface CSpaceRealm { + + /** + * Retrieves the "salt" used to encrypt the user's password + * @param username + * @return + * @throws AccountException + */ + public String getSalt(String username) throws AccountException; /** * Retrieves the hashed password used to authenticate a user. diff --git a/services/authentication/service/src/main/java/org/collectionspace/authentication/realm/db/CSpaceDbRealm.java b/services/authentication/service/src/main/java/org/collectionspace/authentication/realm/db/CSpaceDbRealm.java index 2b13bf7eb..620457111 100644 --- a/services/authentication/service/src/main/java/org/collectionspace/authentication/realm/db/CSpaceDbRealm.java +++ b/services/authentication/service/src/main/java/org/collectionspace/authentication/realm/db/CSpaceDbRealm.java @@ -68,6 +68,7 @@ import javax.sql.DataSource; import org.collectionspace.authentication.AuthN; import org.collectionspace.authentication.CSpaceTenant; import org.collectionspace.authentication.realm.CSpaceRealm; +import org.postgresql.util.PSQLState; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -80,6 +81,7 @@ public class CSpaceDbRealm implements CSpaceRealm { private String datasourceName; private String principalsQuery; + private String saltQuery; private String rolesQuery; private String tenantsQueryNoDisabled; private String tenantsQueryWithDisabled; @@ -140,6 +142,10 @@ public class CSpaceDbRealm implements CSpaceRealm { if (tmp != null) { principalsQuery = tmp.toString(); } + tmp = options.get("saltQuery"); + if (tmp != null) { + saltQuery = tmp.toString(); + } tmp = options.get("rolesQuery"); if (tmp != null) { rolesQuery = tmp.toString(); @@ -642,5 +648,68 @@ public class CSpaceDbRealm implements CSpaceRealm { return result; } + + @Override + public String getSalt(String username) throws AccountException { + String salt = null; + Connection conn = null; + PreparedStatement ps = null; + ResultSet rs = null; + try { + conn = getConnection(); + // Get the salt + if (logger.isDebugEnabled()) { + logger.debug("Executing query: " + saltQuery + ", with username: " + username); + } + ps = conn.prepareStatement(saltQuery); + ps.setString(1, username); + rs = ps.executeQuery(); + if (rs.next() == false) { + if (logger.isDebugEnabled()) { + logger.debug(saltQuery + " returned no matches from db"); + } + throw new AccountNotFoundException("No matching username found"); + } + + salt = rs.getString(1); + } catch (SQLException ex) { + // Assuming PostgreSQL + if (PSQLState.UNDEFINED_COLUMN.getState().equals(ex.getSQLState())) { + String msg = "'USERS' table is missing 'salt' column for password encyrption. Assuming existing passwords are unsalted."; + logger.warn(msg); + } else { + AccountException ae = new AccountException("Authentication query failed: " + ex.getLocalizedMessage()); + ae.initCause(ex); + throw ae; + } + } catch (AccountNotFoundException ex) { + throw ex; + } catch (Exception ex) { + AccountException ae = new AccountException("Unknown Exception"); + ae.initCause(ex); + throw ae; + } finally { + if (rs != null) { + try { + rs.close(); + } catch (SQLException e) { + } + } + if (ps != null) { + try { + ps.close(); + } catch (SQLException e) { + } + } + if (conn != null) { + try { + conn.close(); + } catch (SQLException ex) { + } + } + } + + return salt; + } } diff --git a/services/authentication/service/src/main/java/org/collectionspace/authentication/spring/CSpaceUserDetailsService.java b/services/authentication/service/src/main/java/org/collectionspace/authentication/spring/CSpaceUserDetailsService.java index 5085d1590..fba9868ff 100644 --- a/services/authentication/service/src/main/java/org/collectionspace/authentication/spring/CSpaceUserDetailsService.java +++ b/services/authentication/service/src/main/java/org/collectionspace/authentication/spring/CSpaceUserDetailsService.java @@ -74,11 +74,13 @@ public class CSpaceUserDetailsService implements UserDetailsService { @Override public UserDetails loadUserByUsername(String username) throws UsernameNotFoundException { String password = null; + String salt = null; Set tenants = null; Set grantedAuthorities = null; try { password = realm.getPassword(username); + salt = realm.getSalt(username); tenants = getTenants(username); grantedAuthorities = getAuthorities(username); } @@ -89,12 +91,15 @@ public class CSpaceUserDetailsService implements UserDetailsService { throw new AuthenticationServiceException(e.getMessage(), e); } - return + CSpaceUser cspaceUser = new CSpaceUser( username, password, + salt, tenants, grantedAuthorities); + + return cspaceUser; } protected Set getAuthorities(String username) throws AccountException { diff --git a/services/authorization/pstore/build.xml b/services/authorization/pstore/build.xml index 1a599a2a3..ec73e8f9c 100644 --- a/services/authorization/pstore/build.xml +++ b/services/authorization/pstore/build.xml @@ -112,7 +112,7 @@ - + diff --git a/services/authorization/pstore/pom.xml b/services/authorization/pstore/pom.xml index 8557e668c..147e4117f 100644 --- a/services/authorization/pstore/pom.xml +++ b/services/authorization/pstore/pom.xml @@ -107,13 +107,10 @@ - - mysql - mysql-connector-java - org.postgresql postgresql + ${postgres.driver.version} diff --git a/services/authorization/service/src/main/java/org/collectionspace/services/authorization/AuthZ.java b/services/authorization/service/src/main/java/org/collectionspace/services/authorization/AuthZ.java index 016954d53..d37156b86 100644 --- a/services/authorization/service/src/main/java/org/collectionspace/services/authorization/AuthZ.java +++ b/services/authorization/service/src/main/java/org/collectionspace/services/authorization/AuthZ.java @@ -277,7 +277,7 @@ public class AuthZ { HashSet tenantSet = new HashSet(); tenantSet.add(tenant); - CSpaceUser principal = new CSpaceUser(user, password, tenantSet, grantedAuthorities); + CSpaceUser principal = new CSpaceUser(user, password, null, tenantSet, grantedAuthorities); Authentication authRequest = new UsernamePasswordAuthenticationToken(principal, password, grantedAuthorities); SecurityContextHolder.getContext().setAuthentication(authRequest); diff --git a/services/common/src/main/java/org/collectionspace/services/common/authorization_mgt/AuthorizationCommon.java b/services/common/src/main/java/org/collectionspace/services/common/authorization_mgt/AuthorizationCommon.java index 66e3acfd5..ed28f4766 100644 --- a/services/common/src/main/java/org/collectionspace/services/common/authorization_mgt/AuthorizationCommon.java +++ b/services/common/src/main/java/org/collectionspace/services/common/authorization_mgt/AuthorizationCommon.java @@ -142,8 +142,8 @@ public class AuthorizationCommon { final private static String QUERY_USERS_SQL = "SELECT username FROM users WHERE username LIKE '" +TENANT_ADMIN_ACCT_PREFIX+"%' OR username LIKE '"+TENANT_READER_ACCT_PREFIX+"%'"; - final private static String INSERT_USER_SQL = - "INSERT INTO users (username,passwd, created_at) VALUES (?,?, now())"; + final private static String INSERT_USER_SQL = + "INSERT INTO users (username,passwd,salt, created_at) VALUES (?,?,?, now())"; final private static String INSERT_ACCOUNT_SQL = "INSERT INTO accounts_common " + "(csid, email, userid, status, screen_name, metadata_protection, roles_protection, created_at) " @@ -466,10 +466,12 @@ public class AuthorizationCommon { for(String tName : tenantInfo.values()) { String adminAcctName = getDefaultAdminUserID(tName); if(!usersInRepo.contains(adminAcctName)) { + String salt = UUID.randomUUID().toString(); String secEncPasswd = SecurityUtils.createPasswordHash( - adminAcctName, DEFAULT_ADMIN_PASSWORD); + adminAcctName, DEFAULT_ADMIN_PASSWORD, salt); pstmt.setString(1, adminAcctName); // set username param pstmt.setString(2, secEncPasswd); // set passwd param + pstmt.setString(3, salt); if (logger.isDebugEnabled()) { logger.debug("createDefaultUsersAndAccounts adding user: " +adminAcctName+" for tenant: "+tName); @@ -483,10 +485,12 @@ public class AuthorizationCommon { String readerAcctName = getDefaultReaderUserID(tName); if(!usersInRepo.contains(readerAcctName)) { + String salt = UUID.randomUUID().toString(); String secEncPasswd = SecurityUtils.createPasswordHash( - readerAcctName, DEFAULT_READER_PASSWORD); + readerAcctName, DEFAULT_READER_PASSWORD, salt); pstmt.setString(1, readerAcctName); // set username param pstmt.setString(2, secEncPasswd); // set passwd param + pstmt.setString(3, salt); if (logger.isDebugEnabled()) { logger.debug("createDefaultUsersAndAccounts adding user: " +readerAcctName+" for tenant: "+tName); @@ -583,11 +587,13 @@ public class AuthorizationCommon { } rs.close(); if(!foundTMgrUser) { + String salt = UUID.randomUUID().toString(); pstmt = conn.prepareStatement(INSERT_USER_SQL); // create a statement String secEncPasswd = SecurityUtils.createPasswordHash( - TENANT_MANAGER_USER, DEFAULT_TENANT_MANAGER_PASSWORD); + TENANT_MANAGER_USER, DEFAULT_TENANT_MANAGER_PASSWORD, salt); pstmt.setString(1, TENANT_MANAGER_USER); // set username param pstmt.setString(2, secEncPasswd); // set passwd param + pstmt.setString(3, salt); if (logger.isDebugEnabled()) { logger.debug("findOrCreateTenantManagerUserAndAccount adding tenant manager user: " +TENANT_MANAGER_USER); diff --git a/services/common/src/main/java/org/collectionspace/services/common/security/SecurityUtils.java b/services/common/src/main/java/org/collectionspace/services/common/security/SecurityUtils.java index f45dd5a39..37d7c6cd3 100644 --- a/services/common/src/main/java/org/collectionspace/services/common/security/SecurityUtils.java +++ b/services/common/src/main/java/org/collectionspace/services/common/security/SecurityUtils.java @@ -44,11 +44,41 @@ import javax.ws.rs.core.UriInfo; import org.slf4j.Logger; import org.slf4j.LoggerFactory; + +import org.springframework.security.authentication.encoding.BasePasswordEncoder; import org.jboss.crypto.digest.DigestCallback; import org.jboss.resteasy.spi.HttpRequest; import org.jboss.security.Base64Encoder; import org.jboss.security.Base64Utils; +/** + * Extends Spring Security's base class for encoding passwords. We use only the + * mergePasswordAndSalt() method. + * @author remillet + * + */ +class CSpacePasswordEncoder extends BasePasswordEncoder { + public CSpacePasswordEncoder() { + //Do nothing + } + + String mergePasswordAndSalt(String password, String salt) { + return this.mergePasswordAndSalt(password, salt, false); + } + + @Override + public String encodePassword(String rawPass, Object salt) { + // TODO Auto-generated method stub + return null; + } + + @Override + public boolean isPasswordValid(String encPass, String rawPass, Object salt) { + // TODO Auto-generated method stub + return false; + } +} + /** * * @author @@ -65,7 +95,6 @@ public class SecurityUtils { public static final String RFC2617_ENCODING = "RFC2617"; private static char MD5_HEX[] = "0123456789abcdef".toCharArray(); - /** * createPasswordHash creates password has using configured digest algorithm * and encoding @@ -73,13 +102,14 @@ public class SecurityUtils { * @param password in cleartext * @return hashed password */ - public static String createPasswordHash(String username, String password) { + public static String createPasswordHash(String username, String password, String salt) { //TODO: externalize digest algo and encoding return createPasswordHash("SHA-256", //digest algo "base64", //encoding null, //charset username, - password); + password, + salt); } /** @@ -311,26 +341,31 @@ public class SecurityUtils { return result; } - public static String createPasswordHash(String hashAlgorithm, String hashEncoding, String hashCharset, String username, String password) + public static String createPasswordHash(String hashAlgorithm, String hashEncoding, String hashCharset, + String username, String password, String salt) { - return createPasswordHash(hashAlgorithm, hashEncoding, hashCharset, username, password, null); + return createPasswordHash(hashAlgorithm, hashEncoding, hashCharset, username, password, salt, null); } - public static String createPasswordHash(String hashAlgorithm, String hashEncoding, String hashCharset, String username, String password, DigestCallback callback) + public static String createPasswordHash(String hashAlgorithm, String hashEncoding, String hashCharset, + String username, String password, String salt, DigestCallback callback) { + CSpacePasswordEncoder passwordEncoder = new CSpacePasswordEncoder(); + String saltedPassword = passwordEncoder.mergePasswordAndSalt(password, salt); // + String passwordHash = null; byte passBytes[]; try { if(hashCharset == null) - passBytes = password.getBytes(); + passBytes = saltedPassword.getBytes(); else - passBytes = password.getBytes(hashCharset); + passBytes = saltedPassword.getBytes(hashCharset); } catch(UnsupportedEncodingException uee) { logger.error((new StringBuilder()).append("charset ").append(hashCharset).append(" not found. Using platform default.").toString(), uee); - passBytes = password.getBytes(); + passBytes = saltedPassword.getBytes(); } try { -- 2.47.3