Code review request, 7106773: 512 bits RSA key cannot work with SHA384 and SHA512
Xuelei Fan
xuelei.fan at oracle.com
Thu Nov 17 15:14:52 UTC 2011
Sean, could you also review the fix? I plan to backport the fix to JDK
7, so that the key size constraints for both JSSE and CertPath can work
with PKCS11 and MSCPI.
updated webrev:
http://javaweb.us.oracle.com/~xf138604/bugbios/7106773/webrev.01/
In the new webrev, I add a new KeyLength class under sun.security.util.
I think it may be useful for other models.
On 11/11/2011 2:33 PM, Weijun Wang wrote:
> SSLContextVersion.java:
>
> typo: suported -> supported
>
Updated.
> DisabledAlgorithmConstraints.java:
>
> When can size == 0? Why it's not allowed?
>
It's a line of dummy code in case the get key size method return 0.
> Yes, I think the reflection calls should be wrapped in doPriv
>
> Please also consider keys from MSCAPI
>
Support MSCAPI now.
> Try looking at sun.misc.SharedSecrets and see if it's better
> for you. Probably not. That class is used to share non-public
> methods in public classes. Not the same case here.
>
> SignatureAndHashAlgorithm.java:
>
> Is it possible to combine expected and signingKey into a single
> signingKey? Of course that means it might need to be never null
> and all key alg call it in a consistent way. I wonder if this
> is OK for you.
>
It not always work. For example, the key algorithm may be "EC", but the
expected algorithm may be "ECDSA".
> The RSAKey interface seems not used anywhere.
>
Yes, removed.
> How about changing
>
> 288 } // Otherwise, cannot determine the key size, prefer the most
> 289 // perferable hash algorithm.
>
> to
>
> maxDigestLength = Integer.MAX_VALUE;
>
> then you don't need to assign -1 and check for < 0 later.
>
Good point.
> I would be glad if lines 302-306 can be further indented 4 spaces
>
> The comment on lines 268-273 is confusing, because you did calculate
> maxDigestAlgorithm for 512 bits key later.
>
> Personally, I prefer comments on 274-281 and codes on 284-288
> to be from a smaller keySize to bigger ones, but everything is OK.
>
Updated.
> Test:
>
> I would like a unit test on DisabledAlgorithmConstraints::getKeySize.
> Hopefully the test can cover all SecretKey and PrivateKey from all
> providers with all keysizes.
>
Added some new tests for PKCS11 and MSCAPI.
Thanks,
Xuelei
> Thanks
> Max
>
> On 11/11/2011 01:09 PM, Xuelei Fan wrote:
>> Hi Weijun,
>>
>> Are you available to review my fix for CR 7106773? The fix in JSSE part
>> is straightforward in that the call to
>> SignatureAndHashAlgorithm.getPreferableAlgorithm() needs an additional
>> Key parameter for RSA key exchanges. The significant change is at
>> sun/security/util/DisabledAlgorithmConstraints.java. I modified the code
>> in order to get the key size of the unextractable key in PKCS#11 device.
>>
>> webrev: http://cr.openjdk.java.net/~xuelei/7106773/webrev.00/
>>
>> Thanks,
>> Xuelei
More information about the security-dev
mailing list