More constantTimeAreEquals refactoring

maxsom · Oct 21, 2019 · 5a37e83 · 5a37e83
1 parent 6a00a8c
commit 5a37e83
Show file tree

Hide file tree

Showing 6 changed files with 33 additions and 34 deletions.
diff --git a/tls/src/main/java/org/bouncycastle/tls/TlsUtils.java b/tls/src/main/java/org/bouncycastle/tls/TlsUtils.java
@@ -1370,6 +1370,16 @@ public static TlsSecret PRF(TlsContext context, TlsSecret secret, String asciiLa
         return PRF(context.getSecurityParametersHandshake(), secret, asciiLabel, seed, length);
     }
 
+    public static boolean constantTimeAreEqual(int len, byte[] a, int aOff, byte[] b, int bOff)
+    {
+        int d = 0;
+        for (int i = 0; i < len; ++i)
+        {
+            d |= (a[aOff + i] ^ b[bOff + i]);
+        }
+        return 0 == d;
+    }
+
     public static byte[] copyOfRangeExact(byte[] original, int from, int to)
     {
         int newLength = to - from;
@@ -3959,8 +3969,8 @@ static TlsCredentialedSigner requireSignerCredentials(TlsCredentials credentials
 
     private static void checkDowngradeMarker(byte[] randomBlock, byte[] downgradeMarker) throws IOException
     {
-        byte[] bytes = copyOfRangeExact(randomBlock, randomBlock.length - downgradeMarker.length, randomBlock.length);
-        if (Arrays.constantTimeAreEqual(bytes, downgradeMarker))
+        int len = downgradeMarker.length;
+        if (constantTimeAreEqual(len, downgradeMarker, 0, randomBlock, randomBlock.length - len))
         {
             throw new TlsFatalAlert(AlertDescription.illegal_parameter);
         }

diff --git a/tls/src/main/java/org/bouncycastle/tls/crypto/impl/TlsBlockCipher.java b/tls/src/main/java/org/bouncycastle/tls/crypto/impl/TlsBlockCipher.java
@@ -7,6 +7,7 @@
 import org.bouncycastle.tls.ProtocolVersion;
 import org.bouncycastle.tls.SecurityParameters;
 import org.bouncycastle.tls.TlsFatalAlert;
+import org.bouncycastle.tls.TlsUtils;
 import org.bouncycastle.tls.crypto.TlsCipher;
 import org.bouncycastle.tls.crypto.TlsCrypto;
 import org.bouncycastle.tls.crypto.TlsCryptoParameters;
@@ -302,7 +303,7 @@ public TlsDecodeResult decodeCiphertext(long seqNo, short contentType, byte[] ci
         {
             byte[] expectedMac = readMac.calculateMac(seqNo, contentType, ciphertext, offset, len - macSize);
 
-            boolean badMac = !TlsImplUtils.constantTimeAreEqual(macSize, expectedMac, 0, ciphertext,
+            boolean badMac = !TlsUtils.constantTimeAreEqual(macSize, expectedMac, 0, ciphertext,
                 offset + len - macSize);
             if (badMac)
             {
@@ -341,8 +342,7 @@ public TlsDecodeResult decodeCiphertext(long seqNo, short contentType, byte[] ci
             byte[] expectedMac = readMac.calculateMacConstantTime(seqNo, contentType, ciphertext, offset, dec_output_length,
                 blocks_length - macSize, randomData);
 
-            badMac |= !TlsImplUtils.constantTimeAreEqual(macSize, expectedMac, 0, ciphertext,
-                offset + dec_output_length);
+            badMac |= !TlsUtils.constantTimeAreEqual(macSize, expectedMac, 0, ciphertext, offset + dec_output_length);
         }
 
         if (badMac)

diff --git a/tls/src/main/java/org/bouncycastle/tls/crypto/impl/TlsImplUtils.java b/tls/src/main/java/org/bouncycastle/tls/crypto/impl/TlsImplUtils.java
@@ -76,14 +76,4 @@ public static TlsSecret PRF(TlsCryptoParameters cryptoParams, TlsSecret secret,
     {
         return PRF(cryptoParams.getSecurityParametersHandshake(), secret, asciiLabel, seed, length);
     }
-
-    static boolean constantTimeAreEqual(int len, byte[] a, int aOff, byte[] b, int bOff)
-    {
-        int d = 0;
-        for (int i = 0; i < len; ++i)
-        {
-            d |= (a[aOff + i] ^ b[bOff + i]);
-        }
-        return 0 == d;
-    }
 }
diff --git a/tls/src/main/java/org/bouncycastle/tls/crypto/impl/TlsNullCipher.java b/tls/src/main/java/org/bouncycastle/tls/crypto/impl/TlsNullCipher.java
@@ -4,6 +4,7 @@
 
 import org.bouncycastle.tls.AlertDescription;
 import org.bouncycastle.tls.TlsFatalAlert;
+import org.bouncycastle.tls.TlsUtils;
 import org.bouncycastle.tls.crypto.TlsCipher;
 import org.bouncycastle.tls.crypto.TlsCryptoParameters;
 import org.bouncycastle.tls.crypto.TlsDecodeResult;
@@ -88,7 +89,7 @@ public TlsDecodeResult decodeCiphertext(long seqNo, short contentType, byte[] ci
 
         byte[] expectedMac = readMac.calculateMac(seqNo, contentType, ciphertext, offset, macInputLen);
 
-        boolean badMac = !TlsImplUtils.constantTimeAreEqual(macSize, expectedMac, 0, ciphertext, offset + macInputLen);
+        boolean badMac = !TlsUtils.constantTimeAreEqual(macSize, expectedMac, 0, ciphertext, offset + macInputLen);
         if (badMac)
         {
             throw new TlsFatalAlert(AlertDescription.bad_record_mac);

diff --git a/tls/src/main/java/org/bouncycastle/tls/crypto/impl/bc/BcChaCha20Poly1305.java b/tls/src/main/java/org/bouncycastle/tls/crypto/impl/bc/BcChaCha20Poly1305.java
@@ -58,15 +58,14 @@ public int doFinal(byte[] input, int inputOffset, int inputLength, byte[] output
 
             updateMAC(input, inputOffset, ciphertextLength);
 
-            byte[] calculatedMAC = new byte[16];
-            Pack.longToLittleEndian(additionalDataLength & 0xFFFFFFFFL, calculatedMAC, 0);
-            Pack.longToLittleEndian(ciphertextLength & 0xFFFFFFFFL, calculatedMAC, 8);
-            mac.update(calculatedMAC, 0, 16);
-            mac.doFinal(calculatedMAC, 0);
-
-            byte[] receivedMAC = TlsUtils.copyOfRangeExact(input, inputOffset + ciphertextLength, inputOffset + inputLength);
-
-            if (!Arrays.constantTimeAreEqual(calculatedMAC, receivedMAC))
+            byte[] expectedMac = new byte[16];
+            Pack.longToLittleEndian(additionalDataLength & 0xFFFFFFFFL, expectedMac, 0);
+            Pack.longToLittleEndian(ciphertextLength & 0xFFFFFFFFL, expectedMac, 8);
+            mac.update(expectedMac, 0, 16);
+            mac.doFinal(expectedMac, 0);
+
+            boolean badMac = !TlsUtils.constantTimeAreEqual(16, expectedMac, 0, input, inputOffset + ciphertextLength);
+            if (badMac)
             {
                 throw new TlsFatalAlert(AlertDescription.bad_record_mac);
             }

diff --git a/tls/src/main/java/org/bouncycastle/tls/crypto/impl/jcajce/JceChaCha20Poly1305.java b/tls/src/main/java/org/bouncycastle/tls/crypto/impl/jcajce/JceChaCha20Poly1305.java
@@ -15,7 +15,6 @@
 import org.bouncycastle.tls.TlsFatalAlert;
 import org.bouncycastle.tls.TlsUtils;
 import org.bouncycastle.tls.crypto.impl.TlsAEADCipherImpl;
-import org.bouncycastle.util.Arrays;
 import org.bouncycastle.util.Pack;
 
 public class JceChaCha20Poly1305 implements TlsAEADCipherImpl
@@ -114,15 +113,15 @@ public int doFinal(byte[] input, int inputOffset, int inputLength, byte[] output
                 updateMAC(additionalData, 0, additionalData.length);
                 updateMAC(input, inputOffset, ciphertextLength);
 
-                byte[] calculatedMAC = new byte[16];
-                Pack.longToLittleEndian(additionalData.length & 0xFFFFFFFFL, calculatedMAC, 0);
-                Pack.longToLittleEndian(ciphertextLength & 0xFFFFFFFFL, calculatedMAC, 8);
-                mac.update(calculatedMAC, 0, 16);
-                mac.doFinal(calculatedMAC, 0);
+                byte[] expectedMac = new byte[16];
+                Pack.longToLittleEndian(additionalData.length & 0xFFFFFFFFL, expectedMac, 0);
+                Pack.longToLittleEndian(ciphertextLength & 0xFFFFFFFFL, expectedMac, 8);
+                mac.update(expectedMac, 0, 16);
+                mac.doFinal(expectedMac, 0);
 
-                byte[] receivedMAC = TlsUtils.copyOfRangeExact(input, inputOffset + ciphertextLength, inputOffset + inputLength);
-
-                if (!Arrays.constantTimeAreEqual(calculatedMAC, receivedMAC))
+                boolean badMac = !TlsUtils.constantTimeAreEqual(16, expectedMac, 0, input,
+                    inputOffset + ciphertextLength);
+                if (badMac)
                 {
                     throw new TlsFatalAlert(AlertDescription.bad_record_mac);
                 }