F-2375 - https://fenrir.wolfssl.com/finding/2375 - Fix AES-CBC-PAD single-shot C_Encrypt output length for block-aligned inputs

Author: aidangarske

src/crypto.c

@@ -2272,9 +2272,9 @@ CK_RV C_Encrypt(CK_SESSION_HANDLE hSession, CK_BYTE_PTR pData,
             if (!CK_ULONG_FITS_WORD32(ulDataLen))
                 return CKR_DATA_LEN_RANGE;
 
-            /* PKCS#5 pad makes the output a multiple of 16 */
-            encDataLen = (word32)((ulDataLen + AES_BLOCK_SIZE - 1) /
-                        AES_BLOCK_SIZE) * AES_BLOCK_SIZE;
+            /* PKCS#7 padding always adds at least 1 byte */
+            encDataLen = (word32)((ulDataLen / AES_BLOCK_SIZE) + 1) *
+                        AES_BLOCK_SIZE;
             if (pEncryptedData == NULL) {
                 *pulEncryptedDataLen = encDataLen;
                 return CKR_OK;

tests/pkcs11test.c

@@ -10597,6 +10597,83 @@ static CK_RV test_aes_cbc_pad_len_test(void* args)
     return ret;
 }
 
+static CK_RV test_aes_cbc_pad_block_aligned_size(void* args)
+{
+    CK_SESSION_HANDLE session = *(CK_SESSION_HANDLE*)args;
+    CK_RV ret;
+    CK_OBJECT_HANDLE key;
+    CK_MECHANISM mech;
+    byte iv[16];
+    CK_ULONG ivSz = sizeof(iv);
+    CK_ULONG encSz;
+    CK_ULONG i;
+    /* Test block-aligned sizes: 0, 16, 32, 48 */
+    CK_ULONG sizes[] = { 0, 16, 32, 48 };
+    byte plain[48];
+    byte enc[48 + 16];
+    byte dec[48];
+    CK_ULONG decSz;
+
+    memset(plain, 9, sizeof(plain));
+    memset(iv, 9, sizeof(iv));
+
+    mech.mechanism      = CKM_AES_CBC_PAD;
+    mech.ulParameterLen = ivSz;
+    mech.pParameter     = iv;
+
+    ret = get_aes_128_key(session, NULL, 0, &key);
+    CHECK_CKR(ret, "Getting AES key");
+
+    for (i = 0; i < sizeof(sizes)/sizeof(sizes[0]) && ret == CKR_OK; i++) {
+        CK_ULONG plainSz = sizes[i];
+        CK_ULONG expectedEncSz = plainSz + 16; /* PKCS#7 always adds padding */
+
+        /* Size query with pEncryptedData=NULL */
+        ret = funcList->C_EncryptInit(session, &mech, key);
+        CHECK_CKR(ret, "AES-CBC-PAD Encrypt Init for size query");
+        if (ret == CKR_OK) {
+            encSz = 0;
+            ret = funcList->C_Encrypt(session, plain, plainSz, NULL, &encSz);
+            CHECK_CKR(ret, "AES-CBC-PAD Encrypt size query");
+        }
+        if (ret == CKR_OK && encSz != expectedEncSz) {
+            ret = -1;
+            CHECK_CKR(ret, "AES-CBC-PAD size query must be plainSz+16");
+        }
+
+        /* Actual encrypt-then-decrypt roundtrip */
+        if (ret == CKR_OK) {
+            encSz = sizeof(enc);
+            ret = funcList->C_Encrypt(session, plain, plainSz, enc, &encSz);
+            CHECK_CKR(ret, "AES-CBC-PAD Encrypt");
+        }
+        if (ret == CKR_OK && encSz != expectedEncSz) {
+            ret = -1;
+            CHECK_CKR(ret, "AES-CBC-PAD encrypt output size must be plainSz+16");
+        }
+        if (ret == CKR_OK) {
+            ret = funcList->C_DecryptInit(session, &mech, key);
+            CHECK_CKR(ret, "AES-CBC-PAD Decrypt Init");
+        }
+        if (ret == CKR_OK) {
+            decSz = sizeof(dec);
+            ret = funcList->C_Decrypt(session, enc, encSz, dec, &decSz);
+            CHECK_CKR(ret, "AES-CBC-PAD Decrypt");
+        }
+        if (ret == CKR_OK && decSz != plainSz) {
+            ret = -1;
+            CHECK_CKR(ret, "AES-CBC-PAD roundtrip size mismatch");
+        }
+        if (ret == CKR_OK && plainSz > 0 &&
+                                      XMEMCMP(plain, dec, plainSz) != 0) {
+            ret = -1;
+            CHECK_CKR(ret, "AES-CBC-PAD roundtrip data mismatch");
+        }
+    }
+
+    return ret;
+}
+
 static CK_RV test_aes_cbc_pad_encdec(CK_SESSION_HANDLE session,
                                      unsigned char* exp, CK_OBJECT_HANDLE key)
 {
@@ -10623,7 +10700,7 @@ static CK_RV test_aes_cbc_pad_encdec(CK_SESSION_HANDLE session,
         ret = funcList->C_Encrypt(session, plain, plainSz, NULL, &encSz);
         CHECK_CKR(ret, "AES-CBC Pad Encrypt no enc");
     }
-    if (ret == CKR_OK && encSz != plainSz) {
+    if (ret == CKR_OK && encSz != plainSz + 16) {
         ret = -1;
         CHECK_CKR(ret, "AES-CBC Pad Encrypt encrypted length");
     }
@@ -10652,7 +10729,7 @@ static CK_RV test_aes_cbc_pad_encdec(CK_SESSION_HANDLE session,
         ret = funcList->C_Decrypt(session, enc, encSz, NULL, &decSz);
         CHECK_CKR(ret, "AES-CBC Pad Decrypt");
     }
-    if (ret == CKR_OK && decSz != encSz-1) {
+    if (ret == CKR_OK && decSz != encSz - 1) {
         ret = -1;
         CHECK_CKR(ret, "AES-CBC Pad Decrypt decrypted length");
     }
@@ -16960,6 +17037,7 @@ static TEST_FUNC testFunc[] = {
 #ifndef NO_AES
 #ifdef HAVE_AES_CBC
     PKCS11TEST_FUNC_SESS_DECL(test_aes_cbc_pad_len_test),
+    PKCS11TEST_FUNC_SESS_DECL(test_aes_cbc_pad_block_aligned_size),
     PKCS11TEST_FUNC_SESS_DECL(test_aes_cbc_fixed_key),
     PKCS11TEST_FUNC_SESS_DECL(test_aes_cbc_fail),
     PKCS11TEST_FUNC_SESS_DECL(test_aes_cbc_gen_key),