Skip to content

Change CSP allowed-sources registration method signature #6499

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.

Sign up for GitHub

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

Merged
labkey-adam merged 1 commit into from
Mar 28, 2025
Merged

Change CSP allowed-sources registration method signature #6499

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion api/src/org/labkey/api/security/SecurityManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ public static void registerAllowedConnectionSource(String key, String serviceURL
return;
}

ContentSecurityPolicyFilter.registerAllowedSources(Directive.Connection, key, serviceURL);
ContentSecurityPolicyFilter.registerAllowedSources(key, Directive.Connection, serviceURL);
LOG.trace(String.format("Registered [%1$s] as an allowed connection source", serviceURL));
}

Expand Down
2 changes: 1 addition & 1 deletion api/src/org/labkey/api/settings/AppPropsImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -596,7 +596,7 @@ public String getStaticFilesPrefix()
{
var url = new URLHelper(s).setPath("");
if (StringUtils.isNotEmpty(url.getHost()))
ContentSecurityPolicyFilter.registerAllowedSources(Directive.Connection, "static.files.prefix", url.toString());
ContentSecurityPolicyFilter.registerAllowedSources("static.files.prefix", Directive.Connection, url.toString());
prefix = s;
}
catch (URISyntaxException ignore)
Expand Down
26 changes: 17 additions & 9 deletions api/src/org/labkey/filters/ContentSecurityPolicyFilter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,8 @@ public String getHeaderName()
// and connections.
if (AppProps.getInstance().isDevMode())
{
registerAllowedSources(Directive.Connection, "reactjs.hot.reload", "localhost:3001 ws://localhost:3001");
registerAllowedSources(Directive.Font, "reactjs.hot.reload", "localhost:3001");
registerAllowedSources("reactjs.hot.reload", Directive.Connection, "localhost:3001 ws://localhost:3001");
registerAllowedSources("reactjs.hot.reload", Directive.Font, "localhost:3001");
}
}

Expand Down Expand Up @@ -242,10 +242,18 @@ public static String getScriptNonceHeader(HttpServletRequest request)

private static final SecureRandom rand = new SecureRandom();

@Deprecated // Keep around to ease the transition to the new method signature
public static void registerAllowedSources(Directive directive, String key, String... allowedSources)
{
registerAllowedSources(key, directive, allowedSources);
}

