RFR 8051408: JEP 273: DRBG-Based SecureRandom Implementations

Xuelei Fan xuelei.fan at oracle.com
Tue Apr 19 13:09:57 UTC 2016

On 4/15/2016 9:
> http://cr.openjdk.java.net/~weijun/8051408/webrev.10/

Please update copyright dates.

145-151:  Looks like the comment are not correct.  There are
getInstance(alg,params) since JDK 1.4 (See CertStore)

156-157:  Fallback change the behavior significantly.  Construct with or
without parameter can be very differently.  For example, LDAP cert store
get requested, but the CertStore.getInstane(String,
LDAPCertStoreParameters) may not return an LDAP cert store.  Can you
make more comment why the fallback is OK?

 456    SecureRandomSpi spi = (SecureRandomSpi) instance.impl;
 457    SecureRandom r = new SecureRandom(spi, instance.provider,
 458    return r;

456, there is a whitespace of the casting.

The compiler may optimize the code, but I may use the old style as line

Similar comment for 507-510, 553-556.

 477 -       secureRandomSpi.engineNextBytes(bytes);
 667 +       secureRandomSpi.engineNextBytes(
 668 +               Objects.requireNonNull(bytes));
Some provider may tolerant null bytes input (for example SunPKCS11).
Please consider this update more for compatibility.

 124         throw new UnsupportedOperationException("not supported");
 156         throw new UnsupportedOperationException("not supported");

I would like to use no-parameter exception, or more specific message.

 174     @Override
 175     public String toString() {
 176         return getClass().getSimpleName();
 177     }
What's the usage of this new toString() method?

Review the DRBG impl later.


More information about the security-dev mailing list