Updated Branches: refs/heads/master 5b0314fff -> dce355103
CLOUDSTACK-2312, CLOUDSTACK-2314 : SHA256 timing attack and brute force attack fix Signed-off-by: Chiradeep Vittal <chirad...@apache.org> Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/dce35510 Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/dce35510 Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/dce35510 Branch: refs/heads/master Commit: dce35510313428dd52cda214258cfdfa40d0ed32 Parents: 5b0314f Author: Amogh Vasekar <amogh.vase...@citrix.com> Authored: Wed Aug 7 12:15:07 2013 -0700 Committer: Chiradeep Vittal <chirad...@apache.org> Committed: Wed Aug 7 12:15:07 2013 -0700 ---------------------------------------------------------------------- .../auth/SHA256SaltedUserAuthenticator.java | 39 +++++++--- .../server/auth/test/AuthenticatorTest.java | 79 +++++++++++++++++--- 2 files changed, 99 insertions(+), 19 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cloudstack/blob/dce35510/plugins/user-authenticators/sha256salted/src/com/cloud/server/auth/SHA256SaltedUserAuthenticator.java ---------------------------------------------------------------------- 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 da93927..91be922 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 @@ -36,10 +36,11 @@ import com.cloud.utils.exception.CloudRuntimeException; @Local(value={UserAuthenticator.class}) public class SHA256SaltedUserAuthenticator extends DefaultUserAuthenticator { public static final Logger s_logger = Logger.getLogger(SHA256SaltedUserAuthenticator.class); - + private static final String s_defaultPassword = "000000000000000000000000000="; + private static final String s_defaultSalt = "0000000000000000000000000000000="; @Inject private UserAccountDao _userAccountDao; - private static int s_saltlen = 20; + private static final int s_saltlen = 32; @Override public boolean configure(String name, Map<String, Object> params) @@ -60,21 +61,29 @@ public class SHA256SaltedUserAuthenticator extends DefaultUserAuthenticator { if (s_logger.isDebugEnabled()) { s_logger.debug("Retrieving user: " + username); } + boolean realUser = true; UserAccount user = _userAccountDao.getUserAccount(username, domainId); if (user == null) { s_logger.debug("Unable to find user with " + username + " in domain " + domainId); - return false; + realUser = false; } - - try { + /* Fake Data */ + String realPassword = new String(s_defaultPassword); + byte[] salt = new String(s_defaultSalt).getBytes(); + if (realUser) { String storedPassword[] = user.getPassword().split(":"); if (storedPassword.length != 2) { s_logger.warn("The stored password for " + username + " isn't in the right format for this authenticator"); - return false; + realUser = false; + } else { + realPassword = storedPassword[1]; + salt = Base64.decode(storedPassword[0]); } - byte salt[] = Base64.decode(storedPassword[0]); + } + try { String hashedPassword = encode(password, salt); - return storedPassword[1].equals(hashedPassword); + /* constantTimeEquals comes first in boolean since we need to thwart timing attacks */ + return constantTimeEquals(realPassword, hashedPassword) && realUser; } catch (NoSuchAlgorithmException e) { throw new CloudRuntimeException("Unable to hash password", e); } catch (UnsupportedEncodingException e) { @@ -109,9 +118,9 @@ public class SHA256SaltedUserAuthenticator extends DefaultUserAuthenticator { public String encode(String password, byte[] salt) throws UnsupportedEncodingException, NoSuchAlgorithmException { byte[] passwordBytes = password.getBytes("UTF-8"); - byte[] hashSource = new byte[passwordBytes.length + s_saltlen]; + byte[] hashSource = new byte[passwordBytes.length + salt.length]; System.arraycopy(passwordBytes, 0, hashSource, 0, passwordBytes.length); - System.arraycopy(salt, 0, hashSource, passwordBytes.length, s_saltlen); + System.arraycopy(salt, 0, hashSource, passwordBytes.length, salt.length); // 2. Hash the password with the salt MessageDigest md = MessageDigest.getInstance("SHA-256"); @@ -120,4 +129,14 @@ public class SHA256SaltedUserAuthenticator extends DefaultUserAuthenticator { return new String(Base64.encode(digest)); } + + private static boolean constantTimeEquals(String a, String b) { + byte[] aBytes = a.getBytes(); + byte[] bBytes = b.getBytes(); + int result = aBytes.length ^ bBytes.length; + for (int i = 0; i < aBytes.length && i < bBytes.length; i++) { + result |= aBytes[i] ^ bBytes[i]; + } + return result == 0; + } } http://git-wip-us.apache.org/repos/asf/cloudstack/blob/dce35510/plugins/user-authenticators/sha256salted/test/src/com/cloud/server/auth/test/AuthenticatorTest.java ---------------------------------------------------------------------- diff --git a/plugins/user-authenticators/sha256salted/test/src/com/cloud/server/auth/test/AuthenticatorTest.java b/plugins/user-authenticators/sha256salted/test/src/com/cloud/server/auth/test/AuthenticatorTest.java index 4e23d14..0d3f883 100644 --- a/plugins/user-authenticators/sha256salted/test/src/com/cloud/server/auth/test/AuthenticatorTest.java +++ b/plugins/user-authenticators/sha256salted/test/src/com/cloud/server/auth/test/AuthenticatorTest.java @@ -17,36 +17,66 @@ package src.com.cloud.server.auth.test; -import static org.junit.Assert.*; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.fail; +import static org.mockito.Mockito.when; import java.io.UnsupportedEncodingException; import java.security.NoSuchAlgorithmException; import java.util.Collections; +import java.util.HashMap; +import java.util.Map; import javax.naming.ConfigurationException; import org.bouncycastle.util.encoders.Base64; +import org.junit.Assert; import org.junit.Before; import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.runners.MockitoJUnitRunner; import com.cloud.server.auth.SHA256SaltedUserAuthenticator; +import com.cloud.user.UserAccount; +import com.cloud.user.dao.UserAccountDao; +@RunWith(MockitoJUnitRunner.class) public class AuthenticatorTest { + @Mock + UserAccount adminAccount; + + @Mock + UserAccount adminAccount20Byte; + + @Mock + UserAccountDao _userAccountDao; + + @InjectMocks + SHA256SaltedUserAuthenticator authenticator; + @Before public void setUp() throws Exception { + try { + authenticator.configure("SHA256", Collections.<String, Object> emptyMap()); + } catch (ConfigurationException e) { + fail(e.toString()); + } + + when(_userAccountDao.getUserAccount("admin", 0L)).thenReturn(adminAccount); + when(_userAccountDao.getUserAccount("admin20Byte", 0L)).thenReturn(adminAccount20Byte); + when(_userAccountDao.getUserAccount("fake", 0L)).thenReturn(null); + //32 byte salt, and password="password" + when(adminAccount.getPassword()).thenReturn("WS3UHhBPKHZeV+G3jnn7G2N3luXgLSfL+2ORDieXa1U=:VhuFOrOU2IpsjKYH8cH1VDaDBh/VivjMcuADjeEbIig="); + //20 byte salt, and password="password" + when(adminAccount20Byte.getPassword()).thenReturn("QL2NsxVEmRuDaNRkvIyADny7C5w=:JoegiytiWnoBAxmSD/PwBZZYqkr746x2KzPrZNw4NgI="); + } @Test public void testEncode() throws UnsupportedEncodingException, NoSuchAlgorithmException { - SHA256SaltedUserAuthenticator authenticator = - new SHA256SaltedUserAuthenticator(); - - try { - authenticator.configure("SHA256", Collections.<String,Object>emptyMap()); - } catch (ConfigurationException e) { - fail(e.toString()); - } String encodedPassword = authenticator.encode("password"); @@ -60,4 +90,35 @@ public class AuthenticatorTest { } + @Test + public void testAuthentication() throws UnsupportedEncodingException, NoSuchAlgorithmException { + Map<String, Object[]> dummyMap = new HashMap<String, Object[]>(); + assertEquals("32 byte salt authenticated", true, authenticator.authenticate("admin", "password", 0L, dummyMap)); + assertEquals("20 byte salt authenticated", true, authenticator.authenticate("admin20Byte", "password", 0L, dummyMap)); + assertEquals("fake user not authenticated", false, authenticator.authenticate("fake", "fake", 0L, dummyMap)); + assertEquals("bad password not authenticated", false, authenticator.authenticate("admin", "fake", 0L, dummyMap)); + assertEquals("20 byte user bad password not authenticated", false, authenticator.authenticate("admin20Byte", "fake", 0L, dummyMap)); + } + + @Test + public void testTiming() throws UnsupportedEncodingException, NoSuchAlgorithmException { + Map<String, Object[]> dummyMap = new HashMap<String, Object[]>(); + Double threshold = (double)500000; //half a millisecond + + Long t1 = System.nanoTime(); + authenticator.authenticate("admin", "password", 0L, dummyMap); + Long t2 = System.nanoTime(); + authenticator.authenticate("admin20Byte", "password", 0L, dummyMap); + Long t3 = System.nanoTime(); + authenticator.authenticate("fake", "fake", 0L, dummyMap); + Long t4 = System.nanoTime(); + authenticator.authenticate("admin", "fake", 0L, dummyMap); + Long t5 = System.nanoTime(); + Long diff1 = t2 - t1; + Long diff2 = t3 - t2; + Long diff3 = t4 - t3; + Long diff4 = t5 - t4; + Assert.assertTrue("All computation times within " + threshold / 1000000 + " milisecond", + (diff1 <= threshold) && (diff2 <= threshold) && (diff3 <= threshold) && (diff4 <= threshold)); + } }
