RFR 8051408: JEP 273: DRBG-Based SecureRandom Implementations

[Sean Mullan]

Just a few comments:

- SunJCE

707                     // TODO: aliases with OIDs

leftover TODO.

- SecureRandom

604      * @implSpec The default implementation returns {@code null}.

Technically, I don't think that is correct, since it is really dependent 
on what the underlying Spi is doing. The same comment applies to the 
other @implSpec sections in this class.


683      * @throws UnsupportedOperationException if the implementation
684      *         has not overridden this method.

Would it be more accurate to say "if the underlying provider 
implementation (SecureRandomSpi) has not overridden this method". Same 
comment applies to other UOEs in this class.

- SecureRandomSpi

   86     protected SecureRandomSpi(SecureRandomInstantiateParameters 
params) {
   87         // ignored
   88     }

If you changed this to:

protected SecureRandomSpi(SecureRandomInstantiateParameters params) {
     this();
}

couldn't you avoid the code which catchs a NoSuchMethodExc and retries, 
etc? It would be nice to not have these extra rules about calling this 
constructor or that constructor, and instead you could just always call 
the constructor above and it would do the right thing. Just thinking out 
loud here, not sure if it is the right thing to do.

- java.security

what happens if you have parsing/syntax errors in the drbg property? 
Also, does the order of the aspects matter?

- DrbgParameters

  249          * @return If used in {@code getInstance}, returns the 
minimum strength

s/If/if/

  253          * strengh requested.

s/strengh/strength/

  290          * @return If used in {@code getInstance}, returns the 
minimum capability
  301          * @return If used in {@code getInstance}, returns the 
requested

a/If/if/

  428     public static Instantiate instantiate(int strength,
  429                                           Capability capability,
  430                                           byte[] 
personalizationString) {

Should this throw NPE if capability is null? Should it throw 
IllegalArgExc if strength < -1?

- EntropySource

Is this interface used anywhere?

Should getEntropy throw IllegalArgumentExceptions if int params are less 
than a certain value or if maxLength < minLength? Does it return a new 
byte array each time it is invoked?

--Sean

On 03/29/2016 04:47 AM, Wang Weijun wrote:
> Ping again. No comment?
>
> --Max
>
>> On Mar 21, 2016, at 1:15 PM, Wang Weijun <weijun.wang at oracle.com> wrote:
>>
>> Hi All
>>
>> Please take a review at the design and implementation of DRBG at:
>>
>> http://cr.openjdk.java.net/~weijun/8051408/webrev.07
>> http://cr.openjdk.java.net/~weijun/8051408/webrev.07/spec
>> http://cr.openjdk.java.net/~weijun/8051408/webrev.07/specdiff/overview-summary.html
>>
>> An example:
>>
>> SecureRandom drbg;
>> byte[] buffer = new byte[32];
>>
>> drbg = SecureRandom.getInstance("DRBG",
>>         DrbgParameters.instantiate(256, PR_ONLY, "hello".getBytes()));
>>
>> drbg.nextBytes(buffer,
>>         DrbgParameters.nextBytes(-1, false, "more".getBytes()));
>>
>> SecureRandomInstantiateParameters params = drbg.getParameters();
>> if (params instanceof DrbgParameters.Instantiate) {
>>     DrbgParameters.Instantiate ins = (DrbgParameters.Instantiate) params;
>>     if (ins.getCapability() != NONE) {
>>         drbg.reseed(DrbgParameters.reseed(false, "extra".getBytes()));
>>     }
>> }
>>
>> Thanks
>> Max
>>
>