RFR 8051408: JEP 273: DRBG-Based SecureRandom Implementations

[Wang Weijun]

> On Apr 19, 2016, at 12:34 PM, Xuelei Fan <xuelei.fan at oracle.com> wrote:
> 
> Two minor comments about the spec.
> 
> 1. DrbgParameters.Capability
> The spec for each enum item is not very clear.  Please add more comments
> about the meaning and behavior of each item.

OK. I probably should move the text in DrbgParameters$Instantiate#getCapability here.

> 
> 2. DrbgParameters.Instantiate
> Looks like this class would be better to an extendable individual class.
> Declare this class as "static final" may limit it extension.

NIST SP 800-90A&C are quite clear on what parameters are required for a DRBG. If some implementation really wants to add more parameters, it can implement SecureRandomParameters directly as a new type. In fact, SUN's DRBG uses this technique to pass in the configuration details encoded in the "drbg" security property.

In fact, if you extend DrbgParameters.Instantiate and call getInstance("drbg", params), another implementation may hijack your params even if it does not understand the extra parameters. So I suppose you will only call getInstance("drbg", params, yourProvider), then why care about using a common base class?

Also, it would make documenting SecureRandom#getParameters a little more complex. Should it return the new type or only return the base type?

> 
> Further more, if you add a public construct (the same as
> DrbgParameters.instantiate()), this class may be not necessary to be
> wrapped in DrbgParameters.
> 
> DrbgParameters {
>  public static final class Instantiate
>       implements SecureRandomParameters;
>  public static Instantiate instantiate(int strength,
>       Capability capability, byte[] personalizationString);
> }
> 
> ->
> Public class DrbgInstantiateParameters {
>  public DrbgInstantiateParameters(int, Capability, byte[]);
>  ...
> }
> 
> Similar comments to NextBytes and Reseed.
> 
> If classes Instantiate, NextBytes and Reseed are not wrapped in
> DrbgParameters, you may not need DrbgParameters any more.

A constructor is not very useful for a final class.

The reason I put them into one compilation unit (is that the correct word?) is that I don't want to pollute the java.security package too much. DRBG is only one kind of SecureRandom. I know after compiled they will be different classes but putting the source code in one place makes me more comfortable.

Thanks
Max

> 
> Otherwise, the spec looks fine to me.
> 
> Xuelei
> 
> On 4/15/2016 9:35 PM, Wang Weijun wrote:
>> Hi All
>> 
>> Webrev updated again at
>> 
>> http://cr.openjdk.java.net/~weijun/8051408/webrev.10/
>> http://cr.openjdk.java.net/~weijun/8051408/webrev.10/spec
>> http://cr.openjdk.java.net/~weijun/8051408/webrev.10/specdiff
>> 
>> Changes since webrev.09:
>> 
>> 1. The first line in DrbgParameters:
>> 
>> - * This class specifies the parameters used by a DRBG.
>> + * This class specifies the parameters used by a DRBG (Deterministic
>> + * Random Bit Generator).
>> 
>> 2. Two new methods for DrbgParameters$Capability:
>> 
>> public boolean supportsReseeding();
>> public boolean supportsPredictionResistance();
>> 
>> So you will be able to write
>> 
>> drbg = SecureRandom.getInstance("DRBG");
>> 
>> SecureRandomParameters params = drbg.getParameters();
>> if (params instanceof DrbgParameters.Instantiate) {
>>   DrbgParameters.Instantiate ins = (DrbgParameters.Instantiate) params;
>>   if (ins.getCapability().supportsReseeding()) {
>>       drbg.reseed();
>>   }
>> }
>> 
>> 3. More descriptive text in exception thrown.
>> 
>> Thanks
>> Max
>> 
>> 
>> 
>>> On Apr 5, 2016, at 10:34 AM, Wang Weijun <weijun.wang at oracle.com> wrote:
>>> 
>>> Updated webrev again at
>>> 
>>> http://cr.openjdk.java.net/~weijun/8051408/webrev.09/
>>> http://cr.openjdk.java.net/~weijun/8051408/webrev.09/spec
>>> http://cr.openjdk.java.net/~weijun/8051408/webrev.09/specdiff
>>> 
>>> The only change is that SecureRandomInstantiateParameters, SecureRandomNextBytesParameters and SecureRandomReseedParameters are removed and only a single SecureRandomParameters is added. There seems no reason to introduce 3 marker interfaces.
>>> 
>>> Thanks
>>> Max
>> 
>