Improvements to PKCS1Encoding

Author: peterdettman

core/src/main/java/org/bouncycastle/crypto/encodings/PKCS1Encoding.java

@@ -233,52 +233,86 @@ private byte[] encodeBlock(
     }
 
     /**
-     * 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.
+     * Check the argument is a valid encoding with type 1. Returns the plaintext length if valid, or -1 if invalid.
      */
-    private static int checkPkcs1Encoding(byte[] encoded, int pLen)
+    private static int checkPkcs1Encoding1(byte[] buf)
     {
-        int correct = 0;
-        /*
-         * Check if the first two bytes are 0 2
-         */
-        correct |= (encoded[0] ^ 2);
+        int foundZeroMask = 0;
+        int lastPadPos = 0;
 
-        /*
-         * Now the padding check, check for no 0 byte in the padding
-         */
-        int plen = encoded.length - (
-            pLen /* Length of the PMS */
-                + 1 /* Final 0-byte before PMS */
-        );
+        // The first byte should be 0x01
+        int badPadSign = -((buf[0] & 0xFF) ^ 0x01);
 
-        for (int i = 1; i < plen; i++)
+        // There must be a zero terminator for the padding somewhere
+        for (int i = 1; i < buf.length; ++i)
         {
-            int tmp = encoded[i];
-            tmp |= tmp >> 1;
-            tmp |= tmp >> 2;
-            tmp |= tmp >> 4;
-            correct |= (tmp & 1) - 1;
+            int padByte = buf[i] & 0xFF;
+            int is0x00Mask = ((padByte ^ 0x00) - 1) >> 31;
+            int is0xFFMask = ((padByte ^ 0xFF) - 1) >> 31;
+            lastPadPos ^= i & ~foundZeroMask & is0x00Mask;
+            foundZeroMask |= is0x00Mask;
+            badPadSign |= ~(foundZeroMask | is0xFFMask);
         }
 
-        /*
-         * Make sure the padding ends with a 0 byte.
-         */
-        correct |= encoded[encoded.length - (pLen + 1)];
+        // The header should be at least 10 bytes
+        badPadSign |= lastPadPos - 9;
 
-        /*
-         * Return 0 or 1, depending on the result.
-         */
-        correct |= correct >> 1;
-        correct |= correct >> 2;
-        correct |= correct >> 4;
-        return ~((correct & 1) - 1);
+        int plaintextLength = buf.length - 1 - lastPadPos;
+        return plaintextLength | badPadSign >> 31;
     }
 
