diff --git a/src/main/src/main/java/org/geoserver/security/SecureCatalogImpl.java b/src/main/src/main/java/org/geoserver/security/SecureCatalogImpl.java index 6ac73c4e1c6..2a240b5354a 100644 --- a/src/main/src/main/java/org/geoserver/security/SecureCatalogImpl.java +++ b/src/main/src/main/java/org/geoserver/security/SecureCatalogImpl.java @@ -35,9 +35,7 @@ import org.geoserver.catalog.StoreInfo; import org.geoserver.catalog.StyleInfo; import org.geoserver.catalog.ValidationResult; -import org.geoserver.catalog.WMSLayerInfo; import org.geoserver.catalog.WMSStoreInfo; -import org.geoserver.catalog.WMTSLayerInfo; import org.geoserver.catalog.WMTSStoreInfo; import org.geoserver.catalog.WorkspaceInfo; import org.geoserver.catalog.event.CatalogListener; @@ -48,11 +46,11 @@ import org.geoserver.platform.GeoServerResourceLoader; import org.geoserver.security.decorators.DecoratingCatalogFactory; import org.geoserver.security.decorators.SecuredCoverageInfo; -import org.geoserver.security.decorators.SecuredCoverageStoreInfo; import org.geoserver.security.decorators.SecuredDataStoreInfo; import org.geoserver.security.decorators.SecuredFeatureTypeInfo; import org.geoserver.security.decorators.SecuredLayerGroupInfo; import org.geoserver.security.decorators.SecuredLayerInfo; +import org.geoserver.security.decorators.SecuredObjects; import org.geoserver.security.decorators.SecuredWMSLayerInfo; import org.geoserver.security.decorators.SecuredWMTSLayerInfo; import org.geoserver.security.impl.DataAccessRuleDAO; @@ -69,6 +67,10 @@ import com.google.common.base.Function; import com.google.common.collect.ImmutableList; +import org.geoserver.security.decorators.SecuredCoverageStoreInfo; +import org.geoserver.security.decorators.SecuredWMSStoreInfo; +import org.geoserver.security.decorators.SecuredWMTSStoreInfo; + /** * Wraps the catalog and applies the security directives provided by a {@link ResourceAccessManager} * or a {@link DataAccessManager} registered in the Spring application context @@ -532,17 +534,7 @@ else if(policy.level == AccessLevel.READ_WRITE && policy.getLimits() == null) // otherwise we are in a mixed case where the user can read but not write, or // cannot read but is allowed by the operation mode to access the metadata - if(info instanceof FeatureTypeInfo) { - return (T) new SecuredFeatureTypeInfo((FeatureTypeInfo) info, policy); - } else if(info instanceof CoverageInfo) { - return (T) new SecuredCoverageInfo((CoverageInfo) info, policy); - } else if(info instanceof WMSLayerInfo) { - return (T) new SecuredWMSLayerInfo((WMSLayerInfo) info, policy); - } else if(info instanceof WMTSLayerInfo) { - return (T) new SecuredWMTSLayerInfo((WMTSLayerInfo) info, policy); - } else { - throw new RuntimeException("Unknown resource type " + info.getClass()); - } + return (T) SecuredObjects.secure(info, policy); } /** @@ -586,16 +578,9 @@ else if(policy.level == AccessLevel.READ_WRITE || // write, or // cannot read but is allowed by the operation mode to access the // metadata - if (store instanceof DataStoreInfo) { - return (T) new SecuredDataStoreInfo((DataStoreInfo) store, policy); - } else if(store instanceof CoverageStoreInfo) { - return (T) new SecuredCoverageStoreInfo((CoverageStoreInfo) store, policy); - } else if (store instanceof WMSStoreInfo) { - // TODO: implement WMSStoreInfo wrappring if necessary - return store; - } else if (store instanceof WMTSStoreInfo) { - // TODO: implement WMTSStoreInfo wrappring if necessary - return store; + if (store instanceof DataStoreInfo || store instanceof CoverageStoreInfo || store instanceof WMSStoreInfo || + store instanceof WMTSStoreInfo) { + return (T) SecuredObjects.secure(store, policy); } else { throw new RuntimeException("Unknown store type " + store.getClass()); } @@ -621,7 +606,7 @@ else if(policy.level == AccessLevel.READ_WRITE && policy.getLimits() == null) // otherwise we are in a mixed case where the user can read but not write, or // cannot read but is allowed by the operation mode to access the metadata - return new SecuredLayerInfo(layer, policy); + return (LayerInfo) SecuredObjects.secure(layer, policy); } /** @@ -1122,6 +1107,12 @@ static ResourceInfo unwrap(ResourceInfo info) { static StoreInfo unwrap(StoreInfo info) { if(info instanceof SecuredDataStoreInfo) return ((SecuredDataStoreInfo) info).unwrap(StoreInfo.class); + if (info instanceof SecuredCoverageStoreInfo) + return ((SecuredCoverageStoreInfo) info).unwrap(StoreInfo.class); + if (info instanceof SecuredWMSStoreInfo) + return ((SecuredWMSStoreInfo) info).unwrap(StoreInfo.class); + if (info instanceof SecuredWMTSStoreInfo) + return ((SecuredWMTSStoreInfo) info).unwrap(StoreInfo.class); return info; } diff --git a/src/main/src/main/java/org/geoserver/security/decorators/DefaultSecureCatalogFactory.java b/src/main/src/main/java/org/geoserver/security/decorators/DefaultSecureCatalogFactory.java index 5593dfb41a9..e6de84eafa3 100644 --- a/src/main/src/main/java/org/geoserver/security/decorators/DefaultSecureCatalogFactory.java +++ b/src/main/src/main/java/org/geoserver/security/decorators/DefaultSecureCatalogFactory.java @@ -5,13 +5,19 @@ */ package org.geoserver.security.decorators; +import java.util.logging.Logger; + import org.geoserver.catalog.CoverageInfo; import org.geoserver.catalog.CoverageStoreInfo; import org.geoserver.catalog.DataStoreInfo; import org.geoserver.catalog.FeatureTypeInfo; import org.geoserver.catalog.LayerInfo; +import org.geoserver.catalog.WMSLayerInfo; +import org.geoserver.catalog.WMTSLayerInfo; +import org.geoserver.catalog.impl.ModificationProxy; import org.geoserver.platform.ExtensionPriority; import org.geoserver.security.WrapperPolicy; +import org.geotools.util.logging.Logging; /** * Creates security wrappers for the most common catalog objects @@ -21,6 +27,9 @@ */ public class DefaultSecureCatalogFactory implements SecuredObjectFactory { + private static final Logger LOGGER = Logging.getLogger(DefaultSecureCatalogFactory.class); + + @Override public boolean canSecure(Class clazz) { return CoverageInfo.class.isAssignableFrom(clazz) || CoverageStoreInfo.class.isAssignableFrom(clazz) @@ -29,32 +38,157 @@ public boolean canSecure(Class clazz) { || LayerInfo.class.isAssignableFrom(clazz); } + @Override public Object secure(Object object, WrapperPolicy policy) { // null safe if (object == null) return null; Class clazz = object.getClass(); + // for each supported Info type, log a warning if the object to be secured is already secured. If this happens, + // it could lead to a StackOverflowError if the object is re-wrapped, over time, over and over agian. if (CoverageInfo.class.isAssignableFrom(clazz)) - return new SecuredCoverageInfo((CoverageInfo) object, policy); + return new SecuredCoverageInfo(logIfSecured((CoverageInfo) object), policy); else if (CoverageStoreInfo.class.isAssignableFrom(clazz)) - return new SecuredCoverageStoreInfo((CoverageStoreInfo) object, policy); + return new SecuredCoverageStoreInfo(logIfSecured((CoverageStoreInfo) object), policy); else if (DataStoreInfo.class.isAssignableFrom(clazz)) - return new SecuredDataStoreInfo((DataStoreInfo) object, policy); + return new SecuredDataStoreInfo(logIfSecured((DataStoreInfo) object), policy); else if (FeatureTypeInfo.class.isAssignableFrom(clazz)) - return new SecuredFeatureTypeInfo((FeatureTypeInfo) object, policy); + return new SecuredFeatureTypeInfo(logIfSecured((FeatureTypeInfo) object), policy); else if (LayerInfo.class.isAssignableFrom(clazz)) - return new SecuredLayerInfo((LayerInfo) object, policy); + return new SecuredLayerInfo(logIfSecured((LayerInfo) object), policy); + else if (WMSLayerInfo.class.isAssignableFrom(clazz)) + return new SecuredWMSLayerInfo(logIfSecured((WMSLayerInfo) object), policy); + else if (WMTSLayerInfo.class.isAssignableFrom(clazz)) + return new SecuredWMTSLayerInfo(logIfSecured((WMTSLayerInfo) object), policy); else - throw new IllegalArgumentException("Don't know how to wrap"); + throw new IllegalArgumentException("Don't know how to wrap " + object); } - /** * Returns {@link ExtensionPriority#LOWEST} since the wrappers generated by * this factory + * @return {@link ExtensionPriority#LOWEST} */ + @Override public int getPriority() { return ExtensionPriority.LOWEST; } + private void logDoubleWrap(Object unwrapped, Object orig) { + String msg = String.format("Tried to double secure: %s already securing %s", orig, unwrapped); + LOGGER.warning(msg); + } + + /** + * Generates a warning log if the Info object is already wrapped with a Secured decorator. This method is only + * intended to log a situation where a Catalog Info object is being secured, but is already secured. Repeated calls + * to this will keep adding additional wrapper layers and may eventually cause a StackOverflowError. The log + * generated is merely to aid in finding the real issue, as opposed to masking it here. + * @param object {@link WMTSLayerInfo} to check. + * @return The original object to be checked. + */ + private WMTSLayerInfo logIfSecured(WMTSLayerInfo object) { + WMTSLayerInfo unwrapped = ModificationProxy.unwrap(object); + if(unwrapped instanceof SecuredWMTSLayerInfo) { + logDoubleWrap(unwrapped, object); + } + return object; + } + + /** + * Generates a warning log if the Info object is already wrapped with a Secured decorator. This method is only + * intended to log a situation where a Catalog Info object is being secured, but is already secured. Repeated calls + * to this will keep adding additional wrapper layers and may eventually cause a StackOverflowError. The log + * generated is merely to aid in finding the real issue, as opposed to masking it here. + * @param object {@link WMSLayerInfo} to check. + * @return The original object to be checked. + */ + private WMSLayerInfo logIfSecured(WMSLayerInfo object) { + WMSLayerInfo unwrapped = ModificationProxy.unwrap(object); + if(unwrapped instanceof SecuredWMSLayerInfo) { + logDoubleWrap(unwrapped, object); + } + return object; + } + + /** + * Generates a warning log if the Info object is already wrapped with a Secured decorator. This method is only + * intended to log a situation where a Catalog Info object is being secured, but is already secured. Repeated calls + * to this will keep adding additional wrapper layers and may eventually cause a StackOverflowError. The log + * generated is merely to aid in finding the real issue, as opposed to masking it here. + * @param object {@link LayerInfo} to check. + * @return The original object to be checked. + */ + private LayerInfo logIfSecured(LayerInfo object) { + LayerInfo unwrapped = ModificationProxy.unwrap(object); + if(unwrapped instanceof SecuredLayerInfo) { + logDoubleWrap(unwrapped, object); + } + return object; + } + + /** + * Generates a warning log if the Info object is already wrapped with a Secured decorator. This method is only + * intended to log a situation where a Catalog Info object is being secured, but is already secured. Repeated calls + * to this will keep adding additional wrapper layers and may eventually cause a StackOverflowError. The log + * generated is merely to aid in finding the real issue, as opposed to masking it here. + * @param object {@link FeatureTypeInfo} to check. + * @return The original object to be checked. + */ + private FeatureTypeInfo logIfSecured(FeatureTypeInfo object) { + FeatureTypeInfo unwrapped = ModificationProxy.unwrap(object); + if(unwrapped instanceof SecuredFeatureTypeInfo) { + logDoubleWrap(unwrapped, object); + } + return object; + } + + /** + * Generates a warning log if the Info object is already wrapped with a Secured decorator. This method is only + * intended to log a situation where a Catalog Info object is being secured, but is already secured. Repeated calls + * to this will keep adding additional wrapper layers and may eventually cause a StackOverflowError. The log + * generated is merely to aid in finding the real issue, as opposed to masking it here. + * @param object {@link CoverageStoreInfo} to check. + * @return The original object to be checked. + */ + private CoverageStoreInfo logIfSecured(CoverageStoreInfo object) { + CoverageStoreInfo unwrapped = ModificationProxy.unwrap(object); + if(unwrapped instanceof SecuredCoverageStoreInfo) { + logDoubleWrap(unwrapped, object); + } + return object; + } + + /** + * Generates a warning log if the Info object is already wrapped with a Secured decorator. This method is only + * intended to log a situation where a Catalog Info object is being secured, but is already secured. Repeated calls + * to this will keep adding additional wrapper layers and may eventually cause a StackOverflowError. The log + * generated is merely to aid in finding the real issue, as opposed to masking it here. + * @param object {@link CoverageInfo} to check. + * @return The original object to be checked. + */ + private CoverageInfo logIfSecured(CoverageInfo object) { + CoverageInfo unwrapped = ModificationProxy.unwrap(object); + if(unwrapped instanceof SecuredDataStoreInfo) { + logDoubleWrap(unwrapped, object); + } + return object; + } + + /** + * Generates a warning log if the Info object is already wrapped with a Secured decorator. This method is only + * intended to log a situation where a Catalog Info object is being secured, but is already secured. Repeated calls + * to this will keep adding additional wrapper layers and may eventually cause a StackOverflowError. The log + * generated is merely to aid in finding the real issue, as opposed to masking it here. + * @param object {@link DataStoreInfo} to check. + * @return The original object to be checked. + */ + private DataStoreInfo logIfSecured(DataStoreInfo object) { + DataStoreInfo unwrapped = ModificationProxy.unwrap(object); + if(unwrapped instanceof SecuredDataStoreInfo) { + logDoubleWrap(unwrapped, object); + } + return object; + } + } diff --git a/src/main/src/main/java/org/geoserver/security/decorators/SecuredCoverageInfo.java b/src/main/src/main/java/org/geoserver/security/decorators/SecuredCoverageInfo.java index 7c6dd785ad4..982ab8bfd63 100644 --- a/src/main/src/main/java/org/geoserver/security/decorators/SecuredCoverageInfo.java +++ b/src/main/src/main/java/org/geoserver/security/decorators/SecuredCoverageInfo.java @@ -9,6 +9,7 @@ import org.geoserver.catalog.CoverageInfo; import org.geoserver.catalog.CoverageStoreInfo; +import org.geoserver.catalog.StoreInfo; import org.geoserver.ows.Dispatcher; import org.geoserver.ows.Request; import org.geoserver.security.AccessLevel; @@ -65,7 +66,12 @@ public GridCoverageReader getGridCoverageReader(ProgressListener listener, @Override public CoverageStoreInfo getStore() { - return new SecuredCoverageStoreInfo(super.getStore(), policy); + return (CoverageStoreInfo) SecuredObjects.secure(super.getStore(), policy); } + @Override + public void setStore(StoreInfo store) { + // need to make sure the store isn't secured + super.setStore((StoreInfo)SecureCatalogImpl.unwrap(store)); + } } diff --git a/src/main/src/main/java/org/geoserver/security/decorators/SecuredFeatureTypeInfo.java b/src/main/src/main/java/org/geoserver/security/decorators/SecuredFeatureTypeInfo.java index bcc7c5ed884..f6d73e54de5 100644 --- a/src/main/src/main/java/org/geoserver/security/decorators/SecuredFeatureTypeInfo.java +++ b/src/main/src/main/java/org/geoserver/security/decorators/SecuredFeatureTypeInfo.java @@ -17,6 +17,7 @@ import org.geoserver.catalog.DataStoreInfo; import org.geoserver.catalog.FeatureTypeInfo; +import org.geoserver.catalog.StoreInfo; import org.geoserver.security.AccessLevel; import org.geoserver.security.SecureCatalogImpl; import org.geoserver.security.VectorAccessLimits; @@ -97,6 +98,7 @@ public FeatureType getFeatureType() throws IOException { // WRAPPED METHODS TO ENFORCE SECURITY POLICY //-------------------------------------------------------------------------- + @Override public FeatureSource getFeatureSource(ProgressListener listener, Hints hints) throws IOException { final FeatureSource fs = delegate.getFeatureSource(listener, hints); @@ -108,8 +110,15 @@ public FeatureSource getFeatureSource(ProgressListener listener, Hints hints) } } + @Override public DataStoreInfo getStore() { return (DataStoreInfo) SecuredObjects.secure(delegate.getStore(), policy); } - + + @Override + public void setStore(StoreInfo store) { + // need to make sure the store isn't secured + super.setStore((StoreInfo)SecureCatalogImpl.unwrap(store)); + } + } diff --git a/src/main/src/main/java/org/geoserver/security/decorators/SecuredLayerInfo.java b/src/main/src/main/java/org/geoserver/security/decorators/SecuredLayerInfo.java index 43eefb63b0f..4ccc88f379b 100644 --- a/src/main/src/main/java/org/geoserver/security/decorators/SecuredLayerInfo.java +++ b/src/main/src/main/java/org/geoserver/security/decorators/SecuredLayerInfo.java @@ -33,13 +33,13 @@ public ResourceInfo getResource() { if (r == null) return null; else if (r instanceof FeatureTypeInfo) - return new SecuredFeatureTypeInfo((FeatureTypeInfo) r, policy); + return (FeatureTypeInfo) SecuredObjects.secure(r, policy); else if (r instanceof CoverageInfo) - return new SecuredCoverageInfo((CoverageInfo) r, policy); + return (CoverageInfo) SecuredObjects.secure(r, policy); else if (r instanceof WMSLayerInfo) - return new SecuredWMSLayerInfo((WMSLayerInfo) r, policy); + return (WMSLayerInfo) SecuredObjects.secure(r, policy); else if (r instanceof WMTSLayerInfo) - return new SecuredWMTSLayerInfo((WMTSLayerInfo) r, policy); + return (WMTSLayerInfo) SecuredObjects.secure(r, policy); else throw new RuntimeException("Don't know how to make resource of type " + r.getClass()); } diff --git a/src/main/src/main/java/org/geoserver/security/decorators/SecuredObjects.java b/src/main/src/main/java/org/geoserver/security/decorators/SecuredObjects.java index 9f7bb972c64..bb868ceab42 100644 --- a/src/main/src/main/java/org/geoserver/security/decorators/SecuredObjects.java +++ b/src/main/src/main/java/org/geoserver/security/decorators/SecuredObjects.java @@ -47,7 +47,7 @@ public static Object secure(Object object, WrapperPolicy policy) { // if we already know what can handle the wrapping, just do it, don't // scan the extension points once more - Class clazz = object.getClass(); + Class clazz = object.getClass(); SecuredObjectFactory candidate = FACTORY_CACHE.get(clazz); // otherwise scan and store (or complain) diff --git a/src/main/src/main/java/org/geoserver/security/decorators/SecuredWMSLayerInfo.java b/src/main/src/main/java/org/geoserver/security/decorators/SecuredWMSLayerInfo.java index 7656f086c96..ea7a1f8d964 100644 --- a/src/main/src/main/java/org/geoserver/security/decorators/SecuredWMSLayerInfo.java +++ b/src/main/src/main/java/org/geoserver/security/decorators/SecuredWMSLayerInfo.java @@ -5,10 +5,13 @@ */ package org.geoserver.security.decorators; +import org.geoserver.catalog.StoreInfo; + import java.io.IOException; import org.geoserver.catalog.WMSLayerInfo; import org.geoserver.catalog.WMSStoreInfo; +import org.geoserver.security.SecureCatalogImpl; import org.geoserver.security.WrapperPolicy; import org.geotools.data.ows.Layer; import org.opengis.util.ProgressListener; @@ -41,4 +44,10 @@ public Layer getWMSLayer(ProgressListener listener) throws IOException { public WMSStoreInfo getStore() { return new SecuredWMSStoreInfo(delegate.getStore(), policy); } + + @Override + public void setStore(StoreInfo store) { + // need to make sure the store isn't secured + super.setStore((StoreInfo)SecureCatalogImpl.unwrap(store)); + } } diff --git a/src/main/src/main/java/org/geoserver/security/decorators/SecuredWMTSLayerInfo.java b/src/main/src/main/java/org/geoserver/security/decorators/SecuredWMTSLayerInfo.java index 331c4396938..ed2cf28b097 100644 --- a/src/main/src/main/java/org/geoserver/security/decorators/SecuredWMTSLayerInfo.java +++ b/src/main/src/main/java/org/geoserver/security/decorators/SecuredWMTSLayerInfo.java @@ -4,11 +4,14 @@ */ package org.geoserver.security.decorators; +import org.geoserver.catalog.StoreInfo; + import java.io.IOException; import org.geoserver.catalog.WMSLayerInfo; import org.geoserver.catalog.WMTSLayerInfo; import org.geoserver.catalog.WMTSStoreInfo; +import org.geoserver.security.SecureCatalogImpl; import org.geoserver.security.WrapperPolicy; import org.geotools.data.ows.Layer; import org.opengis.util.ProgressListener; @@ -41,4 +44,10 @@ public Layer getWMTSLayer(ProgressListener listener) throws IOException { public WMTSStoreInfo getStore() { return new SecuredWMTSStoreInfo(delegate.getStore(), policy); } + + @Override + public void setStore(StoreInfo store) { + // need to make sure the store isn't secured + super.setStore((StoreInfo)SecureCatalogImpl.unwrap(store)); + } } diff --git a/src/main/src/test/java/org/geoserver/security/decorators/SecuredCoverageInfoTest.java b/src/main/src/test/java/org/geoserver/security/decorators/SecuredCoverageInfoTest.java new file mode 100644 index 00000000000..d95347c6db2 --- /dev/null +++ b/src/main/src/test/java/org/geoserver/security/decorators/SecuredCoverageInfoTest.java @@ -0,0 +1,49 @@ +/* (c) 2018 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.decorators; + + +import org.geoserver.catalog.CoverageInfo; +import org.geoserver.catalog.CoverageStoreInfo; +import org.geoserver.catalog.impl.CoverageInfoImpl; +import org.geoserver.catalog.impl.CoverageStoreInfoImpl; + +public class SecuredCoverageInfoTest extends SecuredResourceInfoTest { + + @Override + CoverageInfo createDelegate() { + final CoverageInfo info = new CoverageInfoImpl(getCatalog()); + final CoverageStoreInfo storeInfo = new CoverageStoreInfoImpl(getCatalog()); + info.setStore(storeInfo); + return info; + } + + @Override + SecuredCoverageInfo createSecuredDecorator(CoverageInfo delegate) { + return new SecuredCoverageInfo(delegate, policy); + } + + @Override + Class getDelegateClass() { + return CoverageInfo.class; + } + + @Override + Class getSecuredDecoratorClass() { + return SecuredCoverageInfo.class; + } + + @Override + Class getSecuredStoreInfoClass() { + return SecuredCoverageStoreInfo.class; + } + + @Override + int getStackOverflowCount() { + return 500; + } + + +} diff --git a/src/main/src/test/java/org/geoserver/security/decorators/SecuredFeatureTypeInfoTest.java b/src/main/src/test/java/org/geoserver/security/decorators/SecuredFeatureTypeInfoTest.java new file mode 100644 index 00000000000..98a2d354ba2 --- /dev/null +++ b/src/main/src/test/java/org/geoserver/security/decorators/SecuredFeatureTypeInfoTest.java @@ -0,0 +1,45 @@ +/* (c) 2018 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.decorators; + +import org.geoserver.catalog.FeatureTypeInfo; +import org.geoserver.catalog.impl.DataStoreInfoImpl; +import org.geoserver.catalog.impl.FeatureTypeInfoImpl; + +public class SecuredFeatureTypeInfoTest extends SecuredResourceInfoTest { + + @Override + FeatureTypeInfo createDelegate() { + FeatureTypeInfo info = new FeatureTypeInfoImpl(getCatalog()); + info.setStore(new DataStoreInfoImpl(getCatalog())); + return info; + } + + @Override + SecuredFeatureTypeInfo createSecuredDecorator(FeatureTypeInfo delegate) { + return new SecuredFeatureTypeInfo(delegate, policy); + } + + @Override + Class getDelegateClass() { + return FeatureTypeInfo.class; + } + + @Override + Class getSecuredDecoratorClass() { + return SecuredFeatureTypeInfo.class; + } + + @Override + Class getSecuredStoreInfoClass() { + return SecuredDataStoreInfo.class; + } + + @Override + int getStackOverflowCount() { + return 500; + } + +} diff --git a/src/main/src/test/java/org/geoserver/security/decorators/SecuredResourceInfoTest.java b/src/main/src/test/java/org/geoserver/security/decorators/SecuredResourceInfoTest.java new file mode 100644 index 00000000000..cc181fe7162 --- /dev/null +++ b/src/main/src/test/java/org/geoserver/security/decorators/SecuredResourceInfoTest.java @@ -0,0 +1,140 @@ +/* (c) 2018 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.decorators; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + +import org.geoserver.catalog.ResourceInfo; +import org.geoserver.catalog.StoreInfo; +import org.junit.Test; + +import org.geoserver.security.AccessLimits; +import org.geoserver.security.CatalogMode; +import org.geoserver.security.SecureCatalogImpl; +import org.geoserver.security.WrapperPolicy; +import org.geoserver.test.GeoServerSystemTestSupport; + +import java.io.PrintWriter; +import java.io.StringWriter; + +public abstract class SecuredResourceInfoTest + extends GeoServerSystemTestSupport { + + protected final WrapperPolicy policy = WrapperPolicy.readOnlyHide(new AccessLimits(CatalogMode.HIDE)); + + /** + * Creates an instance of a non-secure wrapped ResourceInfo. + * @return An instance of a non-secure wrapped ResourceInfo implementation. + */ + abstract D createDelegate(); + + /** + * Retrieves the Class of the non-secure wrapped ResourceInfo type. + * @return the Class of the non-secure wrapped ResourceInfo type. + */ + abstract Class getDelegateClass(); + + /** + * Wraps a non-Secured ResourceInfo with an appropriate security {@link Wrapper}. + * @param delegate An instance of the associated non-secure wrapped ResourceInfo type. + * @return A secured instance wrapping the supplied non-secure ResourceInfo instance. + */ + abstract S createSecuredDecorator(D delegate); + + /** + * Retrieves the Class of the secure wrapped ResourceInfo type. + * @return the Class of the secure wrapped ResourceInfo type. + */ + abstract Class getSecuredDecoratorClass(); + + /** + * Retrieves the Class of the secure wrapped StoreInfo type associated with the secure wrapped ResourceInfo type. + * @return the Class of the secure wrapped StoreInfo type associated with the secure wrapped ResourceInfo type. + */ + abstract Class getSecuredStoreInfoClass(); + + /** + * Retrieves the minimum number of times a secure {@link Wrapper} needs to re-wrap an object to cause a + * {@link java.lang.StackOverflowError} when setting a StoreInfo on a ResourceInfo, or when unwrapping nested + * {@link Wrapper}s. + * @return the number of nestings required to cause a {@link java.lang.StackOverflowError}. + */ + abstract int getStackOverflowCount(); + + /** + * Creates a Thread that will repeatedly set the StoreInfo on the target ResourceInfo with the StoreInfo retrieved + * from the source ResourceInfo. When the source ResourceInfo is a secure-wrapped instance, this process should + * not continually nest secure {@link Wrapper}s around the StoreInfo instance. If it does, a + * {@link java.lang.StackOverflowError} could result. + * @param source A secure wrapped ResourceInfo instance with a non-null StoreInfo attribute. + * @param target A secure wrapped ResourceInfo instance in which to store the StoreInfo retrieved from the source. + * @return A Thread instance that will repeated call target.setStore(source.getStore()); + */ + private Thread getRoundTripThread(final S source, final S target) { + // This is just a simple thread that will loop a bunch of times copying the info onto itself. + // If the info is secured, and the copy causes nested Secure wrappings of the data or its attributes, this will + // eventually throw a StackOverflowError. + final Runnable runnable = new Runnable() { + @Override + public void run() { + for (int i = 0; i < getStackOverflowCount(); ++i) { + target.setStore(source.getStore()); + } + } + }; + // use a very small stack size so the stack overflow happens quickly if it's going to happen. + // this may not fail on all platforms if set/get is broken however, as some platforms may + // ignore the stack size in the Thread constructor. + return new Thread(Thread.currentThread().getThreadGroup(), runnable, "RoundTripThread", 5); + } + + @Test + public void testSecureWrapping() throws Exception { + // get a delegate + final D delegate = createDelegate(); + // assert the delegate is not secured + assertFalse("ResourceInfo delegate should not be Secured", + getSecuredDecoratorClass().isAssignableFrom(delegate.getClass())); + // create a Secure wrapped instance + S secured = createSecuredDecorator(delegate); + assertTrue("ResourceInfo delegate should be Secured", + getSecuredDecoratorClass().isAssignableFrom(secured.getClass())); + // get the StoreInfo + final StoreInfo securedStore = secured.getStore(); + assertTrue("Secured ResourceInfo should return a Secured StoreInfo", + getSecuredStoreInfoClass().isAssignableFrom(securedStore.getClass())); + // copy non secured into secured + Thread roundTripThread = getRoundTripThread(secured, secured); + // catch Errors + final StringWriter sw = new StringWriter(); + roundTripThread.setUncaughtExceptionHandler(new Thread.UncaughtExceptionHandler() { + @Override + public void uncaughtException(Thread t, Throwable e) { + // print the stack to the StringWriter + e.printStackTrace(new PrintWriter(sw, true)); + } + }); + // start the thread and wait for it to finish + roundTripThread.start(); + roundTripThread.join(); + // If there was an Error in the thread, the StringWriter will have it + StringBuffer buffer = sw.getBuffer(); + if (buffer.length() > 0) { + fail(buffer.toString()); + } + // just in case, unwrap the StoreInfo and ensure it doesn't throw a StackOverflow + try { + SecureCatalogImpl.unwrap(secured.getStore()); + } catch (Throwable t) { + t.printStackTrace(new PrintWriter(sw, true)); + } + buffer = sw.getBuffer(); + if (buffer.length() > 0) { + fail(buffer.toString()); + } + } +} diff --git a/src/main/src/test/java/org/geoserver/security/decorators/SecuredWMSLayerInfoTest.java b/src/main/src/test/java/org/geoserver/security/decorators/SecuredWMSLayerInfoTest.java new file mode 100644 index 00000000000..d742e0a6276 --- /dev/null +++ b/src/main/src/test/java/org/geoserver/security/decorators/SecuredWMSLayerInfoTest.java @@ -0,0 +1,45 @@ +/* (c) 2018 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.decorators; + +import org.geoserver.catalog.WMSLayerInfo; +import org.geoserver.catalog.impl.WMSLayerInfoImpl; +import org.geoserver.catalog.impl.WMSStoreInfoImpl; + +public class SecuredWMSLayerInfoTest extends SecuredResourceInfoTest { + + @Override + WMSLayerInfo createDelegate() { + WMSLayerInfo info = new WMSLayerInfoImpl(getCatalog()); + info.setStore(new WMSStoreInfoImpl(getCatalog())); + return info; + } + + @Override + Class getDelegateClass() { + return WMSLayerInfo.class; + } + + @Override + SecuredWMSLayerInfo createSecuredDecorator(WMSLayerInfo delegate) { + return new SecuredWMSLayerInfo(delegate, policy); + } + + @Override + Class getSecuredDecoratorClass() { + return SecuredWMSLayerInfo.class; + } + + @Override + Class getSecuredStoreInfoClass() { + return SecuredWMSStoreInfo.class; + } + + @Override + int getStackOverflowCount() { + return 50_000; + } + +} diff --git a/src/main/src/test/java/org/geoserver/security/decorators/SecuredWMTSLayerInfoTest.java b/src/main/src/test/java/org/geoserver/security/decorators/SecuredWMTSLayerInfoTest.java new file mode 100644 index 00000000000..ba9933935fe --- /dev/null +++ b/src/main/src/test/java/org/geoserver/security/decorators/SecuredWMTSLayerInfoTest.java @@ -0,0 +1,45 @@ +/* (c) 2018 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.decorators; + +import org.geoserver.catalog.WMTSLayerInfo; +import org.geoserver.catalog.impl.WMTSLayerInfoImpl; +import org.geoserver.catalog.impl.WMTSStoreInfoImpl; + +public class SecuredWMTSLayerInfoTest extends SecuredResourceInfoTest { + + @Override + WMTSLayerInfo createDelegate() { + WMTSLayerInfo info = new WMTSLayerInfoImpl(getCatalog()); + info.setStore(new WMTSStoreInfoImpl(getCatalog())); + return info; + } + + @Override + Class getDelegateClass() { + return WMTSLayerInfo.class; + } + + @Override + SecuredWMTSLayerInfo createSecuredDecorator(WMTSLayerInfo delegate) { + return new SecuredWMTSLayerInfo(delegate, policy); + } + + @Override + Class getSecuredDecoratorClass() { + return SecuredWMTSLayerInfo.class; + } + + @Override + Class getSecuredStoreInfoClass() { + return SecuredWMTSStoreInfo.class; + } + + @Override + int getStackOverflowCount() { + return 50_000; + } + +}