RFR 8051408: JEP 273: DRBG-Based SecureRandom Implementations

Xuelei Fan xuelei.fan at oracle.com
Wed Apr 20 03:34:59 UTC 2016


On 4/19/2016 9:09 PM, Xuelei Fan wrote:
> On 4/15/2016 9:
>> http://cr.openjdk.java.net/~weijun/8051408/webrev.10/ 

src/java.base/share/classes/sun/security/provider/AbstractDrbg.java
===================================================================
line 66-68: My understanding is that ...

I would suggest rewords or remove this sentence.  "Not used much" does
not mean needing no synchronization.  As you have add synchronized
keyword for engineGenerateSeed, I may suggest you remove lines 63-68,
and move 57-61 to class description.

I think comment are mainly used for code readers.  You used a lot "my"
and "I", and questions to yourself in the comments.  Would you mind
change to use neutral tones and remove unnecessary comments in the final
code?

------
662     // 6/7. Get entropy. TODO: 1st arg security strength?

Do you want to complete the TODO task.   Similar comments to other TODO
tasks.

------
670     nonce = longToByteArray(cc.incrementAndGet());
685     private static byte[] longToByteArray(long l)

The nonce is leading with "sun.drbg", and following by a 8-bytes integer
value.  This scheme only provider 8-bytes randomness.  Looks like the
quality does not meet the nonce requirements (8.7.6 NIST SP-800-90Ar1)
for 256-bit strength.

------
598     protected final synchronized void engineConfigure(

engineConfigure is not part of SPI.  Would you mind change the name and
remove the leading "engine" in case of miss-understanding?

It would be nice grant as less accessibility as possible.  For the
"protected" keyword, should it be package accessibility at present?
Similar comment for other protected methods other than the override
ones, and some public classes in the package.

------
Section 7.2 of NIST SP 800-90Ar1 says: "The personalization string
should be unique for all instantiations of the same DRBG mechanism type".

Please check the unique for the personalization string in the
implementation.

Looking forward to your next webrev.

Xuelei




More information about the security-dev mailing list