On 4/20/2016 4:30 PM, Wang Weijun wrote:
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.
Ah...something like:
# /*
# * Prediction Resistance options
# * "pr_and_reseed" - Both Prediction Resistance and
# * reseeding support requested
# * "reseed_only" - Only reseeding support requested
# * "none" - Neither prediction resistance nor
# * reseeding support requested (DEFAULT)
# */
# pr:
# "pr_and_reseed" | "reseed_only" | "none"
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.
Are you going to at least list all the algorithms in
SunProviders.html#SUNProvider too?
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?
Other way around. In the internal class where the defaults are set, you
mention that if any of these change, you need to update the
java.security accordingly.
Brad
