From d6b5600afe75e1086dd564344e1d085966e4237d Mon Sep 17 00:00:00 2001 From: Mark Thomas Date: Mon, 22 Aug 2016 21:00:21 +0000 Subject: [PATCH] When adding and removing ResourceLinks dynamically, ensure that the global resource is only visible via the ResourceLinkFactory when it is meant to be. git-svn-id: https://svn.apache.org/repos/asf/tomcat/tc8.5.x/trunk@1757272 13f79535-47bb-0310-9956-ffa450edef68 --- .../catalina/core/NamingContextListener.java | 19 ++++ .../naming/factory/ResourceLinkFactory.java | 61 +++++++++++++ test/org/apache/naming/TestNamingContext.java | 87 +++++++++++++++++++ webapps/docs/changelog.xml | 5 ++ 4 files changed, 172 insertions(+) create mode 100644 test/org/apache/naming/TestNamingContext.java diff --git a/java/org/apache/catalina/core/NamingContextListener.java b/java/org/apache/catalina/core/NamingContextListener.java index b96abf7d062f..971c1c0cf6ac 100644 --- a/java/org/apache/catalina/core/NamingContextListener.java +++ b/java/org/apache/catalina/core/NamingContextListener.java @@ -40,6 +40,7 @@ import org.apache.catalina.ContainerEvent; import org.apache.catalina.ContainerListener; import org.apache.catalina.Context; +import org.apache.catalina.Engine; import org.apache.catalina.Host; import org.apache.catalina.Lifecycle; import org.apache.catalina.LifecycleEvent; @@ -58,6 +59,7 @@ import org.apache.naming.ResourceRef; import org.apache.naming.ServiceRef; import org.apache.naming.TransactionRef; +import org.apache.naming.factory.ResourceLinkFactory; import org.apache.tomcat.util.descriptor.web.ContextEjb; import org.apache.tomcat.util.descriptor.web.ContextEnvironment; import org.apache.tomcat.util.descriptor.web.ContextHandler; @@ -316,6 +318,11 @@ public void lifecycleEvent(LifecycleEvent event) { registry.unregisterComponent(objectName); } } + + javax.naming.Context global = getGlobalNamingContext(); + if (global != null) { + ResourceLinkFactory.deregisterGlobalResourceAccess(global); + } } finally { objectNames.clear(); @@ -1152,6 +1159,17 @@ public void addResourceLink(ContextResourceLink resourceLink) { log.error(sm.getString("naming.bindFailed", e)); } + ResourceLinkFactory.registerGlobalResourceAccess( + getGlobalNamingContext(), resourceLink.getName(), resourceLink.getGlobal()); + } + + + private javax.naming.Context getGlobalNamingContext() { + if (container instanceof Context) { + Engine e = (Engine) ((Context) container).getParent().getParent(); + return e.getService().getServer().getGlobalNamingContext(); + } + return null; } @@ -1269,6 +1287,7 @@ public void removeResourceLink(String name) { log.error(sm.getString("naming.unbindFailed", e)); } + ResourceLinkFactory.deregisterGlobalResourceAccess(getGlobalNamingContext(), name); } diff --git a/java/org/apache/naming/factory/ResourceLinkFactory.java b/java/org/apache/naming/factory/ResourceLinkFactory.java index 006e6f4d7612..b68c4d58802b 100644 --- a/java/org/apache/naming/factory/ResourceLinkFactory.java +++ b/java/org/apache/naming/factory/ResourceLinkFactory.java @@ -16,7 +16,10 @@ */ package org.apache.naming.factory; +import java.util.HashMap; import java.util.Hashtable; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; import javax.naming.Context; import javax.naming.Name; @@ -41,6 +44,8 @@ public class ResourceLinkFactory implements ObjectFactory { */ private static Context globalContext = null; + private static Map> globalResourceRegistrations = + new ConcurrentHashMap<>(); // --------------------------------------------------------- Public Methods @@ -59,6 +64,56 @@ public static void setGlobalContext(Context newGlobalContext) { } + public static void registerGlobalResourceAccess(Context globalContext, String localName, + String globalName) { + validateGlobalContext(globalContext); + ClassLoader cl = Thread.currentThread().getContextClassLoader(); + Map registrations = globalResourceRegistrations.get(cl); + if (registrations == null) { + // Web application initialization is single threaded so this is + // safe. + registrations = new HashMap<>(); + globalResourceRegistrations.put(cl, registrations); + } + registrations.put(localName, globalName); + } + + + public static void deregisterGlobalResourceAccess(Context globalContext, String localName) { + validateGlobalContext(globalContext); + ClassLoader cl = Thread.currentThread().getContextClassLoader(); + Map registrations = globalResourceRegistrations.get(cl); + if (registrations != null) { + registrations.remove(localName); + } + } + + + public static void deregisterGlobalResourceAccess(Context globalContext) { + validateGlobalContext(globalContext); + ClassLoader cl = Thread.currentThread().getContextClassLoader(); + globalResourceRegistrations.remove(cl); + } + + + private static void validateGlobalContext(Context globalContext) { + if (ResourceLinkFactory.globalContext != null && + ResourceLinkFactory.globalContext != globalContext) { + throw new SecurityException("Caller provided invalid global context"); + } + } + + + private static boolean validateGlobalResourceAccess(String globalName) { + ClassLoader cl = Thread.currentThread().getContextClassLoader(); + Map registrations = globalResourceRegistrations.get(cl); + if (registrations != null && registrations.containsValue(globalName)) { + return true; + } + return false; + } + + // -------------------------------------------------- ObjectFactory Methods /** @@ -82,6 +137,12 @@ public Object getObjectInstance(Object obj, Name name, Context nameCtx, RefAddr refAddr = ref.get(ResourceLinkRef.GLOBALNAME); if (refAddr != null) { globalName = refAddr.getContent().toString(); + // When running under a security manager confirm that the current + // web application has really been configured to access the specified + // global resource + if (!validateGlobalResourceAccess(globalName)) { + return null; + } Object result = null; result = globalContext.lookup(globalName); // Check the expected type diff --git a/test/org/apache/naming/TestNamingContext.java b/test/org/apache/naming/TestNamingContext.java new file mode 100644 index 000000000000..cd95b00f668d --- /dev/null +++ b/test/org/apache/naming/TestNamingContext.java @@ -0,0 +1,87 @@ +package org.apache.naming; + +import javax.naming.Context; +import javax.naming.NamingException; + +import org.apache.catalina.startup.Tomcat; +import org.apache.catalina.startup.TomcatBaseTest; +import org.apache.naming.factory.ResourceLinkFactory; +import org.apache.tomcat.util.descriptor.web.ContextEnvironment; +import org.apache.tomcat.util.descriptor.web.ContextResourceLink; +import org.junit.Assert; +import org.junit.Test; + +public class TestNamingContext extends TomcatBaseTest { + + private static final String COMP_ENV = "comp/env"; + private static final String GLOBAL_NAME = "global"; + private static final String LOCAL_NAME = "local"; + private static final String DATA = "Cabbage"; + + + @Test + public void testGlobalNaming() throws Exception { + Tomcat tomcat = getTomcatInstance(); + tomcat.enableNaming(); + + org.apache.catalina.Context ctx = tomcat.addContext("", null); + + tomcat.start(); + + Context webappInitial = ContextBindings.getContext(ctx); + + // Nothing added at the moment so should be null + Object obj = doLookup(webappInitial, COMP_ENV + "/" + LOCAL_NAME); + Assert.assertNull(obj); + + ContextEnvironment ce = new ContextEnvironment(); + ce.setName(GLOBAL_NAME); + ce.setValue(DATA); + ce.setType(DATA.getClass().getName()); + + tomcat.getServer().getGlobalNamingResources().addEnvironment(ce); + + // No link so still should be null + obj = doLookup(webappInitial, COMP_ENV + "/" + LOCAL_NAME); + Assert.assertNull(obj); + + // Now add a resource link to the context + ContextResourceLink crl = new ContextResourceLink(); + crl.setGlobal(GLOBAL_NAME); + crl.setName(LOCAL_NAME); + crl.setType(DATA.getClass().getName()); + ctx.getNamingResources().addResourceLink(crl); + + // Link exists so should be OK now + obj = doLookup(webappInitial, COMP_ENV + "/" + LOCAL_NAME); + Assert.assertEquals(DATA, obj); + + // Try shortcut + ResourceLinkFactory factory = new ResourceLinkFactory(); + ResourceLinkRef rlr = new ResourceLinkRef(DATA.getClass().getName(), GLOBAL_NAME, null, null); + obj = factory.getObjectInstance(rlr, null, null, null); + Assert.assertEquals(DATA, obj); + + // Remove the link + ctx.getNamingResources().removeResourceLink(LOCAL_NAME); + + // No link so should be null + obj = doLookup(webappInitial, COMP_ENV + "/" + LOCAL_NAME); + Assert.assertNull(obj); + + // Shortcut should fail too + obj = factory.getObjectInstance(rlr, null, null, null); + Assert.assertNull(obj); + } + + + private Object doLookup(Context context, String name) { + Object result = null; + try { + result = context.lookup(name); + } catch (NamingException nnfe) { + // Ignore + } + return result; + } +} diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index 0a7040b11640..8a64c6c93b0d 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -153,6 +153,11 @@ >CVE-2016-5388) by default and to provide a mechanism that can be used to mitigate any future, similar issues. (markt) + + When adding and removing ResourceLinks dynamically, ensure + that the global resource is only visible via the + ResourceLinkFactory when it is meant to be. (markt) +