Skip to content

Clarification on connection of Content-Security-Policy header with HttpSecurity DSL support #14092

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.

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

Closed
sandipchitale opened this issue Nov 2, 2023 · 4 comments
Assignees
Labels
status: invalid An issue that we don't feel is valid type: enhancement A general enhancement

Comments

@sandipchitale
Copy link

sandipchitale commented Nov 2, 2023

According to MDN we can set the CSP protection via HTML like so.

<meta
  http-equiv="Content-Security-Policy"
  content="default-src 'self https://*;" />

This tag goes in the head section of the web page. Which seems to imply that this is only needed in the html page being loaded in the browser (tab, frame, iframe).

Spring security supports this like this:

httpSecurity.headers((HeadersConfigurer<HttpSecurity> httpSecurityHeadersConfigurer) -> {
            httpSecurityHeadersConfigurer.contentSecurityPolicy(
                    (HeadersConfigurer<HttpSecurity>.ContentSecurityPolicyConfig contentSecurityPolicyConfig) -> {
                contentSecurityPolicyConfig.policyDirectives("default-src 'self' https://*");
            });
        });

which means that the Content-Security-Policy header is sent back on all responses irrespective content-type under the security matcher associated with httpSecurity. It seems wasteful to send the header on every response.

Could this be implemented instead by a global filter?

    @Component
    @Order(Ordered.HIGHEST_PRECEDENCE)
    public static class CSPFilter extends OncePerRequestFilter {

        @Override
        protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain filterChain) throws ServletException, IOException {
            response.setHeader("Content-Security-Policy", "default-src 'self' https://*");
            filterChain.doFilter(request, response);
        }
    }

I know I can do this already. But would like to ask - is there a reason I am not thinking of that this does need to be implemented in httpSecurity DSL?

Could Spring Security provide a way to do this declaratively as a global filter instead? Or could this be implemented in a way that it is integrated with html (html generating mechanisms like .jsp)?

Context

If I have multiple httpSecurity in my application I have to configure CSP handling for each.

@sandipchitale sandipchitale added status: waiting-for-triage An issue we've not yet triaged type: enhancement A general enhancement labels Nov 2, 2023
@marcusdacoregio marcusdacoregio self-assigned this Nov 3, 2023
@marcusdacoregio marcusdacoregio removed the status: waiting-for-triage An issue we've not yet triaged label Nov 3, 2023
@marcusdacoregio
Copy link
Contributor

marcusdacoregio commented Nov 3, 2023

Hello, @sandipchitale.

I know I can do this already. But would like to ask - is there a reason I am not thinking of that this does need to be implemented in httpSecurity DSL?

Spring Security will only add its filters on the SecurityFilterChain and won't touch the global filters, since you can have multiple SecurityFilterChains, the configuration has to happen on HttpSecurity.

Could Spring Security provide a way to do this declaratively as a global filter instead? Or could this be implemented in a way that it is integrated with html (html generating mechanisms like .jsp)?

I don't think a global filter would be considered here for the reasons I mentioned before. You can specify your own HeaderWriter that checks the Content-Type before adding the header, something like:

public final class HtmlContentSecurityPolicyHeaderWriter implements HeaderWriter {

  private final ContentSecurityPolicyHeaderWriter delegate = new ContentSecurityPolicyHeaderWriter("default-src 'self' https://*");

  @Override
  public void writeHeaders(HttpServletRequest request, HttpServletResponse response) {
    boolean isHtml = MediaType.TEXT_HTML.equals(MimeType.valueOf(response.getContentType()));
    if (isHtml) {
      this.delegate.writeHeaders(request, response);
    }
  }

}

and then:

@Bean
SecurityFilterChain filterChain(HttpSecurity http) throws Exception {
	http
		.headers((headers) -> headers
			.addHeaderWriter(new HtmlContentSecurityPolicyHeaderWriter())
		);
	return http.build();
}

Or you can even implement your own global filter and add that condition to it.

Does that make sense?

@marcusdacoregio marcusdacoregio added the status: waiting-for-feedback We need additional information before we can continue label Nov 3, 2023
@sandipchitale
Copy link
Author

sandipchitale commented Nov 3, 2023

@marcusdacoregio Thanks for the clarification. The above makes sense. So to be more optimal in terms of sending the CSP response header only on the responses that make sense, the approach you suggest could be better:

public final class HtmlContentSecurityPolicyHeaderWriter implements HeaderWriter {

  private final ContentSecurityPolicyHeaderWriter delegate = new ContentSecurityPolicyHeaderWriter("default-src 'self' https://*");

  @Override
  public void writeHeaders(HttpServletRequest request, HttpServletResponse response) {
    boolean isHtml = MediaType.TEXT_HTML.equals(MimeType.valueOf(response.getContentType()));
    if (isHtml) {
      this.delegate.writeHeaders(request, response);
    }
  }
}

QUESTION: Are there any issue related to if the response has been commited and header not getting written out?

compared to using the DSL:

httpSecurityHeadersConfigurer.contentSecurityPolicy(
                    (HeadersConfigurer<HttpSecurity>.ContentSecurityPolicyConfig contentSecurityPolicyConfig) -> {
                contentSecurityPolicyConfig.policyDirectives("default-src 'self' https://*");
            });

Unless there is some other reason I am not thinking of that currently CSP header is sent on all responses if one uses DSL.

May be a note could be added to the section of the documentation related to CSP handling suggesting potentially more optimal alternative.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Nov 3, 2023
@marcusdacoregio
Copy link
Contributor

marcusdacoregio commented Nov 3, 2023

QUESTION: Are there any issue related to if the response has been commited and header not getting written out?

I suggest that you read #6501 and the related issues to get more information about that.

An even better solution for your use case is documented here. By using the DelegatingRequestMatcherHeaderWriter you can simplify the code I mentioned above to:

@Bean
public SecurityFilterChain filterChain(HttpSecurity http) throws Exception {
	RequestMatcher matcher = new MediaTypeRequestMatcher(MediaType.TEXT_HTML);
	DelegatingRequestMatcherHeaderWriter headerWriter =
		new DelegatingRequestMatcherHeaderWriter(matcher, new ContentSecurityPolicyHeaderWriter("default-src 'self' https://*"));
	http
		// ...
		.headers(headers -> headers
			.addHeaderWriter(headerWriter)
		);
	return http.build();
}

That way you only use classes that Spring Security provides and avoid having to write custom implementations.

Since the documentation covers that, I'll close this as resolved.

@marcusdacoregio marcusdacoregio added status: invalid An issue that we don't feel is valid and removed status: feedback-provided Feedback has been provided labels Nov 3, 2023
@sandipchitale
Copy link
Author

sandipchitale commented Nov 3, 2023

Minor point. We want to write the CSP header only when response content type is text/html, not request media type. So the above based on DelegatingRequestMatcherHeaderWriter will not work. Thanks anyway for your help with HtmlContentSecurityPolicyHeaderWriter .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: invalid An issue that we don't feel is valid type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

3 participants