You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ranger.apache.org by pr...@apache.org on 2021/09/09 09:53:27 UTC
[ranger] 02/02: RANGER-3387 : Ranger Admin Header Validation
This is an automated email from the ASF dual-hosted git repository.
pradeep pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ranger.git
commit 15f5c387335a428a36737f54fc803189614c40bd
Author: mateenmansoori <ma...@gmail.com>
AuthorDate: Thu Sep 9 14:34:32 2021 +0530
RANGER-3387 : Ranger Admin Header Validation
Signed-off-by: pradeep <pr...@apache.org>
---
.../java/org/apache/ranger/rest/ServiceREST.java | 20 ++++++++++--
.../web/filter/RangerCSRFPreventionFilter.java | 24 ++++++++++++--
.../conf.dist/ranger-admin-default-site.xml | 4 +++
.../src/main/webapp/scripts/modules/RestCsrf.js | 12 ++++++-
.../web/filter/TestRangerCSRFPreventionFilter.java | 38 +++++++++++++++++++++-
5 files changed, 91 insertions(+), 7 deletions(-)
diff --git a/security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java b/security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java
index 7d26b0a..a685325 100644
--- a/security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java
+++ b/security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java
@@ -21,6 +21,7 @@ package org.apache.ranger.rest;
import java.io.IOException;
import java.io.InputStream;
+import java.security.SecureRandom;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
@@ -55,6 +56,7 @@ import org.apache.commons.collections.CollectionUtils;
import org.apache.commons.collections.MapUtils;
import org.apache.commons.io.IOUtils;
import org.apache.commons.lang.ArrayUtils;
+import org.apache.commons.lang.RandomStringUtils;
import org.apache.commons.lang.StringUtils;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
@@ -179,6 +181,7 @@ public class ServiceREST {
public static final String BROWSER_USER_AGENT_PARAM = "ranger.rest-csrf.browser-useragents-regex";
public static final String CUSTOM_METHODS_TO_IGNORE_PARAM = "ranger.rest-csrf.methods-to-ignore";
public static final String CUSTOM_HEADER_PARAM = "ranger.rest-csrf.custom-header";
+ public static final String CSRF_TOKEN_LENGTH = "ranger.rest-csrf.token.length";
final static public String POLICY_MATCHING_ALGO_BY_POLICYNAME = "matchByName";
final static public String POLICY_MATCHING_ALGO_BY_RESOURCE = "matchByPolicySignature";
final static public String PARAM_POLICY_MATCHING_ALGORITHM = "policyMatchingAlgorithm";
@@ -3655,8 +3658,8 @@ public class ServiceREST {
@GET
@Path("/csrfconf")
@Produces({ "application/json"})
- public HashMap<String, Object> getCSRFProperties() {
- return getCSRFPropertiesMap();
+ public HashMap<String, Object> getCSRFProperties(@Context HttpServletRequest request) {
+ return getCSRFPropertiesMap(request);
}
@GET
@@ -3751,15 +3754,26 @@ public class ServiceREST {
return new ResponseEntity<>(deletedServices, responseStatus);
}
- private HashMap<String, Object> getCSRFPropertiesMap() {
+ private HashMap<String, Object> getCSRFPropertiesMap(HttpServletRequest request) {
HashMap<String, Object> map = new HashMap<String, Object>();
map.put(isCSRF_ENABLED, PropertiesUtil.getBooleanProperty(isCSRF_ENABLED, true));
map.put(CUSTOM_HEADER_PARAM, PropertiesUtil.getProperty(CUSTOM_HEADER_PARAM, RangerCSRFPreventionFilter.HEADER_DEFAULT));
map.put(BROWSER_USER_AGENT_PARAM, PropertiesUtil.getProperty(BROWSER_USER_AGENT_PARAM, RangerCSRFPreventionFilter.BROWSER_USER_AGENTS_DEFAULT));
map.put(CUSTOM_METHODS_TO_IGNORE_PARAM, PropertiesUtil.getProperty(CUSTOM_METHODS_TO_IGNORE_PARAM, RangerCSRFPreventionFilter.METHODS_TO_IGNORE_DEFAULT));
+ map.put(RangerCSRFPreventionFilter.CSRF_TOKEN, getCSRFToken(request));
return map;
}
+ private static String getCSRFToken(HttpServletRequest request) {
+ String salt = (String) request.getSession().getAttribute(RangerCSRFPreventionFilter.CSRF_TOKEN);
+ if (StringUtils.isEmpty(salt)) {
+ final int tokenLength = PropertiesUtil.getIntProperty(CSRF_TOKEN_LENGTH, 20);
+ salt = RandomStringUtils.random(tokenLength, 0, 0, true, true, null, new SecureRandom());
+ request.getSession().setAttribute(RangerCSRFPreventionFilter.CSRF_TOKEN, salt);
+ }
+ return salt;
+ }
+
private RangerPolicyList toRangerPolicyList(List<RangerPolicy> policyList, SearchFilter filter) {
RangerPolicyList ret = new RangerPolicyList();
diff --git a/security-admin/src/main/java/org/apache/ranger/security/web/filter/RangerCSRFPreventionFilter.java b/security-admin/src/main/java/org/apache/ranger/security/web/filter/RangerCSRFPreventionFilter.java
index 3736bb4..254f225 100644
--- a/security-admin/src/main/java/org/apache/ranger/security/web/filter/RangerCSRFPreventionFilter.java
+++ b/security-admin/src/main/java/org/apache/ranger/security/web/filter/RangerCSRFPreventionFilter.java
@@ -31,7 +31,9 @@ import javax.servlet.ServletRequest;
import javax.servlet.ServletResponse;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
+import javax.servlet.http.HttpSession;
+import org.apache.commons.lang.StringUtils;
import org.apache.log4j.Logger;
import org.apache.ranger.common.PropertiesUtil;
@@ -46,6 +48,7 @@ public class RangerCSRFPreventionFilter implements Filter {
public static final String CUSTOM_HEADER_PARAM = "ranger.rest-csrf.custom-header";
public static final String HEADER_DEFAULT = "X-XSRF-HEADER";
public static final String HEADER_USER_AGENT = "User-Agent";
+ public static final String CSRF_TOKEN = "_csrfToken";
private static final boolean IS_CSRF_ENABLED = PropertiesUtil.getBooleanProperty("ranger.rest-csrf.enabled", true);
private String headerName = HEADER_DEFAULT;
@@ -149,12 +152,25 @@ public class RangerCSRFPreventionFilter implements Filter {
public void handleHttpInteraction(HttpInteraction httpInteraction)
throws IOException, ServletException {
- if (httpInteraction.getHeader(headerName) != null
+
+ HttpSession session = ((ServletFilterHttpInteraction) httpInteraction).getSession();
+ String clientCsrfToken = httpInteraction.getHeader(headerName);
+ String actualCsrfToken = StringUtils.EMPTY;
+
+ if (session != null) {
+ actualCsrfToken = (String) session.getAttribute(CSRF_TOKEN);
+ } else {
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("Session is null");
+ }
+ }
+
+ if (clientCsrfToken != null && clientCsrfToken.equals(actualCsrfToken)
|| !isBrowser(httpInteraction.getHeader(HEADER_USER_AGENT))
|| methodsToIgnore.contains(httpInteraction.getMethod())) {
httpInteraction.proceed();
}else {
- httpInteraction.sendError(HttpServletResponse.SC_BAD_REQUEST,"Missing Required Header for CSRF Vulnerability Protection");
+ httpInteraction.sendError(HttpServletResponse.SC_BAD_REQUEST,"Missing header or invalid Header value for CSRF Vulnerability Protection");
}
}
@@ -210,6 +226,10 @@ public class RangerCSRFPreventionFilter implements Filter {
chain.doFilter(httpRequest, httpResponse);
}
+ public HttpSession getSession() {
+ return httpRequest.getSession();
+ }
+
@Override
public void sendError(int code, String message) throws IOException {
httpResponse.sendError(code, message);
diff --git a/security-admin/src/main/resources/conf.dist/ranger-admin-default-site.xml b/security-admin/src/main/resources/conf.dist/ranger-admin-default-site.xml
index 5ffeb02..0a11286 100644
--- a/security-admin/src/main/resources/conf.dist/ranger-admin-default-site.xml
+++ b/security-admin/src/main/resources/conf.dist/ranger-admin-default-site.xml
@@ -531,6 +531,10 @@
<value>GET,OPTIONS,HEAD,TRACE</value>
</property>
<property>
+ <name>ranger.rest-csrf.token.length</name>
+ <value>20</value>
+ </property>
+ <property>
<name>ranger.rest-csrf.browser-useragents-regex</name>
<value>Mozilla,Opera,Chrome</value>
</property>
diff --git a/security-admin/src/main/webapp/scripts/modules/RestCsrf.js b/security-admin/src/main/webapp/scripts/modules/RestCsrf.js
index 50ac5f1..63561e3 100644
--- a/security-admin/src/main/webapp/scripts/modules/RestCsrf.js
+++ b/security-admin/src/main/webapp/scripts/modules/RestCsrf.js
@@ -28,6 +28,7 @@ define(function(require) {
require('jquery');
var restCsrfCustomHeader = null;
var restCsrfMethodsToIgnore = null;
+ var restCsrfToken = null;
if(!window.location.origin){
window.location.origin = window.location.protocol + "//" + window.location.hostname + (window.location.port ? ':' + window.location.port: '');
@@ -61,6 +62,7 @@ define(function(require) {
var $xml = $(data);
var csrfEnabled = false;
var header = null;
+ var headerToken = null;
var methods = [];
$xml.each(function(indx,element){
if(element['ranger.rest-csrf.enabled']) {
@@ -73,6 +75,9 @@ define(function(require) {
if (element['ranger.rest-csrf.methods-to-ignore']) {
methods = getTrimmedStringArrayValue(element['ranger.rest-csrf.methods-to-ignore']);
}
+ if (element['_csrfToken']) {
+ headerToken = element['_csrfToken'];
+ }
});
// If enabled, set up all subsequent AJAX calls with a pre-send callback
@@ -80,6 +85,7 @@ define(function(require) {
if (csrfEnabled) {
restCsrfCustomHeader = header;
restCsrfMethodsToIgnore = {};
+ restCsrfToken = headerToken;
methods.map(function(method) { restCsrfMethodsToIgnore[method] = true; });
$.ajaxSetup({
beforeSend: addRestCsrfCustomHeader
@@ -97,7 +103,11 @@ define(function(require) {
var method = settings.type;
if (restCsrfCustomHeader != null && !restCsrfMethodsToIgnore[method]) {
// The value of the header is unimportant. Only its presence matters.
- xhr.setRequestHeader(restCsrfCustomHeader, '""');
+ if (restCsrfToken != null) {
+ xhr.setRequestHeader(restCsrfCustomHeader, restCsrfToken);
+ } else {
+ xhr.setRequestHeader(restCsrfCustomHeader, '""');
+ }
}
}
});
\ No newline at end of file
diff --git a/security-admin/src/test/java/org/apache/ranger/security/web/filter/TestRangerCSRFPreventionFilter.java b/security-admin/src/test/java/org/apache/ranger/security/web/filter/TestRangerCSRFPreventionFilter.java
index b05afb5..ac9712e 100644
--- a/security-admin/src/test/java/org/apache/ranger/security/web/filter/TestRangerCSRFPreventionFilter.java
+++ b/security-admin/src/test/java/org/apache/ranger/security/web/filter/TestRangerCSRFPreventionFilter.java
@@ -19,11 +19,13 @@
package org.apache.ranger.security.web.filter;
import java.io.IOException;
+import java.io.PrintWriter;
import javax.servlet.FilterChain;
import javax.servlet.ServletException;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
+import javax.servlet.http.HttpSession;
import org.junit.Test;
import org.mockito.Mockito;
@@ -32,7 +34,7 @@ import static org.mockito.Mockito.atLeastOnce;
public class TestRangerCSRFPreventionFilter {
- private static final String EXPECTED_MESSAGE = "Missing Required Header for CSRF Vulnerability Protection";
+ private static final String EXPECTED_MESSAGE = "Missing header or invalid Header value for CSRF Vulnerability Protection";
private static final String X_CUSTOM_HEADER = "X-CUSTOM_HEADER";
private String userAgent = "Mozilla";
@@ -43,6 +45,12 @@ public class TestRangerCSRFPreventionFilter {
Mockito.when(mockReq.getHeader(RangerCSRFPreventionFilter.HEADER_DEFAULT)).thenReturn(null);
Mockito.when(mockReq.getHeader(RangerCSRFPreventionFilter.HEADER_USER_AGENT)).thenReturn(userAgent);
+ Mockito.when(mockReq.getMethod()).thenReturn("POST");
+
+ HttpSession session = Mockito.mock(HttpSession.class);
+ Mockito.when(session.getAttribute(RangerCSRFPreventionFilter.CSRF_TOKEN)).thenReturn("valueUnimportant");
+ Mockito.when(mockReq.getSession()).thenReturn(session);
+
// Objects to verify interactions based on request
HttpServletResponse mockRes = Mockito.mock(HttpServletResponse.class);
FilterChain mockChain = Mockito.mock(FilterChain.class);
@@ -62,6 +70,12 @@ public class TestRangerCSRFPreventionFilter {
Mockito.when(mockReq.getHeader(RangerCSRFPreventionFilter.HEADER_DEFAULT)).thenReturn("valueUnimportant");
Mockito.when(mockReq.getHeader(RangerCSRFPreventionFilter.HEADER_USER_AGENT)).thenReturn(userAgent);
+ Mockito.when(mockReq.getMethod()).thenReturn("POST");
+
+ HttpSession session = Mockito.mock(HttpSession.class);
+ Mockito.when(session.getAttribute(RangerCSRFPreventionFilter.CSRF_TOKEN)).thenReturn("valueUnimportant");
+ Mockito.when(mockReq.getSession()).thenReturn(session);
+
// Objects to verify interactions based on request
HttpServletResponse mockRes = Mockito.mock(HttpServletResponse.class);
FilterChain mockChain = Mockito.mock(FilterChain.class);
@@ -74,6 +88,28 @@ public class TestRangerCSRFPreventionFilter {
}
@Test
+ public void testHeaderPresentDefaultConfig_badRequest() throws ServletException, IOException {
+ // CSRF HAS been sent
+ HttpServletRequest mockReq = Mockito.mock(HttpServletRequest.class);
+ Mockito.when(mockReq.getHeader(RangerCSRFPreventionFilter.HEADER_DEFAULT)).thenReturn("valueUnimportant");
+ Mockito.when(mockReq.getHeader(RangerCSRFPreventionFilter.HEADER_USER_AGENT)).thenReturn(userAgent);
+ Mockito.when(mockReq.getMethod()).thenReturn("POST");
+
+ // Objects to verify interactions based on request
+ HttpServletResponse mockRes = Mockito.mock(HttpServletResponse.class);
+ PrintWriter mockWriter = Mockito.mock(PrintWriter.class);
+ Mockito.when(mockRes.getWriter()).thenReturn(mockWriter);
+
+ FilterChain mockChain = Mockito.mock(FilterChain.class);
+
+ // Object under test
+ RangerCSRFPreventionFilter filter = new RangerCSRFPreventionFilter();
+ filter.doFilter(mockReq, mockRes, mockChain);
+
+ Mockito.verify(mockChain, Mockito.never()).doFilter(mockReq, mockRes);
+ }
+
+ @Test
public void testHeaderPresentCustomHeaderConfig_goodRequest() throws ServletException, IOException {
// CSRF HAS been sent
HttpServletRequest mockReq = Mockito.mock(HttpServletRequest.class);