[icedtea-web] RFC: Patch for PR861: Code in a jar cannot connect to jar host server
Jiri Vanek
jvanek at redhat.com
Tue Jun 5 07:05:03 PDT 2012
On 06/05/2012 03:53 PM, Deepak Bhole wrote:
> * Jiri Vanek<jvanek at redhat.com> [2012-06-04 06:39]:
>> On 05/28/2012 07:16 PM, Deepak Bhole wrote:
>>> Hi,
>>>
>>> Attached patch resolves PR861:
>>> http://icedtea.classpath.org/bugzilla/show_bug.cgi?id=861
>>>
>>> ChangeLog:
>>> 2012-05-28 Deepak Bhole<dbhole at redhat.com>
>>>
>>> PR861: Allow loading from non codebase hosts. Allow code to connect to
>>> hosting server.
>>> * netx/net/sourceforge/jnlp/SecurityDesc.java
>>> (getSandBoxPermissions): Only add host if it is not empty.
>>> * netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java
>>> (getPermissions): Add SocketPermission for code source host.
>>> (findLoadedClassAll): Call super methods privileged so that connection to
>>> non codebase hosts can be made.
>>> (findClass): Same.
>>> (findResourcesBySearching): Same. Also use privileged context for enum
>>> operations because the enum is defined on the fly by URLClassLoader and
>>> checks for hosting server connectivity via next().
>>> (getAccessControlContextForClassLoading): New method. Returns a control
>>> context for classloader operations like find/load/etc.
>>> (CodeBaseClassLoader::findClass): Call super methods privileged so that
>>> connection to non codebase hosts can be made.
>>> (CodeBaseClassLoader::findResource): Same.
>>>
>>> Applets may specify jars from different host servers. The current code only
>>> allows socket connect permissions for codebase. This is insufficient as a class
>> >from one jar may need a class from another jar that is hosted on a different
>>> server.
>>>
>>> Furthermore, if a class comes from server foo, it should be allowed to connect
>>> to foo throughout its lifetime.
>>>
>>> Attached patch resolves the above issues and makes the site mentioned in the
>>> bug (http://broadband.mpi-sws.org/transparency/glasnost.php) work again.
>>>
>>> OK for HEAD?
>>>
>>> Cheers,
>>> Deepak
>>
>> Hi!
>> I have walked across the code and looks ok.
>> Two questions inline.
>>
>> Btw - can you suggest me a bit How I can write an reproducer?
>>
>> There is ability in current framework to run several wirtual servers on "different urls" by different ports.
>> So you can run application from localhost:1234 and downlaod resource from localhsot:56789. Will this be enough for this testing ( I will elaborate on this reproducers)
>>
>
> Hi Jiri,
>
> Different URLs will not be enough unfortunately. We will need different
> DNSs to reproduce this :(
ugh. Thats an challenge...
>
> Is the patch itself OK to push btw?
sure
>
> Cheers,
> Deepak
>
>>>
>>>
>>> PR861.patch
>>>
>>>
>>> diff -r 6df151bb5320 NEWS
>>> --- a/NEWS Fri May 25 11:44:13 2012 -0400
>>> +++ b/NEWS Mon May 28 13:11:12 2012 -0400
>>> @@ -15,6 +15,7 @@
>>> * Plugin
>>> - PR820: IcedTea-Web 1.1.3 crashing Firefox when loading Citrix XenApp
>>> - PR895: IcedTea-Web searches for missing classes on each loadClass or findClass
>>> + - PR861: Allow loading from non codebase hosts. Allow code to connect to hosting server.
>>> * Common
>>> - PR918: java applet windows uses a low resulution black/white icon
>>>
>>> diff -r 6df151bb5320 netx/net/sourceforge/jnlp/SecurityDesc.java
>>> --- a/netx/net/sourceforge/jnlp/SecurityDesc.java Fri May 25 11:44:13 2012 -0400
>>> +++ b/netx/net/sourceforge/jnlp/SecurityDesc.java Mon May 28 13:11:12 2012 -0400
>>> @@ -238,7 +238,7 @@
>>> for (int i = 0; i< jnlpRIAPermissions.length; i++)
>>> permissions.add(jnlpRIAPermissions[i]);
>>>
>>> - if (downloadHost != null)
>>> + if (downloadHost != null&& downloadHost.length()> 0)
>>> permissions.add(new SocketPermission(downloadHost,
>>> "connect, accept"));
>>
>> You are adding privileges - shouldn't application request for SocketPermission itself?
>>>
>>> diff -r 6df151bb5320 netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java
>>> --- a/netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java Fri May 25 11:44:13 2012 -0400
>>> +++ b/netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java Mon May 28 13:11:12 2012 -0400
>>> @@ -25,9 +25,11 @@
>>> import java.io.InputStream;
>>> import java.io.InputStreamReader;
>>> import java.net.MalformedURLException;
>>> +import java.net.SocketPermission;
>>> import java.net.URL;
>>> import java.net.URLClassLoader;
>>> import java.security.AccessControlContext;
>>> +import java.security.AccessControlException;
>>> import java.security.AccessController;
>>> import java.security.AllPermission;
>>> import java.security.CodeSource;
>>> @@ -35,9 +37,12 @@
>>> import java.security.PermissionCollection;
>>> import java.security.Permissions;
>>> import java.security.PrivilegedAction;
>>> +import java.security.PrivilegedActionException;
>>> import java.security.PrivilegedExceptionAction;
>>> +import java.security.ProtectionDomain;
>>> import java.util.ArrayList;
>>> import java.util.Arrays;
>>> +import java.util.Collection;
>>> import java.util.Collections;
>>> import java.util.Enumeration;
>>> import java.util.HashMap;
>>> @@ -948,6 +953,11 @@
>>> result.add(runtimePermissions.get(i));
>>> }
>>>
>>> + // Class from host X should be allowed to connect to host X
>>> + if (cs.getLocation().getHost().length()> 0)
>>> + result.add(new SocketPermission(cs.getLocation().getHost(),
>>> + "connect, accept"));
>>> +
>>> return result;
>>> } catch (RuntimeException ex) {
>>> if (JNLPRuntime.isDebug()) {
>>> @@ -1316,10 +1326,21 @@
>>> for (int i = 0; i< loaders.length; i++) {
>>> Class result = null;
>>>
>>> - if (loaders[i] == this)
>>> - result = super.findLoadedClass(name);
>>> - else
>>> + if (loaders[i] == this) {
>>> + final String fName = name;
>>> + try {
>>> + result = AccessController.doPrivileged(
>>> + new PrivilegedExceptionAction<Class<?>>() {
>>> + public Class<?> run() {
>>> + return JNLPClassLoader.super.findLoadedClass(fName);
>>> + }
>>> + }, getAccessControlContextForClassLoading());
>>> + } catch (PrivilegedActionException pae) {
>>> + result = null;
>>> + }
>>> + } else {
>>> result = loaders[i].findLoadedClassAll(name);
>>> + }
>>>
>>> if (result != null)
>>> return result;
>>> @@ -1517,12 +1538,20 @@
>>> protected Class findClass(String name) throws ClassNotFoundException {
>>> for (int i = 0; i< loaders.length; i++) {
>>> try {
>>> - if (loaders[i] == this)
>>> - return super.findClass(name);
>>> - else
>>> + if (loaders[i] == this) {
>>> + final String fName = name;
>>> + return AccessController.doPrivileged(
>>> + new PrivilegedExceptionAction<Class<?>>() {
>>> + public Class<?> run() throws ClassNotFoundException {
>>> + return JNLPClassLoader.super.findClass(fName);
>>> + }
>>> + }, getAccessControlContextForClassLoading());
>>> + } else {
>>> return loaders[i].findClass(name);
>>> + }
>>> } catch (ClassNotFoundException ex) {
>>> } catch (ClassFormatError cfe) {
>>> + } catch (PrivilegedActionException pae) {
>>> }
>>
>> Although unrelated this consumed exception scares me.
>>> }
>>>
>>> @@ -1631,20 +1660,42 @@
>>> */
>>> private Enumeration<URL> findResourcesBySearching(String name) throws IOException {
>>> List<URL> resources = new ArrayList<URL>();
>>> - Enumeration<URL> e;
>>> + Enumeration<URL> e = null;
>>>
>>> for (int i = 0; i< loaders.length; i++) {
>>> // TODO check if this will blow up or not
>>> // if loaders[1].getResource() is called, wont it call getResource() on
>>> // the original caller? infinite recursion?
>>>
>>> - if (loaders[i] == this)
>>> - e = super.findResources(name);
>>> - else
>>> + if (loaders[i] == this) {
>>> + final String fName = name;
>>> + try {
>>> + e = AccessController.doPrivileged(
>>> + new PrivilegedExceptionAction<Enumeration<URL>>() {
>>> + public Enumeration<URL> run() throws IOException {
>>> + return JNLPClassLoader.super.findResources(fName);
>>> + }
>>> + }, getAccessControlContextForClassLoading());
>>> + } catch (PrivilegedActionException pae) {
>>> + }
>>> + } else {
>>> e = loaders[i].findResources(name);
>>> + }
>>>
>>> - while (e.hasMoreElements())
>>> - resources.add(e.nextElement());
>>> + final Enumeration<URL> fURLEnum = e;
>>> + try {
>>> + resources.addAll(AccessController.doPrivileged(
>>> + new PrivilegedExceptionAction<Collection<URL>>() {
>>> + public Collection<URL> run() {
>>> + List<URL> resources = new ArrayList<URL>();
>>> + while (fURLEnum != null&& fURLEnum.hasMoreElements()) {
>>> + resources.add(fURLEnum.nextElement());
>>> + }
>>> + return resources;
>>> + }
>>> + }, getAccessControlContextForClassLoading()));
>>> + } catch (PrivilegedActionException pae) {
>>> + }
>>> }
>>>
>>> // Add resources from codebase (only if nothing was found above,
>>> @@ -1896,6 +1947,56 @@
>>> }
>>> }
>>>
>>> + /**
>>> + * Returns an appropriate AccessControlContext for loading classes in
>>> + * the running instance.
>>> + *
>>> + * The default context during class-loading only allows connection to
>>> + * codebase. However applets are allowed to load jars from arbitrary
>>> + * locations and the codebase only access falls short if a class from
>>> + * one location needs a class from another.
>>> + *
>>> + * Given protected access since CodeBaseClassloader uses this function too.
>>> + *
>>> + * @return The appropriate AccessControlContext for loading classes for this instance
>>> + */
>>> + public AccessControlContext getAccessControlContextForClassLoading() {
>>> + AccessControlContext context = AccessController.getContext();
>>> +
>>> + try {
>>> + context.checkPermission(new AllPermission());
>>> + return context; // If context already has all permissions, don't bother
>>> + } catch (AccessControlException ace) {
>>> + // continue below
>>> + }
>>> +
>>> + // Since this is for class-loading, technically any class from one jar
>>> + // should be able to access a class from another, therefore making the
>>> + // original context code source irrelevant
>>> + PermissionCollection permissions = this.security.getSandBoxPermissions();
>>> +
>>> + // Local cache access permissions
>>> + for (Permission resourcePermission : resourcePermissions) {
>>> + permissions.add(resourcePermission);
>>> + }
>>> +
>>> + // Permissions for all remote hosting urls
>>> + for (URL u: jarLocationSecurityMap.keySet()) {
>>> + permissions.add(new SocketPermission(u.getHost(),
>>> + "connect, accept"));
>>> + }
>>> +
>>> + // Permissions for codebase urls
>>> + for (URL u : codeBaseLoader.getURLs()) {
>>> + permissions.add(new SocketPermission(u.getHost(),
>>> + "connect, accept"));
>>> + }
>>> +
>>> + ProtectionDomain pd = new ProtectionDomain(null, permissions);
>>> +
>>> + return new AccessControlContext(new ProtectionDomain[] { pd });
>>> + }
>>> +
>>> /*
>>> * Helper class to expose protected URLClassLoader methods.
>>> */
>>> @@ -1927,10 +2028,16 @@
>>> throw new ClassNotFoundException(name);
>>>
>>> try {
>>> - return super.findClass(name);
>>> - } catch (ClassNotFoundException cnfe) {
>>> + final String fName = name;
>>> + return AccessController.doPrivileged(
>>> + new PrivilegedExceptionAction<Class<?>>() {
>>> + public Class<?> run() throws ClassNotFoundException {
>>> + return CodeBaseClassLoader.super.findClass(fName);
>>> + }
>>> + }, parentJNLPClassLoader.getAccessControlContextForClassLoading());
>>> + } catch (PrivilegedActionException pae) {
>>> notFoundResources.put(name, super.getURLs());
>>> - throw cnfe;
>>> + throw new ClassNotFoundException("Could not find class " + name);
>>> }
>>> }
>>>
>>> @@ -1983,8 +2090,18 @@
>>> if (Arrays.equals(super.getURLs(), notFoundResources.get(name)))
>>> return null;
>>>
>>> + URL url = null;
>>> if (!name.startsWith("META-INF")) {
>>> - URL url = super.findResource(name);
>>> + try {
>>> + final String fName = name;
>>> + url = AccessController.doPrivileged(
>>> + new PrivilegedExceptionAction<URL>() {
>>> + public URL run() {
>>> + return CodeBaseClassLoader.super.findResource(fName);
>>> + }
>>> + }, parentJNLPClassLoader.getAccessControlContextForClassLoading());
>>> + } catch (PrivilegedActionException pae) {
>>> + }
>>>
>>> if (url == null) {
>>> notFoundResources.put(name, super.getURLs());
>>
>> Thanx for patch!
>> J.
More information about the distro-pkg-dev
mailing list