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
