Skip to content

Commit 884fa58

Browse files
authored
Stop sending labkeyVersion in our CSPs (#7432)
1 parent 4b57fb7 commit 884fa58

2 files changed

Lines changed: 16 additions & 22 deletions

File tree

api/src/org/labkey/filters/ContentSecurityPolicyFilter.java

Lines changed: 6 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,6 @@ public class ContentSecurityPolicyFilter implements Filter
5353
private static final Logger LOG = LogHelper.getLogger(ContentSecurityPolicyFilter.class, "Register/unregister allowed resource hosts");
5454

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

@@ -120,14 +119,8 @@ public void init(FilterConfig filterConfig) throws ServletException
120119
String paramValue = filterConfig.getInitParameter(paramName);
121120
if ("policy".equalsIgnoreCase(paramName))
122121
{
123-
String s = filterPolicy(paramValue);
124-
125-
// Replace REPORT_PARAMETER_SUBSTITUTION now since its value is static
126-
s = substituteReportParams(s);
127-
128-
_stashedTemplate = s;
129-
130-
extractCspVersion(s);
122+
_stashedTemplate = filterPolicy(paramValue);
123+
extractCspVersion(_stashedTemplate);
131124
}
132125
else if ("disposition".equalsIgnoreCase(paramName))
133126
{
@@ -150,12 +143,6 @@ else if ("disposition".equalsIgnoreCase(paramName))
150143
_reportToEndpointName = "csp-" + getType().name().toLowerCase();
151144
}
152145

153-
private String substituteReportParams(String expression)
154-
{
155-
return StringExpressionFactory.create(expression, false, NullValueBehavior.KeepSubstitution)
156-
.eval(Map.of(REPORT_PARAMETER_SUBSTITUTION, "labkeyVersion=" + PageFlowUtil.encodeURIComponent(AppProps.getInstance().getReleaseVersion())));
157-
}
158-
159146
/** Filter out block comments and replace special characters in the provided policy */
160147
public static String filterPolicy(String policy)
161148
{
@@ -292,7 +279,7 @@ private CspFilterSettings(ContentSecurityPolicyFilter filter, String baseServerU
292279
@SuppressWarnings("DataFlowIssue")
293280
ActionURL violationUrl = PageFlowUtil.urlProvider(AdminUrls.class).getCspReportToURL(filter.getCspVersion());
294281
// Use an absolute URL so we always post to https:, even if the violating request uses http:
295-
_reportingEndpointsHeaderValue = filter.getReportToEndpointName() + "=\"" + filter.substituteReportParams(violationUrl.getURIString() + "&${CSP.REPORT.PARAMS}") + "\"";
282+
_reportingEndpointsHeaderValue = filter.getReportToEndpointName() + "=\"" + violationUrl.getURIString() + "\"";
296283

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

306293
_previousBaseServerUrl = baseServerUrl;
307294

308-
final String allowSubstitutedPolicy;
295+
final String substitutedPolicy;
309296

310297
synchronized (SUBSTITUTION_LOCK)
311298
{
312-
allowSubstitutedPolicy = StringExpressionFactory.create(_policyTemplate, false, NullValueBehavior.KeepSubstitution)
299+
substitutedPolicy = StringExpressionFactory.create(_policyTemplate, false, NullValueBehavior.KeepSubstitution)
313300
.eval(SUBSTITUTION_MAP);
314301
}
315302

316-
_policyExpression = StringExpressionFactory.create(allowSubstitutedPolicy, false, NullValueBehavior.ReplaceNullAndMissingWithBlank);
303+
_policyExpression = StringExpressionFactory.create(substitutedPolicy, false, NullValueBehavior.ReplaceNullAndMissingWithBlank);
317304
}
318305

319306
public String getPolicyTemplate()

core/src/org/labkey/core/admin/AdminController.java

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import com.fasterxml.jackson.annotation.JsonIgnore;
1919
import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
2020
import com.fasterxml.jackson.core.JsonProcessingException;
21+
import com.fasterxml.jackson.databind.DeserializationFeature;
2122
import com.fasterxml.jackson.databind.ObjectMapper;
2223
import com.google.common.base.Joiner;
2324
import com.google.common.util.concurrent.UncheckedExecutionException;
@@ -12032,6 +12033,14 @@ public void handleReports(ReportToJsonObjects jsonObjects, HttpServletRequest re
1203212033
if (!reportsToForward.isEmpty())
1203312034
forwardReports(LABKEY_ORG_REPORT_TO_ACTION, request, reportsToForward.toString(2));
1203412035
}
12036+
12037+
@Override
12038+
protected ObjectMapper createRequestObjectMapper()
12039+
{
12040+
// Annoyingly, Chrome posts an array of JSON objects but Safari posts individual JSON objects. Set a flag
12041+
// that ensures both cases deserialize into List<JSONObject>.
12042+
return JsonUtil.DEFAULT_MAPPER.copy().enable(DeserializationFeature.ACCEPT_SINGLE_VALUE_AS_ARRAY);
12043+
}
1203512044
}
1203612045

1203712046
@RequiresNoPermission
@@ -12125,6 +12134,7 @@ protected boolean handleOneReport(JSONObject jsonObj, HttpServletRequest request
1212512134
boolean forwarded = jsonObj.optBoolean("forwarded", false);
1212612135
if (!forwarded)
1212712136
{
12137+
jsonObj.put("labkeyVersion", AppProps.getInstance().getReleaseVersion());
1212812138
User user = getUser();
1212912139
String email = null;
1213012140
// If the user is not logged in, we may still be able to snag the email address from our cookie
@@ -12139,9 +12149,6 @@ protected boolean handleOneReport(JSONObject jsonObj, HttpServletRequest request
1213912149
jsonObj.put("ip", ipAddress);
1214012150
if (isNotBlank(userAgent) && !jsonObj.has("user_agent"))
1214112151
jsonObj.put("user_agent", userAgent);
12142-
String labkeyVersion = request.getParameter("labkeyVersion");
12143-
if (null != labkeyVersion)
12144-
jsonObj.put("labkeyVersion", labkeyVersion);
1214512152
String cspVersion = request.getParameter("cspVersion");
1214612153
if (null != cspVersion)
1214712154
jsonObj.put("cspVersion", cspVersion);

0 commit comments

Comments
 (0)