forked from geoserver/geoserver
-
Notifications
You must be signed in to change notification settings - Fork 6
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
[GEOS-11289] Enable Spring Security StrictHttpFirewall by default
- Loading branch information
Showing
6 changed files
with
236 additions
and
4 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
92 changes: 92 additions & 0 deletions
92
src/main/src/main/java/org/geoserver/security/GeoServerHttpFirewall.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,92 @@ | ||
| /* (c) 2024 Open Source Geospatial Foundation - all rights reserved | ||
| * This code is licensed under the GPL 2.0 license, available at the root | ||
| * application directory. | ||
| */ | ||
| package org.geoserver.security; | ||
|
|
||
| import java.util.regex.Pattern; | ||
| import javax.servlet.http.HttpServletRequest; | ||
| import javax.servlet.http.HttpServletRequestWrapper; | ||
| import javax.servlet.http.HttpServletResponse; | ||
| import org.geoserver.platform.GeoServerExtensions; | ||
| import org.springframework.security.web.firewall.DefaultHttpFirewall; | ||
| import org.springframework.security.web.firewall.FirewalledRequest; | ||
| import org.springframework.security.web.firewall.HttpFirewall; | ||
| import org.springframework.security.web.firewall.StrictHttpFirewall; | ||
|
|
||
| /** | ||
| * A custom {@link HttpFirewall} implementation that allows GeoServer administrators to enable or | ||
| * disable the use of the stricter {@link StrictHttpFirewall} through a system property. The {@link | ||
| * DefaultHttpFirewall} provides weaker protections but may be necessary for some users who need to | ||
| * use certain special characters in URL paths. | ||
| */ | ||
| public class GeoServerHttpFirewall implements HttpFirewall { | ||
|
|
||
| /** | ||
| * System property to control whether or not to run requests through {@link StrictHttpFirewall}. | ||
| * When set to false, requests will only be run through {@link DefaultHttpFirewall} which is | ||
| * more lenient but also more likely to allow malicious requests. Default is true. | ||
| */ | ||
| public static final String USE_STRICT_FIREWALL = "GEOSERVER_USE_STRICT_FIREWALL"; | ||
|
|
||
| private final DefaultHttpFirewall defaultFirewall = new DefaultHttpFirewall(); | ||
|
|
||
| private final StrictHttpFirewall strictFirewall = new StrictHttpFirewall(); | ||
|
|
||
| @Override | ||
| public FirewalledRequest getFirewalledRequest(HttpServletRequest request) { | ||
| // run a modified request with normalized URL paths through Spring Security's | ||
| // StrictHttpFirewall but do not forward the normalized request to the proxy | ||
| if (!"false".equalsIgnoreCase(GeoServerExtensions.getProperty(USE_STRICT_FIREWALL))) { | ||
| this.strictFirewall.getFirewalledRequest(new NormalizedHttpServletRequest(request)); | ||
| } | ||
| // use DefaultHttpFirewall here with the original request | ||
| return this.defaultFirewall.getFirewalledRequest(request); | ||
| } | ||
|
|
||
| @Override | ||
| public HttpServletResponse getFirewalledResponse(HttpServletResponse response) { | ||
| // strict and default firewalls have identical behavior here | ||
| return this.defaultFirewall.getFirewalledResponse(response); | ||
| } | ||
|
|
||
| /** | ||
| * An {@link HttpServletRequestWrapper} that allows running a {@link HttpServletRequest} through | ||
| * {@link StrictHttpFirewall} even when the URL path contains two consecutive slashes since | ||
| * there are use cases where GeoServer needs to allow this type of non-normalized URL path. | ||
| */ | ||
| private static class NormalizedHttpServletRequest extends HttpServletRequestWrapper { | ||
|
|
||
| /** Regular expression for two or more consecutive forward slashes. */ | ||
| private static final Pattern FORWARD_SLASHES = Pattern.compile("//+"); | ||
|
|
||
| private NormalizedHttpServletRequest(HttpServletRequest request) { | ||
| super(request); | ||
| } | ||
|
|
||
| /** Replaces consecutive forward slashes with a single slash. */ | ||
| private static String normalizeSlashes(String path) { | ||
| return path != null ? FORWARD_SLASHES.matcher(path).replaceAll("/") : null; | ||
| } | ||
|
|
||
| @Override | ||
| public String getContextPath() { | ||
| return normalizeSlashes(super.getContextPath()); | ||
| } | ||
|
|
||
| @Override | ||
| public String getPathInfo() { | ||
| return normalizeSlashes(super.getPathInfo()); | ||
| } | ||
|
|
||
| @Override | ||
| public String getRequestURI() { | ||
| return normalizeSlashes(super.getRequestURI()); | ||
| } | ||
|
|
||
| @Override | ||
| public String getServletPath() { | ||
| return normalizeSlashes(super.getServletPath()); | ||
| } | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
119 changes: 119 additions & 0 deletions
119
src/main/src/test/java/org/geoserver/security/GeoServerHttpFirewallTest.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,119 @@ | ||
| /* (c) 2024 Open Source Geospatial Foundation - all rights reserved | ||
| * This code is licensed under the GPL 2.0 license, available at the root | ||
| * application directory. | ||
| */ | ||
| package org.geoserver.security; | ||
|
|
||
| import static org.junit.Assert.assertNotNull; | ||
| import static org.junit.Assert.assertThrows; | ||
|
|
||
| import org.junit.After; | ||
| import org.junit.Test; | ||
| import org.springframework.mock.web.MockHttpServletRequest; | ||
| import org.springframework.security.web.firewall.RequestRejectedException; | ||
|
|
||
| public class GeoServerHttpFirewallTest { | ||
|
|
||
| private final GeoServerHttpFirewall firewall = new GeoServerHttpFirewall(); | ||
|
|
||
| @After | ||
| public void resetProperty() { | ||
| setProperty(null); | ||
| } | ||
|
|
||
| @Test | ||
| public void testForwardSlashEncoded() { | ||
| // decoded forward slashes are not blocked | ||
| // this is blocked by both strict and default firewalls | ||
| // some servers may block this even if this firewall wasn't blocking it | ||
| doTestPath(null, "%2f", true); | ||
| doTestPath(true, "%2f", true); | ||
| doTestPath(false, "%2f", true); | ||
| doTestPath(null, "%2F", true); | ||
| doTestPath(true, "%2F", true); | ||
| doTestPath(false, "%2F", true); | ||
| } | ||
|
|
||
| @Test | ||
| public void testBackSlashDecoded() { | ||
| // this is actually an invalid URL that servers will probably block | ||
| doTestPath(null, "\\", true); | ||
| doTestPath(true, "\\", true); | ||
| doTestPath(false, "\\", false); | ||
| } | ||
|
|
||
| @Test | ||
| public void testBackSlashEncoded() { | ||
| // some servers may block this even when the strict firewall is disabled | ||
| doTestPath(null, "%5c", true); | ||
| doTestPath(true, "%5c", true); | ||
| doTestPath(false, "%5c", false); | ||
| doTestPath(null, "%5C", true); | ||
| doTestPath(true, "%5C", true); | ||
| doTestPath(false, "%5C", false); | ||
| } | ||
|
|
||
| @Test | ||
| public void testPercentEncoded() { | ||
| // decoded percents create invalid URLs that don't work | ||
| doTestPath(null, "%25", true); | ||
| doTestPath(true, "%25", true); | ||
| doTestPath(false, "%25", false); | ||
| } | ||
|
|
||
| @Test | ||
| public void testPeriodEncoded() { | ||
| // decoded periods are not blocked | ||
| doTestPath(null, "%2e", true); | ||
| doTestPath(true, "%2e", true); | ||
| doTestPath(false, "%2e", false); | ||
| doTestPath(null, "%2E", true); | ||
| doTestPath(true, "%2E", true); | ||
| doTestPath(false, "%2E", false); | ||
| } | ||
|
|
||
| @Test | ||
| public void testSemicolonDecoded() { | ||
| doTestPath(null, ";", true); | ||
| doTestPath(true, ";", true); | ||
| doTestPath(false, ";", false); | ||
| } | ||
|
|
||
| @Test | ||
| public void testSemicolonEncoded() { | ||
| doTestPath(null, "%3b", true); | ||
| doTestPath(true, "%3b", true); | ||
| doTestPath(false, "%3b", false); | ||
| doTestPath(null, "%3B", true); | ||
| doTestPath(true, "%3B", true); | ||
| doTestPath(false, "%3B", false); | ||
| } | ||
|
|
||
| @Test | ||
| public void testNonNormalizedPath() { | ||
| // GeoServer needs this to be allowed | ||
| doTestPath(null, "//", false); | ||
| doTestPath(true, "//", false); | ||
| doTestPath(false, "//", false); | ||
| } | ||
|
|
||
| private void doTestPath(Boolean strict, String path, boolean exceptionExpected) { | ||
| setProperty(strict); | ||
| MockHttpServletRequest request = new MockHttpServletRequest("GET", path); | ||
| if (exceptionExpected) { | ||
| assertThrows( | ||
| RequestRejectedException.class, | ||
| () -> this.firewall.getFirewalledRequest(request)); | ||
| } else { | ||
| assertNotNull(this.firewall.getFirewalledRequest(request)); | ||
| } | ||
| } | ||
|
|
||
| private static void setProperty(Boolean strict) { | ||
| if (strict == null) { | ||
| System.clearProperty(GeoServerHttpFirewall.USE_STRICT_FIREWALL); | ||
| } else { | ||
| System.setProperty(GeoServerHttpFirewall.USE_STRICT_FIREWALL, strict.toString()); | ||
| } | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters