-
Notifications
You must be signed in to change notification settings - Fork 7
Support report-to CSP directive and Reporting-Endpoints header #7374
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
base: develop
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,6 +11,7 @@ | |
| import org.apache.commons.collections4.SetValuedMap; | ||
| import org.apache.commons.collections4.multimap.HashSetValuedHashMap; | ||
| import org.apache.commons.lang3.StringUtils; | ||
| import org.apache.commons.lang3.Strings; | ||
| import org.apache.logging.log4j.Logger; | ||
| import org.junit.Assert; | ||
| import org.junit.Test; | ||
|
|
@@ -66,6 +67,7 @@ public class ContentSecurityPolicyFilter implements Filter | |
| private ContentSecurityPolicyType _type = ContentSecurityPolicyType.Enforce; | ||
| private String _policyTemplate = null; | ||
| private String _cspVersion = "Unknown"; | ||
| private String _reportingEndpoints = null; | ||
|
|
||
| // Updated after every change to "allowed sources" | ||
| private StringExpression _policyExpression = null; | ||
|
|
@@ -104,7 +106,6 @@ public String getHeaderName() | |
| public void init(FilterConfig filterConfig) throws ServletException | ||
| { | ||
| LogHelper.getLogger(ContentSecurityPolicyFilter.class, "CSP filter initialization").info("Initializing {}", filterConfig.getFilterName()); | ||
|
|
||
| Enumeration<String> paramNames = filterConfig.getInitParameterNames(); | ||
| while (paramNames.hasMoreElements()) | ||
| { | ||
|
|
@@ -115,8 +116,7 @@ public void init(FilterConfig filterConfig) throws ServletException | |
| String s = filterPolicy(paramValue); | ||
|
|
||
| // Replace REPORT_PARAMETER_SUBSTITUTION now since its value is static | ||
| s = StringExpressionFactory.create(s, false, NullValueBehavior.KeepSubstitution) | ||
| .eval(Map.of(REPORT_PARAMETER_SUBSTITUTION, "labkeyVersion=" + PageFlowUtil.encodeURIComponent(AppProps.getInstance().getReleaseVersion()))); | ||
| s = substituteReportParams(s); | ||
|
|
||
| _policyTemplate = s; | ||
|
|
||
|
|
@@ -136,12 +136,32 @@ else if ("disposition".equalsIgnoreCase(paramName)) | |
| } | ||
| } | ||
|
|
||
| String baseServerUrl = AppProps.getInstance().getBaseServerUrl(); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this work when bootstrapping a server? And what happens if the base URL is changed after startup?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll check. Can probably switch to populating this lazily and any time the base server URL changes. |
||
| // Add "Reporting-Endpoints" header and "report-to" directive only if https: is configured on this server. This | ||
| // ensures that browsers fall-back on report-uri if https: isn't configured. | ||
| if (Strings.CI.startsWith(baseServerUrl, "https://")) | ||
| { | ||
| // Generate the Reporting-Endpoints header value now since its value is static. Use an absolute URL so we | ||
| // always post reports to https:, even when the violating request happens to be http: | ||
| String violationEndpoint = substituteReportParams(baseServerUrl + "/admin-contentSecurityPolicyReportTo.api?${CSP.REPORT.PARAMS}"); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could this use ActionURL, at least for the parts other than the substitution syntax? It could add the CSP version too, which could theoretically need URI encoding
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I tried, but it's quite awkward given the substitution has to be in the middle of the URL. I took another stab. I guess it's okay. |
||
| if (_cspVersion != null) | ||
| violationEndpoint += "&cspVersion=" + _cspVersion; | ||
| _reportingEndpoints = "csp-endpoint=\"" + violationEndpoint + "\""; | ||
labkey-adam marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| _policyTemplate = _policyTemplate + " report-to csp-endpoint ;"; | ||
| } | ||
|
|
||
| if (CSP_FILTERS.put(_type, this) != null) | ||
| throw new ServletException("ContentSecurityPolicyFilter is misconfigured, duplicate policies of type: " + _type); | ||
|
|
||
| regeneratePolicyExpression(); | ||
| } | ||
|
|
||
| private String substituteReportParams(String expression) | ||
| { | ||
| return StringExpressionFactory.create(expression, false, NullValueBehavior.KeepSubstitution) | ||
| .eval(Map.of(REPORT_PARAMETER_SUBSTITUTION, "labkeyVersion=" + PageFlowUtil.encodeURIComponent(AppProps.getInstance().getReleaseVersion()))); | ||
| } | ||
|
|
||
| /** Filter out block comments and replace special characters in the provided policy */ | ||
| public static String filterPolicy(String policy) | ||
| { | ||
|
|
@@ -224,6 +244,10 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha | |
| Map<String, String> map = Map.of(NONCE_SUBST, getScriptNonceHeader(req)); | ||
| var csp = _policyExpression.eval(map); | ||
| resp.setHeader(_type.getHeaderName(), csp); | ||
|
|
||
| // non-null if https: is configured on this server | ||
| if (_reportingEndpoints != null) | ||
| resp.setHeader("Reporting-Endpoints", _reportingEndpoints); | ||
| } | ||
| } | ||
| chain.doFilter(request, response); | ||
|
|
||
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.
Consider
CSP_V1andCSP_V3or similar, as they both seems like CSP reports.