Merge branch 'main' of gitlab.cryptoworkshop.com:root/bc-java

Author: dghgit

docs/releasenotes.html

@@ -24,6 +24,7 @@ <h2>2.0 Release History</h2>
 <h3>2.1.2 Defects Fixed</h3>
 <ul>
 <li>SNOVA and MAYO are now correctly added to the JCA provider module-info file.</li>
+<li>TLS: Avoid nonce reuse error in JCE AEAD workaround for pre-Java7.</li>
 </ul>
 <h3>2.1.3 Additional Features and Functionality</h3>
 <ul>

tls/src/main/java/org/bouncycastle/jsse/provider/ProvSSLSessionBase.java

@@ -3,9 +3,7 @@
 import java.security.Principal;
 import java.security.cert.Certificate;
 import java.security.cert.X509Certificate;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.atomic.AtomicLong;
 import java.util.concurrent.atomic.AtomicReference;
 
@@ -27,7 +25,7 @@
 abstract class ProvSSLSessionBase
     extends BCExtendedSSLSession
 {
-    protected final Map<String, Object> valueMap = Collections.synchronizedMap(new HashMap<String, Object>());
+    protected final ConcurrentHashMap<String, Object> valueMap = new ConcurrentHashMap<String, Object>();
 
     protected final AtomicReference<ProvSSLSessionContext> sslSessionContext;
     protected final boolean fipsMode;
@@ -237,15 +235,17 @@ public SSLSessionContext getSessionContext()
 
     public Object getValue(String name)
     {
+        if (name == null)
+        {
+            throw new IllegalArgumentException("'name' cannot be null");
+        }
+
         return valueMap.get(name);
     }
 
     public String[] getValueNames()
     {
-        synchronized (valueMap)
-        {
-            return valueMap.keySet().toArray(new String[valueMap.size()]);
-        }
+        return valueMap.keySet().toArray(new String[0]);
     }
 
     @Override
@@ -287,12 +287,26 @@ public boolean isValid()
 
     public void putValue(String name, Object value)
     {
+        if (name == null)
+        {
+            throw new IllegalArgumentException("'name' cannot be null");
+        }
+        if (value == null)
+        {
+            throw new IllegalArgumentException("'value' cannot be null");
+        }
+
         notifyUnbound(name, valueMap.put(name, value));
         notifyBound(name, value);
     }
 
     public void removeValue(String name)
     {
+        if (name == null)
+        {
+            throw new IllegalArgumentException("'name' cannot be null");
+        }
+
         notifyUnbound(name, valueMap.remove(name));
     }
 

tls/src/main/java/org/bouncycastle/tls/DTLSClientProtocol.java

@@ -152,7 +152,8 @@ protected DTLSTransport clientHandshake(ClientHandshakeState state)
 
             handshake.finish();
 
-            if (securityParameters.isExtendedMasterSecret())
+            if (securityParameters.isExtendedMasterSecret() &&
+                ProtocolVersion.DTLSv12.isEqualOrLaterVersionOf(securityParameters.getNegotiatedVersion()))
             {
                 securityParameters.tlsUnique = securityParameters.getPeerVerifyData();
             }
@@ -380,7 +381,10 @@ protected DTLSTransport clientHandshake(ClientHandshakeState state)
 
         state.tlsSession = TlsUtils.importSession(securityParameters.getSessionID(), state.sessionParameters);
 
-        securityParameters.tlsUnique = securityParameters.getLocalVerifyData();
+        if (ProtocolVersion.DTLSv12.isEqualOrLaterVersionOf(securityParameters.getNegotiatedVersion()))
+        {
+            securityParameters.tlsUnique = securityParameters.getLocalVerifyData();
+        }
 
         clientContext.handshakeComplete(client, state.tlsSession);
 

tls/src/main/java/org/bouncycastle/tls/DTLSServerProtocol.java

@@ -161,7 +161,8 @@ protected DTLSTransport serverHandshake(ServerHandshakeState state, DTLSRequest
 
             handshake.finish();
 
-            if (securityParameters.isExtendedMasterSecret())
+            if (securityParameters.isExtendedMasterSecret() &&
+                ProtocolVersion.DTLSv12.isEqualOrLaterVersionOf(securityParameters.getNegotiatedVersion()))
             {
                 securityParameters.tlsUnique = securityParameters.getLocalVerifyData();
             }
@@ -409,7 +410,10 @@ protected DTLSTransport serverHandshake(ServerHandshakeState state, DTLSRequest
 
         state.tlsSession = TlsUtils.importSession(securityParameters.getSessionID(), state.sessionParameters);
 
-        securityParameters.tlsUnique = securityParameters.getPeerVerifyData();
+        if (ProtocolVersion.DTLSv12.isEqualOrLaterVersionOf(securityParameters.getNegotiatedVersion()))
+        {
+            securityParameters.tlsUnique = securityParameters.getPeerVerifyData();
+        }
 
         serverContext.handshakeComplete(server, state.tlsSession);
 

tls/src/main/java/org/bouncycastle/tls/crypto/impl/jcajce/JceAEADCipherImpl.java

@@ -6,6 +6,7 @@
 import java.security.GeneralSecurityException;
 import java.security.PrivilegedAction;
 import java.security.SecureRandom;
+import java.security.spec.AlgorithmParameterSpec;
 
 import javax.crypto.Cipher;
 import javax.crypto.SecretKey;
@@ -43,6 +44,7 @@ public Object run()
         });
     }
 
