/hg/icedtea-web: PR861: Allow loading from non codebase hosts. A...
Deepak Bhole
dbhole at redhat.com
Wed Jun 6 10:51:36 PDT 2012
* Jiri Vanek <jvanek at redhat.com> [2012-06-06 09:31]:
> Hi!
>
> This patch have broken most of javaws reproducers. I'm suggesting to
> revert it until it is discovered why. Blame is on my head.
> Sorry.
>
Hi Jiri,
Thanks for spotting it. Sorry for missing it myself too :/ I will make
sure I run the tests with a proper clean tree from now before push.
I found the error, please see attached patch.
ChangeLog:
2012-06-06 Deepak Bhole <dbhole at redhat.com>
* netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java
(getAccessControlContextForClassLoading): Iterate over codebase URLs only
if codeBaseLoader is not null.
After this, the pass rate is back to same as before.
OK for push?
Thanks,
Deepak
> J.
> On 06/05/2012 05:00 PM, dbhole at icedtea.classpath.org wrote:
> >changeset 954131311826 in /hg/icedtea-web
> >details: http://icedtea.classpath.org/hg/icedtea-web?cmd=changeset;node=954131311826
> >author: Deepak Bhole<dbhole at redhat.com>
> >date: Tue Jun 05 10:11:09 2012 -0400
> >
> > PR861: Allow loading from non codebase hosts. Allow code to connect to hosting server
> >
> >
> >diffstat:
> >
> > ChangeLog | 20 ++
> > NEWS | 1 +
> > netx/net/sourceforge/jnlp/SecurityDesc.java | 2 +-
> > netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java | 149 +++++++++++++++-
> > 4 files changed, 155 insertions(+), 17 deletions(-)
> >
> >diffs (285 lines):
> >
> >diff -r 67c462561ee3 -r 954131311826 ChangeLog
> >--- a/ChangeLog Tue Jun 05 16:39:27 2012 +0200
> >+++ b/ChangeLog Tue Jun 05 10:11:09 2012 -0400
> >@@ -1,3 +1,23 @@
> >+2012-06-05 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.
> >+
> > 2012-06-05 Jiri Vanek<jvanek at redhat.com>
> >
> > * tests/netx/jnlp_testsengine/net/sourceforge/jnlp/annotations/KnownToFail.java
> >diff -r 67c462561ee3 -r 954131311826 NEWS
> >--- a/NEWS Tue Jun 05 16:39:27 2012 +0200
> >+++ b/NEWS Tue Jun 05 10:11:09 2012 -0400
> >@@ -16,6 +16,7 @@
> > - PR820: IcedTea-Web 1.1.3 crashing Firefox when loading Citrix XenApp
> > - PR863: Error passing strings to applet methods in Chromium
> > - 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 67c462561ee3 -r 954131311826 netx/net/sourceforge/jnlp/SecurityDesc.java
> >--- a/netx/net/sourceforge/jnlp/SecurityDesc.java Tue Jun 05 16:39:27 2012 +0200
> >+++ b/netx/net/sourceforge/jnlp/SecurityDesc.java Tue Jun 05 10:11:09 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"));
> >
> >diff -r 67c462561ee3 -r 954131311826 netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java
> >--- a/netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java Tue Jun 05 16:39:27 2012 +0200
> >+++ b/netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java Tue Jun 05 10:11:09 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;
> >@@ -952,6 +957,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()) {
> >@@ -1320,10 +1330,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;
> >@@ -1521,12 +1542,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) {
> > }
> > }
> >
> >@@ -1635,20 +1664,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,
> >@@ -1900,6 +1951,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.
> > */
> >@@ -1931,10 +2032,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);
> > }
> > }
> >
> >@@ -1987,8 +2094,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());
>
-------------- next part --------------
diff -r 954131311826 netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java
--- a/netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java Tue Jun 05 10:11:09 2012 -0400
+++ b/netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java Wed Jun 06 13:47:48 2012 -0400
@@ -1990,10 +1990,12 @@
"connect, accept"));
}
- // Permissions for codebase urls
- for (URL u : codeBaseLoader.getURLs()) {
- permissions.add(new SocketPermission(u.getHost(),
- "connect, accept"));
+ // Permissions for codebase urls (if there is a loader)
+ if (codeBaseLoader != null) {
+ for (URL u : codeBaseLoader.getURLs()) {
+ permissions.add(new SocketPermission(u.getHost(),
+ "connect, accept"));
+ }
}
ProtectionDomain pd = new ProtectionDomain(null, permissions);
More information about the distro-pkg-dev
mailing list