SEC-506: Fix as suggested by reporter. Split the disgest header string ignoring separating commas which occur between quotes.
This commit is contained in:
parent
3f123e1478
commit
c8077c5e87
|
@ -105,7 +105,8 @@ public class DigestProcessingFilter implements Filter, InitializingBean, Message
|
||||||
Assert.notNull(authenticationEntryPoint, "A DigestProcessingFilterEntryPoint is required");
|
Assert.notNull(authenticationEntryPoint, "A DigestProcessingFilterEntryPoint is required");
|
||||||
}
|
}
|
||||||
|
|
||||||
public void destroy() {}
|
public void destroy() {
|
||||||
|
}
|
||||||
|
|
||||||
public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain)
|
public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain)
|
||||||
throws IOException, ServletException {
|
throws IOException, ServletException {
|
||||||
|
@ -128,7 +129,7 @@ public class DigestProcessingFilter implements Filter, InitializingBean, Message
|
||||||
if ((header != null) && header.startsWith("Digest ")) {
|
if ((header != null) && header.startsWith("Digest ")) {
|
||||||
String section212response = header.substring(7);
|
String section212response = header.substring(7);
|
||||||
|
|
||||||
String[] headerEntries = StringUtils.commaDelimitedListToStringArray(section212response);
|
String[] headerEntries = StringSplitUtils.splitIgnoringQuotes(section212response, ',');
|
||||||
Map headerMap = StringSplitUtils.splitEachArrayElementAndCreateMap(headerEntries, "=", "\"");
|
Map headerMap = StringSplitUtils.splitEachArrayElementAndCreateMap(headerEntries, "=", "\"");
|
||||||
|
|
||||||
String username = (String) headerMap.get("username");
|
String username = (String) headerMap.get("username");
|
||||||
|
@ -149,7 +150,7 @@ public class DigestProcessingFilter implements Filter, InitializingBean, Message
|
||||||
|
|
||||||
fail(request, response,
|
fail(request, response,
|
||||||
new BadCredentialsException(messages.getMessage("DigestProcessingFilter.missingMandatory",
|
new BadCredentialsException(messages.getMessage("DigestProcessingFilter.missingMandatory",
|
||||||
new Object[] {section212response}, "Missing mandatory digest value; received header {0}")));
|
new Object[]{section212response}, "Missing mandatory digest value; received header {0}")));
|
||||||
|
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
@ -163,7 +164,7 @@ public class DigestProcessingFilter implements Filter, InitializingBean, Message
|
||||||
|
|
||||||
fail(request, response,
|
fail(request, response,
|
||||||
new BadCredentialsException(messages.getMessage("DigestProcessingFilter.missingAuth",
|
new BadCredentialsException(messages.getMessage("DigestProcessingFilter.missingAuth",
|
||||||
new Object[] {section212response}, "Missing mandatory digest value; received header {0}")));
|
new Object[]{section212response}, "Missing mandatory digest value; received header {0}")));
|
||||||
|
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
@ -173,7 +174,7 @@ public class DigestProcessingFilter implements Filter, InitializingBean, Message
|
||||||
if (!this.getAuthenticationEntryPoint().getRealmName().equals(realm)) {
|
if (!this.getAuthenticationEntryPoint().getRealmName().equals(realm)) {
|
||||||
fail(request, response,
|
fail(request, response,
|
||||||
new BadCredentialsException(messages.getMessage("DigestProcessingFilter.incorrectRealm",
|
new BadCredentialsException(messages.getMessage("DigestProcessingFilter.incorrectRealm",
|
||||||
new Object[] {realm, this.getAuthenticationEntryPoint().getRealmName()},
|
new Object[]{realm, this.getAuthenticationEntryPoint().getRealmName()},
|
||||||
"Response realm name '{0}' does not match system realm name of '{1}'")));
|
"Response realm name '{0}' does not match system realm name of '{1}'")));
|
||||||
|
|
||||||
return;
|
return;
|
||||||
|
@ -183,7 +184,7 @@ public class DigestProcessingFilter implements Filter, InitializingBean, Message
|
||||||
if (!Base64.isArrayByteBase64(nonce.getBytes())) {
|
if (!Base64.isArrayByteBase64(nonce.getBytes())) {
|
||||||
fail(request, response,
|
fail(request, response,
|
||||||
new BadCredentialsException(messages.getMessage("DigestProcessingFilter.nonceEncoding",
|
new BadCredentialsException(messages.getMessage("DigestProcessingFilter.nonceEncoding",
|
||||||
new Object[] {nonce}, "Nonce is not encoded in Base64; received nonce {0}")));
|
new Object[]{nonce}, "Nonce is not encoded in Base64; received nonce {0}")));
|
||||||
|
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
@ -197,7 +198,7 @@ public class DigestProcessingFilter implements Filter, InitializingBean, Message
|
||||||
if (nonceTokens.length != 2) {
|
if (nonceTokens.length != 2) {
|
||||||
fail(request, response,
|
fail(request, response,
|
||||||
new BadCredentialsException(messages.getMessage("DigestProcessingFilter.nonceNotTwoTokens",
|
new BadCredentialsException(messages.getMessage("DigestProcessingFilter.nonceNotTwoTokens",
|
||||||
new Object[] {nonceAsPlainText}, "Nonce should have yielded two tokens but was {0}")));
|
new Object[]{nonceAsPlainText}, "Nonce should have yielded two tokens but was {0}")));
|
||||||
|
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
@ -210,7 +211,7 @@ public class DigestProcessingFilter implements Filter, InitializingBean, Message
|
||||||
} catch (NumberFormatException nfe) {
|
} catch (NumberFormatException nfe) {
|
||||||
fail(request, response,
|
fail(request, response,
|
||||||
new BadCredentialsException(messages.getMessage("DigestProcessingFilter.nonceNotNumeric",
|
new BadCredentialsException(messages.getMessage("DigestProcessingFilter.nonceNotNumeric",
|
||||||
new Object[] {nonceAsPlainText},
|
new Object[]{nonceAsPlainText},
|
||||||
"Nonce token should have yielded a numeric first token, but was {0}")));
|
"Nonce token should have yielded a numeric first token, but was {0}")));
|
||||||
|
|
||||||
return;
|
return;
|
||||||
|
@ -223,7 +224,7 @@ public class DigestProcessingFilter implements Filter, InitializingBean, Message
|
||||||
if (!expectedNonceSignature.equals(nonceTokens[1])) {
|
if (!expectedNonceSignature.equals(nonceTokens[1])) {
|
||||||
fail(request, response,
|
fail(request, response,
|
||||||
new BadCredentialsException(messages.getMessage("DigestProcessingFilter.nonceCompromised",
|
new BadCredentialsException(messages.getMessage("DigestProcessingFilter.nonceCompromised",
|
||||||
new Object[] {nonceAsPlainText}, "Nonce token compromised {0}")));
|
new Object[]{nonceAsPlainText}, "Nonce token compromised {0}")));
|
||||||
|
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
@ -242,7 +243,7 @@ public class DigestProcessingFilter implements Filter, InitializingBean, Message
|
||||||
} catch (UsernameNotFoundException notFound) {
|
} catch (UsernameNotFoundException notFound) {
|
||||||
fail(request, response,
|
fail(request, response,
|
||||||
new BadCredentialsException(messages.getMessage("DigestProcessingFilter.usernameNotFound",
|
new BadCredentialsException(messages.getMessage("DigestProcessingFilter.usernameNotFound",
|
||||||
new Object[] {username}, "Username {0} not found")));
|
new Object[]{username}, "Username {0} not found")));
|
||||||
|
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
@ -275,7 +276,7 @@ public class DigestProcessingFilter implements Filter, InitializingBean, Message
|
||||||
// Would very rarely happen, as user existed earlier
|
// Would very rarely happen, as user existed earlier
|
||||||
fail(request, response,
|
fail(request, response,
|
||||||
new BadCredentialsException(messages.getMessage("DigestProcessingFilter.usernameNotFound",
|
new BadCredentialsException(messages.getMessage("DigestProcessingFilter.usernameNotFound",
|
||||||
new Object[] {username}, "Username {0} not found")));
|
new Object[]{username}, "Username {0} not found")));
|
||||||
}
|
}
|
||||||
|
|
||||||
userCache.putUserInCache(user);
|
userCache.putUserInCache(user);
|
||||||
|
@ -361,9 +362,7 @@ public class DigestProcessingFilter implements Filter, InitializingBean, Message
|
||||||
* @param nonce the nonce supplied by the server
|
* @param nonce the nonce supplied by the server
|
||||||
* @param nc the "nonce-count" as defined in RFC 2617.
|
* @param nc the "nonce-count" as defined in RFC 2617.
|
||||||
* @param cnonce opaque string supplied by the client when qop is set.
|
* @param cnonce opaque string supplied by the client when qop is set.
|
||||||
*
|
|
||||||
* @return the MD5 of the digest authentication response, encoded in hex
|
* @return the MD5 of the digest authentication response, encoded in hex
|
||||||
*
|
|
||||||
* @throws IllegalArgumentException if the supplied qop value is unsupported.
|
* @throws IllegalArgumentException if the supplied qop value is unsupported.
|
||||||
*/
|
*/
|
||||||
public static String generateDigest(boolean passwordAlreadyEncoded, String username, String realm, String password,
|
public static String generateDigest(boolean passwordAlreadyEncoded, String username, String realm, String password,
|
||||||
|
@ -408,7 +407,8 @@ public class DigestProcessingFilter implements Filter, InitializingBean, Message
|
||||||
return userDetailsService;
|
return userDetailsService;
|
||||||
}
|
}
|
||||||
|
|
||||||
public void init(FilterConfig ignored) throws ServletException {}
|
public void init(FilterConfig ignored) throws ServletException {
|
||||||
|
}
|
||||||
|
|
||||||
public void setAuthenticationDetailsSource(AuthenticationDetailsSource authenticationDetailsSource) {
|
public void setAuthenticationDetailsSource(AuthenticationDetailsSource authenticationDetailsSource) {
|
||||||
Assert.notNull(authenticationDetailsSource, "AuthenticationDetailsSource required");
|
Assert.notNull(authenticationDetailsSource, "AuthenticationDetailsSource required");
|
||||||
|
|
|
@ -20,6 +20,8 @@ import org.springframework.util.StringUtils;
|
||||||
|
|
||||||
import java.util.HashMap;
|
import java.util.HashMap;
|
||||||
import java.util.Map;
|
import java.util.Map;
|
||||||
|
import java.util.ArrayList;
|
||||||
|
import java.util.List;
|
||||||
|
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
@ -29,6 +31,9 @@ import java.util.Map;
|
||||||
* @version $Id$
|
* @version $Id$
|
||||||
*/
|
*/
|
||||||
public final class StringSplitUtils {
|
public final class StringSplitUtils {
|
||||||
|
//~ Static fields/initializers =====================================================================================
|
||||||
|
private static final String[] EMPTY_STRING_ARRAY = new String[0];
|
||||||
|
|
||||||
//~ Constructors ===================================================================================================
|
//~ Constructors ===================================================================================================
|
||||||
|
|
||||||
private StringSplitUtils() {
|
private StringSplitUtils() {
|
||||||
|
@ -42,10 +47,8 @@ public final class StringSplitUtils {
|
||||||
*
|
*
|
||||||
* @param toSplit the string to split
|
* @param toSplit the string to split
|
||||||
* @param delimiter to split the string up with
|
* @param delimiter to split the string up with
|
||||||
*
|
|
||||||
* @return a two element array with index 0 being before the delimiter, and index 1 being after the delimiter
|
* @return a two element array with index 0 being before the delimiter, and index 1 being after the delimiter
|
||||||
* (neither element includes the delimiter)
|
* (neither element includes the delimiter)
|
||||||
*
|
|
||||||
* @throws IllegalArgumentException if an argument was invalid
|
* @throws IllegalArgumentException if an argument was invalid
|
||||||
*/
|
*/
|
||||||
public static String[] split(String toSplit, String delimiter) {
|
public static String[] split(String toSplit, String delimiter) {
|
||||||
|
@ -65,7 +68,7 @@ public final class StringSplitUtils {
|
||||||
String beforeDelimiter = toSplit.substring(0, offset);
|
String beforeDelimiter = toSplit.substring(0, offset);
|
||||||
String afterDelimiter = toSplit.substring(offset + 1);
|
String afterDelimiter = toSplit.substring(offset + 1);
|
||||||
|
|
||||||
return new String[] {beforeDelimiter, afterDelimiter};
|
return new String[]{beforeDelimiter, afterDelimiter};
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
@ -78,7 +81,6 @@ public final class StringSplitUtils {
|
||||||
* @param delimiter to split each element using (typically the equals symbol)
|
* @param delimiter to split each element using (typically the equals symbol)
|
||||||
* @param removeCharacters one or more characters to remove from each element prior to attempting the split
|
* @param removeCharacters one or more characters to remove from each element prior to attempting the split
|
||||||
* operation (typically the quotation mark symbol) or <code>null</code> if no removal should occur
|
* operation (typically the quotation mark symbol) or <code>null</code> if no removal should occur
|
||||||
*
|
|
||||||
* @return a <code>Map</code> representing the array contents, or <code>null</code> if the array to process was
|
* @return a <code>Map</code> representing the array contents, or <code>null</code> if the array to process was
|
||||||
* null or empty
|
* null or empty
|
||||||
*/
|
*/
|
||||||
|
@ -135,4 +137,58 @@ public final class StringSplitUtils {
|
||||||
return str.substring(pos + separator.length());
|
return str.substring(pos + separator.length());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Splits a given string on the given separator character, skips the contents of quoted substrings
|
||||||
|
* when looking for separators.
|
||||||
|
* Introduced for use in DigestProcessingFilter (see SEC-506).
|
||||||
|
* <p/>
|
||||||
|
* This was copied and modified from commons-lang StringUtils
|
||||||
|
*/
|
||||||
|
public static String[] splitIgnoringQuotes(String str, char separatorChar) {
|
||||||
|
if (str == null) {
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
|
||||||
|
int len = str.length();
|
||||||
|
|
||||||
|
if (len == 0) {
|
||||||
|
return EMPTY_STRING_ARRAY;
|
||||||
|
}
|
||||||
|
|
||||||
|
List list = new ArrayList();
|
||||||
|
int i = 0;
|
||||||
|
int start = 0;
|
||||||
|
boolean match = false;
|
||||||
|
|
||||||
|
while (i < len) {
|
||||||
|
if (str.charAt(i) == '"') {
|
||||||
|
i++;
|
||||||
|
while (i < len) {
|
||||||
|
if (str.charAt(i) == '"') {
|
||||||
|
i++;
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
i++;
|
||||||
|
}
|
||||||
|
match = true;
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
if (str.charAt(i) == separatorChar) {
|
||||||
|
if (match) {
|
||||||
|
list.add(str.substring(start, i));
|
||||||
|
match = false;
|
||||||
|
}
|
||||||
|
start = ++i;
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
match = true;
|
||||||
|
i++;
|
||||||
|
}
|
||||||
|
if (match) {
|
||||||
|
list.add(str.substring(start, i));
|
||||||
|
}
|
||||||
|
|
||||||
|
return (String[]) list.toArray(new String[list.size()]);
|
||||||
|
}
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
|
@ -62,25 +62,28 @@ public class DigestProcessingFilterTests extends MockObjectTestCase {
|
||||||
|
|
||||||
private static final String NC = "00000002";
|
private static final String NC = "00000002";
|
||||||
private static final String CNONCE = "c822c727a648aba7";
|
private static final String CNONCE = "c822c727a648aba7";
|
||||||
private static final String REALM = "The Correct Realm Name";
|
private static final String REALM = "The Actual, Correct Realm Name";
|
||||||
private static final String KEY = "acegi";
|
private static final String KEY = "acegi";
|
||||||
private static final String QOP = "auth";
|
private static final String QOP = "auth";
|
||||||
private static final String USERNAME = "marissa";
|
private static final String USERNAME = "marissa,ok";
|
||||||
private static final String PASSWORD = "koala";
|
private static final String PASSWORD = "koala";
|
||||||
private static final String REQUEST_URI = "/some_file.html";
|
private static final String REQUEST_URI = "/some_file.html";
|
||||||
|
|
||||||
/** A standard valid nonce with a validity period of 60 seconds */
|
/**
|
||||||
|
* A standard valid nonce with a validity period of 60 seconds
|
||||||
|
*/
|
||||||
private static final String NONCE = generateNonce(60);
|
private static final String NONCE = generateNonce(60);
|
||||||
|
|
||||||
//~ Instance fields ================================================================================================
|
//~ Instance fields ================================================================================================
|
||||||
|
|
||||||
// private ApplicationContext ctx;
|
// private ApplicationContext ctx;
|
||||||
private DigestProcessingFilter filter;
|
private DigestProcessingFilter filter;
|
||||||
private MockHttpServletRequest request;
|
private MockHttpServletRequest request;
|
||||||
|
|
||||||
//~ Constructors ===================================================================================================
|
//~ Constructors ===================================================================================================
|
||||||
|
|
||||||
public DigestProcessingFilterTests() {}
|
public DigestProcessingFilterTests() {
|
||||||
|
}
|
||||||
|
|
||||||
public DigestProcessingFilterTests(String arg0) {
|
public DigestProcessingFilterTests(String arg0) {
|
||||||
super(arg0);
|
super(arg0);
|
||||||
|
@ -118,10 +121,6 @@ public class DigestProcessingFilterTests extends MockObjectTestCase {
|
||||||
return new String(Base64.encodeBase64(nonceValue.getBytes()));
|
return new String(Base64.encodeBase64(nonceValue.getBytes()));
|
||||||
}
|
}
|
||||||
|
|
||||||
public static void main(String[] args) {
|
|
||||||
junit.textui.TestRunner.run(DigestProcessingFilterTests.class);
|
|
||||||
}
|
|
||||||
|
|
||||||
protected void setUp() throws Exception {
|
protected void setUp() throws Exception {
|
||||||
super.setUp();
|
super.setUp();
|
||||||
SecurityContextHolder.clearContext();
|
SecurityContextHolder.clearContext();
|
||||||
|
@ -129,7 +128,7 @@ public class DigestProcessingFilterTests extends MockObjectTestCase {
|
||||||
// Create User Details Service
|
// Create User Details Service
|
||||||
InMemoryDaoImpl dao = new InMemoryDaoImpl();
|
InMemoryDaoImpl dao = new InMemoryDaoImpl();
|
||||||
UserMapEditor editor = new UserMapEditor();
|
UserMapEditor editor = new UserMapEditor();
|
||||||
editor.setAsText("marissa=koala,ROLE_ONE,ROLE_TWO,enabled\r\n");
|
editor.setAsText("marissa,ok=koala,ROLE_ONE,ROLE_TWO,enabled\r\n");
|
||||||
dao.setUserMap((UserMap) editor.getValue());
|
dao.setUserMap((UserMap) editor.getValue());
|
||||||
|
|
||||||
DigestProcessingFilterEntryPoint ep = new DigestProcessingFilterEntryPoint();
|
DigestProcessingFilterEntryPoint ep = new DigestProcessingFilterEntryPoint();
|
||||||
|
|
|
@ -32,6 +32,7 @@ public class StringSplitUtilsTests extends TestCase {
|
||||||
//~ Constructors ===================================================================================================
|
//~ Constructors ===================================================================================================
|
||||||
|
|
||||||
// ===========================================================
|
// ===========================================================
|
||||||
|
|
||||||
public StringSplitUtilsTests() {
|
public StringSplitUtilsTests() {
|
||||||
super();
|
super();
|
||||||
}
|
}
|
||||||
|
@ -43,6 +44,7 @@ public class StringSplitUtilsTests extends TestCase {
|
||||||
//~ Methods ========================================================================================================
|
//~ Methods ========================================================================================================
|
||||||
|
|
||||||
// ================================================================
|
// ================================================================
|
||||||
|
|
||||||
public static void main(String[] args) {
|
public static void main(String[] args) {
|
||||||
junit.textui.TestRunner.run(StringSplitUtilsTests.class);
|
junit.textui.TestRunner.run(StringSplitUtilsTests.class);
|
||||||
}
|
}
|
||||||
|
@ -84,7 +86,7 @@ public class StringSplitUtilsTests extends TestCase {
|
||||||
|
|
||||||
public void testSplitEachArrayElementAndCreateMapReturnsNullIfArrayEmptyOrNull() {
|
public void testSplitEachArrayElementAndCreateMapReturnsNullIfArrayEmptyOrNull() {
|
||||||
assertNull(StringSplitUtils.splitEachArrayElementAndCreateMap(null, "=", "\""));
|
assertNull(StringSplitUtils.splitEachArrayElementAndCreateMap(null, "=", "\""));
|
||||||
assertNull(StringSplitUtils.splitEachArrayElementAndCreateMap(new String[] {}, "=", "\""));
|
assertNull(StringSplitUtils.splitEachArrayElementAndCreateMap(new String[]{}, "=", "\""));
|
||||||
}
|
}
|
||||||
|
|
||||||
public void testSplitNormalOperation() {
|
public void testSplitNormalOperation() {
|
||||||
|
@ -137,4 +139,14 @@ public class StringSplitUtilsTests extends TestCase {
|
||||||
// only guarantees to split at FIRST delimiter, not EACH delimiter
|
// only guarantees to split at FIRST delimiter, not EACH delimiter
|
||||||
assertEquals(2, StringSplitUtils.split("18|marissa|foo|bar", "|").length);
|
assertEquals(2, StringSplitUtils.split("18|marissa|foo|bar", "|").length);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
|
public void testAuthorizationHeaderWithCommasIsSplitCorrectly() {
|
||||||
|
String header = "Digest username=\"hamilton,bob\", realm=\"bobs,ok,realm\", nonce=\"the,nonce\", " +
|
||||||
|
"uri=\"the,Uri\", response=\"the,response,Digest\", qop=theqop, nc=thenc, cnonce=\"the,cnonce\"";
|
||||||
|
|
||||||
|
String[] parts = StringSplitUtils.splitIgnoringQuotes(header, ',');
|
||||||
|
|
||||||
|
assertEquals(8, parts.length);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in New Issue