From d55fb91b0f2f62b081a8732e66dbbd3ea66961a6 Mon Sep 17 00:00:00 2001 From: exceptionfactory Date: Wed, 23 Nov 2022 16:30:20 -0600 Subject: [PATCH] NIFI-10871 Skipped CSRF processing for replicated HTTP requests - Updated Security Filter Configuration to avoid unnecessary CSRF Request Token generation for requests replicated between cluster nodes This closes #6715. Signed-off-by: Tamas Palfy --- .../web/NiFiWebApiSecurityConfiguration.java | 2 + .../csrf/SkipReplicatedCsrfFilter.java | 61 ++++++++++ .../csrf/SkipReplicatedCsrfFilterTest.java | 113 ++++++++++++++++++ 3 files changed, 176 insertions(+) create mode 100644 nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-security/src/main/java/org/apache/nifi/web/security/csrf/SkipReplicatedCsrfFilter.java create mode 100644 nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-security/src/test/java/org/apache/nifi/web/security/csrf/SkipReplicatedCsrfFilterTest.java diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/NiFiWebApiSecurityConfiguration.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/NiFiWebApiSecurityConfiguration.java index d43824d122..2108b09abe 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/NiFiWebApiSecurityConfiguration.java +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/NiFiWebApiSecurityConfiguration.java @@ -20,6 +20,7 @@ import org.apache.nifi.util.NiFiProperties; import org.apache.nifi.web.security.StandardAuthenticationEntryPoint; import org.apache.nifi.web.security.anonymous.NiFiAnonymousAuthenticationFilter; import org.apache.nifi.web.security.csrf.CsrfCookieRequestMatcher; +import org.apache.nifi.web.security.csrf.SkipReplicatedCsrfFilter; import org.apache.nifi.web.security.csrf.StandardCookieCsrfTokenRepository; import org.apache.nifi.web.security.knox.KnoxAuthenticationFilter; import org.apache.nifi.web.security.log.AuthenticationUserFilter; @@ -109,6 +110,7 @@ public class NiFiWebApiSecurityConfiguration { ).permitAll() .anyRequest().authenticated() ) + .addFilterBefore(new SkipReplicatedCsrfFilter(), CsrfFilter.class) .csrf(csrf -> csrf .csrfTokenRepository( new StandardCookieCsrfTokenRepository() diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-security/src/main/java/org/apache/nifi/web/security/csrf/SkipReplicatedCsrfFilter.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-security/src/main/java/org/apache/nifi/web/security/csrf/SkipReplicatedCsrfFilter.java new file mode 100644 index 0000000000..e3637fdb55 --- /dev/null +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-security/src/main/java/org/apache/nifi/web/security/csrf/SkipReplicatedCsrfFilter.java @@ -0,0 +1,61 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.nifi.web.security.csrf; + +import org.springframework.security.web.csrf.CsrfFilter; +import org.springframework.security.web.util.matcher.AndRequestMatcher; +import org.springframework.security.web.util.matcher.NegatedRequestMatcher; +import org.springframework.security.web.util.matcher.RequestHeaderRequestMatcher; +import org.springframework.security.web.util.matcher.RequestMatcher; +import org.springframework.web.filter.OncePerRequestFilter; + +import javax.servlet.FilterChain; +import javax.servlet.ServletException; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import java.io.IOException; + +/** + * Skip Replicated Cross-Site Request Forgery Filter disables subsequent filtering for matched requests + */ +public class SkipReplicatedCsrfFilter extends OncePerRequestFilter { + /** RequestReplicator.REQUEST_TRANSACTION_ID_HEADER applied to replicated cluster requests */ + protected static final String REPLICATED_REQUEST_HEADER = "X-RequestTransactionId"; + + /** Requests containing replicated header and not containing authorization cookies will be skipped */ + private static final RequestMatcher REQUEST_MATCHER = new AndRequestMatcher( + new RequestHeaderRequestMatcher(REPLICATED_REQUEST_HEADER), + new NegatedRequestMatcher(new CsrfCookieRequestMatcher()) + ); + + /** + * Set request attribute to disable standard CSRF Filter when request matches + * + * @param request HTTP Servlet Request + * @param response HTTP Servlet Response + * @param filterChain Filter Chain + * @throws ServletException Thrown on FilterChain.doFilter() + * @throws IOException Thrown on FilterChain.doFilter() + */ + @Override + protected void doFilterInternal(final HttpServletRequest request, final HttpServletResponse response, final FilterChain filterChain) throws ServletException, IOException { + if (REQUEST_MATCHER.matches(request)) { + CsrfFilter.skipRequest(request); + } + filterChain.doFilter(request, response); + } +} diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-security/src/test/java/org/apache/nifi/web/security/csrf/SkipReplicatedCsrfFilterTest.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-security/src/test/java/org/apache/nifi/web/security/csrf/SkipReplicatedCsrfFilterTest.java new file mode 100644 index 0000000000..a2e0727c44 --- /dev/null +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-security/src/test/java/org/apache/nifi/web/security/csrf/SkipReplicatedCsrfFilterTest.java @@ -0,0 +1,113 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.nifi.web.security.csrf; + +import org.apache.nifi.web.security.http.SecurityCookieName; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.http.HttpMethod; +import org.springframework.mock.web.MockHttpServletRequest; +import org.springframework.mock.web.MockHttpServletResponse; +import org.springframework.security.web.csrf.CsrfFilter; + +import javax.servlet.FilterChain; +import javax.servlet.ServletException; +import javax.servlet.http.Cookie; + +import java.io.IOException; +import java.util.UUID; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.verify; + +@ExtendWith(MockitoExtension.class) +class SkipReplicatedCsrfFilterTest { + private static final String SHOULD_NOT_FILTER = String.format("SHOULD_NOT_FILTER%s", CsrfFilter.class.getName()); + + MockHttpServletRequest httpServletRequest; + + MockHttpServletResponse httpServletResponse; + + @Mock + FilterChain filterChain; + + SkipReplicatedCsrfFilter filter; + + @BeforeEach + void setHandler() { + filter = new SkipReplicatedCsrfFilter(); + httpServletRequest = new MockHttpServletRequest(); + httpServletResponse = new MockHttpServletResponse(); + } + + @Test + void testDoFilterInternalNotSkipped() throws ServletException, IOException { + httpServletRequest.setMethod(HttpMethod.GET.name()); + + filter.doFilterInternal(httpServletRequest, httpServletResponse, filterChain); + + assertCsrfFilterNotSkipped(); + } + + @Test + void testDoFilterInternalBearerCookieNotSkipped() throws ServletException, IOException { + httpServletRequest.setMethod(HttpMethod.GET.name()); + final Cookie bearerCookie = new Cookie(SecurityCookieName.AUTHORIZATION_BEARER.getName(), UUID.randomUUID().toString()); + httpServletRequest.setCookies(bearerCookie); + + filter.doFilterInternal(httpServletRequest, httpServletResponse, filterChain); + + assertCsrfFilterNotSkipped(); + } + + @Test + void testDoFilterInternalReplicatedHeaderAndBearerCookieNotSkipped() throws ServletException, IOException { + httpServletRequest.setMethod(HttpMethod.GET.name()); + httpServletRequest.addHeader(SkipReplicatedCsrfFilter.REPLICATED_REQUEST_HEADER, UUID.randomUUID().toString()); + final Cookie bearerCookie = new Cookie(SecurityCookieName.AUTHORIZATION_BEARER.getName(), UUID.randomUUID().toString()); + httpServletRequest.setCookies(bearerCookie); + + filter.doFilterInternal(httpServletRequest, httpServletResponse, filterChain); + + assertCsrfFilterNotSkipped(); + } + + @Test + void testDoFilterInternalReplicatedHeaderSkipped() throws ServletException, IOException { + httpServletRequest.setMethod(HttpMethod.GET.name()); + httpServletRequest.addHeader(SkipReplicatedCsrfFilter.REPLICATED_REQUEST_HEADER, UUID.randomUUID().toString()); + + filter.doFilterInternal(httpServletRequest, httpServletResponse, filterChain); + + final Object shouldNotFilter = httpServletRequest.getAttribute(SHOULD_NOT_FILTER); + + assertEquals(Boolean.TRUE, shouldNotFilter); + verify(filterChain).doFilter(eq(httpServletRequest), eq(httpServletResponse)); + } + + private void assertCsrfFilterNotSkipped() throws ServletException, IOException { + final Object shouldNotFilter = httpServletRequest.getAttribute(SHOULD_NOT_FILTER); + + assertNotEquals(Boolean.TRUE, shouldNotFilter); + verify(filterChain).doFilter(eq(httpServletRequest), eq(httpServletResponse)); + } +}