SEC-3190: Add support for colons in remember-me token values

We have an issue where token strings that contain a colon break
the existing decoding strategy, which tokenizes on colons.  so this 
change urlencodes the individual tokens when creating the cookie 
string; and urldecodes them decoding the cookie and extracting the 
tokens.  This also eliminates the need for existing code to deal with
openid tokens which contain urls, and thus colons.
This commit is contained in:
Jeremy Waters 2016-01-11 13:16:55 -05:00 committed by Rob Winch
parent 8d717c62af
commit 832f5c39c1
2 changed files with 25 additions and 11 deletions

View File

@ -15,8 +15,12 @@
*/ */
package org.springframework.security.web.authentication.rememberme; package org.springframework.security.web.authentication.rememberme;
import java.io.UnsupportedEncodingException;
import java.lang.reflect.Method; import java.lang.reflect.Method;
import java.util.Base64; import java.util.Base64;
import java.net.URLDecoder;
import java.net.URLEncoder;
import java.nio.charset.StandardCharsets;
import javax.servlet.http.Cookie; import javax.servlet.http.Cookie;
import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletRequest;
@ -50,7 +54,7 @@ import org.springframework.util.StringUtils;
* *
* @author Luke Taylor * @author Luke Taylor
* @author Rob Winch * @author Rob Winch
* @author Edd<EFBFBD> Mel<EFBFBD>ndez * @author Edd? Mel?ndez
* @since 2.0 * @since 2.0
*/ */
public abstract class AbstractRememberMeServices implements RememberMeServices, public abstract class AbstractRememberMeServices implements RememberMeServices,
@ -229,13 +233,16 @@ public abstract class AbstractRememberMeServices implements RememberMeServices,
String[] tokens = StringUtils.delimitedListToStringArray(cookieAsPlainText, String[] tokens = StringUtils.delimitedListToStringArray(cookieAsPlainText,
DELIMITER); DELIMITER);
if ((tokens[0].equalsIgnoreCase("http") || tokens[0].equalsIgnoreCase("https")) for (int i = 0; i < tokens.length; i++)
&& tokens[1].startsWith("//")) { {
// Assume we've accidentally split a URL (OpenID identifier) try
String[] newTokens = new String[tokens.length - 1]; {
newTokens[0] = tokens[0] + ":" + tokens[1]; tokens[i] = URLDecoder.decode(tokens[i], StandardCharsets.UTF_8.toString());
System.arraycopy(tokens, 2, newTokens, 1, newTokens.length - 1); }
tokens = newTokens; catch (UnsupportedEncodingException e)
{
logger.error(e.getMessage(), e);
}
} }
return tokens; return tokens;
@ -250,7 +257,14 @@ public abstract class AbstractRememberMeServices implements RememberMeServices,
protected String encodeCookie(String[] cookieTokens) { protected String encodeCookie(String[] cookieTokens) {
StringBuilder sb = new StringBuilder(); StringBuilder sb = new StringBuilder();
for (int i = 0; i < cookieTokens.length; i++) { for (int i = 0; i < cookieTokens.length; i++) {
sb.append(cookieTokens[i]); try
{
sb.append(URLEncoder.encode(cookieTokens[i], StandardCharsets.UTF_8.toString()));
}
catch (UnsupportedEncodingException e)
{
logger.error(e.getMessage(), e);
}
if (i < cookieTokens.length - 1) { if (i < cookieTokens.length - 1) {
sb.append(DELIMITER); sb.append(DELIMITER);

View File

@ -90,7 +90,7 @@ public class AbstractRememberMeServicesTests {
@Test @Test
public void cookieShouldBeCorrectlyEncodedAndDecoded() throws Exception { public void cookieShouldBeCorrectlyEncodedAndDecoded() throws Exception {
String[] cookie = new String[] { "name", "cookie", "tokens", "blah" }; String[] cookie = new String[] { "name:with:colon", "cookie", "tokens", "blah" };
MockRememberMeServices services = new MockRememberMeServices(uds); MockRememberMeServices services = new MockRememberMeServices(uds);
String encoded = services.encodeCookie(cookie); String encoded = services.encodeCookie(cookie);
@ -98,7 +98,7 @@ public class AbstractRememberMeServicesTests {
assertThat(encoded.endsWith("=")).isFalse(); assertThat(encoded.endsWith("=")).isFalse();
String[] decoded = services.decodeCookie(encoded); String[] decoded = services.decodeCookie(encoded);
assertThat(decoded).containsExactly("name", "cookie", "tokens", "blah"); assertThat(decoded).containsExactly("name:with:colon", "cookie", "tokens", "blah");
} }
@Test @Test