Update #4: JEP 123: SecureRandom Draft and Implementation. (original) (raw)

Xuelei Fan xuelei.fan at oracle.com
Thu Jan 17 15:44:33 UTC 2013

Hi Brad,

Please note the priorities for each comment. As the M6 is coming, you can only take P1 comments.

P1: need to update P2: suggested update, you can take it after M6. P3: minor comments, it's OK to leave it unchanged. P4: personal preference for your consideration, or my question.

java/security/SecureRandom.java

1-1. the performance of Pattern.compile [P2] Pattern.compile() is expensive. I would suggest to use private static lazy-initialized class attribute the patterns.

public class SecureRandom extends java.util.Random { private static final String regex = "..."; private static final Pattern pattern;

   public static SecureRandom getStrongSecureRandom() {
       ...
       if (pattern == null) {
           pattern = Pattern.compile(regex);
       }
       ...
   }

}

1-2. spaces are allowed between algorithm and provider [P4] According to the regex ("\s*(\S+)\s\:\s(\S+)\s*"), spaces are allowed around the tokens. For example, " NativePRNGBlocking : SUN " is valid. I would like to use a stricter syntax at the beginning in case of any special requirement comes in the future.

1-3. may only need one regex for both "algorithm" and "algorithm:provider" [P3] I think one regex is OK for both: "([\S&&[^:]]+)(\:([\S&&[^:]]+))?".

NativePRNGBlocking:Sun group 1: NativePRNGBlocking group 2: :Sun group 3: Sun

NativePRNGBlocking group 1: NativePRNGBlocking group 2: null group 3: null

If group 2 is non-null, it is of the "algorithm:provider" style.

1-4. a typo at line 614 [P1]:

java.security-windows

2-1. Is it possible to enable "NativePRNGBlocking:SUN" in Windows? [P2]

I was wondering to enable "NativePRNGBlocking:SUN" here before I know that the "NativePRNGBlocking:SUN" is not available on Windows:

The availability of "securerandom.strongAlgorithms" property depends on the enabled security providers, and the platform. If "SunMSCAPI" provider is not enabled, the "Windows-PRNG:SunMSCAPI" will not work. I think "NativePRNGBlocking:SUN" is not available on Windows system. It is not as obviously as that the "Windows-PRNG:SunMSCAPI" is not available on Unix/Linux/Mac OS systems. We need to documentation this behaviors clear somewhere else.

P11SecureRandom.java

3-1: to support strong algorithm in PKCS11 [P4] Is SHA1PRNG:SunPKCS11 a strong algorithm? I think it would be nice to add it as a backup in the strong algorithm property.

SeedGenerator.java

4-1: downgrade normative reference to java security property file [P3] [line 57-60] As you have already there, I would suggest to use the new style of security property. See http://hg.openjdk.java.net/jdk8/tl/jdk/rev/346c0af4af41 and the description from SeanM, http://mail.openjdk.java.net/pipermail/security-dev/2012-December/006144.html.

line 57-60:

SunEntries.java

5-1: what's the usage of "NativePRNGNonBlocking"? [P2]

I did not find the description of this algorithm in the specification (CCC) or other export documentation. Do you want to add it to Oracle provider names doc? Otherwise, I would suggest to comment out this algorithm. The above would set a external SecureRandom algorithm, I think.

sun/security/provider/NativePRNG.java

6-1: line 36-42, wordsmithing. [P3]

"It obtains seed and random numbers by reading system files such as the special device files /dev/random and /dev/urandom. This implementation respects the {@code securerandom.source} security property and {@code java.security.egd} system property for obtaining seed material. If the file specified by the properties does not exist, /dev/random is the default seed source, and /dev/urandom is the default source of random numbers."

6-2: Do you want to put something here? [P4]

321 // XXX change the urandom/random to seed/next

src/windows/classes/sun/security/provider/NativeSeedGenerator.java

7-1: Not about this fix, but the code looks strange to me. [P4] The constructor calls: 44 super();

The SeedGenerator static block will be called and SeedGenerator.instance will be initialized.

According to the code in SeedGenerator.java: 145 static public void generateSeed(byte[] result) { 146 instance.getSeedBytes(result); 147 }

The getSeedBytes() method of the initialized instance will be used. However, in Windows platform, I think the NativeSeedGenerator.getSeedBytes() should be called, I think.

I think the NativeSeedGenerator class should override the generateSeed() method. About the super() call in NativeSeedGenerator, I think the initialization of instance (line 92-142, SeedGenerator.java) may be not necessary. The initialized instance in SeedGenerator is useless in Windows from my understand. Am I missing something?

Otherwise, looks fine to me.

Xuelei

On 1/11/2013 8:24 AM, Brad Wetmore wrote:

Minor tweak. It occurred to me that people might use "." as separators (for example using some OIDs scheme), so I changed the syntax slightly of the system property to use ":" instead.

For example: # This is a comma-separated list of algorithm and/or algorithm:provider # entries. # securerandom.strongAlgorithms=NativePRNGBlocking:SUN, SP800-90A/AES/CTR:IBMJDK Latest is now webrev.04. http://cr.openjdk.java.net/~wetmore/6425477/ Brad

More information about the security-dev mailing list