Here are some more comments on the API. I will send some comments on the
impl next.
* DrbgParameters
38 * A DRBG mechanism should extend this class.
Is this sentence necessary? None of the builtin DRBG mechs extend this
class.
175 * If this method is not called, the implementation will
select
176 * a default {@code EntropyInput}.
I think you need to be more specific about what you mean by "the
implementation" (and in other methods of this class). It is not the
implementation of this class, it is the DRBG SecureRandom implementations.
236 * Requests that a derivation function to be used by the
DRBG mechanism.
s/to be used/is to be used/
280 * @return teh newly built {@code DrbgParameters} object
s/teh/the/
The get methods should state whether they can return null or not. Some
of the methods should be more specific about whether they return or make
copies of mutable parameters.
* SecureRandomSpi
67 protected SecureRandomSpi() {
This seems like an incompatible change to me. The previous version had a
default public constructor. I think this needs to be public.
75 * This argument can be {@code null}.
Why can't we call the SecureRandomSpi ctor that takes no args when the
parameter is null? It seems it would be cleaner to do that, but there is
probably a good reason.
109 * @param bytes output.
Too terse. How about "the array to be filled in with random bytes"
110 * @param additionalInput additional input. {@code null} if none.
I think you should throw NPE if this is null because callers should call
engineNextBytes(byte[]) in that case.
110, 134: both of these methods need an @throws
UnsupportedOperationException
134 * @param additionalInput additional string, {@code null} if none.
I think it would be better to add 2 engineReseed methods, one which
takes no arguments and one with an additionalInput arg that throws NPE
if it is null. This would also be consistent with the corresponding
methods on SecureRandom.
* SecureRandom
68 * {@link SecureRandomParameters} parameter. For example, a DRBG
69 * can be initialized with a {@link DrbgParameters} object.
s/a DRBG/a DRBG implementation/
87 * <p> Except for the one created by {@link #SecureRandom(byte[])}, a
newly
s/one/instance/
88 * instantiated SecureRandom object is not seeded. Call one of
{@code reseed}
89 * or {@code setSeed} methods to seed it. If none is called, the
first call
s/Call one of/Call the/
s/methods/method/
s/If none/If neither method/
92 * at instantiate time through a {@code SecureRandomParameters} object.
s/instantiate/instantiation/ (or creation)
94 * This self-seeding will not occur if one of {@code reseed} or
{@code setSeed}
95 * methods was previously called.
s/one of/the/
s/methods/method/
97 * <p> A SecureRandom can be reseeded at any time by calling one of the
s/one of the/the/
s/methods/method/
883 * {@code additionalInput} may contain entropy but its main
usage is to
884 * provide diversity.
s/entropy but/entropy, its/
--Sean
On 12/16/2015 02:20 AM, Wang Weijun wrote:
Webrev updated:
http://cr.openjdk.java.net/~weijun/8051408/webrev.02/
http://cr.openjdk.java.net/~weijun/8051408/webrev.02/specdiff/java/security/package-summary.html
Changes:
1. DrbgParameters has a Builder now
2. No more default implementation for reseed()
3. Synchronization is now inside implementation, on engineXXX() methods
4. engineConfigure() now throws InvalidAlgorithmParameterException instead of
IllegalArgumentException
5. CtrDRBG uses up all input in engineSetSeed()
Thanks
Max
