Merge pull request #3906 from douzzer/AES-BAD_ALIGN_E-consistency

Adds optional AES CBC length checking

configure.ac

@@ -353,6 +353,7 @@ then
     test "$enable_aesctr" = "" && enable_aesctr=yes
     test "$enable_aesofb" = "" && enable_aesofb=yes
     test "$enable_aescfb" = "" && enable_aescfb=yes
+    test "$enable_aescbc_length_checks" = "" && enable_aescbc_length_checks=yes
     test "$enable_camellia" = "" && enable_camellia=yes
     test "$enable_ripemd" = "" && enable_ripemd=yes
     test "$enable_sha512" = "" && enable_sha512=yes
@@ -1288,6 +1289,18 @@ then
     AM_CFLAGS="$AM_CFLAGS -DNO_AES_CBC"
 fi
 
+# AES-CBC length checks (checks that input lengths are multiples of block size)
+AC_ARG_ENABLE([aescbc_length_checks],
+    [AS_HELP_STRING([--enable-aescbc-length-checks],[Enable AES-CBC length validity checks (default: disabled)])],
+    [ ENABLED_AESCBC_LENGTH_CHECKS=$enableval ],
+    [ ENABLED_AESCBC_LENGTH_CHECKS=no ]
+    )
+
+if test "$ENABLED_AESCBC_LENGTH_CHECKS" = "yes"
+then
+    AM_CFLAGS="$AM_CFLAGS -DWOLFSSL_AES_CBC_LENGTH_CHECKS"
+fi
+
 # leanpsk and leantls don't need gcm
 
 # AES-GCM
@@ -6505,6 +6518,7 @@ echo "   * ARC4:                       $ENABLED_ARC4"
 echo "   * AES:                        $ENABLED_AES"
 echo "   * AES-NI:                     $ENABLED_AESNI"
 echo "   * AES-CBC:                    $ENABLED_AESCBC"
+echo "   * AES-CBC length checks:      $ENABLED_AESCBC_LENGTH_CHECKS"
 echo "   * AES-GCM:                    $ENABLED_AESGCM"
 echo "   * AES-CCM:                    $ENABLED_AESCCM"
 echo "   * AES-CTR:                    $ENABLED_AESCTR"

doc/dox_comments/header_files/aes.h

@@ -66,16 +66,22 @@ WOLFSSL_API int  wc_AesSetIV(Aes* aes, const byte* iv);
     the resulting cipher text in the output buffer out using cipher block
     chaining with AES. This function requires that the AES object has been
     initialized by calling AesSetKey before a message is able to be encrypted.
-    This function assumes that the input message is AES block length aligned.
+    This function assumes that the input message is AES block length aligned,
-    PKCS#7 style padding should be added beforehand. This differs from the
+    and expects the input length to be a multiple of the block length, which
-    OpenSSL AES-CBC methods which add the padding for you. To make the wolfSSL
+    will optionally be checked and enforced if WOLFSSL_AES_CBC_LENGTH_CHECKS
-    function and equivalent OpenSSL functions interoperate, one should specify
+    is defined in the build configuration.  In order to assure block-multiple
+    input, PKCS#7 style padding should be added beforehand. This differs from
+    the OpenSSL AES-CBC methods which add the padding for you. To make the
+    wolfSSL and corresponding OpenSSL functions interoperate, one should specify
     the -nopad option in the OpenSSL command line function so that it behaves
     like the wolfSSL AesCbcEncrypt method and does not add extra padding
     during encryption.
 
     \return 0 On successfully encrypting message.
-    \return BAD_ALIGN_E: Returned on block align error
+    \return BAD_ALIGN_E: may be returned on block align error
+    \return BAD_LENGTH_E will be returned if the input length isn't a
+    multiple of the AES block length, when the library is built with
+    WOLFSSL_AES_CBC_LENGTH_CHECKS.
 
     \param aes pointer to the AES object used to encrypt data
     \param out pointer to the output buffer in which to store the ciphertext
