RFR 8051408: JEP 273: DRBG-Based SecureRandom Implementations

Xuelei Fan xuelei.fan at oracle.com
Fri Apr 22 13:50:45 UTC 2016


On 4/21/2016 4:52 PM, Wang Weijun wrote:
> http://cr.openjdk.java.net/~weijun/8051408/webrev.11/

src/java.base/share/classes/sun/security/provider/AbstractDrbg.java
=========================

 343         if (debug != null) {
 344             debug.println("nextBytes");
 345         }

Would you mind add more information in the log except printing the
method name?  I like the following debug more:

 588    debug.println("configure " + this + " with " + params);

If you're OK with this comment, I would suggest you check the debug log
message in other places and expose more debugging information.

BTW, security random may be a operation used frequently.  Performance is
critical.  Maybe, it is worthy to consider to remove some debug log if
the log is not necessary for application debugging (for example
HmacDrbg.status() might be not very useful once some basic tests passed).

src/java.base/share/conf/security/java.security
============
 211 #   algorithm_name:
 212 #     Any supported MessageDigest or Cipher algorithm name as described
 213 #     in Section 10 of SP 800-90Ar1
The reader may have to look for and look at SP 800-90r1, and may not
find the right name.  For example, "3 Key TDEA" is a name in NIST SP
800-90r1, we may want to use Java standard name instead.  Using Java
Standard names may be a little bit easier to read.  A Java Doc would
help to explain the supported names.

 235 #   securerandom.drbg.config=Hash_DRBG,SHA-1,112,none
What do you think if we use SHA2 algorithm here, and probably does not
support SHA-1 based DRBG as SHA-1 may be fading out slowly?


src/java.base/share/classes/sun/security/provider/AbstractHashDrbg.java
======================
  42             case "SHA-1":
  43                 return 128;
See the previous comment.  Please consider whether it is OK to remove
the support of SHA-1 based DRBG.


src/java.base/share/classes/sun/security/provider/HashDrbg.java
======================
  37 public class HashDrbg extends AbstractHashDrbg {

Instead of public, is it good to use package accessibility?  Please
consider other classes in the package except DRBG.java if you would like
to remove the public keyword.

Xuelei



More information about the security-dev mailing list