Skip to content

RANGER-5677: Plugin REST endpoints silently ignore supplied credentia… #1062

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
vyommani wants to merge 1 commit into
base: master
Choose a base branch
from
+130 −5
Open

RANGER-5677: Plugin REST endpoints silently ignore supplied credentia… #1062

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 32 additions & 0 deletions ...in/src/main/java/org/apache/ranger/security/web/filter/RangerKRBAuthenticationFilter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,11 +70,13 @@
import java.io.InputStream;
import java.net.URL;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.Enumeration;
import java.util.EventListener;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
Expand Down Expand Up @@ -106,6 +108,12 @@ public class RangerKRBAuthenticationFilter extends RangerKrbFilter {
static final String RULES_MECHANISM_PARAM = "kerberos.name.rules.mechanism";

private static final String KERBEROS_TYPE = "kerberos";
private static final Set<String> OPTIONAL_AUTH_DOWNLOAD_URL_PREFIXES = new HashSet<>(Arrays.asList(
"/service/gds/download/",
"/service/plugins/policies/download/",
"/service/tags/download/",
"/service/roles/download/",
"/service/xusers/download/"));

protected static ServletContext noContext = new ServletContext() {
@Override
Expand Down Expand Up @@ -468,6 +476,15 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha
String authtype = PropertiesUtil.getProperty(RANGER_AUTH_TYPE);
HttpServletRequest httpRequest = (HttpServletRequest) request;
Authentication existingAuth = SecurityContextHolder.getContext().getAuthentication();
// Optional-auth endpoints must not be forced through a SPNEGO challenge when the client
// presents no credentials at all - let the request proceed as anonymous and leave the
// access decision to the REST layer. A request that does present an Authorization header
// (Negotiate or Basic) falls through to the normal logic below and is validated as usual.
if ((existingAuth == null || !existingAuth.isAuthenticated()) && StringUtils.isEmpty(httpRequest.getHeader("Authorization")) && isOptionalAuthDownloadUrl(httpRequest)) {
LOG.debug("RangerKRBAuthenticationFilter: no credentials presented for optional-auth download URL [{}]; skipping SPNEGO challenge", httpRequest.getRequestURI());
filterChain.doFilter(request, response);
return;
}

if (isSpnegoEnable(authtype) && (existingAuth == null || !existingAuth.isAuthenticated())) {
KerberosName.setRules(PropertiesUtil.getProperty(NAME_RULES, "DEFAULT"));
Expand Down Expand Up @@ -708,6 +725,21 @@ private void handleTimeoutRequest(HttpServletRequest httpRequest, HttpServletRes
httpResponse.sendRedirect(redirectUrl);
}

private boolean isOptionalAuthDownloadUrl(HttpServletRequest request) {
String uri = request.getRequestURI();
if (uri == null) {
return false;
}
String ctx = request.getContextPath();
String path = (ctx != null && !ctx.isEmpty() && uri.startsWith(ctx)) ? uri.substring(ctx.length()) : uri;
for (String prefix : OPTIONAL_AUTH_DOWNLOAD_URL_PREFIXES) {
if (path.startsWith(prefix)) {
return true;
}
}
return false;
}

private boolean isSpnegoEnable(String authType) {
String principal = PropertiesUtil.getProperty(PRINCIPAL);
String keytabPath = PropertiesUtil.getProperty(KEYTAB);
Expand Down
10 changes: 5 additions & 5 deletions security-admin/src/main/resources/conf.dist/security-applicationContext.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,8 @@ http://www.springframework.org/schema/security/spring-security-oauth2-2.0.xsd">
<security:http pattern="/service/assets/policyList/*" security="none"/>
<security:http pattern="/service/assets/resources/grant" security="none"/>
<security:http pattern="/service/assets/resources/revoke" security="none"/>
<security:http pattern="/service/gds/download/*" security="none"/>
<security:http pattern="/service/plugins/policies/download/*" security="none"/>
<security:http pattern="/service/plugins/services/grant/*" security="none"/>
<security:http pattern="/service/plugins/services/revoke/*" security="none"/>
<security:http pattern="/service/tags/download/*" security="none"/>
<security:http pattern="/service/roles/download/*" security="none"/>
<security:http pattern="/service/xusers/download/*" security="none"/>
<security:http pattern="/service/actuator/health" security="none" />
<security:http pattern="/service/metrics/**" security="none" />
<security:http disable-url-rewriting="true" use-expressions="true" create-session="always" entry-point-ref="authenticationProcessingFilterEntryPoint">
Expand All @@ -63,6 +58,11 @@ http://www.springframework.org/schema/security/spring-security-oauth2-2.0.xsd">
<security:content-security-policy policy-directives="default-src 'none'; script-src 'self' 'unsafe-inline' 'unsafe-eval'; connect-src 'self'; img-src 'self' data:; style-src 'self' 'unsafe-inline';font-src 'self'"/>
</security:headers>
<security:session-management session-fixation-protection="newSession" />
<intercept-url pattern="/service/gds/download/**" access="permitAll"/>
<intercept-url pattern="/service/plugins/policies/download/**" access="permitAll"/>
<intercept-url pattern="/service/tags/download/**" access="permitAll"/>
<intercept-url pattern="/service/roles/download/**" access="permitAll"/>
<intercept-url pattern="/service/xusers/download/**" access="permitAll"/>
<intercept-url pattern="/**" access="isAuthenticated()"/>
<security:custom-filter position="PRE_AUTH_FILTER" ref="headerPreAuthFilter" />
<custom-filter ref="ssoAuthenticationFilter" after="BASIC_AUTH_FILTER" />
Expand Down
93 changes: 93 additions & 0 deletions ...rc/test/java/org/apache/ranger/security/web/filter/TestRangerKRBAuthenticationFilter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.doNothing;
import static org.mockito.Mockito.never;
Expand Down Expand Up @@ -473,4 +474,96 @@ public void testProtectedDoFilter_trustedProxy_successSetsAuthAndDelegates() thr
assertNotNull(SecurityContextHolder.getContext().getAuthentication());
verify(chain, times(1)).doFilter(req, res);
}

@Test
public void testIsOptionalAuthDownloadUrl_reflection() throws Exception {
RangerKRBAuthenticationFilter filter = new RangerKRBAuthenticationFilter();
Method m = RangerKRBAuthenticationFilter.class.getDeclaredMethod("isOptionalAuthDownloadUrl", HttpServletRequest.class);
m.setAccessible(true);
// matches: each of the five known prefixes, with and without a trailing slash
String[] matchingUris = {
"/service/gds/download/dev_hive",
"/service/gds/download/dev_hive/",
"/service/plugins/policies/download/dev_hive",
"/service/plugins/policies/download/dev_hive/",
"/service/tags/download/dev_hive",
"/service/roles/download/dev_hive",
"/service/xusers/download/dev_hive"
};
for (String uri : matchingUris) {
HttpServletRequest req = Mockito.mock(HttpServletRequest.class);
when(req.getRequestURI()).thenReturn(uri);
when(req.getContextPath()).thenReturn("");
boolean result = (Boolean) m.invoke(filter, req);
assertTrue(result, "expected match for URI: " + uri);
}

// non-match: a similarly-shaped but unrelated URL
HttpServletRequest nonMatching = Mockito.mock(HttpServletRequest.class);
when(nonMatching.getRequestURI()).thenReturn("/service/plugins/secure/policies/download/dev_hive");
when(nonMatching.getContextPath()).thenReturn("");
assertEquals(false, m.invoke(filter, nonMatching));

// non-match: unrelated path entirely
HttpServletRequest unrelated = Mockito.mock(HttpServletRequest.class);
when(unrelated.getRequestURI()).thenReturn("/service/public/api/repository/dev_hive");
when(unrelated.getContextPath()).thenReturn("");
assertEquals(false, m.invoke(filter, unrelated));

HttpServletRequest nullUri = Mockito.mock(HttpServletRequest.class);
assertEquals(false, m.invoke(filter, nullUri));

// context-path stripping: URI includes a context path prefix that must be stripped
// before matching against the download-URL prefixes
HttpServletRequest withContext = Mockito.mock(HttpServletRequest.class);
when(withContext.getRequestURI()).thenReturn("/ranger/service/tags/download/dev_hive");
when(withContext.getContextPath()).thenReturn("/ranger");
assertEquals(true, m.invoke(filter, withContext));
}

@Test
public void testDoFilter_optionalAuthDownloadUrl_noAuthHeader_bypassesSpnegoChallenge() throws Exception {
File kt = File.createTempFile("krb", ".keytab");
kt.deleteOnExit();
PropertiesUtil.getPropertiesMap().put("hadoop.security.authentication", "kerberos");
PropertiesUtil.getPropertiesMap().put("ranger.spnego.kerberos.principal", "HTTP/localhost@EXAMPLE.COM");
PropertiesUtil.getPropertiesMap().put("ranger.spnego.kerberos.keytab", kt.getAbsolutePath());
RangerKRBAuthenticationFilter filter = new RangerKRBAuthenticationFilter();
HttpServletRequest req = Mockito.mock(HttpServletRequest.class);
HttpServletResponse res = Mockito.mock(HttpServletResponse.class);
FilterChain chain = Mockito.mock(FilterChain.class);

when(req.getRequestURI()).thenReturn("/service/plugins/policies/download/dev_hive");
when(req.getContextPath()).thenReturn("");
when(req.getHeader("Authorization")).thenReturn(null);
filter.doFilter(req, res, chain);
verify(chain, times(1)).doFilter(any(ServletRequest.class), any(ServletResponse.class));
}

@Test
public void testDoFilter_optionalAuthDownloadUrl_withAuthorizationHeader_doesNotBypass() throws Exception {
File kt = File.createTempFile("krb", ".keytab");
kt.deleteOnExit();
PropertiesUtil.getPropertiesMap().put("hadoop.security.authentication", "kerberos");
PropertiesUtil.getPropertiesMap().put("ranger.spnego.kerberos.principal", "HTTP/localhost@EXAMPLE.COM");
PropertiesUtil.getPropertiesMap().put("ranger.spnego.kerberos.keytab", kt.getAbsolutePath());

RangerKRBAuthenticationFilter filter = new RangerKRBAuthenticationFilter();

HttpServletRequest req = Mockito.mock(HttpServletRequest.class);
HttpServletResponse res = Mockito.mock(HttpServletResponse.class);
FilterChain chain = Mockito.mock(FilterChain.class);

when(req.getRequestURI()).thenReturn("/service/plugins/policies/download/dev_hive");
when(req.getContextPath()).thenReturn("");
when(req.getHeader("Authorization")).thenReturn("Negotiate abcd1234");

try {
filter.doFilter(req, res, chain);
} catch (Exception expected) {
// real SPNEGO negotiation was attempted, as expected, and failed due to this
// minimal test setup not initializing the filter's SPNEGO internals.
}
verify(chain, never()).doFilter(any(ServletRequest.class), any(ServletResponse.class));
}
}