A few comments from supportability side of the table.
=============
sun/security/provider/AbstractDrbg.java
+ if (dp.getStrength() > strength) {
+ throw new IllegalArgumentException("strength too high");
+ }
+ if (result.length > maxNbLength) {
+ throw new IllegalArgumentException("result too long");
+ }
Please print these bad strengths / results in the exception.
Similar corrections are needed in the engineConfigure method :
608 if (config.getStrength() > highestSecurity) {
609 throw new IllegalArgumentException("strength too
big");
610 }
611 if (config.getPersonalizationString() != null &&
config.getPersonalizationString().length > maxPsLength) {
612 throw new IllegalArgumentException("ps too long");
613 }
throw new IllegalArgumentException("unknown params type");
can you print the type of params that was passed in ? (X 2 calls)
+ if (dp.getAdditionalInput() != null && dp.getAdditionalInput().length
> maxAiLength) {
+ throw new IllegalArgumentException("ai too long");
Please print ai value.
+ // This SEI does not support pr
+ throw new IllegalArgumentException();
Cou you put your comment in the body of the IllegalArgumentException ?
=============
sun/security/provider/CtrDrbg.java
+ try {
+ aesLimit = Cipher.getMaxAllowedKeyLength("AES");
+ } catch (Exception e) {
+ // should not happen
+ throw new AssertionError("Cannot detect AES");
+ }
Just to be safe, can you add e as Throwable variable for AssertionError ?
+ if (input.length != seedLen) {
+ // Should not happen
+ throw new IllegalArgumentException("input must be of seedlen
bytes");
can you print the lengths expected ?
=============
sun/security/provider/DRBG.java
+ if (strength < 0) {
+ throw new IllegalArgumentException(
+ "strength in drbg cannot be negative");
+ }
Let's print the value of the 'part' string in this exception.
+ } else {
+ throw new IllegalArgumentException("Unsupported params");
+ }
can you print the type of params that were passed in ?
+ default:
+ throw new IllegalArgumentException("Unsupported mech");
+ }
can yuo print the mech value encoutered ?
=============
sun/security/provider/HashDrbg.java
+ } catch (DigestException e) {
+ throw new AssertionError("will not happen");
+ }
Famous last words ;)
Can you add e as Throwable cause to AssertionError ? (happens in two areas)
Regards,
Sean.
On 05/04/2016 03:34, Wang Weijun wrote:
Updated webrev again at
http://cr.openjdk.java.net/~weijun/8051408/webrev.09/
http://cr.openjdk.java.net/~weijun/8051408/webrev.09/spec
http://cr.openjdk.java.net/~weijun/8051408/webrev.09/specdiff
The only change is that SecureRandomInstantiateParameters,
SecureRandomNextBytesParameters and SecureRandomReseedParameters are removed
and only a single SecureRandomParameters is added. There seems no reason to
introduce 3 marker interfaces.
Thanks
Max
On Apr 1, 2016, at 7:34 PM, Wang Weijun <weijun.w...@oracle.com> wrote:
Hi All
Updated webrev at
http://cr.openjdk.java.net/~weijun/8051408/webrev.08/
http://cr.openjdk.java.net/~weijun/8051408/webrev.08/spec
http://cr.openjdk.java.net/~weijun/8051408/webrev.08/specdiff
Spec changes:
- More text in @implNote of DrbgParameters.java, which somehow matches the
Minimal Documentation Requirements described in 11.1 of NIST SP 800-90Ar1.
- DrbgParameters.instantiate(strength,cap,ps) throws NPE if cap is null
- SecureRandom.java: no more @implSpec for new methods since impl is in
SecureRandomSpi. Also, make the following word changes in all UOE cases:
- * @throws UnsupportedOperationException if the implementation
- * has not overridden this method.
+ * @throws UnsupportedOperationException if the underlying provider
+ * implementation has not overridden this method.
"drbg" security property changes:
- delimiter is now ",". Otherwise, "SHA-512/256" is ambiguous.
- AbstractDrbg#toString and DrbgParameters$Instantiate#toString also use ","
now.
- default value is "", thus each aspect uses its own default as described in
the comment.
- examples
Code changes:
- DRBG.java: more check for the "drbg" security property, one aspect cannot
be set twice,
and strength must be positive
- HashDrbg.java optimization
* Use MessageDigest#digest(output,offset,length) instead of
digest()+arraycopy.
(BTW, why is DigestException a checked exception?) -- a little useful
* addBytes() now updates its first argument, therefore less round of adding
-- very useful
* store "new byte[1]" and "new byte[]{1}" as constants -- a little useful
- MoreDrbgParameters: now including mech, so it can fully cover the "drbg"
security property. It still includes non-publicly configurable items like entropy source
and nonce, that the DRBG (known-answer) Test Vectors require.
Thanks
Max
