HADOOP-12758. Extend CSRF Filter with UserAgent Checks. Contributed by Larry McCay.
This commit is contained in:
parent
49e176c29f
commit
a37e423e84
|
@ -1137,6 +1137,9 @@ Release 2.8.0 - UNRELEASED
|
|||
HADOOP-12450. UserGroupInformation should not log at WARN level if no groups
|
||||
are found. (Elliott Clark via stevel)
|
||||
|
||||
HADOOP-12758. Extend CSRF Filter with UserAgent Checks
|
||||
(Larry McCay via cnauroth)
|
||||
|
||||
BUG FIXES
|
||||
|
||||
HADOOP-12352. Delay in checkpointing Trash can leave trash for 2 intervals
|
||||
|
|
|
@ -19,6 +19,8 @@ package org.apache.hadoop.security.http;
|
|||
|
||||
import java.io.IOException;
|
||||
import java.util.HashSet;
|
||||
import java.util.regex.Matcher;
|
||||
import java.util.regex.Pattern;
|
||||
import java.util.Set;
|
||||
|
||||
import javax.servlet.Filter;
|
||||
|
@ -38,13 +40,18 @@ import javax.servlet.http.HttpServletResponse;
|
|||
* attempt as a bad request.
|
||||
*/
|
||||
public class RestCsrfPreventionFilter implements Filter {
|
||||
public static final String HEADER_USER_AGENT = "User-Agent";
|
||||
public static final String BROWSER_USER_AGENT_PARAM =
|
||||
"browser-useragents-regex";
|
||||
public static final String CUSTOM_HEADER_PARAM = "custom-header";
|
||||
public static final String CUSTOM_METHODS_TO_IGNORE_PARAM =
|
||||
"methods-to-ignore";
|
||||
static final String BROWSER_USER_AGENTS_DEFAULT = "^Mozilla.*,^Opera.*";
|
||||
static final String HEADER_DEFAULT = "X-XSRF-HEADER";
|
||||
static final String METHODS_TO_IGNORE_DEFAULT = "GET,OPTIONS,HEAD,TRACE";
|
||||
private String headerName = HEADER_DEFAULT;
|
||||
private Set<String> methodsToIgnore = null;
|
||||
private Set<Pattern> browserUserAgents;
|
||||
|
||||
@Override
|
||||
public void init(FilterConfig filterConfig) throws ServletException {
|
||||
|
@ -59,6 +66,20 @@ public class RestCsrfPreventionFilter implements Filter {
|
|||
} else {
|
||||
parseMethodsToIgnore(METHODS_TO_IGNORE_DEFAULT);
|
||||
}
|
||||
|
||||
String agents = filterConfig.getInitParameter(BROWSER_USER_AGENT_PARAM);
|
||||
if (agents == null) {
|
||||
agents = BROWSER_USER_AGENTS_DEFAULT;
|
||||
}
|
||||
parseBrowserUserAgents(agents);
|
||||
}
|
||||
|
||||
void parseBrowserUserAgents(String userAgents) {
|
||||
String[] agentsArray = userAgents.split(",");
|
||||
browserUserAgents = new HashSet<Pattern>();
|
||||
for (String patternString : agentsArray) {
|
||||
browserUserAgents.add(Pattern.compile(patternString));
|
||||
}
|
||||
}
|
||||
|
||||
void parseMethodsToIgnore(String mti) {
|
||||
|
@ -69,17 +90,46 @@ public class RestCsrfPreventionFilter implements Filter {
|
|||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* This method interrogates the User-Agent String and returns whether it
|
||||
* refers to a browser. If its not a browser, then the requirement for the
|
||||
* CSRF header will not be enforced; if it is a browser, the requirement will
|
||||
* be enforced.
|
||||
* <p>
|
||||
* A User-Agent String is considered to be a browser if it matches
|
||||
* any of the regex patterns from browser-useragent-regex; the default
|
||||
* behavior is to consider everything a browser that matches the following:
|
||||
* "^Mozilla.*,^Opera.*". Subclasses can optionally override
|
||||
* this method to use different behavior.
|
||||
*
|
||||
* @param userAgent The User-Agent String, or null if there isn't one
|
||||
* @return true if the User-Agent String refers to a browser, false if not
|
||||
*/
|
||||
protected boolean isBrowser(String userAgent) {
|
||||
if (userAgent == null) {
|
||||
return false;
|
||||
}
|
||||
for (Pattern pattern : browserUserAgents) {
|
||||
Matcher matcher = pattern.matcher(userAgent);
|
||||
if (matcher.matches()) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
@Override
|
||||
public void doFilter(ServletRequest request, ServletResponse response,
|
||||
FilterChain chain) throws IOException, ServletException {
|
||||
HttpServletRequest httpRequest = (HttpServletRequest)request;
|
||||
if (methodsToIgnore.contains(httpRequest.getMethod()) ||
|
||||
if (!isBrowser(httpRequest.getHeader(HEADER_USER_AGENT)) ||
|
||||
methodsToIgnore.contains(httpRequest.getMethod()) ||
|
||||
httpRequest.getHeader(headerName) != null) {
|
||||
chain.doFilter(request, response);
|
||||
} else {
|
||||
((HttpServletResponse)response).sendError(
|
||||
HttpServletResponse.SC_BAD_REQUEST,
|
||||
"Missing Required Header for Vulnerability Protection");
|
||||
"Missing Required Header for CSRF Vulnerability Protection");
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
@ -34,8 +34,12 @@ import org.mockito.Mockito;
|
|||
|
||||
public class TestRestCsrfPreventionFilter {
|
||||
|
||||
private static final String NON_BROWSER = "java";
|
||||
private static final String BROWSER_AGENT =
|
||||
"Mozilla/5.0 (compatible; U; ABrowse 0.6; Syllable)" +
|
||||
" AppleWebKit/420+ (KHTML, like Gecko)";
|
||||
private static final String EXPECTED_MESSAGE =
|
||||
"Missing Required Header for Vulnerability Protection";
|
||||
"Missing Required Header for CSRF Vulnerability Protection";
|
||||
private static final String X_CUSTOM_HEADER = "X-CUSTOM_HEADER";
|
||||
|
||||
@Test
|
||||
|
@ -52,7 +56,9 @@ public class TestRestCsrfPreventionFilter {
|
|||
// CSRF has not been sent
|
||||
HttpServletRequest mockReq = Mockito.mock(HttpServletRequest.class);
|
||||
Mockito.when(mockReq.getHeader(RestCsrfPreventionFilter.HEADER_DEFAULT)).
|
||||
thenReturn(null);
|
||||
thenReturn(null);
|
||||
Mockito.when(mockReq.getHeader(RestCsrfPreventionFilter.HEADER_USER_AGENT)).
|
||||
thenReturn(BROWSER_AGENT);
|
||||
|
||||
// Objects to verify interactions based on request
|
||||
HttpServletResponse mockRes = Mockito.mock(HttpServletResponse.class);
|
||||
|
@ -68,6 +74,71 @@ public class TestRestCsrfPreventionFilter {
|
|||
Mockito.verifyZeroInteractions(mockChain);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testNoHeaderCustomAgentConfig_badRequest()
|
||||
throws ServletException, IOException {
|
||||
// Setup the configuration settings of the server
|
||||
FilterConfig filterConfig = Mockito.mock(FilterConfig.class);
|
||||
Mockito.when(filterConfig.getInitParameter(
|
||||
RestCsrfPreventionFilter.CUSTOM_HEADER_PARAM)).thenReturn(null);
|
||||
Mockito.when(filterConfig.getInitParameter(
|
||||
RestCsrfPreventionFilter.CUSTOM_METHODS_TO_IGNORE_PARAM)).
|
||||
thenReturn(null);
|
||||
Mockito.when(filterConfig.getInitParameter(
|
||||
RestCsrfPreventionFilter.BROWSER_USER_AGENT_PARAM)).
|
||||
thenReturn("^Mozilla.*,^Opera.*,curl");
|
||||
|
||||
// CSRF has not been sent
|
||||
HttpServletRequest mockReq = Mockito.mock(HttpServletRequest.class);
|
||||
Mockito.when(mockReq.getHeader(RestCsrfPreventionFilter.HEADER_DEFAULT)).
|
||||
thenReturn(null);
|
||||
Mockito.when(mockReq.getHeader(RestCsrfPreventionFilter.HEADER_USER_AGENT)).
|
||||
thenReturn("curl");
|
||||
|
||||
// Objects to verify interactions based on request
|
||||
HttpServletResponse mockRes = Mockito.mock(HttpServletResponse.class);
|
||||
FilterChain mockChain = Mockito.mock(FilterChain.class);
|
||||
|
||||
// Object under test
|
||||
RestCsrfPreventionFilter filter = new RestCsrfPreventionFilter();
|
||||
filter.init(filterConfig);
|
||||
filter.doFilter(mockReq, mockRes, mockChain);
|
||||
|
||||
verify(mockRes, atLeastOnce()).sendError(
|
||||
HttpServletResponse.SC_BAD_REQUEST, EXPECTED_MESSAGE);
|
||||
Mockito.verifyZeroInteractions(mockChain);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testNoHeaderDefaultConfigNonBrowser_goodRequest()
|
||||
throws ServletException, IOException {
|
||||
// Setup the configuration settings of the server
|
||||
FilterConfig filterConfig = Mockito.mock(FilterConfig.class);
|
||||
Mockito.when(filterConfig.getInitParameter(
|
||||
RestCsrfPreventionFilter.CUSTOM_HEADER_PARAM)).thenReturn(null);
|
||||
Mockito.when(filterConfig.getInitParameter(
|
||||
RestCsrfPreventionFilter.CUSTOM_METHODS_TO_IGNORE_PARAM)).
|
||||
thenReturn(null);
|
||||
|
||||
// CSRF has not been sent
|
||||
HttpServletRequest mockReq = Mockito.mock(HttpServletRequest.class);
|
||||
Mockito.when(mockReq.getHeader(RestCsrfPreventionFilter.HEADER_DEFAULT)).
|
||||
thenReturn(null);
|
||||
Mockito.when(mockReq.getHeader(RestCsrfPreventionFilter.HEADER_USER_AGENT)).
|
||||
thenReturn(NON_BROWSER);
|
||||
|
||||
// Objects to verify interactions based on request
|
||||
HttpServletResponse mockRes = Mockito.mock(HttpServletResponse.class);
|
||||
FilterChain mockChain = Mockito.mock(FilterChain.class);
|
||||
|
||||
// Object under test
|
||||
RestCsrfPreventionFilter filter = new RestCsrfPreventionFilter();
|
||||
filter.init(filterConfig);
|
||||
filter.doFilter(mockReq, mockRes, mockChain);
|
||||
|
||||
Mockito.verify(mockChain).doFilter(mockReq, mockRes);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testHeaderPresentDefaultConfig_goodRequest()
|
||||
throws ServletException, IOException {
|
||||
|
@ -136,9 +207,11 @@ public class TestRestCsrfPreventionFilter {
|
|||
Mockito.when(filterConfig.getInitParameter(
|
||||
RestCsrfPreventionFilter.CUSTOM_METHODS_TO_IGNORE_PARAM)).
|
||||
thenReturn(null);
|
||||
HttpServletRequest mockReq = Mockito.mock(HttpServletRequest.class);
|
||||
Mockito.when(mockReq.getHeader(RestCsrfPreventionFilter.HEADER_USER_AGENT)).
|
||||
thenReturn(BROWSER_AGENT);
|
||||
|
||||
// CSRF has not been sent
|
||||
HttpServletRequest mockReq = Mockito.mock(HttpServletRequest.class);
|
||||
Mockito.when(mockReq.getHeader(RestCsrfPreventionFilter.HEADER_DEFAULT)).
|
||||
thenReturn(null);
|
||||
|
||||
|
@ -164,9 +237,11 @@ public class TestRestCsrfPreventionFilter {
|
|||
Mockito.when(filterConfig.getInitParameter(
|
||||
RestCsrfPreventionFilter.CUSTOM_METHODS_TO_IGNORE_PARAM)).
|
||||
thenReturn("");
|
||||
HttpServletRequest mockReq = Mockito.mock(HttpServletRequest.class);
|
||||
Mockito.when(mockReq.getHeader(RestCsrfPreventionFilter.HEADER_USER_AGENT)).
|
||||
thenReturn(BROWSER_AGENT);
|
||||
|
||||
// CSRF has not been sent
|
||||
HttpServletRequest mockReq = Mockito.mock(HttpServletRequest.class);
|
||||
Mockito.when(mockReq.getHeader(RestCsrfPreventionFilter.HEADER_DEFAULT)).
|
||||
thenReturn(null);
|
||||
Mockito.when(mockReq.getMethod()).
|
||||
|
@ -194,9 +269,11 @@ public class TestRestCsrfPreventionFilter {
|
|||
Mockito.when(filterConfig.getInitParameter(
|
||||
RestCsrfPreventionFilter.CUSTOM_METHODS_TO_IGNORE_PARAM)).
|
||||
thenReturn("GET");
|
||||
HttpServletRequest mockReq = Mockito.mock(HttpServletRequest.class);
|
||||
Mockito.when(mockReq.getHeader(RestCsrfPreventionFilter.HEADER_USER_AGENT)).
|
||||
thenReturn(BROWSER_AGENT);
|
||||
|
||||
// CSRF has not been sent
|
||||
HttpServletRequest mockReq = Mockito.mock(HttpServletRequest.class);
|
||||
Mockito.when(mockReq.getHeader(RestCsrfPreventionFilter.HEADER_DEFAULT)).
|
||||
thenReturn(null);
|
||||
Mockito.when(mockReq.getMethod()).
|
||||
|
@ -224,9 +301,11 @@ public class TestRestCsrfPreventionFilter {
|
|||
Mockito.when(filterConfig.getInitParameter(
|
||||
RestCsrfPreventionFilter.CUSTOM_METHODS_TO_IGNORE_PARAM)).
|
||||
thenReturn("GET,OPTIONS");
|
||||
HttpServletRequest mockReq = Mockito.mock(HttpServletRequest.class);
|
||||
Mockito.when(mockReq.getHeader(RestCsrfPreventionFilter.HEADER_USER_AGENT)).
|
||||
thenReturn(BROWSER_AGENT);
|
||||
|
||||
// CSRF has not been sent
|
||||
HttpServletRequest mockReq = Mockito.mock(HttpServletRequest.class);
|
||||
Mockito.when(mockReq.getHeader(RestCsrfPreventionFilter.HEADER_DEFAULT)).
|
||||
thenReturn(null);
|
||||
Mockito.when(mockReq.getMethod()).
|
||||
|
@ -254,9 +333,11 @@ public class TestRestCsrfPreventionFilter {
|
|||
Mockito.when(filterConfig.getInitParameter(
|
||||
RestCsrfPreventionFilter.CUSTOM_METHODS_TO_IGNORE_PARAM)).
|
||||
thenReturn("GET,OPTIONS");
|
||||
HttpServletRequest mockReq = Mockito.mock(HttpServletRequest.class);
|
||||
Mockito.when(mockReq.getHeader(RestCsrfPreventionFilter.HEADER_USER_AGENT)).
|
||||
thenReturn(BROWSER_AGENT);
|
||||
|
||||
// CSRF has not been sent
|
||||
HttpServletRequest mockReq = Mockito.mock(HttpServletRequest.class);
|
||||
Mockito.when(mockReq.getHeader(RestCsrfPreventionFilter.HEADER_DEFAULT)).
|
||||
thenReturn(null);
|
||||
Mockito.when(mockReq.getMethod()).
|
||||
|
|
Loading…
Reference in New Issue