Merge branch 'pkcs15-decoding' of https://github.com/eriktews/bc-java into eriktews-pkcs15-decoding

4 files changed, 203 insertions, 34 deletions

diff --git a/core/src/main/java/org/bouncycastle/crypto/encodings/PKCS1Encoding.java b/core/src/main/java/org/bouncycastle/crypto/encodings/PKCS1Encoding.java
index 09f1537e..33ff5b2e 100644
--- a/core/src/main/java/org/bouncycastle/crypto/encodings/PKCS1Encoding.java
+++ b/core/src/main/java/org/bouncycastle/crypto/encodings/PKCS1Encoding.java
@@ -34,6 +34,8 @@ public class PKCS1Encoding
     private boolean                 forEncryption;
     private boolean                 forPrivateKey;
     private boolean                 useStrictLength;
+    private int                     pLen = -1;
+    private byte[]                  fallback = null;
 
     /**
      * Basic constructor.
@@ -46,6 +48,42 @@ public class PKCS1Encoding
         this.useStrictLength = useStrict();
     }   
 
+    /**
+     * Constructor for decryption with a fixed plaintext length.
+     * 
+     * @param cipher The cipher to use for cryptographic operation.
+     * @param pLen Length of the expected plaintext.
+     */
+    public PKCS1Encoding(
+        AsymmetricBlockCipher   cipher,
+        int pLen)
+    {
+        this.engine = cipher;
+        this.useStrictLength = useStrict();
+        this.pLen = pLen;
+    }
+
+	/**
+	 * Constructor for decryption with a fixed plaintext length and a fallback
+	 * value that is returned, if the padding is incorrect.
+	 * 
+	 * @param cipher
+	 *            The cipher to use for cryptographic operation.
+	 * @param fallback
+	 *            The fallback value, we don't to a arraycopy here.
+	 */
+	public PKCS1Encoding(
+    	AsymmetricBlockCipher   cipher,
+        byte[] fallback)
+    {
+    	this.engine = cipher;
+    	this.useStrictLength = useStrict();
+    	this.fallback = fallback;
+    	this.pLen = fallback.length;
+    }
+        
+
+    
     //
     // for J2ME compatibility
     //
@@ -183,6 +221,112 @@ public class PKCS1Encoding
 
         return engine.processBlock(block, 0, block.length);
     }
+    
+    /**
+     * Checks if the argument is a correctly PKCS#1.5 encoded Plaintext
+     * for encryption.
+     * 
+     * @param encoded The Plaintext.
+     * @param pLen Expected length of the plaintext.
+     * @return Either 0, if the encoding is correct, or -1, if it is incorrect.
+     */
+	private static int checkPkcs1Encoding(byte[] encoded, int pLen) {
+		int correct = 0;
+		/*
+		 * Check if the first two bytes are 0 2
+		 */
+		correct |= (encoded[0] ^ 2);
+
+		/*
+		 * Now the padding check, check for no 0 byte in the padding
+		 */
+		int plen = encoded.length - (
+				  pLen /* Lenght of the PMS */
+				+  1 /* Final 0-byte before PMS */
+		);
+
+		for (int i = 1; i < plen; i++) {
+			int tmp = encoded[i];
+			tmp |= tmp >> 1;
+			tmp |= tmp >> 2;
+			tmp |= tmp >> 4;
+			correct |= (tmp & 1) - 1;
+		}
+
+		/*
+		 * Make sure the padding ends with a 0 byte.
+		 */
+		correct |= encoded[encoded.length - (pLen +1)];
+
+		/*
+		 * Return 0 or 1, depending on the result.
+		 */
+		correct |= correct >> 1;
+		correct |= correct >> 2;
+		correct |= correct >> 4;
+		return ~((correct & 1) - 1);
+	}
+    
+
+    /**
+     * Decode PKCS#1.5 encoding, and return a random value if the padding is not correct.
+     * 
+     * @param in The encrypted block.
+     * @param inOff Offset in the encrypted block.
+     * @param inLen Length of the encrypted block.
+     * @param pLen Length of the desired output.
+     * @return The plaintext without padding, or a random value if the padding was incorrect.
+     * 
+     * @throws InvalidCipherTextException
+     */
+	private byte[] decodeBlockOrRandom(byte[] in, int inOff, int inLen)
+			throws InvalidCipherTextException {
+		if (!forPrivateKey) {
+			throw new InvalidCipherTextException("sorry, this method is only for decryption, not for signing");
+		}
+		byte[] block = engine.processBlock(in, inOff, inLen);
+		byte[] random = null;
+		if (this.fallback == null) {
+			random = new byte[this.pLen];
+			this.random.nextBytes(random);
+		} else {
+			random = fallback;
+		}
+		
+		/*
+		 * TODO: This is a potential dangerous side channel. However, you can
+		 * fix this by changing the RSA engine in a way, that it will always
+		 * return blocks of the same length and prepend them with 0 bytes if
+		 * needed.
+		 */
+		if (block.length < getOutputBlockSize()) {
+			throw new InvalidCipherTextException("block truncated");
+		}
+
+		/*
+		 * TODO: Potential side channel. Fix it by making the engine always
+		 * return blocks of the correct length.
+		 */
+		if (useStrictLength && block.length != engine.getOutputBlockSize()) {
+			throw new InvalidCipherTextException("block incorrect size");
+		}
+
+		/*
+		 * Check the padding.
+		 */
+		int correct = PKCS1Encoding.checkPkcs1Encoding(block, this.pLen);
+		
+		/*
+		 * Now, to a constant time constant memory copy of the decrypted value
+		 * or the random value, depending on the validity of the padding.
+		 */
+		byte[] result = new byte[this.pLen];
+		for (int i = 0; i < this.pLen; i++) {
+			result[i] = (byte)((block[i+(block.length-pLen)]&(~correct)) | (random[i]&correct));
+		}
+		
+		return result;
+	}
 
     /**
      * @exception InvalidCipherTextException if the decrypted block is not in PKCS1 format.
@@ -193,6 +337,14 @@ public class PKCS1Encoding
         int     inLen)
         throws InvalidCipherTextException
     {
+		/*
+		 * If the lenght of the expected plaintext is known, we use a constant
+		 * time decryption. If the decryption fails, we return a random value.
+		 */
+		if (this.pLen != -1) {
+    		return this.decodeBlockOrRandom(in, inOff, inLen);
+    	}
+    	
         byte[]  block = engine.processBlock(in, inOff, inLen);
 
         if (block.length < getOutputBlockSize())
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 dcdb4936..ea7bea76 100644
--- a/core/src/main/java/org/bouncycastle/crypto/tls/DefaultTlsEncryptionCredentials.java
+++ b/core/src/main/java/org/bouncycastle/crypto/tls/DefaultTlsEncryptionCredentials.java
@@ -55,11 +55,11 @@ public class DefaultTlsEncryptionCredentials
         return certificate;
     }
 
