RFR 8051408: JEP 273: DRBG-Based SecureRandom Implementations

[Wang Weijun]

> On Apr 19, 2016, at 5:41 PM, Xuelei Fan <xuelei.fan at oracle.com> wrote:
> 
> On 4/19/2016 4:05 PM, Wang Weijun wrote:
>>>> 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.
> ;-) Yes, a spec should be clear about what parameter are required for a
> algorithm.
> 
>> If some implementation really wants to add more parameters, it can implement SecureRandomParameters directly as a new type.
> I was wondering, the current APIs may not define all parameters the NIST
> SP 800-90A&C defined.  I'm OK with the current design if the current
> APIs has already define all parameter the NIST defined, or you don't
> want to define other parameters and don't allow other provider implement
> other parameters at present.
> 
> Please ignore the following comments if you decide go with the current
> design.
> 
>> In fact, SUN's DRBG uses this technique to pass in the configuration details encoded in the "drbg" security property.
>> 
> Can you explain more about what kind of hijack will happen?

If you call SecureRandom.getInstance("drbg", params) and params is an instance of YourInstantiate that extends DrbgP.Instantiate, the DRBG implementation inside SUN will accept it and be returned.

I call this hijack, even if DRBG in SUN does not fully understand params.

> 
>> 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?
>> 
> See previous comment.
> 
>> Also, it would make documenting SecureRandom#getParameters a little more complex. Should it return the new type or only return the base type?
>> 
> The provider implemented type should be returned. It's up to the
> provider to make the decision of the returned type
> (SecureRandomSpi.engineGetParameters).  It is not necessary to return
> the object of the exactly DrbgParameters.Instantiate (can be a sub-class).

But if DRBG in SUN hijack the getInstance() call, it will not be able to return one.

Finally, the new type is restricted to your own implementation in a single provider.

> 
>>>> 
>>>> 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.
>> 
> I cannot agree with you on this point.  Final class need and can have
> constructors.

I should say it's no more useful than a static method. The functions are equivalent.

> 
>> 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.
>> 
> ;-) But you did pollute the package with one additional class.
> 
> If you use DrbgParameters, there are five classes:
>   DrbgParameters
>   DrbgParameters.Instantiate
>   DrbgParameters.NextBytes
>   DrbgParameters.Reseed
>   DrbgParameters.Capability
> 
> If you don't define DrbgParameters, need four classes only.  ;-) Less
> pollution, right?

Well, n vs n+1 is not very different, but 1 vs n is not. :-)

It depends how you look at this. From a user's perspective who is using an IDE, he only needs to type "DrbgParameters." and everything is shown to him.

I also find it convenient to document all DRBG-related things in a single place. In fact, those general concepts and examples do not seem to fit any single class, but they look more natural in this "useless" DrbgParameters outer class.

> 
> In the source code, they are put into one unit class.  But in Java doc,
> they are not displayed individually.
> 
> Anyway, minor comments.  You can go with the current design, please let
> me know.

I'd like to keep the current design.

Thanks
Max

> 
> Xuelei