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

Deepak Bhole dbhole at redhat.com
Wed Feb 23 08:50:06 PST 2011


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

Other than that, patch looks fine to me.

Cheers,
Deepak

>      /**
> @@ -67,6 +75,10 @@
>          if (source.equals(systemSource) || source.equals(shellSource))
>              return getAllPermissions();
>  
> +        if (isSystemJar(source)) {
> +            return getAllPermissions();
> +        }
> +
>          // if we check the SecurityDesc here then keep in mind that
>          // code can add properties at runtime to the ResourcesDesc!
>          if (JNLPRuntime.getApplication() != null) {
> @@ -123,6 +135,22 @@
>      }
>  
>      /**
> +     * Returns true if the CodeSource corresponds to a system jar. That is,
> +     * it's part of the JRE.
> +     */
> +    private boolean isSystemJar(CodeSource source) {
> +        // anything in JRE/lib/ext is a system jar and has full permissions
> +        String sourceProtocol = source.getLocation().getProtocol();
> +        String sourcePath = source.getLocation().getPath();
> +        if (sourceProtocol.toUpperCase().equals("FILE") &&
> +                sourcePath.startsWith(jreExtDir)) {
> +            return true;
> +        }
> +
> +        return false;
> +    }
> +
> +    /**
>       * Constructs a delegate policy based on a config setting
>       * @param key a KEY_* in DeploymentConfiguration
>       * @return a policy based on the configuration set by the user
> diff -r f14bd72dbb29 netx/net/sourceforge/jnlp/runtime/JNLPSecurityManager.java
> --- a/netx/net/sourceforge/jnlp/runtime/JNLPSecurityManager.java	Tue Feb 22 19:15:05 2011 -0500
> +++ b/netx/net/sourceforge/jnlp/runtime/JNLPSecurityManager.java	Wed Feb 23 11:30:44 2011 -0500
> @@ -306,27 +306,6 @@
>                              }
>                          }
>                      }
> -
> -                } else if (perm instanceof SecurityPermission) {
> -                    tmpPerm = perm;
> -
> -                    // JCE's initialization requires putProviderProperty permission
> -                    if (perm.equals(new SecurityPermission("putProviderProperty.SunJCE"))) {
> -                        if (inTrustedCallChain("com.sun.crypto.provider.SunJCE", "run")) {
> -                            return;
> -                        }
> -                    }
> -
> -                } else if (perm instanceof RuntimePermission) {
> -                    tmpPerm = perm;
> -
> -                    // KeyGenerator's init method requires internal spec access
> -                    if (perm.equals(new SecurityPermission("accessClassInPackage.sun.security.internal.spec"))) {
> -                        if (inTrustedCallChain("javax.crypto.KeyGenerator", "init")) {
> -                            return;
> -                        }
> -                    }
> -
>                  } else {
>                      tmpPerm = perm;
>                  }
> @@ -351,34 +330,6 @@
>      }
>  
>      /**
> -     * Returns weather the given class and method are in the current stack,
> -     * and whether or not everything upto then is trusted
> -     *
> -     * @param className The name of the class to look for in the stack
> -     * @param methodName The name of the method for the given class to look for in the stack
> -     * @return Weather or not class::method() are in the chain, and everything upto there is trusted
> -     */
> -    private boolean inTrustedCallChain(String className, String methodName) {
> -
> -        StackTraceElement[] stack = Thread.currentThread().getStackTrace();
> -
> -        for (int i = 0; i < stack.length; i++) {
> -
> -            // Everything up to the desired class/method must be trusted
> -            if (!stack[i].getClass().getProtectionDomain().implies(new AllPermission())) {
> -                return false;
> -            }
> -
> -            if (stack[i].getClassName().equals(className) &&
> -                    stack[i].getMethodName().equals(methodName)) {
> -                return true;
> -            }
> -        }
> -
> -        return false;
> -    }
> -
> -    /**
>       * Asks the user whether or not to grant permission.
>       * @param perm the permission to be granted
>       * @return true if the permission was granted, false otherwise.




More information about the distro-pkg-dev mailing list