8156501: DRBG not synchronized at reseeding
authorweijun
Thu, 12 May 2016 09:49:42 +0800
changeset 37895 f59fdd7fb4fb
parent 37894 3acc9608c162
child 37896 cd841af7dcd0
8156501: DRBG not synchronized at reseeding Reviewed-by: xuelei
jdk/src/java.base/share/classes/sun/security/provider/AbstractDrbg.java
jdk/src/java.base/share/classes/sun/security/provider/CtrDrbg.java
jdk/src/java.base/share/classes/sun/security/provider/HashDrbg.java
jdk/src/java.base/share/classes/sun/security/provider/HmacDrbg.java
--- a/jdk/src/java.base/share/classes/sun/security/provider/AbstractDrbg.java	Wed May 11 17:37:11 2016 -0700
+++ b/jdk/src/java.base/share/classes/sun/security/provider/AbstractDrbg.java	Thu May 12 09:49:42 2016 +0800
@@ -51,12 +51,9 @@
  * configuration is eagerly called to set up parameters, and instantiation
  * is lazily called only when nextBytes or reseed is called.
  * <p>
- * Synchronized keyword should be added to all externally callable engine
- * methods including {@link #engineReseed}, {@link #engineSetSeed}, and
- * {@link #engineNextBytes} (but not {@link #engineGenerateSeed}).
- * Internal methods like {@link #configure} and {@link #instantiateAlgorithm}
- * are not synchronized. They will either be called in a constructor or
- * in another synchronized method.
+ * SecureRandom methods like reseed and nextBytes are not thread-safe.
+ * An implementation is required to protect shared access to instantiate states
+ * (instantiated, nonce) and DRBG states (v, c, key, reseedCounter).
  */
 public abstract class AbstractDrbg extends SecureRandomSpi {
 
@@ -78,8 +75,10 @@
      * Reseed counter of a DRBG instance. A mechanism should increment it
      * after each random bits generation and reset it in reseed. A mechanism
      * does <em>not</em> need to compare it to {@link #reseedInterval}.
+     *
+     * Volatile, will be used in a double checked locking.
      */
-    protected transient int reseedCounter = 0;
+    protected transient volatile int reseedCounter = 0;
 
     // Mech features. If not same as below, must be redefined in constructor.
 
@@ -383,9 +382,14 @@
             instantiateIfNecessary(null);
 
             // Step 7: Auto reseed
+            // Double checked locking, safe because reseedCounter is volatile
             if (reseedCounter > reseedInterval || pr) {
-                reseedAlgorithm(getEntropyInput(pr), ai);
-                ai = null;
+                synchronized (this) {
+                    if (reseedCounter > reseedInterval || pr) {
+                        reseedAlgorithm(getEntropyInput(pr), ai);
+                        ai = null;
+                    }
+                }
             }
 
             // Step 8, 10: Generate_algorithm
@@ -615,8 +619,7 @@
      * @throws IllegalArgumentException if {@code params} is
      *         inappropriate for this SecureRandom.
      */
-    protected final synchronized void configure(
-            SecureRandomParameters params) {
+    protected final void configure(SecureRandomParameters params) {
         if (debug != null) {
             debug.println(this, "configure " + this + " with " + params);
         }
--- a/jdk/src/java.base/share/classes/sun/security/provider/CtrDrbg.java	Wed May 11 17:37:11 2016 -0700
+++ b/jdk/src/java.base/share/classes/sun/security/provider/CtrDrbg.java	Thu May 12 09:49:42 2016 +0800
@@ -350,7 +350,7 @@
     }
 
     @Override
-    protected void reseedAlgorithm(
+    protected synchronized void reseedAlgorithm(
             byte[] ei,
             byte[] additionalInput) {
         if (usedf) {
--- a/jdk/src/java.base/share/classes/sun/security/provider/HashDrbg.java	Wed May 11 17:37:11 2016 -0700
+++ b/jdk/src/java.base/share/classes/sun/security/provider/HashDrbg.java	Thu May 12 09:49:42 2016 +0800
@@ -115,7 +115,7 @@
 
     // This method is used by both instantiation and reseeding.
     @Override
-    protected final void hashReseedInternal(byte[] input) {
+    protected final synchronized void hashReseedInternal(byte[] input) {
 
         // 800-90Ar1 10.1.1.2: Instantiate Process.
         // 800-90Ar1 10.1.1.3: Reseed Process.
--- a/jdk/src/java.base/share/classes/sun/security/provider/HmacDrbg.java	Wed May 11 17:37:11 2016 -0700
+++ b/jdk/src/java.base/share/classes/sun/security/provider/HmacDrbg.java	Thu May 12 09:49:42 2016 +0800
@@ -115,7 +115,7 @@
 
     // This method is used by both instantiation and reseeding.
     @Override
-    protected final void hashReseedInternal(byte[] input) {
+    protected final synchronized void hashReseedInternal(byte[] input) {
 
         // 800-90Ar1 10.1.2.3: Instantiate Process.
         // 800-90Ar1 10.1.2.4: Reseed Process.