[PR] ATLAS-5386: Harden CSP headers by removing unsafe directives (unsafe-… [atlas]

["chaitalicod (via GitHub)" <gi...@apache.org>]

chaitalicod opened a new pull request, #742:
URL: https://github.com/apache/atlas/pull/742

   …inline/unsafe-eval) and enabling nonce-based policy
   
   ## What changes were proposed in this pull request?
   
   (Please fill in changes proposed in this fix. Create an issue in ASF JIRA before opening a pull request and
   set the title of the pull request which starts with
   the corresponding JIRA issue number. (e.g. ATLAS-XXXX: Fix a typo in YYY))
   
   
   ## How was this patch tested?
   
   (Please explain how this patch was tested. Ex: unit tests, manual tests)
   (If this patch involves UI changes, please attach a screenshot; otherwise, remove this)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@atlas.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] ATLAS-5386: Harden CSP headers by removing unsafe directives (unsafe-… [atlas]

["pawarprasad123 (via GitHub)" <gi...@apache.org>]

pawarprasad123 commented on code in PR #742:
URL: https://github.com/apache/atlas/pull/742#discussion_r3862315159


##########
server-common/src/main/java/org/apache/atlas/server/common/filters/AtlasHeaderFilter.java:
##########
@@ -34,7 +34,10 @@ public void init(FilterConfig filterConfig) {
 
     @Override
     public void doFilter(ServletRequest request, ServletResponse response, FilterChain filterChain) throws IOException, ServletException {
-        setHeaders((HttpServletResponse) response);
+        String cspNonce = HeadersUtil.generateCspNonce();

Review Comment:
   (String cspNonce = HeadersUtil.generateCspNonce();)
   AtlasHeaderFilter generates its own nonce and overwrites the request attribute set by Spring Security's header writer. For admin endpoints that pass through both filters, the nonce in the CSP header may not match atlas.csp.nonce. Should read existing nonce from request attribute first, not regenerate.
   
   



##########
server-common/src/main/java/org/apache/atlas/server/common/security/AtlasSecurityConfig.java:
##########
@@ -129,7 +129,15 @@ protected void configureCommonHttpSecurity(HttpSecurity httpSecurity) throws Exc
                 .authorizeRequests().anyRequest().authenticated()
                 .and()
                 .headers()
-                .addHeaderWriter(new StaticHeadersWriter(HeadersUtil.CONTENT_SEC_POLICY_KEY, HeadersUtil.getHeaderMap(HeadersUtil.CONTENT_SEC_POLICY_KEY)))
+                .addHeaderWriter((request, response) -> {

Review Comment:
   String cspNonce = HeadersUtil.generateCspNonce();)
   
   -This generates a new nonce independently of other filters. If any downstream filter calls setSecurityHeaders(wrapper) without passing this same nonce, the header and request attribute will diverge. Use a shared getOrCreateNonce(request) helper and ensure all header-writing paths consume the same value.
   
   



##########
webapp/src/test/java/org/apache/atlas/web/filters/HeaderUtilsTest.java:
##########
@@ -98,6 +100,18 @@ public void testDefaultHeadersArePresent() {
         assertEquals("1; mode=block", HeadersUtil.getHeaderMap(HeadersUtil.X_XSS_PROTECTION_KEY));
     }
 
+    @Test
+    public void testContentSecurityPolicyValueUsesNonce() {

Review Comment:
   (testContentSecurityPolicyValueUsesNonce)
   
   Good start. Please also add tests in server-common/.../HeadersUtilTest.java for: (1) custom CSP override with and without ${nonce}, (2) generateCspNonce() produces unique nonces, (3) both script-src and style-src placeholders are replaced, (4) null/empty nonce fallback behavior.
   
   



##########
server-common/src/main/java/org/apache/atlas/server/common/filters/AtlasHeaderFilter.java:
##########
@@ -43,8 +46,12 @@ public void destroy() {
     }
 
     public void setHeaders(HttpServletResponse httpResponse) {

Review Comment:
   (public void setHeaders(HttpServletResponse httpResponse))
   This overload calls `generateCspNonce()` again, producing yet another nonce if invoked directly. Consider removing this pattern or delegating to the request-scoped nonce to avoid accidental mismatch in tests or future callers.



##########
server-common/src/main/java/org/apache/atlas/server/common/filters/HeadersUtil.java:
##########
@@ -71,8 +77,40 @@ public static void setHeaderMapAttributes(AtlasResponseRequestWrapper responseWr
         responseWrapper.setHeader(headerKey, HEADER_MAP.get(headerKey));
     }
 
+    public static String generateCspNonce() {
+        byte[] nonceBytes = new byte[CSP_NONCE_BYTES];
+
+        CSP_NONCE_RANDOM.nextBytes(nonceBytes);
+
+        return Base64.getEncoder().withoutPadding().encodeToString(nonceBytes);
+    }
+
+    public static String getContentSecurityPolicyValue(String cspNonce) {
+        String cspTemplate = HEADER_MAP.get(CONTENT_SEC_POLICY_KEY);
+
+        if (cspTemplate == null) {
+            return null;
+        }
+
+        if (cspNonce == null || cspNonce.trim().isEmpty()) {
+            cspNonce = generateCspNonce();
+        }
+
+        return cspTemplate.replace(CONTENT_SEC_POLICY_NONCE_PLACEHOLDER, cspNonce);
+    }
+
+    public static void setSecurityHeaders(AtlasResponseRequestWrapper responseWrapper, String cspNonce) {
+        HEADER_MAP.forEach((key, value) -> {
+            if (CONTENT_SEC_POLICY_KEY.equals(key)) {
+                responseWrapper.setHeader(key, getContentSecurityPolicyValue(cspNonce));
+            } else {
+                responseWrapper.setHeader(key, value);
+            }
+        });
+    }
+

Review Comment:
    setSecurityHeaders(AtlasResponseRequestWrapper responseWrapper)
   -
   setSecurityHeaders(wrapper) generates a fresh nonce on every call. This is invoked from AtlasAuthenticationFilter, AtlasCSRFPreventionFilter, AtlasHeaderPreAuthFilter, AtlasKnoxSSOAuthenticationFilter, and AtlasHttpServlet, each potentially overwriting the CSP header set by AtlasSecurityConfig with a different nonce. Please add getOrCreateNonce(HttpServletRequest request) that generates once per request (stored in CONTENT_SEC_POLICY_NONCE_REQUEST_ATTRIBUTE) and reuse it in all call sites.



##########
server-common/src/main/java/org/apache/atlas/server/common/filters/HeadersUtil.java:
##########
@@ -40,11 +42,13 @@ public class HeadersUtil {
     public static final String X_XSS_PROTECTION_KEY                = "X-XSS-Protection";
     public static final String STRICT_TRANSPORT_SEC_KEY            = "Strict-Transport-Security";
     public static final String CONTENT_SEC_POLICY_KEY              = "Content-Security-Policy";
+    public static final String CONTENT_SEC_POLICY_NONCE_PLACEHOLDER = "${nonce}";
+    public static final String CONTENT_SEC_POLICY_NONCE_REQUEST_ATTRIBUTE = "atlas.csp.nonce";
     public static final String X_FRAME_OPTIONS_VAL                 = "DENY";
     public static final String X_CONTENT_TYPE_OPTIONS_VAL          = "nosniff";
     public static final String X_XSS_PROTECTION_VAL                = "1; mode=block";
     public static final String STRICT_TRANSPORT_SEC_VAL            = "max-age=31536000; includeSubDomains";
-    public static final String CONTENT_SEC_POLICY_VAL              = "default-src 'self'; script-src 'self' 'unsafe-inline' 'unsafe-eval' blob: data:; connect-src 'self'; img-src 'self' blob: data:; style-src 'self' 'unsafe-inline';font-src 'self' data:";
+    public static final String CONTENT_SEC_POLICY_VAL              = "default-src 'self'; script-src 'self' 'nonce-${nonce}' blob:; connect-src 'self'; img-src 'self' blob: data:; style-src 'self' 'nonce-${nonce}'; font-src 'self' data:; object-src 'none'; base-uri 'self'; frame-ancestors 'none';";

Review Comment:
   (CONTENT_SEC_POLICY_VAL)
   
   Policy change looks good — removes unsafe directives and adds nonce placeholders for both `script-src` and `style-src`.
   
   Note: this policy alone will block runtime `<style>` tags (e.g. MUI/Emotion in React Dashboard) unless:
   1. HTML responses inject matching `nonce="..."` on script/style tags, and
   2. The React UI reads the nonce and passes it to Emotion cache.
   
   The HTML injection pipeline is not part of this PR — please confirm the follow-up plan.



##########
server-common/src/main/java/org/apache/atlas/server/common/filters/AtlasHeaderFilter.java:
##########
@@ -34,7 +34,10 @@ public void init(FilterConfig filterConfig) {
 
     @Override
     public void doFilter(ServletRequest request, ServletResponse response, FilterChain filterChain) throws IOException, ServletException {
-        setHeaders((HttpServletResponse) response);
+        String cspNonce = HeadersUtil.generateCspNonce();

Review Comment:
   This is general:
   Missing filter updates (general comment)
   The following files still call HeadersUtil.setSecurityHeaders(responseWrapper) without a request-scoped nonce and are not updated in this PR:
   
   - AtlasAuthenticationFilter.java:397
   - AtlasCSRFPreventionFilter.java:107
   - AtlasHeaderPreAuthFilter.java:111
   - AtlasKnoxSSOAuthenticationFilter.java:126
   - AtlasHttpServlet.java:39
   These need to pass the same per-request nonce to avoid header/attribute mismatch.



##########
server-common/src/main/java/org/apache/atlas/server/common/filters/HeadersUtil.java:
##########
@@ -71,8 +77,40 @@ public static void setHeaderMapAttributes(AtlasResponseRequestWrapper responseWr
         responseWrapper.setHeader(headerKey, HEADER_MAP.get(headerKey));
     }
 
+    public static String generateCspNonce() {
+        byte[] nonceBytes = new byte[CSP_NONCE_BYTES];
+
+        CSP_NONCE_RANDOM.nextBytes(nonceBytes);
+
+        return Base64.getEncoder().withoutPadding().encodeToString(nonceBytes);
+    }
+
+    public static String getContentSecurityPolicyValue(String cspNonce) {

Review Comment:
   (getContentSecurityPolicyValue(String cspNonce))
   -
   When atlas.headers.Content-Security-Policy is overridden via config without the ${nonce} placeholder, this method silently returns a policy with no nonce. Consider logging a warning when the template doesn't contain CONTENT_SEC_POLICY_NONCE_PLACEHOLDER, or rejecting unsafe custom overrides.
   
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@atlas.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org