This is an automated email from the ASF dual-hosted git repository. joerghoh pushed a commit to branch SLING-13312 in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-security.git
commit 5b9cf10919b5566a3fa53f6c92482307f42151c7 Author: Joerg Hoh <[email protected]> AuthorDate: Tue Aug 18 19:46:23 2026 +0200 SLING-13312 harden request-origin validation --- .../apache/sling/security/impl/ReferrerFilter.java | 85 ++++++++++++-- .../sling/security/impl/ReferrerFilterTest.java | 125 ++++++++++++++++++++- 2 files changed, 199 insertions(+), 11 deletions(-) diff --git a/src/main/java/org/apache/sling/security/impl/ReferrerFilter.java b/src/main/java/org/apache/sling/security/impl/ReferrerFilter.java index a65dde5..bc42c74 100644 --- a/src/main/java/org/apache/sling/security/impl/ReferrerFilter.java +++ b/src/main/java/org/apache/sling/security/impl/ReferrerFilter.java @@ -108,16 +108,33 @@ public class ReferrerFilter implements Preprocessor { @AttributeDefinition(name = "Allow Empty", description = "Allow an empty or missing referrer") boolean allow_empty() default false; + /** + * Allow loopback and server NIC addresses (legacy behavior). + */ + @AttributeDefinition( + name = "Allow Server Addresses (legacy)", + description = + "If enabled, any request whose Referer/Origin claims to originate from this server itself " + + "— \"localhost\", \"127.0.0.1\", \"[::1]\", or any IP bound to this server's own network " + + "interfaces — is trusted on ANY port, for both \"http\" and \"https\". This also trusts every " + + "other application running on this machine (or sharing its IP) on a different port, so leave " + + "disabled unless you specifically need that trust. Well-behaved applications on \"localhost\" or " + + "a locally assigned IP send a Referer/Origin matching their own origin (scheme, host AND port), " + + "which is always allowed regardless of this setting — so they keep working correctly whether " + + "this option is enabled or not. It should NOT be activated unless you have a specific, understood " + + "need for it: it exists only for backwards compatibility with legacy setups.") + boolean allow_server_addresses() default false; + /** * Allow referrer uri hosts property. */ @AttributeDefinition( name = "Allow Hosts", description = - "List of allowed hosts for the referrer which are added to the list of default hosts. " + "List of allowed hosts for the referrer. " + "It is matched against the full referrer URL in the format \"<scheme>://<host>:<port>\". " - + "If port is 0, it is not taken into consideration. The default list contains all host names " - + "and IPs bound to all NICs found in the system plus \"localhost\", \"127.0.0.1\", \"[::1]\" for protocols \"http\" and \"https\". " + + "If port is 0, it is not taken into consideration. If \"Allow Server Addresses\" is enabled, all host names " + + "and IPs bound to all NICs found in the system plus \"localhost\", \"127.0.0.1\", \"[::1]\" are added to this list for protocols \"http\" and \"https\". " + "If given value does not have a \":\" entries for both http and https are transparently generated.") String[] allow_hosts() default {}; @@ -173,7 +190,11 @@ public class ReferrerFilter implements Preprocessor { private final String[] excludedPaths; /** - * Create a default list of referrers + * Create the legacy list of default referrers (loopback plus every NIC-bound + * address, any port, http and https). Only used when the operator explicitly + * opts in via {@link Config#allow_server_addresses()}: these host names describe + * the origin of the page in the requesting user's browser, not this server, so + * they must not be trusted by default. */ private Set<String> getDefaultAllowedReferrers() { final Set<String> referrers = new HashSet<>(); @@ -287,7 +308,8 @@ public class ReferrerFilter implements Preprocessor { this.excludedPaths = mergeValues(config.exclude_paths(), amendments, a -> a.excludePaths()) .toArray(new String[0]); - final Set<String> allowUriReferrers = getDefaultAllowedReferrers(); + final Set<String> allowUriReferrers = + config.allow_server_addresses() ? getDefaultAllowedReferrers() : new HashSet<>(); if (config.allow_hosts() != null) { allowUriReferrers.addAll(mergeValues(config.allow_hosts(), amendments, a -> a.allowHosts())); } @@ -388,9 +410,11 @@ public class ReferrerFilter implements Preprocessor { } String referrer = request.getHeader("referer"); + boolean fromOriginHeader = false; // use the origin if the referrer is not set if (referrer == null || referrer.trim().length() == 0) { referrer = request.getHeader("origin"); + fromOriginHeader = true; } // check for missing/empty referrer @@ -403,9 +427,35 @@ public class ReferrerFilter implements Preprocessor { } return this.allowEmpty; } + // The literal value "null" is the serialization of an opaque origin + // (RFC 6454): browsers send "Origin: null" for cross-site requests + // issued from sandboxed iframes, data: documents or pages with a + // no-referrer referrer policy. It explicitly marks an untrustworthy + // origin and must never be treated as a relative referrer, so it is + // handled exactly like a missing referrer (subject to allow.empty). + if ("null".equalsIgnoreCase(referrer.trim())) { + if (!this.allowEmpty) { + this.logger.info( + "Rejected 'null' (opaque) origin/referrer for {} request to {}", + request.getMethod(), + request.getRequestURI()); + } + return this.allowEmpty; + } // check for relative referrer - which is always allowed if (!referrer.contains(":/")) { - return true; + // only the Referer header may legitimately carry a relative + // value; the Origin header is always an absolute origin (or + // "null", handled above), so a non-absolute Origin is illegal + if (!fromOriginHeader) { + return true; + } + this.logger.info( + "Rejected non-absolute origin header for {} request to {} : {}", + request.getMethod(), + request.getRequestURI(), + referrer); + return false; } final HostInfo info = getHost(referrer); @@ -419,9 +469,11 @@ public class ReferrerFilter implements Preprocessor { return false; } - // allow the request if the host name of the referrer is - // the same as the request's host name - if (info.host.equals(request.getServerName())) { + // allow the request if the referrer designates the request's own + // origin: scheme, host AND port must match. + if (info.host.equals(request.getServerName()) + && info.scheme.equals(request.getScheme()) + && info.port == normalizePort(request.getScheme(), request.getServerPort())) { return true; } @@ -454,6 +506,21 @@ public class ReferrerFilter implements Preprocessor { // nothing to do } + /** + * Normalize a port number by replacing an unknown port with the default + * port of the given scheme. + * + * @param scheme The scheme the port is used with + * @param port The port number + * @return the given port, or the scheme's default port if the given port is not positive + */ + private static int normalizePort(final String scheme, final int port) { + if (port <= 0) { + return "https".equals(scheme) ? 443 : 80; + } + return port; + } + /** * @param hostInfo The hostInfo to check for validity * @return <code>true</code> if the hostInfo matches any of the allowed URI referrer. diff --git a/src/test/java/org/apache/sling/security/impl/ReferrerFilterTest.java b/src/test/java/org/apache/sling/security/impl/ReferrerFilterTest.java index b4e46f0..6f66a85 100644 --- a/src/test/java/org/apache/sling/security/impl/ReferrerFilterTest.java +++ b/src/test/java/org/apache/sling/security/impl/ReferrerFilterTest.java @@ -54,6 +54,16 @@ public class ReferrerFilterTest { String[] allowHostsRexexp, String[] excludeAgentsRegexp, String[] excludePaths) { + return createConfiguration(allowEmpty, false, allowHosts, allowHostsRexexp, excludeAgentsRegexp, excludePaths); + } + + private static ReferrerFilter.Config createConfiguration( + boolean allowEmpty, + boolean allowServerAddresses, + String[] allowHosts, + String[] allowHostsRexexp, + String[] excludeAgentsRegexp, + String[] excludePaths) { return new ReferrerFilter.Config() { @Override public Class<? extends Annotation> annotationType() { @@ -65,6 +75,11 @@ public class ReferrerFilterTest { return allowEmpty; } + @Override + public boolean allow_server_addresses() { + return allowServerAddresses; + } + @Override public String[] allow_hosts() { return allowHosts; @@ -142,8 +157,10 @@ public class ReferrerFilterTest { assertTrue(filter.isValidRequest(getRequest("/relative/too"))); assertTrue(filter.isValidRequest(getRequest("/relative/but/[illegal]"))); assertFalse(filter.isValidRequest(getRequest("http://somehost";))); - assertTrue(filter.isValidRequest(getRequest("http://localhost";))); - assertTrue(filter.isValidRequest(getRequest("http://127.0.0.1";))); + // loopback referrers identify the user's machine, not this server: + // they are no longer trusted by default + assertFalse(filter.isValidRequest(getRequest("http://localhost";))); + assertFalse(filter.isValidRequest(getRequest("http://127.0.0.1";))); assertFalse(filter.isValidRequest(getRequest("http://somehost/but/[illegal]";))); assertTrue(filter.isValidRequest(getRequest("http://relhost";))); assertTrue(filter.isValidRequest(getRequest("http://relhost:9001";))); @@ -221,6 +238,37 @@ public class ReferrerFilterTest { assertTrue(rf.isValidRequest(getRequest("http://test2.com:80";, null, "/test_path"))); } + private static HttpServletRequest getSameHostRequest(final String referrer, final String scheme, final int port) { + final HttpServletRequest request = getRequest(referrer); + when(request.getServerName()).thenReturn("myhost"); + when(request.getScheme()).thenReturn(scheme); + when(request.getServerPort()).thenReturn(port); + return request; + } + + @Test + public void testSameOriginReferrerAllowed() { + assertTrue(filter.isValidRequest(getSameHostRequest("https://myhost/page";, "https", 443))); + assertTrue(filter.isValidRequest(getSameHostRequest("https://myhost:443/page";, "https", 443))); + assertTrue(filter.isValidRequest(getSameHostRequest("http://myhost/page";, "http", 80))); + assertTrue(filter.isValidRequest(getSameHostRequest("http://myhost:8080/page";, "http", 8080))); + } + + @Test + public void testSameHostDifferentSchemeRejected() { + // an http:// page (active-network attacker) must not vouch for the + // https deployment of the same host + assertFalse(filter.isValidRequest(getSameHostRequest("http://myhost/page";, "https", 443))); + } + + @Test + public void testSameHostDifferentPortRejected() { + // another service on a different port of the same host is a + // different origin + assertFalse(filter.isValidRequest(getSameHostRequest("https://myhost:8443/page";, "https", 443))); + assertFalse(filter.isValidRequest(getSameHostRequest("http://myhost:8081/page";, "http", 80))); + } + @Test public void testAllowsWithOrigin() { HttpServletRequest request = getRequest(null); @@ -228,6 +276,44 @@ public class ReferrerFilterTest { Assert.assertEquals(true, filter.isValidRequest(request)); } + @Test + public void testRejectsNullOrigin() { + // browsers serialize an opaque origin (sandboxed iframe, data: document, + // no-referrer policy) as the literal string "null" - it must not be + // treated as an allowed relative referrer + HttpServletRequest request = getRequest(null); + when(request.getHeader("origin")).thenReturn("null"); + assertFalse(filter.isValidRequest(request)); + } + + @Test + public void testRejectsNullReferrer() { + // the literal string "null" in the Referer header must not be treated + // as an allowed relative referrer either + assertFalse(filter.isValidRequest(getRequest("null"))); + assertFalse(filter.isValidRequest(getRequest("NULL"))); + } + + @Test + public void testNullOriginFollowsAllowEmpty() { + // "null" carries the same information as a missing referrer and thus + // follows the allow.empty configuration + ReferrerFilter rf = + new ReferrerFilter(createConfiguration(true, null, null, null, null), Collections.emptyList()); + HttpServletRequest request = getRequest(null); + when(request.getHeader("origin")).thenReturn("null"); + assertTrue(rf.isValidRequest(request)); + } + + @Test + public void testRejectsNonAbsoluteOrigin() { + // the Origin header is always an absolute origin or "null"; a + // non-absolute value must not take the relative-referrer shortcut + HttpServletRequest request = getRequest(null); + when(request.getHeader("origin")).thenReturn("relative"); + assertFalse(filter.isValidRequest(request)); + } + @Test public void testAllowEmpty() { ReferrerFilter rf = @@ -237,6 +323,41 @@ public class ReferrerFilterTest { assertTrue(rf.isValidRequest(getRequest("", null, null))); } + /** + * Regression: the Referer/Origin header describes the page in the *user's* browser, so a + * page served by a local application on the victim's machine (e.g. a dev server on + * 127.0.0.1:3000) must not be trusted by default to send state-changing requests (CSRF). + */ + @Test + public void testServerAddressesNotTrustedByDefault() { + final ReferrerFilter rf = + new ReferrerFilter(createConfiguration(false, null, null, null, null), Collections.emptyList()); + + assertFalse(rf.isValidRequest(getRequest("http://localhost";))); + assertFalse(rf.isValidRequest(getRequest("http://localhost:3000";))); + assertFalse(rf.isValidRequest(getRequest("http://127.0.0.1:3000";))); + assertFalse(rf.isValidRequest(getRequest("https://127.0.0.1";))); + + // same-host referrers are still allowed if the request originates from there + final HttpServletRequest request = getRequest("http://localhost:3000";); + when(request.getServerName()).thenReturn("localhost"); + when(request.getScheme()).thenReturn("http"); + when(request.getServerPort()).thenReturn(3000); + assertTrue(rf.isValidRequest(request)); + } + + @Test + public void testServerAddressesOptIn() { + final ReferrerFilter rf = + new ReferrerFilter(createConfiguration(false, true, null, null, null, null), Collections.emptyList()); + + assertTrue(rf.isValidRequest(getRequest("http://localhost";))); + assertTrue(rf.isValidRequest(getRequest("http://localhost:3000";))); + assertTrue(rf.isValidRequest(getRequest("http://127.0.0.1:3000";))); + assertTrue(rf.isValidRequest(getRequest("https://127.0.0.1";))); + assertFalse(rf.isValidRequest(getRequest("http://somehost";))); + } + @Test public void testIsBrowserRequest() { String userAgent =
