I like this update. On 4/27/2013 7:29 AM, Valerie (Yu-Ching) Peng wrote: > Xuelei, > > I have updated the webrev for 7196382 so it uses the key size range info > from the underlying PKCS library for key size checking: > http://cr.openjdk.java.net/~valeriep/7196382/webrev.01/ > 107 //TBD: auto-adjust default keysize in case it's out-of-range?
I think it's nice to enable this following block. The key size will be checked in initialize(), so I think it is a little bit reasonable to select a proper default key size instead of throwing an exception later. 209 private void checkKeySize(int ks, RSAKeyGenParameterSpec params) I think when minKeySize is -1, we need to consider the default key size limit (EC 112, RSA/DH/DSA 512). In this update, it seems that if minKeySize is -1, we can generate small keys. I don't think it is the intended design. It is similar when maxKeySize is -1. I was wondering that if minKeySize is -1 (or less than the default hard-coded key size), or maxKeySize is -1 (or greater than the hard-coded default key size), we may be able to reset them to the default hard-coded sizes. Thanks, Xuelei > Thanks, > Valerie > > On 04/25/13 10:59, Valerie (Yu-Ching) Peng wrote: >> Xuelei, >> >> Thanks for the review and comments. >> >> Supposedly, we don't have to have default parameters for all valid key >> sizes. >> The pre-generated default parameters are for the most-commonly used >> keysizes. >> As for the rest of supported key sizes, the needed parameters will be >> generated at runtime upon request. >> >> Well, I don't quite like the current approach of hardcoding ranges >> inside the checkKeySize(...) method. >> There is a way to query the supported keysize ranges from the PKCS11 >> library and I think that should be the values that we base the key >> size check on, plus any additional algorithm-specific check (e.g. >> multiples of 64 bits) that can't be expressed through the ranges. I am >> still testing out the changes. Will post an updated webrev for 7196382 >> once I am done testing... >> >> Thanks! >> Valerie >> >> On 04/18/13 21:45, Xuelei Fan wrote: >>> On 4/19/2013 10:43 AM, Valerie (Yu-Ching) Peng wrote: >>>> Xuelei, >>>> >>>> Do you have time to review the following two fixes? >>>> 7196382: PKCS11 provider should support 2048-bit DH >>>> 8010134: A finalizer in sun.security.pkcs11.wrapper.PKCS11 perhaps >>>> should be protected >>>> >>>> The first one removes the hardcoded limit of 1024 for DH and the second >>>> one is making the finalize() method protected. >>>> >>>> Webrevs: >>>> http://cr.openjdk.java.net/~valeriep/7196382/webrev.00/ >>> Looks fine. >>> >>> Do we plan to support DH keys bwteen 1024 and 2048 with default (null) >>> parameters, for example 1536, in PKCS11 provider? Recently, I run into >>> a case that uses DH public keys of 1536 bits. I was wondering we may >>> also want to support more. >>> >>>> http://cr.openjdk.java.net/~valeriep/8010134/webrev.00/ >>> Looks fine. >>> >>> Xuelei >>> >>>> Thanks! >>>> Valerie >> >
