[GEOS-10438] ENTITY_RESOLUTION_ALLOWLIST property not parsing empty s…

…etting

doc/en/user/source/installation/upgrade.rst

@@ -30,6 +30,25 @@ The general GeoServer upgrade process is as follows:
 Notes on upgrading specific versions
 ------------------------------------
 
+External Entity Allow List default (GeoServer 2.25 and newer)
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+The external entity allow list has changed to the following default locations:
+
+* ``www.w3.org``
+* ``schemas.opengis.net``
+* ``www.opengis.net``
+* ``inspire.ec.europa.eu/schemas``
+* proxy base url if configured
+
+The external entity allow list is an important setting from a security standpoint. This update changes its use from a recommended best practice to a default covering the most common locations used for OGC web services.
+
+.. note:: In general only application schema extension users need to update this setting.
+
+.. note:: To restore the previous behavour use system property ``ENTITY_RESOLUTION_ALLOWLIST=*`` to allow external entity resolution from any `http` or `https` location.
+
+For more information, including how to add additional allowed locations see :ref:`production_config_external_entities`.
+
 FreeMarker Template HTML Auto-escaping (GeoServer 2.25 and newer)
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 

doc/en/user/source/production/config.rst

@@ -219,23 +219,28 @@ When processing XML documents from service requests (POST requests, and GET requ
 
 GeoServer provides a number of facilities to control external entity resolution:
 
-* By default `http` and `https` entity resolution is unrestricted, with access to local `file` references prevented.
-
-* To restrict `http` and `https` entity resolution::
-
-     -DENTITY_RESOLUTION_ALLOWLIST
-  
-  The built-in allow list includes w3c, ogc, and inspire schema locations::
+* By default `http` and `https` entity resolution is restricted to the following default::
   
      www.w3.org|schemas.opengis.net|www.opengis.net|inspire.ec.europa.eu/schemas
      
-  In addition the proxy base url is included, if available from global settings.
-  
-  Access to local `file` references remains restricted. 
+  The default list includes the common w3c, ogc, and inspire schema locations required for OGC Web Service operation.
   
+  Access is provided to the proxy base url from global settings.
+  Access to local `file` references is restricted.
+
 * To allow additional external entity `http` and `https` locations use a comma or bar separated list::
 
      -DENTITY_RESOLUTION_ALLOWLIST=server1|server2|server3/schemas
+  
+  These locations are in addition to the default w3c, ogc, and inspire schema locations above.
+  Access is provided to the proxy base url from global settings.
+  Access to local `file` references remains restricted.
+
+* To allow all `http` and `https` entity resolution ise `*` wildcard::
+
+     -DENTITY_RESOLUTION_ALLOWLIST=*
+  
+  Access to local `file` references remains restricted.
 
 * To turn off all restrictions (allowing ``http``, ``https``, and ``file`` references) use the global setting :ref:`config_globalsettings_external_entities`.
 

src/main/src/main/java/org/geoserver/util/AllowListEntityResolver.java

@@ -24,16 +24,19 @@
  */
 public class AllowListEntityResolver implements EntityResolver2, Serializable {
 
+    /** Wildcard '*' location indicating unrestricted http(s) access */
+    public static String UNRESTRICTED = "*";
+
     /** Location of Open Geospatical Consortium schemas for OGC OpenGIS standards */
-    private static String OGC = "schemas.opengis.net|www.opengis.net";
+    public static String OGC = "schemas.opengis.net|www.opengis.net";
 
     /**
      * Location of {@code http://inspire.ec.europa.eu/schemas/ } XSD documents for INSPIRE program
      */
-    private static String INSPIRE = "inspire.ec.europa.eu/schemas";
+    public static String INSPIRE = "inspire.ec.europa.eu/schemas";
 
     /** Location of W3C schema documents (for xlink, etc...) */
-    private static String W3C = "www.w3.org";
+    public static String W3C = "www.w3.org";
 
     /** Prefix used for SAXException message */
     private static final String ERROR_MESSAGE_BASE = "Entity resolution disallowed for ";
@@ -72,8 +75,9 @@ public AllowListEntityResolver(GeoServer geoServer) {
     public AllowListEntityResolver(GeoServer geoServer, String baseURL) {
         this.geoServer = geoServer;
         this.baseURL = baseURL;
+
         if (EntityResolverProvider.ALLOW_LIST == null
-                || EntityResolverProvider.ALLOW_LIST.length == 0) {
+                || EntityResolverProvider.ALLOW_LIST.isEmpty()) {
             // Restrict using the built-in allow list
             ALLOWED_URIS =
                     Pattern.compile(
@@ -86,11 +90,14 @@ public AllowListEntityResolver(GeoServer geoServer, String baseURL) {
                                     + ")/[^?#;]*\\.xsd");
         } else {
             StringBuilder pattern = new StringBuilder("(?i)(http|https)://(");
-            pattern.append(W3C).append('|');
-            pattern.append(OGC).append('|');
-            pattern.append(INSPIRE);
+            boolean first = true;
             for (String allow : EntityResolverProvider.ALLOW_LIST) {
-                pattern.append('|').append(allow);
+                if (first) {
+                    first = false;
+                } else {
+                    pattern.append('|');
+                }
+                pattern.append(allow);
             }
             pattern.append(")/[^?#;]*\\.xsd");
             String regex = pattern.toString();

src/main/src/main/java/org/geoserver/util/EntityResolverProvider.java

@@ -5,6 +5,9 @@
  */
 package org.geoserver.util;
 
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.Set;
 import org.geoserver.config.GeoServer;
 import org.geoserver.platform.GeoServerExtensions;
 import org.geotools.util.PreventLocalEntityResolver;
@@ -20,7 +23,7 @@ public class EntityResolverProvider {
     private static final String ENTITY_RESOLUTION_ALLOWLIST = "ENTITY_RESOLUTION_ALLOWLIST";
 
     /** Allow list configuration from ENTITY_RESOLUTION_ALLOWLIST property. */
-    static String[] ALLOW_LIST = entityResolutionAllowlist();
+    static Set<String> ALLOW_LIST = entityResolutionAllowlist();
 
     /**
      * EntityResolve provided for use, acts as an override of settings and system properties.
@@ -79,30 +82,28 @@ public EntityResolver getEntityResolver() {
                 // XML parser is unrestricted, and can access any XSD location
                 return null;
             }
-
-            if (ALLOW_LIST != null) {
-                // External entity resolution limited to those approved for use
-                // those built-in to GeoSever, while restricting file access
-                return ALLOWLIST_ENTITY_RESOLVER;
-            }
         }
         if (entityResolver != null) {
             // override provided (usually by a test case)
             return entityResolver;
-        } else {
-            // Allows access to any http(s) location, and those built-in to GeoServer jars, while
-            // restricting file access.
-            return PreventLocalEntityResolver.INSTANCE;
         }
+        if (ALLOW_LIST != null) {
+            // External entity resolution limited to those approved for use
+            // those built-in to GeoSever, while restricting file access
+            return ALLOWLIST_ENTITY_RESOLVER;
+        }
+        // Allows access to any http(s) location, and those built-in to GeoServer jars, while
+        // restricting file access.
+        return PreventLocalEntityResolver.INSTANCE;
     }
 
     /**
      * Locations allowed for external entity expansion from application property
      * "ENTITY_RESOLUTION_ALLOWLIST".
      *
      * <ul>
-     *   <li><code>"*" or undefined</code>: Allow all http(s) schema locations
-     *   <li><code>"": Restrict to schemas provided by w3c, ogc and inspire</code>
+     *   <li><code>"*"</code>: Allow all http(s) schema locations
+     *   <li><code>"" or undefined: Restrict to schemas provided by w3c, ogc and inspire</code>
      *   <li><code>
      *       "location1,location2": Restrict to the provided locations, and those list by w4c, ogc and inspire
      *       </code>
@@ -113,19 +114,42 @@ public EntityResolver getEntityResolver() {
      * proxy base url if known. This setting is used by {@link EntityResolverProvider} to limit
      * external entity resolution.
      *
-     * @return Restrict external http(s) entity expansion to these external locations, or null to
-     *     disable restriction.
+     * @return Restrict external http(s) entity expansion to these external locations, with "*"
+     *     wildcard indicating unrestricted.
      */
-    public static String[] entityResolutionAllowlist() {
+    public static Set<String> entityResolutionAllowlist() {
         String allowed = GeoServerExtensions.getProperty(ENTITY_RESOLUTION_ALLOWLIST);
-        if (allowed == null || "*".equals(allowed.trim())) {
-            // allow all external http(s) access
+        return entityResolutionAllowlist(allowed);
+    }
+
+    /**
+     * Provides parsing of ENTITY_RESOLUTION_ALLOWLIST property for {@link
+     * #entityResolutionAllowlist()}.
+     *
+     * @param allowed Allowed list of expansion locations seperated by | character.
+     * @return set of allowed http(s) entity expansion external locations.
+     */
+    static Set<String> entityResolutionAllowlist(String allowed) {
+        final String[] DEFAULT_LIST =
+                String.join(
+                                "|",
+                                AllowListEntityResolver.W3C,
+                                AllowListEntityResolver.OGC,
+                                AllowListEntityResolver.INSPIRE)
+                        .split("\\|");
+
+        if (allowed == null || allowed.trim().isEmpty()) {
+            return new HashSet<>(Arrays.asList(DEFAULT_LIST));
+        } else if (allowed.contains(AllowListEntityResolver.UNRESTRICTED)) {
             return null;
-        } else if (!"".equals(allowed.trim())) {
-            // allow external http(s) access to ogc, w3c, and
-            return new String[0];
         } else {
-            return allowed.split("\\s*,\\s*|\\s+");
+            Set<String> allowedList = new HashSet<>(Arrays.asList(DEFAULT_LIST));
+            for (String domain : allowed.split("\\s*,\\s*|\\s+")) {
+                if (!domain.isEmpty()) {
+                    allowedList.add(domain);
+                }
+            }
+            return allowedList;
         }
     }
 }

src/main/src/test/java/org/geoserver/util/EntityResolverProviderTest.java

@@ -0,0 +1,52 @@
+/* (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.util;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
+
+import java.util.Arrays;
+import java.util.Set;
+import org.junit.Test;
+
+public class EntityResolverProviderTest {
+
+    @Test
+    public void testAllowListDefaults() throws Exception {
+        Set<String> allowed = EntityResolverProvider.entityResolutionAllowlist("");
+        assertNotNull(allowed);
+        assertEquals(4, allowed.size());
+        assertTrue(allowed.contains(AllowListEntityResolver.W3C));
+        assertTrue(allowed.contains(AllowListEntityResolver.INSPIRE));
+        assertTrue(allowed.containsAll(Arrays.asList(AllowListEntityResolver.OGC.split("\\|"))));
+
+        allowed = EntityResolverProvider.entityResolutionAllowlist(null);
+        assertNotNull(allowed);
+        assertEquals(4, allowed.size());
+        assertTrue(allowed.contains(AllowListEntityResolver.W3C));
+        assertTrue(allowed.contains(AllowListEntityResolver.INSPIRE));
+        assertTrue(allowed.containsAll(Arrays.asList(AllowListEntityResolver.OGC.split("\\|"))));
+    }
+
+    @Test
+    public void testAllowListWildCard() throws Exception {
+        Set<String> allowed = EntityResolverProvider.entityResolutionAllowlist("*");
+        assertNull(allowed);
+    }
+
+    @Test
+    public void testAllowListDomains() throws Exception {
+        Set<String> allowed = EntityResolverProvider.entityResolutionAllowlist("how2map.com");
+
+        assertNotNull(allowed);
+        assertEquals(5, allowed.size());
+        assertTrue(allowed.contains(AllowListEntityResolver.W3C));
+        assertTrue(allowed.contains(AllowListEntityResolver.INSPIRE));
+        assertTrue(allowed.containsAll(Arrays.asList(AllowListEntityResolver.OGC.split("\\|"))));
+        assertTrue(allowed.contains("how2map.com"));
+    }
+}