Code review request, 7106773: 512 bits RSA key cannot work with SHA384 and SHA512

Weijun Wang weijun.wang at oracle.com
Mon Nov 21 08:34:29 UTC 2011


I removed most of the thread and keep some sections below:

On 11/21/2011 03:30 PM, Xuelei Fan wrote:
> new webrev: http://cr.openjdk.java.net/~xuelei/7106773/webrev.02/

What is synchronized for? Why only on the for block? This won't prevent 
map iteration and insertion at the same time.

>
>>
>> Another solution is to define a new interface:
>>      package sun.security.util;
>>      public interface Measurable {
>>          public int getLength();
>>      }
>>
>> And updated P11Key and MSCPI Key accordingly to implement the
>> Measureable interface:
>>    package sun.security.pkcs11;
>>    ...
>> - abstract class P11Key implements Key, Measurable {
>> + abstract class P11Key implements Key {
>>    ...
>> -     int keyLength() {
>> +     public int getLength() {
>>    ...
>>    }
>>
>> It is similar for MSCAPI Key.

How about this approach? This looks very safe.

>>
>>>>>
>>>>> Calling getDeclaredMethod each time is a waste. How about just
>>>>>
>>>>>           while (clazz != null) {
>>>>>               if (clazz.getName().equals(baseClassName)) {
>>>>>                   // do sth
>>>>>                   break;
>>>>>               }
>>>>>               clazz = clazz.getSuperclass();
>>>>>           }
>>>>>
>>>>> Also, you're using methods in reflection. Is it faster to use fields?
>>>>>
>>>> I also concerns about the override of the target method, although there
>>>> is no such override at present. From some reports, it seems that fields
>>>> reflection cost more cycles than method reflection.

You mentioned "concerns about the override of the target method".

Thanks
Max



More information about the security-dev mailing list