+    /**
+     * Check the argument is a valid encoding with type 2. Returns the plaintext length if valid, or -1 if invalid.
+     */
+    private static int checkPkcs1Encoding2(byte[] buf)
+    {
+        int foundZeroMask = 0;
+        int lastPadPos = 0;
+
+        // The first byte should be 0x02
+        int badPadSign = -((buf[0] & 0xFF) ^ 0x02);
+
+        // There must be a zero terminator for the padding somewhere
+        for (int i = 1; i < buf.length; ++i)
+        {
+            int padByte = buf[i] & 0xFF;
+            int is0x00Mask = ((padByte ^ 0x00) - 1) >> 31;
+            lastPadPos ^= i & ~foundZeroMask & is0x00Mask;
+            foundZeroMask |= is0x00Mask;
+        }
+
+        // The header should be at least 10 bytes
+        badPadSign |= lastPadPos - 9;
+
+        int plaintextLength = buf.length - 1 - lastPadPos;
+        return plaintextLength | badPadSign >> 31;
+    }
+
+    /**
+     * Check the argument is a valid encoding with type 2 of a plaintext with the given length. Returns 0 if
+     * valid, or -1 if invalid.
+     */
+    private static int checkPkcs1Encoding2(byte[] buf, int plaintextLength)
+    {
+        // The first byte should be 0x02
+        int badPadSign = -((buf[0] & 0xFF) ^ 0x02);
+
+        int lastPadPos = buf.length - 1 - plaintextLength;
+
+        // The header should be at least 10 bytes
+        badPadSign |= lastPadPos - 9;
+
+        // All pad bytes before the last one should be non-zero
+        for (int i = 1; i < lastPadPos; ++i)
+        {
+            badPadSign |= (buf[i] & 0xFF) - 1;
+        }
+
+        // Last pad byte should be zero
+        badPadSign |= -(buf[lastPadPos] & 0xFF);
+
+        return badPadSign >> 31;
+    }
 
     /**
      * Decode PKCS#1.5 encoding, and return a random value if the padding is not correct.
@@ -298,132 +332,94 @@ private byte[] decodeBlockOrRandom(byte[] in, int inOff, int inLen)
             throw new InvalidCipherTextException("sorry, this method is only for decryption, not for signing");
         }
 
-        byte[] block = engine.processBlock(in, inOff, inLen);
-        byte[] random;
-        if (this.fallback == null)
+        int plaintextLength = this.pLen;
+
+        byte[] random = fallback;
+        if (fallback == null)
         {
-            random = new byte[this.pLen];
+            random = new byte[plaintextLength];
             this.random.nextBytes(random);
         }
-        else
+
+        int badPadMask = 0;
+        int strictBlockSize = engine.getOutputBlockSize();
+        byte[] block = engine.processBlock(in, inOff, inLen);
+
+        byte[] data = block;
+        if (block.length != strictBlockSize)
         {
-            random = fallback;
+            if (useStrictLength || block.length < strictBlockSize)
+            {
+                data = blockBuffer;
+            }
         }
 
-        byte[] data = (useStrictLength & (block.length != engine.getOutputBlockSize())) ? blockBuffer : block;
+        badPadMask |= checkPkcs1Encoding2(data, plaintextLength);
 
-        /*
-         * Check the padding.
-         */
-        int correct = PKCS1Encoding.checkPkcs1Encoding(data, 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++)
+        int dataOff = data.length - plaintextLength; 
+        byte[] result = new byte[plaintextLength];
+        for (int i = 0; i < plaintextLength; ++i)
         {
-            result[i] = (byte)((data[i + (data.length - pLen)] & (~correct)) | (random[i] & correct));
+            result[i] = (byte)((data[dataOff + i] & ~badPadMask) | (random[i] & badPadMask));
         }
 
-        Arrays.fill(data, (byte)0);
+        Arrays.fill(block, (byte)0);
+        Arrays.fill(blockBuffer, 0, Math.max(0, blockBuffer.length - block.length), (byte)0);
 
         return result;
     }
 
     /**
      * @throws InvalidCipherTextException if the decrypted block is not in PKCS1 format.
      */
