[icedtea-web] RFC: Patch for PR861: Code in a jar cannot connect to jar host server

Jiri Vanek jvanek at redhat.com
Mon Jun 4 03:43:14 PDT 2012


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)

>
>
> 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