public static void registerAllowedSources(String key, Directive directive, String... allowedSources)
{
synchronized (ALLOWED_SOURCES_LOCK)
{
if (allowedSources.length == 0)
throw new IllegalStateException("Registering no sources is not allowed");
LOG.debug("Registering {} for {}: {}", directive, key, Arrays.toString(allowedSources));
SetValuedMap<String, String> multiMap = ALLOWED_SOURCES.computeIfAbsent(directive, d -> new HashSetValuedHashMap<>());
Arrays.stream(allowedSources).forEach(s -> multiMap.put(key, s));
Expand Down Expand Up @@ -400,21 +408,21 @@ public void testSubstitutionMap()
unregisterAllowedSources(Directive.Connection, "foo");
assertTrue(ALLOWED_SOURCES.isEmpty());
verifySubstitutionMapSize(0);
registerAllowedSources(Directive.Connection, "foo", "MySource");
registerAllowedSources("foo", Directive.Connection, "MySource");
assertEquals(1, ALLOWED_SOURCES.size());
verifySubstitutionMapSize(2); // Old connection substitution key should be added as well
verifySubstitutionInPolicyExpressions("MySource", 1);
registerAllowedSources(Directive.Connection, "bar", "MySource");
registerAllowedSources("bar", Directive.Connection, "MySource");
assertEquals(1, ALLOWED_SOURCES.size());
verifySubstitutionMapSize(2);
verifySubstitutionInPolicyExpressions("MySource", 1); // Duplicate source should be filtered out

unregisterAllowedSources(Directive.Font, "font");
registerAllowedSources(Directive.Font, "font", "MySource");
registerAllowedSources("font", Directive.Font, "MySource");
assertEquals(2, ALLOWED_SOURCES.size());
verifySubstitutionMapSize(3);
verifySubstitutionInPolicyExpressions("MySource", 2);
registerAllowedSources(Directive.Font, "font2", "MyFontSource");
registerAllowedSources("font2", Directive.Font, "MyFontSource");
assertEquals(2, ALLOWED_SOURCES.size());
verifySubstitutionMapSize(3);
verifySubstitutionInPolicyExpressions("MySource", 2);
Expand All @@ -432,21 +440,21 @@ public void testSubstitutionMap()
verifySubstitutionInPolicyExpressions("MyFontSource", 0);

unregisterAllowedSources(Directive.Frame, "frame");
registerAllowedSources(Directive.Frame, "frame", "FrameSource", "FrameStore");
registerAllowedSources("frame", Directive.Frame, "FrameSource", "FrameStore");
assertEquals(3, ALLOWED_SOURCES.size());
verifySubstitutionMapSize(3);
verifySubstitutionInPolicyExpressions("FrameSource", 1);
verifySubstitutionInPolicyExpressions("FrameStore", 1);

unregisterAllowedSources(Directive.Style, "style");
registerAllowedSources(Directive.Style, "style", "StyleSource", "MoreStylishStore");
registerAllowedSources("style", Directive.Style, "StyleSource", "MoreStylishStore");
assertEquals(4, ALLOWED_SOURCES.size());
verifySubstitutionMapSize(4);
verifySubstitutionInPolicyExpressions("StyleSource", 1);
verifySubstitutionInPolicyExpressions("MoreStylishStore", 1);

unregisterAllowedSources(Directive.Image, "image");
registerAllowedSources(Directive.Image, "image", "ImageSource", "BetterImageStore");
registerAllowedSources("image", Directive.Image, "ImageSource", "BetterImageStore");
assertEquals(5, ALLOWED_SOURCES.size());
verifySubstitutionMapSize(5);
verifySubstitutionInPolicyExpressions("ImageSource", 1);
Expand Down
2 changes: 1 addition & 1 deletion core/src/org/labkey/core/analytics/AnalyticsServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ public void resetCSP()

if (getTrackingStatus().contains(TrackingStatus.ga4FullUrl))
{
ContentSecurityPolicyFilter.registerAllowedSources(Directive.Connection, ANALYTICS_CSP_KEY, "https://*.googletagmanager.com", "https://*.google-analytics.com", "https://*.analytics.google.com");
ContentSecurityPolicyFilter.registerAllowedSources(ANALYTICS_CSP_KEY, Directive.Connection, "https://*.googletagmanager.com", "https://*.google-analytics.com", "https://*.analytics.google.com");
}
}

Expand Down
4 changes: 2 additions & 2 deletions core/src/org/labkey/core/security/AllowedExternalResourceHosts.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ public static void saveAllowedHosts(@Nullable Collection<AllowedHost> allowedHos
ContentSecurityPolicyFilter.unregisterAllowedSources(dir, ALLOWED_EXTERNAL_RESOURCES);
List<String> list = map.get(dir);
if (list != null)
ContentSecurityPolicyFilter.registerAllowedSources(dir, ALLOWED_EXTERNAL_RESOURCES, list.toArray(new String[0]));
ContentSecurityPolicyFilter.registerAllowedSources(ALLOWED_EXTERNAL_RESOURCES, dir, list.toArray(new String[0]));
});
}
}
Expand All @@ -92,7 +92,7 @@ public static void registerHosts()
return;
}

list.forEach(sub -> ContentSecurityPolicyFilter.registerAllowedSources(sub.directive(), "External Sources", sub.host()));
list.forEach(sub -> ContentSecurityPolicyFilter.registerAllowedSources("External Sources", sub.directive(), sub.host()));
LOG.debug("Registered [{}] as allowed external sources", list);
}

Expand Down