diff --git a/api/src/org/labkey/api/ApiModule.java b/api/src/org/labkey/api/ApiModule.java index e6029ffb627..a5b92e03c19 100644 --- a/api/src/org/labkey/api/ApiModule.java +++ b/api/src/org/labkey/api/ApiModule.java @@ -254,6 +254,7 @@ protected void doStartup(ModuleContext moduleContext) { SystemMaintenance.addTask(new ApiKeyMaintenanceTask()); AuthenticationManager.registerMetricsProvider(); + ContentSecurityPolicyFilter.registerMetricsProvider(); ApiKeyManager.get().handleStartupProperties(); MailHelper.init(); // Handle optional feature and system maintenance startup properties as late as possible; we want all optional diff --git a/api/src/org/labkey/api/collections/CaseInsensitiveHashSetValuedMap.java b/api/src/org/labkey/api/collections/CaseInsensitiveHashSetValuedMap.java index 079c8c6cc4b..bbfe5a3fefe 100644 --- a/api/src/org/labkey/api/collections/CaseInsensitiveHashSetValuedMap.java +++ b/api/src/org/labkey/api/collections/CaseInsensitiveHashSetValuedMap.java @@ -17,22 +17,22 @@ import org.apache.commons.collections4.multimap.AbstractSetValuedMap; -import java.util.HashSet; +import java.util.HashMap; import java.util.Set; /** - * A case-insensitive version of org.apache.commons.collections4.multimap.HashSetValuedHashMap + * An org.apache.commons.collections4.multimap.HashSetValuedHashMap with case-insensitive String values */ -public class CaseInsensitiveHashSetValuedMap extends AbstractSetValuedMap implements CaseInsensitiveCollection +public class CaseInsensitiveHashSetValuedMap extends AbstractSetValuedMap implements CaseInsensitiveCollection { public CaseInsensitiveHashSetValuedMap() { - super(new CaseInsensitiveHashMap<>()); + super(new HashMap<>()); } @Override - protected Set createCollection() + protected Set createCollection() { - return new HashSet<>(); + return new CaseInsensitiveHashSet(); } } diff --git a/api/src/org/labkey/api/collections/CaseInsensitiveKeyedHashSetValuedMap.java b/api/src/org/labkey/api/collections/CaseInsensitiveKeyedHashSetValuedMap.java new file mode 100644 index 00000000000..400d4384b87 --- /dev/null +++ b/api/src/org/labkey/api/collections/CaseInsensitiveKeyedHashSetValuedMap.java @@ -0,0 +1,38 @@ +/* + * Copyright (c) 2016 LabKey Corporation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.labkey.api.collections; + +import org.apache.commons.collections4.multimap.AbstractSetValuedMap; + +import java.util.HashSet; +import java.util.Set; + +/** + * An org.apache.commons.collections4.multimap.HashSetValuedHashMap with case-insensitive String keys + */ +public class CaseInsensitiveKeyedHashSetValuedMap extends AbstractSetValuedMap implements CaseInsensitiveCollection +{ + public CaseInsensitiveKeyedHashSetValuedMap() + { + super(new CaseInsensitiveHashMap<>()); + } + + @Override + protected Set createCollection() + { + return new HashSet<>(); + } +} diff --git a/api/src/org/labkey/api/module/ModuleLoader.java b/api/src/org/labkey/api/module/ModuleLoader.java index 3fb89f2c21e..7c39b7ea772 100644 --- a/api/src/org/labkey/api/module/ModuleLoader.java +++ b/api/src/org/labkey/api/module/ModuleLoader.java @@ -30,7 +30,7 @@ import org.labkey.api.action.UrlProviderService; import org.labkey.api.collections.CaseInsensitiveHashMap; import org.labkey.api.collections.CaseInsensitiveHashSet; -import org.labkey.api.collections.CaseInsensitiveHashSetValuedMap; +import org.labkey.api.collections.CaseInsensitiveKeyedHashSetValuedMap; import org.labkey.api.collections.CaseInsensitiveTreeMap; import org.labkey.api.collections.CaseInsensitiveTreeSet; import org.labkey.api.collections.CopyOnWriteHashMap; @@ -229,7 +229,7 @@ public enum ModuleState // Allow multiple StartupPropertyHandlers with the same scope as long as the StartupProperty impl class is different. private final Set> _startupPropertyHandlers = new ConcurrentSkipListSet<>(Comparator.comparing((StartupPropertyHandler sph)->sph.getScope(), String.CASE_INSENSITIVE_ORDER).thenComparing(StartupPropertyHandler::getStartupPropertyClassName)); - private final MultiValuedMap _startupPropertyMap = new CaseInsensitiveHashSetValuedMap<>(); + private final MultiValuedMap _startupPropertyMap = new CaseInsensitiveKeyedHashSetValuedMap<>(); private ModuleLoader() { diff --git a/api/src/org/labkey/api/security/Directive.java b/api/src/org/labkey/api/security/Directive.java index 2fff4323bf7..22c9d645f50 100644 --- a/api/src/org/labkey/api/security/Directive.java +++ b/api/src/org/labkey/api/security/Directive.java @@ -1,22 +1,27 @@ package org.labkey.api.security; +import org.labkey.api.settings.StartupProperty; +import org.labkey.api.util.SafeToRenderEnum; + /** * All CSP directives that support substitutions. These constant names are persisted to the database, so be careful with * any changes. If adding a Directive, make sure to add the corresponding substitutions to application.properties. */ -public enum Directive +public enum Directive implements StartupProperty, SafeToRenderEnum { - Connection("connect-src"), - Font("font-src"), - Frame("frame-src"), - Image("image-src"), - Style("style-src"); + Connection("connect-src", "Sources for fetch/XHR requests"), + Font("font-src", "Sources for fonts"), + Frame("frame-src", "Sources for iframes"), + Image("image-src", "Sources for images"), + Style("style-src", "Sources for stylesheets"); private final String _cspDirective; + private final String _description; - Directive(String cspDirective) + Directive(String cspDirective, String description) { _cspDirective = cspDirective; + _description = description; } public String getCspDirective() @@ -28,4 +33,10 @@ public String getSubstitutionKey() { return name().toUpperCase() + ".SOURCES"; } + + @Override + public String getDescription() + { + return _description + " (" + _cspDirective + "). Multiple hosts, separated by a space, can be provided."; + } } diff --git a/api/src/org/labkey/api/settings/AppProps.java b/api/src/org/labkey/api/settings/AppProps.java index 485cba78e88..5eb3a6d52d8 100644 --- a/api/src/org/labkey/api/settings/AppProps.java +++ b/api/src/org/labkey/api/settings/AppProps.java @@ -47,6 +47,7 @@ public interface AppProps String EXPERIMENTAL_BLOCKER = "blockMaliciousClients"; String EXPERIMENTAL_RESOLVE_PROPERTY_URI_COLUMNS = "resolve-property-uri-columns"; String DEPRECATED_OBJECT_LEVEL_DISCUSSIONS = "deprecatedObjectLevelDiscussions"; + String ALLOWED_EXTERNAL_RESOURCES = "allowedExternalResources"; String UNKNOWN_VERSION = "Unknown Release Version"; @@ -237,16 +238,15 @@ static WriteableAppProps getWriteableInstance() boolean isIncludeServerHttpHeader(); /** - * * @return List of configured external redirect hosts */ @NotNull List getExternalRedirectHosts(); /** - * * @return List of configured external resource hosts */ + @Deprecated // Left for upgrade code only @NotNull List getExternalSourceHosts(); @@ -259,4 +259,6 @@ static WriteableAppProps getWriteableInstance() @NotNull Set getDistributionSupportedDatabases(); @NotNull List getAllowedExtensions(); + + @NotNull String getAllowedExternalResourceHosts(); } diff --git a/api/src/org/labkey/api/settings/AppPropsImpl.java b/api/src/org/labkey/api/settings/AppPropsImpl.java index 8768a2a912e..ef832b1edd5 100644 --- a/api/src/org/labkey/api/settings/AppPropsImpl.java +++ b/api/src/org/labkey/api/settings/AppPropsImpl.java @@ -667,13 +667,6 @@ public List getExternalRedirectHosts() return getExternalHosts(externalRedirectHostURLs); } - @Override - @NotNull - public List getExternalSourceHosts() - { - return getExternalHosts(externalSourceHostURLs); - } - @Override @NotNull public List getAllowedExtensions() @@ -732,4 +725,23 @@ public Map getStashedStartupProp { return DISTRIBUTION_SUPPORTED_DATABASES; } + + @Deprecated + @Override + @NotNull + public List getExternalSourceHosts() + { + String urls = lookupStringValue("externalSourceHostURLs", ""); + if (StringUtils.isNotBlank(urls)) + { + return new ArrayList<>(Arrays.asList(urls.split(EXTERNAL_HOST_DELIMITER))); + } + return new ArrayList<>(); + } + + @Override + public @NotNull String getAllowedExternalResourceHosts() + { + return lookupStringValue(ALLOWED_EXTERNAL_RESOURCES, "[]"); + } } diff --git a/api/src/org/labkey/api/settings/RandomStartupProperties.java b/api/src/org/labkey/api/settings/RandomStartupProperties.java index 5af8d7b4da7..0363a54c538 100644 --- a/api/src/org/labkey/api/settings/RandomStartupProperties.java +++ b/api/src/org/labkey/api/settings/RandomStartupProperties.java @@ -28,14 +28,6 @@ public void setValue(WriteableAppProps writeable, String value) writeable.setExternalRedirectHosts(Arrays.asList(StringUtils.split(value, AppPropsImpl.EXTERNAL_HOST_DELIMITER))); } }, - externalSourceHostURLs("Allowed external source hosts") - { - @Override - public void setValue(WriteableAppProps writeable, String value) - { - writeable.setExternalSourceHosts(Arrays.asList(StringUtils.split(value, AppPropsImpl.EXTERNAL_HOST_DELIMITER))); - } - }, allowedFileExtensions("Allowed file extensions") { @Override diff --git a/api/src/org/labkey/api/settings/WriteableAppProps.java b/api/src/org/labkey/api/settings/WriteableAppProps.java index 83051137173..aba9b5503bf 100644 --- a/api/src/org/labkey/api/settings/WriteableAppProps.java +++ b/api/src/org/labkey/api/settings/WriteableAppProps.java @@ -232,11 +232,6 @@ public void setExternalRedirectHosts(@NotNull Collection externalRedirec setExternalHosts(externalRedirectHostURLs, externalRedirectHosts); } - public void setExternalSourceHosts(@NotNull Collection externalSourceHosts) - { - setExternalHosts(externalSourceHostURLs, externalSourceHosts); - } - private void setExternalHosts(RandomStartupProperties propName, @NotNull Collection externalSourceHosts) { storeStringValue(propName, String.join(EXTERNAL_HOST_DELIMITER, externalSourceHosts)); @@ -247,4 +242,9 @@ public void setAllowedFileExtensions(Collection allowedFileExtensions) setExternalHosts(RandomStartupProperties.allowedFileExtensions, allowedFileExtensions); FileUtil.setExtensionChecker(AppProps.getInstance()); } + + public void setAllowedExternalResourceHosts(String jsonArray) + { + storeStringValue(ALLOWED_EXTERNAL_RESOURCES, jsonArray); + } } diff --git a/api/src/org/labkey/filters/ContentSecurityPolicyFilter.java b/api/src/org/labkey/filters/ContentSecurityPolicyFilter.java index 1879f54c690..64e645b7c76 100644 --- a/api/src/org/labkey/filters/ContentSecurityPolicyFilter.java +++ b/api/src/org/labkey/filters/ContentSecurityPolicyFilter.java @@ -10,17 +10,23 @@ import jakarta.servlet.http.HttpServletResponse; import org.apache.commons.collections4.SetValuedMap; import org.apache.commons.collections4.multimap.HashSetValuedHashMap; +import org.apache.commons.lang3.StringUtils; +import org.apache.logging.log4j.Logger; import org.junit.Assert; import org.junit.Test; +import org.labkey.api.collections.CopyOnWriteHashMap; +import org.labkey.api.collections.LabKeyCollectors; import org.labkey.api.security.Directive; import org.labkey.api.settings.AppProps; import org.labkey.api.settings.OptionalFeatureService; +import org.labkey.api.usageMetrics.UsageMetricsService; import org.labkey.api.util.CspCommentScanner; import org.labkey.api.util.PageFlowUtil; import org.labkey.api.util.StringExpression; import org.labkey.api.util.StringExpressionFactory; import org.labkey.api.util.StringExpressionFactory.AbstractStringExpression.NullValueBehavior; import org.labkey.api.util.logging.LogHelper; +import org.labkey.api.view.ActionURL; import java.io.IOException; import java.security.SecureRandom; @@ -28,6 +34,7 @@ import java.util.Collections; import java.util.Enumeration; import java.util.HashMap; +import java.util.List; import java.util.Map; import java.util.Set; import java.util.stream.Collectors; @@ -43,14 +50,25 @@ public class ContentSecurityPolicyFilter implements Filter private static final String NONCE_SUBST = "REQUEST.SCRIPT.NONCE"; private static final String REPORT_PARAMETER_SUBSTITUTION = "CSP.REPORT.PARAMS"; private static final String HEADER_NONCE = "org.labkey.filters.ContentSecurityPolicyFilter#NONCE"; // needs to match PageConfig.HEADER_NONCE + + private static final Map CSP_FILTERS = new CopyOnWriteHashMap<>(); + + // Lock that protects the static data structures below private static final Object ALLOWED_SOURCES_LOCK = new Object(); private static final Map> ALLOWED_SOURCES = new HashMap<>(); - - private static Map _substitutionMap = Collections.emptyMap(); + // Regenerate and stash on every "allowed source" change as a convenience (so every filter don't need to recalculate + // it on every init() and change) + private static Map ALLOWED_SOURCES_SUBSTITUTION_MAP = Collections.emptyMap(); // Per-filter-instance parameters that are set in init() and never changed - private StringExpression policyExpression = null; - private ContentSecurityPolicyType type = ContentSecurityPolicyType.Enforce; + private ContentSecurityPolicyType _type = ContentSecurityPolicyType.Enforce; + private String _policyTemplate = null; + private String _cspVersion = "Unknown"; + + // Updated after every change to "allowed sources" + private StringExpression _policyExpression = null; + + private static final Logger LOG = LogHelper.getLogger(ContentSecurityPolicyFilter.class, "Register/unregister allowed resource hosts"); public enum ContentSecurityPolicyType { @@ -98,7 +116,9 @@ public void init(FilterConfig filterConfig) throws ServletException s = StringExpressionFactory.create(s, false, NullValueBehavior.KeepSubstitution) .eval(Map.of(REPORT_PARAMETER_SUBSTITUTION, "labkeyVersion=" + PageFlowUtil.encodeURIComponent(AppProps.getInstance().getReleaseVersion()))); - policyExpression = StringExpressionFactory.create(s, false, NullValueBehavior.ReplaceNullAndMissingWithBlank); + _policyTemplate = s; + + extractCspVersion(s); } else if ("disposition".equalsIgnoreCase(paramName)) { @@ -106,13 +126,18 @@ else if ("disposition".equalsIgnoreCase(paramName)) if (!"report".equalsIgnoreCase(s) && !"enforce".equalsIgnoreCase(s)) throw new ServletException("ContentSecurityPolicyFilter is misconfigured, unexpected disposition value: " + s); if ("report".equalsIgnoreCase(s)) - type = ContentSecurityPolicyType.Report; + _type = ContentSecurityPolicyType.Report; } else { throw new ServletException("ContentSecurityPolicyFilter is misconfigured, unexpected parameter name: " + paramName); } } + + if (CSP_FILTERS.put(_type, this) != null) + throw new ServletException("ContentSecurityPolicyFilter is misconfigured, duplicate policies of type: " + _type); + + regeneratePolicyExpression(); } /** Filter out block comments and replace special characters in the provided policy */ @@ -136,17 +161,67 @@ public static String filterPolicy(String policy) return s; } + /** + * Extract the cspVersion parameter value from the report-uri directive, if possible. Otherwise, cspVersion is left + * as "Unknown". This value is reported as part of usage metrics. + */ + private void extractCspVersion(String s) + { + // Simple parser that should be compliant with https://www.w3.org/TR/CSP3/#parse-serialized-policy + Map cspMap = Arrays.stream(s.split(";")) + .map(String::trim) + .filter(line -> !line.isEmpty()) + .map(line -> line.split("\\s+", 2)) + .filter(parts -> parts.length == 2) + .collect(LabKeyCollectors.toCaseInsensitiveLinkedMap(parts -> parts[0], parts -> parts[1])); + + String directive = "report-uri"; + String reportUri = cspMap.get(directive); + + if (reportUri != null) + { + try + { + ActionURL reportUrl = new ActionURL(reportUri); + String cspVersion = reportUrl.getParameter("cspVersion"); + + if (null != cspVersion) + _cspVersion = cspVersion; + } + catch (IllegalArgumentException e) + { + LOG.warn("Unable to parse {} URI", directive, e); + } + } + + LOG.debug("CspVersion: {}", _cspVersion); + } + + // Make all the "allowed sources" substitutions at init() and whenever the allowed sources map changes. With this, + // the only substitution needed on a per-request basis is the nonce value. + private void regeneratePolicyExpression() + { + final String allowSubstitutedPolicy; + + synchronized (ALLOWED_SOURCES_LOCK) + { + allowSubstitutedPolicy = StringExpressionFactory.create(_policyTemplate, false, NullValueBehavior.KeepSubstitution) + .eval(ALLOWED_SOURCES_SUBSTITUTION_MAP); + } + + _policyExpression = StringExpressionFactory.create(allowSubstitutedPolicy, false, NullValueBehavior.KeepSubstitution); + } + @Override public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException { - if (request instanceof HttpServletRequest req && response instanceof HttpServletResponse resp && null != policyExpression) + if (request instanceof HttpServletRequest req && response instanceof HttpServletResponse resp && null != _policyExpression) { - if (type != ContentSecurityPolicyType.Enforce || !OptionalFeatureService.get().isFeatureEnabled(FEATURE_FLAG_DISABLE_ENFORCE_CSP)) + if (_type != ContentSecurityPolicyType.Enforce || !OptionalFeatureService.get().isFeatureEnabled(FEATURE_FLAG_DISABLE_ENFORCE_CSP)) { - Map map = new HashMap<>(_substitutionMap); - map.put(NONCE_SUBST, getScriptNonceHeader(req)); - var csp = policyExpression.eval(map); - resp.setHeader(type.getHeaderName(), csp); + Map map = Map.of(NONCE_SUBST, getScriptNonceHeader(req)); + var csp = _policyExpression.eval(map); + resp.setHeader(_type.getHeaderName(), csp); } } chain.doFilter(request, response); @@ -167,16 +242,11 @@ public static String getScriptNonceHeader(HttpServletRequest request) private static final SecureRandom rand = new SecureRandom(); - @Deprecated // Use registerAllowedSources(Directive.Connection...) - public static void registerAllowedConnectionSource(String key, String... allowedUrls) - { - registerAllowedSources(Directive.Connection, key, allowedUrls); - } - public static void registerAllowedSources(Directive directive, String key, String... allowedSources) { synchronized (ALLOWED_SOURCES_LOCK) { + LOG.debug("Registering {} for {}: {}", directive, key, Arrays.toString(allowedSources)); SetValuedMap multiMap = ALLOWED_SOURCES.computeIfAbsent(directive, d -> new HashSetValuedHashMap<>()); Arrays.stream(allowedSources).forEach(s -> multiMap.put(key, s)); regenerateSubstitutionMap(); @@ -187,6 +257,7 @@ public static void unregisterAllowedSources(Directive directive, String key) { synchronized (ALLOWED_SOURCES_LOCK) { + LOG.debug("Unregistering {} for {}", directive, key); SetValuedMap multiMap = ALLOWED_SOURCES.get(directive); if (multiMap != null) { @@ -198,22 +269,65 @@ public static void unregisterAllowedSources(Directive directive, String key) } } - // Pre-generate the substitution map on every register/unregister + // Regenerate the substitution map and all policy expressions on every register/unregister private static void regenerateSubstitutionMap() { - _substitutionMap = ALLOWED_SOURCES.entrySet().stream() - .filter(e -> !e.getValue().isEmpty()) - .collect(Collectors.toMap( - e -> e.getKey().getSubstitutionKey(), - e -> e.getValue().values().stream() - .distinct() - .collect(Collectors.joining(" "))) - ); - - // Backward compatibility for CSPs using old substitution key - // TODO: Remove in 25.4 and adjust the junit test below - if (_substitutionMap.containsKey(Directive.Connection.getSubstitutionKey())) - _substitutionMap.put("LABKEY.ALLOWED.CONNECTIONS", _substitutionMap.get(Directive.Connection.getSubstitutionKey())); + synchronized (ALLOWED_SOURCES_LOCK) + { + ALLOWED_SOURCES_SUBSTITUTION_MAP = ALLOWED_SOURCES.entrySet().stream() + .filter(e -> !e.getValue().isEmpty()) + .collect(Collectors.toMap( + e -> e.getKey().getSubstitutionKey(), + e -> e.getValue().values().stream() + .distinct() + .collect(Collectors.joining(" "))) + ); + + // Add an empty substitution for sources that lack registrations. This strips them from the stashed policy, + // meaning less work on every request. + Arrays.stream(Directive.values()) + .forEach(dir -> ALLOWED_SOURCES_SUBSTITUTION_MAP.putIfAbsent(dir.getSubstitutionKey(), "")); + + // Backward compatibility for CSPs using old substitution key + // TODO: Remove in 25.4 and adjust the junit test below + if (ALLOWED_SOURCES_SUBSTITUTION_MAP.containsKey(Directive.Connection.getSubstitutionKey())) + ALLOWED_SOURCES_SUBSTITUTION_MAP.put("LABKEY.ALLOWED.CONNECTIONS", ALLOWED_SOURCES_SUBSTITUTION_MAP.get(Directive.Connection.getSubstitutionKey())); + + // Tell each registered ContentSecurityPolicyFilter to refresh its policy template based on the new substitution map + CSP_FILTERS.values().forEach(ContentSecurityPolicyFilter::regeneratePolicyExpression); + } + } + + public static boolean hasCsp(ContentSecurityPolicyType type) + { + return CSP_FILTERS.get(type) != null; + } + + public static List getMissingSubstitutions(ContentSecurityPolicyType type) + { + ContentSecurityPolicyFilter filter = CSP_FILTERS.get(type); + final List ret; + if (filter == null) + { + ret = Collections.emptyList(); + } + else + { + String template = filter._policyTemplate; + ret = Arrays.stream(Directive.values()) + .map(dir -> "${" + dir.getSubstitutionKey() + "}") + .filter(key -> !template.contains(key)) + .toList(); + } + + return ret; + } + + public static void registerMetricsProvider() + { + UsageMetricsService.get().registerUsageMetrics("API", () -> Map.of("cspFilters", CSP_FILTERS.values().stream() + .collect(Collectors.toMap(filter -> filter._type, + filter -> Map.of("version", filter._cspVersion, "csp", filter._policyTemplate, "cspSubstituted", filter._policyExpression.getSource()))))); } public static class TestCase extends Assert @@ -222,33 +336,33 @@ public static class TestCase extends Assert public void testPolicyFiltering() { String fakePolicyForTesting = """ - /* Beginning of line comment should be removed */default-src\t'self' https: http: ; - connect-src 'self' http://www.labkey.org /* this is a mistake! */ localhost:* ws: ${LABKEY.ALLOWED.CONNECTIONS} ; - object-src https://* ‘none’ ; /* Hard to see, but there are curly quotes surrounding "none" on this line */\r - style-src 'self'\rhttps: 'unsafe-inline' ; - img-src 'self'\thttps: data: ; - font-src 'self' http://www.labkey.com https://* http: /* I don't know why we're doing this! */ https: data: ; - script-src 'unsafe-eval' 'strict-dynamic' 'nonce-${REQUEST.SCRIPT.NONCE}' ; - base-uri 'self' ; /* what in the world?! */ - frame-ancestors 'self' ; /* This here comment spans - multiple lines - for testing purposes - */ - report-uri /* Whoa! */ /admin-contentsecuritypolicyreport.api?${CSP.REPORT.PARAMS} https://*; - """; + /* Beginning of line comment should be removed */default-src\t'self' https: http: ; + connect-src 'self' http://www.labkey.org /* this is a mistake! */ localhost:* ws: ${LABKEY.ALLOWED.CONNECTIONS} ; + object-src https://* ‘none’ ; /* Hard to see, but there are curly quotes surrounding "none" on this line */\r + style-src 'self'\rhttps: 'unsafe-inline' ; + img-src 'self'\thttps: data: ; + font-src 'self' http://www.labkey.com https://* http: /* I don't know why we're doing this! */ https: data: ; + script-src 'unsafe-eval' 'strict-dynamic' 'nonce-${REQUEST.SCRIPT.NONCE}' ; + base-uri 'self' ; /* what in the world?! */ + frame-ancestors 'self' ; /* This here comment spans + multiple lines + for testing purposes + */ + report-uri /* Whoa! */ /admin-contentsecuritypolicyreport.api?${CSP.REPORT.PARAMS} https://*; + """; // Multi-line for readability, but notice that newlines are replaced before assignment String expected = """ - default-src 'self' https: http: ; - connect-src 'self' http://www.labkey.org localhost:* ws: ${LABKEY.ALLOWED.CONNECTIONS} ; - object-src https://* 'none' ; - style-src 'self' https: 'unsafe-inline' ; - img-src 'self' https: data: ; - font-src 'self' http://www.labkey.com https://* http: https: data: ; - script-src 'unsafe-eval' 'strict-dynamic' 'nonce-${REQUEST.SCRIPT.NONCE}' ; - base-uri 'self' ; - frame-ancestors 'self' ; - report-uri /admin-contentsecuritypolicyreport.api?${CSP.REPORT.PARAMS} https://*;""".replace('\n', ' '); + default-src 'self' https: http: ; + connect-src 'self' http://www.labkey.org localhost:* ws: ${LABKEY.ALLOWED.CONNECTIONS} ; + object-src https://* 'none' ; + style-src 'self' https: 'unsafe-inline' ; + img-src 'self' https: data: ; + font-src 'self' http://www.labkey.com https://* http: https: data: ; + script-src 'unsafe-eval' 'strict-dynamic' 'nonce-${REQUEST.SCRIPT.NONCE}' ; + base-uri 'self' ; + frame-ancestors 'self' ; + report-uri /admin-contentsecuritypolicyreport.api?${CSP.REPORT.PARAMS} https://*;""".replace('\n', ' '); Assert.assertEquals(expected, filterPolicy(fakePolicyForTesting)); } @@ -256,74 +370,119 @@ public void testPolicyFiltering() @Test public void testSubstitutionMap() { - // Make a deep copy of ALLOWED_SOURCES so we can restore it after testing - final Map> savedSources; - final int sourceMapSize; - final int substitutionMapSize; synchronized (ALLOWED_SOURCES_LOCK) { - sourceMapSize = ALLOWED_SOURCES.size(); - substitutionMapSize = _substitutionMap.size(); - savedSources = ALLOWED_SOURCES.entrySet().stream() + // Make a deep copy of ALLOWED_SOURCES so we can restore it after testing + int sourceMapSize = ALLOWED_SOURCES.size(); + int substitutionMapSize = ALLOWED_SOURCES_SUBSTITUTION_MAP.size(); + Map> savedSources = ALLOWED_SOURCES.entrySet().stream() .collect(Collectors.toMap(Map.Entry::getKey, e -> new HashSetValuedHashMap<>(e.getValue()))); - ALLOWED_SOURCES.clear(); - regenerateSubstitutionMap(); - } - try - { - assertTrue(ALLOWED_SOURCES.isEmpty()); - assertTrue(_substitutionMap.isEmpty()); - unregisterAllowedSources(Directive.Connection, "foo"); - assertTrue(ALLOWED_SOURCES.isEmpty()); - assertTrue(_substitutionMap.isEmpty()); - registerAllowedSources(Directive.Connection, "foo", "MySource"); - assertEquals(1, ALLOWED_SOURCES.size()); - assertEquals(2, _substitutionMap.size()); // Old connection substitution key should be added as well - registerAllowedSources(Directive.Connection, "bar", "MySource"); - assertEquals(1, ALLOWED_SOURCES.size()); - assertEquals(2, _substitutionMap.size()); // Duplicate source should be filtered out - - registerAllowedSources(Directive.Font, "font", "MySource"); - assertEquals(2, ALLOWED_SOURCES.size()); - assertEquals(3, _substitutionMap.size()); - registerAllowedSources(Directive.Font, "font2", "MyOtherSource"); - assertEquals(2, ALLOWED_SOURCES.size()); - assertEquals(3, _substitutionMap.size()); - String value = _substitutionMap.get("FONT.SOURCES"); - assertEquals("! !", value.replace("MyOtherSource", "!").replace("MySource", "!")); - unregisterAllowedSources(Directive.Font, "font2"); - assertEquals(2, ALLOWED_SOURCES.size()); - assertEquals(3, _substitutionMap.size()); - unregisterAllowedSources(Directive.Font, "font"); - assertEquals(2, ALLOWED_SOURCES.size()); // Font entry still exists, but should be empty - assertTrue(ALLOWED_SOURCES.get(Directive.Font).isEmpty()); - assertEquals(2, _substitutionMap.size()); // Back to the way it was - - registerAllowedSources(Directive.Frame, "frame", "FrameSource", "FrameStore"); - assertEquals(3, ALLOWED_SOURCES.size()); - assertEquals(3, _substitutionMap.size()); - - registerAllowedSources(Directive.Style, "style", "StyleSource", "MoreStylishStore"); - assertEquals(4, ALLOWED_SOURCES.size()); - assertEquals(4, _substitutionMap.size()); - - registerAllowedSources(Directive.Image, "image", "ImageSource", "BetterImageStore"); - assertEquals(5, ALLOWED_SOURCES.size()); - assertEquals(5, _substitutionMap.size()); - } - finally - { - // Restore the previous ALLOWED_SOURCES - synchronized (ALLOWED_SOURCES_LOCK) + try { + ALLOWED_SOURCES.clear(); + regenerateSubstitutionMap(); + + // Initial checks + assertTrue(ALLOWED_SOURCES.isEmpty()); + verifySubstitutionMapSize(0); + // All "allowed sources" substitutions should be replaced with empty strings + verifySubstitutionInPolicyExpressions(".SOURCES}", 0); + // Should have been substitution in init() + verifySubstitutionInPolicyExpressions("${CSP.REPORT.PARAMS}", 0); + // A single substitution parameter (${REQUEST.SCRIPT.NONCE}) should remain + verifySubstitutionInPolicyExpressions("${REQUEST.SCRIPT.NONCE}", 1); + verifySubstitutionInPolicyExpressions("${", 1); + + // Now unregister and register sources for each Directive, testing expectations along the way + unregisterAllowedSources(Directive.Connection, "foo"); + assertTrue(ALLOWED_SOURCES.isEmpty()); + verifySubstitutionMapSize(0); + registerAllowedSources(Directive.Connection, "foo", "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"); + 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"); + assertEquals(2, ALLOWED_SOURCES.size()); + verifySubstitutionMapSize(3); + verifySubstitutionInPolicyExpressions("MySource", 2); + registerAllowedSources(Directive.Font, "font2", "MyFontSource"); + assertEquals(2, ALLOWED_SOURCES.size()); + verifySubstitutionMapSize(3); + verifySubstitutionInPolicyExpressions("MySource", 2); + verifySubstitutionInPolicyExpressions("MyFontSource", 1); + unregisterAllowedSources(Directive.Font, "font2"); + assertEquals(2, ALLOWED_SOURCES.size()); + verifySubstitutionMapSize(3); + verifySubstitutionInPolicyExpressions("MySource", 2); + verifySubstitutionInPolicyExpressions("MyFontSource", 0); + unregisterAllowedSources(Directive.Font, "font"); + assertEquals(2, ALLOWED_SOURCES.size()); // Font entry still exists, but should be empty + assertTrue(ALLOWED_SOURCES.get(Directive.Font).isEmpty()); + verifySubstitutionMapSize(2);// Back to the way it was + verifySubstitutionInPolicyExpressions("MySource", 1); + verifySubstitutionInPolicyExpressions("MyFontSource", 0); + + unregisterAllowedSources(Directive.Frame, "frame"); + registerAllowedSources(Directive.Frame, "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"); + assertEquals(4, ALLOWED_SOURCES.size()); + verifySubstitutionMapSize(4); + verifySubstitutionInPolicyExpressions("StyleSource", 1); + verifySubstitutionInPolicyExpressions("MoreStylishStore", 1); + + unregisterAllowedSources(Directive.Image, "image"); + registerAllowedSources(Directive.Image, "image", "ImageSource", "BetterImageStore"); + assertEquals(5, ALLOWED_SOURCES.size()); + verifySubstitutionMapSize(5); + verifySubstitutionInPolicyExpressions("ImageSource", 1); + verifySubstitutionInPolicyExpressions("BetterImageStore", 1); + } + finally + { + // Restore the previous ALLOWED_SOURCES ALLOWED_SOURCES.clear(); ALLOWED_SOURCES.putAll(savedSources); regenerateSubstitutionMap(); assertEquals(sourceMapSize, ALLOWED_SOURCES.size()); - assertEquals(substitutionMapSize, _substitutionMap.size()); + assertEquals(substitutionMapSize, ALLOWED_SOURCES_SUBSTITUTION_MAP.size()); } } } + + private void verifySubstitutionInPolicyExpressions(String value, int expectedCount) + { + List failures = CSP_FILTERS.values().stream() + .map(filter -> filter._policyExpression.eval(Map.of())) + .filter(policy -> StringUtils.countMatches(policy, value) != expectedCount) + .toList(); + + if (!failures.isEmpty()) + { + fail("Occurrences of value \"" + value + "\" was not the expected count (" + expectedCount + ") in policies [\"" + String.join("\", \"", failures) + "\"]"); + } + } + + private void verifySubstitutionMapSize(long expectedNonEmptyValues) + { + // Actual map size should stay static throughout test + int expectedSubstitutionMapSize = Directive.values().length + 1; // One extra for old "connections" key + + assertEquals(expectedSubstitutionMapSize, ALLOWED_SOURCES_SUBSTITUTION_MAP.size()); + long nonEmptyValues = ALLOWED_SOURCES_SUBSTITUTION_MAP.entrySet().stream().filter(e -> !e.getValue().isEmpty()).count(); + assertEquals(expectedNonEmptyValues, nonEmptyValues); + } } } diff --git a/core/module.properties b/core/module.properties index 0d75d4beb95..0c1eb10334b 100644 --- a/core/module.properties +++ b/core/module.properties @@ -1,6 +1,6 @@ Name: Core ModuleClass: org.labkey.core.CoreModule -SchemaVersion: 25.000 +SchemaVersion: 25.001 Label: Administration and Essential Services Description: The Core module provides central services such as login, \ security, administration, folder management, user management, \ diff --git a/core/resources/schemas/dbscripts/postgresql/core-25.000-25.001.sql b/core/resources/schemas/dbscripts/postgresql/core-25.000-25.001.sql new file mode 100644 index 00000000000..d2fc4660427 --- /dev/null +++ b/core/resources/schemas/dbscripts/postgresql/core-25.000-25.001.sql @@ -0,0 +1 @@ +SELECT core.executeJavaUpgradeCode('migrateAllowedExternalConnectionHosts'); diff --git a/core/resources/schemas/dbscripts/sqlserver/core-25.000-25.001.sql b/core/resources/schemas/dbscripts/sqlserver/core-25.000-25.001.sql new file mode 100644 index 00000000000..e4af0fa1909 --- /dev/null +++ b/core/resources/schemas/dbscripts/sqlserver/core-25.000-25.001.sql @@ -0,0 +1 @@ +EXEC core.executeJavaUpgradeCode 'migrateAllowedExternalConnectionHosts'; diff --git a/core/src/org/labkey/core/CoreModule.java b/core/src/org/labkey/core/CoreModule.java index 525eb1f7e40..ace4488d70a 100644 --- a/core/src/org/labkey/core/CoreModule.java +++ b/core/src/org/labkey/core/CoreModule.java @@ -213,7 +213,6 @@ import org.labkey.core.admin.CustomizeMenuForm; import org.labkey.core.admin.DisplayFormatAnalyzer; import org.labkey.core.admin.DisplayFormatValidationProviderFactory; -import org.labkey.core.admin.ExternalServerType; import org.labkey.core.admin.FilesSiteSettingsAction; import org.labkey.core.admin.MenuViewFactory; import org.labkey.core.admin.importer.FolderTypeImporterFactory; @@ -276,6 +275,7 @@ import org.labkey.core.reports.DocumentConversionServiceImpl; import org.labkey.core.reports.ScriptEngineManagerImpl; import org.labkey.core.script.RhinoService; +import org.labkey.core.security.AllowedExternalResourceHosts; import org.labkey.core.security.ApiKeyViewProvider; import org.labkey.core.security.SecurityApiActions; import org.labkey.core.security.SecurityController; @@ -1050,7 +1050,8 @@ public void shutdownStarted() // populate look and feel settings and site settings with values read from startup properties as appropriate for not bootstrap populateLookAndFeelResourcesWithStartupProps(); - registerAllowedConnectionSources(); + AllowedExternalResourceHosts.registerStartupProperties(); + AllowedExternalResourceHosts.registerHosts(); WriteableLookAndFeelProperties.populateLookAndFeelWithStartupProps(); WriteableAppProps.populateSiteSettingsWithStartupProps(); // create users and groups and assign roles with values read from startup properties as appropriate for not bootstrap @@ -1241,16 +1242,6 @@ public void moduleStartupComplete(ServletContext servletContext) UserManager.addUserListener(new EmailPreferenceUserListener()); } - private void registerAllowedConnectionSources() - { - ContentSecurityPolicyFilter.registerAllowedConnectionSource( - ExternalServerType.getExternalSourceHostsKey(), - ExternalServerType.Source.getHosts().toArray(new String[0]) - ); - - LOG.debug("Registered [{}] as an allowed connection source", () -> String.join(", ", AppProps.getInstance().getExternalSourceHosts())); - } - // Issue 7527: Auto-detect missing sql views and attempt to recreate private void checkForMissingDbViews() { diff --git a/core/src/org/labkey/core/CoreUpgradeCode.java b/core/src/org/labkey/core/CoreUpgradeCode.java index 7c10438dbee..be1d5204592 100644 --- a/core/src/org/labkey/core/CoreUpgradeCode.java +++ b/core/src/org/labkey/core/CoreUpgradeCode.java @@ -15,6 +15,7 @@ */ package org.labkey.core; +import com.fasterxml.jackson.core.JsonProcessingException; import org.apache.logging.log4j.Logger; import org.jetbrains.annotations.NotNull; import org.labkey.api.collections.CaseInsensitiveHashSet; @@ -29,8 +30,13 @@ import org.labkey.api.data.UpgradeCode; import org.labkey.api.module.ModuleContext; import org.labkey.api.module.ModuleLoader; +import org.labkey.api.security.Directive; +import org.labkey.api.settings.AppProps; +import org.labkey.api.util.ExceptionUtil; import org.labkey.api.util.GUID; import org.labkey.api.util.logging.LogHelper; +import org.labkey.core.security.AllowedExternalResourceHosts; +import org.labkey.core.security.AllowedExternalResourceHosts.AllowedHost; import java.util.HashSet; import java.util.List; @@ -122,9 +128,9 @@ private static void toLowerCaseWithCounterSeqs(Container container) { TableInfo tableInfo = CoreSchema.getInstance().getTableInfoDbSequences(); SQLFragment toLowerSql = new SQLFragment("UPDATE ").append(tableInfo) - .append(" SET Name = LOWER(Name) ") - .append(" WHERE Container = ? AND NAME LIKE 'SampleNameGenCounter-%'") - .add(container); + .append(" SET Name = LOWER(Name) ") + .append(" WHERE Container = ? AND NAME LIKE 'SampleNameGenCounter-%'") + .add(container); new SqlExecutor(tableInfo.getSchema()).execute(toLowerSql); } @@ -141,9 +147,9 @@ public static void makeWithCounterCaseInsensitive(ModuleContext context) TableInfo tableInfo = CoreSchema.getInstance().getTableInfoDbSequences(); SQLFragment sql = new SQLFragment() - .append("SELECT DISTINCT Container\n") - .append("FROM ").append(tableInfo, "seq") - .append(" WHERE seq.NAME LIKE 'SampleNameGenCounter-%'"); + .append("SELECT DISTINCT Container\n") + .append("FROM ").append(tableInfo, "seq") + .append(" WHERE seq.NAME LIKE 'SampleNameGenCounter-%'"); @NotNull List containers = new SqlSelector(tableInfo.getSchema(), sql).getArrayList(String.class); if (containers.isEmpty()) @@ -167,4 +173,21 @@ public static void makeWithCounterCaseInsensitive(ModuleContext context) LOG.info("** finished upgrade withCounter DBSequences for container: " + container.getPath()); } } + + /** + * Called from core-25.000-25.001.sql + */ + @SuppressWarnings("unused") + public static void migrateAllowedExternalConnectionHosts(ModuleContext context) + { + if (context.isNewInstall()) + return; + + List hosts = AppProps.getInstance().getExternalSourceHosts(); + List allowedHosts = hosts.stream() + .map(host -> new AllowedHost(Directive.Connection, host)) + .toList(); + + AllowedExternalResourceHosts.saveAllowedHosts(allowedHosts, context.getUpgradeUser()); + } } \ No newline at end of file diff --git a/core/src/org/labkey/core/admin/AdminController.java b/core/src/org/labkey/core/admin/AdminController.java index edbdcba81f1..ab8e4e4e271 100644 --- a/core/src/org/labkey/core/admin/AdminController.java +++ b/core/src/org/labkey/core/admin/AdminController.java @@ -17,6 +17,7 @@ import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.annotation.JsonIgnoreProperties; +import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; import com.google.common.base.Joiner; import com.google.common.util.concurrent.UncheckedExecutionException; @@ -25,6 +26,7 @@ import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; import org.apache.commons.beanutils.ConversionException; +import org.apache.commons.collections4.MultiValuedMap; import org.apache.commons.collections4.map.LRUMap; import org.apache.commons.io.FileUtils; import org.apache.commons.lang3.ArrayUtils; @@ -92,6 +94,7 @@ import org.labkey.api.cloud.CloudStoreService; import org.labkey.api.collections.CaseInsensitiveHashMap; import org.labkey.api.collections.CaseInsensitiveHashSet; +import org.labkey.api.collections.CaseInsensitiveHashSetValuedMap; import org.labkey.api.compliance.ComplianceFolderSettings; import org.labkey.api.compliance.ComplianceService; import org.labkey.api.compliance.PhiColumnBehavior; @@ -181,6 +184,7 @@ import org.labkey.api.security.ActionNames; import org.labkey.api.security.AdminConsoleAction; import org.labkey.api.security.CSRF; +import org.labkey.api.security.Directive; import org.labkey.api.security.Group; import org.labkey.api.security.GroupManager; import org.labkey.api.security.IgnoresTermsOfUse; @@ -317,6 +321,8 @@ import org.labkey.core.query.CoreQuerySchema; import org.labkey.core.query.PostgresUserSchema; import org.labkey.core.reports.ExternalScriptEngineDefinitionImpl; +import org.labkey.core.security.AllowedExternalResourceHosts; +import org.labkey.core.security.AllowedExternalResourceHosts.AllowedHost; import org.labkey.core.security.BlockListFilter; import org.labkey.core.security.SecurityController; import org.labkey.data.xml.TablesDocument; @@ -452,9 +458,9 @@ public static void registerAdminConsoleLinks() AdminConsole.addLink(Configuration, "short urls", new ActionURL(ShortURLAdminAction.class, root), AdminPermission.class); AdminConsole.addLink(Configuration, "site settings", new AdminUrlsImpl().getCustomizeSiteURL()); AdminConsole.addLink(Configuration, "system maintenance", new ActionURL(ConfigureSystemMaintenanceAction.class, root)); - AdminConsole.addLink(Configuration, "external redirect hosts", new ActionURL(ExternalHostsAdminAction.class, root).addParameter("type", ExternalServerType.Redirect.name()), TroubleshooterPermission.class); - AdminConsole.addLink(Configuration, "external allowed sources", new ActionURL(ExternalHostsAdminAction.class, root).addParameter("type", ExternalServerType.Source.name()), TroubleshooterPermission.class); - AdminConsole.addLink(Configuration, "allowed file extensions", new ActionURL(ExternalHostsAdminAction.class, root).addParameter("type", ExternalServerType.FileExtension.name()), TroubleshooterPermission.class); + AdminConsole.addLink(Configuration, "allowed external redirect hosts", new ActionURL(AllowListAction.class, root).addParameter("type", AllowListType.Redirect.name()), TroubleshooterPermission.class); + AdminConsole.addLink(Configuration, "allowed external resource hosts", new ActionURL(ExternalSourcesAction.class, root), TroubleshooterPermission.class); + AdminConsole.addLink(Configuration, "allowed file extensions", new ActionURL(AllowListAction.class, root).addParameter("type", AllowListType.FileExtension.name()), TroubleshooterPermission.class); // Diagnostics AdminConsole.addLink(Diagnostics, "actions", new ActionURL(ActionsAction.class, root)); @@ -10841,30 +10847,30 @@ public class SuspiciousAction extends SimpleViewAction public ModelAndView getView(Object o, BindException errors) { Collection list = BlockListFilter.reportSuspicious(); - HtmlStringBuilder sb = HtmlStringBuilder.of(); + HtmlStringBuilder html = HtmlStringBuilder.of(); if (list.isEmpty()) { - sb.append("No suspicious activity.\n"); + html.append("No suspicious activity.\n"); } else { - sb.unsafeAppend(""); - sb.unsafeAppend("\n"); + html.unsafeAppend("
host (user)user-agentcount
") + .unsafeAppend("\n"); for (BlockListFilter.Suspicious s : list) { - sb.unsafeAppend("\n"); + html.append(HtmlString.NBSP).append("(" + s.user + ")"); + html.unsafeAppend("\n"); } - sb.unsafeAppend("
host (user)user-agentcount
") - .append(s.host); + html.unsafeAppend("
") + .append(s.host); if (!isBlank(s.user)) - sb.append(HtmlString.NBSP).append("(" + s.user + ")"); - sb.unsafeAppend("") - .append(s.userAgent) - .unsafeAppend("") - .append(s.count) - .unsafeAppend("
") + .append(s.userAgent) + .unsafeAppend("") + .append(s.count) + .unsafeAppend("
"); + html.unsafeAppend(""); } - return new HtmlView(sb); + return new HtmlView(html); } @Override @@ -10880,7 +10886,7 @@ public void addNavTrail(NavTree root) @Marshal(Marshaller.Jackson) @RequiresNoPermission @AllowedBeforeInitialUserIsSet - public class ConfigurationSummaryAction extends ReadOnlyApiAction + public static class ConfigurationSummaryAction extends ReadOnlyApiAction { @Override public Object execute(Object o, BindException errors) @@ -10902,6 +10908,37 @@ protected ObjectMapper createResponseObjectMapper() result.addMixIn(ExternalScriptEngineDefinitionImpl.class, IgnorePasswordMixIn.class); return result; } + + /* returns a jackson serializable object that reports superset of information returned in admin console */ + private JSONObject getConfigurationJson() + { + JSONObject res = new JSONObject(); + + res.put("server", AdminBean.getPropertyMap()); + + final Map> sets = new TreeMap<>(); + new SqlSelector(CoreSchema.getInstance().getScope(), + new SQLFragment("SELECT category, name, value FROM prop.propertysets PS inner join prop.properties P on PS.\"set\" = P.\"set\"\n" + + "WHERE objectid = ? AND category IN ('SiteConfig') AND encryption='None' AND LOWER(name) NOT LIKE '%password%'", ContainerManager.getRoot())).forEachMap(m -> + { + String category = (String)m.get("category"); + String name = (String)m.get("name"); + Object value = m.get("value"); + if (!sets.containsKey(category)) + sets.put(category, new TreeMap<>()); + sets.get(category).put(name,value); + } + ); + res.put("siteSettings", sets); + + HealthCheck.Result result = HealthCheckRegistry.get().checkHealth(Arrays.asList("all")); + res.put("health", result); + + LabKeyScriptEngineManager mgr = LabKeyScriptEngineManager.get(); + res.put("scriptEngines", mgr.getEngineDefinitions()); + + return res; + } } @JsonIgnoreProperties(value = { "password", "changePassword", "configuration" }) @@ -10910,25 +10947,26 @@ private static class IgnorePasswordMixIn } @AdminConsoleAction() - public class ExternalHostsAdminAction extends FormViewAction + public class AllowListAction extends FormViewAction { - private ExternalServerType _type; + private AllowListType _type; + @Override - public void validateCommand(ExternalHostsForm target, Errors errors) + public void validateCommand(AllowListForm target, Errors errors) { } @Override - public ModelAndView getView(ExternalHostsForm form, boolean reshow, BindException errors) + public ModelAndView getView(AllowListForm form, boolean reshow, BindException errors) { _type = form.getTypeEnum(); - form.setExistingHostsList(form.getTypeEnum().getHosts()); + form.setExistingValuesList(form.getTypeEnum().getValues()); - JspView newView = new JspView<>("/org/labkey/core/admin/addNewExternalHost.jsp", form, errors); + JspView newView = new JspView<>("/org/labkey/core/admin/addNewListValue.jsp", form, errors); newView.setTitle("Register New " + form.getTypeEnum().getTitle()); newView.setFrame(WebPartView.FrameType.PORTAL); - JspView existingView = new JspView<>("/org/labkey/core/admin/existingExternalHosts.jsp", form, errors); + JspView existingView = new JspView<>("/org/labkey/core/admin/existingListValues.jsp", form, errors); existingView.setTitle("Existing " + form.getTypeEnum().getTitle() + "s"); existingView.setFrame(WebPartView.FrameType.PORTAL); @@ -10936,20 +10974,20 @@ public ModelAndView getView(ExternalHostsForm form, boolean reshow, BindExceptio } @Override - public boolean handlePost(ExternalHostsForm form, BindException errors) throws Exception + public boolean handlePost(AllowListForm form, BindException errors) throws Exception { - ExternalServerType hostType = form.getTypeEnum(); - //handle delete of existing external redirect host + AllowListType allowListType = form.getTypeEnum(); + //handle delete of existing value if (form.isDelete()) { - String urlToDelete = form.getExistingExternalHost(); - List hosts = hostType.getHosts(); - for (String externalHost : hosts) + String urlToDelete = form.getExistingValue(); + List values = allowListType.getValues(); + for (String value : values) { - if (null != urlToDelete && urlToDelete.trim().equalsIgnoreCase(externalHost.trim())) + if (null != urlToDelete && urlToDelete.trim().equalsIgnoreCase(value.trim())) { - hosts.remove(externalHost); - hostType.setHosts(hosts, getUser()); + values.remove(value); + allowListType.setValues(values, getUser()); break; } } @@ -10957,27 +10995,27 @@ public boolean handlePost(ExternalHostsForm form, BindException errors) throws E //handle updates - clicking on Save button under Existing will save the updated urls else if (form.isSaveAll()) { - Set validatedHosts = form.validateHostList(errors); + Set validatedValues = form.validateValues(errors); if (errors.hasErrors()) return false; - hostType.setHosts(validatedHosts.stream().toList(), getUser()); + allowListType.setValues(validatedValues.stream().toList(), getUser()); } - //save new external host + //save new external value else if (form.isSaveNew()) { - Set hostSet = form.validateNewExternalHost(errors); + Set valueSet = form.validateNewValue(errors); if (errors.hasErrors()) return false; - hostType.setHosts(hostSet, getUser()); + allowListType.setValues(valueSet, getUser()); } return true; } @Override - public URLHelper getSuccessURL(ExternalHostsForm form) + public URLHelper getSuccessURL(AllowListForm form) { return form.getTypeEnum().getSuccessURL(getContainer()); } @@ -10990,36 +11028,38 @@ public void addNavTrail(NavTree root) } } - public static class ExternalHostsForm + public static class AllowListForm { - private String _newExternalHost; - private String _existingExternalHost; + private String _newValue; + private String _existingValue; private boolean _delete; - private String _existingExternalHosts; + private String _existingValues; private boolean _saveAll; private boolean _saveNew; private String _type; - private List _existingHostURLList; + private List _existingValuesList; - public String getNewExternalHost() + public String getNewValue() { - return _newExternalHost; + return _newValue; } - public void setNewExternalHost(String newExternalHost) + @SuppressWarnings("unused") + public void setNewValue(String newValue) { - _newExternalHost = newExternalHost; + _newValue = newValue; } - public String getExistingExternalHost() + public String getExistingValue() { - return _existingExternalHost; + return _existingValue; } - public void setExistingExternalHost(String existingExternalHost) + @SuppressWarnings("unused") + public void setExistingValue(String existingValue) { - _existingExternalHost = existingExternalHost; + _existingValue = existingValue; } public boolean isDelete() @@ -11027,19 +11067,21 @@ public boolean isDelete() return _delete; } + @SuppressWarnings("unused") public void setDelete(boolean delete) { _delete = delete; } - public String getExistingExternalHosts() + public String getExistingValues() { - return _existingExternalHosts; + return _existingValues; } - public void setExistingExternalHosts(String existingExternalHosts) + @SuppressWarnings("unused") + public void setExistingValues(String existingValues) { - _existingExternalHosts = existingExternalHosts; + _existingValues = existingValues; } public boolean isSaveAll() @@ -11047,6 +11089,7 @@ public boolean isSaveAll() return _saveAll; } + @SuppressWarnings("unused") public void setSaveAll(boolean saveAll) { _saveAll = saveAll; @@ -11057,24 +11100,26 @@ public boolean isSaveNew() return _saveNew; } + @SuppressWarnings("unused") public void setSaveNew(boolean saveNew) { _saveNew = saveNew; } - public List getExistingHostList() + public List getExistingValuesList() { //for updated urls that comes in as String values from the jsp/html form - if (null != getExistingExternalHosts()) + if (null != getExistingValues()) { - return new ArrayList<>(Arrays.asList(getExistingExternalHosts().split("\n"))); + // The JavaScript delimits with "\n". Not sure where these "\r"s are coming from, but we need to strip them. + return new ArrayList<>(Arrays.asList(getExistingValues().replace("\r", "").split("\n"))); } - return _existingHostURLList; + return _existingValuesList; } - public void setExistingHostsList(List urlList) + public void setExistingValuesList(List valuesList) { - _existingHostURLList = urlList; + _existingValuesList = valuesList; } public String getType() @@ -11089,90 +11134,331 @@ public void setType(String type) } @NotNull - public ExternalServerType getTypeEnum() + public AllowListType getTypeEnum() { - return EnumUtils.getEnum(ExternalServerType.class, getType(), ExternalServerType.Redirect); + return EnumUtils.getEnum(AllowListType.class, getType(), AllowListType.Redirect); } @JsonIgnore - public Set validateNewExternalHost(BindException errors) + public Set validateNewValue(BindException errors) { - String newExternalHost = StringUtils.trimToEmpty(getNewExternalHost()); - getTypeEnum().validateHostFormat(newExternalHost, errors); + String value = StringUtils.trimToEmpty(getNewValue()); + getTypeEnum().validateValueFormat(value, errors); if (errors.hasErrors()) return null; - Set hostSet = new CaseInsensitiveHashSet(getTypeEnum().getHosts()); - checkDuplicatesByAddition(newExternalHost, hostSet, errors); - return hostSet; + Set valueSet = new CaseInsensitiveHashSet(getTypeEnum().getValues()); + checkDuplicatesByAddition(value, valueSet, errors); + return valueSet; } @JsonIgnore - public Set validateHostList(BindException errors) + public Set validateValues(BindException errors) { - List hosts = getExistingHostList(); //get hosts from the form, this includes updated hosts - Set hostSet = new CaseInsensitiveHashSet(); + List values = getExistingValuesList(); //get values from the form, this includes updated values + Set valueSet = new CaseInsensitiveHashSet(); - if (null != hosts && !hosts.isEmpty()) + if (null != values && !values.isEmpty()) { - for (String host : hosts) + for (String value : values) { - getTypeEnum().validateHostFormat(host, errors); + getTypeEnum().validateValueFormat(value, errors); if (errors.hasErrors()) continue; - checkDuplicatesByAddition(host, hostSet, errors); + checkDuplicatesByAddition(value, valueSet, errors); } } - return hostSet; + return valueSet; } /** - * Adds host to host set unless it is a duplicate, in which case it adds an error - * Note: Attempts to validate host string using URLHelper - * @param host to check - * @param hostSet of existing hosts + * Adds value to value set unless it is a duplicate, in which case it adds an error + * @param value to check + * @param valueSet of existing values * @param errors collections of errors observed */ @JsonIgnore - private void checkDuplicatesByAddition(String host, Set hostSet, BindException errors) + private void checkDuplicatesByAddition(String value, Set valueSet, BindException errors) { - String trimHost = StringUtils.trimToEmpty(host); - if (!hostSet.add(trimHost)) - errors.addError(new LabKeyError(String.format("'%1$s' already exists. Duplicate hosts not allowed.", trimHost))); + String trimValue = StringUtils.trimToEmpty(value); + if (!valueSet.add(trimValue)) + errors.addError(new LabKeyError(String.format("'%1$s' already exists. Duplicate values not allowed.", trimValue))); } } - /* returns a jackson serializable object that reports superset of information returned in admin console */ - JSONObject getConfigurationJson() + public static class ExternalSourcesForm { - JSONObject res = new JSONObject(); + private boolean _delete; + private boolean _saveNew; + private boolean _saveAll; + + private String _newDirective; + private String _newHost; + private String _existingValue; + private String _existingValues; + + public boolean isDelete() + { + return _delete; + } + + @SuppressWarnings("unused") + public void setDelete(boolean delete) + { + _delete = delete; + } + + public boolean isSaveNew() + { + return _saveNew; + } + + @SuppressWarnings("unused") + public void setSaveNew(boolean saveNew) + { + _saveNew = saveNew; + } + + public boolean isSaveAll() + { + return _saveAll; + } - res.put("server", AdminBean.getPropertyMap()); + @SuppressWarnings("unused") + public void setSaveAll(boolean saveAll) + { + _saveAll = saveAll; + } + + public String getNewDirective() + { + return _newDirective; + } - final Map> sets = new TreeMap<>(); - new SqlSelector(CoreSchema.getInstance().getScope(), - new SQLFragment("SELECT category, name, value FROM prop.propertysets PS inner join prop.properties P on PS.\"set\" = P.\"set\"\n" + - "WHERE objectid = ? AND category IN ('SiteConfig') AND encryption='None' AND LOWER(name) NOT LIKE '%password%'", ContainerManager.getRoot())).forEachMap(m -> + @SuppressWarnings("unused") + public void setNewDirective(String newDirective) + { + _newDirective = newDirective; + } + + public String getNewHost() + { + return _newHost; + } + + @SuppressWarnings("unused") + public void setNewHost(String newHost) + { + _newHost = newHost; + } + + public String getExistingValue() + { + return _existingValue; + } + + @SuppressWarnings("unused") + public void setExistingValue(String existingValue) + { + _existingValue = existingValue; + } + + public String getExistingValues() + { + // The JSP JavaScript delimits with "\n". Not sure where these "\r"s are coming from, but we need to strip them. + return _existingValues.replace("\r", ""); + } + + @SuppressWarnings("unused") + public void setExistingValues(String existingValues) + { + _existingValues = existingValues; + } + + private AllowedHost getExistingAllowedHost(BindException errors) + { + return getAllowedHost(getExistingValue(), errors); + } + + private AllowedHost getAllowedHost(String value, BindException errors) + { + String[] parts = value.split("\\|", 2); // Stop after the first bar to produce two parts + if (parts.length != 2) { - String category = (String)m.get("category"); - String name = (String)m.get("name"); - Object value = m.get("value"); - if (!sets.containsKey(category)) - sets.put(category, new TreeMap<>()); - sets.get(category).put(name,value); + errors.addError(new LabKeyError("Can't parse allowed host.")); + return null; } - ); - res.put("siteSettings", sets); + return validateHost(parts[0], parts[1], errors); + } - HealthCheck.Result result = HealthCheckRegistry.get().checkHealth(Arrays.asList("all")); - res.put("health", result); + private List getExistingAllowedHosts(BindException errors) + { + List existing = Arrays.stream(getExistingValues().split("\n")) + .map(value-> getAllowedHost(value, errors)) + .toList(); - LabKeyScriptEngineManager mgr = LabKeyScriptEngineManager.get(); - res.put("scriptEngines", mgr.getEngineDefinitions()); + if (errors.hasErrors()) + return null; - return res; + return checkDuplicates(existing, errors); + } + + private List validateNewAllowedHost(BindException errors) throws JsonProcessingException + { + AllowedHost newAllowedHost = validateHost(getNewDirective(), getNewHost(), errors); + + if (errors.hasErrors()) + return null; + + List hosts = getSavedAllowedHosts(); + hosts.add(newAllowedHost); + + return checkDuplicates(hosts, errors); + } + + // Lenient for now: no unknown directives, no blank hosts or hosts with semicolons + public static AllowedHost validateHost(String directiveString, String host, BindException errors) + { + AllowedHost ret = null; + + if (StringUtils.isEmpty(directiveString)) + { + errors.addError(new LabKeyError("Directive must not be blank")); + } + else if (StringUtils.isEmpty(host)) + { + errors.addError(new LabKeyError("Host must not be blank")); + } + else if (host.contains(";")) + { + errors.addError(new LabKeyError("Semicolons are not allowed in host names")); + } + else + { + Directive directive = EnumUtils.getEnum(Directive.class, directiveString); + + if (null == directive) + { + errors.addError(new LabKeyError("Unknown directive: " + directiveString)); + } + else + { + ret = new AllowedHost(directive, host.trim()); + } + } + + return ret; + } + + /** + * Check for duplicates in hosts: within each Directive, hosts are checked using case-insensitive comparisons + + * @param hosts a list of AllowedHost objects to check for duplicates + * @param errors errors to populate + * @return hosts if there are no duplicates, otherwise {@code null} + */ + public static @Nullable List checkDuplicates(List hosts, BindException errors) + { + // Not a simple Set check since we want host check to be case-insensitive + MultiValuedMap map = new CaseInsensitiveHashSetValuedMap<>(); + + hosts.forEach(allowedHost -> { + String host = allowedHost.host().trim(); + if (!map.put(allowedHost.directive(), host)) + errors.addError(new LabKeyError(String.format("'%1$s' already exists. Duplicate values are not allowed.", allowedHost))); + }); + + return errors.hasErrors() ? null : hosts; + } + + // Returns a mutable list + public List getSavedAllowedHosts() throws JsonProcessingException + { + return AllowedExternalResourceHosts.readAllowedHosts(); + } + } + + @AdminConsoleAction() + public class ExternalSourcesAction extends FormViewAction + { + @Override + public void validateCommand(ExternalSourcesForm form, Errors errors) + { + } + + @Override + public ModelAndView getView(ExternalSourcesForm form, boolean reshow, BindException errors) + { + boolean isTroubleshooter = !getContainer().hasPermission(getUser(), ApplicationAdminPermission.class); + + JspView newView = new JspView<>("/org/labkey/core/admin/addNewExternalSource.jsp", form, errors); + newView.setTitle(isTroubleshooter ? "Overview" : "Register New External Resource Host"); + newView.setFrame(WebPartView.FrameType.PORTAL); + JspView existingView = new JspView<>("/org/labkey/core/admin/existingExternalSources.jsp", form, errors); + existingView.setTitle("Existing External Resource Hosts"); + existingView.setFrame(WebPartView.FrameType.PORTAL); + + return new VBox(newView, existingView); + } + + @Override + public boolean handlePost(ExternalSourcesForm form, BindException errors) throws Exception + { + List allowedHosts = null; + + //handle delete of existing value + if (form.isDelete()) + { + AllowedHost subToDelete = form.getExistingAllowedHost(errors); + if (errors.hasErrors()) + return false; + allowedHosts = form.getSavedAllowedHosts(); + var iter = allowedHosts.listIterator(); + while (iter.hasNext()) + { + AllowedHost sub = iter.next(); + if (sub.equals(subToDelete)) + { + iter.remove(); + break; + } + } + } + //handle updates - clicking on Save button under Existing will save the updated urls + else if (form.isSaveAll()) + { + allowedHosts = form.getExistingAllowedHosts(errors); + if (errors.hasErrors()) + return false; + } + //save new external value + else if (form.isSaveNew()) + { + allowedHosts = form.validateNewAllowedHost(errors); + } + + if (errors.hasErrors()) + return false; + + AllowedExternalResourceHosts.saveAllowedHosts(allowedHosts, getUser()); + + return true; + } + + @Override + public URLHelper getSuccessURL(ExternalSourcesForm form) + { + return null; + } + + @Override + public void addNavTrail(NavTree root) + { + setHelpTopic("externalHosts"); + addAdminNavTrail(root, "Allowed External Resource Hosts", getClass()); + } } @RequiresPermission(AdminPermission.class) @@ -11708,6 +11994,9 @@ public Object execute(SimpleApiJsonForm form, BindException errors) throws Excep String labkeyVersion = request.getParameter("labkeyVersion"); if (null != labkeyVersion) jsonObj.put("labkeyVersion", labkeyVersion); + String cspVersion = request.getParameter("cspVersion"); + if (null != cspVersion) + jsonObj.put("cspVersion", cspVersion); var jsonStr = jsonObj.toString(2); _log.warn("ContentSecurityPolicy warning on page: " + urlString + "\n" + jsonStr); } diff --git a/core/src/org/labkey/core/admin/ExternalServerType.java b/core/src/org/labkey/core/admin/AllowListType.java similarity index 51% rename from core/src/org/labkey/core/admin/ExternalServerType.java rename to core/src/org/labkey/core/admin/AllowListType.java index d7e4ed5258a..e5a9eea79ae 100644 --- a/core/src/org/labkey/core/admin/ExternalServerType.java +++ b/core/src/org/labkey/core/admin/AllowListType.java @@ -6,69 +6,21 @@ import org.labkey.api.action.LabKeyError; import org.labkey.api.data.Container; import org.labkey.api.security.User; -import org.labkey.api.security.Directive; import org.labkey.api.settings.AppProps; import org.labkey.api.settings.WriteableAppProps; import org.labkey.api.util.HtmlString; import org.labkey.api.util.URLHelper; import org.labkey.api.view.ActionURL; -import org.labkey.filters.ContentSecurityPolicyFilter; import org.springframework.validation.BindException; import java.util.Collection; import java.util.List; -public enum ExternalServerType +public enum AllowListType { - Source { - @Override - public HtmlString getDescription() - { - return HtmlString.unsafe(""" -
-

- For security reasons, LabKey Server restricts the hosts that can be used as resource origins. By default, only LabKey sources are allowed, other server URLs must be configured below to enable them to be used as script sources. - For more information on the security concern, please refer to the OWASP cheat sheet. -

-

- Add allowed source URLs or IP address as they will be referenced in script source values. - For example: www.myexternalhost.com or 1.2.3.4 -

-
- """); - } - - @Override - public List getHosts() - { - return AppProps.getInstance().getExternalSourceHosts(); - } - - @Override - public void setHosts(Collection hosts, User user) - { - WriteableAppProps props = AppProps.getWriteableInstance(); - props.setExternalSourceHosts(hosts); - props.save(user); - - // Refresh the CSP with new values. - ContentSecurityPolicyFilter.unregisterAllowedSources(Directive.Connection, EXTERNAL_SOURCE_HOSTS_KEY); - ContentSecurityPolicyFilter.registerAllowedSources(Directive.Connection, EXTERNAL_SOURCE_HOSTS_KEY, getHosts().toArray(new String[0])); - } - - @Override - public HtmlString getTitle() - { - return HtmlString.of(String.format("External %1$s Host", name())); - } - - @Override - public HtmlString getLabel() - { - return HtmlString.of("Host"); - } - }, Redirect { + private static final AuthorityValidator AUTHORITY_VALIDATOR = new AuthorityValidator(UrlValidator.ALLOW_LOCAL_URLS); + @Override public HtmlString getDescription() { @@ -90,19 +42,33 @@ public HtmlString getDescription() } @Override - public List getHosts() + public List getValues() { return AppProps.getInstance().getExternalRedirectHosts(); } @Override - public void setHosts(Collection hosts, User user) + public void setValues(Collection hosts, User user) { WriteableAppProps props = AppProps.getWriteableInstance(); props.setExternalRedirectHosts(hosts); props.save(user); } + @Override + @JsonIgnore + public void validateValueFormat(String host, BindException errors) + { + if (StringUtils.isEmpty(host)) + { + errors.addError(new LabKeyError("Redirect host name must not be blank.")); + } + else if (!AUTHORITY_VALIDATOR.isValidAuthority(host)) + { + errors.addError(new LabKeyError(String.format("Redirect host name %1$s is not formatted correctly", host))); + } + } + @Override public HtmlString getTitle() { @@ -132,13 +98,13 @@ public HtmlString getDescription() } @Override - public List getHosts() + public List getValues() { return AppProps.getInstance().getAllowedExtensions(); } @Override - public void setHosts(Collection allowedExtensions, User user) + public void setValues(Collection allowedExtensions, User user) { WriteableAppProps props = AppProps.getWriteableInstance(); props.setAllowedFileExtensions(allowedExtensions); @@ -146,11 +112,12 @@ public void setHosts(Collection allowedExtensions, User user) } @Override - public void validateHostFormat(String externalHost, BindException errors) + @JsonIgnore + public void validateValueFormat(String value, BindException errors) { - if (StringUtils.isEmpty(externalHost)) + if (StringUtils.isEmpty(value)) errors.addError(new LabKeyError("File extension must not be blank.")); - else if (!externalHost.startsWith(".")) + else if (!value.startsWith(".")) errors.addError(new LabKeyError("File extension must start with a '.'")); } @@ -167,20 +134,13 @@ public HtmlString getLabel() } }; - private static final AuthorityValidator AUTHORITY_VALIDATOR = new AuthorityValidator(UrlValidator.ALLOW_LOCAL_URLS); - private static final String EXTERNAL_SOURCE_HOSTS_KEY = "External Sources"; - public static String getExternalSourceHostsKey() - { - return EXTERNAL_SOURCE_HOSTS_KEY; - } - public abstract HtmlString getDescription(); - public abstract List getHosts(); - public abstract void setHosts(Collection redirectHosts, User user); + public abstract List getValues(); + public abstract void setValues(Collection redirectHosts, User user); + public abstract void validateValueFormat(String value, BindException errors); public abstract HtmlString getTitle(); public abstract HtmlString getLabel(); - public String getHelpTopic() { return "externalHosts#" + name().toLowerCase(); @@ -188,34 +148,6 @@ public String getHelpTopic() public URLHelper getSuccessURL(Container container) { - return new ActionURL(AdminController.ExternalHostsAdminAction .class, container).addParameter("type", name()); + return new ActionURL(AdminController.AllowListAction.class, container).addParameter("type", name()); } - - @JsonIgnore - public void validateHostFormat(String externalHost, BindException errors) - { - if (StringUtils.isEmpty(externalHost)) - { - errors.addError(new LabKeyError("External host name must not be blank.")); - } - else if (!AUTHORITY_VALIDATOR.isValidAuthority(externalHost)) - { - errors.addError(new LabKeyError(String.format("External host name %1$s is not formatted correctly", externalHost))); - } - } - - private static class AuthorityValidator extends UrlValidator - { - public AuthorityValidator(long options) - { - super(options); - } - - @Override - public boolean isValidAuthority(String authority) - { - String base = authority.startsWith("*.") ? authority.substring(2) : authority; - return super.isValidAuthority(base); - } - }; } diff --git a/core/src/org/labkey/core/admin/AuthorityValidator.java b/core/src/org/labkey/core/admin/AuthorityValidator.java new file mode 100644 index 00000000000..3a18d4f3bc8 --- /dev/null +++ b/core/src/org/labkey/core/admin/AuthorityValidator.java @@ -0,0 +1,18 @@ +package org.labkey.core.admin; + +import org.apache.commons.validator.routines.UrlValidator; + +class AuthorityValidator extends UrlValidator +{ + public AuthorityValidator(long options) + { + super(options); + } + + @Override + public boolean isValidAuthority(String authority) + { + String base = authority.startsWith("*.") ? authority.substring(2) : authority; + return super.isValidAuthority(base); + } +} diff --git a/core/src/org/labkey/core/admin/addNewExternalSource.jsp b/core/src/org/labkey/core/admin/addNewExternalSource.jsp new file mode 100644 index 00000000000..e3b9e5a42c1 --- /dev/null +++ b/core/src/org/labkey/core/admin/addNewExternalSource.jsp @@ -0,0 +1,123 @@ +<% +/* + * Copyright (c) 2019 LabKey Corporation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +%> +<%@ page import="org.apache.commons.lang3.EnumUtils" %> +<%@ page import="org.labkey.api.collections.LabKeyCollectors" %> +<%@ page import="org.labkey.api.data.Container" %> +<%@ page import="org.labkey.api.security.Directive" %> +<%@ page import="org.labkey.api.security.permissions.ApplicationAdminPermission" %> +<%@ page import="org.labkey.api.settings.OptionalFeatureService" %> +<%@ page import="org.labkey.core.admin.AdminController.ExternalSourcesForm" %> +<%@ page import="org.labkey.filters.ContentSecurityPolicyFilter" %> +<%@ page import="org.labkey.filters.ContentSecurityPolicyFilter.ContentSecurityPolicyType" %> +<%@ page import="java.util.Arrays" %> +<%@ page import="java.util.List" %> +<%@ page import="static org.labkey.filters.ContentSecurityPolicyFilter.FEATURE_FLAG_DISABLE_ENFORCE_CSP" %> +<%@ page extends="org.labkey.api.jsp.JspBase" %> +<%@ taglib prefix="labkey" uri="http://www.labkey.org/taglib" %> +<% + Container c = getContainer(); + boolean isTroubleshooter = !c.hasPermission(getUser(), ApplicationAdminPermission.class); + + String noEffect = "External resource hosts can be configured below, but they'll have no effect until "; + String message; + + boolean hasEnforce = ContentSecurityPolicyFilter.hasCsp(ContentSecurityPolicyType.Enforce); + if (hasEnforce) + { + message = "This server is configured with an enforce Content Security Policy (CSP) "; + boolean disabled = OptionalFeatureService.get().isFeatureEnabled(FEATURE_FLAG_DISABLE_ENFORCE_CSP); + + if (disabled) + { + message += "but it's currently disabled via an experimental feature flag! " + noEffect + "the enforce CSP is re-enabled."; + } + else + { + List missing = ContentSecurityPolicyFilter.getMissingSubstitutions(ContentSecurityPolicyType.Enforce); + int count = missing.size(); + message += missing.isEmpty() ? + "that includes all the expected substitutions." : + "but the following substitution" + (count > 1 ? "s are" : " is") + " missing: " + String.join(", ", missing) + + ". External resource hosts for " + (count > 1 ? "these directives" : "this directive") + " can be configured below, but they'll have no effect."; + } + } + else + { + message = "This server is not configured with an enforce Content Security Policy (CSP); LabKey strongly recommends " + + "configuring a strict enforce CSP. " + noEffect + "an enforce CSP is configured."; + } +%> + +
+

+ <%=h(message)%> +

+

+ The standard LabKey CSP restricts hosts that browsers can use as resource origins. By default, only sources from + this server are allowed; other server hosts must be configured below to enable them to be used as external + sources. All provided hosts are added into the CSP using the \${} substitution key shown next to each directive. +<% + if (!isTroubleshooter) + { +%> + To allow a resource, pick a directive and add the associated hostname or IP address, for example: www.myexternalhost.com or 1.2.3.4. +<% + } +%> +

+

+ For information about configuring CSPs with LabKey, refer to our <%=helpLink("cspConfig", "Content Security Policy Configuration page")%>. +

+

+ For more information on the security concern, refer to the + <%=link("OWASP cheat sheet", "https://cheatsheetseries.owasp.org/cheatsheets/HTML5_Security_Cheat_Sheet.html#cross-origin-resource-sharing").target("_owasp").clearClasses()%>. +

+
+<% + if (!isTroubleshooter) + { + ExternalSourcesForm form = (ExternalSourcesForm)getModelBean(); + Directive directive = EnumUtils.getEnum(Directive.class, form.getNewDirective()); +%> + + + + + + + + + + + + + +
<%=select() + .name("newDirective") + .id("newDirective") + .addStyle("width:300px") + .selected(directive) + .addOptions( + Arrays.stream(Directive.values()) + .collect(LabKeyCollectors.toLinkedMap(Enum::name, d->d.getCspDirective() + " ${" + d.getSubstitutionKey() + "}") + ) + )%>

<%= button("Add").submit(true) %>
+ +<% + } +%> diff --git a/core/src/org/labkey/core/admin/addNewExternalHost.jsp b/core/src/org/labkey/core/admin/addNewListValue.jsp similarity index 70% rename from core/src/org/labkey/core/admin/addNewExternalHost.jsp rename to core/src/org/labkey/core/admin/addNewListValue.jsp index 5e76e16ece2..d592738ad66 100644 --- a/core/src/org/labkey/core/admin/addNewExternalHost.jsp +++ b/core/src/org/labkey/core/admin/addNewListValue.jsp @@ -1,8 +1,3 @@ -<%@ page import="org.labkey.api.admin.AdminUrls" %> -<%@ page import="org.labkey.api.data.Container" %> -<%@ page import="org.labkey.api.security.permissions.AdminOperationsPermission" %> -<%@ page import="org.labkey.core.admin.AdminController" %> -<%@ page import="org.labkey.api.view.HttpView" %> <% /* * Copyright (c) 2019 LabKey Corporation @@ -20,12 +15,17 @@ * limitations under the License. */ %> +<%@ page import="org.labkey.api.admin.AdminUrls" %> +<%@ page import="org.labkey.api.data.Container" %> +<%@ page import="org.labkey.api.security.permissions.ApplicationAdminPermission" %> +<%@ page import="org.labkey.api.view.HttpView" %> +<%@ page import="org.labkey.core.admin.AdminController.AllowListForm" %> <%@ page extends="org.labkey.api.jsp.JspBase" %> <%@ taglib prefix="labkey" uri="http://www.labkey.org/taglib" %> <% Container c = getContainer(); - boolean isTroubleshooter = c.isRoot() && !c.hasPermission(getUser(), AdminOperationsPermission.class); - AdminController.ExternalHostsForm bean = (AdminController.ExternalHostsForm) HttpView.currentModel(); + boolean isTroubleshooter = !c.hasPermission(getUser(), ApplicationAdminPermission.class);; + AllowListForm bean = (AllowListForm) HttpView.currentModel(); %> <%=bean.getTypeEnum().getDescription()%> @@ -43,8 +43,8 @@ - - + + diff --git a/core/src/org/labkey/core/admin/existingExternalSources.jsp b/core/src/org/labkey/core/admin/existingExternalSources.jsp new file mode 100644 index 00000000000..e39ecf11c2e --- /dev/null +++ b/core/src/org/labkey/core/admin/existingExternalSources.jsp @@ -0,0 +1,120 @@ +<% +/* + * Copyright (c) 2019 LabKey Corporation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +%> +<%@ page import="org.labkey.api.admin.AdminUrls" %> +<%@ page import="org.labkey.api.data.Container" %> +<%@ page import="org.labkey.api.security.permissions.ApplicationAdminPermission" %> +<%@ page import="org.labkey.api.util.HtmlString" %> +<%@ page import="org.labkey.api.view.HttpView" %> +<%@ page import="org.labkey.core.admin.AdminController.ExternalSourcesForm" %> +<%@ page import="org.labkey.core.security.AllowedExternalResourceHosts.AllowedHost" %> +<%@ page import="java.util.Comparator" %> +<%@ page import="java.util.List" %> +<%@ page extends="org.labkey.api.jsp.JspBase" %> +<%@ taglib prefix="labkey" uri="http://www.labkey.org/taglib" %> +<% + Container c = getContainer(); + boolean isTroubleshooter = !c.hasPermission(getUser(), ApplicationAdminPermission.class); +%> + + + + +<% + ExternalSourcesForm bean = (ExternalSourcesForm) HttpView.currentModel(); + List existingAllowedHosts = bean.getSavedAllowedHosts(); + existingAllowedHosts.sort(Comparator.comparing(AllowedHost::directive).thenComparing(AllowedHost::host)); +%> +

<%= button("Save").submit(true) %>
+<% + if (existingAllowedHosts.isEmpty()) + { +%> + +<% + } + else + { +%> + +<% + int num = 1; + for (AllowedHost sub : existingAllowedHosts) { + String directiveId = "directive" + num; + String hostId = "host" + num; +%> + + + + + + +<% + num++; + } + + if (!existingAllowedHosts.isEmpty()) + { +%> +
No External Resource Hosts have been configured.
DirectiveHost
/><%=isTroubleshooter ? + HtmlString.EMPTY_STRING : + button("Delete") + .primary(true) + .onClick("return deleteExisting(" + + q(sub.directive() + "|" + sub.host()) + // Using | separator is safe because directive name never contains it + ");") %> + +
+ + + +
<%=isTroubleshooter ? button("Done").href(urlProvider(AdminUrls.class).getAdminConsoleURL()) : button("Save").primary(true).onClick("return saveAll();")%> +<% + } + } +%> + diff --git a/core/src/org/labkey/core/admin/existingExternalHosts.jsp b/core/src/org/labkey/core/admin/existingListValues.jsp similarity index 62% rename from core/src/org/labkey/core/admin/existingExternalHosts.jsp rename to core/src/org/labkey/core/admin/existingListValues.jsp index bf2275da22d..d7c5d2cca55 100644 --- a/core/src/org/labkey/core/admin/existingExternalHosts.jsp +++ b/core/src/org/labkey/core/admin/existingListValues.jsp @@ -17,70 +17,70 @@ %> <%@ page import="org.labkey.api.admin.AdminUrls" %> <%@ page import="org.labkey.api.data.Container" %> -<%@ page import="org.labkey.api.security.permissions.AdminOperationsPermission" %> +<%@ page import="org.labkey.api.security.permissions.ApplicationAdminPermission" %> <%@ page import="org.labkey.api.util.HtmlString" %> <%@ page import="org.labkey.api.view.HttpView" %> -<%@ page import="org.labkey.core.admin.AdminController" %> +<%@ page import="org.labkey.core.admin.AdminController.AllowListForm" %> <%@ page extends="org.labkey.api.jsp.JspBase" %> <%@ taglib prefix="labkey" uri="http://www.labkey.org/taglib" %> <% Container c = getContainer(); - boolean isTroubleshooter = c.isRoot() && !c.hasPermission(getUser(), AdminOperationsPermission.class); + boolean isTroubleshooter = !c.hasPermission(getUser(), ApplicationAdminPermission.class); %> - + <% - AdminController.ExternalHostsForm bean = (AdminController.ExternalHostsForm) HttpView.currentModel(); + AllowListForm bean = (AllowListForm) HttpView.currentModel(); %> - <% if (bean.getExistingHostList().isEmpty()) { %> + <% if (bean.getExistingValuesList().isEmpty()) { %> <% } %> <% int num = 1; - for (String externalHost : bean.getExistingHostList()) { - String inputNameExisting = "existingExternalHost" + num; + for (String value : bean.getExistingValuesList()) { + String inputNameExisting = "existingValue" + num; %> - + - @@ -89,10 +89,10 @@ } %>
<%=h(bean.getTypeEnum().getTitle() + "s")%>
No <%=h(bean.getTypeEnum().getTitle())%>s have been configured.
<%=isTroubleshooter ? HtmlString.EMPTY_STRING : button("Delete").primary(true).onClick("return deleteExisting(\"" + h(externalHost) + "\");") %> + <%=isTroubleshooter ? HtmlString.EMPTY_STRING : button("Delete").primary(true).onClick("return deleteExisting(\"" + h(value) + "\");") %>
- <% if (!bean.getExistingHostList().isEmpty()) { %> + <% if (!bean.getExistingValuesList().isEmpty()) { %> - - + +
<%=isTroubleshooter ? button("Done").href(urlProvider(AdminUrls.class).getAdminConsoleURL()) : button("Save").primary(true).onClick("return saveAll();")%> diff --git a/core/src/org/labkey/core/security/AllowedExternalResourceHosts.java b/core/src/org/labkey/core/security/AllowedExternalResourceHosts.java new file mode 100644 index 00000000000..39fcb8fe768 --- /dev/null +++ b/core/src/org/labkey/core/security/AllowedExternalResourceHosts.java @@ -0,0 +1,134 @@ +package org.labkey.core.security; + +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.core.type.TypeReference; +import org.apache.logging.log4j.Logger; +import org.jetbrains.annotations.Nullable; +import org.labkey.api.module.ModuleLoader; +import org.labkey.api.security.Directive; +import org.labkey.api.security.User; +import org.labkey.api.settings.AppProps; +import org.labkey.api.settings.StandardStartupPropertyHandler; +import org.labkey.api.settings.StartupPropertyEntry; +import org.labkey.api.settings.WriteableAppProps; +import org.labkey.api.util.ExceptionUtil; +import org.labkey.api.util.JsonUtil; +import org.labkey.api.util.logging.LogHelper; +import org.labkey.core.admin.AdminController; +import org.labkey.filters.ContentSecurityPolicyFilter; +import org.springframework.validation.BindException; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; + +import static org.labkey.api.settings.AppProps.ALLOWED_EXTERNAL_RESOURCES; + +public class AllowedExternalResourceHosts +{ + private static final Logger LOG = LogHelper.getLogger(AllowedExternalResourceHosts.class, "Exceptions reading persisted external resource hosts"); + + private AllowedExternalResourceHosts() + { + } + + public record AllowedHost(Directive directive, String host) { } + + public static void saveAllowedHosts(@Nullable Collection allowedHosts, User user) + { + if (null != allowedHosts) + { + final String json; + + try + { + json = JsonUtil.createDefaultMapper().writeValueAsString(allowedHosts); + } + catch (JsonProcessingException e) + { + ExceptionUtil.logExceptionToMothership(null, e); + return; + } + + WriteableAppProps props = AppProps.getWriteableInstance(); + props.setAllowedExternalResourceHosts(json); + props.save(user); + + // Group the allowed hosts by directive + Map> map = allowedHosts.stream() + .collect(Collectors.groupingBy(AllowedHost::directive, Collectors.mapping(AllowedHost::host, Collectors.toCollection(ArrayList::new)))); + + // Unregister all supported directives then register the directives that have at least one allowed host + Arrays.stream(Directive.values()).forEach(dir -> { + ContentSecurityPolicyFilter.unregisterAllowedSources(dir, ALLOWED_EXTERNAL_RESOURCES); + List list = map.get(dir); + if (list != null) + ContentSecurityPolicyFilter.registerAllowedSources(dir, ALLOWED_EXTERNAL_RESOURCES, list.toArray(new String[0])); + }); + } + } + + // Returns a mutable list (mutating it won't affect any cached values) + public static List readAllowedHosts() throws JsonProcessingException + { + String json = AppProps.getInstance().getAllowedExternalResourceHosts(); + return JsonUtil.createDefaultMapper().readValue(json, new TypeReference<>() {}); + } + + public static void registerHosts() + { + final List list; + + try + { + list = AllowedExternalResourceHosts.readAllowedHosts(); + } + catch (JsonProcessingException e) + { + ExceptionUtil.logExceptionToMothership(null, e); + return; + } + + list.forEach(sub -> ContentSecurityPolicyFilter.registerAllowedSources(sub.directive(), sub.host())); + LOG.debug("Registered [{}] as allowed external sources", list); + } + + public static void registerStartupProperties() + { + ModuleLoader.getInstance().handleStartupProperties(new StandardStartupPropertyHandler<>("AllowedExternalResourceHosts", Directive.class) + { + @Override + public void handle(Map properties) + { + // If any allowed-hosts startup properties are provided, they completely replace *all* values that were + // previously configured + if (!properties.isEmpty()) + { + BindException errors = new BindException(new Object(), "form"); + List allowedHosts = properties.entrySet().stream() + .flatMap(e -> Arrays.stream(e.getValue().getValue().trim().split("\\s+")) + .map(host -> AdminController.ExternalSourcesForm.validateHost(e.getKey().name(), host, errors)) + ) + .toList(); + + if (!errors.hasErrors()) + { + AdminController.ExternalSourcesForm.checkDuplicates(allowedHosts, errors); + } + + if (errors.hasErrors()) + { + LOG.error("Invalid AllowedExternalResourceHosts startup properties", errors); + } + else + { + saveAllowedHosts(allowedHosts, User.getAdminServiceUser()); + } + } + } + }); + } +}