> On Apr 21, 2016, at 3:06 AM, Bradford Wetmore <[email protected]>
> wrote:
>
>
>>> 175: Should we add DRBG:SUN as a backup for non-windows?
>>
>> If NativePRNGBlocking:SUN is not always available, yes we can.
>
> It should be available, unless someone decides to blow away /dev/(u)random.
> But then DRBG will have the same problem.
They will have to redefine securerandom.source then.
>
> One advantage about listing it here is that deployments know there is a good
> replacement if they don't like NativePRNG for some reason.
Good point.
>
> I'm fine with adding or leaving alone.
>
>>> 198: Should we add a short 1-liner description for the fields? The
>>> variable meanings (esp pr/df) may not be obvious to a casual observer.
>>> For example, using these three fields as an example:
>>>
>>> mech_name: default "Hash_DRBG"
>>> "Hash_DRBG" | "HMAC_DRBG" | "CTR_DRBG"
>>>
>>> + The DRBG mechanism to use.
>>>
>>> pr: default "none"
>>> "pr_and_reseed" | "reseed_only" | "none"
>>>
>>> + Prediction resistance...
>>>
>>> df: default "use_df", only applicable to CTR_DRBG
>>> "use_df" | "no_df"
>>>
>>> + Derivation Function...
>>
>> Not sure about the format, how about
>>
>> # # The DRBG mechanism to use. default "Hash_DRBG"
>> # mech_name:
>> # "Hash_DRBG" | "HMAC_DRBG" | "CTR_DRBG"
>>
>> So this is double comment. Please note I also put the default value into the
>> comment.
>
> Yes, that works. Or java-style comments /* */ or //.
>
> Maybe expanding the values too?
>
> # /*
> # * Prediction Resistance options
> # * default: "none"
> # */
> # pr:
> # "pr_and_reseed" | "reseed_only" | "none"
> #
> # "pr_and_reseed" - Both Prediction Resistance and
> # reseeding support requested
> # "reseed_only" - Only reseeding support requested
> # "none" - Neither prediction resistance nor
> # reseeding support requested
I'd keep the
"pr_and_reseed" | "reseed_only" | "none"
line and put all descriptions into the comment, to make this still BNF style.
>
>>> 210: Instead of pointing to 800-90A here, you should instead point to
>>> the Sun Provider document. That document will then reference
>>> 800-90A/Section 10, and will use the Standard Algorithm names that
>>> you have defined for these algorithms. (I assume you'll be adding
>>> those to the standard algorithm name doc, right? I don't recall seeing
>>> that part of the review yet, but maybe haven't gotten that far.)
>>
>> Yes, I will.
>
> StandardNames.html#SecureRandom
> ===============================
> NativePRNG Obtains...
> ...
> + DRBG Obtains from an 800-90A impl...
>
> SunProviders.html#SUNProvider
> =============================
> SecureRandom: SHA1PRNG
> NativePRNG
> ...
> + DRBG
>
>> The standard algorithm name is only "DRBG", but here it's the DRBG algorithm
> > name (SHA-256 etc). Are we going to talk about this in Sun
> > Provider doc?
>
> Valid point, since we haven't standardized these options, then we probably
> shouldn't mention them in the Standard Alg names document. But we can still
> talk about this in the Sun Provider doc.
>
> You can make a new table and put it at the end of the Sun provider section.
> I suppose you could also put the expanded definitions here instead of
> java.security (or both).
java.security is better.
>
>>> 229: I hadn't noticed this before, but the Security variable "drbg"
>>> doesn't match the style of the other variables, securerandom.* or
>>> otherwise. I think we should use something like either:
>>>
>>> securerandom.drbg
>>> securerandom.drbg.config
>>
>> Will choose "securerandom.drbg.config".
>
> Great, thanks. The CCC will need an update with the new name, or else a new
> CCC. There's been no votes on your "finalized" version yet, so you might be
> able to withdraw/resubmit.
Yes.
>
>>> 229: Why an empty string here? Why not actually specify the default here
>>> instead of burying the default somewhere where it might changed without
>>> a corresponding update to this file?
>>
>> There is a small technical issue here. The problem is that the default value
>> of algorithm_name depends on mech_name, and the default value of strength
>> depends on algorithm_name. If we set "Hash_DRBG,SHA-256,128" here and we
>> only change one of them, another one might be illegal.
>>
>> For example, if I want to create a SecureRandom using MoreDrbgParameters
>> (yes, this is an internal class) with
>>
>> SecureRandom.getInstance("drbg", new MoreDrbgParameters(
>> null, "CTR_DRBG", "3KeyTDEA", null, false,
>> DrbgParameters.instantiate(-1, NONE, null)))
>>
>> it will fail, because with -1 in DrbgParameters.instantiate(), the new
>> configuration will look like
>>
>> CTR_DRBG,3KeyTDEA,128
>>
>> but CTR_DRBG,3KeyTDEA does not support 128 bit strength.
>>
>> I will have to use DrbgParameters.instantiate(112, NONE, null) but that does
>> not looks comfortable because I thought the system can give me a default
>> working strength.
>>
>> This won't be a problem if only public API is used, but I will have to redo
>> some implementation codes.
>
> Ok. If you haven't already, please add a comment where the default value is
> stored to update the java.security file if this default value is ever changed.
A comment in java.security mentioning an internal class?
Thanks
Max
>
> Thanks,
>
> Brad
>
>
>
