From 01a6dded7241be347543ec610e97a7b5c501bc70 Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Mon, 27 Apr 2020 13:35:51 +0200 Subject: [PATCH 1/2] Fix AES-GCM in EVP layer to have compatiblity with OpenSSL - Tag checking in AES-GCM is done in Final call - Reset `WOLFSSL_EVP_CIPHER_CTX` structure after Final call - Don't zero `ctx->authTag` struct in Init call so that user can get the AES-GCM tag using `EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_GCM_GET_TAG, AES_BLOCK_SIZE, tag)` - `ctx->authTag` is only zeroed before authenticated, non-confidential data Update call since this means we are entering a new Udate-Final cycle. This doesn't need to be done in the decrypt case since the tag should be supplied by the user before the final call using `EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_GCM_SET_TAG, AES_BLOCK_SIZE, tag)` --- configure.ac | 4 +- tests/api.c | 7 +- wolfcrypt/src/evp.c | 246 +++++++++++++++++++++---------------- wolfssl/openssl/evp.h | 4 + wolfssl/openssl/opensslv.h | 4 +- 5 files changed, 151 insertions(+), 114 deletions(-) diff --git a/configure.ac b/configure.ac index ff6b79f86..b67cc7a74 100644 --- a/configure.ac +++ b/configure.ac @@ -440,7 +440,7 @@ AC_ARG_ENABLE([mcast], # List of open source project defines using our openssl compatibility layer: # openssh (--enable-openssh) WOLFSSL_OPENSSH -# openvpn (--enable-openvpn) +# openvpn (--enable-openvpn) WOLFSSL_OPENVPN # nginix (--enable-nginx) WOLFSSL_NGINX # haproxy (--enable-haproxy) WOLFSSL_HAPROXY # wpa_supplicant (--enable-wpas) WOLFSSL_WPAS @@ -3543,7 +3543,7 @@ fi if test "$ENABLED_OPENVPN" = "yes" then - AM_CFLAGS="$AM_CFLAGS -DWOLFSSL_DES_ECB -DHAVE_EX_DATA -DWOLFSSL_KEY_GEN" + AM_CFLAGS="$AM_CFLAGS -DWOLFSSL_DES_ECB -DHAVE_EX_DATA -DWOLFSSL_KEY_GEN -DWOLFSSL_OPENVPN" fi diff --git a/tests/api.c b/tests/api.c index b69812605..3cf7d71b5 100644 --- a/tests/api.c +++ b/tests/api.c @@ -30844,10 +30844,10 @@ static void test_wolfssl_EVP_aes_gcm(void) } AssertIntEQ(1, EVP_DecryptUpdate(&de[i], NULL, &len, aad, aadSz)); - AssertIntEQ(1, EVP_CIPHER_CTX_ctrl(&de[i], EVP_CTRL_GCM_SET_TAG, AES_BLOCK_SIZE, tag)); AssertIntEQ(1, EVP_DecryptUpdate(&de[i], decryptedtxt, &len, ciphertxt, ciphertxtSz)); decryptedtxtSz = len; - AssertIntGT(EVP_DecryptFinal_ex(&de[i], decryptedtxt, &len), 0); + AssertIntEQ(1, EVP_CIPHER_CTX_ctrl(&de[i], EVP_CTRL_GCM_SET_TAG, AES_BLOCK_SIZE, tag)); + AssertIntEQ(1, EVP_DecryptFinal_ex(&de[i], decryptedtxt, &len)); decryptedtxtSz += len; AssertIntEQ(ciphertxtSz, decryptedtxtSz); AssertIntEQ(0, XMEMCMP(plaintxt, decryptedtxt, decryptedtxtSz)); @@ -30857,7 +30857,8 @@ static void test_wolfssl_EVP_aes_gcm(void) AssertIntEQ(1, EVP_DecryptUpdate(&de[i], NULL, &len, aad, aadSz)); AssertIntEQ(1, EVP_CIPHER_CTX_ctrl(&de[i], EVP_CTRL_GCM_SET_TAG, AES_BLOCK_SIZE, tag)); /* fail due to wrong tag */ - AssertIntEQ(0, EVP_DecryptUpdate(&de[i], decryptedtxt, &len, ciphertxt, ciphertxtSz)); + AssertIntEQ(1, EVP_DecryptUpdate(&de[i], decryptedtxt, &len, ciphertxt, ciphertxtSz)); + AssertIntEQ(0, EVP_DecryptFinal_ex(&de[i], decryptedtxt, &len)); AssertIntEQ(0, len); } printf(resultFmt, passed); diff --git a/wolfcrypt/src/evp.c b/wolfcrypt/src/evp.c index d9207900c..033ba9074 100644 --- a/wolfcrypt/src/evp.c +++ b/wolfcrypt/src/evp.c @@ -417,47 +417,6 @@ static int evpCipherBlock(WOLFSSL_EVP_CIPHER_CTX *ctx, ret = wc_AesCbcDecrypt(&ctx->cipher.aes, out, in, inl); break; #endif - #if defined(HAVE_AESGCM) - case AES_128_GCM_TYPE: - case AES_192_GCM_TYPE: - case AES_256_GCM_TYPE: - if (ctx->enc) { - if (out){ - /* encrypt confidential data*/ - ret = wc_AesGcmEncrypt(&ctx->cipher.aes, out, in, inl, - ctx->iv, ctx->ivSz, ctx->authTag, ctx->authTagSz, - NULL, 0); - } - else { - /* authenticated, non-confidential data */ - ret = wc_AesGcmEncrypt(&ctx->cipher.aes, NULL, NULL, 0, - ctx->iv, ctx->ivSz, ctx->authTag, ctx->authTagSz, - in, inl); - /* Reset partial authTag error for AAD*/ - if (ret == AES_GCM_AUTH_E) - ret = 0; - } - } - else { - if (out){ - /* decrypt confidential data*/ - ret = wc_AesGcmDecrypt(&ctx->cipher.aes, out, in, inl, - ctx->iv, ctx->ivSz, ctx->authTag, ctx->authTagSz, - NULL, 0); - } - else { - /* authenticated, non-confidential data*/ - ret = wc_AesGcmDecrypt(&ctx->cipher.aes, NULL, NULL, 0, - ctx->iv, ctx->ivSz, - ctx->authTag, ctx->authTagSz, - in, inl); - /* Reset partial authTag error for AAD*/ - if (ret == AES_GCM_AUTH_E) - ret = 0; - } - } - break; - #endif #if defined(WOLFSSL_AES_COUNTER) case AES_128_CTR_TYPE: case AES_192_CTR_TYPE: @@ -575,10 +534,60 @@ static int wolfSSL_EVP_CipherUpdate_GCM(WOLFSSL_EVP_CIPHER_CTX *ctx, unsigned char *out, int *outl, const unsigned char *in, int inl) { - /* process blocks */ - if (evpCipherBlock(ctx, out, in, inl) == 0) - return WOLFSSL_FAILURE; + int ret = 0; + *outl = inl; + if (ctx->enc) { + if (out) { + /* encrypt confidential data*/ + ret = wc_AesGcmEncrypt(&ctx->cipher.aes, out, in, inl, + ctx->iv, ctx->ivSz, ctx->authTag, ctx->authTagSz, + NULL, 0); + } + else { + /* authenticated, non-confidential data */ + XMEMSET(ctx->authTag, 0, ctx->authTagSz); + ret = wc_AesGcmEncrypt(&ctx->cipher.aes, NULL, NULL, 0, + ctx->iv, ctx->ivSz, ctx->authTag, ctx->authTagSz, + in, inl); + /* Reset partial authTag error for AAD*/ + if (ret == AES_GCM_AUTH_E) + ret = 0; + } + } + else { + if (out) { + byte* tmp; + tmp = (byte*)XREALLOC(ctx->gcmDecryptBuffer, + ctx->gcmDecryptBufferLen + inl, NULL, + DYNAMIC_TYPE_OPENSSL); + if (tmp) { + XMEMCPY(tmp + ctx->gcmDecryptBufferLen, in, inl); + ctx->gcmDecryptBufferLen += inl; + ctx->gcmDecryptBuffer = tmp; + *outl = 0; + } + else { + ret = WOLFSSL_FAILURE; + } + } + else { + /* authenticated, non-confidential data*/ + ret = wc_AesGcmDecrypt(&ctx->cipher.aes, NULL, NULL, 0, + ctx->iv, ctx->ivSz, + ctx->authTag, ctx->authTagSz, + in, inl); + /* Reset partial authTag error for AAD*/ + if (ret == AES_GCM_AUTH_E) + ret = 0; + } + } + + if (ret != 0) { + *outl = 0; + return WOLFSSL_FAILURE; + } + return WOLFSSL_SUCCESS; } #endif @@ -739,76 +748,95 @@ int wolfSSL_EVP_CipherFinal(WOLFSSL_EVP_CIPHER_CTX *ctx, return WOLFSSL_FAILURE; WOLFSSL_ENTER("wolfSSL_EVP_CipherFinal"); - + switch (ctx->cipherType) { #if !defined(NO_AES) && defined(HAVE_AESGCM) - switch (ctx->cipherType) { - case AES_128_GCM_TYPE: - case AES_192_GCM_TYPE: - case AES_256_GCM_TYPE: - *outl = 0; - /* Clear IV, since IV reuse is not recommended for AES GCM. */ - XMEMSET(ctx->iv, 0, AES_BLOCK_SIZE); - return WOLFSSL_SUCCESS; - default: - /* fall-through */ - break; - } -#endif /* !NO_AES && HAVE_AESGCM */ + case AES_128_GCM_TYPE: + case AES_192_GCM_TYPE: + case AES_256_GCM_TYPE: + if (!ctx->enc && ctx->gcmDecryptBuffer && ctx->gcmDecryptBufferLen > 0) { + /* decrypt confidential data*/ + ret = wc_AesGcmDecrypt(&ctx->cipher.aes, out, + ctx->gcmDecryptBuffer, ctx->gcmDecryptBufferLen, + ctx->iv, ctx->ivSz, ctx->authTag, ctx->authTagSz, + NULL, 0); + if (ret == 0) { + ret = WOLFSSL_SUCCESS; + *outl = ctx->gcmDecryptBufferLen; + } + else { + ret = WOLFSSL_FAILURE; + *outl = 0; + } - if (!out) - return WOLFSSL_FAILURE; - - if (ctx->flags & WOLFSSL_EVP_CIPH_NO_PADDING) { - if (ctx->bufUsed != 0) return WOLFSSL_FAILURE; - *outl = 0; - } - else if (ctx->enc) { - if (ctx->block_size == 1) { - *outl = 0; - } - else if ((ctx->bufUsed >= 0) && (ctx->block_size != 1)) { - padBlock(ctx); - PRINT_BUF(ctx->buf, ctx->block_size); - if (evpCipherBlock(ctx, out, ctx->buf, ctx->block_size) == 0) { - WOLFSSL_MSG("Final Cipher Block failed"); - ret = WOLFSSL_FAILURE; + XFREE(ctx->gcmDecryptBuffer, NULL, DYNAMIC_TYPE_OPENSSL); + ctx->gcmDecryptBuffer = NULL; + ctx->gcmDecryptBufferLen = 0; } else { - PRINT_BUF(out, ctx->block_size); - *outl = ctx->block_size; + *outl = 0; } - } - } - else { - if (ctx->block_size == 1) { - *outl = 0; - } - else if ((ctx->bufUsed % ctx->block_size) != 0) { - *outl = 0; - /* not enough padding for decrypt */ - WOLFSSL_MSG("Final Cipher Block not enough padding"); - ret = WOLFSSL_FAILURE; - } - else if (ctx->lastUsed) { - PRINT_BUF(ctx->lastBlock, ctx->block_size); - if ((fl = checkPad(ctx, ctx->lastBlock)) >= 0) { - XMEMCPY(out, ctx->lastBlock, fl); - *outl = fl; - if (ctx->lastUsed == 0 && ctx->bufUsed == 0) { - /* return error in cases where the block length is incorrect */ - WOLFSSL_MSG("Final Cipher Block bad length"); - ret = WOLFSSL_FAILURE; + /* Clear IV, since IV reuse is not recommended for AES GCM. */ + XMEMSET(ctx->iv, 0, AES_BLOCK_SIZE); + break; +#endif /* !NO_AES && HAVE_AESGCM */ + default: + if (!out) + return WOLFSSL_FAILURE; + + if (ctx->flags & WOLFSSL_EVP_CIPH_NO_PADDING) { + if (ctx->bufUsed != 0) return WOLFSSL_FAILURE; + *outl = 0; + } + else if (ctx->enc) { + if (ctx->block_size == 1) { + *outl = 0; + } + else if ((ctx->bufUsed >= 0) && (ctx->block_size != 1)) { + padBlock(ctx); + PRINT_BUF(ctx->buf, ctx->block_size); + if (evpCipherBlock(ctx, out, ctx->buf, ctx->block_size) == 0) { + WOLFSSL_MSG("Final Cipher Block failed"); + ret = WOLFSSL_FAILURE; + } + else { + PRINT_BUF(out, ctx->block_size); + *outl = ctx->block_size; + } } } else { - ret = WOLFSSL_FAILURE; + if (ctx->block_size == 1) { + *outl = 0; + } + else if ((ctx->bufUsed % ctx->block_size) != 0) { + *outl = 0; + /* not enough padding for decrypt */ + WOLFSSL_MSG("Final Cipher Block not enough padding"); + ret = WOLFSSL_FAILURE; + } + else if (ctx->lastUsed) { + PRINT_BUF(ctx->lastBlock, ctx->block_size); + if ((fl = checkPad(ctx, ctx->lastBlock)) >= 0) { + XMEMCPY(out, ctx->lastBlock, fl); + *outl = fl; + if (ctx->lastUsed == 0 && ctx->bufUsed == 0) { + /* return error in cases where the block length is incorrect */ + WOLFSSL_MSG("Final Cipher Block bad length"); + ret = WOLFSSL_FAILURE; + } + } + else { + ret = WOLFSSL_FAILURE; + } + } + else if (ctx->lastUsed == 0 && ctx->bufUsed == 0) { + /* return error in cases where the block length is incorrect */ + ret = WOLFSSL_FAILURE; + } } - } - else if (ctx->lastUsed == 0 && ctx->bufUsed == 0) { - /* return error in cases where the block length is incorrect */ - ret = WOLFSSL_FAILURE; - } + break; } + if (ret == WOLFSSL_SUCCESS) { /* reset cipher state after final */ wolfSSL_EVP_CipherInit(ctx, NULL, NULL, NULL, -1); @@ -4010,6 +4038,13 @@ int wolfSSL_EVP_MD_type(const WOLFSSL_EVP_MD *md) if (ctx) { ctx->cipherType = WOLFSSL_EVP_CIPH_TYPE_INIT; /* not yet initialized */ ctx->keyLen = 0; +#ifdef HAVE_AESGCM + if (ctx->gcmDecryptBuffer) { + XFREE(ctx->gcmDecryptBuffer, NULL, DYNAMIC_TYPE_OPENSSL); + ctx->gcmDecryptBuffer = NULL; + } + ctx->gcmDecryptBufferLen = 0; +#endif } return WOLFSSL_SUCCESS; @@ -4236,7 +4271,6 @@ int wolfSSL_EVP_MD_type(const WOLFSSL_EVP_MD *md) ctx->authTagSz = AES_BLOCK_SIZE; ctx->ivSz = GCM_NONCE_MID_SZ; - XMEMSET(ctx->authTag, 0, ctx->authTagSz); if (key && wc_AesGcmSetKey(&ctx->cipher.aes, key, ctx->keyLen)) { WOLFSSL_MSG("wc_AesGcmSetKey() failed"); return WOLFSSL_FAILURE; @@ -4261,7 +4295,6 @@ int wolfSSL_EVP_MD_type(const WOLFSSL_EVP_MD *md) ctx->authTagSz = AES_BLOCK_SIZE; ctx->ivSz = GCM_NONCE_MID_SZ; - XMEMSET(ctx->authTag, 0, ctx->authTagSz); if (key && wc_AesGcmSetKey(&ctx->cipher.aes, key, ctx->keyLen)) { WOLFSSL_MSG("wc_AesGcmSetKey() failed"); return WOLFSSL_FAILURE; @@ -4286,7 +4319,6 @@ int wolfSSL_EVP_MD_type(const WOLFSSL_EVP_MD *md) ctx->authTagSz = AES_BLOCK_SIZE; ctx->ivSz = GCM_NONCE_MID_SZ; - XMEMSET(ctx->authTag, 0, ctx->authTagSz); if (key && wc_AesGcmSetKey(&ctx->cipher.aes, key, ctx->keyLen)) { WOLFSSL_MSG("wc_AesGcmSetKey() failed"); return WOLFSSL_FAILURE; diff --git a/wolfssl/openssl/evp.h b/wolfssl/openssl/evp.h index 3f9026ac3..f7fff7e47 100644 --- a/wolfssl/openssl/evp.h +++ b/wolfssl/openssl/evp.h @@ -350,6 +350,10 @@ struct WOLFSSL_EVP_CIPHER_CTX { defined(HAVE_AESGCM) || defined (WOLFSSL_AES_XTS) #define HAVE_WOLFSSL_EVP_CIPHER_CTX_IV int ivSz; +#ifdef HAVE_AESGCM + byte* gcmDecryptBuffer; + int gcmDecryptBufferLen; +#endif ALIGN16 unsigned char authTag[AES_BLOCK_SIZE]; int authTagSz; #endif diff --git a/wolfssl/openssl/opensslv.h b/wolfssl/openssl/opensslv.h index 1ec8db137..7f82800d3 100644 --- a/wolfssl/openssl/opensslv.h +++ b/wolfssl/openssl/opensslv.h @@ -31,10 +31,10 @@ #define OPENSSL_VERSION_NUMBER 0x10100000L #elif defined(OPENSSL_ALL) || defined(HAVE_STUNNEL) || defined(HAVE_LIGHTY) || \ defined(WOLFSSL_NGINX) || defined(WOLFSSL_HAPROXY) || \ - defined(WOLFSSL_OPENSSH) || defined(WOLFSSL_QT) + defined(WOLFSSL_OPENSSH) || defined(WOLFSSL_QT) || defined(WOLFSSL_OPENVPN) /* version number can be increased for Lighty after compatibility for ECDH is added */ - #define OPENSSL_VERSION_NUMBER 0x1000100fL + #define OPENSSL_VERSION_NUMBER 0x10001040L #else #define OPENSSL_VERSION_NUMBER 0x0090810fL #endif From c02c408409ae8b9b2ebcb540db27195f9cb72786 Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Tue, 28 Apr 2020 12:38:02 +0200 Subject: [PATCH 2/2] Only 80 characters a line --- wolfcrypt/src/evp.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/wolfcrypt/src/evp.c b/wolfcrypt/src/evp.c index 033ba9074..d24563132 100644 --- a/wolfcrypt/src/evp.c +++ b/wolfcrypt/src/evp.c @@ -753,7 +753,8 @@ int wolfSSL_EVP_CipherFinal(WOLFSSL_EVP_CIPHER_CTX *ctx, case AES_128_GCM_TYPE: case AES_192_GCM_TYPE: case AES_256_GCM_TYPE: - if (!ctx->enc && ctx->gcmDecryptBuffer && ctx->gcmDecryptBufferLen > 0) { + if (!ctx->enc && ctx->gcmDecryptBuffer && + ctx->gcmDecryptBufferLen > 0) { /* decrypt confidential data*/ ret = wc_AesGcmDecrypt(&ctx->cipher.aes, out, ctx->gcmDecryptBuffer, ctx->gcmDecryptBufferLen, @@ -820,7 +821,8 @@ int wolfSSL_EVP_CipherFinal(WOLFSSL_EVP_CIPHER_CTX *ctx, XMEMCPY(out, ctx->lastBlock, fl); *outl = fl; if (ctx->lastUsed == 0 && ctx->bufUsed == 0) { - /* return error in cases where the block length is incorrect */ + /* return error in cases where the block length is + * incorrect */ WOLFSSL_MSG("Final Cipher Block bad length"); ret = WOLFSSL_FAILURE; } @@ -830,7 +832,8 @@ int wolfSSL_EVP_CipherFinal(WOLFSSL_EVP_CIPHER_CTX *ctx, } } else if (ctx->lastUsed == 0 && ctx->bufUsed == 0) { - /* return error in cases where the block length is incorrect */ + /* return error in cases where the block length is + * incorrect */ ret = WOLFSSL_FAILURE; } }