-    private byte[] decodeBlock(
-        byte[] in,
-        int inOff,
-        int inLen)
+    private byte[] decodeBlock(byte[] in, int inOff, int inLen)
         throws InvalidCipherTextException
     {
         /*
          * If the length 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)
+        if (forPrivateKey && this.pLen != -1)
         {
             return this.decodeBlockOrRandom(in, inOff, inLen);
         }
 
+        int strictBlockSize = engine.getOutputBlockSize();
         byte[] block = engine.processBlock(in, inOff, inLen);
-        boolean incorrectLength = (useStrictLength & (block.length != engine.getOutputBlockSize()));
-
-        byte[] data;
-        if (block.length < getOutputBlockSize())
-        {
-            data = blockBuffer;
-        }
-        else
-        {
-            data = block;
-        }
-
-        byte type = data[0];
-
-        boolean badType;
-        if (forPrivateKey)
-        {
-            badType = (type != 2);
-        }
-        else
-        {
-            badType = (type != 1);
-        }
-
-        //
-        // find and extract the message block.
-        //
-        int start = findStart(type, data);
-
-        start++;           // data should start at the next byte
 
-        if (badType | start < HEADER_LENGTH)
-        {
-            Arrays.fill(data, (byte)0);
-            throw new InvalidCipherTextException("block incorrect");
-        }
+        boolean incorrectLength = useStrictLength & (block.length != strictBlockSize);
 
-        // if we get this far, it's likely to be a genuine encoding error
-        if (incorrectLength)
+        byte[] data = block;
+        if (block.length < strictBlockSize)
         {
-            Arrays.fill(data, (byte)0);
-            throw new InvalidCipherTextException("block incorrect size");
+            data = blockBuffer;
         }
 
-        byte[] result = new byte[data.length - start];
-
-        System.arraycopy(data, start, result, 0, result.length);
-
-        return result;
-    }
+        int plaintextLength = forPrivateKey ? checkPkcs1Encoding2(data) : checkPkcs1Encoding1(data);
 
-    private int findStart(byte type, byte[] block)
-        throws InvalidCipherTextException
-    {
-        int start = -1;
-        boolean padErr = false;
-
-        for (int i = 1; i != block.length; i++)
+        try
         {
-            byte pad = block[i];
-
-            if (pad == 0 & start < 0)
+            if (plaintextLength < 0)
             {
-                start = i;
+                throw new InvalidCipherTextException("block incorrect");
+            }
+            if (incorrectLength)
+            {
+                throw new InvalidCipherTextException("block incorrect size");
             }
-            padErr |= (type == 1 & start < 0 & pad != (byte)0xff);
-        }
 
-        if (padErr)
+            byte[] result = new byte[plaintextLength];
+            System.arraycopy(data, data.length - plaintextLength, result, 0, plaintextLength);
+            return result;
+        }
+        finally
         {
-            return -1;
+            Arrays.fill(block, (byte)0);
+            Arrays.fill(blockBuffer, 0, Math.max(0, blockBuffer.length - block.length), (byte)0);
         }
-
-        return start;
     }
 }

prov/src/main/java/org/bouncycastle/jcajce/provider/asymmetric/rsa/CipherSpi.java

@@ -29,7 +29,6 @@
 import org.bouncycastle.crypto.InvalidCipherTextException;
 import org.bouncycastle.crypto.encodings.ISO9796d1Encoding;
 import org.bouncycastle.crypto.encodings.OAEPEncoding;
-import org.bouncycastle.crypto.encodings.PKCS1Encoding;
 import org.bouncycastle.crypto.engines.RSABlindedEngine;
 import org.bouncycastle.crypto.params.ParametersWithRandom;
 import org.bouncycastle.jcajce.provider.asymmetric.util.BaseCipherSpi;
@@ -200,7 +199,7 @@ protected void engineSetPadding(
         }
         else if (pad.equals("PKCS1PADDING"))
         {
-            cipher = new PKCS1Encoding(new RSABlindedEngine());
+            cipher = new CustomPKCS1Encoding(new RSABlindedEngine());
         }
         else if (pad.equals("ISO9796-1PADDING"))
         {
@@ -531,15 +530,26 @@ private byte[] getOutput()
     {
         try
         {
-            return cipher.processBlock(bOut.getBuf(), 0, bOut.size());
-        }
-        catch (InvalidCipherTextException e)
-        {
-            throw new BadBlockException("unable to decrypt block", e);
-        }
-        catch (ArrayIndexOutOfBoundsException e)
-        {
-            throw new BadBlockException("unable to decrypt block", e);
+            byte[] output;
+            try
+            {
+                output = cipher.processBlock(bOut.getBuf(), 0, bOut.size());
+            }
+            catch (InvalidCipherTextException e)
+            {
+                throw new BadBlockException("unable to decrypt block", e);
+            }
+            catch (ArrayIndexOutOfBoundsException e)
+            {
+                throw new BadBlockException("unable to decrypt block", e);
+            }
+
+            if (output == null)
+            {
+                throw new BadBlockException("unable to decrypt block", null);
+            }
+
+            return output;
         }
         finally
         {
@@ -565,7 +575,7 @@ static public class PKCS1v1_5Padding
     {
         public PKCS1v1_5Padding()
         {
-            super(new PKCS1Encoding(new RSABlindedEngine()));
+            super(new CustomPKCS1Encoding(new RSABlindedEngine()));
         }
     }
 
@@ -574,7 +584,7 @@ static public class PKCS1v1_5Padding_PrivateOnly
     {
         public PKCS1v1_5Padding_PrivateOnly()
         {
-            super(false, true, new PKCS1Encoding(new RSABlindedEngine()));
+            super(false, true, new CustomPKCS1Encoding(new RSABlindedEngine()));
         }
     }
 
@@ -583,7 +593,7 @@ static public class PKCS1v1_5Padding_PublicOnly
     {
         public PKCS1v1_5Padding_PublicOnly()
         {
-            super(true, false, new PKCS1Encoding(new RSABlindedEngine()));
+            super(true, false, new CustomPKCS1Encoding(new RSABlindedEngine()));
         }
     }