NIFI-10871 Skip CSRF processing for replicated HTTP requests #6715
Conversation
- Updated Security Filter Configuration to avoid unnecessary CSRF Request Token generation for requests replicated between cluster nodes
| @Override | ||
| protected void doFilterInternal(final HttpServletRequest request, final HttpServletResponse response, final FilterChain filterChain) throws ServletException, IOException { | ||
| if (REQUEST_MATCHER.matches(request)) { | ||
| CsrfFilter.skipRequest(request); |
There was a problem hiding this comment.
Wondering, won't it become too easy to mount a CSRF attack by adding such a header to a forged request?
There was a problem hiding this comment.
Thanks for the feedback @tpalfy. Setting a custom HTTP header requires running custom JavaScript code, which is not possible from standard CSRF attacks.
I pushed a change to also check for the absence of the __Secure-Authorization-Bearer Cookie as part of the Request Matcher. This means that requests containing the Bearer Cookie will trigger the standard CSRF Filter regardless of the presence of the Replicated Header. This serves as an additional sanity check, and still works for replicated requests, because the cluster Request Replicator removes the Bearer Cookie before sending to other nodes.
There was a problem hiding this comment.
I'm okay with it. Just noting that IF the server is/becomes vulnerable to XSS, with this change it may automatically become vulnerable to CSRF as well.
There was a problem hiding this comment.
A Cross-Site Scripting vulnerability would be able to make use of existing JavaScript request processing that adds the custom request token header to all requests. The __Secure-Authorization-Bearer token is not accessible from JavaScript, so the impact of an XSS attack is somewhat limited in that regard. The introduction of this new filter for skipping replicated requests should not alter the impact of a theoretical XSS vulnerability.
| @Override | ||
| protected void doFilterInternal(final HttpServletRequest request, final HttpServletResponse response, final FilterChain filterChain) throws ServletException, IOException { | ||
| if (REQUEST_MATCHER.matches(request)) { | ||
| CsrfFilter.skipRequest(request); |
There was a problem hiding this comment.
I would suggest you to mark forged requests in logs using MDC. So that any activity in a NiFi instance resulted by a forged request could have a mark like [forged] in the log entry. This could improve clarity.
There was a problem hiding this comment.
Thanks for the feedback @kulikg. The standard Spring Security CsrfFilter will throw a specialized CSRF Token Exception for requests with mismatching tokens. Enabling Spring Security logging provides additional details, and if some other tracking is necessary, it seems like that would be better to address in a separate issue.
There was a problem hiding this comment.
I took a look and it seems MDC is not introduced yet. We may consider to add it later on.
|
Thanks @exceptionfactory for your work! |
- 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 <tpalfy@apache.org>
- Updated Security Filter Configuration to avoid unnecessary CSRF Request Token generation for requests replicated between cluster nodes This closes apache#6715. Signed-off-by: Tamas Palfy <tpalfy@apache.org>
Summary
NIFI-10871 Adds a new Servlet Filter to the application Spring Security Filter Chain configuration for skipping Cross-Site Request Forgery processing on HTTP requests replicated between cluster nodes.
The introduction of CSRF mitigation using the standard Spring Security
CsrfFiltercauses the application to generate and set a__Secure-Request-Tokencookie on HTTP responses. This is not necessary for requests replicated from one cluster node to another. As a result of the current behavior, requests to cluster nodes can return intermittent HTTP 403 responses when a client web browser sends mismatched__Secure-Request-TokenCookie andRequest-TokenHeader values.Skipping CSRF token generation for cluster replicated requests avoids introducing a new value for the
__Secure-Request-TokenCookie, which resolves intermittent HTTP 403 responses in clustered deployments. The newSkipReplicatedCsrfFiltermatches HTTP requests based on the presence of the customX-RequestTransactionId, which the standard Request Replicator applies to replicated requests.Tracking
Please complete the following tracking steps prior to pull request creation.
Issue Tracking
Pull Request Tracking
NIFI-00000NIFI-00000Pull Request Formatting
mainbranchVerification
Please indicate the verification steps performed prior to pull request creation.
Build
mvn clean install -P contrib-checkLicensing
LICENSEandNOTICEfilesDocumentation