Merge pull request #146 from cconlon/pkcs7Pad

Fixes for PKCS#7 pad/unpad

Author: rlm2002

src/main/java/com/wolfssl/wolfcrypt/BlockCipher.java

@@ -287,12 +287,16 @@ public static synchronized int getPKCS7PadSize(int inputSize,
 
         int padSz = 0;
 
-        if (inputSize == 0 || blockSize == 0) {
-            throw new WolfCryptException(
-                "Input or block size is 0");
+        if (blockSize == 0) {
+            throw new WolfCryptException("Block size is 0");
         }
 
-        padSz = blockSize - (inputSize % blockSize);
+        /* PKCS#7 padding: if input size is 0, pad with full block */
+        if (inputSize == 0) {
+            padSz = blockSize;
+        } else {
+            padSz = blockSize - (inputSize % blockSize);
+        }
 
         return padSz;
     }
@@ -366,20 +370,20 @@ public static synchronized byte[] unPadPKCS7(byte[] in, int blockSize) {
         padValue = in[in.length - 1];
 
         /* verify pad value is less than or equal to block size */
-        if (padValue > (byte)blockSize) {
+        if ((padValue & 0xff) > blockSize) {
             throw new WolfCryptException(
                 "Invalid pad value, larger than block size");
         }
 
         /* verify pad bytes are consistent */
-        for (int i = in.length; i > in.length - padValue; i--) {
+        for (int i = in.length; i > in.length - (padValue & 0xff); i--) {
             if (in[i - 1] != padValue) {
                 valid = false;
             }
         }
 
-        unpadded = new byte[in.length - padValue];
-        System.arraycopy(in, 0, unpadded, 0, in.length - padValue);
+        unpadded = new byte[in.length - (padValue & 0xff)];
+        System.arraycopy(in, 0, unpadded, 0, in.length - (padValue & 0xff));
 
         if (!valid) {
             throw new WolfCryptException(

src/test/java/com/wolfssl/provider/jce/test/WolfCryptCipherTest.java

@@ -6381,5 +6381,58 @@ public void testAESAlgorithmParametersWithCipher()
         assertArrayEquals("Should decrypt correctly with cipher parameters",
             plaintext, decrypted2);
     }
+
+    /**
+     * Test that calling getOutputSize(0) doesn't throw an exception
+     * when using PKCS5 padding modes.
+     */
+    @Test
+    public void testGetOutputSizeZeroInputPKCS5Padding() throws Exception {
+
+        if (!enabledJCEAlgos.contains("AES/ECB/PKCS5Padding")) {
+            /* skip test if AES/ECB/PKCS5Padding not supported */
+            return;
+        }
+
+        byte[] key = new byte[] {
+            0x00, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77,
+            (byte)0x88, (byte)0x99, (byte)0xAA, (byte)0xBB,
+            (byte)0xCC, (byte)0xDD, (byte)0xEE, (byte)0xFF
+        };
+
+        SecretKeySpec keySpec = new SecretKeySpec(key, "AES");
+
+        Cipher cipher = Cipher.getInstance("AES/ECB/PKCS5Padding", jceProvider);
+        cipher.init(Cipher.ENCRYPT_MODE, keySpec);
+
+        /* This should not throw an exception and should return block size */
+        int outputSize = cipher.getOutputSize(0);
+        assertEquals("Output size for zero input should be one block " +
+            "(16 bytes)", 16, outputSize);
+
+        /* Test AES/CBC/PKCS5Padding if available */
+        if (enabledJCEAlgos.contains("AES/CBC/PKCS5Padding")) {
+            cipher = Cipher.getInstance("AES/CBC/PKCS5Padding", jceProvider);
+            cipher.init(Cipher.ENCRYPT_MODE, keySpec);
+
+            outputSize = cipher.getOutputSize(0);
+            assertEquals("CBC: Output size for zero input should be one " +
+                "block (16 bytes)", 16, outputSize);
+        }
+
+        /* Test DESede if available */
+        if (enabledJCEAlgos.contains("DESede/CBC/PKCS5Padding")) {
+            byte[] desKey = new byte[24]; /* 3DES requires 24-byte key */
+            Arrays.fill(desKey, (byte)0x42);
+            SecretKeySpec desKeySpec = new SecretKeySpec(desKey, "DESede");
+
+            cipher = Cipher.getInstance("DESede/CBC/PKCS5Padding", jceProvider);
+            cipher.init(Cipher.ENCRYPT_MODE, desKeySpec);
+
+            outputSize = cipher.getOutputSize(0);
+            assertEquals("DESede: Output size for zero input should be one " +
+                "block (8 bytes)", 8, outputSize);
+        }
+    }
 }
 

src/test/java/com/wolfssl/wolfcrypt/test/AesTest.java

@@ -557,6 +557,49 @@ public void testPadPKCS7() {
         }
     }
 
+    /**
+     * Tests that padding bytes with values greater than or equal to 128 are
+     * handled correctly.
+     */
+    @Test
+    public void testUnPadPKCS7HighValueBytes() {
+
+        /* Test with pad value 0x80 (128) - first problematic value */
+        byte[] input = new byte[128 - Aes.BLOCK_SIZE]; /* 112 bytes */
+        Arrays.fill(input, (byte)0xAA);
+        byte[] padded = new byte[8 * Aes.BLOCK_SIZE]; /* 128 bytes */
+        System.arraycopy(input, 0, padded, 0, input.length);
+        /* 16 pad bytes */
+        Arrays.fill(padded, input.length, padded.length, (byte)0x10);
+
+        /* This should work without throwing ArrayIndexOutOfBoundsException */
+        byte[] unpadded = Aes.unPadPKCS7(padded, Aes.BLOCK_SIZE);
+        assertEquals(input.length, unpadded.length);
+        assertArrayEquals(input, unpadded);
+
+        /* Test with various high-value padding bytes */
+        int[] testPadValues = {128, 129, 200, 255};
+        for (int padValue : testPadValues) {
+            if (padValue <= Aes.BLOCK_SIZE) {
+                int dataSize = Aes.BLOCK_SIZE - padValue;
+                byte[] data = new byte[dataSize];
+                Arrays.fill(data, (byte)0xBB);
+
+                byte[] paddedData = new byte[Aes.BLOCK_SIZE];
+                System.arraycopy(data, 0, paddedData, 0, dataSize);
+                Arrays.fill(paddedData, dataSize, paddedData.length,
+                    (byte)(padValue & 0xff));
+
+                byte[] unpaddedData =
+                    Aes.unPadPKCS7(paddedData, Aes.BLOCK_SIZE);
+                assertEquals("Failed for pad value " + padValue,
+                    dataSize, unpaddedData.length);
+                assertArrayEquals("Data mismatch for pad value " + padValue,
+                    data, unpaddedData);
+            }
+        }
+    }
+
     @Test
     public void threadedAesTest() throws InterruptedException {