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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
