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
