[icedtea-web] RFC: fix RH677772 - NoSuchAlgorithmException using SSL/TLS in javaws

[Deepak Bhole]

* Omair Majid <omajid at redhat.com> [2011-02-23 12:05]:
> On 02/23/2011 11:50 AM, Deepak Bhole wrote:
> >* Omair Majid<omajid at redhat.com>  [2011-02-23 11:37]:
> >>Hi,
> >>
> >>I have attached a patch to fix RH677772 [1]. Please note that I am
> >>particularly concerned as it is reverting a patch [2] that was added
> >>to fix another (quite similar) bug, RH524387 [3].
> >>
> >>As this new bug shows, there are issues with the current system. To
> >>fix these, we will have to whitelist all possible classes/method
> >>that are allowed at this place. And I am not sure at all how to
> >>determine these classes/methods. I especially dont think this is
> >>maintainable (all the more so if you consider third party code which
> >>we have no idea about).
> >>
> >>The real problem, I think, is that we are not granting full
> >>permissions to code originating from jre/lib/ext (which the default
> >>java.policy file does). If we do that, then all code that's
> >>installed there (3rd party JCE providers, proprietary jars, or
> >>really anything) will run with proper permissions, and we wont even
> >>need to deal with the current system of whitelisting. This is what
> >>the proposed patch does.
> >>
> >>I hope that explains why we should just grant appropriate
> >>permissions to code loaded from jre/lib/ext and let java's normal
> >>security mechanism handle everything from then on.
> >>
> >>Any thoughts or comments on the patch?
> >>
> >>Thanks,
> >>Omair
> >>
> >>[1] https://bugzilla.redhat.com/show_bug.cgi?id=677772
> >>[2] http://icedtea.classpath.org/hg/icedtea6?cmd=changeset;node=6d1e2fae468a
> >>[3] https://bugzilla.redhat.com/show_bug.cgi?id=524387
> >
> >>diff -r f14bd72dbb29 netx/net/sourceforge/jnlp/runtime/JNLPPolicy.java
> >>--- a/netx/net/sourceforge/jnlp/runtime/JNLPPolicy.java	Tue Feb 22 19:15:05 2011 -0500
> >>+++ b/netx/net/sourceforge/jnlp/runtime/JNLPPolicy.java	Wed Feb 23 11:30:44 2011 -0500
> >>@@ -16,6 +16,7 @@
> >>
> >>  package net.sourceforge.jnlp.runtime;
> >>
> >>+import java.io.File;
> >>  import java.net.URI;
> >>  import java.net.URISyntaxException;
> >>  import java.security.*;
> >>@@ -44,6 +45,8 @@
> >>      /** the previous policy */
> >>      private static Policy systemPolicy;
> >>
> >>+    private final String jreExtDir;
> >>+
> >>      /** the system level policy for jnlps */
> >>      private Policy systemJnlpPolicy = null;
> >>
> >>@@ -57,6 +60,11 @@
> >>
> >>          systemJnlpPolicy = getPolicyFromConfig(DeploymentConfiguration.KEY_SYSTEM_SECURITY_POLICY);
> >>          userJnlpPolicy = getPolicyFromConfig(DeploymentConfiguration.KEY_USER_SECURITY_POLICY);
> >>+
> >>+        String jre = System.getProperty("java.home");
> >>+        File libDir = new File(jre, "lib");
> >>+        File extDir = new File(libDir, "ext");
> >>+        jreExtDir = extDir.toString();
> >>      }
> >>
> >
> >Neither jre, libDir and extDir are used anywhere else.. IMO they should be
> >removed.
> >
> 
> Err... jre, libDir and extDir are all local variables used to
> compute jreExtDir (which is used in other places). I dont see any
> benefit of removing them. Do you think it would make more sense to
> replace it with something like this?
>     jreExtDir = new File(new File(System.getProperty("java.home"),
> "lib"), "ext").toString();
> 

Yes. Or even:
String jre = System.getProperty("java.home");
jreExtDir = jre + File.separator + "lib" + File.separator + "ext";

My only concern is that we are initializing File variables that are
never used in that context (only the strings are used).

Cheers,
Deepak

> >Other than that, patch looks fine to me.
> >
> 
> Thanks for looking over the patch!
> 
> Thanks,
> Omair