From 50aeb4156920310e17c3a9a46f34b8514b4b7054 Mon Sep 17 00:00:00 2001 From: Karl Pauls Date: Thu, 8 Nov 2018 21:23:52 +0000 Subject: [PATCH] FELIX-5978: Ensure getClassLoader() is called in a safe way when security is enabled - patched provided by Tim Ward - thanks! (This closes #160) git-svn-id: https://svn.apache.org/repos/asf/felix/trunk@1846198 13f79535-47bb-0310-9956-ffa450edef68 --- .../framework/BundleProtectionDomain.java | 155 ++++++++---------- .../org/apache/felix/framework/Felix.java | 6 +- .../apache/felix/framework/URLHandlers.java | 9 +- .../URLHandlersStreamHandlerProxy.java | 19 ++- .../felix/framework/util/SecureAction.java | 27 +++ .../felix/framework/URLHandlersTest.java | 30 +++- 6 files changed, 142 insertions(+), 104 deletions(-) diff --git a/framework/src/main/java/org/apache/felix/framework/BundleProtectionDomain.java b/framework/src/main/java/org/apache/felix/framework/BundleProtectionDomain.java index 384a9defa3c..9b2b9bf6ad1 100644 --- a/framework/src/main/java/org/apache/felix/framework/BundleProtectionDomain.java +++ b/framework/src/main/java/org/apache/felix/framework/BundleProtectionDomain.java @@ -26,6 +26,7 @@ import java.io.InputStream; import java.io.OutputStream; import java.lang.ref.WeakReference; +import java.lang.reflect.InvocationTargetException; import java.net.JarURLConnection; import java.net.MalformedURLException; import java.net.URL; @@ -53,6 +54,7 @@ import org.osgi.framework.PackagePermission; import org.osgi.framework.wiring.BundleRevision; +import sun.net.www.protocol.file.FileURLConnection; public class BundleProtectionDomain extends ProtectionDomain { @@ -210,8 +212,6 @@ private void readNext(String path) throws IOException } m_output.closeEntry(); - - m_output.flush(); } } @@ -238,100 +238,91 @@ public void write(byte[] buffer, int offset, int length) private static final class RevisionAsJarURL extends URLStreamHandler { private final WeakReference m_revision; + private volatile URL url; private RevisionAsJarURL(BundleRevisionImpl revision) { m_revision = new WeakReference(revision); } - @Override protected URLConnection openConnection(URL u) throws IOException { - return new JarURLConnection(u) + if (url != null) { + return url.openConnection(); + } + BundleRevisionImpl revision = (BundleRevisionImpl) m_revision.get(); + + if (revision != null) { - @Override - public JarFile getJarFile() throws IOException + File target; + Content content = revision.getContent(); + if (content instanceof JarContent) { - BundleRevisionImpl revision = (BundleRevisionImpl) m_revision.get(); - - if (revision != null) + target = ((JarContent) content).getFile(); + } + else + { + target = Felix.m_secureAction.createTempFile("jar", null, null); + Felix.m_secureAction.deleteFileOnExit(target); + FileOutputStream output = null; + InputStream input = null; + IOException rethrow = null; + try { - Content content = revision.getContent(); - if (content instanceof JarContent) + output = new FileOutputStream(target); + input = new BundleInputStream(content); + byte[] buffer = new byte[64 * 1024]; + for (int i = input.read(buffer);i != -1; i = input.read(buffer)) { - return Felix.m_secureAction.openJarFile(((JarContent) content).getFile()); + output.write(buffer,0, i); } - else + } + catch (IOException ex) + { + rethrow = ex; + } + finally + { + if (output != null) { - File target = Felix.m_secureAction.createTempFile("jar", null, null); - Felix.m_secureAction.deleteFileOnExit(target); - FileOutputStream output = null; - InputStream input = null; - IOException rethrow = null; try { - output = new FileOutputStream(target); - input = new BundleInputStream(content); - byte[] buffer = new byte[64 * 1024]; - for (int i = input.read(buffer);i != -1; i = input.read(buffer)) - { - output.write(buffer,0, i); - } + output.close(); } catch (IOException ex) { - rethrow = ex; - } - finally - { - if (output != null) + if (rethrow == null) { - try - { - output.close(); - } - catch (IOException ex) - { - if (rethrow == null) - { - rethrow = ex; - } - } - } - - if (input != null) - { - try - { - input.close(); - } - catch (IOException ex) - { - if (rethrow == null) - { - rethrow = ex; - } - } + rethrow = ex; } + } + } - if (rethrow != null) + if (input != null) + { + try + { + input.close(); + } + catch (IOException ex) + { + if (rethrow == null) { - throw rethrow; + rethrow = ex; } } - return Felix.m_secureAction.openJarFile(target); } - } - throw new IOException("Unable to access bundle revision."); - } - - @Override - public void connect() throws IOException - { + if (rethrow != null) + { + throw rethrow; + } + } } - }; + return (url = new URL("jar:" + target.toURI().toURL() + "!/")).openConnection(); + } + throw new IOException("Unable to access bundle revision."); } private static boolean getUseCachedURL(final BundleRevisionImpl revision) @@ -382,31 +373,25 @@ private static URL create(BundleRevisionImpl revision) throws MalformedURLExcept { location = location.substring("reference:".length()); } - URL url; + try { - url = Felix.m_secureAction.createURL( - Felix.m_secureAction.createURL(null, "jar:", handler), location, null); + return Felix.m_secureAction.createURL( + Felix.m_secureAction.createURL(null, "jar:", handler), + location, + handler + ); } catch (MalformedURLException ex) { - url = null; - } + location = "jar:" + revision.getEntry("/") + "!/"; - if (url != null && !url.getProtocol().equalsIgnoreCase("jar")) - { - return url; - } - else if (url == null) - { - location = "jar:" + revision.getEntry("/") + "!/"; + return Felix.m_secureAction.createURL( + Felix.m_secureAction.createURL(null, "jar:", handler), + location, + handler + ); } - - return Felix.m_secureAction.createURL( - Felix.m_secureAction.createURL(null, "jar:", handler), - location, - handler - ); } } diff --git a/framework/src/main/java/org/apache/felix/framework/Felix.java b/framework/src/main/java/org/apache/felix/framework/Felix.java index cf028cd26a3..46792c51f6f 100644 --- a/framework/src/main/java/org/apache/felix/framework/Felix.java +++ b/framework/src/main/java/org/apache/felix/framework/Felix.java @@ -4023,10 +4023,12 @@ Bundle getBundle(Class clazz) { // If the class comes from bundle class loader, then return // associated bundle if it is from this framework instance. - if (clazz.getClassLoader() instanceof BundleReference) + ClassLoader classLoader = m_secureAction.getClassLoader(clazz); + + if (classLoader instanceof BundleReference) { // Only return the bundle if it is from this framework. - BundleReference br = (BundleReference) clazz.getClassLoader(); + BundleReference br = (BundleReference) classLoader; return ((br.getBundle() instanceof BundleImpl) && (((BundleImpl) br.getBundle()).getFramework() == this)) ? br.getBundle() : null; diff --git a/framework/src/main/java/org/apache/felix/framework/URLHandlers.java b/framework/src/main/java/org/apache/felix/framework/URLHandlers.java index 9f34da52f52..2c327164ef3 100644 --- a/framework/src/main/java/org/apache/felix/framework/URLHandlers.java +++ b/framework/src/main/java/org/apache/felix/framework/URLHandlers.java @@ -685,16 +685,19 @@ else if (attempts++ > 3) Class[] stack = m_sm.getClassContext(); // Find the first class that is loaded from a bundle. Class targetClass = null; + ClassLoader targetClassLoader = null; for (int i = 0; i < stack.length; i++) { - if (stack[i].getClassLoader() != null) + ClassLoader classLoader = m_secureAction.getClassLoader(stack[i]); + if (classLoader != null) { - String name = stack[i].getClassLoader().getClass().getName(); + String name = classLoader.getClass().getName(); if (name.startsWith("org.apache.felix.framework.ModuleImpl$ModuleClassLoader") || name.equals("org.apache.felix.framework.searchpolicy.ContentClassLoader") || name.startsWith("org.apache.felix.framework.BundleWiringImpl$BundleClassLoader")) { targetClass = stack[i]; + targetClassLoader = classLoader; break; } } @@ -705,7 +708,7 @@ else if (attempts++ > 3) // the bundle that loaded the class. if (targetClass != null) { - ClassLoader index = targetClass.getClassLoader().getClass().getClassLoader(); + ClassLoader index = m_secureAction.getClassLoader(targetClassLoader.getClass()); List frameworks = (List) m_classloaderToFrameworkLists.get(index); diff --git a/framework/src/main/java/org/apache/felix/framework/URLHandlersStreamHandlerProxy.java b/framework/src/main/java/org/apache/felix/framework/URLHandlersStreamHandlerProxy.java index 195ec588f25..7208ff9c89a 100644 --- a/framework/src/main/java/org/apache/felix/framework/URLHandlersStreamHandlerProxy.java +++ b/framework/src/main/java/org/apache/felix/framework/URLHandlersStreamHandlerProxy.java @@ -28,6 +28,7 @@ import java.net.URL; import java.net.URLConnection; import java.net.URLStreamHandler; +import java.security.PrivilegedAction; import org.apache.felix.framework.util.SecureAction; import org.osgi.service.url.URLStreamHandlerService; @@ -599,10 +600,11 @@ private Object getStreamHandlerService() { return (URLStreamHandlerService) service; } - return (URLStreamHandlerService) Proxy.newProxyInstance( - URLStreamHandlerService.class.getClassLoader(), - new Class[]{URLStreamHandlerService.class}, - new URLHandlersStreamHandlerProxy(service, m_action)); + + return m_action.createProxy( + m_action.getClassLoader(URLStreamHandlerService.class), + new Class[]{URLStreamHandlerService.class}, + new URLHandlersStreamHandlerProxy(service, m_action)); } catch (ThreadDeath td) { @@ -631,11 +633,10 @@ public Object invoke(Object obj, Method method, Object[] params) } if ("parseURL".equals(method.getName())) { - types[0] = m_service.getClass().getClassLoader().loadClass( - URLStreamHandlerSetter.class.getName()); - params[0] = Proxy.newProxyInstance( - m_service.getClass().getClassLoader(), new Class[]{types[0]}, - (URLHandlersStreamHandlerProxy) params[0]); + ClassLoader loader = m_action.getClassLoader(m_service.getClass()); + types[0] = loader.loadClass(URLStreamHandlerSetter.class.getName()); + params[0] = m_action.createProxy(loader, new Class[]{types[0]}, + (URLHandlersStreamHandlerProxy) params[0]); } return m_action.invokeDirect(m_action.getDeclaredMethod(m_service.getClass(), method.getName(), types), m_service, params); diff --git a/framework/src/main/java/org/apache/felix/framework/util/SecureAction.java b/framework/src/main/java/org/apache/felix/framework/util/SecureAction.java index c0228028ee3..4a202a747ff 100644 --- a/framework/src/main/java/org/apache/felix/framework/util/SecureAction.java +++ b/framework/src/main/java/org/apache/felix/framework/util/SecureAction.java @@ -20,6 +20,7 @@ import java.io.*; import java.lang.reflect.*; +import java.lang.reflect.Proxy; import java.net.*; import java.security.*; import java.util.Collection; @@ -1525,6 +1526,28 @@ public String getCanonicalPath(File dataFile) throws IOException } } + public Object createProxy(ClassLoader classLoader, + Class[] interfaces, InvocationHandler handler) + { + if (System.getSecurityManager() != null) + { + Actions actions = (Actions) m_actions.get(); + actions.set(Actions.CREATE_PROXY, classLoader, interfaces, handler); + try + { + return AccessController.doPrivileged(actions, m_acc); + } + catch (PrivilegedActionException e) + { + throw (RuntimeException) e.getException(); + } + } + else + { + return Proxy.newProxyInstance(classLoader, interfaces, handler); + } + } + private static class Actions implements PrivilegedExceptionAction { public static final int INITIALIZE_CONTEXT_ACTION = 0; @@ -1585,6 +1608,7 @@ private static class Actions implements PrivilegedExceptionAction public static final int DELETE_FILEONEXIT_ACTION = 55; public static final int INVOKE_WOVEN_CLASS_LISTENER = 56; public static final int GET_CANONICAL_PATH = 57; + public static final int CREATE_PROXY = 58; private int m_action = -1; private Object m_arg1 = null; @@ -1845,6 +1869,9 @@ public Object run() throws Exception return null; case GET_CANONICAL_PATH: return ((File) arg1).getCanonicalPath(); + case CREATE_PROXY: + return Proxy.newProxyInstance((ClassLoader)arg1, (Class[])arg2, + (InvocationHandler) arg3); } return null; diff --git a/framework/src/test/java/org/apache/felix/framework/URLHandlersTest.java b/framework/src/test/java/org/apache/felix/framework/URLHandlersTest.java index 0f08c32329e..ec9e4a34b4f 100644 --- a/framework/src/test/java/org/apache/felix/framework/URLHandlersTest.java +++ b/framework/src/test/java/org/apache/felix/framework/URLHandlersTest.java @@ -23,6 +23,7 @@ import java.io.InputStream; import java.net.ContentHandler; import java.net.InetAddress; +import java.net.JarURLConnection; import java.net.URL; import java.net.URLClassLoader; import java.net.URLConnection; @@ -300,7 +301,9 @@ public void start(final BundleContext context) throws Exception new URL("test" + System.identityHashCode(TestURLHandlersActivator.this) + ":").openConnection(); - + if (!(getClass().getProtectionDomain().getCodeSource().getLocation().openConnection() instanceof JarURLConnection)) { + throw new Exception("Unexpted Code Source"); + } try { @@ -349,14 +352,19 @@ public void start(final BundleContext context) throws Exception reg.unregister(); } + boolean fail; try { new URL("test" + System.identityHashCode(TestURLHandlersActivator.this) + ":").openConnection(); - throw new Exception("Unexpected url resolve"); + fail = true; } catch (Exception ex) { // pass + fail = false; + } + if (fail) { + throw new Exception("Unexpected url resolve"); } Bundle bundle2 = null; @@ -376,36 +384,48 @@ public void start(final BundleContext context) throws Exception } if (bundle2 != null) { + fail = false; try { new URL("test" + System.identityHashCode(bundle2) + ":").openConnection(); - throw new Exception("Unexpected url2 resolve"); + fail = true; } catch (Exception ex) { } + if (fail) { + throw new Exception("Unexpected url resolve"); + } bundle2.start(); new URL("test" + System.identityHashCode(bundle2) + ":").openConnection(); bundle2.stop(); + fail = false; try { new URL("test" + System.identityHashCode(bundle2) + ":").openConnection(); - throw new Exception("Unexpected url2 resolve"); + fail = true; } catch (Exception ex) { } + if (fail) { + throw new Exception("Unexpected url resolve"); + } } else { + fail = false; try { new URL("test" + System.identityHashCode(context.getBundle()) + ":").openConnection(); - throw new Exception("Unexpected url2 resolve"); + fail = true; } catch (Exception ex) { } + if (fail) { + throw new Exception("Unexpected url2 resolve"); + } props = new Hashtable(); props.put(URLConstants.URL_HANDLER_PROTOCOL, "test" + System.identityHashCode(context.getBundle())); m_reg = context.registerService(URLStreamHandlerService.class, this, props);