RFR 8051408: JEP 273: DRBG-Based SecureRandom Implementations
weijun.wang at oracle.com
Wed Apr 20 01:41:55 UTC 2016
> On Apr 20, 2016, at 8:54 AM, Bradford Wetmore <bradford.wetmore at oracle.com> wrote:
> > 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
> Some initial comments.
> 123-133: Would you consider changing the ordering of the algorithm
> names to be consistent between the two sections? Say:
> NativePRNG, SHA1PRNG, and DRBG.
> 175: Should we add DRBG:SUN as a backup for non-windows?
If NativePRNGBlocking:SUN is not always available, yes we can.
> # NIST SP 800-90Ar1 lists several DRBG mechanisms, each can be configured with
> # NIST SP 800-90Ar1 lists several DRBG mechanisms. Each can be configured with
> request of "DRBG" SecureRandom implemented in the SUN provider.
> request of "DRBG" SecureRandom implementations in the SUN provider.
> (Other DRBG implementations might also use this property.)
> 188: Can you adjust the line lengths so it looks polished, and not
> edited in place? I use a vi macro (using fmt) for this.
> 189: Do you want to add "currently", or leave as is?
> names are not configurable
> names are not currently configurable
> 194: Am I missing something?
> is a slash-separated list
> is a comma-separated list
Oops, will correct.
> 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"
# "Hash_DRBG" | "HMAC_DRBG" | "CTR_DRBG"
So this is double comment. Please note I also put the default value into the comment.
> 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.
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?
> 214: In the past, we've used spaces as a field separator for names in
> a provider, I'm wondering if we should make this name something like
> "3KeyTDEA" instead?
> 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:
Will choose "securerandom.drbg.config".
> 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
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.
More information about the security-dev