> On Apr 1, 2016, at 3:24 AM, Sean Mullan <[email protected]> wrote:
>
> Just a few comments:
>
> - SunJCE
>
> 707 // TODO: aliases with OIDs
>
> leftover TODO.
Every other HMAC algorithm has an OID that can be used as an Alg.Alias.***, but
the 2 new algorithms do not have. I expect one day they will have.
>
> - SecureRandom
>
> 604 * @implSpec The default implementation returns {@code null}.
>
> Technically, I don't think that is correct, since it is really dependent on
> what the underlying Spi is doing. The same comment applies to the other
> @implSpec sections in this class.
I see. So @implSpec should only appear in SecureRandomSpi.
>
>
> 683 * @throws UnsupportedOperationException if the implementation
> 684 * has not overridden this method.
>
> Would it be more accurate to say "if the underlying provider implementation
> (SecureRandomSpi) has not overridden this method". Same comment applies to
> other UOEs in this class.
Yes.
> - SecureRandomSpi
>
> 86 protected SecureRandomSpi(SecureRandomInstantiateParameters params) {
> 87 // ignored
> 88 }
>
> If you changed this to:
>
> protected SecureRandomSpi(SecureRandomInstantiateParameters params) {
> this();
> }
>
> couldn't you avoid the code which catchs a NoSuchMethodExc and retries, etc?
> It would be nice to not have these extra rules about calling this constructor
> or that constructor, and instead you could just always call the constructor
> above and it would do the right thing. Just thinking out loud here, not sure
> if it is the right thing to do.
I'll think about it. I assume you are talking about my change in Provider.java.
The problem is that old and new SecureRandom implementations do not have the
same constructors and I need them both working.
>
> - java.security
>
> what happens if you have parsing/syntax errors in the drbg property?
Well, there is no parsing error now since I treat everything non-parseable as
algorithm and it will be caught later.
Said that, maybe I should at least check if one aspect is provided 2 times and
if there is an empty aspect, and if an integer is non-positive.
> Also, does the order of the aspects matter?
No. The only non-literals are algorithm name and strength. The strength should
always be an integer and the algorithm name should not be the same as other
aspects.
>
> - DrbgParameters
>
> 249 * @return If used in {@code getInstance}, returns the minimum
> strength
>
> s/If/if/
>
> 253 * strengh requested.
>
> s/strengh/strength/
>
> 290 * @return If used in {@code getInstance}, returns the minimum
> capability
> 301 * @return If used in {@code getInstance}, returns the requested
>
> a/If/if/
>
> 428 public static Instantiate instantiate(int strength,
> 429 Capability capability,
> 430 byte[] personalizationString) {
>
> Should this throw NPE if capability is null?
Yes.
> Should it throw IllegalArgExc if strength < -1?
I can, but necessary?
>
> - EntropySource
>
> Is this interface used anywhere?
In AbstractDrbg.SeedHolder, as the default entropy sources for pr_true and
pr_false are different.
And in tests.
>
> Should getEntropy throw IllegalArgumentExceptions if int params are less than
> a certain value or if maxLength < minLength? Does it return a new byte array
> each time it is invoked?
Yes and yes, but I haven't checked/documented it since it's not a public API
yet.
Thanks
Max
>
> --Sean
>
> On 03/29/2016 04:47 AM, Wang Weijun wrote:
>> Ping again. No comment?
>>
>> --Max
>>
>>> On Mar 21, 2016, at 1:15 PM, Wang Weijun <[email protected]> wrote:
>>>
>>> Hi All
>>>
>>> Please take a review at the design and implementation of DRBG at:
>>>
>>> http://cr.openjdk.java.net/~weijun/8051408/webrev.07
>>> http://cr.openjdk.java.net/~weijun/8051408/webrev.07/spec
>>> http://cr.openjdk.java.net/~weijun/8051408/webrev.07/specdiff/overview-summary.html
>>>
>>> An example:
>>>
>>> SecureRandom drbg;
>>> byte[] buffer = new byte[32];
>>>
>>> drbg = SecureRandom.getInstance("DRBG",
>>> DrbgParameters.instantiate(256, PR_ONLY, "hello".getBytes()));
>>>
>>> drbg.nextBytes(buffer,
>>> DrbgParameters.nextBytes(-1, false, "more".getBytes()));
>>>
>>> SecureRandomInstantiateParameters params = drbg.getParameters();
>>> if (params instanceof DrbgParameters.Instantiate) {
>>> DrbgParameters.Instantiate ins = (DrbgParameters.Instantiate) params;
>>> if (ins.getCapability() != NONE) {
>>> drbg.reseed(DrbgParameters.reseed(false, "extra".getBytes()));
>>> }
>>> }
>>>
>>> Thanks
>>> Max
>>>
>>
