diff --git a/doc/en/user/source/configuration/properties/index.rst b/doc/en/user/source/configuration/properties/index.rst index 5d9f2d722b1..4c0ec64f8fe 100644 --- a/doc/en/user/source/configuration/properties/index.rst +++ b/doc/en/user/source/configuration/properties/index.rst @@ -314,6 +314,12 @@ GeoServer Property Reference - x - x - x + * - GEOSERVER_FILESYSTEM_SANDBOX + + :doc:`/security/sandbox` + - x + - x + - x Setting Application property ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/doc/en/user/source/extensions/importer/index.rst b/doc/en/user/source/extensions/importer/index.rst index fc9ae4148a4..bb94cbf29dc 100644 --- a/doc/en/user/source/extensions/importer/index.rst +++ b/doc/en/user/source/extensions/importer/index.rst @@ -11,6 +11,10 @@ There are two primary advantages to using the Importer over the standard GeoServ #. **Creates unique styles** for each layer, rather than linking to the same (existing) styles. +.. warning:: The importer extension allows upload of data and is currently unable to respect the file system sandbox, + it uses a configurable location inside the data directory instead. Store creation will fail if the importer + is used and the sandbox is set. See also :ref:`security_sandbox`. + This section will discuss the Importer extension. .. toctree:: diff --git a/doc/en/user/source/security/index.rst b/doc/en/user/source/security/index.rst index c0d15b28681..ad47e49acc0 100644 --- a/doc/en/user/source/security/index.rst +++ b/doc/en/user/source/security/index.rst @@ -17,6 +17,7 @@ The first page discusses configuration options in the web administration interfa root service layer + sandbox rest urlchecks disable diff --git a/doc/en/user/source/security/sandbox.rst b/doc/en/user/source/security/sandbox.rst new file mode 100644 index 00000000000..d9838784746 --- /dev/null +++ b/doc/en/user/source/security/sandbox.rst @@ -0,0 +1,61 @@ +.. _security_sandbox: + +Filesystem sandboxing +===================== + +GeoServer administrators can usually explore the full file system of the server where GeoServer +is running into, with the same privileges as the user running the servlet container. + +This can be limited by setting up a sandbox, which will restrict the access to the file system +to a specific directory tree. The sandbox can be set up at two levels: + +* **System sandbox**: the GeoServer administrator is sandboxed into a specific directory, and won't be + able to access files outside of it, nor change the sandbox configuration. +* **Regular sandbox**: the GeoServer administrator can still access the full file system, but can set up + a sandbox for each workspace, where the workspace administrators will be sandboxed into. + +.. warning:: The importer extension allows upload of data and is currently unable to respect the file system sandbox, + it uses a configurable location inside the data directory instead. Store creation will fail if the importer + is used and the sandbox is set. + +Setting up a system sandbox +--------------------------- + +The system sandbox is configured by setting the ``GEOSERVER_FILESYSTEM_SANDBOX`` variable to the +directory where the GeoServer administrator should be sandboxed into. +The variable can be provided as a Java system variable, as a servlet context parameter, or as an +environment variable, please consult the :ref:`application_properties` section for more details. + +When the system sandbox is set: + +* The GeoServer administrator will be sandboxed into the configured directory, + and won't be able to access files outside of it, nor change the sandbox configuration. +* The GeoServer workspace administrators will be sandboxed into ``/``, where + ```` is the name of any workspace they can access. + +The system sandbox is best suited in hosting environments, where the GeoServer administrator and the +operating system administrator are different people, and the GeoServer administrator should not be +able to access the full file system. + +Setting up a regular sandbox +---------------------------- + +The regular sandbox can be configured by GeoServer full administrators in the user interface, +from the :guilabel:`Security` -> :guilabel:`Data` page, or by adding the following entry in the +``layers.properties`` file: + +.. code-block:: properties + + # Set the sandbox for the workspace + filesystemSandbox=/path/to/sandbox + +When the regular sandbox is set: + +* The GeoServer administrator will still be able to access the full file system, + as well as change the sandbox configuration if they so desire. +* The GeoServer workspace administrators will be sandboxed into ``/``, where + ```` is the name of any workspace they can access. + +The regular sandbox is best suited in multi-tenant environments where the main GeoServer administrator +also has access to the server operating system, while each tenant is modelled as a workspace +administrator and should be able to manage its own data, but not access the data of other tenants. diff --git a/doc/en/user/source/security/webadmin/data.rst b/doc/en/user/source/security/webadmin/data.rst index 947016a4011..675aad52f13 100644 --- a/doc/en/user/source/security/webadmin/data.rst +++ b/doc/en/user/source/security/webadmin/data.rst @@ -58,3 +58,34 @@ This mode configures how GeoServer will advertise secured layers and behave when :align: center *Catalog mode* + +File sandbox +------------ + +The sandbox allows a GeoServer full administrator to limit file system access to workspace administrators. +In particular, both GUI and REST API will sandbox workspace administrators to ``/``, +and prevent them from accessing files outside of it. + +.. figure:: images/fs_sandbox.png + :align: center + + *Filesystem sandbox* + +This part of the page is not visible if the operating system administrator has established +a sandbox for the whole GeoServer instance, in which case even the GeoServer full administrators +will be limited in the configured sandbox and won't be able to change it. + +When the sandbox is configured, the file system chooser will show the accessible directories as +the file system roots. E.g., if a sandbox has been set to `/var/lib/geoserver`, and the current +workspace administrator has access to both the `sf` and `ne` workspaces, the file system +chooser will look as follows: + +.. figure:: images/fs_sandbox_chooser.png + :align: center + + *File system chooser* + +Any attempt to manually set a path outside of the sandbox will result in a validation error +and prevent the store from being saved (both from the UI and the REST API). + +For more information please refer to :ref:`security_sandbox` for more details. \ No newline at end of file diff --git a/doc/en/user/source/security/webadmin/images/fs_sandbox.png b/doc/en/user/source/security/webadmin/images/fs_sandbox.png new file mode 100644 index 00000000000..bf425cb4906 Binary files /dev/null and b/doc/en/user/source/security/webadmin/images/fs_sandbox.png differ diff --git a/doc/en/user/source/security/webadmin/images/fs_sandbox_chooser.png b/doc/en/user/source/security/webadmin/images/fs_sandbox_chooser.png new file mode 100644 index 00000000000..c92bd70a4c0 Binary files /dev/null and b/doc/en/user/source/security/webadmin/images/fs_sandbox_chooser.png differ diff --git a/src/extension/importer/web/src/main/java/org/geoserver/importer/web/CoverageStoreEditPage.java b/src/extension/importer/web/src/main/java/org/geoserver/importer/web/CoverageStoreEditPage.java index cca15f9ea39..b5667ebeaab 100644 --- a/src/extension/importer/web/src/main/java/org/geoserver/importer/web/CoverageStoreEditPage.java +++ b/src/extension/importer/web/src/main/java/org/geoserver/importer/web/CoverageStoreEditPage.java @@ -14,10 +14,11 @@ public CoverageStoreEditPage(CoverageStoreInfo store) { } @Override - protected void doSaveStore(CoverageStoreInfo info) { + protected boolean doSaveStore(CoverageStoreInfo info) { if (info.getId() != null) { - super.doSaveStore(info); + return super.doSaveStore(info); } // do nothing, not part of catalog yet + return true; } } diff --git a/src/extension/importer/web/src/main/java/org/geoserver/importer/web/DataStoreEditPage.java b/src/extension/importer/web/src/main/java/org/geoserver/importer/web/DataStoreEditPage.java index 38c758ea0c4..6587e108b31 100644 --- a/src/extension/importer/web/src/main/java/org/geoserver/importer/web/DataStoreEditPage.java +++ b/src/extension/importer/web/src/main/java/org/geoserver/importer/web/DataStoreEditPage.java @@ -15,11 +15,12 @@ public DataStoreEditPage(DataStoreInfo store) { } @Override - protected void doSaveStore(DataStoreInfo info) { + protected boolean doSaveStore(DataStoreInfo info) { if (info.getId() != null) { - super.doSaveStore(info); + return super.doSaveStore(info); } // do nothing, not part of catalog yet + return true; } } diff --git a/src/main/src/main/java/applicationSecurityContext.xml b/src/main/src/main/java/applicationSecurityContext.xml index cf336f95727..8bc7de89abc 100644 --- a/src/main/src/main/java/applicationSecurityContext.xml +++ b/src/main/src/main/java/applicationSecurityContext.xml @@ -183,4 +183,14 @@ + + + + + + + + + + diff --git a/src/main/src/main/java/org/geoserver/catalog/LockingCatalogFacade.java b/src/main/src/main/java/org/geoserver/catalog/LockingCatalogFacade.java index ee8279a730a..c9238980cae 100644 --- a/src/main/src/main/java/org/geoserver/catalog/LockingCatalogFacade.java +++ b/src/main/src/main/java/org/geoserver/catalog/LockingCatalogFacade.java @@ -5,6 +5,7 @@ package org.geoserver.catalog; import java.lang.reflect.InvocationHandler; +import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import org.geoserver.GeoServerConfigurationLock; import org.geoserver.GeoServerConfigurationLock.LockType; @@ -33,7 +34,13 @@ public Object invoke(Object proxy, Method method, Object[] args) throws Throwabl if (lockType == LockType.READ && isWriteMethod(method)) { configurationLock.tryUpgradeLock(); } - return method.invoke(delegate, args); + try { + return method.invoke(delegate, args); + } catch (InvocationTargetException e) { + // preserve the original exception + Throwable cause = e.getCause(); + throw cause; + } } private boolean isWriteMethod(Method method) { diff --git a/src/main/src/main/java/org/geoserver/catalog/ResourcePool.java b/src/main/src/main/java/org/geoserver/catalog/ResourcePool.java index a6e593bdd27..3c0286824da 100644 --- a/src/main/src/main/java/org/geoserver/catalog/ResourcePool.java +++ b/src/main/src/main/java/org/geoserver/catalog/ResourcePool.java @@ -1690,7 +1690,8 @@ private GridCoverageReader getGridCoverageReader( // ///////////////////////////////////////////////////////// final String urlString = expandedStore.getURL(); Object readObject = - getObjectToRead(urlString, coverageInfo, expandedStore, hints); + getCoverageStoreSource( + urlString, coverageInfo, expandedStore, hints); // readers might change the provided hints, pass down a defensive copy reader = gridFormat.getReader(readObject, hints); @@ -1763,13 +1764,13 @@ private GridCoverageReader getGridCoverageReader( } /** - * Attempted to convert the URL-ish string to a parseable input object, otherwise just returns - * the string itself + * Attempted to convert the URL-ish string to a parseable input object for coverage reading + * purposes, otherwise just returns the string itself * * @param urlString the url string to parse, which may actually be a path * @return an object appropriate for passing to a grid coverage reader */ - private Object getObjectToRead( + public static Object getCoverageStoreSource( String urlString, CoverageInfo coverageInfo, CoverageStoreInfo coverageStoreInfo, diff --git a/src/main/src/main/java/org/geoserver/catalog/event/AbstractCatalogListener.java b/src/main/src/main/java/org/geoserver/catalog/event/AbstractCatalogListener.java new file mode 100644 index 00000000000..bae9e63df52 --- /dev/null +++ b/src/main/src/main/java/org/geoserver/catalog/event/AbstractCatalogListener.java @@ -0,0 +1,39 @@ +/* (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.catalog.event; + +import org.geoserver.catalog.CatalogException; + +/** + * A base class for {@link CatalogListener} that implements all listener methods without any action. + * Useful for listeners that are only interested in a subset of events. + */ +public class AbstractCatalogListener implements CatalogListener { + + @Override + public void handleAddEvent(CatalogAddEvent event) throws CatalogException { + // nothing to do + } + + @Override + public void handleRemoveEvent(CatalogRemoveEvent event) throws CatalogException { + // nothing to do + } + + @Override + public void handleModifyEvent(CatalogModifyEvent event) throws CatalogException { + // nothing to do + } + + @Override + public void handlePostModifyEvent(CatalogPostModifyEvent event) throws CatalogException { + // nothing to do + } + + @Override + public void reloaded() { + // nothing to do + } +} diff --git a/src/main/src/main/java/org/geoserver/security/FileAccessManager.java b/src/main/src/main/java/org/geoserver/security/FileAccessManager.java new file mode 100644 index 00000000000..b72412d1d35 --- /dev/null +++ b/src/main/src/main/java/org/geoserver/security/FileAccessManager.java @@ -0,0 +1,67 @@ +/* (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.io.File; +import java.util.List; +import org.geoserver.platform.GeoServerExtensions; +import org.geoserver.security.impl.DefaultFileAccessManager; +import org.springframework.security.core.Authentication; +import org.springframework.security.core.context.SecurityContextHolder; + +/** + * Provides the GUI, REST API and catalog checks with directives on what parts of the file system + * the current user can access. + */ +public interface FileAccessManager { + /** + * Returns the file system roots available for the current user (or null if there + * are no restrictions) + */ + public List getAvailableRoots(); + + /** + * Returns the sandbox root directory, if there is one, or null if there is none + * (i.e., the user can access the whole file system). This is used by the REST API to + * automatically prepend the sandbox root to the uploaded file paths. + */ + public File getSandbox(); + + /** + * Checks if the specified file is accessible in the context of the current request + * + * @param file the file to check + */ + public boolean checkAccess(File file); + + /** + * Looks up the {@link FileAccessManager} to use, preferring a custom implementation if + * available, otherwise falling back on the default one. Mimics the behavior in {@link + * org.geoserver.security.SecureCatalogImpl} + */ + public static FileAccessManager lookupFileAccessManager() { + List managers = GeoServerExtensions.extensions(FileAccessManager.class); + if (managers.isEmpty()) + throw new RuntimeException("Unexpected, no FileAdminAccessManager found"); + + FileAccessManager manager = null; + for (FileAccessManager resourceAccessManager : managers) { + if (!DefaultFileAccessManager.class.equals(resourceAccessManager.getClass())) { + manager = resourceAccessManager; + break; + } + } + + // no custom manager found? + if (manager == null) manager = managers.get(0); + + return manager; + } + + /** Returns the current user authentication */ + default Authentication user() { + return SecurityContextHolder.getContext().getAuthentication(); + } +} diff --git a/src/main/src/main/java/org/geoserver/security/impl/DataAccessRuleDAO.java b/src/main/src/main/java/org/geoserver/security/impl/DataAccessRuleDAO.java index a52b2172fb7..db863e58207 100644 --- a/src/main/src/main/java/org/geoserver/security/impl/DataAccessRuleDAO.java +++ b/src/main/src/main/java/org/geoserver/security/impl/DataAccessRuleDAO.java @@ -37,6 +37,8 @@ */ public class DataAccessRuleDAO extends AbstractAccessRuleDAO { + public static final String KEY_MODE = "mode"; + private static final String KEY_SANDBOX = "filesystemSandbox"; private static Pattern DOT = Pattern.compile("\\."); private static final Logger LOGGER = Logging.getLogger(DataAccessRuleDAO.class); @@ -50,6 +52,8 @@ public class DataAccessRuleDAO extends AbstractAccessRuleDAO { /** Default to the highest security mode */ CatalogMode catalogMode = CatalogMode.HIDE; + String filesystemSandbox; + /** Returns the instanced contained in the Spring context for the UI to use */ public static DataAccessRuleDAO get() { return GeoServerExtensions.bean(DataAccessRuleDAO.class); @@ -78,12 +82,13 @@ public CatalogMode getMode() { protected void loadRules(Properties props) { SortedSet result = new ConcurrentSkipListSet<>(); CatalogMode catalogMode = CatalogMode.HIDE; + this.filesystemSandbox = null; for (Map.Entry entry : props.entrySet()) { String ruleKey = (String) entry.getKey(); String ruleValue = (String) entry.getValue(); // check for the mode - if ("mode".equalsIgnoreCase(ruleKey)) { + if (KEY_MODE.equalsIgnoreCase(ruleKey)) { try { catalogMode = CatalogMode.valueOf(ruleValue.toUpperCase()); } catch (Exception e) { @@ -93,6 +98,8 @@ protected void loadRules(Properties props) { + " acceptable values are " + Arrays.asList(CatalogMode.values())); } + } else if (KEY_SANDBOX.equalsIgnoreCase(ruleKey)) { + filesystemSandbox = ruleValue; } else { DataAccessRule rule = parseDataAccessRule(ruleKey, ruleValue); if (rule != null) { @@ -208,7 +215,10 @@ DataAccessRule parseDataAccessRule(String ruleKey, String ruleValue) { @Override protected Properties toProperties() { Properties props = new Properties(); - props.put("mode", catalogMode.toString()); + props.put(KEY_MODE, catalogMode.toString()); + if (filesystemSandbox != null) { + props.put(KEY_SANDBOX, filesystemSandbox); + } for (DataAccessRule rule : rules) { StringBuilder sbKey = new StringBuilder(DOT.matcher(rule.getRoot()).replaceAll("\\\\.")); @@ -259,4 +269,25 @@ public SortedSet getRulesAssociatedWithRole(String role) { for (DataAccessRule rule : getRules()) if (rule.getRoles().contains(role)) result.add(rule); return result; } + + /** + * Returns the file system sandbox configured in layers.properties, if set, or null + * otherwise. + */ + public String getFilesystemSandbox() { + return filesystemSandbox; + } + + /** + * Sets the file system sandbox to be used by the file access manager. + * + * @param filesystemSandbox the file system sandbox to be used by the file access manager, or + * null if the sandbox should be removed + */ + public void setFilesystemSandbox(String filesystemSandbox) { + // sanitize in case a store-like path has ben provided + if (filesystemSandbox.startsWith("file://")) + filesystemSandbox = filesystemSandbox.substring("file://".length()); + this.filesystemSandbox = filesystemSandbox; + } } diff --git a/src/main/src/main/java/org/geoserver/security/impl/DefaultFileAccessManager.java b/src/main/src/main/java/org/geoserver/security/impl/DefaultFileAccessManager.java new file mode 100644 index 00000000000..069cb613fec --- /dev/null +++ b/src/main/src/main/java/org/geoserver/security/impl/DefaultFileAccessManager.java @@ -0,0 +1,182 @@ +/* (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.impl; + +import java.io.File; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.ArrayList; +import java.util.List; +import java.util.logging.Level; +import java.util.logging.Logger; +import java.util.stream.Collectors; +import org.geoserver.catalog.WorkspaceInfo; +import org.geoserver.catalog.util.CloseableIterator; +import org.geoserver.platform.GeoServerExtensions; +import org.geoserver.security.FileAccessManager; +import org.geoserver.security.GeoServerSecurityManager; +import org.geoserver.security.ResourceAccessManager; +import org.geoserver.security.ResourceAccessManagerWrapper; +import org.geoserver.security.SecureCatalogImpl; +import org.geoserver.security.WorkspaceAccessLimits; +import org.geotools.api.filter.Filter; +import org.geotools.util.logging.Logging; +import org.springframework.security.core.Authentication; + +/** + * Default implementation of the {@link FileAccessManager} interface, that uses the {@link + * DataAccessRuleDAO} to determine the file system sandbox and the {@link SecureCatalogImpl} to + * determine the workspaces that can be accessed. In case the GEOSERVER_FILESYSTEM_SANDBOX system + * property is set, the sandbox is imposed by the system administrator and even the GeoServer + * administrator will be limited to it, otherwise it is imposed by GeoServer administrator throught + * the security subsystem and the full administrator will be able to see the whole file system while + * workspace administrators will be limited to their own workspace directories. + */ +public class DefaultFileAccessManager implements FileAccessManager { + + private static final Logger LOGGER = Logging.getLogger(DefaultFileAccessManager.class); + + public static String GEOSERVER_DATA_SANDBOX = "GEOSERVER_FILESYSTEM_SANDBOX"; + + private final DataAccessRuleDAO dao; + private final GeoServerSecurityManager securityManager; + private final ResourceAccessManager resourceAccessManager; + private final SecureCatalogImpl catalog; + private String systemSandbox; + + public DefaultFileAccessManager( + DataAccessRuleDAO dao, + SecureCatalogImpl catalog, + GeoServerSecurityManager securityManager) { + this.dao = dao; + this.systemSandbox = GeoServerExtensions.getProperty(GEOSERVER_DATA_SANDBOX); + this.catalog = catalog; + this.securityManager = securityManager; + this.resourceAccessManager = catalog.getResourceAccessManager(); + } + + @Override + public List getAvailableRoots() { + String sandboxPath = systemSandbox != null ? systemSandbox : dao.getFilesystemSandbox(); + if (sandboxPath == null) return null; + + // the full administrator is either locked into the sandbox, if that was set + // by the OS sysadmin, or can see the whole file system + Authentication auth = user(); + boolean fullAdmin = securityManager.checkAuthenticationForAdminRole(auth); + if (fullAdmin) { + if (systemSandbox != null) return List.of(new File(systemSandbox)); + else return null; + } + + // it's a workspace admin then + List accessibleWorkspaces = new ArrayList<>(); + try (CloseableIterator workspaces = + catalog.list(WorkspaceInfo.class, Filter.INCLUDE)) { + while (workspaces.hasNext()) { + WorkspaceInfo ws = workspaces.next(); + WorkspaceAccessLimits accessLimits = + resourceAccessManager.getAccessLimits(auth, ws); + if (accessLimits != null && accessLimits.isAdminable()) { + accessibleWorkspaces.add(ws.getName()); + } + } + } + + // maps the workspace names to actual directories + // and creates them if they are missing, as the rest of GeoServer needs file system + // roots that are actually there + List roots = + accessibleWorkspaces.stream() + .map(ws -> new File(sandboxPath, ws)) + .collect(Collectors.toList()); + roots.forEach(File::mkdirs); + return roots; + } + + @Override + public File getSandbox() { + String sandboxPath = systemSandbox != null ? systemSandbox : dao.getFilesystemSandbox(); + if (sandboxPath == null) return null; + return new File(sandboxPath); + } + + @Override + public boolean checkAccess(File file) { + // Convert File to Path + String sandboxPath = systemSandbox != null ? systemSandbox : dao.getFilesystemSandbox(); + LOGGER.log(Level.FINE, () -> "Filesystem sandbox: " + sandboxPath); + if (sandboxPath == null) return true; + Path sandbox = canonical(sandboxPath); + Path path = canonical(file); + + // Check if the user is a full administrator + Authentication auth = user(); + boolean fullAdmin = securityManager.checkAuthenticationForAdminRole(); + if (fullAdmin) { + if (systemSandbox != null) { + if (!path.startsWith(sandbox)) { + LOGGER.log( + Level.FINE, + () -> "Checked path " + path + " does not start with " + sandbox); + return false; + } + } + return true; + } + + // Check if the file is within the sandbox + if (!path.startsWith(sandbox)) return false; + + // Check if the workspace is accessible + String workspace = sandbox.relativize(path).getName(0).toString(); + WorkspaceInfo wi = catalog.getWorkspaceByName(workspace); + if (wi == null) { + LOGGER.log(Level.FINE, () -> "Sandbox check, workspace not authorized " + workspace); + return false; + } + WorkspaceAccessLimits accessLimits = resourceAccessManager.getAccessLimits(auth, wi); + LOGGER.log( + Level.FINE, + () -> + "Sandbox auth check, workspace " + + workspace + + " access limits " + + accessLimits); + return accessLimits != null && accessLimits.isAdminable(); + } + + /** Forces reloading the DAO and the system sandbox definitions */ + public void reload() { + this.systemSandbox = GeoServerExtensions.getProperty(GEOSERVER_DATA_SANDBOX); + if (systemSandbox != null) + LOGGER.log(Level.FINE, () -> "System sandbox property found: " + systemSandbox); + dao.reload(); + ResourceAccessManager ram = this.resourceAccessManager; + while (ram instanceof ResourceAccessManagerWrapper) { + ram = ((ResourceAccessManagerWrapper) ram).unwrap(); + } + if (ram instanceof DefaultResourceAccessManager) { + ((DefaultResourceAccessManager) ram).reload(); + } + } + + private static Path canonical(String fileName) { + return Paths.get(fileName).toAbsolutePath().normalize(); + } + + private static Path canonical(File file) { + return file.toPath().toAbsolutePath().normalize(); + } + + /** + * Returns true if a system sandbox is imposed via system properties, false if the sanbox is + * defined in the security subsystem instead. + */ + public boolean isSystemSanboxEnabled() { + return systemSandbox != null; + } +} diff --git a/src/main/src/main/java/org/geoserver/security/impl/DefaultResourceAccessManager.java b/src/main/src/main/java/org/geoserver/security/impl/DefaultResourceAccessManager.java index 4fd79fee978..c09d059529e 100644 --- a/src/main/src/main/java/org/geoserver/security/impl/DefaultResourceAccessManager.java +++ b/src/main/src/main/java/org/geoserver/security/impl/DefaultResourceAccessManager.java @@ -335,6 +335,14 @@ void checkPropertyFile() { rebuildAuthorizationTree(false); } + /** + * Forcefully reloads the configuration from the file system. Mostly used for testing (Windows + * notifications for file changes are sometimes too slow) + */ + public void reload() { + rebuildAuthorizationTree(true); + } + private void rebuildAuthorizationTree(boolean force) { long daoLastModified = dao.getLastModified(); if (lastLoaded < daoLastModified || force) { diff --git a/src/main/src/main/java/org/geoserver/security/impl/FileSandboxEnforcer.java b/src/main/src/main/java/org/geoserver/security/impl/FileSandboxEnforcer.java new file mode 100644 index 00000000000..b2ae08c12f4 --- /dev/null +++ b/src/main/src/main/java/org/geoserver/security/impl/FileSandboxEnforcer.java @@ -0,0 +1,217 @@ +/* (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.impl; + +import java.io.File; +import java.io.Serializable; +import java.net.MalformedURLException; +import java.net.URI; +import java.net.URISyntaxException; +import java.net.URL; +import java.nio.file.InvalidPathException; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.List; +import java.util.Map; +import java.util.logging.Level; +import java.util.logging.Logger; +import org.apache.commons.lang3.StringUtils; +import org.geoserver.catalog.Catalog; +import org.geoserver.catalog.CatalogException; +import org.geoserver.catalog.CatalogInfo; +import org.geoserver.catalog.CoverageStoreInfo; +import org.geoserver.catalog.DataStoreInfo; +import org.geoserver.catalog.HTTPStoreInfo; +import org.geoserver.catalog.ResourcePool; +import org.geoserver.catalog.StoreInfo; +import org.geoserver.catalog.event.AbstractCatalogListener; +import org.geoserver.catalog.event.CatalogBeforeAddEvent; +import org.geoserver.catalog.event.CatalogListener; +import org.geoserver.catalog.event.CatalogModifyEvent; +import org.geoserver.platform.GeoServerResourceLoader; +import org.geoserver.security.FileAccessManager; +import org.geotools.api.data.DataAccessFactory; +import org.geotools.api.data.DataAccessFactory.Param; +import org.geotools.util.URLs; +import org.geotools.util.factory.Hints; +import org.geotools.util.logging.Logging; + +/** + * A {@link CatalogListener} that enforce the file sandbox rules. Checks if the user is allowed to + * access the file system in response to store modification events. + */ +public class FileSandboxEnforcer extends AbstractCatalogListener { + + private static final Logger LOGGER = Logging.getLogger(FileSandboxEnforcer.class); + + private final ResourcePool resourcePool; + private final FileAccessManager fileAccessManager; + + public FileSandboxEnforcer(Catalog catalog) { + catalog.addListener(this); + this.resourcePool = catalog.getResourcePool(); + this.fileAccessManager = FileAccessManager.lookupFileAccessManager(); + } + + @Override + public void handlePreAddEvent(CatalogBeforeAddEvent event) throws CatalogException { + CatalogInfo source = event.getSource(); + if (!(source instanceof StoreInfo)) return; + + if (source instanceof DataStoreInfo) { + DataStoreInfo store = (DataStoreInfo) source; + checkDataStoreParameters(store, store.getConnectionParameters()); + } else if (source instanceof CoverageStoreInfo) { + CoverageStoreInfo store = (CoverageStoreInfo) source; + checkAccess(store.getURL(), store); + } else if (source instanceof HTTPStoreInfo) { + HTTPStoreInfo store = (HTTPStoreInfo) source; + checkAccess(store.getCapabilitiesURL(), store); + } else { + // the above should cover all the store types, but let's make sure we're not missing + // possible future extensions, as this is security + throw new CatalogException("Unsupported store type: " + source.getClass()); + } + } + + @Override + @SuppressWarnings("unchecked") + public void handleModifyEvent(CatalogModifyEvent event) throws CatalogException { + CatalogInfo source = event.getSource(); + if (!(source instanceof StoreInfo)) return; + + if (source instanceof DataStoreInfo) { + Object params = getNewPropertyValue(event, "connectionParameters"); + if (params instanceof Map) { + checkDataStoreParameters( + (DataStoreInfo) source, (Map) params); + } + } else if (source instanceof CoverageStoreInfo) { + CoverageStoreInfo store = (CoverageStoreInfo) source; + Object url = getNewPropertyValue(event, "uRL"); + if (url instanceof String) { + checkAccess((String) url, store); + } + if (url instanceof URL) { + checkAccess(url.toString(), store); + } + } else if (source instanceof HTTPStoreInfo) { + HTTPStoreInfo store = (HTTPStoreInfo) source; + String capabilitiesURL = (String) getNewPropertyValue(event, "capabilitiesURL"); + if (capabilitiesURL != null) { + checkAccess(capabilitiesURL, store); + } + } else { + throw new CatalogException("Unsupported store type: " + source.getClass()); + } + } + + private void checkDataStoreParameters( + DataStoreInfo store, Map connectionParameters) { + try { + GeoServerResourceLoader loader = resourcePool.getCatalog().getResourceLoader(); + // expand environment variables and data dir local references + Map params = ResourcePool.getParams(connectionParameters, loader); + DataAccessFactory factory = resourcePool.getDataStoreFactory(store); + + if (factory != null) { + for (Param param : factory.getParametersInfo()) { + if (File.class.isAssignableFrom(param.getType())) { + File value = (File) param.lookUp(params); + if (value == null) continue; + checkAccess(value); + } else if (URL.class.isAssignableFrom(param.getType())) { + URL value = (URL) param.lookUp(params); + if (value == null) continue; + if ("file".equals(value.getProtocol()) || value.getProtocol() == null) + checkAccess(URLs.urlToFile(value)); + } + } + } + } catch (SandboxException e) { + throw e; + } catch (Exception e) { + throw new CatalogException("Error checking data store parameters", e); + } + } + + private Object getNewPropertyValue(CatalogModifyEvent event, String propertyName) { + List propertyNames = event.getPropertyNames(); + for (int i = 0; i < propertyNames.size(); i++) { + if (propertyName.equals(propertyNames.get(i))) { + return event.getNewValues().get(i); + } + } + return null; + } + + private void checkAccess(File value) { + if (!fileAccessManager.checkAccess(value)) { + throw new SandboxException("Access to " + value + " denied by file sandboxing", value); + } + } + + private void checkAccess(String url, CoverageStoreInfo storeInfo) { + Object converted = ResourcePool.getCoverageStoreSource(url, null, storeInfo, new Hints()); + if (converted instanceof File) { + checkAccess((File) converted); + } else if (converted instanceof URL) { + URL u = (URL) converted; + if ("file".equals(u.getProtocol()) || u.getProtocol() == null) { + checkAccess(URLs.urlToFile(u)); + } + } else { + // may be a COG, e.g., s3://... or http://... + try { + // see if there is a scheme, if not assume it's a file reference and test + URI uri = new URI(url); + if (StringUtils.isEmpty(uri.getScheme()) || "file".equals(uri.getScheme())) { + checkAccess(new File(url)); + } else { + LOGGER.log( + Level.FINE, + "Not a file URI in coverage store, not validating it against the sandbox: {0}", + uri); + } + } catch (URISyntaxException e) { + // not a valid URI, but it may still be a Windows path + try { + Path path = Paths.get(url); + checkAccess(path.toFile()); + } catch (InvalidPathException ex) { + LOGGER.log( + Level.FINEST, + "Not a valid URI/Path in coverage store, not validating it", + ex); + } + } + } + } + + private void checkAccess(String urlSpec, HTTPStoreInfo storeInfo) { + try { + URL url = new URL(urlSpec); + if ("file".equals(url.getProtocol())) { + checkAccess(URLs.urlToFile(url)); + } + } catch (MalformedURLException e) { + LOGGER.log(Level.FINE, "Not a valid URL in HTTP store, not validating it", e); + } + } + + /** Sandbox exception, thrown when a user tries to access a file outside the sandbox */ + public static class SandboxException extends CatalogException { + private final File file; + + public SandboxException(String message, File file) { + super(message); + this.file = file; + } + + public File getFile() { + return file; + } + } +} diff --git a/src/main/src/test/java/org/geoserver/security/impl/AbstractFileAccessTest.java b/src/main/src/test/java/org/geoserver/security/impl/AbstractFileAccessTest.java new file mode 100644 index 00000000000..84a893acf27 --- /dev/null +++ b/src/main/src/test/java/org/geoserver/security/impl/AbstractFileAccessTest.java @@ -0,0 +1,144 @@ +/* (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.impl; + +import static org.geoserver.security.impl.DefaultFileAccessManager.GEOSERVER_DATA_SANDBOX; +import static org.junit.Assert.assertTrue; + +import java.io.File; +import java.io.IOException; +import java.io.OutputStream; +import java.util.Properties; +import org.geoserver.catalog.Catalog; +import org.geoserver.catalog.NamespaceInfo; +import org.geoserver.catalog.WorkspaceInfo; +import org.geoserver.config.GeoServerDataDirectory; +import org.geoserver.data.test.MockData; +import org.geoserver.data.test.SystemTestData; +import org.geoserver.platform.GeoServerExtensions; +import org.geoserver.platform.resource.Resource; +import org.geoserver.test.GeoServerSystemTestSupport; +import org.junit.Before; + +public class AbstractFileAccessTest extends GeoServerSystemTestSupport { + private static final String MISSING = "missing"; + protected static final String ROLE_CITE = "role_" + MockData.CITE_PREFIX; + protected static final String ROLE_CGF = "role_" + MockData.CGF_PREFIX; + protected static final String ROLE_CDF = "role_" + MockData.CDF_PREFIX; + protected static final String ROLE_MISSING = "role_" + MISSING; + protected File sandbox; + protected File citeFolder; + protected File cgfFolder; + protected File cdfFolder; + protected File missingFolder; + protected DefaultFileAccessManager fileAccessManager; + + @Before + public void lookupFileAccessManager() { + fileAccessManager = + GeoServerExtensions.bean(DefaultFileAccessManager.class, applicationContext); + } + + @Before + public void cleanupRestrictions() throws Exception { + // clean up the security restrictions + GeoServerDataDirectory dd = getDataDirectory(); + Resource layerSecurity = dd.get("security/layers.properties"); + Properties properties = new Properties(); + properties.put("*.*.r", "*"); + properties.put("*.*.w", "*"); + try (OutputStream os = layerSecurity.out()) { + properties.store(os, "everyone can read and write"); + } + + // clear the system sandbox + System.clearProperty(GEOSERVER_DATA_SANDBOX); + + // force reloading definitions + DefaultFileAccessManager fam = + GeoServerExtensions.bean(DefaultFileAccessManager.class, applicationContext); + fam.reload(); + } + + @Before + public void setupDirectories() throws IOException { + sandbox = new File("./target/sandbox").getCanonicalFile(); + citeFolder = new File(sandbox, MockData.CITE_PREFIX); + cgfFolder = new File(sandbox, MockData.CGF_PREFIX); + cdfFolder = new File(sandbox, MockData.CDF_PREFIX); + missingFolder = new File(sandbox, MISSING); + if (!citeFolder.exists()) assertTrue(citeFolder.mkdirs()); + if (!cgfFolder.exists()) assertTrue(cgfFolder.mkdirs()); + if (!cdfFolder.exists()) assertTrue(cdfFolder.mkdirs()); + } + + @Before + @Override + public void logout() { + super.logout(); + } + + @Override + protected void onSetUp(SystemTestData testData) throws Exception { + super.onSetUp(testData); + + // configure a workspace for the "missing" directory (that's not going to be created) + Catalog catalog = getCatalog(); + WorkspaceInfo missingWs = catalog.getFactory().createWorkspace(); + missingWs.setName(MISSING); + catalog.add(missingWs); + NamespaceInfo missingNs = catalog.getFactory().createNamespace(); + missingNs.setPrefix(MISSING); + missingNs.setURI("http://www.geoserver.org/" + MISSING); + catalog.add(missingNs); + } + + protected void configureCiteCgfMissingAccess() throws IOException { + // setup a workspace sandbox + Resource layerSecurity = getDataDirectory().get("security/layers.properties"); + Properties properties = new Properties(); + properties.put("filesystemSandbox", sandbox.getAbsolutePath()); + properties.put("cite.*.a", ROLE_CGF); + properties.put("cgf.*.a", ROLE_CGF); + properties.put("missing.*.a", ROLE_MISSING); + try (OutputStream os = layerSecurity.out()) { + properties.store(os, "sandbox"); + os.flush(); + } + + // force reloading definitions + fileAccessManager.reload(); + } + + protected void configureCiteAccess() throws IOException { + // setup a workspace sandbox + Resource layerSecurity = getDataDirectory().get("security/layers.properties"); + Properties properties = new Properties(); + properties.put("filesystemSandbox", sandbox.getAbsolutePath()); + properties.put("cite.*.a", ROLE_CITE); + try (OutputStream os = layerSecurity.out()) { + properties.store(os, "sandbox"); + os.flush(); + } + + // force reloading definitions + fileAccessManager.reload(); + } + + /** Login as a user with ROLE_CITE and ROLE_CGF */ + protected void loginCiteCgfMissing() { + login("cite", "pwd", ROLE_CITE, ROLE_CGF, ROLE_MISSING); + } + + /** Login as a full administrator */ + protected void loginAdmin() { + login("admin", "geoserver", GeoServerRole.ADMIN_ROLE.getAuthority()); + } + + /** Login with ROLE_CITE */ + protected void loginCite() { + login("cite", "pwd", ROLE_CITE); + } +} diff --git a/src/main/src/test/java/org/geoserver/security/impl/AbstractSandboxEnforcerTest.java b/src/main/src/test/java/org/geoserver/security/impl/AbstractSandboxEnforcerTest.java new file mode 100644 index 00000000000..9150467c833 --- /dev/null +++ b/src/main/src/test/java/org/geoserver/security/impl/AbstractSandboxEnforcerTest.java @@ -0,0 +1,144 @@ +/* (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.impl; + +import static org.geoserver.security.impl.DefaultFileAccessManager.GEOSERVER_DATA_SANDBOX; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.allOf; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.endsWith; +import static org.hamcrest.Matchers.startsWith; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertThrows; + +import java.io.File; +import java.io.IOException; +import org.geoserver.catalog.Catalog; +import org.geoserver.catalog.StoreInfo; +import org.geoserver.data.test.SystemTestData; +import org.geoserver.platform.GeoServerExtensions; +import org.geoserver.platform.resource.Resource; +import org.geoserver.security.impl.FileSandboxEnforcer.SandboxException; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +public abstract class AbstractSandboxEnforcerTest extends AbstractFileAccessTest { + + protected static final String ADMIN_STORE = "lakesAdmin"; + protected static final String CITE_STORE = "lakesCite"; + protected static final String CGF_STORE = "lakesCgf"; + protected static final String CDF_STORE = "lakesCdf"; + + @Override + protected void onSetUp(SystemTestData testData) throws Exception { + super.onSetUp(testData); + + // force creation of the FileSanboxEnforcer (beans are lazy loaded in tests, and this + // one registers itself on the catalog on creation) + GeoServerExtensions.bean(FileSandboxEnforcer.class, applicationContext); + } + + @After + public void clearFileAccessManagerConfiguration() { + System.clearProperty(GEOSERVER_DATA_SANDBOX); + Resource layerSecurity = getDataDirectory().get("security/layers.properties"); + layerSecurity.delete(); + fileAccessManager.reload(); + } + + @Before + public void clearStores() throws Exception { + Catalog catalog = getCatalog(); + for (StoreInfo ds : catalog.getStores(StoreInfo.class)) { + String name = ds.getName(); + if (ADMIN_STORE.equals(name) || CITE_STORE.equals(name)) { + catalog.remove(ds); + } + } + } + + @Test + public void testNoRestrictions() throws Exception { + // a real test directory + File testDirectory = new File("./target/test").getCanonicalFile(); + + // now, an administrator should be able to create a store in the test directory + loginAdmin(); + addStore(ADMIN_STORE, testDirectory); + + // a normal user should be able to do the same (from the test we have no admin restrictions) + loginCite(); + addStore(CITE_STORE, testDirectory); + } + + @Test + public void testSystemSandbox() throws Exception { + // setup a system sandbox + File systemSandbox = new File("./target/systemSandbox").getCanonicalFile(); + systemSandbox.mkdirs(); + System.setProperty(GEOSERVER_DATA_SANDBOX, systemSandbox.getAbsolutePath()); + fileAccessManager.reload(); + + // a real test directory + File testDirectory = new File("./target/test").getCanonicalFile(); + + // an administrator should not be able to create a store in the test directory any longer + loginAdmin(); + Catalog catalog = getCatalog(); + SandboxException se = + assertThrows(SandboxException.class, () -> addStore(ADMIN_STORE, testDirectory)); + assertThat( + se.getMessage(), + allOf( + startsWith("Access to "), + containsString(testDirectory.getAbsolutePath()), + endsWith(" denied by file sandboxing"))); + + // check the store really has not been created + assertNull(catalog.getDataStoreByName(ADMIN_STORE)); + } + + @Test + public void testWorkspaceAdminSandbox() throws Exception { + configureCiteAccess(); + + // force reloading definitions + fileAccessManager.reload(); + + // add as a workspace admin + loginCite(); + addStore(CITE_STORE, citeFolder); + assertThrows(SandboxException.class, () -> addStore(CGF_STORE, cgfFolder)); + assertThrows(SandboxException.class, () -> addStore(CDF_STORE, cdfFolder)); + + // now try to escape the sandbox + assertThrows(SandboxException.class, () -> modifyStore(CITE_STORE, cgfFolder)); + + // check the above save did not work + StoreInfo citeStore = getCatalog().getStoreByName(CITE_STORE, StoreInfo.class); + testLocation(citeStore, citeFolder); + } + + @Test + public void testMultipleWorkspaceAdminSandbox() throws Exception { + configureCiteCgfMissingAccess(); + + // add as a workspace admin + loginCiteCgfMissing(); + addStore(CITE_STORE, citeFolder); + addStore(CGF_STORE, cgfFolder); + assertThrows(SandboxException.class, () -> addStore(CDF_STORE, cdfFolder)); + } + + /** Checks that the location in the store is the expected one */ + protected abstract void testLocation(StoreInfo store, File location) throws Exception; + + /** Adds a store of the desired name and location */ + protected abstract void addStore(String storeName, File location) throws IOException; + + /** Modifies the store's location to the desired one */ + protected abstract void modifyStore(String storeName, File location); +} diff --git a/src/main/src/test/java/org/geoserver/security/impl/CoverageStoreSandboxEnforcerTest.java b/src/main/src/test/java/org/geoserver/security/impl/CoverageStoreSandboxEnforcerTest.java new file mode 100644 index 00000000000..59d7e77cf74 --- /dev/null +++ b/src/main/src/test/java/org/geoserver/security/impl/CoverageStoreSandboxEnforcerTest.java @@ -0,0 +1,40 @@ +/* (c) 2024 Open Source Geospatial Foundation - all rights reserved + * This cod e is licensed under the GPL 2.0 license, available at the root + * application directory. + */ +package org.geoserver.security.impl; + +import static org.junit.Assert.assertEquals; + +import java.io.File; +import org.geoserver.catalog.Catalog; +import org.geoserver.catalog.CatalogBuilder; +import org.geoserver.catalog.CoverageStoreInfo; +import org.geoserver.catalog.StoreInfo; + +public class CoverageStoreSandboxEnforcerTest extends AbstractSandboxEnforcerTest { + + @Override + protected void testLocation(StoreInfo store, File location) throws Exception { + CoverageStoreInfo coverageStore = (CoverageStoreInfo) store; + assertEquals(location.getAbsolutePath(), coverageStore.getURL()); + } + + @Override + protected void addStore(String storeName, File location) { + Catalog catalog = getCatalog(); + CatalogBuilder builder = new CatalogBuilder(catalog); + CoverageStoreInfo store = builder.buildCoverageStore(storeName); + store.setWorkspace(catalog.getDefaultWorkspace()); + store.setURL(location.getPath()); + catalog.add(store); + } + + @Override + protected void modifyStore(String storeName, File location) { + Catalog catalog = getCatalog(); + CoverageStoreInfo store = catalog.getCoverageStoreByName(storeName); + store.setURL(location.getPath()); + catalog.save(store); + } +} diff --git a/src/main/src/test/java/org/geoserver/security/impl/DataStoreSandboxEnforcerTest.java b/src/main/src/test/java/org/geoserver/security/impl/DataStoreSandboxEnforcerTest.java new file mode 100644 index 00000000000..71dd56beb57 --- /dev/null +++ b/src/main/src/test/java/org/geoserver/security/impl/DataStoreSandboxEnforcerTest.java @@ -0,0 +1,41 @@ +/* (c) 2024 Open Source Geospatial Foundation - all rights reserved + * This cod e is licensed under the GPL 2.0 license, available at the root + * application directory. + */ +package org.geoserver.security.impl; + +import static org.junit.Assert.assertEquals; + +import java.io.File; +import org.geoserver.catalog.Catalog; +import org.geoserver.catalog.CatalogBuilder; +import org.geoserver.catalog.DataStoreInfo; +import org.geoserver.catalog.StoreInfo; +import org.geotools.data.property.PropertyDataStoreFactory; + +public class DataStoreSandboxEnforcerTest extends AbstractSandboxEnforcerTest { + + @Override + protected void addStore(String storeName, File location) { + Catalog catalog = getCatalog(); + CatalogBuilder builder = new CatalogBuilder(catalog); + DataStoreInfo store = builder.buildDataStore(storeName); + store.setWorkspace(catalog.getDefaultWorkspace()); + store.getConnectionParameters().put("directory", location); + store.setType(new PropertyDataStoreFactory().getDisplayName()); + catalog.add(store); + } + + @Override + protected void modifyStore(String storeName, File location) { + Catalog catalog = getCatalog(); + DataStoreInfo store = catalog.getDataStoreByName(storeName); + store.getConnectionParameters().put("directory", location); + catalog.save(store); + } + + @Override + protected void testLocation(StoreInfo citeStore, File location) { + assertEquals(location, citeStore.getConnectionParameters().get("directory")); + } +} diff --git a/src/main/src/test/java/org/geoserver/security/impl/DefaultFileAccessManagerTest.java b/src/main/src/test/java/org/geoserver/security/impl/DefaultFileAccessManagerTest.java new file mode 100644 index 00000000000..25f4cab9b36 --- /dev/null +++ b/src/main/src/test/java/org/geoserver/security/impl/DefaultFileAccessManagerTest.java @@ -0,0 +1,84 @@ +/* (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.impl; + +import static org.geoserver.security.impl.DefaultFileAccessManager.GEOSERVER_DATA_SANDBOX; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; + +import java.io.File; +import java.util.List; +import org.hamcrest.Matchers; +import org.junit.Test; + +public class DefaultFileAccessManagerTest extends AbstractFileAccessTest { + + @Test + public void testNoRestrictions() throws Exception { + // a real test directory + File testDirectory = new File("./target/test").getCanonicalFile(); + + // no restrictions for admin + loginAdmin(); + assertNull(fileAccessManager.getAvailableRoots()); + assertTrue(fileAccessManager.checkAccess(testDirectory)); + + // but also none for a workspace admin + login("cite", "pwd", ROLE_CITE); + assertNull(fileAccessManager.getAvailableRoots()); + assertTrue(fileAccessManager.checkAccess(testDirectory)); + } + + @Test + public void testSystemSandbox() throws Exception { + // setup a system sandbox + File systemSandbox = new File("./target/systemSandbox").getCanonicalFile(); + System.setProperty(GEOSERVER_DATA_SANDBOX, systemSandbox.getAbsolutePath()); + fileAccessManager.reload(); + + // a real test directory outside of the sandbox + File testDirectory = new File("./target/test").getCanonicalFile(); + + // the admin is sandboxed + loginAdmin(); + assertEquals(fileAccessManager.getAvailableRoots(), List.of(systemSandbox)); + assertFalse(fileAccessManager.checkAccess(testDirectory)); + assertTrue( + fileAccessManager.checkAccess( + new File("./target/systemSandbox/test/a/b/c").getCanonicalFile())); + // there is no escaping it + assertFalse( + fileAccessManager.checkAccess( + new File("./target/systemSandbox/../a/b/c").getCanonicalFile())); + } + + @Test + public void testWorkspaceAdminSandbox() throws Exception { + configureCiteAccess(); + + login("cite", "pwd", ROLE_CITE); + assertEquals(List.of(citeFolder), fileAccessManager.getAvailableRoots()); + assertTrue(fileAccessManager.checkAccess(citeFolder)); + assertFalse(fileAccessManager.checkAccess(cgfFolder)); + assertFalse(fileAccessManager.checkAccess(cdfFolder)); + } + + @Test + public void testMultipleWorkspaceAdminSandbox() throws Exception { + configureCiteCgfMissingAccess(); + + loginCiteCgfMissing(); + // "missing" is not there as it does not exist (would confuse the file browser if + // it was returned as a root, but wasn't actually there on the file system) + assertThat(fileAccessManager.getAvailableRoots(), Matchers.hasItems(citeFolder, cgfFolder)); + assertTrue(fileAccessManager.checkAccess(citeFolder)); + assertTrue(fileAccessManager.checkAccess(cgfFolder)); + assertTrue(fileAccessManager.checkAccess(missingFolder)); // formally allowed + assertFalse(fileAccessManager.checkAccess(cdfFolder)); + } +} diff --git a/src/main/src/test/java/org/geoserver/security/impl/HTTPStoreSandboxEnforcerTest.java b/src/main/src/test/java/org/geoserver/security/impl/HTTPStoreSandboxEnforcerTest.java new file mode 100644 index 00000000000..3ab5363571e --- /dev/null +++ b/src/main/src/test/java/org/geoserver/security/impl/HTTPStoreSandboxEnforcerTest.java @@ -0,0 +1,44 @@ +/* (c) 2024 Open Source Geospatial Foundation - all rights reserved + * This cod e is licensed under the GPL 2.0 license, available at the root + * application directory. + */ +package org.geoserver.security.impl; + +import static org.junit.Assert.assertEquals; + +import java.io.File; +import java.io.IOException; +import java.net.URL; +import org.geoserver.catalog.Catalog; +import org.geoserver.catalog.CatalogBuilder; +import org.geoserver.catalog.HTTPStoreInfo; +import org.geoserver.catalog.StoreInfo; +import org.geoserver.catalog.WMSStoreInfo; +import org.geotools.util.URLs; + +public class HTTPStoreSandboxEnforcerTest extends AbstractSandboxEnforcerTest { + + @Override + protected void testLocation(StoreInfo store, File location) throws Exception { + HTTPStoreInfo http = (HTTPStoreInfo) store; + assertEquals(location, URLs.urlToFile(new URL(http.getCapabilitiesURL()))); + } + + @Override + protected void addStore(String storeName, File location) throws IOException { + Catalog catalog = getCatalog(); + CatalogBuilder builder = new CatalogBuilder(catalog); + WMSStoreInfo store = builder.buildWMSStore(storeName); + store.setWorkspace(catalog.getDefaultWorkspace()); + store.setCapabilitiesURL(URLs.fileToUrl(location).toExternalForm()); + catalog.add(store); + } + + @Override + protected void modifyStore(String storeName, File location) { + Catalog catalog = getCatalog(); + HTTPStoreInfo store = catalog.getWMSStoreByName(storeName); + store.setCapabilitiesURL(URLs.fileToUrl(location).toExternalForm()); + catalog.save(store); + } +} diff --git a/src/main/src/test/java/org/geoserver/test/GeoServerSystemTestSupport.java b/src/main/src/test/java/org/geoserver/test/GeoServerSystemTestSupport.java index d2ae0f18310..6bd6ec561eb 100644 --- a/src/main/src/test/java/org/geoserver/test/GeoServerSystemTestSupport.java +++ b/src/main/src/test/java/org/geoserver/test/GeoServerSystemTestSupport.java @@ -143,6 +143,7 @@ import org.springframework.mock.web.MockServletConfig; import org.springframework.mock.web.MockServletContext; import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; +import org.springframework.security.core.Authentication; import org.springframework.security.core.GrantedAuthority; import org.springframework.security.core.authority.SimpleGrantedAuthority; import org.springframework.security.core.context.SecurityContextHolder; @@ -837,15 +838,17 @@ protected void setRequestAuth(String username, String password) { * @param password The password. * @param roles Roles to assign. */ - protected void login(String username, String password, String... roles) { + protected Authentication login(String username, String password, String... roles) { SecurityContextHolder.setContext(new SecurityContextImpl()); List l = new ArrayList<>(); for (String role : roles) { l.add(new SimpleGrantedAuthority(role)); } - SecurityContextHolder.getContext() - .setAuthentication(new UsernamePasswordAuthenticationToken(username, password, l)); + Authentication authentication = + new UsernamePasswordAuthenticationToken(username, password, l); + SecurityContextHolder.getContext().setAuthentication(authentication); + return authentication; } protected void addUser( diff --git a/src/rest/src/main/java/org/geoserver/rest/util/RESTUploadExternalPathMapper.java b/src/rest/src/main/java/org/geoserver/rest/util/RESTUploadExternalPathMapper.java index 5d40ec6cd8c..15ed2e20e99 100644 --- a/src/rest/src/main/java/org/geoserver/rest/util/RESTUploadExternalPathMapper.java +++ b/src/rest/src/main/java/org/geoserver/rest/util/RESTUploadExternalPathMapper.java @@ -12,6 +12,7 @@ import org.geoserver.catalog.MetadataMap; import org.geoserver.config.SettingsInfo; import org.geoserver.platform.ExtensionPriority; +import org.geoserver.security.FileAccessManager; /** * Default implementation of the {@link RESTUploadPathMapper} interface. This implementation simply @@ -55,6 +56,14 @@ public void mapStorePath( rootDir.append(File.separator); rootDir.append(store); } + + // Check if the user has access to the external root directory (should never happen, + // but since it's security, better take a belt and suspenders approach) + FileAccessManager fam = FileAccessManager.lookupFileAccessManager(); + if (!fam.checkAccess(new File(rootDir.toString()))) { + throw new IOException( + "Access to the external root directory is not allowed: " + rootDir); + } } @Override diff --git a/src/rest/src/main/java/org/geoserver/rest/util/RESTUtils.java b/src/rest/src/main/java/org/geoserver/rest/util/RESTUtils.java index b3ad9a21352..adc5b958942 100644 --- a/src/rest/src/main/java/org/geoserver/rest/util/RESTUtils.java +++ b/src/rest/src/main/java/org/geoserver/rest/util/RESTUtils.java @@ -36,6 +36,7 @@ import org.geoserver.platform.resource.Resource; import org.geoserver.platform.resource.Resources; import org.geoserver.rest.RestException; +import org.geoserver.security.FileAccessManager; import org.geotools.data.ows.URLCheckerException; import org.geotools.util.URLs; import org.geotools.util.logging.Logging; @@ -385,7 +386,14 @@ public static String extractMapItem(MetadataMap map, String key) { } public static String getRootDirectory(String workspaceName, String storeName, Catalog catalog) { - String rootDir = getItem(workspaceName, storeName, catalog, ROOT_KEY); + String rootDir; + FileAccessManager fam = FileAccessManager.lookupFileAccessManager(); + if (fam.getSandbox() != null) { + File root = fam.getSandbox(); + rootDir = root.getAbsolutePath(); + } else { + rootDir = getItem(workspaceName, storeName, catalog, ROOT_KEY); + } if (rootDir != null) { // Check if it already exists File rootFile = new File(rootDir); diff --git a/src/rest/src/main/resources/applicationContext.xml b/src/rest/src/main/resources/applicationContext.xml index b211f0bc4fd..8cec067c069 100644 --- a/src/rest/src/main/resources/applicationContext.xml +++ b/src/rest/src/main/resources/applicationContext.xml @@ -12,4 +12,8 @@ + + + + \ No newline at end of file diff --git a/src/restconfig/src/test/java/org/geoserver/rest/catalog/CoverageStoreFileUploadTest.java b/src/restconfig/src/test/java/org/geoserver/rest/catalog/CoverageStoreFileUploadTest.java index 88fdb8afb31..2e3a75cde3e 100644 --- a/src/restconfig/src/test/java/org/geoserver/rest/catalog/CoverageStoreFileUploadTest.java +++ b/src/restconfig/src/test/java/org/geoserver/rest/catalog/CoverageStoreFileUploadTest.java @@ -6,6 +6,7 @@ package org.geoserver.rest.catalog; import static org.geoserver.rest.RestBaseController.ROOT_PATH; +import static org.geoserver.security.impl.DefaultFileAccessManager.GEOSERVER_DATA_SANDBOX; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.startsWith; import static org.junit.Assert.assertEquals; @@ -43,6 +44,8 @@ import org.geoserver.rest.RestBaseController; import org.geoserver.rest.util.IOUtils; import org.geoserver.rest.util.RESTUtils; +import org.geoserver.security.FileAccessManager; +import org.geoserver.security.impl.DefaultFileAccessManager; import org.geotools.api.coverage.grid.GridCoverageReader; import org.geotools.api.data.DataStore; import org.geotools.api.data.Query; @@ -54,6 +57,7 @@ import org.geotools.geometry.jts.ReferencedEnvelope; import org.geotools.util.URLs; import org.geotools.util.factory.GeoTools; +import org.hamcrest.Matchers; import org.junit.Before; import org.junit.ClassRule; import org.junit.Ignore; @@ -97,21 +101,7 @@ public void cleanup() throws IOException { @Test public void testWorldImageUploadZipped() throws Exception { - URL zip = getClass().getResource("test-data/usa.zip"); - byte[] bytes = FileUtils.readFileToByteArray(URLs.urlToFile(zip)); - - MockHttpServletResponse response = - putAsServletResponse( - RestBaseController.ROOT_PATH - + "/workspaces/sf/coveragestores/usa/file.worldimage", - bytes, - "application/zip"); - assertEquals(201, response.getStatus()); - assertEquals(MediaType.APPLICATION_XML_VALUE, response.getContentType()); - - String content = response.getContentAsString(); - Document d = dom(new ByteArrayInputStream(content.getBytes())); - assertEquals("coverageStore", d.getDocumentElement().getNodeName()); + uploadUSAWorldImage(); CoverageStoreInfo cs = getCatalog().getCoverageStoreByName("sf", "usa"); assertNotNull(cs); @@ -856,4 +846,47 @@ public void testWorldImageUploadExternalZipValid() throws Exception { Resource.Type.UNDEFINED, getResourceLoader().get("data/sf/usa/test3.zip").getType()); } + + @Test + public void testFilesystemSandbox() throws Exception { + // set up a system sandbox + File systemSandbox = new File("./target/systemSandbox").getCanonicalFile(); + System.setProperty(GEOSERVER_DATA_SANDBOX, systemSandbox.getAbsolutePath()); + DefaultFileAccessManager fam = + (DefaultFileAccessManager) FileAccessManager.lookupFileAccessManager(); + fam.reload(); + + try { + uploadUSAWorldImage(); + + // check the coverage has been uploaded inside the system sandbox + CoverageStoreInfo cs = getCatalog().getCoverageStoreByName("sf", "usa"); + assertNotNull(cs); + // compute a OS independent test string (replacement is for Windows) + String expected = + new File(systemSandbox, "/sf/usa/usa.png").getAbsolutePath().replace("\\", "/"); + assertThat(cs.getURL(), Matchers.containsString(expected)); + } finally { + System.clearProperty(GEOSERVER_DATA_SANDBOX); + fam.reload(); + } + } + + private void uploadUSAWorldImage() throws Exception { + URL zip = getClass().getResource("test-data/usa.zip"); + byte[] bytes = FileUtils.readFileToByteArray(URLs.urlToFile(zip)); + + MockHttpServletResponse response = + putAsServletResponse( + RestBaseController.ROOT_PATH + + "/workspaces/sf/coveragestores/usa/file.worldimage", + bytes, + "application/zip"); + assertEquals(201, response.getStatus()); + assertEquals(MediaType.APPLICATION_XML_VALUE, response.getContentType()); + + String content = response.getContentAsString(); + Document d = dom(new ByteArrayInputStream(content.getBytes())); + assertEquals("coverageStore", d.getDocumentElement().getNodeName()); + } } diff --git a/src/restconfig/src/test/java/org/geoserver/rest/catalog/DataStoreFileUploadTest.java b/src/restconfig/src/test/java/org/geoserver/rest/catalog/DataStoreFileUploadTest.java index fbedbc07958..88c67e136b7 100644 --- a/src/restconfig/src/test/java/org/geoserver/rest/catalog/DataStoreFileUploadTest.java +++ b/src/restconfig/src/test/java/org/geoserver/rest/catalog/DataStoreFileUploadTest.java @@ -5,7 +5,9 @@ package org.geoserver.rest.catalog; import static org.geoserver.rest.RestBaseController.ROOT_PATH; +import static org.geoserver.security.impl.DefaultFileAccessManager.GEOSERVER_DATA_SANDBOX; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.startsWith; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; @@ -38,6 +40,8 @@ import org.geoserver.filters.LoggingFilter; import org.geoserver.platform.GeoServerResourceLoader; import org.geoserver.platform.resource.Resource; +import org.geoserver.security.FileAccessManager; +import org.geoserver.security.impl.DefaultFileAccessManager; import org.geotools.util.URLs; import org.h2.tools.DeleteDbFiles; import org.junit.After; @@ -221,18 +225,7 @@ public void testRenamingFeatureTypeAlreadyInCatalog() throws Exception { @Test public void testShapefileUploadZip() throws Exception { - Catalog cat = getCatalog(); - assertNull(cat.getDataStoreByName("gs", "san_andres_y_providencia")); - - put( - ROOT_PATH + "/workspaces/gs/datastores/san_andres_y_providencia/file.shp", - shpSanAndresShapefilesZipAsBytes(), - "application/zip"); - - DataStoreInfo ds = cat.getDataStoreByName("gs", "san_andres_y_providencia"); - assertNotNull(ds); - - assertEquals(1, cat.getFeatureTypesByDataStore(ds).size()); + uploadSanAndreas(); } @Test @@ -503,4 +496,45 @@ public void testShapefileUploadExternalZipValid() throws Exception { Resource.Type.UNDEFINED, getResourceLoader().get("data/gs/san_andres_y_providencia/test3.zip").getType()); } + + @Test + public void testFilesystemSandbox() throws Exception { + // set up a system sandbox + File systemSandbox = new File("./target/systemSandbox").getCanonicalFile(); + System.setProperty(GEOSERVER_DATA_SANDBOX, systemSandbox.getAbsolutePath()); + DefaultFileAccessManager fam = + (DefaultFileAccessManager) FileAccessManager.lookupFileAccessManager(); + fam.reload(); + + try { + DataStoreInfo ds = uploadSanAndreas(); + + // the files have been stored in the system sandbox (replace is for Windows) + String expected = + new File(systemSandbox, "gs/san_andres_y_providencia") + .getAbsolutePath() + .replace("\\", "/"); + assertThat( + String.valueOf(ds.getConnectionParameters().get("url")), + containsString(expected)); + } finally { + System.clearProperty(GEOSERVER_DATA_SANDBOX); + fam.reload(); + } + } + + private DataStoreInfo uploadSanAndreas() throws Exception { + Catalog cat = getCatalog(); + assertNull(cat.getDataStoreByName("gs", "san_andres_y_providencia")); + + put( + ROOT_PATH + "/workspaces/gs/datastores/san_andres_y_providencia/file.shp", + shpSanAndresShapefilesZipAsBytes(), + "application/zip"); + + DataStoreInfo ds = cat.getDataStoreByName("gs", "san_andres_y_providencia"); + assertNotNull(ds); + assertEquals(1, cat.getFeatureTypesByDataStore(ds).size()); + return ds; + } } diff --git a/src/web/core/src/main/java/org/geoserver/web/data/store/AbstractWMSStorePage.java b/src/web/core/src/main/java/org/geoserver/web/data/store/AbstractWMSStorePage.java index c0c85e937a8..cdfa93f3cea 100644 --- a/src/web/core/src/main/java/org/geoserver/web/data/store/AbstractWMSStorePage.java +++ b/src/web/core/src/main/java/org/geoserver/web/data/store/AbstractWMSStorePage.java @@ -17,6 +17,7 @@ import org.apache.wicket.model.ResourceModel; import org.apache.wicket.validation.validator.RangeValidator; import org.geoserver.catalog.WMSStoreInfo; +import org.geoserver.security.impl.FileSandboxEnforcer; import org.geoserver.web.ComponentAuthorizer; import org.geoserver.web.GeoServerSecuredPage; import org.geoserver.web.data.store.panel.CheckBoxParamPanel; @@ -239,6 +240,13 @@ protected void onSubmit(AjaxRequestTarget target) { WMSStoreInfo info = (WMSStoreInfo) getForm().getModelObject(); try { onSave(info, target); + } catch (FileSandboxEnforcer.SandboxException e) { + // this one is non recoverable, give up and inform the user + form.error( + new ParamResourceModel( + "sandboxError", this, e.getFile().getAbsolutePath()) + .getString()); + target.add(getForm()); } catch (IllegalArgumentException e) { form.error(e.getMessage()); target.add(getForm()); diff --git a/src/web/core/src/main/java/org/geoserver/web/data/store/AbstractWMTSStorePage.java b/src/web/core/src/main/java/org/geoserver/web/data/store/AbstractWMTSStorePage.java index fd48fae018b..7c4316d41e7 100644 --- a/src/web/core/src/main/java/org/geoserver/web/data/store/AbstractWMTSStorePage.java +++ b/src/web/core/src/main/java/org/geoserver/web/data/store/AbstractWMTSStorePage.java @@ -17,6 +17,7 @@ import org.apache.wicket.model.ResourceModel; import org.apache.wicket.validation.validator.RangeValidator; import org.geoserver.catalog.WMTSStoreInfo; +import org.geoserver.security.impl.FileSandboxEnforcer; import org.geoserver.web.ComponentAuthorizer; import org.geoserver.web.GeoServerSecuredPage; import org.geoserver.web.data.store.panel.CheckBoxParamPanel; @@ -235,6 +236,13 @@ protected void onSubmit(AjaxRequestTarget target) { WMTSStoreInfo info = (WMTSStoreInfo) getForm().getModelObject(); try { onSave(info, target); + } catch (FileSandboxEnforcer.SandboxException e) { + // this one is non recoverable, give up and inform the user + error( + new ParamResourceModel( + "sandboxError", this, e.getFile().getAbsolutePath()) + .getString()); + target.add(form); } catch (IllegalArgumentException e) { getForm().error(e.getMessage()); target.add(getForm()); diff --git a/src/web/core/src/main/java/org/geoserver/web/data/store/CoverageStoreEditPage.java b/src/web/core/src/main/java/org/geoserver/web/data/store/CoverageStoreEditPage.java index 376569d9f3e..ff14a7ae897 100644 --- a/src/web/core/src/main/java/org/geoserver/web/data/store/CoverageStoreEditPage.java +++ b/src/web/core/src/main/java/org/geoserver/web/data/store/CoverageStoreEditPage.java @@ -16,6 +16,7 @@ import org.geoserver.catalog.CoverageStoreInfo; import org.geoserver.catalog.NamespaceInfo; import org.geoserver.catalog.ResourcePool; +import org.geoserver.security.impl.FileSandboxEnforcer; import org.geoserver.web.wicket.GeoServerDialog; import org.geoserver.web.wicket.ParamResourceModel; import org.geotools.api.coverage.grid.GridCoverageReader; @@ -130,7 +131,7 @@ protected final void onSave( + " validated. Got a " + reader.getClass().getName() + ". Saving store"); - doSaveStore(info); + if (!doSaveStore(info)) return; if (doReturn) { doReturn(StorePage.class); } @@ -139,7 +140,7 @@ protected final void onSave( } } else { // store's disabled, no need to check for availability - doSaveStore(info); + if (!doSaveStore(info)) return; if (doReturn) { doReturn(StorePage.class); } @@ -191,7 +192,7 @@ public void onClose(AjaxRequestTarget target) { * *

