From fd78d055aa5c2b1845445b78777882fe915e1845 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Thu, 21 Dec 2017 21:38:43 -0800 Subject: [PATCH] Lazily initialize userNotFoundEncodedPassword Update `DaoAuthenticationProvider` so that `userNotFoundEncodedPassword` is lazily initialized on the first call to `retrieveUser`, rather than in `doAfterPropertiesSet`. Since some `PasswordEncoder` implementations can be slow, this change can help to improve application startup times and the expense of some delay with the first login. Note that `userNotFoundEncodedPassword` creation occurs on the first user retrieval, regardless of whether the user is ultimately found. This ensures consistent processing times, regardless of the outcome. First Call: Found = encode(userNotFound) + decode(supplied) Not-Found = encode(userNotFound) + decode(userNotFound) Subsequent Call: Found = decode(supplied) Not-Found = decode(userNotFound) Fixes gh-4915 --- .../dao/DaoAuthenticationProvider.java | 45 +++++++++++-------- 1 file changed, 27 insertions(+), 18 deletions(-) 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 3cdc43622f..f9bdc94611 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 @@ -58,7 +58,7 @@ public class DaoAuthenticationProvider extends AbstractUserDetailsAuthentication * {@link PasswordEncoder} implementations will short circuit if the password is not * in a valid format. */ - private String userNotFoundEncodedPassword; + private volatile String userNotFoundEncodedPassword; private UserDetailsService userDetailsService; @@ -94,34 +94,43 @@ public class DaoAuthenticationProvider extends AbstractUserDetailsAuthentication protected void doAfterPropertiesSet() throws Exception { Assert.notNull(this.userDetailsService, "A UserDetailsService must be set"); - this.userNotFoundEncodedPassword = this.passwordEncoder.encode(USER_NOT_FOUND_PASSWORD); } protected final UserDetails retrieveUser(String username, UsernamePasswordAuthenticationToken authentication) throws AuthenticationException { - UserDetails loadedUser; - + prepareTimingAttackProtection(); try { - loadedUser = this.getUserDetailsService().loadUserByUsername(username); - } - catch (UsernameNotFoundException notFound) { - if (authentication.getCredentials() != null) { - String presentedPassword = authentication.getCredentials().toString(); - passwordEncoder.matches(presentedPassword, userNotFoundEncodedPassword); + UserDetails loadedUser = this.getUserDetailsService().loadUserByUsername(username); + if (loadedUser == null) { + throw new InternalAuthenticationServiceException( + "UserDetailsService returned null, which is an interface contract violation"); } - throw notFound; + return loadedUser; } - catch (Exception repositoryProblem) { - throw new InternalAuthenticationServiceException( - repositoryProblem.getMessage(), repositoryProblem); + catch (UsernameNotFoundException ex) { + mitigateAgainstTimingAttack(authentication); + throw ex; } + catch (InternalAuthenticationServiceException ex) { + throw ex; + } + catch (Exception ex) { + throw new InternalAuthenticationServiceException(ex.getMessage(), ex); + } + } - if (loadedUser == null) { - throw new InternalAuthenticationServiceException( - "UserDetailsService returned null, which is an interface contract violation"); + private void prepareTimingAttackProtection() { + if (this.userNotFoundEncodedPassword == null) { + this.userNotFoundEncodedPassword = this.passwordEncoder.encode(USER_NOT_FOUND_PASSWORD); + } + } + + private void mitigateAgainstTimingAttack(UsernamePasswordAuthenticationToken authentication) { + if (authentication.getCredentials() != null) { + String presentedPassword = authentication.getCredentials().toString(); + this.passwordEncoder.matches(presentedPassword, this.userNotFoundEncodedPassword); } - return loadedUser; } /**