- Refactor the safe premaster decryption to avoid the

TlsEncryptionCredentils API change.
- Reset the client version number "check" to previous method

4 files changed, 24 insertions, 45 deletions

diff --git a/core/src/main/java/org/bouncycastle/crypto/tls/DefaultTlsEncryptionCredentials.java b/core/src/main/java/org/bouncycastle/crypto/tls/DefaultTlsEncryptionCredentials.java
index 6eaa75e3..54bcd90c 100644
--- a/core/src/main/java/org/bouncycastle/crypto/tls/DefaultTlsEncryptionCredentials.java
+++ b/core/src/main/java/org/bouncycastle/crypto/tls/DefaultTlsEncryptionCredentials.java
@@ -54,23 +54,9 @@ public class DefaultTlsEncryptionCredentials extends AbstractTlsEncryptionCreden
         return certificate;
     }
 
-    public byte[] decryptPreMasterSecret(byte[] encryptedPreMasterSecret, byte[] fallback)
+    public byte[] decryptPreMasterSecret(byte[] encryptedPreMasterSecret)
         throws IOException
     {
-        PKCS1Encoding encoding = new PKCS1Encoding(new RSABlindedEngine(), fallback);
-        encoding.init(false, new ParametersWithRandom(this.privateKey, context.getSecureRandom()));
-
-        try
-        {
-            return encoding.processBlock(encryptedPreMasterSecret, 0,
-                encryptedPreMasterSecret.length);
-        }
-        catch (InvalidCipherTextException e)
-        {
-            /*
-             * This should never happen, the decryption should always succeed, or return a random value.
-             */
-            throw new TlsFatalAlert(AlertDescription.illegal_parameter);
-        }
+        return TlsRSAUtils.safeDecryptPreMasterSecret(context, (RSAKeyParameters)privateKey, encryptedPreMasterSecret);
     }
 }
diff --git a/core/src/main/java/org/bouncycastle/crypto/tls/TlsEncryptionCredentials.java b/core/src/main/java/org/bouncycastle/crypto/tls/TlsEncryptionCredentials.java
index 27b8e232..f2928963 100644
--- a/core/src/main/java/org/bouncycastle/crypto/tls/TlsEncryptionCredentials.java
+++ b/core/src/main/java/org/bouncycastle/crypto/tls/TlsEncryptionCredentials.java
@@ -4,6 +4,6 @@ import java.io.IOException;
 
 public interface TlsEncryptionCredentials extends TlsCredentials
 {
-    byte[] decryptPreMasterSecret(byte[] encryptedPreMasterSecret, byte[] fallback)
+    byte[] decryptPreMasterSecret(byte[] encryptedPreMasterSecret)
         throws IOException;
 }
diff --git a/core/src/main/java/org/bouncycastle/crypto/tls/TlsRSAKeyExchange.java b/core/src/main/java/org/bouncycastle/crypto/tls/TlsRSAKeyExchange.java
index 89709685..cd3eb0c3 100644
--- a/core/src/main/java/org/bouncycastle/crypto/tls/TlsRSAKeyExchange.java
+++ b/core/src/main/java/org/bouncycastle/crypto/tls/TlsRSAKeyExchange.java
@@ -130,7 +130,7 @@ public class TlsRSAKeyExchange
             encryptedPreMasterSecret = TlsUtils.readOpaque16(input);
         }
 
-        this.premasterSecret = TlsRSAUtils.safeDecryptPreMasterSecret(context, serverCredentials, encryptedPreMasterSecret);
+        this.premasterSecret = serverCredentials.decryptPreMasterSecret(encryptedPreMasterSecret);
     }
 
     public byte[] generatePremasterSecret()
diff --git a/core/src/main/java/org/bouncycastle/crypto/tls/TlsRSAUtils.java b/core/src/main/java/org/bouncycastle/crypto/tls/TlsRSAUtils.java
index 1d680f01..df54e1d9 100644
--- a/core/src/main/java/org/bouncycastle/crypto/tls/TlsRSAUtils.java
+++ b/core/src/main/java/org/bouncycastle/crypto/tls/TlsRSAUtils.java
@@ -49,7 +49,16 @@ public class TlsRSAUtils
         return premasterSecret;
     }
 
+    /**
+     * @deprecated {@link TlsEncryptionCredentials#decryptPreMasterSecret(byte[])} is expected to decrypt safely
+     */
     public static byte[] safeDecryptPreMasterSecret(TlsContext context, TlsEncryptionCredentials encryptionCredentials,
+        byte[] encryptedPreMasterSecret) throws IOException
+    {
+        return encryptionCredentials.decryptPreMasterSecret(encryptedPreMasterSecret);
+    }
+
+    public static byte[] safeDecryptPreMasterSecret(TlsContext context, RSAKeyParameters rsaServerPrivateKey,
         byte[] encryptedPreMasterSecret)
     {
         /*
@@ -61,22 +70,19 @@ public class TlsRSAUtils
         boolean versionNumberCheckDisabled = false;
 
         /*
-         * Bleichenbacher side channel countermeasures have been implemented in
-         * decryptPreMasterSecret, so we con't need to do something here.
-         */
-        byte[] M = TlsUtils.EMPTY_BYTES;
-
-
-        /*
          * Generate 48 random bytes we can use as a Pre-Master-Secret, if the
          * PKCS1 padding check should fail.
          */
-        byte[] fallback = new byte[48];
-        context.getSecureRandom().nextBytes(fallback);
+        byte[] M = new byte[48];
+        context.getSecureRandom().nextBytes(M);
 
         try
         {
-            M = encryptionCredentials.decryptPreMasterSecret(encryptedPreMasterSecret, fallback);
+            PKCS1Encoding encoding = new PKCS1Encoding(new RSABlindedEngine(), M);
+            encoding.init(false, new ParametersWithRandom(rsaServerPrivateKey, context.getSecureRandom()));
+
+            M = encoding.processBlock(encryptedPreMasterSecret, 0,
+                encryptedPreMasterSecret.length);
         }
         catch (Exception e)
         {
@@ -106,24 +112,11 @@ public class TlsRSAUtils
              */
         } else {
             /*
-             * OK, we need to compare the version number in the decrypted
-             * Pre-Master-Secret with the clientVersion received during the
-             * handshake. If they don't match, we replace the decrypted
-             * Pre-Master-Secret with a random one.
-             */
-            int correct = (clientVersion.getMajorVersion() ^ (M[0]&0xff)) | (clientVersion.getMinorVersion() ^ (M[1]&0xff));
-            correct |= correct>>1;
-            correct |= correct>>2;
-            correct |= correct>>4;
-            int mask = ~((correct & 1) - 1);
-
-            /*
-             * mask will be all bits set to 0xff if the version number differed.
+             * Note that explicitly constructing the pre_master_secret with the
+             * ClientHello.client_version produces an invalid master_secret if the client
+             * has sent the wrong version in the original pre_master_secret.
              */
-
-            for (int i = 0; i < 48; i++) {
-                M[i] = (byte)((M[i]&(~mask))|(fallback[i]&mask));
-            }
+            TlsUtils.writeVersion(clientVersion, M, 0);
         }
         return M;
     }