From 915b2acf73a75c4e51e67a3c7fd85f908df6259b Mon Sep 17 00:00:00 2001 From: Rob Winch Date: Sun, 7 Oct 2012 17:46:54 -0500 Subject: [PATCH] SEC-2056: DaoAuthenticationProvider performs isPasswordValid when user not found Previously authenticating a user could take significantly longer than determining that a user does not exist. This was due to the fact that only users that were found would use the password encoder and comparing a password can take a significant amount of time. The difference in the time required could allow a side channel attack that reveals if a user exists. The code has been updated to do comparison against a dummy password even when the the user was not found. Conflicts: core/src/main/java/org/springframework/security/authentication/dao/DaoAuthenticationProvider.java --- .../dao/DaoAuthenticationProvider.java | 31 ++++++++++++- .../dao/DaoAuthenticationProviderTests.java | 43 +++++++++++++++++++ 2 files changed, 73 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/org/springframework/security/authentication/dao/DaoAuthenticationProvider.java b/core/src/main/java/org/springframework/security/authentication/dao/DaoAuthenticationProvider.java index a0fe5eee6d..b25487d191 100644 --- a/core/src/main/java/org/springframework/security/authentication/dao/DaoAuthenticationProvider.java +++ b/core/src/main/java/org/springframework/security/authentication/dao/DaoAuthenticationProvider.java @@ -24,6 +24,7 @@ import org.springframework.security.authentication.encoding.PlaintextPasswordEnc import org.springframework.security.core.AuthenticationException; import org.springframework.security.core.userdetails.UserDetails; import org.springframework.security.core.userdetails.UserDetailsService; +import org.springframework.security.core.userdetails.UsernameNotFoundException; import org.springframework.dao.DataAccessException; import org.springframework.util.Assert; @@ -32,12 +33,27 @@ import org.springframework.util.Assert; * from an {@link UserDetailsService}. * * @author Ben Alex + * @author Rob Winch */ public class DaoAuthenticationProvider extends AbstractUserDetailsAuthenticationProvider { + //~ Static fields/initializers ===================================================================================== + + /** + * The plaintext password used to perform {@link PasswordEncoder#isPasswordValid(String, String, Object)} on when the user is + * not found to avoid SEC-2056. + */ + private static final String USER_NOT_FOUND_PASSWORD = "userNotFoundPassword"; //~ Instance fields ================================================================================================ - private PasswordEncoder passwordEncoder = new PlaintextPasswordEncoder(); + private PasswordEncoder passwordEncoder; + + /** + * The password used to perform {@link PasswordEncoder#isPasswordValid(String, String, Object)} on when the user is + * not found to avoid SEC-2056. This is necessary, because some {@link PasswordEncoder} implementations will short circuit if the + * password is not in a valid format. + */ + private String userNotFoundEncodedPassword; private SaltSource saltSource; @@ -45,8 +61,13 @@ public class DaoAuthenticationProvider extends AbstractUserDetailsAuthentication private boolean includeDetailsObject = true; + public DaoAuthenticationProvider() { + setPasswordEncoder(new PlaintextPasswordEncoder()); + } + //~ Methods ======================================================================================================== + @SuppressWarnings("deprecation") protected void additionalAuthenticationChecks(UserDetails userDetails, UsernamePasswordAuthenticationToken authentication) throws AuthenticationException { Object salt = null; @@ -88,6 +109,13 @@ public class DaoAuthenticationProvider extends AbstractUserDetailsAuthentication catch (DataAccessException repositoryProblem) { throw new AuthenticationServiceException(repositoryProblem.getMessage(), repositoryProblem); } + catch (UsernameNotFoundException notFound) { + if(authentication.getCredentials() != null) { + String presentedPassword = authentication.getCredentials().toString(); + passwordEncoder.isPasswordValid(userNotFoundEncodedPassword, presentedPassword, null); + } + throw notFound; + } if (loadedUser == null) { throw new AuthenticationServiceException( @@ -103,6 +131,7 @@ public class DaoAuthenticationProvider extends AbstractUserDetailsAuthentication * @param passwordEncoder The passwordEncoder to use */ public void setPasswordEncoder(PasswordEncoder passwordEncoder) { + this.userNotFoundEncodedPassword = passwordEncoder.encodePassword(USER_NOT_FOUND_PASSWORD, null); this.passwordEncoder = passwordEncoder; } diff --git a/core/src/test/java/org/springframework/security/authentication/dao/DaoAuthenticationProviderTests.java b/core/src/test/java/org/springframework/security/authentication/dao/DaoAuthenticationProviderTests.java index 4d1b9365ac..2f380c4ff6 100644 --- a/core/src/test/java/org/springframework/security/authentication/dao/DaoAuthenticationProviderTests.java +++ b/core/src/test/java/org/springframework/security/authentication/dao/DaoAuthenticationProviderTests.java @@ -15,6 +15,14 @@ package org.springframework.security.authentication.dao; +import static org.mockito.Matchers.anyObject; +import static org.mockito.Matchers.anyString; +import static org.mockito.Matchers.isA; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + import java.util.List; import junit.framework.TestCase; @@ -29,6 +37,7 @@ import org.springframework.security.authentication.DisabledException; import org.springframework.security.authentication.LockedException; import org.springframework.security.authentication.TestingAuthenticationToken; import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; +import org.springframework.security.authentication.encoding.PasswordEncoder; import org.springframework.security.authentication.encoding.ShaPasswordEncoder; import org.springframework.security.core.Authentication; import org.springframework.security.core.GrantedAuthority; @@ -45,6 +54,7 @@ import org.springframework.security.core.userdetails.cache.NullUserCache; * Tests {@link DaoAuthenticationProvider}. * * @author Ben Alex + * @author Rob Winch */ public class DaoAuthenticationProviderTests extends TestCase { private static final List ROLES_12 = AuthorityUtils.createAuthorityList("ROLE_ONE", "ROLE_TWO"); @@ -434,6 +444,39 @@ public class DaoAuthenticationProviderTests extends TestCase { assertTrue(!provider.supports(TestingAuthenticationToken.class)); } + // SEC-2056 + public void testUserNotFoundEncodesPassword() { + UsernamePasswordAuthenticationToken token = new UsernamePasswordAuthenticationToken("missing", "koala"); + PasswordEncoder encoder = mock(PasswordEncoder.class); + when(encoder.encodePassword(anyString(), anyObject())).thenReturn("koala"); + DaoAuthenticationProvider provider = new DaoAuthenticationProvider(); + provider.setHideUserNotFoundExceptions(false); + provider.setPasswordEncoder(encoder); + provider.setUserDetailsService(new MockAuthenticationDaoUserrod()); + try { + provider.authenticate(token); + fail("Expected Exception"); + } catch(UsernameNotFoundException success) {} + + // ensure encoder invoked w/ non-null strings since PasswordEncoder impls may fail if encoded password is null + verify(encoder).isPasswordValid(isA(String.class), isA(String.class), anyObject()); + } + + public void testUserNotFoundNullCredentials() { + UsernamePasswordAuthenticationToken token = new UsernamePasswordAuthenticationToken("missing", null); + PasswordEncoder encoder = mock(PasswordEncoder.class); + DaoAuthenticationProvider provider = new DaoAuthenticationProvider(); + provider.setHideUserNotFoundExceptions(false); + provider.setPasswordEncoder(encoder); + provider.setUserDetailsService(new MockAuthenticationDaoUserrod()); + try { + provider.authenticate(token); + fail("Expected Exception"); + } catch(UsernameNotFoundException success) {} + + verify(encoder,times(0)).isPasswordValid(anyString(), anyString(), anyObject()); + } + //~ Inner Classes ================================================================================================== private class MockAuthenticationDaoReturnsNull implements UserDetailsService {