> On Apr 21, 2016, at 3:06 AM, Bradford Wetmore <bradford.wetm...@oracle.com> > 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 > > >