From 2dbdc46337be375940441ac4b41f95f25bbbf21d Mon Sep 17 00:00:00 2001 From: Vijayendra Bhamidipati Date: Tue, 2 Apr 2013 11:07:41 -0700 Subject: [PATCH] CLOUDSTACK-1734: Make SHA1 default password encoding mechanism Description: Making SHA256SALT the default encoding algorithm to encode passwords when creating/updating users. Introducing a new configurable list to allow admins to separately configure the order of preference for encoding and authentication schemes. Since passwords are now sent by clients as clear text, fixing the Plain text authenticator to check against the password passed in rather than its md5 digest. --- .../admin/account/CreateAccountCmd.java | 2 +- .../api/command/admin/user/CreateUserCmd.java | 2 +- .../api/command/admin/user/UpdateUserCmd.java | 2 +- client/tomcatconf/applicationContext.xml.in | 54 +++++++++++++++++++ client/tomcatconf/componentContext.xml.in | 28 ---------- .../tomcatconf/nonossComponentContext.xml.in | 28 ---------- developer/developer-prefill.sql | 2 +- .../server/auth/LDAPUserAuthenticator.java | 5 +- .../server/auth/MD5UserAuthenticator.java | 4 ++ .../auth/PlainTextUserAuthenticator.java | 32 +++-------- .../auth/SHA256SaltedUserAuthenticator.java | 3 ++ .../cloud/server/ManagementServerImpl.java | 15 ++++-- .../com/cloud/user/AccountManagerImpl.java | 15 ++++-- 13 files changed, 99 insertions(+), 93 deletions(-) diff --git a/api/src/org/apache/cloudstack/api/command/admin/account/CreateAccountCmd.java b/api/src/org/apache/cloudstack/api/command/admin/account/CreateAccountCmd.java index 89673ea6123..95d0d07d9ce 100644 --- a/api/src/org/apache/cloudstack/api/command/admin/account/CreateAccountCmd.java +++ b/api/src/org/apache/cloudstack/api/command/admin/account/CreateAccountCmd.java @@ -63,7 +63,7 @@ public class CreateAccountCmd extends BaseCmd { @Parameter(name=ApiConstants.LASTNAME, type=CommandType.STRING, required=true, description="lastname") private String lastName; - @Parameter(name=ApiConstants.PASSWORD, type=CommandType.STRING, required=true, description="Hashed password (Default is MD5). If you wish to use any other hashing algorithm, you would need to write a custom authentication adapter See Docs section.") + @Parameter(name=ApiConstants.PASSWORD, type=CommandType.STRING, required=true, description="Clear text password (Default hashed to SHA256SALT). If you wish to use any other hashing algorithm, you would need to write a custom authentication adapter See Docs section.") private String password; @Parameter(name=ApiConstants.TIMEZONE, type=CommandType.STRING, description="Specifies a timezone for this command. For more information on the timezone parameter, see Time Zone Format.") diff --git a/api/src/org/apache/cloudstack/api/command/admin/user/CreateUserCmd.java b/api/src/org/apache/cloudstack/api/command/admin/user/CreateUserCmd.java index fb29e1a2629..7b3f230d1ec 100644 --- a/api/src/org/apache/cloudstack/api/command/admin/user/CreateUserCmd.java +++ b/api/src/org/apache/cloudstack/api/command/admin/user/CreateUserCmd.java @@ -56,7 +56,7 @@ public class CreateUserCmd extends BaseCmd { @Parameter(name=ApiConstants.LASTNAME, type=CommandType.STRING, required=true, description="lastname") private String lastname; - @Parameter(name=ApiConstants.PASSWORD, type=CommandType.STRING, required=true, description="Hashed password (Default is MD5). If you wish to use any other hashing algorithm, you would need to write a custom authentication adapter See Docs section.") + @Parameter(name=ApiConstants.PASSWORD, type=CommandType.STRING, required=true, description="Clear text password (Default hashed to SHA256SALT). If you wish to use any other hashing algorithm, you would need to write a custom authentication adapter See Docs section.") private String password; @Parameter(name=ApiConstants.TIMEZONE, type=CommandType.STRING, description="Specifies a timezone for this command. For more information on the timezone parameter, see Time Zone Format.") diff --git a/api/src/org/apache/cloudstack/api/command/admin/user/UpdateUserCmd.java b/api/src/org/apache/cloudstack/api/command/admin/user/UpdateUserCmd.java index 1f31662e8ca..5ea2dbdef55 100644 --- a/api/src/org/apache/cloudstack/api/command/admin/user/UpdateUserCmd.java +++ b/api/src/org/apache/cloudstack/api/command/admin/user/UpdateUserCmd.java @@ -59,7 +59,7 @@ public class UpdateUserCmd extends BaseCmd { @Parameter(name=ApiConstants.LASTNAME, type=CommandType.STRING, description="last name") private String lastname; - @Parameter(name=ApiConstants.PASSWORD, type=CommandType.STRING, description="Hashed password (default is MD5). If you wish to use any other hasing algorithm, you would need to write a custom authentication adapter") + @Parameter(name=ApiConstants.PASSWORD, type=CommandType.STRING, description="Clear text password (default hashed to SHA256SALT). If you wish to use any other hasing algorithm, you would need to write a custom authentication adapter") private String password; @Parameter(name=ApiConstants.SECRET_KEY, type=CommandType.STRING, description="The secret key for the user. Must be specified with userApiKey") diff --git a/client/tomcatconf/applicationContext.xml.in b/client/tomcatconf/applicationContext.xml.in index 636eac2b939..d3699b93cfb 100644 --- a/client/tomcatconf/applicationContext.xml.in +++ b/client/tomcatconf/applicationContext.xml.in @@ -379,6 +379,60 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + - - - - - - - - - - - - - - - - - - - - - - - - - - - - diff --git a/client/tomcatconf/nonossComponentContext.xml.in b/client/tomcatconf/nonossComponentContext.xml.in index 0b02eb687c9..11472adf7b8 100644 --- a/client/tomcatconf/nonossComponentContext.xml.in +++ b/client/tomcatconf/nonossComponentContext.xml.in @@ -131,34 +131,6 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - diff --git a/developer/developer-prefill.sql b/developer/developer-prefill.sql index 6300d35df64..e4f90cad6e8 100644 --- a/developer/developer-prefill.sql +++ b/developer/developer-prefill.sql @@ -36,7 +36,7 @@ INSERT INTO `cloud`.`user` (id, uuid, username, password, account_id, firstname, -- Add system user with encrypted password=password INSERT INTO `cloud`.`user` (id, uuid, username, password, account_id, firstname, lastname, email, state, created) VALUES (2, UUID(), 'admin', '5f4dcc3b5aa765d61d8327deb882cf99', - '2', 'Admin', 'User', 'admin@mailprovider.com', 'enabled', NOW()); + '2', 'Admin', 'User', 'admin@mailprovider.com', 'disabled', NOW()); -- Add configurations INSERT INTO `cloud`.`configuration` (category, instance, component, name, value) diff --git a/plugins/user-authenticators/ldap/src/com/cloud/server/auth/LDAPUserAuthenticator.java b/plugins/user-authenticators/ldap/src/com/cloud/server/auth/LDAPUserAuthenticator.java index 61eebe5fc93..d928a5b9e17 100644 --- a/plugins/user-authenticators/ldap/src/com/cloud/server/auth/LDAPUserAuthenticator.java +++ b/plugins/user-authenticators/ldap/src/com/cloud/server/auth/LDAPUserAuthenticator.java @@ -151,7 +151,10 @@ public class LDAPUserAuthenticator extends DefaultUserAuthenticator { @Override public boolean configure(String name, Map params) throws ConfigurationException { - super.configure(name, params); + if (name == null) { + name = "LDAP"; + } + super.configure(name, params); return true; } diff --git a/plugins/user-authenticators/md5/src/com/cloud/server/auth/MD5UserAuthenticator.java b/plugins/user-authenticators/md5/src/com/cloud/server/auth/MD5UserAuthenticator.java index 026125ea0f6..e5b169fc456 100644 --- a/plugins/user-authenticators/md5/src/com/cloud/server/auth/MD5UserAuthenticator.java +++ b/plugins/user-authenticators/md5/src/com/cloud/server/auth/MD5UserAuthenticator.java @@ -59,8 +59,12 @@ public class MD5UserAuthenticator extends DefaultUserAuthenticator { return true; } + @Override public boolean configure(String name, Map params) throws ConfigurationException { + if(name == null) { + name = "MD5"; + } super.configure(name, params); return true; } diff --git a/plugins/user-authenticators/plain-text/src/com/cloud/server/auth/PlainTextUserAuthenticator.java b/plugins/user-authenticators/plain-text/src/com/cloud/server/auth/PlainTextUserAuthenticator.java index 52e7cb3e297..f102275905f 100644 --- a/plugins/user-authenticators/plain-text/src/com/cloud/server/auth/PlainTextUserAuthenticator.java +++ b/plugins/user-authenticators/plain-text/src/com/cloud/server/auth/PlainTextUserAuthenticator.java @@ -28,7 +28,6 @@ import org.apache.log4j.Logger; import com.cloud.user.UserAccount; import com.cloud.user.dao.UserAccountDao; - import com.cloud.utils.exception.CloudRuntimeException; @@ -43,45 +42,26 @@ public class PlainTextUserAuthenticator extends DefaultUserAuthenticator { if (s_logger.isDebugEnabled()) { s_logger.debug("Retrieving user: " + username); } + UserAccount user = _userAccountDao.getUserAccount(username, domainId); if (user == null) { s_logger.debug("Unable to find user with " + username + " in domain " + domainId); return false; } - - MessageDigest md5; - try { - md5 = MessageDigest.getInstance("MD5"); - } catch (NoSuchAlgorithmException e) { - throw new CloudRuntimeException("Error", e); - } - md5.reset(); - BigInteger pwInt = new BigInteger(1, md5.digest(password.getBytes())); - - // make sure our MD5 hash value is 32 digits long... - StringBuffer sb = new StringBuffer(); - String pwStr = pwInt.toString(16); - int padding = 32 - pwStr.length(); - for (int i = 0; i < padding; i++) { - sb.append('0'); - } - sb.append(pwStr); - - - // Will: The MD5Authenticator is now a straight pass-through comparison of the - // the passwords because we will not assume that the password passed in has - // already been MD5 hashed. I am keeping the above code in case this requirement changes - // or people need examples of how to MD5 hash passwords in java. - if (!user.getPassword().equals(sb.toString())) { + if (!user.getPassword().equals(password)) { s_logger.debug("Password does not match"); return false; } return true; } + @Override public boolean configure(String name, Map params) throws ConfigurationException { + if (name == null) { + name = "PLAINTEXT"; + } super.configure(name, params); return true; } diff --git a/plugins/user-authenticators/sha256salted/src/com/cloud/server/auth/SHA256SaltedUserAuthenticator.java b/plugins/user-authenticators/sha256salted/src/com/cloud/server/auth/SHA256SaltedUserAuthenticator.java index 1b29f69794a..da939273ea1 100644 --- a/plugins/user-authenticators/sha256salted/src/com/cloud/server/auth/SHA256SaltedUserAuthenticator.java +++ b/plugins/user-authenticators/sha256salted/src/com/cloud/server/auth/SHA256SaltedUserAuthenticator.java @@ -44,6 +44,9 @@ public class SHA256SaltedUserAuthenticator extends DefaultUserAuthenticator { @Override public boolean configure(String name, Map params) throws ConfigurationException { + if (name == null) { + name = "SHA256SALT"; + } super.configure(name, params); return true; } diff --git a/server/src/com/cloud/server/ManagementServerImpl.java b/server/src/com/cloud/server/ManagementServerImpl.java index d0904e1049c..af77ba5645f 100755 --- a/server/src/com/cloud/server/ManagementServerImpl.java +++ b/server/src/com/cloud/server/ManagementServerImpl.java @@ -457,7 +457,8 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe private Map _availableIdsMap; - List _userAuthenticators; + private List _userAuthenticators; + private List _userPasswordEncoders; @Inject ClusterManager _clusterMgr; private String _hashKey = null; @@ -473,7 +474,15 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe public void setUserAuthenticators(List authenticators) { _userAuthenticators = authenticators; } - + + public List getUserPasswordEncoders() { + return _userPasswordEncoders; + } + + public void setUserPasswordEncoders(List encoders) { + _userPasswordEncoders = encoders; + } + public List getHostAllocators() { return _hostAllocators; } @@ -3342,7 +3351,7 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe // This means its a new account, set the password using the // authenticator - for (UserAuthenticator authenticator: _userAuthenticators) { + for (UserAuthenticator authenticator: _userPasswordEncoders) { encodedPassword = authenticator.encode(password); if (encodedPassword != null) { break; diff --git a/server/src/com/cloud/user/AccountManagerImpl.java b/server/src/com/cloud/user/AccountManagerImpl.java index 40db4ed2f86..52ca79d5a60 100755 --- a/server/src/com/cloud/user/AccountManagerImpl.java +++ b/server/src/com/cloud/user/AccountManagerImpl.java @@ -222,6 +222,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M @Inject VolumeManager volumeMgr; private List _userAuthenticators; + List _userPasswordEncoders; private final ScheduledExecutorService _executor = Executors.newScheduledThreadPool(1, new NamedThreadFactory("AccountChecker")); @@ -241,7 +242,15 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M public void setUserAuthenticators(List authenticators) { _userAuthenticators = authenticators; } - + + public List getUserPasswordEncoders() { + return _userPasswordEncoders; + } + + public void setUserPasswordEncoders(List encoders) { + _userPasswordEncoders = encoders; + } + public List getSecurityCheckers() { return _securityCheckers; } @@ -947,7 +956,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M if (password != null) { String encodedPassword = null; - for (Iterator en = _userAuthenticators.iterator(); en.hasNext();) { + for (Iterator en = _userPasswordEncoders.iterator(); en.hasNext();) { UserAuthenticator authenticator = en.next(); encodedPassword = authenticator.encode(password); if (encodedPassword != null) { @@ -1733,7 +1742,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M } String encodedPassword = null; - for (UserAuthenticator authenticator : _userAuthenticators) { + for (UserAuthenticator authenticator : _userPasswordEncoders) { encodedPassword = authenticator.encode(password); if (encodedPassword != null) { break;