From 20e8724bab1bc143bc34c73a516f71b6dddd74c8 Mon Sep 17 00:00:00 2001 From: Marcus Da Coregio Date: Mon, 3 Jan 2022 09:37:19 -0300 Subject: [PATCH] Polish Remember-Me SHA-256 Algorithm Issue gh-8549 --- .../web/configurers/RememberMeConfigurer.java | 7 +- .../http/RememberMeBeanDefinitionParser.java | 2 +- .../security/config/spring-security-5.5.rnc | 5 - .../security/config/spring-security-5.5.xsd | 13 -- .../security/config/spring-security-5.7.rnc | 3 + .../security/config/spring-security-5.7.xsd | 12 ++ .../RememberMeConfigurerTests.java | 82 ++++++++--- .../config/http/RememberMeConfigTests.java | 11 +- .../servlet/authentication/rememberme.adoc | 1 + .../RememberMeHashingAlgorithm.java | 33 ++--- .../TokenBasedRememberMeServices.java | 138 ++++++++++-------- .../TokenBasedRememberMeServicesTests.java | 31 +++- 12 files changed, 201 insertions(+), 137 deletions(-) diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/RememberMeConfigurer.java b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/RememberMeConfigurer.java index d19a555eac6..680572cb254 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/RememberMeConfigurer.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/RememberMeConfigurer.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2021 the original author or authors. + * Copyright 2002-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -92,7 +92,7 @@ public final class RememberMeConfigurer> private String key; - private RememberMeHashingAlgorithm hashingAlgorithm = RememberMeHashingAlgorithm.UNSET; + private RememberMeHashingAlgorithm hashingAlgorithm = RememberMeHashingAlgorithm.MD5; private RememberMeServices rememberMeServices; @@ -199,9 +199,10 @@ public RememberMeConfigurer key(String key) { * {@link #rememberMeServices(RememberMeServices)} are used. * @param hashingAlgorithm the algorithm used when creating new cookies * @return the {@link RememberMeConfigurer} for further customization - * @since 5.5 + * @since 5.7 */ public RememberMeConfigurer hashingAlgorithm(RememberMeHashingAlgorithm hashingAlgorithm) { + Assert.notNull(hashingAlgorithm, "hashingAlgorithm cannot be null"); this.hashingAlgorithm = hashingAlgorithm; return this; } diff --git a/config/src/main/java/org/springframework/security/config/http/RememberMeBeanDefinitionParser.java b/config/src/main/java/org/springframework/security/config/http/RememberMeBeanDefinitionParser.java index a6d14f8b7de..79179eb3e23 100644 --- a/config/src/main/java/org/springframework/security/config/http/RememberMeBeanDefinitionParser.java +++ b/config/src/main/java/org/springframework/security/config/http/RememberMeBeanDefinitionParser.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2021 the original author or authors. + * Copyright 2002-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/config/src/main/resources/org/springframework/security/config/spring-security-5.5.rnc b/config/src/main/resources/org/springframework/security/config/spring-security-5.5.rnc index 88a96f901a7..a227e804d93 100644 --- a/config/src/main/resources/org/springframework/security/config/spring-security-5.5.rnc +++ b/config/src/main/resources/org/springframework/security/config/spring-security-5.5.rnc @@ -757,10 +757,6 @@ remember-me.attlist &= ## The period (in seconds) for which the remember-me cookie should be valid. attribute token-validity-seconds {xsd:string}? -remember-me.attlist &= - ## The algorithm of cookie which store the token for remember-me authentication. - attribute hashing-algorithm {"UNSET" | "MD5" | "SHA256"}? - remember-me.attlist &= ## Reference to an AuthenticationSuccessHandler bean which should be used to handle a successful remember-me authentication. attribute authentication-success-handler-ref {xsd:token}? @@ -781,7 +777,6 @@ remember-me-data-source-ref = ## DataSource bean for the database that contains the token repository schema. data-source-ref - anonymous = ## Adds support for automatically granting all anonymous web requests a particular principal identity and a corresponding granted authority. element anonymous {anonymous.attlist} diff --git a/config/src/main/resources/org/springframework/security/config/spring-security-5.5.xsd b/config/src/main/resources/org/springframework/security/config/spring-security-5.5.xsd index ca1244ab4e8..8078b0af406 100644 --- a/config/src/main/resources/org/springframework/security/config/spring-security-5.5.xsd +++ b/config/src/main/resources/org/springframework/security/config/spring-security-5.5.xsd @@ -2262,19 +2262,6 @@ - - - The algorithm of cookie which store the token for remember-me authentication. - - - - - - - - - - Reference to an AuthenticationSuccessHandler bean which should be used to handle a diff --git a/config/src/main/resources/org/springframework/security/config/spring-security-5.7.rnc b/config/src/main/resources/org/springframework/security/config/spring-security-5.7.rnc index 42b1349c0e2..491bfcbc372 100644 --- a/config/src/main/resources/org/springframework/security/config/spring-security-5.7.rnc +++ b/config/src/main/resources/org/springframework/security/config/spring-security-5.7.rnc @@ -790,6 +790,9 @@ remember-me.attlist &= remember-me.attlist &= ## The name of cookie which store the token for remember-me authentication. Defaults to 'remember-me'. attribute remember-me-cookie {xsd:token}? +remember-me.attlist &= + ## The algorithm of cookie which store the token for remember-me authentication. + attribute hashing-algorithm {"MD5" | "SHA256"}? token-repository-ref = ## Reference to a PersistentTokenRepository bean for use with the persistent token remember-me implementation. diff --git a/config/src/main/resources/org/springframework/security/config/spring-security-5.7.xsd b/config/src/main/resources/org/springframework/security/config/spring-security-5.7.xsd index 201953d2cf1..be3073d8148 100644 --- a/config/src/main/resources/org/springframework/security/config/spring-security-5.7.xsd +++ b/config/src/main/resources/org/springframework/security/config/spring-security-5.7.xsd @@ -2353,6 +2353,18 @@ + + + The algorithm of cookie which store the token for remember-me authentication. + + + + + + + + + diff --git a/config/src/test/java/org/springframework/security/config/annotation/web/configurers/RememberMeConfigurerTests.java b/config/src/test/java/org/springframework/security/config/annotation/web/configurers/RememberMeConfigurerTests.java index a3cbb4723d6..935988bd9f0 100644 --- a/config/src/test/java/org/springframework/security/config/annotation/web/configurers/RememberMeConfigurerTests.java +++ b/config/src/test/java/org/springframework/security/config/annotation/web/configurers/RememberMeConfigurerTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,6 +16,7 @@ package org.springframework.security.config.annotation.web.configurers; +import java.util.Base64; import java.util.Collections; import javax.servlet.http.Cookie; @@ -30,6 +31,7 @@ import org.springframework.mock.web.MockHttpSession; import org.springframework.security.authentication.RememberMeAuthenticationToken; import org.springframework.security.authentication.dao.DaoAuthenticationProvider; +import org.springframework.security.config.Customizer; import org.springframework.security.config.annotation.ObjectPostProcessor; import org.springframework.security.config.annotation.authentication.builders.AuthenticationManagerBuilder; import org.springframework.security.config.annotation.web.builders.HttpSecurity; @@ -42,6 +44,7 @@ import org.springframework.security.core.userdetails.UserDetailsService; import org.springframework.security.provisioning.InMemoryUserDetailsManager; import org.springframework.security.test.web.servlet.response.SecurityMockMvcResultMatchers; +import org.springframework.security.web.SecurityFilterChain; import org.springframework.security.web.authentication.RememberMeServices; import org.springframework.security.web.authentication.rememberme.RememberMeAuthenticationFilter; import org.springframework.security.web.authentication.rememberme.RememberMeHashingAlgorithm; @@ -101,7 +104,8 @@ public void postWhenNoUserDetailsServiceThenException() { @Test public void configureWhenRegisteringObjectPostProcessorThenInvokedOnRememberMeAuthenticationFilter() { this.spring.register(ObjectPostProcessorConfig.class).autowire(); - verify(ObjectPostProcessorConfig.objectPostProcessor).postProcess(any(RememberMeAuthenticationFilter.class)); + verify(this.spring.getContext().getBean(ObjectPostProcessor.class)) + .postProcess(any(RememberMeAuthenticationFilter.class)); } @Test @@ -132,7 +136,7 @@ public void loginWhenRememberMeTrueThenRespondsWithRememberMeCookie() throws Exc } @Test - public void loginWithSha256HashingAlgorithmThenRespondsWithSha256RememberMeCookie() throws Exception { + public void loginWhenSha256HashingAlgorithmThenRespondsWithSha256RememberMeCookie() throws Exception { this.spring.register(RememberMeWithSha256Config.class).autowire(); // @formatter:off MockHttpServletRequestBuilder request = post("/login") @@ -144,8 +148,26 @@ public void loginWithSha256HashingAlgorithmThenRespondsWithSha256RememberMeCooki MvcResult result = this.mvc.perform(request).andReturn(); Cookie rememberMe = result.getResponse().getCookie("remember-me"); assertThat(rememberMe).isNotNull(); - assertThat(new String(Base64.decodeBase64(rememberMe.getValue()))) - .contains(RememberMeHashingAlgorithm.SHA256.getIdentifier()); + assertThat(new String(Base64.getDecoder().decode(rememberMe.getValue()))) + .contains(RememberMeHashingAlgorithm.SHA256.name()); + } + + @Test + public void loginWhenSha256HashingAlgorithmConfiguredInLambdaThenRespondsWithSha256RememberMeCookie() + throws Exception { + this.spring.register(RememberMeWithSha256InLambdaConfig.class).autowire(); + // @formatter:off + MockHttpServletRequestBuilder request = post("/login") + .with(csrf()) + .param("username", "user") + .param("password", "password") + .param("remember-me", "true"); + // @formatter:on + MvcResult result = this.mvc.perform(request).andReturn(); + Cookie rememberMe = result.getResponse().getCookie("remember-me"); + assertThat(rememberMe).isNotNull(); + assertThat(new String(Base64.getDecoder().decode(rememberMe.getValue()))) + .contains(RememberMeHashingAlgorithm.SHA256.name()); } @Test @@ -319,14 +341,14 @@ protected void configure(AuthenticationManagerBuilder auth) { @EnableWebSecurity static class ObjectPostProcessorConfig extends WebSecurityConfigurerAdapter { - static ObjectPostProcessor objectPostProcessor = spy(ReflectingObjectPostProcessor.class); + ObjectPostProcessor objectPostProcessor = spy(ReflectingObjectPostProcessor.class); @Override protected void configure(HttpSecurity http) throws Exception { // @formatter:off http .rememberMe() - .userDetailsService(new AuthenticationManagerBuilder(objectPostProcessor).getDefaultUserDetailsService()); + .userDetailsService(new AuthenticationManagerBuilder(this.objectPostProcessor).getDefaultUserDetailsService()); // @formatter:on } @@ -339,8 +361,8 @@ protected void configure(AuthenticationManagerBuilder auth) throws Exception { } @Bean - static ObjectPostProcessor objectPostProcessor() { - return objectPostProcessor; + ObjectPostProcessor objectPostProcessor() { + return this.objectPostProcessor; } } @@ -416,10 +438,10 @@ void configureGlobal(AuthenticationManagerBuilder auth) throws Exception { } @EnableWebSecurity - static class RememberMeWithSha256Config extends WebSecurityConfigurerAdapter { + static class RememberMeWithSha256Config { - @Override - protected void configure(HttpSecurity http) throws Exception { + @Bean + SecurityFilterChain app(HttpSecurity http, UserDetailsService userDetailsService) throws Exception { // @formatter:off http .authorizeRequests() @@ -428,17 +450,41 @@ protected void configure(HttpSecurity http) throws Exception { .formLogin() .and() .rememberMe() - .hashingAlgorithm(RememberMeHashingAlgorithm.SHA256); + .hashingAlgorithm(RememberMeHashingAlgorithm.SHA256) + .userDetailsService(userDetailsService); // @formatter:on + return http.build(); } - @Autowired - void configureGlobal(AuthenticationManagerBuilder auth) throws Exception { + @Bean + UserDetailsService userDetailsService() { + return new InMemoryUserDetailsManager(PasswordEncodedUser.user()); + } + + } + + @EnableWebSecurity + static class RememberMeWithSha256InLambdaConfig { + + @Bean + SecurityFilterChain app(HttpSecurity http, UserDetailsService userDetailsService) throws Exception { // @formatter:off - auth - .inMemoryAuthentication() - .withUser(PasswordEncodedUser.user()); + http + .authorizeRequests((requests) -> requests + .anyRequest().hasRole("USER") + ) + .formLogin(Customizer.withDefaults()) + .rememberMe((rememberMe) -> rememberMe + .hashingAlgorithm(RememberMeHashingAlgorithm.SHA256) + .userDetailsService(userDetailsService) + ); // @formatter:on + return http.build(); + } + + @Bean + UserDetailsService userDetailsService() { + return new InMemoryUserDetailsManager(PasswordEncodedUser.user()); } } diff --git a/config/src/test/java/org/springframework/security/config/http/RememberMeConfigTests.java b/config/src/test/java/org/springframework/security/config/http/RememberMeConfigTests.java index 3d13256d51c..1f7f272de3c 100644 --- a/config/src/test/java/org/springframework/security/config/http/RememberMeConfigTests.java +++ b/config/src/test/java/org/springframework/security/config/http/RememberMeConfigTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,6 +16,7 @@ package org.springframework.security.config.http; +import java.util.Base64; import java.util.Collections; import javax.servlet.http.Cookie; @@ -59,6 +60,7 @@ * @author Rob Winch * @author Oliver Becker */ +@ExtendWith(SpringTestContextExtension.class) public class RememberMeConfigTests { private static final String CONFIG_LOCATION_PREFIX = "classpath:org/springframework/security/config/http/RememberMeConfigTests"; @@ -66,8 +68,7 @@ public class RememberMeConfigTests { @Autowired MockMvc mvc; - @Rule - public final SpringTestRule spring = new SpringTestRule(); + public final SpringTestContext spring = new SpringTestContext(this); @Test public void requestWithRememberMeWhenUsingCustomTokenRepositoryThenAutomaticallyReauthenticates() throws Exception { @@ -165,8 +166,8 @@ public void configureWithHashingAlgorithm() throws Exception { MvcResult result = rememberAuthentication("user", "password").andReturn(); Cookie cookie = rememberMeCookie(result); assertThat(cookie).isNotNull(); - assertThat(new String(Base64.decodeBase64(cookie.getValue()))) - .contains(RememberMeHashingAlgorithm.SHA256.getIdentifier()); + assertThat(new String(Base64.getDecoder().decode(cookie.getValue()))) + .contains(RememberMeHashingAlgorithm.SHA256.name()); } @Test diff --git a/docs/modules/ROOT/pages/servlet/authentication/rememberme.adoc b/docs/modules/ROOT/pages/servlet/authentication/rememberme.adoc index 584e7f161f4..c821f8b434d 100644 --- a/docs/modules/ROOT/pages/servlet/authentication/rememberme.adoc +++ b/docs/modules/ROOT/pages/servlet/authentication/rememberme.adoc @@ -36,6 +36,7 @@ username: As identifiable to the UserDetailsService password: That matches the one in the retrieved UserDetails expirationTime: The date and time when the remember-me token expires, expressed in milliseconds key: A private key to prevent modification of the remember-me token +algorithmName The name of the algorithm used to create the hash ---- As such the remember-me token is valid only for the period specified, and provided that the username, password and key does not change. diff --git a/web/src/main/java/org/springframework/security/web/authentication/rememberme/RememberMeHashingAlgorithm.java b/web/src/main/java/org/springframework/security/web/authentication/rememberme/RememberMeHashingAlgorithm.java index b42c33c4c72..328b27e43d5 100644 --- a/web/src/main/java/org/springframework/security/web/authentication/rememberme/RememberMeHashingAlgorithm.java +++ b/web/src/main/java/org/springframework/security/web/authentication/rememberme/RememberMeHashingAlgorithm.java @@ -1,5 +1,5 @@ /* - * Copyright 2021 the original author or authors. + * Copyright 2002-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,37 +16,21 @@ package org.springframework.security.web.authentication.rememberme; -import java.util.Arrays; -import java.util.Optional; - /** * Hashing algorithms supported by {@link TokenBasedRememberMeServices} * - * @since 5.5 + * @since 5.7 */ public enum RememberMeHashingAlgorithm { - UNSET("", ""), MD5("MD5", "MD5"), SHA256("SHA256", "SHA-256"); - - private final String identifier; + MD5("MD5"), SHA256("SHA-256"); private final String digestAlgorithm; - RememberMeHashingAlgorithm(String identifier, String digestAlgorithm) { - this.identifier = identifier; + RememberMeHashingAlgorithm(String digestAlgorithm) { this.digestAlgorithm = digestAlgorithm; } - /** - * The identifier to use in cookies created by {@link TokenBasedRememberMeServices} to - * signify this algorithm is being used. - * - * If empty, then no algorithm will be specified in the resulting cookie. - */ - public String getIdentifier() { - return this.identifier; - } - /** * The name of the algorithm to use * @@ -57,8 +41,13 @@ public String getDigestAlgorithm() { return this.digestAlgorithm; } - static Optional findByIdentifier(String identifier) { - return Arrays.stream(values()).filter((algorithm) -> algorithm.getIdentifier().equals(identifier)).findAny(); + public static RememberMeHashingAlgorithm from(String name) { + for (RememberMeHashingAlgorithm algorithm : values()) { + if (algorithm.name().equals(name)) { + return algorithm; + } + } + return null; } } diff --git a/web/src/main/java/org/springframework/security/web/authentication/rememberme/TokenBasedRememberMeServices.java b/web/src/main/java/org/springframework/security/web/authentication/rememberme/TokenBasedRememberMeServices.java index 01c7654072f..b3fa6894c47 100644 --- a/web/src/main/java/org/springframework/security/web/authentication/rememberme/TokenBasedRememberMeServices.java +++ b/web/src/main/java/org/springframework/security/web/authentication/rememberme/TokenBasedRememberMeServices.java @@ -1,5 +1,5 @@ /* - * Copyright 2004-2021 Acegi Technology Pty Limited + * Copyright 2004-2022 Acegi Technology Pty Limited * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -20,7 +20,6 @@ import java.security.NoSuchAlgorithmException; import java.util.Arrays; import java.util.Date; -import java.util.Objects; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; @@ -96,48 +95,51 @@ */ public class TokenBasedRememberMeServices extends AbstractRememberMeServices { - private RememberMeHashingAlgorithm hashingAlgorithm; + private static final RememberMeHashingAlgorithm DEFAULT_HASHING_ALGORITHM = RememberMeHashingAlgorithm.MD5; + + private RememberMeHashingAlgorithm hashingAlgorithm = DEFAULT_HASHING_ALGORITHM; public TokenBasedRememberMeServices(String key, UserDetailsService userDetailsService) { - this(key, userDetailsService, RememberMeHashingAlgorithm.UNSET); + super(key, userDetailsService); } /** - * @since 5.5 + * Constructs a {@link TokenBasedRememberMeServices} using the provided parameters + * @param key the key to use in the hashing algorithm + * @param userDetailsService the {@link UserDetailsService} to retrieve users from + * @param hashingAlgorithm the hashing algorithm to use in the Cookie value */ public TokenBasedRememberMeServices(String key, UserDetailsService userDetailsService, RememberMeHashingAlgorithm hashingAlgorithm) { - super(key, userDetailsService); + this(key, userDetailsService); + Assert.notNull(hashingAlgorithm, "hashingAlgorithm cannot be null"); this.hashingAlgorithm = hashingAlgorithm; } @Override protected UserDetails processAutoLoginCookie(String[] cookieTokens, HttpServletRequest request, HttpServletResponse response) { - if (!(cookieTokens.length == 3 || cookieTokens.length == 4)) { - throw new InvalidCookieException( - "Cookie token did not contain 3 or 4 tokens, but contained '" + Arrays.asList(cookieTokens) + "'"); + CookieTokens properties = new CookieTokens(cookieTokens); + if (isTokenExpired(properties.expirationTime)) { + throw new InvalidCookieException("Cookie token[1] has expired (expired on '" + + new Date(properties.expirationTime) + "'; current time is '" + new Date() + "')"); } - long tokenExpiryTime = getTokenExpiryTime(cookieTokens); - if (isTokenExpired(tokenExpiryTime)) { - throw new InvalidCookieException("Cookie token[1] has expired (expired on '" + new Date(tokenExpiryTime) - + "'; current time is '" + new Date() + "')"); - } - RememberMeHashingAlgorithm algorithm = getTokenHashingAlgorithm(cookieTokens); + RememberMeHashingAlgorithm algorithm = properties.algorithm; // Check the user exists. Defer lookup until after expiry time checked, to // possibly avoid expensive database call. - UserDetails userDetails = getUserDetailsService().loadUserByUsername(cookieTokens[0]); - Assert.notNull(userDetails, () -> "UserDetailsService " + getUserDetailsService() - + " returned null for username " + cookieTokens[0] + ". " + "This is an interface contract violation"); + UserDetails userDetails = getUserDetailsService().loadUserByUsername(properties.username); + Assert.notNull(userDetails, + () -> "UserDetailsService " + getUserDetailsService() + " returned null for username " + + properties.username + ". " + "This is an interface contract violation"); // Check signature of token matches remaining details. Must do this after user // lookup, as we need the DAO-derived password. If efficiency was a major issue, // just add in a UserCache implementation, but recall that this method is usually // only called once per HttpSession - if the token is valid, it will cause // SecurityContextHolder population, whilst if invalid, will cause the cookie to // be cancelled. - String expectedTokenSignature = makeTokenSignature(tokenExpiryTime, userDetails.getUsername(), + String expectedTokenSignature = makeTokenSignature(properties.expirationTime, userDetails.getUsername(), userDetails.getPassword(), algorithm); - String tokenSignature = (cookieTokens.length == 4) ? cookieTokens[3] : cookieTokens[2]; + String tokenSignature = properties.signature; if (!equals(expectedTokenSignature, tokenSignature)) { throw new InvalidCookieException("Last Cookie token contained signature '" + tokenSignature + "' but expected '" + expectedTokenSignature + "'"); @@ -145,31 +147,12 @@ protected UserDetails processAutoLoginCookie(String[] cookieTokens, HttpServletR return userDetails; } - private long getTokenExpiryTime(String[] cookieTokens) { - try { - return new Long(cookieTokens[1]); - } - catch (NumberFormatException nfe) { - throw new InvalidCookieException( - "Cookie token[1] did not contain a valid number (contained '" + cookieTokens[1] + "')"); - } - } - - private RememberMeHashingAlgorithm getTokenHashingAlgorithm(String[] cookieTokens) { - if (cookieTokens.length == 3) { - return RememberMeHashingAlgorithm.UNSET; - } - return RememberMeHashingAlgorithm.findByIdentifier(cookieTokens[2]).orElseThrow(IllegalArgumentException::new); - } - /** - * Calculates the digital signature expected in the cookie if no algorithm is declared - * or configured. - * - * Default value is MD5("username:tokenExpiryTime:password:key") + * Calculates the digital signature to be put in the cookie. Default value is the + * hashing algorithm applied to "username:tokenExpiryTime:password:key" */ protected String makeTokenSignature(long tokenExpiryTime, String username, String password) { - return makeTokenSignature(tokenExpiryTime, username, password, RememberMeHashingAlgorithm.MD5); + return makeTokenSignature(tokenExpiryTime, username, password, this.hashingAlgorithm); } /** @@ -178,12 +161,12 @@ protected String makeTokenSignature(long tokenExpiryTime, String username, Strin * * Default value is the hashing algorithm applied to * "username:tokenExpiryTime:password:key" - * @since 5.5 + * @since 5.7 */ protected String makeTokenSignature(long tokenExpiryTime, String username, String password, RememberMeHashingAlgorithm algorithm) { - if (algorithm == RememberMeHashingAlgorithm.UNSET) { - return makeTokenSignature(tokenExpiryTime, username, password); + if (algorithm == null) { + algorithm = DEFAULT_HASHING_ALGORITHM; } String data = username + ":" + tokenExpiryTime + ":" + password + ":" + getKey(); try { @@ -191,7 +174,7 @@ protected String makeTokenSignature(long tokenExpiryTime, String username, Strin return new String(Hex.encode(digest.digest(data.getBytes()))); } catch (NoSuchAlgorithmException ex) { - throw new IllegalStateException("No MD5 algorithm available!"); + throw new IllegalStateException("No " + algorithm.getDigestAlgorithm() + " algorithm available!"); } } @@ -224,9 +207,8 @@ public void onLoginSuccess(HttpServletRequest request, HttpServletResponse respo // SEC-949 expiryTime += 1000L * ((tokenLifetime < 0) ? TWO_WEEKS_S : tokenLifetime); String signatureValue = makeTokenSignature(expiryTime, username, password, this.hashingAlgorithm); - String[] cookieTokens = (this.hashingAlgorithm == RememberMeHashingAlgorithm.UNSET) - ? new String[] { username, Long.toString(expiryTime), signatureValue } : new String[] { username, - Long.toString(expiryTime), this.hashingAlgorithm.getIdentifier(), signatureValue }; + String[] cookieTokens = new String[] { username, Long.toString(expiryTime), this.hashingAlgorithm.name(), + signatureValue }; setCookie(cookieTokens, tokenLifetime, request, response); if (this.logger.isDebugEnabled()) { this.logger.debug( @@ -287,20 +269,52 @@ private static byte[] bytesUtf8(String s) { return (s != null) ? Utf8.encode(s) : null; } - /** - * @since 5.5 - */ - public RememberMeHashingAlgorithm getHashAlgorithm() { - return this.hashingAlgorithm; - } + private static final class CookieTokens { + + private final String username; + + private final long expirationTime; + + private final RememberMeHashingAlgorithm algorithm; + + private final String signature; + + private CookieTokens(String[] cookieTokens) { + if (!(cookieTokens.length == 3 || cookieTokens.length == 4)) { + throw new InvalidCookieException("Cookie token did not contain 3 or 4 tokens, but contained '" + + Arrays.asList(cookieTokens) + "'"); + } + this.username = getUsername(cookieTokens); + this.expirationTime = getTokenExpiryTime(cookieTokens); + this.algorithm = getTokenHashingAlgorithm(cookieTokens); + this.signature = getSignature(cookieTokens); + } + + private String getUsername(String[] cookieTokens) { + return cookieTokens[0]; + } + + private long getTokenExpiryTime(String[] cookieTokens) { + try { + return Long.parseLong(cookieTokens[1]); + } + catch (NumberFormatException nfe) { + throw new InvalidCookieException( + "Cookie token[1] did not contain a valid number (contained '" + cookieTokens[1] + "')"); + } + } + + private RememberMeHashingAlgorithm getTokenHashingAlgorithm(String[] cookieTokens) { + if (cookieTokens.length == 3) { + return null; + } + return RememberMeHashingAlgorithm.from(cookieTokens[2]); + } + + private String getSignature(String[] cookieTokens) { + return (cookieTokens.length == 4) ? cookieTokens[3] : cookieTokens[2]; + } - /** - * Sets the hashing algorithm used in new cookies - * @throws NullPointerException when the algorithm is null - * @since 5.5 - */ - public void setHashAlgorithm(RememberMeHashingAlgorithm hashingAlgorithm) { - this.hashingAlgorithm = Objects.requireNonNull(hashingAlgorithm); } } diff --git a/web/src/test/java/org/springframework/security/web/authentication/rememberme/TokenBasedRememberMeServicesTests.java b/web/src/test/java/org/springframework/security/web/authentication/rememberme/TokenBasedRememberMeServicesTests.java index 71d2c255ae8..0e48d2d7292 100644 --- a/web/src/test/java/org/springframework/security/web/authentication/rememberme/TokenBasedRememberMeServicesTests.java +++ b/web/src/test/java/org/springframework/security/web/authentication/rememberme/TokenBasedRememberMeServicesTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2004-2021 Acegi Technology Pty Limited + * Copyright 2004-2022 Acegi Technology Pty Limited * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,6 +16,7 @@ package org.springframework.security.web.authentication.rememberme; +import java.time.Instant; import java.util.Date; import javax.servlet.http.Cookie; @@ -112,7 +113,7 @@ private String generateCorrectCookieContentForTokenWithExplicitAlgorithm(long ex // password + ":" + key) String signatureValue = new DigestUtils(algorithm.getDigestAlgorithm()) .digestAsHex(username + ":" + expiryTime + ":" + password + ":" + key); - String tokenValue = username + ":" + expiryTime + ":" + algorithm.getIdentifier() + ":" + signatureValue; + String tokenValue = username + ":" + expiryTime + ":" + algorithm.name() + ":" + signatureValue; return new String(Base64.encodeBase64(tokenValue.getBytes())); } @@ -259,18 +260,30 @@ public void autoLoginWithValidTokenAndUserWithSha256Succeeds() { assertThat(result.getPrincipal()).isEqualTo(this.user); } + @Test + public void autoLoginWithValidTokenMd5AndServicesWithSha256Succeeds() { + udsWillReturnUser(); + this.services = new TokenBasedRememberMeServices("key", this.uds, RememberMeHashingAlgorithm.SHA256); + Cookie cookie = new Cookie(AbstractRememberMeServices.SPRING_SECURITY_REMEMBER_ME_COOKIE_KEY, + generateCorrectCookieContentForToken(System.currentTimeMillis() + 1000000, "someone", "password", + "key")); + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setCookies(cookie); + MockHttpServletResponse response = new MockHttpServletResponse(); + Authentication result = this.services.autoLogin(request, response); + assertThat(result).isNotNull(); + assertThat(result.getPrincipal()).isEqualTo(this.user); + } + @Test public void testGettersSetters() { assertThat(this.services.getUserDetailsService()).isEqualTo(this.uds); assertThat(this.services.getKey()).isEqualTo("key"); assertThat(this.services.getParameter()).isEqualTo(AbstractRememberMeServices.DEFAULT_PARAMETER); - assertThat(this.services.getHashAlgorithm()).isEqualTo(RememberMeHashingAlgorithm.UNSET); this.services.setParameter("some_param"); assertThat(this.services.getParameter()).isEqualTo("some_param"); this.services.setTokenValiditySeconds(12); assertThat(this.services.getTokenValiditySeconds()).isEqualTo(12); - this.services.setHashAlgorithm(RememberMeHashingAlgorithm.SHA256); - assertThat(this.services.getHashAlgorithm()).isEqualTo(RememberMeHashingAlgorithm.SHA256); } @Test @@ -334,16 +347,18 @@ public void loginSuccessNormalWithSha256AlgorithmSetsExpectedCookie() { MockHttpServletRequest request = new MockHttpServletRequest(); request.addParameter(AbstractRememberMeServices.DEFAULT_PARAMETER, "true"); MockHttpServletResponse response = new MockHttpServletResponse(); - this.services.setHashAlgorithm(RememberMeHashingAlgorithm.SHA256); + this.services = new TokenBasedRememberMeServices("key", this.uds, RememberMeHashingAlgorithm.SHA256); this.services.loginSuccess(request, response, new TestingAuthenticationToken("someone", "password", "ROLE_ABC")); Cookie cookie = response.getCookie(AbstractRememberMeServices.SPRING_SECURITY_REMEMBER_ME_COOKIE_KEY); assertThat(cookie).isNotNull(); assertThat(cookie.getMaxAge()).isEqualTo(this.services.getTokenValiditySeconds()); assertThat(Base64.isBase64(cookie.getValue().getBytes())).isTrue(); - assertThat(new Date().before(new Date(determineExpiryTimeFromBased64EncodedToken(cookie.getValue())))).isTrue(); + assertThat(Instant.now() + .isBefore(Instant.ofEpochMilli(determineExpiryTimeFromBased64EncodedToken(cookie.getValue())))) + .isTrue(); assertThat(determineHashingAlgorithmFromBase64EncodedToken(cookie.getValue())) - .isEqualTo(RememberMeHashingAlgorithm.SHA256.getIdentifier()); + .isEqualTo(RememberMeHashingAlgorithm.SHA256.name()); } // SEC-933