Merge pull request #153 from haydenroche5/aes_ctr_bug

Fix bug with AES-CTR initialization.

Author: SparkiDev

src/we_aes_ctr.c

@@ -34,8 +34,8 @@ typedef struct we_AesCtr
 {
     /** The wolfSSL AES data object. */
     Aes            aes;
-    /** Flag to indicate whether wolfSSL AES object initialized. */
-    unsigned int   init:1;
+    /** Flag to indicate whether IV has been set. */
+    unsigned int   ivSet:1;
 } we_AesCtr;
 
 
@@ -54,6 +54,7 @@ static int we_aes_ctr_init(EVP_CIPHER_CTX *ctx, const unsigned char *key,
     int ret = 1;
     int rc;
     we_AesCtr *aes;
+    const unsigned char *tmpIv;
 
     WOLFENGINE_ENTER(WE_LOG_CIPHER, "we_aes_ctr_init");
 
@@ -75,13 +76,20 @@ static int we_aes_ctr_init(EVP_CIPHER_CTX *ctx, const unsigned char *key,
     }
 
     if (ret == 1) {
-        /* Must have initialized wolfSSL AES object when here. */
-        aes->init = 1;
-
         if (key != NULL) {
+            if (iv == NULL && aes->ivSet) {
+                /* If the IV was set on a previous call, we don't want to pass
+                 * NULL for the IV to wc_AesSetKey, as that will result in the
+                 * IV being set to 0s. */
+                tmpIv = (unsigned char *)&aes->aes.reg;
+            }
+            else {
+                tmpIv = iv;
+            }
+
             /* No decryption for CTR. */
             rc = wc_AesSetKey(&aes->aes, key, EVP_CIPHER_CTX_key_length(ctx),
-                              iv, AES_ENCRYPTION);
+                              tmpIv, AES_ENCRYPTION);
             if (rc != 0) {
                 WOLFENGINE_ERROR_FUNC(WE_LOG_CIPHER, "wc_AesSetKey", rc);
                 ret = 0;
@@ -93,6 +101,9 @@ static int we_aes_ctr_init(EVP_CIPHER_CTX *ctx, const unsigned char *key,
                 WOLFENGINE_ERROR_FUNC(WE_LOG_CIPHER, "wc_AesSetIV", rc);
                 ret = 0;
             }
+            else {
+                aes->ivSet = 1;
+            }
         }
     }
 

test/test_cipher.c

@@ -514,5 +514,75 @@ int test_aes256_ctr_stream(ENGINE *e, void *data)
     return err;
 }
 
+/* OpenSSL allows the user to call EVP_CipherInit with NULL key or IV. In the
+ * past, setting the IV first (with key NULL) with wolfEngine and then setting
+ * the key (with IV NULL) would result in the IV getting set to 0s on the call
+ * to set the key. This was discovered in testing with OpenSSH. This is a
+ * regression test to ensure we preserve the IV in this scenario. */
+int test_aes_ctr_iv_init_regression(ENGINE *e, void *data)
+{
+    int err = 0;
+    unsigned char iv[AES_BLOCK_SIZE];
+    unsigned char key[16];
+    EVP_CIPHER_CTX* encCtx = NULL;
+    EVP_CIPHER_CTX* decCtx = NULL;
+    const unsigned char plainText[] = "Lorem ipsum dolor sit amet";
+    unsigned char encText[sizeof(plainText)];
+    unsigned char decText[sizeof(plainText)];
+
+    (void)data;
+
+    /* Generate a random IV and key. */
+    err = RAND_bytes(iv, AES_BLOCK_SIZE) != 1;
+    if (err == 0) {
+        err = RAND_bytes(key, 16) != 1;
+    }
+
+    /* Create encryption context. Use OpenSSL for encryption. */
+    if (err == 0) {
+        err = (encCtx = EVP_CIPHER_CTX_new()) == NULL;
+    }
+    if (err == 0) {
+        err = EVP_CipherInit_ex(encCtx, EVP_aes_128_ctr(), NULL, NULL, iv, 1)
+              != 1;
+    }
+    if (err == 0) {
+        err = EVP_CipherInit_ex(encCtx, NULL, NULL, key, NULL, -1) != 1;
+    }
+
+    /* Create decryption context. Use wolfEngine for decryption. */
+    if (err == 0) {
+        err = (decCtx = EVP_CIPHER_CTX_new()) == NULL;
+    }
+    if (err == 0) {
+        err = EVP_CipherInit_ex(decCtx, EVP_aes_128_ctr(), e, NULL, iv, 0) != 1;
+    }
+    if (err == 0) {
+        err = EVP_CipherInit_ex(decCtx, NULL, e, key, NULL, -1) != 1;
+    }
+
+    /* Encrypt. */
+    if (err == 0) {
+        err = EVP_Cipher(encCtx, encText, plainText, sizeof(plainText)) < 0;
+    }
+
+    /* Decrypt. */
+    if (err == 0) {
+        err = EVP_Cipher(decCtx, decText, encText, sizeof(plainText)) < 0;
+    }
+
+    /* Ensure decrypted and plaintext match. */
+    if (err == 0) {
+        err = memcmp(decText, plainText, sizeof(plainText)) != 0;
+    }
+
+    if (encCtx != NULL)
+        EVP_CIPHER_CTX_free(encCtx);
+    if (decCtx != NULL)
+        EVP_CIPHER_CTX_free(decCtx);
+
+    return err;
+}
+
 #endif /* WE_HAVE_AESCTR */
 

test/unit.c

@@ -116,6 +116,7 @@ TEST_CASE test_case[] = {
     TEST_DECL(test_aes128_ctr_stream, NULL),
     TEST_DECL(test_aes192_ctr_stream, NULL),
     TEST_DECL(test_aes256_ctr_stream, NULL),
+    TEST_DECL(test_aes_ctr_iv_init_regression, NULL),
 #endif
 #ifdef WE_HAVE_AESGCM
     TEST_DECL(test_aes128_gcm, NULL),

test/unit.h

@@ -154,6 +154,8 @@ int test_aes128_ctr_stream(ENGINE *e, void *data);
 int test_aes192_ctr_stream(ENGINE *e, void *data);
 int test_aes256_ctr_stream(ENGINE *e, void *data);
 
+int test_aes_ctr_iv_init_regression(ENGINE *, void *);
+
 #endif
 
 #ifdef WE_HAVE_AESGCM