[GEOS-11401] Introduce environmental variables for Module Status page

turn off system-environment and system-property based on environement variable (default = don't show add doc use optional.empty() changes from miceg's review changes from miceg's review jody and andrea feedback doc changes - miceg
geoserver · May 16, 2024 · 5fd5f35 · 5fd5f35
1 parent 7d26b8b
commit 5fd5f35
Show file tree

Hide file tree

Showing 6 changed files with 301 additions and 2 deletions.
diff --git a/doc/en/user/source/configuration/properties/index.rst b/doc/en/user/source/configuration/properties/index.rst
@@ -31,6 +31,18 @@ GeoServer Property Reference
      - x
      - x
      - x
+   * - GEOSERVER_MODULE_SYSTEM_ENVIRONMENT_STATUS_ENABLED
+
+       :ref:`module_status_security_environment_vars`
+     -  
+     -  
+     - x
+   * - GEOSERVER_MODULE_SYSTEM_PROPERTY_STATUS_ENABLED
+
+       :ref:`module_status_security_environment_vars`     
+     -  
+     -  
+     - x
    * - GEOWEBCACHE_CACHE_DIR
 
        :doc:`/geowebcache/config`

diff --git a/doc/en/user/source/production/config.rst b/doc/en/user/source/production/config.rst
@@ -139,6 +139,37 @@ In some circumstances, you might want to completely disable the web administrati
 * Set the Java system property ``GEOSERVER_CONSOLE_DISABLED`` to true by adding ``-DGEOSERVER_CONSOLE_DISABLED=true`` to your container's JVM options
 * Remove all of the :file:`gs-web*-.jar` files from :file:`WEB-INF/lib`
 
+.. _module_status_security_environment_vars:
+
+Showing Environment Variables and Java System Properties
+''''''''''''''''''''''''''''''''''''''''''''''''''''''''
+
+Module status information is available describing the operational environment.
+
+* The :guilabel:`GeoServer Status` page :ref:`config_serverstatus_module`.
+* The REST ``/geoserver/rest/about/status`` endpoint lists module status information
+
+1. By default GeoServer does **not** show Environment Variables and Java System Properties.
+
+2. To show environment variables and Java system properties on the status page and REST API, start GeoServer with these environment variables set to ``true``:
+
+   * `GEOSERVER_MODULE_SYSTEM_ENVIRONMENT_STATUS_ENABLED`
+   * `GEOSERVER_MODULE_SYSTEM_PROPERTY_STATUS_ENABLED`
+
+3. In a production system, these should be set to ``false`` (or leave them undefined).
+
+   .. warning::
+
+      While this feature can help an administrator debug a GeoServer instance's configuration, environment variables can include sensitive information such as database passwords and API access keys/tokens, particularly when running in a containerised environment (such as Docker or Kubernetes) or with ``ALLOW_ENV_PARAMETRIZATION=true``.
+
+   .. note:: Linux
+
+      Linux administrators can get a list of all environment variables set at GeoServer startup with:
+
+      .. code-block:: bash
+
+         tr '\0' '\n' < /proc/${GEOSERVER_PID}/environ
+
 Application Server Guidance
 ---------------------------
 

diff --git a/src/platform/src/main/java/org/geoserver/platform/SystemEnvironmentStatus.java b/src/platform/src/main/java/org/geoserver/platform/SystemEnvironmentStatus.java
@@ -7,10 +7,23 @@
 
 import java.util.Map.Entry;
 import java.util.Optional;
+import java.util.logging.Level;
+import java.util.logging.Logger;
+import org.geotools.util.logging.Logging;
 
 /** Report system environment details to server status page. */
 public class SystemEnvironmentStatus implements ModuleStatus {
 
+    private static final Logger LOGGER = Logging.getLogger(SystemEnvironmentStatus.class);
+
+    /**
+     * Name of the environment variable that turns on the details (listing of all environment
+     * variables) for this module. "false" = don't show, "true" = show all the environment variables
+     * on the web interface.
+     */
+    public static final String SystemEnvironmentStatusEnabledEnvironmentVar =
+            "GEOSERVER_MODULE_SYSTEM_ENVIRONMENT_STATUS_ENABLED";
+
     @Override
     public String getModule() {
         return "system-environment";
@@ -41,8 +54,44 @@ public boolean isEnabled() {
         return true;
     }
 
+    /** For Testing - this can be mocked to change environment variables. */
+    String getEnvironmentVariable(String envVar) {
+        return System.getenv(envVar);
+    }
+
+    /**
+     * returns true if the message (list of variables) should be shown
+     *
+     * <p>Uses environment variable SystemPropertyStatusEnabledEnvironmentVar
+     * ("GEOSERVER_MODULE_SYSTEM_ENVIRONMENT_STATUS_ENABLED") not defined -> false (default) bad
+     * value -> false (default)
+     */
+    public boolean isShow() {
+        String val = getEnvironmentVariable(SystemEnvironmentStatusEnabledEnvironmentVar);
+        if (val == null) {
+            return false; // not defined -> default
+        }
+        if (val.equalsIgnoreCase("true") || val.equalsIgnoreCase("false")) {
+            return val.equalsIgnoreCase("true");
+        }
+        LOGGER.log(
+                Level.WARNING,
+                String.format(
+                        "environment variable '%s' should be 'true' or 'false', but was '%s'",
+                        SystemEnvironmentStatusEnabledEnvironmentVar, val));
+        return false; // bad value -> default
+    }
+
     @Override
     public Optional<String> getMessage() {
+        if (!isShow()) {
+            var message =
+                    String.format(
+                            "Environment variables hidden for security reasons.  Set the environment variable '%s' to 'true' to see them.",
+                            SystemEnvironmentStatusEnabledEnvironmentVar);
+            return Optional.of(message);
+        }
+
         StringBuffer result = new StringBuffer();
         for (Entry<String, String> entry : System.getenv().entrySet()) {
             result.append(entry.getKey().toString() + "=" + entry.getValue().toString() + "\n");

diff --git a/src/platform/src/main/java/org/geoserver/platform/SystemPropertyStatus.java b/src/platform/src/main/java/org/geoserver/platform/SystemPropertyStatus.java
@@ -7,9 +7,22 @@
 
 import java.util.Map.Entry;
 import java.util.Optional;
+import java.util.logging.Level;
+import java.util.logging.Logger;
+import org.geotools.util.logging.Logging;
 
 public class SystemPropertyStatus implements ModuleStatus {
 
+    private static final Logger LOGGER = Logging.getLogger(SystemPropertyStatus.class);
+
+    /**
+     * Name of the environment variable that turns on the details (listing of all property
+     * variables) for this module. "false" = don't show, "true" = show all the environment variables
+     * on the web interface.
+     */
+    public static final String SystemPropertyStatusEnabledEnvironmentVar =
+            "GEOSERVER_MODULE_SYSTEM_PROPERTY_STATUS_ENABLED";
+
     @Override
     public String getModule() {
         return "system-properties";
@@ -40,8 +53,43 @@ public boolean isEnabled() {
         return true;
     }
 
+    /** For Testing - this can be mocked to change environment variables. */
+    String getEnvironmentVariable(String envVar) {
+        return System.getenv(envVar);
+    }
+
+    /**
+     * returns true if the message (list of variables) should be shown
+     *
+     * <p>Uses environment variable SystemPropertyStatusEnabledEnvironmentVar
+     * ("GEOSERVER_MODULE_SYSTEM_PROPERTY_STATUS_ENABLED") not defined -> false (default) bad value
+     * -> false (default)
+     */
+    public boolean isShow() {
+        String val = getEnvironmentVariable(SystemPropertyStatusEnabledEnvironmentVar);
+        if (val == null) {
+            return false; // not defined -> default
+        }
+        if (val.equalsIgnoreCase("true") || val.equalsIgnoreCase("false")) {
+            return val.equalsIgnoreCase("true");
+        }
+        LOGGER.log(
+                Level.WARNING,
+                String.format(
+                        "environment variable '%s' should be 'true' or 'false', but was '%s'",
+                        SystemPropertyStatusEnabledEnvironmentVar, val));
+        return false; // bad value -> default
+    }
+
     @Override
     public Optional<String> getMessage() {
+        if (!isShow()) {
+            var message =
+                    String.format(
+                            "Java system properties hidden for security reasons.  Set the environment variable '%s' to 'true' to see them.",
+                            SystemPropertyStatusEnabledEnvironmentVar);
+            return Optional.of(message);
+        }
         StringBuffer result = new StringBuffer();
         for (Entry<Object, Object> entry : System.getProperties().entrySet()) {
             result.append(entry.getKey().toString() + "=" + entry.getValue().toString() + "\n");

diff --git a/src/platform/src/test/java/org/geoserver/platform/SystemEnvironmentTest.java b/src/platform/src/test/java/org/geoserver/platform/SystemEnvironmentTest.java
@@ -8,6 +8,7 @@
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 
+import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.logging.Logger;
@@ -26,12 +27,90 @@ public void testSystemPropertiesStatus() {
         String key = System.getenv().keySet().iterator().next();
         String value = System.getenv(key);
 
-        SystemEnvironmentStatus status = new SystemEnvironmentStatus();
+        SystemEnvironmentStatus status =
+                new SystemEnvironmentStatus() {
+                    String getEnvironmentVariable(String envVar) {
+                        return "true";
+                    }
+                };
+
         assertTrue(status.getMessage().isPresent());
         assertTrue(status.getMessage().get().contains(key));
         assertTrue(status.getMessage().get().contains(value));
     }
 
+    /**
+     * Tests the SystemEnvironmentStatusEnabledEnvironmentVar so it turns on/off the message (list
+     * of environment vars).
+     */
+    @Test
+    public void testEnabled() {
+        final var VALUE = new ArrayList<String>();
+
+        // create subclass of SystemEnvironmentStatus so we can change the value of the environment
+        // variable.
+        // VALUE empty -> null
+        // otherwise its the first item in the VALUE
+        // if the request is for a different environment var -> throw
+        SystemEnvironmentStatus status =
+                new SystemEnvironmentStatus() {
+                    String getEnvironmentVariable(String envVar) {
+                        if (envVar.equals(
+                                SystemEnvironmentStatus
+                                        .SystemEnvironmentStatusEnabledEnvironmentVar)) {
+                            if (VALUE.isEmpty()) {
+                                return null;
+                            }
+                            return VALUE.get(0);
+                        }
+                        throw new RuntimeException("bad var");
+                    }
+                };
+
+        VALUE.clear();
+        VALUE.add("true");
+        assertTrue(status.isShow());
+        assertTrue(!status.getMessage().isEmpty());
+
+        VALUE.clear();
+        VALUE.add("TRUE");
+        assertTrue(status.isShow());
+        assertTrue(!status.getMessage().isEmpty());
+
+        VALUE.clear();
+        VALUE.add("FALSE");
+        assertFalse(status.isShow());
+        assertTrue(
+                status.getMessage()
+                        .get()
+                        .startsWith("Environment variables hidden for security reasons."));
+
+        VALUE.clear();
+        VALUE.add("false");
+        assertFalse(status.isShow());
+        assertTrue(
+                status.getMessage()
+                        .get()
+                        .startsWith("Environment variables hidden for security reasons."));
+
+        // default -> false
+        VALUE.clear();
+        assertFalse(status.isShow());
+        assertTrue(
+                status.getMessage()
+                        .get()
+                        .startsWith("Environment variables hidden for security reasons."));
+
+        // bad value -> false
+        VALUE.clear();
+        VALUE.add("maybe");
+        assertFalse(status.isShow());
+        assertTrue(
+                status.getMessage()
+                        .get()
+                        .startsWith("Environment variables hidden for security reasons."));
+    }
+
     @Test
     public void testGeoServerEnvironmentDefaultValue() {
         System.clearProperty("ALLOW_ENV_PARAMETRIZATION");

diff --git a/src/platform/src/test/java/org/geoserver/platform/SystemPropertiesStatusTest.java b/src/platform/src/test/java/org/geoserver/platform/SystemPropertiesStatusTest.java
@@ -5,8 +5,10 @@
 
 package org.geoserver.platform;
 
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 
+import java.util.ArrayList;
 import org.junit.Test;
 
 public class SystemPropertiesStatusTest {
@@ -18,9 +20,87 @@ public class SystemPropertiesStatusTest {
     @Test
     public void testSystemPropertiesStatus() {
         System.setProperty(KEY, VALUE);
-        SystemPropertyStatus status = new SystemPropertyStatus();
+
+        SystemPropertyStatus status =
+                new SystemPropertyStatus() {
+                    String getEnvironmentVariable(String envVar) {
+                        return "true";
+                    }
+                };
+
         assertTrue(status.getMessage().isPresent());
         assertTrue(status.getMessage().get().contains(KEY));
         assertTrue(status.getMessage().get().contains(VALUE));
     }
+
+    /**
+     * Tests the SystemPropertyStatusEnabledEnvironmentVar so it turns on/off the message (list of
+     * property vars).
+     */
+    @Test
+    public void testEnabled() {
+        final var VALUE = new ArrayList<String>();
+
+        // create subclass of SystemPropertyStatus so we can change the value of the environment
+        // variable.
+        // VALUE empty -> null
+        // otherwise its the first item in the VALUE
+        // if the request is for a different environment var -> throw
+        SystemPropertyStatus status =
+                new SystemPropertyStatus() {
+                    String getEnvironmentVariable(String envVar) {
+                        if (envVar.equals(
+                                SystemPropertyStatus.SystemPropertyStatusEnabledEnvironmentVar)) {
+                            if (VALUE.isEmpty()) {
+                                return null;
+                            }
+                            return VALUE.get(0);
+                        }
+                        throw new RuntimeException("bad var");
+                    }
+                };
+
+        VALUE.clear();
+        VALUE.add("true");
+        assertTrue(status.isShow());
+        assertTrue(!status.getMessage().isEmpty());
+
+        VALUE.clear();
+        VALUE.add("TRUE");
+        assertTrue(status.isShow());
+        assertTrue(!status.getMessage().isEmpty());
+
+        VALUE.clear();
+        VALUE.add("FALSE");
+        assertFalse(status.isShow());
+        assertTrue(
+                status.getMessage()
+                        .get()
+                        .startsWith("Java system properties hidden for security reasons."));
+
+        VALUE.clear();
+        VALUE.add("false");
+        assertFalse(status.isShow());
+        assertTrue(
+                status.getMessage()
+                        .get()
+                        .startsWith("Java system properties hidden for security reasons."));
+
+        // default -> false
+        VALUE.clear();
+        assertFalse(status.isShow());
+        assertTrue(
+                status.getMessage()
+                        .get()
+                        .startsWith("Java system properties hidden for security reasons."));
+
+        // bad value -> false
+        VALUE.clear();
+        VALUE.add("maybe");
+        assertFalse(status.isShow());
+        assertTrue(
+                status.getMessage()
+                        .get()
+                        .startsWith("Java system properties hidden for security reasons."));
+    }
 }