Browse Source

WOLFSSL_AES_CBC_LENGTH_CHECKS: add gated logic to aes.c wc_AesCbc{En,De}crypt() to return BAD_LENGTH_E when input length is not a multiple of AES_BLOCK_SIZE; add gated tests of new functionality in test_wc_AesCbcEncryptDecrypt(); fix first encrypt-decrypt-memcmp in test_wc_AesCbcEncryptDecrypt() to span all of test vector and extend test vector length to be block-multiple; add ungated logic in platform-specific wc_AesCbc{En,De}crypt() routines to return with early success when blocks == 0 (also mitigates buffer overrun on short (less-than-AES_BLOCK_SIZE) input); add BAD_LENGTH_E error code; update documentation.

Daniel Pouzzner 3 years ago
parent
commit
5d9ee97530
5 changed files with 168 additions and 25 deletions
  1. 20 8
      doc/dox_comments/header_files/aes.h
  2. 23 4
      tests/api.c
  3. 120 12
      wolfcrypt/src/aes.c
  4. 3 0
      wolfcrypt/src/error.c
  5. 2 1
      wolfssl/wolfcrypt/error-crypt.h

+ 20 - 8
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.
-    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
-    function and equivalent OpenSSL functions interoperate, one should specify
+    This function assumes that the input message is AES block length aligned,
+    and expects the input length to be a multiple of the block length, which
+    will optionally be checked and enforced if WOLFSSL_AES_CBC_LENGTH_CHECKS
+    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
-    require alignment as it adds PKCS#7 padding automatically. To make the
+    aligned, and expects the input length to be a multiple of the block length,
+    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

+ 23 - 4
tests/api.c

@@ -13083,11 +13083,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 +13118,8 @@ static int test_wc_AesCbcEncryptDecrypt (void)
                                                     iv, AES_DECRYPTION);
         }
         if (ret == 0) {
-            ret = wc_AesCbcDecrypt(&aes, dec, enc, AES_BLOCK_SIZE);
-            if (ret != 0 || XMEMCMP(vector, dec, AES_BLOCK_SIZE) != 0) {
+            ret = wc_AesCbcDecrypt(&aes, dec, enc, sizeof(vector));
+            if (ret != 0 || XMEMCMP(vector, dec, sizeof(vector)) != 0) {
                 ret = WOLFSSL_FATAL_ERROR;
             } else {
                 /* Set flag. */
@@ -13150,6 +13151,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_ALIGN_E) {
+            cbcE = 0;
+        } else {
+            cbcE = WOLFSSL_FATAL_ERROR;
+        }
+#endif
     }
     if (cbcE == 0) {
         /* Test passing in size of 0  */
@@ -13178,11 +13189,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_ALIGN_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  */

+ 120 - 12
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
-                                       || sz % AES_BLOCK_SIZE != 0) {
+        if (aes == NULL || out == NULL || in == NULL) {
             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);

+ 3 - 0
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";
 

+ 2 - 1
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