-
Notifications
You must be signed in to change notification settings - Fork 813
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
WW-5084: Add Content Security Policy support to Struts #430
Conversation
Support for CSP in Struts 2: - Implements a CSP Interceptor that adds a nonce-based, strict-dynamic policy to HTTP responses. - Implements custom JSP and FTL <script> tags that add nonces to script blocks automatically. This makes these tags compatible with CSP with minimal refactoring. - Implements an extensible action that can be used to collect CSP reports out of the box. This behaviour is extensible, so developers can customise the processing of CSP reports. Co-authored-by: Ecenaz Jen Ozmen <[email protected]> Co-authored-by: Giannis Chatziveroglou <[email protected]> Co-authored-by: Sal <[email protected]>
This PR is associated with a follow up that refactors existing FTL and JSP files in Struts to make them CSP-ready. This will make it much easier to adopt CSP on an existing application. Please find a draft of this PR here: salcho#6 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 💯
That's great news @lukaszlenart! Is there anything else we can do to get these merged? :) |
I'm just giving some time to others to allow them review your work :) |
@salcho @lukaszlenart Do we need |
core/src/main/java/org/apache/struts2/action/CspReportAction.java
Outdated
Show resolved
Hide resolved
Hi @aleksandr-m @lukaszlenart, Re: CspReportAction. In our experience, many developers are put off of adopting CSP in their applications because collecting reports adds more overhead. We decided to add the Alex, I've pushed a new commit that addresses your comment. Thanks for flagging! |
core/src/main/java/org/apache/struts2/interceptor/csp/CspInterceptor.java
Outdated
Show resolved
Hide resolved
core/src/main/java/org/apache/struts2/action/DefaultCspReportAction.java
Show resolved
Hide resolved
…ult, CSP will be in reporting mode with no report directive.
… method, content length and content type are processed and response code is always 204.
Friendly ping! @aleksandr-m @lukaszlenart |
I'm fine with the current state, LGTM 👍 (but I will let @aleksandr-m hit the button :) |
@lukaszlenart Hi, for <s: script><s:/script>, if the src of the tags needs variables, what should I do? |
@TheFergus already answered here struts-community-plugins/struts2-jquery#318 (comment) |
Hello Struts Devs!
This PR adds Content Security Policy support for Struts 2. A very popular security mitigation against XSS and other injection vulnerabilities. CSP comes in many flavours, but we've chosen to add support for the most robust of them: nonce-based, strict-dynamic CSP.
Here's a summary of these changes:
<script>
and<link>
JSP and FTL tags. These (<s:script> in taglib, for instance) set the nonce attribute on script and link blocks automatically, so that they match the nonce set in the policy. This feature allows developers to use both existing and new script blocks that are compatible with CSP with minimal refactoring.Co-authored-by: Ecenaz Jen Ozmen - [email protected]
Co-authored-by: Giannis Chatziveroglou - [email protected]
Co-authored-by: Santiago Diaz - [email protected]