-    public byte[] decryptPreMasterSecret(byte[] encryptedPreMasterSecret)
+    public byte[] decryptPreMasterSecret(byte[] encryptedPreMasterSecret, byte[] fallback)
         throws IOException
     {
 
-        PKCS1Encoding encoding = new PKCS1Encoding(new RSABlindedEngine());
+        PKCS1Encoding encoding = new PKCS1Encoding(new RSABlindedEngine(), fallback);
         encoding.init(false, new ParametersWithRandom(this.privateKey, context.getSecureRandom()));
 
         try
@@ -69,6 +69,9 @@ public class DefaultTlsEncryptionCredentials
         }
         catch (InvalidCipherTextException e)
         {
+        	/*
+        	 * This should never happen, the decryption should always succeed, or return a random value.
+        	 */
             throw new TlsFatalAlert(AlertDescription.illegal_parameter);
         }
     }
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 eddf6847..292d4dbb 100644
--- a/core/src/main/java/org/bouncycastle/crypto/tls/TlsEncryptionCredentials.java
+++ b/core/src/main/java/org/bouncycastle/crypto/tls/TlsEncryptionCredentials.java
@@ -5,6 +5,6 @@ import java.io.IOException;
 public interface TlsEncryptionCredentials
     extends TlsCredentials
 {
-    byte[] decryptPreMasterSecret(byte[] encryptedPreMasterSecret)
+    byte[] decryptPreMasterSecret(byte[] encryptedPreMasterSecret, byte[] fallback)
         throws IOException;
 }
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 e3856bdc..0a464593 100644
--- a/core/src/main/java/org/bouncycastle/crypto/tls/TlsRSAUtils.java
+++ b/core/src/main/java/org/bouncycastle/crypto/tls/TlsRSAUtils.java
@@ -55,27 +55,35 @@ public class TlsRSAUtils
         /*
          * RFC 5246 7.4.7.1.
          */
-
         ProtocolVersion clientVersion = context.getClientVersion();
 
         // TODO Provide as configuration option?
         boolean versionNumberCheckDisabled = false;
 
         /*
-         * See notes regarding Bleichenbacher/Klima attack. The code here implements the first
-         * construction proposed there, which is RECOMMENDED.
+         * Bleichenbacher side channel countermeasures have been implemented in
+         * decryptPreMasterSecret, so we con't need to do something here.
          */
-        byte[] R = new byte[48];
-        context.getSecureRandom().nextBytes(R);
-
         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);
+
         try
         {
-            M = encryptionCredentials.decryptPreMasterSecret(encryptedPreMasterSecret);
+            M = encryptionCredentials.decryptPreMasterSecret(encryptedPreMasterSecret, fallback);
         }
         catch (Exception e)
         {
             /*
+             * This should never happen since the decryption should never throw an exception
+             * and return a random value instead.
+             * 
              * In any case, a TLS server MUST NOT generate an alert if processing an
              * RSA-encrypted premaster secret message fails, or the version number is not as
              * expected. Instead, it MUST continue the handshake with a randomly generated
@@ -83,34 +91,40 @@ public class TlsRSAUtils
              */
         }
 
-        if (M.length != 48)
-        {
-            TlsUtils.writeVersion(clientVersion, R, 0);
-            return R;
-        }
-
         /*
-         * If ClientHello.client_version is TLS 1.1 or higher, server implementations MUST
-         * check the version number [..].
-         */
+-         * If ClientHello.client_version is TLS 1.1 or higher, server implementations MUST
+-         * check the version number [..].
+          */
         if (versionNumberCheckDisabled && clientVersion.isEqualOrEarlierVersionOf(ProtocolVersion.TLSv10))
         {
-            /*
-             * If the version number is TLS 1.0 or earlier, server implementations SHOULD
-             * check the version number, but MAY have a configuration option to disable the
-             * check.
-             */
+			/*
+			 * If the version number is TLS 1.0 or earlier, server
+			 * implementations SHOULD check the version number, but MAY have a
+			 * configuration option to disable the check.
+			 * 
+			 * So there is nothing to do here.
+			 */
+        } 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.
+        	 */
+        	
+        	for (int i = 0; i < 48; i++) {
+        		M[i] = (byte)((M[i]&(~mask))|(fallback[i]&mask));
+        	}
         }
-        else
-        {
-            /*
-             * 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.
-             */
-            TlsUtils.writeVersion(clientVersion, M, 0);
-        }
-
         return M;
     }
 }