Design and impl review: JEP 273: DRBG-Based SecureRandom Implementations

[Sean Mullan]

Here are some more comments on the API:

* EntropyInput:

   29  * An interface of a source of entropy input.

"interface" is implied, so you can just say "A source of entropy input." 
Also, I think this interface should be called "EntropySource". To me, 
"Input" means the byte array that is already populated, whereas "Source" 
produces those bytes. A sentence or two with more details in the 
description would also be helpful.

Do we want to allow the generated entropy input to be longer than the 
size requested (see section 7.1 of sp800-90Ar1)? Perhaps, a method such as:

     byte[] getFullEntropy(int length)

might be more suitable, and specify that a byte array longer than the 
specified length may be returned.

* EntropyNotAvailableException:

   37      * Creates one

Incomplete sentence.

In the class description, add an @see link to EntropyInput.getFullEntropy.

* SecureRandomParameters:

It seems a little odd to have a class with one parameter that can be 
optional, and then accepting null as a value in the ctor, which 
typically will only needed by subclasses. I think it would be nicer to 
avoid exposing that in the API. Have you considered defining this as an 
interface, with a static method that returns an instance with an 
EntropyInput, ex:

public interface SecureRandomParameters {

     EntropyInput getEntropyInput();

     static SecureRandomParameters of(EntropyInput);
}

--Sean

On 12/16/2015 02:20 AM, Wang Weijun wrote:
> Webrev updated:
>
>     http://cr.openjdk.java.net/~weijun/8051408/webrev.02/
>     http://cr.openjdk.java.net/~weijun/8051408/webrev.02/specdiff/java/security/package-summary.html
>
> Changes:
>
> 1. DrbgParameters has a Builder now
>
> 2. No more default implementation for reseed()
>
> 3. Synchronization is now inside implementation, on engineXXX() methods
>
> 4. engineConfigure() now throws InvalidAlgorithmParameterException instead of IllegalArgumentException
>
> 5. CtrDRBG uses up all input in engineSetSeed()
>
> Thanks
> Max