@@ -110,15 +116,21 @@ WOLFSSL_API int  wc_AesCbcEncrypt(Aes* aes, byte* out,
     with AES. This function requires that the AES structure has been
     initialized by calling AesSetKey before a message is able to be decrypted.
     This function assumes that the original message was AES block length
-    aligned. This differs from the OpenSSL AES-CBC methods which do not
+    aligned, and expects the input length to be a multiple of the block length,
-    require alignment as it adds PKCS#7 padding automatically. To make the
+    which will optionally be checked and enforced if
+    WOLFSSL_AES_CBC_LENGTH_CHECKS is defined in the build configuration.
+    This differs from the OpenSSL AES-CBC methods, which add PKCS#7 padding
+    automatically, and so do not require block-multiple input. To make the
     wolfSSL function and equivalent OpenSSL functions interoperate, one
     should specify the -nopad option in the OpenSSL command line function
     so that it behaves like the wolfSSL AesCbcEncrypt method and does not
     create errors during decryption.
 
     \return 0 On successfully decrypting message.
-    \return BAD_ALIGN_E Returned on block align error.
+    \return BAD_ALIGN_E may be returned on block align error.
+    \return BAD_LENGTH_E will be returned if the input length isn't a
+    multiple of the AES block length, when the library is built with
+    WOLFSSL_AES_CBC_LENGTH_CHECKS.
 
     \param aes pointer to the AES object used to decrypt data.
     \param out pointer to the output buffer in which to store the plain text

tests/api.c

@@ -19,6 +19,11 @@
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1335, USA
  */
 
+/* For AES-CBC, input lengths can optionally be validated to be a
+ * multiple of the block size, by defining WOLFSSL_AES_CBC_LENGTH_CHECKS,
+ * also available via the configure option --enable-aescbc-length-checks.
+ */
+
 
 /*----------------------------------------------------------------------------*
  | Includes
@@ -13083,11 +13088,12 @@ static int test_wc_AesCbcEncryptDecrypt (void)
         0x30, 0x31, 0x32, 0x33, 0x34, 0x35, 0x36, 0x37,
         0x38, 0x39, 0x61, 0x62, 0x63, 0x64, 0x65, 0x66
     };
-    byte    vector[] = /* Now is the time for all w/o trailing 0 */
+    byte    vector[] = /* Now is the time for all good men w/o trailing 0 */
     {
         0x4e,0x6f,0x77,0x20,0x69,0x73,0x20,0x74,
         0x68,0x65,0x20,0x74,0x69,0x6d,0x65,0x20,
-        0x66,0x6f,0x72,0x20,0x61,0x6c,0x6c,0x20
+        0x66,0x6f,0x72,0x20,0x61,0x6c,0x6c,0x20,
+        0x67,0x6f,0x6f,0x64,0x20,0x6d,0x65,0x6e
     };
     byte    iv[]    = "1234567890abcdef";
     byte    enc[sizeof(vector)];
@@ -13117,8 +13123,8 @@ static int test_wc_AesCbcEncryptDecrypt (void)
                                                     iv, AES_DECRYPTION);
         }
         if (ret == 0) {
-            ret = wc_AesCbcDecrypt(&aes, dec, enc, AES_BLOCK_SIZE);
+            ret = wc_AesCbcDecrypt(&aes, dec, enc, sizeof(vector));
-            if (ret != 0 || XMEMCMP(vector, dec, AES_BLOCK_SIZE) != 0) {
+            if (ret != 0 || XMEMCMP(vector, dec, sizeof(vector)) != 0) {
                 ret = WOLFSSL_FATAL_ERROR;
             } else {
                 /* Set flag. */
@@ -13150,6 +13156,16 @@ static int test_wc_AesCbcEncryptDecrypt (void)
         } else {
             cbcE = WOLFSSL_FATAL_ERROR;
         }
+#ifdef WOLFSSL_AES_CBC_LENGTH_CHECKS
+        if (cbcE == 0) {
+            cbcE = wc_AesCbcEncrypt(&aes, enc, vector, sizeof(vector) - 1);
+        }
+        if (cbcE == BAD_LENGTH_E) {
+            cbcE = 0;
+        } else {
+            cbcE = WOLFSSL_FATAL_ERROR;
+        }
+#endif
     }
     if (cbcE == 0) {
         /* Test passing in size of 0  */
@@ -13178,11 +13194,19 @@ static int test_wc_AesCbcEncryptDecrypt (void)
         if (cbcD == BAD_FUNC_ARG) {
             cbcD = wc_AesCbcDecrypt(&aes, dec, enc, AES_BLOCK_SIZE * 2 - 1);
         }
+#ifdef WOLFSSL_AES_CBC_LENGTH_CHECKS
+        if (cbcD == BAD_LENGTH_E) {
+            cbcD = 0;
+        } else {
+            cbcD = WOLFSSL_FATAL_ERROR;
+        }
+#else
         if (cbcD == BAD_FUNC_ARG) {
             cbcD = 0;
         } else {
             cbcD = WOLFSSL_FATAL_ERROR;
         }
+#endif
     }
     if (cbcD == 0) {
         /* Test passing in size of 0  */

wolfcrypt/src/aes.c

@@ -3085,8 +3085,16 @@ int wc_AesSetIV(Aes* aes, const byte* iv)
     int wc_AesCbcEncrypt(Aes* aes, byte* out, const byte* in, word32 sz)
     {
         int ret = 0;
-        word32 blocks = (sz / AES_BLOCK_SIZE);
         CRYP_HandleTypeDef hcryp;
+        word32 blocks = (sz / AES_BLOCK_SIZE);
+
+#ifdef WOLFSSL_AES_CBC_LENGTH_CHECKS
+        if (sz % AES_BLOCK_SIZE) {
+            return BAD_LENGTH_E;
+        }
+#endif
+        if (blocks == 0)
+            return 0;
 
         ret = wc_Stm32_Aes_Init(aes, &hcryp);
         if (ret != 0)
@@ -3135,8 +3143,16 @@ int wc_AesSetIV(Aes* aes, const byte* iv)
     int wc_AesCbcDecrypt(Aes* aes, byte* out, const byte* in, word32 sz)
     {
         int ret = 0;
-        word32 blocks = (sz / AES_BLOCK_SIZE);
         CRYP_HandleTypeDef hcryp;
+        word32 blocks = (sz / AES_BLOCK_SIZE);
+
+#ifdef WOLFSSL_AES_CBC_LENGTH_CHECKS
+        if (sz % AES_BLOCK_SIZE) {
+            return BAD_LENGTH_E;
+        }
+#endif
+        if (blocks == 0)
+            return 0;
 
         ret = wc_Stm32_Aes_Init(aes, &hcryp);
         if (ret != 0)
@@ -3191,10 +3207,18 @@ int wc_AesSetIV(Aes* aes, const byte* iv)
     {
         int ret;
         word32 *iv;
-        word32 blocks = (sz / AES_BLOCK_SIZE);
         CRYP_InitTypeDef cryptInit;
         CRYP_KeyInitTypeDef keyInit;
         CRYP_IVInitTypeDef ivInit;
+        word32 blocks = (sz / AES_BLOCK_SIZE);
+
+#ifdef WOLFSSL_AES_CBC_LENGTH_CHECKS
+        if (sz % AES_BLOCK_SIZE) {
+            return BAD_LENGTH_E;
+        }
+#endif
+        if (blocks == 0)
+            return 0;
 
         ret = wc_Stm32_Aes_Init(aes, &cryptInit, &keyInit);
         if (ret != 0)
@@ -3266,10 +3290,18 @@ int wc_AesSetIV(Aes* aes, const byte* iv)
     {
         int ret;
         word32 *iv;
-        word32 blocks = (sz / AES_BLOCK_SIZE);
         CRYP_InitTypeDef cryptInit;
         CRYP_KeyInitTypeDef keyInit;
         CRYP_IVInitTypeDef ivInit;
+        word32 blocks = (sz / AES_BLOCK_SIZE);
+
+#ifdef WOLFSSL_AES_CBC_LENGTH_CHECKS
+        if (sz % AES_BLOCK_SIZE) {
+            return BAD_LENGTH_E;
+        }
+#endif
+        if (blocks == 0)
+            return 0;
 
         ret = wc_Stm32_Aes_Init(aes, &cryptInit, &keyInit);
         if (ret != 0)
@@ -3363,6 +3395,12 @@ int wc_AesSetIV(Aes* aes, const byte* iv)
         if ((pi == NULL) || (po == NULL))
             return BAD_FUNC_ARG;    /*wrong pointer*/
 
+#ifdef WOLFSSL_AES_CBC_LENGTH_CHECKS
+        if (sz % AES_BLOCK_SIZE) {
+            return BAD_LENGTH_E;
+        }
+#endif
+
         wc_LockMutex(&Mutex_AesSEC);
 
         /* Set descriptor for SEC */
@@ -3388,16 +3426,24 @@ int wc_AesSetIV(Aes* aes, const byte* iv)
         secDesc->pointer7 = NULL;
         secDesc->nextDescriptorPtr = NULL;
 
+#ifdef WOLFSSL_AES_CBC_LENGTH_CHECKS
+        size = AES_BUFFER_SIZE;
+#endif
         while (sz) {
             secDesc->header = descHeader;
             XMEMCPY(secReg, aes->reg, AES_BLOCK_SIZE);
-            if ((sz % AES_BUFFER_SIZE) == sz) {
+#ifdef WOLFSSL_AES_CBC_LENGTH_CHECKS
+            sz -= AES_BUFFER_SIZE;
+#else
+            if (sz < AES_BUFFER_SIZE) {
                 size = sz;
                 sz = 0;
             } else {
                 size = AES_BUFFER_SIZE;
                 sz -= AES_BUFFER_SIZE;
             }
+#endif
+
             secDesc->length4 = size;
             secDesc->length5 = size;
 
@@ -3463,6 +3509,14 @@ int wc_AesSetIV(Aes* aes, const byte* iv)
         byte *iv, *enc_key;
         word32 blocks = (sz / AES_BLOCK_SIZE);
 
+#ifdef WOLFSSL_AES_CBC_LENGTH_CHECKS
+        if (sz % AES_BLOCK_SIZE) {
+            return BAD_LENGTH_E;
+        }
+#endif
+        if (blocks == 0)
+            return 0;
+
         iv      = (byte*)aes->reg;
         enc_key = (byte*)aes->key;
 
@@ -3492,8 +3546,16 @@ int wc_AesSetIV(Aes* aes, const byte* iv)
         word32 keySize;
         status_t status;
         byte* iv, *dec_key;
-        word32 blocks = (sz / AES_BLOCK_SIZE);
         byte temp_block[AES_BLOCK_SIZE];
+        word32 blocks = (sz / AES_BLOCK_SIZE);
+
+#ifdef WOLFSSL_AES_CBC_LENGTH_CHECKS
+        if (sz % AES_BLOCK_SIZE) {
+            return BAD_LENGTH_E;
+        }
+#endif
+        if (blocks == 0)
+            return 0;
 
         iv      = (byte*)aes->reg;
         dec_key = (byte*)aes->key;
@@ -3527,9 +3589,17 @@ int wc_AesSetIV(Aes* aes, const byte* iv)
     {
         int i;
         int offset = 0;
-        word32 blocks = (sz / AES_BLOCK_SIZE);
         byte *iv;
         byte temp_block[AES_BLOCK_SIZE];
+        word32 blocks = (sz / AES_BLOCK_SIZE);
+
+#ifdef WOLFSSL_AES_CBC_LENGTH_CHECKS
+        if (sz % AES_BLOCK_SIZE) {
+            return BAD_LENGTH_E;
+        }
+#endif
+        if (blocks == 0)
+            return 0;
 
         iv = (byte*)aes->reg;
 
@@ -3555,9 +3625,17 @@ int wc_AesSetIV(Aes* aes, const byte* iv)
     {
         int i;
         int offset = 0;
-        word32 blocks = (sz / AES_BLOCK_SIZE);
         byte* iv;
         byte temp_block[AES_BLOCK_SIZE];
+        word32 blocks = (sz / AES_BLOCK_SIZE);
+
+#ifdef WOLFSSL_AES_CBC_LENGTH_CHECKS
+        if (sz % AES_BLOCK_SIZE) {
+            return BAD_LENGTH_E;
+        }
+#endif
+        if (blocks == 0)
+            return 0;
 
         iv = (byte*)aes->reg;
 
@@ -3586,9 +3664,16 @@ int wc_AesSetIV(Aes* aes, const byte* iv)
     {
         int ret;
 
+        if (sz == 0)
+            return 0;
+
         /* hardware fails on input that is not a multiple of AES block size */
         if (sz % AES_BLOCK_SIZE != 0) {
+#ifdef WOLFSSL_AES_CBC_LENGTH_CHECKS
+            return BAD_LENGTH_E;
+#else
             return BAD_FUNC_ARG;
+#endif
         }
 
         ret = wc_Pic32AesCrypt(
@@ -3609,9 +3694,16 @@ int wc_AesSetIV(Aes* aes, const byte* iv)
         int ret;
         byte scratch[AES_BLOCK_SIZE];
 
+        if (sz == 0)
+            return 0;
+
         /* hardware fails on input that is not a multiple of AES block size */
         if (sz % AES_BLOCK_SIZE != 0) {
+#ifdef WOLFSSL_AES_CBC_LENGTH_CHECKS
+            return BAD_LENGTH_E;
+#else
             return BAD_FUNC_ARG;
+#endif
         }
         XMEMCPY(scratch, in + sz - AES_BLOCK_SIZE, AES_BLOCK_SIZE);
 
@@ -3666,7 +3758,7 @@ int wc_AesSetIV(Aes* aes, const byte* iv)
     /* Software AES - CBC Encrypt */
     int wc_AesCbcEncrypt(Aes* aes, byte* out, const byte* in, word32 sz)
     {
-        word32 blocks = (sz / AES_BLOCK_SIZE);
+        word32 blocks;
 
         if (aes == NULL || out == NULL || in == NULL) {
             return BAD_FUNC_ARG;
@@ -3675,6 +3767,14 @@ int wc_AesSetIV(Aes* aes, const byte* iv)
         if (sz == 0) {
             return 0;
         }
+
+        blocks = sz / AES_BLOCK_SIZE;
+#ifdef WOLFSSL_AES_CBC_LENGTH_CHECKS
+        if (sz % AES_BLOCK_SIZE) {
+            return BAD_LENGTH_E;
+        }
+#endif
+
     #ifdef WOLFSSL_IMXRT_DCP
         /* Implemented in wolfcrypt/src/port/nxp/dcp_port.c */
         if (aes->keylen == 16)
@@ -3779,14 +3879,23 @@ int wc_AesSetIV(Aes* aes, const byte* iv)
     {
         word32 blocks;
 
-        if (aes == NULL || out == NULL || in == NULL
+        if (aes == NULL || out == NULL || in == NULL) {
-                                       || sz % AES_BLOCK_SIZE != 0) {
             return BAD_FUNC_ARG;
         }
 
         if (sz == 0) {
             return 0;
         }
+
+        blocks = sz / AES_BLOCK_SIZE;
+        if (sz % AES_BLOCK_SIZE) {
+#ifdef WOLFSSL_AES_CBC_LENGTH_CHECKS
+            return BAD_LENGTH_E;
+#else
+            return BAD_FUNC_ARG;
+#endif
+        }
+
     #ifdef WOLFSSL_IMXRT_DCP
         /* Implemented in wolfcrypt/src/port/nxp/dcp_port.c */
         if (aes->keylen == 16)
@@ -3856,7 +3965,6 @@ int wc_AesSetIV(Aes* aes, const byte* iv)
         }
     #endif
 
-        blocks = sz / AES_BLOCK_SIZE;
         while (blocks--) {
             XMEMCPY(aes->tmp, in, AES_BLOCK_SIZE);
             wc_AesDecrypt(aes, (byte*)aes->tmp, out);

wolfcrypt/src/error.c

@@ -527,6 +527,9 @@ const char* wc_GetErrorString(int error)
     case MISSING_KEY:
         return "Required key not set";
 
+    case BAD_LENGTH_E:
+        return "Value of length parameter is invalid.";
+
     default:
         return "unknown error number";
 

wolfssl/wolfcrypt/error-crypt.h

@@ -237,8 +237,9 @@ enum {
     SAKKE_VERIFY_FAIL_E = -276,  /* SAKKE derivation verification error */
     MISSING_IV          = -277,  /* IV was not set */
     MISSING_KEY         = -278,  /* Key was not set */
+    BAD_LENGTH_E        = -279,  /* Value of length parameter is invalid. */
 
-    WC_LAST_E           = -278,  /* Update this to indicate last error */
+    WC_LAST_E           = -279,  /* Update this to indicate last error */
     MIN_CODE_E          = -300   /* errors -101 - -299 */
 
     /* add new companion error id strings for any new error codes