From 0b4c09cd421f85eef8896c42e7a6b04d12dafd1b Mon Sep 17 00:00:00 2001 From: Colton Willey Date: Wed, 5 Jun 2024 11:35:03 -0700 Subject: [PATCH 1/3] Modify AES-CTR to not reinit after being keyed --- src/we_aes_ctr.c | 7 +++++-- test/test_cipher.c | 12 +++++++++++- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/src/we_aes_ctr.c b/src/we_aes_ctr.c index 0252950..08cc23f 100644 --- a/src/we_aes_ctr.c +++ b/src/we_aes_ctr.c @@ -34,6 +34,7 @@ typedef struct we_AesCtr { /* The wolfSSL AES data object. */ Aes aes; + word32 keyed; } we_AesCtr; @@ -67,7 +68,8 @@ static int we_aes_ctr_init(EVP_CIPHER_CTX *ctx, const unsigned char *key, ret = 0; } - if (ret == 1) { + /* Do not reinitialize if already keyed, unless setting a new key */ + if ((ret == 1) && ((aes->keyed == 0) || (key != NULL))) { rc = wc_AesInit(&aes->aes, NULL, INVALID_DEVID); if (rc != 0) { WOLFENGINE_ERROR_FUNC(WE_LOG_CIPHER, "wc_AesInit", rc); @@ -86,6 +88,7 @@ static int we_aes_ctr_init(EVP_CIPHER_CTX *ctx, const unsigned char *key, WOLFENGINE_ERROR_FUNC(WE_LOG_CIPHER, "wc_AesSetKey", rc); ret = 0; } + aes->keyed = 1; } if ((ret == 1) && (iv != NULL)) { rc = wc_AesSetIV(&aes->aes, iv); @@ -94,7 +97,7 @@ static int we_aes_ctr_init(EVP_CIPHER_CTX *ctx, const unsigned char *key, ret = -1; } else { - /* + /* * wc_AesSetIV should clear this field, but it doesn't in some * wolfSSL versions. */ diff --git a/test/test_cipher.c b/test/test_cipher.c index 2c90a87..bea7632 100644 --- a/test/test_cipher.c +++ b/test/test_cipher.c @@ -599,7 +599,17 @@ int test_aes_ctr_leftover_data_regression(ENGINE *e, void *data) } } - /* Try the other way, now. Encrypt with wolfEngine, decrypt with wolfSSL. */ + /* Try the other way, now. Encrypt with wolfEngine, decrypt with openSSL. + * The EVP_CIPHER_CTX remembers any engine it was loaded with, meaning we + * need to reset the ctxs before reuse or the decCtx will still pick up + * wolfEngine */ + if (err == 0) { + err = EVP_CIPHER_CTX_reset(encCtx) != 1; + } + if (err == 0) { + err = EVP_CIPHER_CTX_reset(decCtx) != 1; + } + if (err == 0) { err = EVP_CipherInit_ex(encCtx, EVP_aes_128_ctr(), e, key, NULL, -1) != 1; From e6997da351fa9ecbbf98d7e35397f1f39019cd8b Mon Sep 17 00:00:00 2001 From: Colton Willey Date: Wed, 5 Jun 2024 12:00:29 -0700 Subject: [PATCH 2/3] Change var name to inited, and only allow one init even when rekeying --- src/we_aes_ctr.c | 6 +++--- test/test_cipher.c | 9 +++++++-- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/we_aes_ctr.c b/src/we_aes_ctr.c index 08cc23f..3620907 100644 --- a/src/we_aes_ctr.c +++ b/src/we_aes_ctr.c @@ -34,7 +34,7 @@ typedef struct we_AesCtr { /* The wolfSSL AES data object. */ Aes aes; - word32 keyed; + word32 inited; } we_AesCtr; @@ -69,12 +69,13 @@ static int we_aes_ctr_init(EVP_CIPHER_CTX *ctx, const unsigned char *key, } /* Do not reinitialize if already keyed, unless setting a new key */ - if ((ret == 1) && ((aes->keyed == 0) || (key != NULL))) { + if ((ret == 1) && (aes->inited == 0)) { rc = wc_AesInit(&aes->aes, NULL, INVALID_DEVID); if (rc != 0) { WOLFENGINE_ERROR_FUNC(WE_LOG_CIPHER, "wc_AesInit", rc); ret = 0; } + aes->inited = 1; } if ((ret == 1) && (key != NULL)) { if (tmpIv == NULL) { @@ -88,7 +89,6 @@ static int we_aes_ctr_init(EVP_CIPHER_CTX *ctx, const unsigned char *key, WOLFENGINE_ERROR_FUNC(WE_LOG_CIPHER, "wc_AesSetKey", rc); ret = 0; } - aes->keyed = 1; } if ((ret == 1) && (iv != NULL)) { rc = wc_AesSetIV(&aes->aes, iv); diff --git a/test/test_cipher.c b/test/test_cipher.c index bea7632..218d2b3 100644 --- a/test/test_cipher.c +++ b/test/test_cipher.c @@ -603,11 +603,16 @@ int test_aes_ctr_leftover_data_regression(ENGINE *e, void *data) * The EVP_CIPHER_CTX remembers any engine it was loaded with, meaning we * need to reset the ctxs before reuse or the decCtx will still pick up * wolfEngine */ + if (encCtx != NULL) + EVP_CIPHER_CTX_free(encCtx); + if (decCtx != NULL) + EVP_CIPHER_CTX_free(decCtx); + if (err == 0) { - err = EVP_CIPHER_CTX_reset(encCtx) != 1; + err = (encCtx = EVP_CIPHER_CTX_new()) == NULL; } if (err == 0) { - err = EVP_CIPHER_CTX_reset(decCtx) != 1; + err = (decCtx = EVP_CIPHER_CTX_new()) == NULL; } if (err == 0) { From 4e2e1eb892874c81858aa70771f3bbc683c402fe Mon Sep 17 00:00:00 2001 From: Colton Willey Date: Thu, 6 Jun 2024 12:45:10 -0700 Subject: [PATCH 3/3] Add err check per review comments --- test/test_cipher.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/test/test_cipher.c b/test/test_cipher.c index 218d2b3..0614edc 100644 --- a/test/test_cipher.c +++ b/test/test_cipher.c @@ -603,10 +603,14 @@ int test_aes_ctr_leftover_data_regression(ENGINE *e, void *data) * The EVP_CIPHER_CTX remembers any engine it was loaded with, meaning we * need to reset the ctxs before reuse or the decCtx will still pick up * wolfEngine */ - if (encCtx != NULL) - EVP_CIPHER_CTX_free(encCtx); - if (decCtx != NULL) - EVP_CIPHER_CTX_free(decCtx); + if (err == 0) { + if (encCtx != NULL) + EVP_CIPHER_CTX_free(encCtx); + } + if (err == 0) { + if (decCtx != NULL) + EVP_CIPHER_CTX_free(decCtx); + } if (err == 0) { err = (encCtx = EVP_CIPHER_CTX_new()) == NULL;