+    // TODO[tls] Once Java 7 or higher is the baseline, this will always be true
     private static final boolean canDoAEAD = checkForAEAD();
 
     private static String getAlgParamsName(JcaJceHelper helper, String cipherName)
@@ -68,8 +70,10 @@ private static String getAlgParamsName(JcaJceHelper helper, String cipherName)
     private final String algorithmParamsName;
 
     private SecretKey key;
-    private byte[]    nonce;
-    private int       macSize;
+
+    // TODO[tls] These two are only needed while the baseline is pre-Java7
+    private byte[] noncePre7;
+    private int macSizePre7;
 
     public JceAEADCipherImpl(JcaTlsCrypto crypto, JcaJceHelper helper, String cipherName, String algorithm, int keySize,
         boolean isEncrypting)
@@ -120,10 +124,27 @@ public void init(byte[] nonce, int macSize)
             }
             else
             {
-                // Otherwise fall back to the BC-specific AEADParameterSpec
-                cipher.init(cipherMode, key, new AEADParameterSpec(nonce, macSize * 8, null), random);
-                this.nonce = Arrays.clone(nonce);
-                this.macSize = macSize;
+                /*
+                 * Otherwise fall back to the BC-specific AEADParameterSpec. Since updateAAD is not available, we
+                 * need to use init to pass the associated data (in doFinal), but in order to call getOutputSize we
+                 * technically need to init the cipher first. So we init with a dummy nonce to avoid duplicate nonce
+                 * error from the init in doFinal.
+                 */
+
+                if (this.noncePre7 == null || this.noncePre7.length != nonce.length)
+                {
+                    this.noncePre7 = new byte[nonce.length];
+                }
+
+                System.arraycopy(nonce, 0, this.noncePre7, 0, nonce.length);
+                this.macSizePre7 = macSize;
+
+                this.noncePre7[0] ^= 0x80;
+
+                AlgorithmParameterSpec params = new AEADParameterSpec(noncePre7, macSizePre7 * 8, null);
+                cipher.init(cipherMode, key, params, random);
+
+                this.noncePre7[0] ^= 0x80;
             }
         }
         catch (Exception e)
@@ -150,7 +171,11 @@ public int doFinal(byte[] additionalData, byte[] input, int inputOffset, int inp
             {
                 try
                 {
-                    cipher.init(cipherMode, key, new AEADParameterSpec(nonce, macSize * 8, additionalData));
+                    // NOTE: Shouldn't need a SecureRandom, but this is cheaper if the provider would auto-create one
+                    SecureRandom random = crypto.getSecureRandom();
+
+                    AlgorithmParameterSpec params = new AEADParameterSpec(noncePre7, macSizePre7 * 8, additionalData);
+                    cipher.init(cipherMode, key, params, random);
                 }
                 catch (Exception e)
                 {