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.

One advantage about listing it here is that deployments know there is a good replacement if they don't like NativePRNG for some reason.

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

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).

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.

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.

Thanks,

Brad



Reply via email to