This method may be subclasses to provide custom save functionality. */ - protected void doSaveStore(final CoverageStoreInfo info) { + protected boolean doSaveStore(final CoverageStoreInfo info) { try { Catalog catalog = getCatalog(); @@ -218,9 +219,16 @@ protected void doSaveStore(final CoverageStoreInfo info) { catalog.save(coverage); } LOGGER.finer("Saved store " + info.getName()); + } catch (FileSandboxEnforcer.SandboxException e) { + // this one is non recoverable, give up and inform the user + error( + new ParamResourceModel("sandboxError", this, e.getFile().getAbsolutePath()) + .getString()); + return false; } catch (RuntimeException e) { LOGGER.log(Level.WARNING, "Saving the store for " + info.getURL(), e); throw new IllegalArgumentException("Unable to save the store: " + e.getMessage()); } + return true; } } diff --git a/src/web/core/src/main/java/org/geoserver/web/data/store/CoverageStoreNewPage.java b/src/web/core/src/main/java/org/geoserver/web/data/store/CoverageStoreNewPage.java index dcd2288f842..41119c93f5a 100644 --- a/src/web/core/src/main/java/org/geoserver/web/data/store/CoverageStoreNewPage.java +++ b/src/web/core/src/main/java/org/geoserver/web/data/store/CoverageStoreNewPage.java @@ -11,7 +11,9 @@ import org.geoserver.catalog.Catalog; import org.geoserver.catalog.CoverageStoreInfo; import org.geoserver.catalog.WorkspaceInfo; +import org.geoserver.security.impl.FileSandboxEnforcer; import org.geoserver.web.data.layer.NewLayerPage; +import org.geoserver.web.wicket.ParamResourceModel; import org.geotools.api.coverage.grid.Format; /** @@ -62,6 +64,12 @@ protected void onSave(final CoverageStoreInfo info, AjaxRequestTarget target, bo savedStore = catalog.getResourcePool().clone(info, false); // ... and save catalog.save(savedStore); + } catch (FileSandboxEnforcer.SandboxException e) { + // this one is non recoverable, give up and inform the user + error( + new ParamResourceModel("sandboxError", this, e.getFile().getAbsolutePath()) + .getString()); + return; // do not call onSuccessfulSave } catch (RuntimeException e) { LOGGER.log(Level.INFO, "Adding the store for " + info.getURL(), e); throw new IllegalArgumentException( diff --git a/src/web/core/src/main/java/org/geoserver/web/data/store/DataAccessEditPage.java b/src/web/core/src/main/java/org/geoserver/web/data/store/DataAccessEditPage.java index 25a40fb3b7c..243c47bbc9f 100644 --- a/src/web/core/src/main/java/org/geoserver/web/data/store/DataAccessEditPage.java +++ b/src/web/core/src/main/java/org/geoserver/web/data/store/DataAccessEditPage.java @@ -18,6 +18,7 @@ import org.geoserver.catalog.FeatureTypeInfo; import org.geoserver.catalog.NamespaceInfo; import org.geoserver.catalog.ResourcePool; +import org.geoserver.security.impl.FileSandboxEnforcer; import org.geoserver.web.wicket.GeoServerDialog; import org.geoserver.web.wicket.ParamResourceModel; import org.geotools.api.data.DataAccess; @@ -129,7 +130,8 @@ protected final void onSaveDataStore( + info.getName() + ". Got a " + dataStore.getClass().getName()); - doSaveStore(info); + if (!doSaveStore(info)) return; + if (doReturn) { doReturn(StorePage.class); } @@ -139,7 +141,7 @@ protected final void onSaveDataStore( } } else { // store's disabled, no need to check the connection parameters - doSaveStore(info); + if (!doSaveStore(info)) return; if (doReturn) { doReturn(StorePage.class); } @@ -201,7 +203,7 @@ public void onClose(AjaxRequestTarget target) { * *

This method may be subclasses to provide custom save functionality. */ - protected void doSaveStore(final DataStoreInfo info) { + protected boolean doSaveStore(final DataStoreInfo info) { try { final Catalog catalog = getCatalog(); @@ -227,9 +229,17 @@ protected void doSaveStore(final DataStoreInfo info) { catalog.save(alreadyConfigured); } LOGGER.finer("Saved store " + info.getName()); + } catch (FileSandboxEnforcer.SandboxException e) { + // this one is non recoverable, give up and inform the user + error( + new ParamResourceModel("sandboxError", this, e.getFile().getAbsolutePath()) + .getString()); + return false; } catch (Exception e) { LOGGER.log(Level.WARNING, "Error saving data store to catalog", e); - throw new IllegalArgumentException("Error saving data store:" + e.getMessage()); + throw new IllegalArgumentException("Error saving data store:" + e.getMessage(), e); } + + return true; } } diff --git a/src/web/core/src/main/java/org/geoserver/web/data/store/DataAccessNewPage.java b/src/web/core/src/main/java/org/geoserver/web/data/store/DataAccessNewPage.java index 8eeae0bff58..feef3a6c4e1 100644 --- a/src/web/core/src/main/java/org/geoserver/web/data/store/DataAccessNewPage.java +++ b/src/web/core/src/main/java/org/geoserver/web/data/store/DataAccessNewPage.java @@ -13,7 +13,9 @@ import org.geoserver.catalog.DataStoreInfo; import org.geoserver.catalog.NamespaceInfo; import org.geoserver.catalog.WorkspaceInfo; +import org.geoserver.security.impl.FileSandboxEnforcer; import org.geoserver.web.data.layer.NewLayerPage; +import org.geoserver.web.wicket.ParamResourceModel; import org.geotools.api.data.DataAccess; import org.geotools.api.feature.Feature; import org.geotools.api.feature.type.FeatureType; @@ -46,17 +48,6 @@ public DataAccessNewPage(final String dataStoreFactDisplayName) { throw new IllegalStateException("No default Namespace configured"); } - // Param[] parametersInfo = dsFact.getParametersInfo(); - // for (int i = 0; i < parametersInfo.length; i++) { - // Serializable value; - // final Param param = parametersInfo[i]; - // if (param.sample == null || param.sample instanceof Serializable) { - // value = (Serializable) param.sample; - // } else { - // value = String.valueOf(param.sample); - // } - // } - DataStoreInfo info = getCatalog().getFactory().createDataStore(); info.setWorkspace(defaultWs); info.setEnabled(true); @@ -111,6 +102,12 @@ protected final void onSaveDataStore( savedStore = catalog.getResourcePool().clone(info, false); // ...and save catalog.add(savedStore); + } catch (FileSandboxEnforcer.SandboxException e) { + // this one is non recoverable, give up and inform the user + error( + new ParamResourceModel("sandboxError", this, e.getFile().getAbsolutePath()) + .getString()); + return; // do not exit } catch (Exception e) { LOGGER.log(Level.WARNING, "Error adding data store to catalog", e); String message = e.getMessage(); diff --git a/src/web/core/src/main/java/org/geoserver/web/wicket/browser/FileInput.java b/src/web/core/src/main/java/org/geoserver/web/wicket/browser/FileInput.java index 2f4249b1dbe..c5451ae590e 100644 --- a/src/web/core/src/main/java/org/geoserver/web/wicket/browser/FileInput.java +++ b/src/web/core/src/main/java/org/geoserver/web/wicket/browser/FileInput.java @@ -33,6 +33,7 @@ */ public class FileInput extends Panel { private static final Logger LOGGER = Logging.getLogger(FileInput.class); + private final FileRootsFinder rootsFinder; protected TextField textField; protected GSModalWindow dialog; protected IModel fileFilter; @@ -55,7 +56,7 @@ public FileInput( add(dialog = new GSModalWindow("dialog")); // the text field, with a decorator for validations - FileRootsFinder rootsFinder = new FileRootsFinder(false); + this.rootsFinder = new FileRootsFinder(false); textField = new AutoCompleteTextField<>("paramValue", getFileModel(paramValue)) { @Override @@ -158,4 +159,12 @@ public FormComponent getFormComponent() { public void setFileFilter(IModel fileFilter) { this.fileFilter = fileFilter; } + + /** + * When set to true, will prefix the paths with the file scheme, e.g. + * file:///the/full/path + */ + public void setPrefixPaths(boolean prefixPaths) { + rootsFinder.setPrefixPaths(prefixPaths); + } } diff --git a/src/web/core/src/main/java/org/geoserver/web/wicket/browser/FileRootsFinder.java b/src/web/core/src/main/java/org/geoserver/web/wicket/browser/FileRootsFinder.java index 619f58156f9..033734fb0d6 100644 --- a/src/web/core/src/main/java/org/geoserver/web/wicket/browser/FileRootsFinder.java +++ b/src/web/core/src/main/java/org/geoserver/web/wicket/browser/FileRootsFinder.java @@ -10,11 +10,13 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; +import java.util.List; import java.util.stream.Stream; import org.geoserver.platform.GeoServerExtensions; import org.geoserver.platform.GeoServerResourceLoader; import org.geoserver.platform.resource.Paths; import org.geoserver.platform.resource.Resource; +import org.geoserver.security.FileAccessManager; /** * Support class to locate the file system roots the file chooser uses to locate files, along with @@ -26,7 +28,7 @@ public class FileRootsFinder implements Serializable { * Utility so split and rebuild paths accounting for ResourceStore little own illusion of * working on a *nix file system regardless of the actual file system */ - static class PathSplitter { + class PathSplitter { String separator; boolean dataDirectoryPath; @@ -39,8 +41,7 @@ public PathSplitter(String input, boolean dataDirectoryPath) { this.dataDirectoryPath = dataDirectoryPath; // remove protocol part if needed (we have messy inputs stored that do not always start - // with - // file:// but sometimes with file:/ and sometimes with file: (no / at all) + // with file:// but sometimes with file:/ and sometimes with file: (no / at all) if (input.startsWith("file:")) { if (input.startsWith("file:/")) { if (input.startsWith("file://")) { @@ -72,7 +73,7 @@ public PathSplitter(String input, boolean dataDirectoryPath) { private String buildPath(String name) { // Data dir relative path weirdness, the protocol has to be // file:/ instead of file:// or it won't work. - String prefix = dataDirectoryPath ? "file:" : "file://"; + String prefix = prefixPaths ? (dataDirectoryPath ? "file:" : "file://") : ""; // make data dir relative paths actually relative despite user's input String localBase = base; if (dataDirectoryPath && localBase.startsWith(separator)) { @@ -88,20 +89,34 @@ private String buildPath(String name) { private ArrayList roots; private File dataDirectory; + private boolean prefixPaths = true; public FileRootsFinder(boolean includeDataDir) { this(GeoServerFileChooser.HIDE_FS, includeDataDir); } public FileRootsFinder(boolean hideFileSystem, boolean includeDataDir) { - // build the roots + // set up for file access restrictions + FileAccessManager fam = FileAccessManager.lookupFileAccessManager(); + + // get the roots from the restrictions manager + List famRoots = fam.getAvailableRoots(); + if (famRoots != null) { + this.roots = new ArrayList<>(famRoots); + // if restrictions are in place, we are done, sort and return + Collections.sort(roots); + return; + } + + // if no restrictions are in place, build the roots from the file system roots = new ArrayList<>(); if (!hideFileSystem) { roots.addAll(Arrays.asList(File.listRoots())); } + Collections.sort(roots); - // TODO: find a better way to deal with the data dir + // the data directory is always the first root, if it's not hidden GeoServerResourceLoader loader = getLoader(); dataDirectory = loader.getBaseDirectory(); @@ -115,6 +130,14 @@ public FileRootsFinder(boolean hideFileSystem, boolean includeDataDir) { } } + public boolean isPrefixPaths() { + return prefixPaths; + } + + public void setPrefixPaths(boolean prefixPaths) { + this.prefixPaths = prefixPaths; + } + public ArrayList getRoots() { return roots; } @@ -140,22 +163,31 @@ public File getDataDirectory() { */ @SuppressWarnings("PMD.CloseResource") public Stream getMatches(String input, FileFilter fileFilter) { - // check the data directory (which lives in its own *nix dream, so paths need conversion) - PathSplitter ddSplitter = new PathSplitter(input, true); - GeoServerResourceLoader loader = getLoader(); - Resource resource = loader.get(ddSplitter.base); - File dataDirectoryRoot = loader.get("/").dir(); - Stream result = - resource.list().stream() - .filter(r -> r.name().toLowerCase().contains(ddSplitter.name)) - .filter( - r -> - fileFilter == null - || fileFilter.accept( - new File(dataDirectoryRoot, r.path()))) - .map(r -> ddSplitter.buildPath(r.name())); - - // check all the roots + // null safe, simplify code + FileFilter ff = fileFilter == null ? f -> true : fileFilter; + + FileAccessManager fam = FileAccessManager.lookupFileAccessManager(); + List famRoots = fam.getAvailableRoots(); + + // if there are no sandbox restrictions, start by checking the data directory + Stream result; + if (famRoots == null) { + // check the data directory (which lives in its own *nix dream, so paths need + // conversion) + PathSplitter ddSplitter = new PathSplitter(input, true); + GeoServerResourceLoader loader = getLoader(); + Resource resource = loader.get(ddSplitter.base); + File dataDirectoryRoot = loader.get("/").dir(); + result = + resource.list().stream() + .filter(r -> r.name().toLowerCase().contains(ddSplitter.name)) + .filter(r -> ff.accept(new File(dataDirectoryRoot, r.path()))) + .map(r -> ddSplitter.buildPath(r.name())); + } else { + result = Stream.empty(); + } + + // check all the roots as well PathSplitter fsSplitter = new PathSplitter(input, false); for (File root : getRoots()) { String pathInRoot = fsSplitter.base; @@ -175,16 +207,21 @@ public Stream getMatches(String input, FileFilter fileFilter) { if (names != null) { Stream rootPaths = Arrays.stream(names) - .filter( - name -> - fileFilter == null - || fileFilter.accept( - new File(fsSplitter.base, name))) + .filter(name -> ff.accept(new File(fsSplitter.base, name))) .map(fileName -> fsSplitter.buildPath(fileName)); result = Stream.concat(result, rootPaths); } } + // the above won't work for roots that are full paths (e.g., sandboxing) + // so we need to check the input against the roots themselves + String prefix = prefixPaths ? "file://" : ""; + Stream rootMatches = + getRoots().stream() + .filter(root -> root.getPath().contains(input)) + .map(r -> prefix + r.getPath()); + result = Stream.concat(result, rootMatches); + return result.distinct().sorted(); } } diff --git a/src/web/core/src/main/java/org/geoserver/web/wicket/browser/GeoServerFileChooser.java b/src/web/core/src/main/java/org/geoserver/web/wicket/browser/GeoServerFileChooser.java index a5ff8f0479f..10a89ffa0e3 100644 --- a/src/web/core/src/main/java/org/geoserver/web/wicket/browser/GeoServerFileChooser.java +++ b/src/web/core/src/main/java/org/geoserver/web/wicket/browser/GeoServerFileChooser.java @@ -82,7 +82,6 @@ public GeoServerFileChooser(String id, IModel file, boolean hideFileSystem FileRootsFinder fileRootsFinder = new FileRootsFinder(hideFileSystem, true); ArrayList roots = fileRootsFinder.getRoots(); GeoServerResourceLoader loader = fileRootsFinder.getLoader(); - File dataDirectory = fileRootsFinder.getDataDirectory(); // find under which root the selection should be placed File selection = file.getObject(); @@ -100,7 +99,8 @@ public GeoServerFileChooser(String id, IModel file, boolean hideFileSystem } } - // select the proper root + // select the proper root (the first one is the data directory, unless + // there is file system sandboxing in place, in that case it will be the sandbox) File selectionRoot = null; if (selection != null && selection.exists()) { for (File root : roots) { @@ -111,9 +111,9 @@ public GeoServerFileChooser(String id, IModel file, boolean hideFileSystem } // if the file is not part of the known search paths, give up - // and switch back to the data directory + // and switch back to the first root if (selectionRoot == null) { - selectionRoot = dataDirectory; + selectionRoot = roots.get(0); file = new Model<>(selectionRoot); } else { if (!selection.isDirectory()) { @@ -123,7 +123,8 @@ public GeoServerFileChooser(String id, IModel file, boolean hideFileSystem } } } else { - selectionRoot = dataDirectory; + // the first root is the data directory + selectionRoot = roots.get(0); file = new Model<>(selectionRoot); } this.file = file; diff --git a/src/web/core/src/main/resources/GeoServerApplication.properties b/src/web/core/src/main/resources/GeoServerApplication.properties index 71f8c23ea33..36bf13b294e 100644 --- a/src/web/core/src/main/resources/GeoServerApplication.properties +++ b/src/web/core/src/main/resources/GeoServerApplication.properties @@ -1293,4 +1293,6 @@ PublishedConfigurationPage.attributeNullSource = Attribute source must not be nu PublishedConfigurationPage.cqlUsesInvalidAttribute = The CQL source expression for attribute {0} refers to attributes unavailable in the data source: {1} PublishedConfigurationPage.attributeInvalidConversion = Issue found in attribute {0}, unable to convert from native type, {1}, to target type, {2} PublishedConfigurationPage.attributeInvalidCQL = Invalid CQL for {0} source. {1} -PublishedConfigurationPage.multiAttributeSameName = Found multiple definitions for output attribute {0} \ No newline at end of file +PublishedConfigurationPage.multiAttributeSameName = Found multiple definitions for output attribute {0} + +sandboxError=Access to {0} denied by file sandboxing \ No newline at end of file diff --git a/src/web/core/src/test/java/org/geoserver/web/data/store/CoverageStoreNewPageTest.java b/src/web/core/src/test/java/org/geoserver/web/data/store/CoverageStoreNewPageTest.java index 66fa182878b..da2f58fea88 100644 --- a/src/web/core/src/test/java/org/geoserver/web/data/store/CoverageStoreNewPageTest.java +++ b/src/web/core/src/test/java/org/geoserver/web/data/store/CoverageStoreNewPageTest.java @@ -5,8 +5,11 @@ */ package org.geoserver.web.data.store; +import static org.geoserver.security.impl.DefaultFileAccessManager.GEOSERVER_DATA_SANDBOX; import static org.hamcrest.CoreMatchers.instanceOf; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.allOf; +import static org.hamcrest.Matchers.containsString; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; @@ -14,11 +17,21 @@ import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; +import java.io.File; +import java.io.InputStream; +import java.io.Serializable; +import java.util.List; +import org.apache.commons.io.FileUtils; import org.apache.wicket.Component; +import org.apache.wicket.feedback.FeedbackMessage; import org.apache.wicket.markup.html.form.CheckBox; import org.apache.wicket.util.tester.FormTester; import org.geoserver.catalog.CoverageStoreInfo; +import org.geoserver.data.test.MockData; import org.geoserver.data.test.SystemTestData; +import org.geoserver.platform.GeoServerExtensions; +import org.geoserver.security.impl.DefaultFileAccessManager; +import org.geoserver.security.impl.FileSandboxEnforcer; import org.geoserver.web.GeoServerWicketTestSupport; import org.geoserver.web.data.layer.NewLayerPage; import org.geoserver.web.data.store.panel.FileParamPanel; @@ -42,6 +55,10 @@ public class CoverageStoreNewPageTest extends GeoServerWicketTestSupport { protected void onSetUp(SystemTestData testData) throws Exception { super.onSetUp(testData); testData.setUpDefaultRasterLayers(); + + // force creation of the FileSanboxEnforcer (beans are lazy loaded in tests, and this + // one registers itself on the catalog on creation) + GeoServerExtensions.bean(FileSandboxEnforcer.class, applicationContext); } @Before @@ -52,7 +69,6 @@ public void init() { } private CoverageStoreNewPage startPage() { - login(); final CoverageStoreNewPage page = new CoverageStoreNewPage(formatType); tester.startPage(page); @@ -190,4 +206,66 @@ public void testDisableOnConnFailureCheckbox() { assertNotNull(store); assertTrue(store.isDisableOnConnFailure()); } + + @Test + public void testNewCoverageSandbox() throws Exception { + // setup sandbox on file system and a random directory + File systemSandbox = new File("./target/systemSandbox").getCanonicalFile(); + System.setProperty(GEOSERVER_DATA_SANDBOX, systemSandbox.getAbsolutePath()); + File testDir = new File("./target/test").getCanonicalFile(); + testDir.mkdirs(); + File bmFolder = new File(systemSandbox, "bm"); + bmFolder.mkdirs(); + GeoServerExtensions.bean(DefaultFileAccessManager.class).reload(); + + // copy over tazbmInside in the two directories + String fileName = "tazbm.tiff"; + File tazbmInside = new File(bmFolder, fileName); + File tazbmOutside = new File(testDir, fileName); + try (InputStream is = MockData.class.getResourceAsStream(fileName)) { + FileUtils.copyToFile(is, tazbmInside); + } + try (InputStream is = MockData.class.getResourceAsStream(fileName)) { + FileUtils.copyToFile(is, tazbmOutside); + } + + try { + // try to create a new coverage store outside of the sandbox + startPage(); + FormTester ft = tester.newFormTester("rasterStoreForm"); + ft.setValue( + "parametersPanel:url:fileInput:border:border_body:paramValue", + tazbmOutside.getAbsolutePath()); + ft.setValue("namePanel:border:border_body:paramValue", "tazbm4"); + ft.submit("apply"); + + List messages = tester.getMessages(FeedbackMessage.ERROR); + assertEquals(1, messages.size()); + assertThat( + messages.get(0).toString(), + allOf( + containsString("Access to "), + containsString(tazbmOutside.getAbsolutePath()), + containsString(" denied by file sandboxing"))); + + // now save within the sandbox instead + tester.clearFeedbackMessages(); + ft.setValue( + "parametersPanel:url:fileInput:border:border_body:paramValue", + tazbmInside.getAbsolutePath()); + ft.submit("apply"); + + tester.assertNoErrorMessage(); + + tester.assertRenderedPage(CoverageStoreEditPage.class); + CoverageStoreInfo store = getCatalog().getCoverageStoreByName("tazbm4"); + assertNotNull(store); + // replace is for Windows + assertEquals( + "file://" + tazbmInside.getAbsolutePath().replace("\\", "/"), store.getURL()); + } finally { + System.clearProperty(GEOSERVER_DATA_SANDBOX); + GeoServerExtensions.bean(DefaultFileAccessManager.class).reload(); + } + } } diff --git a/src/web/core/src/test/java/org/geoserver/web/data/store/DataAccessEditPageTest.java b/src/web/core/src/test/java/org/geoserver/web/data/store/DataAccessEditPageTest.java index 2faf5ba953d..4a287ce61ab 100644 --- a/src/web/core/src/test/java/org/geoserver/web/data/store/DataAccessEditPageTest.java +++ b/src/web/core/src/test/java/org/geoserver/web/data/store/DataAccessEditPageTest.java @@ -5,6 +5,9 @@ */ package org.geoserver.web.data.store; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.allOf; +import static org.hamcrest.Matchers.containsString; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; @@ -12,8 +15,10 @@ import com.google.common.collect.Lists; import java.io.IOException; +import java.io.OutputStream; import java.io.Serializable; import java.util.List; +import java.util.Properties; import org.apache.wicket.Component; import org.apache.wicket.MarkupContainer; import org.apache.wicket.feedback.FeedbackMessage; @@ -27,6 +32,11 @@ import org.geoserver.catalog.NamespaceInfo; import org.geoserver.catalog.WorkspaceInfo; import org.geoserver.data.test.MockData; +import org.geoserver.data.test.SystemTestData; +import org.geoserver.platform.GeoServerExtensions; +import org.geoserver.platform.resource.Resource; +import org.geoserver.security.impl.DefaultFileAccessManager; +import org.geoserver.security.impl.FileSandboxEnforcer; import org.geoserver.web.GeoServerWicketTestSupport; import org.geoserver.web.data.store.panel.DropDownChoiceParamPanel; import org.geotools.data.postgis.PostgisNGDataStoreFactory; @@ -37,8 +47,18 @@ public class DataAccessEditPageTest extends GeoServerWicketTestSupport { + private static final String ROLE_CITE = "ROLE_CITE"; private DataStoreInfo store; + @Override + protected void onSetUp(SystemTestData testData) throws Exception { + super.onSetUp(testData); + + // force creation of the FileSanboxEnforcer (beans are lazy loaded in tests, and this + // one registers itself on the catalog on creation) + GeoServerExtensions.bean(FileSandboxEnforcer.class, applicationContext); + } + @Before public void init() { login(); @@ -295,4 +315,70 @@ public void testDataStoreEditEnum() throws Exception { } assertNotNull(dropDown); } + + @Test + public void testDataStoreEditSandbox() throws Exception { + // setup sandbox on file system + java.io.File sandbox = new java.io.File("./target/sandbox").getCanonicalFile(); + java.io.File citeFolder = new java.io.File(sandbox, MockData.CITE_PREFIX); + java.io.File toppFolder = new java.io.File(sandbox, "topp"); // this won't be allowed + citeFolder.mkdirs(); + toppFolder.mkdirs(); + + // no need to have test data, the property data store can use an empty folder + + // setup a sandbox by security config + Resource layerSecurity = getDataDirectory().get("security/layers.properties"); + Properties properties = new Properties(); + properties.put("filesystemSandbox", sandbox.getAbsolutePath()); + properties.put("cite.*.a", ROLE_CITE); + try (OutputStream os = layerSecurity.out()) { + properties.store(os, "sandbox"); + } + DefaultFileAccessManager fam = + GeoServerExtensions.bean(DefaultFileAccessManager.class, applicationContext); + fam.reload(); + + // login as workspace admin (logout happens as @After in base class) + login("cite", "pwd", ROLE_CITE); + try { + tester.startPage(new DataAccessEditPage(store.getId())); + + // cannot save, the current location is outside of the sanbox + FormTester form = tester.newFormTester("dataStoreForm"); + String toppPath = toppFolder.getAbsolutePath(); + String fileInputPath = + "parametersPanel:parameters:0:parameterPanel:fileInput:border:border_body:paramValue"; + form.setValue(fileInputPath, toppPath); + form.submit(); + tester.clickLink("dataStoreForm:save", true); + + List messages = tester.getMessages(FeedbackMessage.ERROR); + assertEquals(1, messages.size()); + assertThat( + messages.get(0).toString(), + allOf( + containsString("Access to "), + containsString(toppPath), + containsString(" denied by file sandboxing"))); + tester.clearFeedbackMessages(); + + // now try within the sandbox + form = tester.newFormTester("dataStoreForm"); + String citePath = citeFolder.getAbsolutePath(); + form.setValue(fileInputPath, citePath); + form.submit(); + tester.clickLink("dataStoreForm:save", true); + + // no messages and save worked + tester.assertNoErrorMessage(); + DataStoreInfo store = getCatalog().getDataStoreByName(MockData.CITE_PREFIX); + assertEquals( + "file://" + citePath.replace("\\", "/"), + store.getConnectionParameters().get("directory")); + } finally { + layerSecurity.delete(); + fam.reload(); + } + } } diff --git a/src/web/core/src/test/java/org/geoserver/web/data/store/DataAccessNewPageTest.java b/src/web/core/src/test/java/org/geoserver/web/data/store/DataAccessNewPageTest.java index 01aa4d26bf0..3ef87630029 100644 --- a/src/web/core/src/test/java/org/geoserver/web/data/store/DataAccessNewPageTest.java +++ b/src/web/core/src/test/java/org/geoserver/web/data/store/DataAccessNewPageTest.java @@ -7,20 +7,33 @@ import static org.hamcrest.CoreMatchers.instanceOf; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.allOf; +import static org.hamcrest.Matchers.containsString; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; +import java.io.OutputStream; +import java.io.Serializable; import java.util.Arrays; import java.util.List; +import java.util.Properties; import org.apache.wicket.Component; +import org.apache.wicket.feedback.FeedbackMessage; import org.apache.wicket.markup.html.form.CheckBox; +import org.apache.wicket.markup.html.form.DropDownChoice; import org.apache.wicket.util.tester.FormTester; import org.geoserver.catalog.DataStoreInfo; import org.geoserver.catalog.NamespaceInfo; import org.geoserver.catalog.WorkspaceInfo; +import org.geoserver.data.test.MockData; +import org.geoserver.data.test.SystemTestData; +import org.geoserver.platform.GeoServerExtensions; +import org.geoserver.platform.resource.Resource; +import org.geoserver.security.impl.DefaultFileAccessManager; +import org.geoserver.security.impl.FileSandboxEnforcer; import org.geoserver.web.GeoServerWicketTestSupport; import org.geoserver.web.data.layer.NewLayerPage; import org.geoserver.web.data.store.panel.FileParamPanel; @@ -39,6 +52,17 @@ public class DataAccessNewPageTest extends GeoServerWicketTestSupport { /** print page structure? */ private static final boolean debugMode = false; + private static final String ROLE_CITE = "ROLE_CITE"; + + @Override + protected void onSetUp(SystemTestData testData) throws Exception { + super.onSetUp(testData); + + // force creation of the FileSanboxEnforcer (beans are lazy loaded in tests, and this + // one registers itself on the catalog on creation) + GeoServerExtensions.bean(FileSandboxEnforcer.class, applicationContext); + } + private AbstractDataAccessPage startPage() { login(); final String dataStoreFactoryDisplayName = new PropertyDataStoreFactory().getDisplayName(); @@ -225,4 +249,89 @@ public void testDisableOnConnFailureCheckbox() { assertNotNull(store); assertTrue(store.isDisableOnConnFailure()); } + + @Test + @SuppressWarnings("unchecked") + public void testDataStoreNewSandbox() throws Exception { + // setup sandbox on file system + java.io.File sandbox = new java.io.File("./target/sandbox").getCanonicalFile(); + java.io.File citeFolder = new java.io.File(sandbox, MockData.CITE_PREFIX); + java.io.File toppFolder = new java.io.File(sandbox, "topp"); // this won't be allowed + citeFolder.mkdirs(); + toppFolder.mkdirs(); + + // no need to have test data, the property data store can use an empty folder + + // setup a sandbox by security config + Resource layerSecurity = getDataDirectory().get("security/layers.properties"); + Properties properties = new Properties(); + properties.put("filesystemSandbox", sandbox.getAbsolutePath()); + properties.put("cite.*.a", ROLE_CITE); + try (OutputStream os = layerSecurity.out()) { + properties.store(os, "sandbox"); + } + DefaultFileAccessManager fam = + GeoServerExtensions.bean(DefaultFileAccessManager.class, applicationContext); + fam.reload(); + + // login as workspace admin (logout happens as @After in base class) + startPage(); + try { + login("cite", "pwd", ROLE_CITE); + FormTester ft = tester.newFormTester("dataStoreForm"); + + DropDownChoice select = + (DropDownChoice) + tester.getComponentFromLastRenderedPage( + "dataStoreForm:workspacePanel:border:border_body:paramValue"); + List workspaces = select.getChoices(); + int citeIdx = -1; + for (int i = 0; i < workspaces.size(); i++) { + if (MockData.CITE_PREFIX.equals(workspaces.get(i).getName())) { + citeIdx = i; + break; + } + } + + // cannot save, the current location is outside of the sandbox + String storeName = "cite2"; + String toppPath = toppFolder.getAbsolutePath(); + ft.setValue("dataStoreNamePanel:border:border_body:paramValue", storeName); + ft.select("workspacePanel:border:border_body:paramValue", citeIdx); + ft.setValue( + "parametersPanel:parameters:0:parameterPanel:fileInput:border:border_body:paramValue", + toppPath); + ft.submit("save"); + + List messages = tester.getMessages(FeedbackMessage.ERROR); + assertEquals(1, messages.size()); + assertThat( + messages.get(0).toString(), + allOf( + containsString("Access to "), + containsString(toppPath), + containsString(" denied by file sandboxing"))); + tester.clearFeedbackMessages(); + + // now try within the sandbox + String citePath = citeFolder.getAbsolutePath(); + ft = tester.newFormTester("dataStoreForm"); + ft.setValue("dataStoreNamePanel:border:border_body:paramValue", storeName); + ft.select("workspacePanel:border:border_body:paramValue", citeIdx); + ft.setValue( + "parametersPanel:parameters:0:parameterPanel:fileInput:border:border_body:paramValue", + citePath); + ft.submit("save"); + + // no messages and save worked + tester.assertNoErrorMessage(); + DataStoreInfo store = getCatalog().getDataStoreByName(storeName); + assertEquals( + "file://" + citePath.replace("\\", "/"), + store.getConnectionParameters().get("directory")); + } finally { + layerSecurity.delete(); + fam.reload(); + } + } } diff --git a/src/web/core/src/test/java/org/geoserver/web/wicket/browser/GeoServerFileChooserTest.java b/src/web/core/src/test/java/org/geoserver/web/wicket/browser/GeoServerFileChooserTest.java index df37d5200fb..06f1727dd10 100644 --- a/src/web/core/src/test/java/org/geoserver/web/wicket/browser/GeoServerFileChooserTest.java +++ b/src/web/core/src/test/java/org/geoserver/web/wicket/browser/GeoServerFileChooserTest.java @@ -5,6 +5,7 @@ */ package org.geoserver.web.wicket.browser; +import static org.geoserver.security.impl.DefaultFileAccessManager.GEOSERVER_DATA_SANDBOX; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.greaterThan; import static org.hamcrest.Matchers.hasItem; @@ -14,8 +15,11 @@ import java.io.File; import java.io.IOException; +import java.io.OutputStream; +import java.util.ArrayList; import java.util.HashSet; import java.util.List; +import java.util.Properties; import java.util.stream.Collectors; import org.apache.commons.io.FileUtils; import org.apache.wicket.Component; @@ -24,7 +28,12 @@ import org.apache.wicket.markup.html.form.DropDownChoice; import org.apache.wicket.markup.repeater.data.DataView; import org.apache.wicket.model.Model; +import org.geoserver.config.GeoServerDataDirectory; import org.geoserver.data.test.MockData; +import org.geoserver.platform.GeoServerExtensions; +import org.geoserver.platform.resource.Resource; +import org.geoserver.security.impl.DefaultFileAccessManager; +import org.geoserver.security.impl.GeoServerRole; import org.geoserver.web.ComponentBuilder; import org.geoserver.web.FormTestPage; import org.geoserver.web.GeoServerWicketTestSupport; @@ -35,10 +44,13 @@ public class GeoServerFileChooserTest extends GeoServerWicketTestSupport { + private static final String ROLE_CITE = "ROLE_CITE"; + private static final String ROLE_SF = "ROLE_SF"; private File root; private File one; private File two; private File child; + private DefaultFileAccessManager fam; @Before public void init() throws IOException { @@ -53,6 +65,26 @@ public void init() throws IOException { two.createNewFile(); } + @Before + public void cleanupRestrictions() throws Exception { + // clean up the security restrictions + GeoServerDataDirectory dd = getDataDirectory(); + Resource layerSecurity = dd.get("security/layers.properties"); + Properties properties = new Properties(); + properties.put("*.*.r", "*"); + properties.put("*.*.w", "*"); + try (OutputStream os = layerSecurity.out()) { + properties.store(os, "everyone can read and write"); + } + + // clear the system sandbox + System.clearProperty(GEOSERVER_DATA_SANDBOX); + + // grab the file access manager and force reloading definitions + fam = GeoServerExtensions.bean(DefaultFileAccessManager.class, applicationContext); + fam.reload(); + } + public void setupChooser(final File file) { tester.startPage( new FormTestPage( @@ -183,4 +215,78 @@ public void testAutocompleteDirectories() throws Exception { values, hasItem("file://" + new File(rootPath, MockData.SF_PREFIX).getAbsolutePath())); } + + @Test + public void testAdminSandbox() throws Exception { + // setup sandbox on file system + File systemSandbox = new File("./target/fc-systemSandbox").getCanonicalFile(); + File citeFolder = new File(systemSandbox, MockData.CITE_PREFIX); + File sfFolder = new File(systemSandbox, MockData.SF_PREFIX); + citeFolder.mkdirs(); + sfFolder.mkdirs(); + + // configure security, make sure the file access manager pays attention + System.setProperty( + DefaultFileAccessManager.GEOSERVER_DATA_SANDBOX, systemSandbox.getAbsolutePath()); + fam.reload(); + + // roots finder limits to that directory + login("admin", "geoserver", GeoServerRole.ADMIN_ROLE.getAuthority()); + FileRootsFinder rootsFinder = new FileRootsFinder(true); + ArrayList files = rootsFinder.getRoots(); + assertEquals(List.of(systemSandbox), files); + + // check autocomplete + List values = + rootsFinder + .getMatches(systemSandbox.getAbsolutePath() + File.separator, null) + .collect(Collectors.toList()); + assertEquals( + List.of( + "file://" + citeFolder.getAbsoluteFile(), + "file://" + sfFolder.getAbsoluteFile()), + values); + } + + @Test + public void testWorkspaceSandbox() throws Exception { + // setup sandbox on file system + File sandbox = new File("./target/sandbox").getCanonicalFile(); + File citeFolder = new File(sandbox, MockData.CITE_PREFIX); + File sfFolder = new File(sandbox, MockData.SF_PREFIX); + File toppFolder = new File(sandbox, "topp"); // this won't be allowed + citeFolder.mkdirs(); + sfFolder.mkdirs(); + toppFolder.mkdirs(); + + // setup a sandbox by security config + Resource layerSecurity = getDataDirectory().get("security/layers.properties"); + Properties properties = new Properties(); + properties.put("filesystemSandbox", sandbox.getAbsolutePath()); + properties.put("cite.*.a", ROLE_CITE); + properties.put("sf.*.a", ROLE_SF); + try (OutputStream os = layerSecurity.out()) { + properties.store(os, "sandbox"); + } + + // force reloading definitions + fam.reload(); + + // roots finder limits two out of three of the available directories + login("multi", "multi", ROLE_CITE, ROLE_SF); + FileRootsFinder rootsFinder = new FileRootsFinder(true); + ArrayList files = rootsFinder.getRoots(); + assertEquals(List.of(citeFolder, sfFolder), files); + + // check autocomplete + List values = + rootsFinder + .getMatches(sandbox.getAbsolutePath() + File.separator, null) + .collect(Collectors.toList()); + assertEquals( + List.of( + "file://" + citeFolder.getAbsoluteFile(), + "file://" + sfFolder.getAbsoluteFile()), + values); + } } diff --git a/src/web/security/core/src/main/java/org/geoserver/security/web/data/DataSecurityPage.html b/src/web/security/core/src/main/java/org/geoserver/security/web/data/DataSecurityPage.html index 392802f9168..fd09c563a53 100644 --- a/src/web/security/core/src/main/java/org/geoserver/security/web/data/DataSecurityPage.html +++ b/src/web/security/core/src/main/java/org/geoserver/security/web/data/DataSecurityPage.html @@ -16,7 +16,7 @@

-
+
@@ -27,6 +27,13 @@ mode 1
+
+ + Filesystem sandbox + + +
+
Save diff --git a/src/web/security/core/src/main/java/org/geoserver/security/web/data/DataSecurityPage.java b/src/web/security/core/src/main/java/org/geoserver/security/web/data/DataSecurityPage.java index f1d42968b1b..966691735a7 100644 --- a/src/web/security/core/src/main/java/org/geoserver/security/web/data/DataSecurityPage.java +++ b/src/web/security/core/src/main/java/org/geoserver/security/web/data/DataSecurityPage.java @@ -5,13 +5,15 @@ */ package org.geoserver.security.web.data; +import java.io.File; +import java.util.ArrayList; import java.util.Arrays; -import java.util.List; import java.util.logging.Level; import org.apache.wicket.Component; import org.apache.wicket.ajax.AjaxRequestTarget; import org.apache.wicket.markup.head.CssHeaderItem; import org.apache.wicket.markup.head.IHeaderResponse; +import org.apache.wicket.markup.html.WebMarkupContainer; import org.apache.wicket.markup.html.basic.Label; import org.apache.wicket.markup.html.form.ChoiceRenderer; import org.apache.wicket.markup.html.form.Form; @@ -20,11 +22,16 @@ import org.apache.wicket.markup.html.form.SubmitLink; import org.apache.wicket.markup.html.link.BookmarkablePageLink; import org.apache.wicket.markup.html.panel.Fragment; -import org.apache.wicket.model.CompoundPropertyModel; import org.apache.wicket.model.IModel; +import org.apache.wicket.model.Model; +import org.apache.wicket.validation.IValidatable; +import org.apache.wicket.validation.IValidator; +import org.apache.wicket.validation.ValidationError; +import org.geoserver.platform.GeoServerExtensions; import org.geoserver.security.CatalogMode; import org.geoserver.security.impl.DataAccessRule; import org.geoserver.security.impl.DataAccessRuleDAO; +import org.geoserver.security.impl.DefaultFileAccessManager; import org.geoserver.security.web.AbstractSecurityPage; import org.geoserver.web.GeoServerHomePage; import org.geoserver.web.wicket.GeoServerDataProvider.Property; @@ -32,19 +39,22 @@ import org.geoserver.web.wicket.HelpLink; import org.geoserver.web.wicket.ParamResourceModel; import org.geoserver.web.wicket.SimpleAjaxLink; +import org.geoserver.web.wicket.browser.DirectoryInput; /** A page listing data access rules, allowing for removal, addition and linking to an edit page */ @SuppressWarnings("serial") public class DataSecurityPage extends AbstractSecurityPage { - static final List CATALOG_MODES = - Arrays.asList(CatalogMode.HIDE, CatalogMode.MIXED, CatalogMode.CHALLENGE); + // ArrayList as it needs to be serializable + static final ArrayList CATALOG_MODES = + new ArrayList<>( + Arrays.asList(CatalogMode.HIDE, CatalogMode.MIXED, CatalogMode.CHALLENGE)); private GeoServerTablePanel rules; private SelectionDataRuleRemovalLink removal; - private RadioChoice catalogModeChoice; + private RadioChoice catalogModeChoice; public DataSecurityPage() { DataAccessRuleProvider provider = new DataAccessRuleProvider(); @@ -77,28 +87,50 @@ protected void onSelectionUpdate(AjaxRequestTarget target) { setHeaderPanel(headerPanel()); - Form form = - new Form<>( - "catalogModeForm", - new CompoundPropertyModel<>( - new CatalogModeModel(DataAccessRuleDAO.get().getMode()))); + Form form = new Form<>("otherSettingsForm"); add(form); form.add(new HelpLink("catalogModeHelp").setDialog(dialog)); + DataAccessRuleDAO dataAccessRuleDAO = DataAccessRuleDAO.get(); catalogModeChoice = - new RadioChoice<>("catalogMode", CATALOG_MODES, new CatalogModeRenderer()); + new RadioChoice<>( + "catalogMode", + new Model<>(dataAccessRuleDAO.getMode()), + new Model<>(CATALOG_MODES), + new CatalogModeRenderer()); catalogModeChoice.add(new FormComponentUpdatingBehavior() {}); catalogModeChoice.setSuffix(" "); form.add(catalogModeChoice); + // Filesystem sandbox configuration, available only if the system administrator did + // not set it via a system property + WebMarkupContainer sandboxContainer = new WebMarkupContainer("sandboxContainer"); + form.add(sandboxContainer); + DefaultFileAccessManager fam = + GeoServerExtensions.bean( + DefaultFileAccessManager.class, + getGeoServerApplication().getApplicationContext()); + sandboxContainer.setVisible(!fam.isSystemSanboxEnabled()); + Model sandboxModel = new Model<>(dataAccessRuleDAO.getFilesystemSandbox()); + DirectoryInput sandboxInput = + new DirectoryInput( + "sandbox", + sandboxModel, + new ParamResourceModel("sandbox", this), + false, + new DirectoryExistsValidator()); + sandboxInput.setPrefixPaths(false); + sandboxContainer.add(sandboxInput); + form.add( new SubmitLink("save") { @Override public void onSubmit() { try { - DataAccessRuleDAO dao = DataAccessRuleDAO.get(); + DataAccessRuleDAO dao = dataAccessRuleDAO; CatalogMode newMode = dao.getByAlias(catalogModeChoice.getValue()); dao.setCatalogMode(newMode); + dao.setFilesystemSandbox(sandboxModel.getObject()); dao.storeRules(); doReturn(); } catch (Exception e) { @@ -171,4 +203,21 @@ public String getIdValue(CatalogMode object, int index) { return object.name(); } } + + private static class DirectoryExistsValidator implements IValidator { + private static final long serialVersionUID = 1L; + + @Override + public void validate(IValidatable validatable) { + String path = validatable.getValue(); + if (path != null && !path.isEmpty()) { + File file = new File(path); + if (!file.exists() || !file.isDirectory()) { + ValidationError error = new ValidationError(this); + error.addKey("DataSecurityPage.sanboxNotFoundError"); + validatable.error(error); + } + } + } + } } diff --git a/src/web/security/core/src/main/resources/GeoServerApplication.properties b/src/web/security/core/src/main/resources/GeoServerApplication.properties index 27d441e2ca7..f1d6ad0e6da 100644 --- a/src/web/security/core/src/main/resources/GeoServerApplication.properties +++ b/src/web/security/core/src/main/resources/GeoServerApplication.properties @@ -338,6 +338,8 @@ that a user does not have privileges or via anonymous access.

\ DataSecurityPage.HIDE=HIDE DataSecurityPage.MIXED=MIXED DataSecurityPage.CHALLENGE=CHALLENGE +DataSecurityPage.sandbox=File system sandbox (for workspace administrators) +DataSecurityPage.sanboxNotFoundError=The sandbox directory does not exist AbstractConfirmRemovelPanel.aboutRemove = Are you sure you want to remove these objects? AbstractConfirmRemovelPanel.removedObjects =The following objects will be removed diff --git a/src/web/security/core/src/test/java/org/geoserver/security/web/data/DataSecurityPageTest.java b/src/web/security/core/src/test/java/org/geoserver/security/web/data/DataSecurityPageTest.java index 68cf266bd5f..cabfe4cd7c1 100644 --- a/src/web/security/core/src/test/java/org/geoserver/security/web/data/DataSecurityPageTest.java +++ b/src/web/security/core/src/test/java/org/geoserver/security/web/data/DataSecurityPageTest.java @@ -9,6 +9,7 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotEquals; +import java.io.File; import java.lang.reflect.Method; import java.util.Collections; import org.apache.wicket.Component; @@ -17,6 +18,7 @@ import org.apache.wicket.request.mapper.parameter.PageParameters; import org.apache.wicket.util.tester.FormTester; import org.geoserver.data.test.MockData; +import org.geoserver.platform.GeoServerExtensions; import org.geoserver.security.AccessMode; import org.geoserver.security.impl.DataAccessRule; import org.geoserver.security.impl.DataAccessRuleDAO; @@ -99,7 +101,7 @@ public void testDefaultCatalogMode() throws Exception { tester.assertRenderedPage(DataSecurityPage.class); assertEquals( "HIDE", - tester.getComponentFromLastRenderedPage("catalogModeForm:catalogMode") + tester.getComponentFromLastRenderedPage("otherSettingsForm:catalogMode") .getDefaultModelObject() .toString()); } @@ -112,18 +114,50 @@ public void testEditCatalogMode() throws Exception { // simple test assertNotEquals( "CHALLENGE", - tester.getComponentFromLastRenderedPage("catalogModeForm:catalogMode") + tester.getComponentFromLastRenderedPage("otherSettingsForm:catalogMode") .getDefaultModelObject()); // edit catalogMode value - final FormTester form = tester.newFormTester("catalogModeForm"); + final FormTester form = tester.newFormTester("otherSettingsForm"); form.select("catalogMode", 1); assertEquals( "MIXED", - tester.getComponentFromLastRenderedPage("catalogModeForm:catalogMode") + tester.getComponentFromLastRenderedPage("otherSettingsForm:catalogMode") .getDefaultModelObject() .toString()); } + + @Test + public void testSandbox() throws Exception { + // setup a sandbox + File sandbox = new File("./target/sandbox").getCanonicalFile(); + sandbox.mkdirs(); + File notThere = new File("./target/notThere").getCanonicalFile(); + + tester.startPage(DataSecurityPage.class); + tester.assertRenderedPage(DataSecurityPage.class); + + // test non existing sandbox + FormTester form = tester.newFormTester("otherSettingsForm"); + form.setValue( + "sandboxContainer:sandbox:border:border_body:paramValue", + notThere.getAbsolutePath()); + form.submit("save"); + tester.assertErrorMessages("The sandbox directory does not exist"); + tester.clearFeedbackMessages(); + + // test existing sandbox + form = tester.newFormTester("otherSettingsForm"); + form.setValue( + "sandboxContainer:sandbox:border:border_body:paramValue", + sandbox.getAbsolutePath()); + form.submit("save"); + tester.assertNoErrorMessage(); + + DataAccessRuleDAO dao = + GeoServerExtensions.bean(DataAccessRuleDAO.class, applicationContext); + assertEquals(sandbox.getAbsolutePath().replace("\\", "/"), dao.getFilesystemSandbox()); + } }