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

[Weijun Wang]

SSLContextVersion.java:

    typo: suported -> supported

DisabledAlgorithmConstraints.java:

    When can size == 0? Why it's not allowed?

    Yes, I think the reflection calls should be wrapped in doPriv

    Please also consider keys from MSCAPI

    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.

    The RSAKey interface seems not used anywhere.

    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.

    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.

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.

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