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

[Omair Majid]

On 02/23/2011 12:26 PM, Deepak Bhole wrote:
> * 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
>>>> @@ -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";
>

Ah, I see what you mean now.

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

I dont think this is a problem. A File object does not have to represent 
an existing file. The overhead should not be high either (since we only 
create 2 File objects per process).

Anyway, I have applied your suggested change and pushed the fix. Thanks 
for looking over the patch.

Cheers,
Omair