mirror of
https://github.com/spring-projects/spring-security.git
synced 2025-07-05 02:02:15 +00:00
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
This commit is contained in:
parent
c3f5f4686e
commit
915b2acf73
@ -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;
|
||||
}
|
||||
|
||||
|
@ -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<GrantedAuthority> 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 {
|
||||
|
Loading…
x
Reference in New Issue
Block a user