Skip to content

[FELIX-5978] Ensure getClassLoader() is called in a safe way when sec… #160

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from
Closed

[FELIX-5978] Ensure getClassLoader() is called in a safe way when sec… #160

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e30d80a
[FELIX-5978] Ensure getClassLoader() is called in a safe way when sec…
timothyjward Nov 8, 2018
a25f0c8
[FELIX-5978] Ensure that getClassLoader and proxying is security-safe
timothyjward Nov 8, 2018
5fe93d1
[FELIX-5978] Avoid making calls into the URL handling stack when defi…
timothyjward Nov 8, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
155 changes: 70 additions & 85 deletions framework/src/main/java/org/apache/felix/framework/BundleProtectionDomain.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
{
Expand Down Expand Up @@ -210,8 +212,6 @@ private void readNext(String path) throws IOException
}

m_output.closeEntry();

m_output.flush();
}
}

Expand All @@ -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)
Expand Down Expand Up @@ -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
);
}
}

Expand Down
6 changes: 4 additions & 2 deletions framework/src/main/java/org/apache/felix/framework/Felix.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
9 changes: 6 additions & 3 deletions framework/src/main/java/org/apache/felix/framework/URLHandlers.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Expand All @@ -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);

Expand Down
19 changes: 10 additions & 9 deletions framework/src/main/java/org/apache/felix/framework/URLHandlersStreamHandlerProxy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)
{
Expand Down Expand Up @@ -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);
Expand Down
27 changes: 27 additions & 0 deletions framework/src/main/java/org/apache/felix/framework/util/SecureAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
Loading