Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 6 additions & 19 deletions api/src/org/labkey/filters/ContentSecurityPolicyFilter.java
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ public class ContentSecurityPolicyFilter implements Filter
private static final Logger LOG = LogHelper.getLogger(ContentSecurityPolicyFilter.class, "Register/unregister allowed resource hosts");

private static final String NONCE_SUBST = "REQUEST.SCRIPT.NONCE";
private static final String REPORT_PARAMETER_SUBSTITUTION = "CSP.REPORT.PARAMS";
private static final String UPGRADE_INSECURE_REQUESTS_SUBSTITUTION = "UPGRADE.INSECURE.REQUESTS";
private static final String HEADER_NONCE = "org.labkey.filters.ContentSecurityPolicyFilter#NONCE"; // needs to match PageConfig.HEADER_NONCE

Expand Down Expand Up @@ -120,14 +119,8 @@ public void init(FilterConfig filterConfig) throws ServletException
String paramValue = filterConfig.getInitParameter(paramName);
if ("policy".equalsIgnoreCase(paramName))
{
String s = filterPolicy(paramValue);

// Replace REPORT_PARAMETER_SUBSTITUTION now since its value is static
s = substituteReportParams(s);

_stashedTemplate = s;

extractCspVersion(s);
_stashedTemplate = filterPolicy(paramValue);
extractCspVersion(_stashedTemplate);
}
else if ("disposition".equalsIgnoreCase(paramName))
{
Expand All @@ -150,12 +143,6 @@ else if ("disposition".equalsIgnoreCase(paramName))
_reportToEndpointName = "csp-" + getType().name().toLowerCase();
}

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)
{
Expand Down Expand Up @@ -292,7 +279,7 @@ private CspFilterSettings(ContentSecurityPolicyFilter filter, String baseServerU
@SuppressWarnings("DataFlowIssue")
ActionURL violationUrl = PageFlowUtil.urlProvider(AdminUrls.class).getCspReportToURL(filter.getCspVersion());
// Use an absolute URL so we always post to https:, even if the violating request uses http:
_reportingEndpointsHeaderValue = filter.getReportToEndpointName() + "=\"" + filter.substituteReportParams(violationUrl.getURIString() + "&${CSP.REPORT.PARAMS}") + "\"";
_reportingEndpointsHeaderValue = filter.getReportToEndpointName() + "=\"" + violationUrl.getURIString() + "\"";

// Add "report-to" directive to the policy
_policyTemplate = filter.getStashedTemplate() + " report-to " + filter.getReportToEndpointName() + " ;";
Expand All @@ -305,15 +292,15 @@ private CspFilterSettings(ContentSecurityPolicyFilter filter, String baseServerU

_previousBaseServerUrl = baseServerUrl;

final String allowSubstitutedPolicy;
final String substitutedPolicy;

synchronized (SUBSTITUTION_LOCK)
{
allowSubstitutedPolicy = StringExpressionFactory.create(_policyTemplate, false, NullValueBehavior.KeepSubstitution)
substitutedPolicy = StringExpressionFactory.create(_policyTemplate, false, NullValueBehavior.KeepSubstitution)
.eval(SUBSTITUTION_MAP);
}

_policyExpression = StringExpressionFactory.create(allowSubstitutedPolicy, false, NullValueBehavior.ReplaceNullAndMissingWithBlank);
_policyExpression = StringExpressionFactory.create(substitutedPolicy, false, NullValueBehavior.ReplaceNullAndMissingWithBlank);
}

public String getPolicyTemplate()
Expand Down
13 changes: 10 additions & 3 deletions core/src/org/labkey/core/admin/AdminController.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import com.fasterxml.jackson.annotation.JsonIgnore;
import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.DeserializationFeature;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.google.common.base.Joiner;
import com.google.common.util.concurrent.UncheckedExecutionException;
Expand Down Expand Up @@ -12032,6 +12033,14 @@ public void handleReports(ReportToJsonObjects jsonObjects, HttpServletRequest re
if (!reportsToForward.isEmpty())
forwardReports(LABKEY_ORG_REPORT_TO_ACTION, request, reportsToForward.toString(2));
}

@Override
protected ObjectMapper createRequestObjectMapper()
{
// Annoyingly, Chrome posts an array of JSON objects but Safari posts individual JSON objects. Set a flag
// that ensures both cases deserialize into List<JSONObject>.
return JsonUtil.DEFAULT_MAPPER.copy().enable(DeserializationFeature.ACCEPT_SINGLE_VALUE_AS_ARRAY);
}
}

@RequiresNoPermission
Expand Down Expand Up @@ -12125,6 +12134,7 @@ protected boolean handleOneReport(JSONObject jsonObj, HttpServletRequest request
boolean forwarded = jsonObj.optBoolean("forwarded", false);
if (!forwarded)
{
jsonObj.put("labkeyVersion", AppProps.getInstance().getReleaseVersion());
User user = getUser();
String email = null;
// If the user is not logged in, we may still be able to snag the email address from our cookie
Expand All @@ -12139,9 +12149,6 @@ protected boolean handleOneReport(JSONObject jsonObj, HttpServletRequest request
jsonObj.put("ip", ipAddress);
if (isNotBlank(userAgent) && !jsonObj.has("user_agent"))
jsonObj.put("user_agent", userAgent);
String labkeyVersion = request.getParameter("labkeyVersion");
if (null != labkeyVersion)
jsonObj.put("labkeyVersion", labkeyVersion);
String cspVersion = request.getParameter("cspVersion");
if (null != cspVersion)
jsonObj.put("cspVersion", cspVersion);
Expand Down