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