A few more comments -
The name of the new interface should be SecureRandomParameterSpec
instead of SecureRandomSpec.
The getInstance methods can now take a SecureRandomParameterSpec object
(rather than an AlgorithmParameterSpec). They should throw
InvalidAlgorithmParameterException (not IllegalArgumentException) if the
parameters are null or not the right type to be consistent with other
Spi classes.
You will also need to add a protected (or public?) constructor to
SecureRandomSpi that takes a SecureRandomParameterSpec parameter.
CertStoreSpi is a good example to follow.
DrbgSpec should really be named DrbgParameterSpec to be consistent with
other AlgorithmParameterSpec subclasses.
nextBytes should throw NPE if bytes is null. I also think it should not
allow additionalInput to be null (should throw NPE), because this is an
overloaded method. If the caller doesn't want to specify
additionalInput, then it should call nextBytes(byte[]).
I would prefer 2 reseed methods, one that didn't take any arguments, and
one that takes an additionalInput (and throws NPE if it is null):
public void reseed()
public void reseed(byte[] additionalInput)
This avoids code having to always do this in the (likely) common case
where additionalInput is not specified:
sr.reseed(null);
In SecureRandomSpi, I think there is a typo: engineGenerate should be
named engineNextBytes, and it should be protected (not public).
I think the new methods in SecureRandomSpi need a default impl and it
should be specified. They should probably throw UnsupportedOperationExc
by default. If so, this should be added in @throws clause and default
behavior documented in an @implSpec.
I still need to review the EntropyInput and DrbgSpec classes ...
Thanks,
Sean
On 11/13/2015 08:43 PM, Wang Weijun wrote:
On 2015年11月14日, at 上午1:56, Sean Mullan <sean.mul...@oracle.com> wrote:
On 11/12/2015 07:58 PM, Wang Weijun wrote:
What happens if configure is called more than once, or
simultaneously by more than one thread?
The state is reset. The last one rules. The implementation can be
made synchronized.
* This method can be called multiple times. After each call, this *
{@code SecureRandom} object must be reseeded.
Instead of a configure method, I would suggest adding new
getInstance methods that take an AlgorithmParameterSpec. This
should simplify the implementation.
getInstance() has 3 flavors, (), (String) and (Provider). Too many
new methods to add.
Only 3 more. It would seem to make the implementation simpler, less state and
synchronization to worry about.
OK.
I also think it might be cleaner and simpler to make EntropyInput
an input parameter of DrbgSpec so that you could have a single
AlgorithmParameterSpec parameter (instead of an AlgParamSpec and
EntropyInput) for the getInstance method.
EntropyInput as a separated parameter means it applies to other
SecureRandom implementations and not only DRBG. For example, SHA1PRNG
can also have a specified entropy source. It is also useful to
describe what reSeed() means.
Ok but it's really just an input parameter, right?
Maybe, you want something like:
public interface SecureRandomSpec extends AlgorithmParameterSpec {
EntropyInput getEntropyInput();
}
public class DrbgSpec implements SecureRandomSpec {
}
Sound good.
I'll update the APIs.
Thanks
Max
--Sean
