Update #5: JEP 123: SecureRandom Draft and Implementation.
Brad Wetmore
bradford.wetmore at oracle.com
Wed Apr 10 07:17:47 UTC 2013
Hi Xuelei/Weijun,
Thanks for the comments.
The version I plan to putback is:
http://cr.openjdk.java.net/~wetmore/6425477/webrev.05/
I probably won't have time to take any further comments before vacation,
but can address things when I'm back. I have a few more from Bernd that
I'd like to look at.
On 1/17/2013 7:44 AM, Xuelei Fan wrote:
> 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.
I think I've addressed all.
> 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);
> }
> ...
> }
> }
Good point. Other people mentioned this privately.
BTW, as you have above, this Pattern can't be final (not a constructor),
and needs to be volatile. To avoid multiple initializations, I went
with the Effective Java 2nd Edition Item 71 (FileHolder) pattern instead.
> 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.
Changed. Weijun also mentioned this. I now allow:
* Entries are alg:prov separated by ,
* Allow for prepended/appended whitespace between entries.
> 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.
I modified this quite a bit and now the entire parsing logic is in one
regexp. I iterate over it until group(5) is null.
* Capture groups:
* 1 - alg
* 2 - :prov (optional)
* 3 - prov (optional)
* 4 - ,nextEntry (optional)
* 5 - nextEntry (optional)
\\s*([\\S&&[^:,]]*)(\\:([\\S&&[^,]]*))?\\s*(\\,(.*))?
> 1-4. a typo at line 614 [P1]:
> - 614 // Pattern for "algorithm.provider"
> + 614 // Pattern for "algorithm:provider"
Done.
> java.security-windows
> -------------------------------------
> 2-1. Is it possible to enable "NativePRNGBlocking:SUN" in Windows? [P2]
To my knowledge, there is no concept of Blocking/NonBlocking or
quality-of-service levels in the SunMSCAPI CryptGenRandom API. There is
just one implementation (Vista SP1+: AES/NIST SP 800-90, earlier uses
FIPS 186-2.)
> + securerandom.strongAlgorithms=Windows-PRNG:SunMSCAPI
>
> I was wondering to enable "NativePRNGBlocking:SUN" here before I know
> that the "NativePRNGBlocking:SUN" is not available on Windows:
> + securerandom.strongAlgorithms=Windows-PRNG:SunMSCAPI, \
> + NativePRNGBlocking:SUN
>
> The availability of "securerandom.strongAlgorithms" property depends on
> the enabled security providers, and the platform.
Yes.
> If "SunMSCAPI"
> provider is not enabled, the "Windows-PRNG:SunMSCAPI" will not work. I
> think "NativePRNGBlocking:SUN" is not available on Windows system.
Correct.
> 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.
I'm planning to have a new section in the Sun Providers document which
describes the various implementations available on which platforms, and
we'll use my internal SecureRandom wiki page as the base.
I'm thinking seriously about adding a NativePRNG in the Sun Provider to
point to similar functionality that is in Windows-PRNG:SunMSCAPI. That
is, SHA1PRNG and NativePRNG would exist on all Oracle platforms.
JDK-8011737: Consider adding a NativePRNG equivalent,
probably based on MSCAPI code.
Of course, I'd also like to add
JDK-8003584: Consider adding a more modern SecureRandom
implementation
> P11SecureRandom.java
> -------------------------------------
> 3-1: to support strong algorithm in PKCS11 [P4]
> Is SHA1PRNG:SunPKCS11 a strong algorithm?
There is no SHA1PRNG in PKCS11. The PKCS11 SecureRandom impl reads
directly from the PKCS11 library. If setSeed was ever called, then use
a "mixer" SHA1PRNG to combine the seed with bytes from the PKCS11 impl.
> I think it would be nice to
> add it as a backup in the strong algorithm property.
One question which I'll answer but you didn't actually ask. :)
AFAIK, SHA1PRNG:SUN has never undergone a formal evaluation for its
strength, so I hesitated including it in the list. All indications,
anecdotal reports, and some graduate level research have shown it is
quite strong, but it was a homebrew algorithm developed back in the days
when export control restricted what we could include in the JDK.
For UNIXn, I'm thinking of sticking with the NonBlocking OS
implementations which return values only when "sufficient entropy has
been collected," and the MSCAPI which uses NIS 800-90.
> 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:
> - * accomplished by setting the value of the "securerandom.source"
> - * Security property (in the Java security properties file) to a URL
> - * specifying the location of the entropy gathering device, or by
setting
> - * the "java.security.egd" System property.
>
> + * accomplished by setting the value of the
> + * {@code securerandom.source} security property to a URL
> + * specifying the location of the entropy gathering device, or by
setting
> + * the {@code java.security.egd} System property.
I also took out the following paragraph, which also talked specifically
about the file and its location.
> SunEntries.java
> -------------------------------------
> 5-1: what's the usage of "NativePRNGNonBlocking"? [P2]
>
> + if (NativePRNG.NonBlocking.isAvailable()) {
> + map.put("SecureRandom.NativePRNGNonBlocking",
> + "sun.security.provider.NativePRNG$NonBlocking");
> + }
>
> 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?
Yes, it will be added.
> Otherwise, I would suggest to comment out this
> algorithm. The above would set a external SecureRandom algorithm, I
think.
Yes, it does.
I talked around this in the CCC, but I didn't actually spell out the new
names. That's an oversight that probably needs to be corrected. I've
sent Joe Darcy an email. Here's the current CCC description:
NativePRNG reads seeds from /dev/random and nextBytes from
/dev/urandom. I added two new NativePRNG implementations which are
completely blocking or nonblocking. The
"securerandom.strongAlgorithms" property points to the blocking
variant.
NativePRNG reads seeds from securerandom.source/java.security.egd
(default: /dev/random), and nextBytes from /dev/urandom
NativePRNGBlocking reads both seed and nextBytes from /dev/random
NativePRNGNonBlocking reads both seed and nextBytes from /dev/urandom.
> 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."
Thanks.
> 6-2: Do you want to put something here? [P4]
>
> 321 // XXX change the urandom/random to seed/next
Yes. XXX was my marker to not forget that. I have finished the
variable renaming and updated the comments.
> 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();
This super() call could be left out, since there is just the
SeedGenerator default constructor. In the Solaris/UNIX equivalent, it
extends URLSeedGenerator, which does have a 1-arg constructor.
> 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?
Let's see if I can explain sufficiently.
When SeedGenerator.generateSeed() is called from
sun.security.SecureRandom, a single class instance of the SeedGenerator
is created via the static initializer. There are three possible
variants, Native/URL/Threaded. Depending on the values of the
egdSource, we'll select one of the three, and set the global "instance"
variable accordingly. Calls to SeedGenerator.generateSeed() are then
rerouted to the underlying instance.getSeedBytes(). getSeedBytes is an
abstract method in SeedGenerator, but concrete implementations are
available in all three variants.
Hope that helps.
Brad
> 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