RFR 8044199: tests for RSA keys and key specifications

Valerie Peng valerie.peng at oracle.com
Mon Aug 10 22:47:15 UTC 2015


Tristan,

- Given that the tests are against SunRsaSign provider, I think 
sun/security/rsa subdirectory is a better choice than java/security/rsa.
- Put @test and @bug on separate lines
- Elaborate more on what is tested and what is the expected result in 
the bug record. The current content isn't very clear.
- Can u have more unique names for these tests? Currently, they are too 
similarly named and it's somewhat confusing. The directory already 
contains RSA, I don't feel that u need to have RSA again in the names of 
the test especially when SunRsaSign provider doesn't support non-RSA 
algorithms.

For KeyTest.java
1) typo on line 69, rsaPrivateKey2 should be rsaPrivateKey
2) This test is based on the provider specific behavior that CRT key is 
generated by default for RSA. It'd be much clearer if you can add an 
instanceof check on the generated RSA private key objects before the 
KeyFactory + RSAPrivateKeySpec code. In addition, add additional code to 
check for equality with the matching KeySpec (e.g. RSAPrivateCrtKeySpec 
for RSAPrivateCrtKey) is used.
3) line 58, add a space between '//" and text.

For RSAKeySizeTest.java
1) specify provider "SunRSASign" for all getInstance() calls.
2) line 55 can be moved up to before the for-loop.
3) instead of using toString(int), probably bitLength() would be a 
better choice.
4) sizeTest() doesn't really check size but rather it compares the 
modulus values being equal. This check can be moved to RSAKeySizeTest.java.

For RSATest.java
1) specify provider "SunRSASign" for all getInstance() calls.

Thanks,
Valerie

On 8/3/2015 10:27 AM, Tristan Yan wrote:
> Hi
>
> Please review new tests for RSA keys and key specifications
>
> Bug:https://bugs.openjdk.java.net/browse/JDK-8044199
> Webrev:http://cr.openjdk.java.net/~tyan/8044199/webrev.01/  <http://cr.openjdk.java.net/%7Etyan/8044199/webrev.01/>  <http://cr.openjdk.java.net/~tyan/8044199/webrev.01/  <http://cr.openjdk.java.net/%7Etyan/8044199/webrev.01/>>
>
> Thanks
> Tristan
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/security-dev/attachments/20150810/b69139eb/attachment.htm>


More information about the security-dev mailing list