20231201-openssl-compat-fixes #7031
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…itialized data and resource leak defects around wc_AesInit() and wc_AesFree(): * followup to wolfSSL#7009 "20231128-misc-fixes" and wolfSSL#7011 "Add missing wc_AesInit calls." * adds WC_DEBUG_CIPHER_LIFECYCLE, which embeds asserts in low-level AES implementations for proper usage of wc_AesInit() and wc_AesFree(). * fixes native CMAC, AES-EAX, and AES-XTS implementations to assure resource release. * adds missing wc_AesXtsInit() API, and adds a new wc_AesXtsSetKey_NoInit(). * fixes misspellings in EVP that unconditionally gated out AES-OFB and AES-XTS. * fixes misspellings in EVP that unconditionally gated out AES-CBC and AES-CFB code in wolfSSL_EVP_CIPHER_CTX_cleanup_cipher(). * openssl compat AES low level cipher API has no counterpart to wc_AesFree(), so these compat APIs will now be gated out in configurations where they would otherwise leak memory or file descriptors (WOLFSSL_AFALG, WOLFSSL_DEVCRYPTO, WOLF_CRYPTO_CB, etc.). A new macro, WC_AESFREE_IS_MANDATORY, is defined in wolfcrypt/aes.h to streamline this dependency. * fixes 40 missing EVP_CIPHER_CTX_cleanup()s and 11 wc_AesFree()s in src/ssl.c, src/ssl_crypto.c, tests/api.c, and wolfcrypt/test/test.c.
philljj
reviewed
Dec 6, 2023
JacobBarthelmeh
previously requested changes
Dec 6, 2023
|
re wolfssl/wolfcrypt/aes.h L 402:
The macros at issue refer only to the macros used in I.e. |
philljj
reviewed
Dec 6, 2023
…e wc_CmacFinal() as wc_CmacFinalNoFree() removing its deallocation clauses, and add new wc_CmacFinal() that calls wc_CmacFinalNoFree() then calls wc_CmacFree() unconditionally, for compatibility with legacy client code (some of which may have previously leaked). tests/api.c: modify test_wc_CmacFinal() to use wc_CmacFinalNoFree() except for the final call. wolfcrypt/src/aes.c: * fix wc_AesEaxEncryptAuth() and wc_AesEaxDecryptAuth() to call wc_AesEaxFree() only if wc_AesEaxInit() succeeded. * fix wc_AesEaxInit() to free all resources on failure. * revert wc_AesEaxEncryptFinal() and wc_AesEaxDecryptFinal() changes, then change wc_CmacFinal() calls in them to wc_CmacFinalNoFree() calls. * wc_AesEaxFree(): add wc_CmacFree() calls.
SparkiDev
requested changes
Dec 6, 2023
…acFinalNoFree(), and wc_CmacFree(); rename wc_AesXtsSetKey_NoInit() to wc_AesXtsSetKeyNoInit() for morphological consistency; refactor wc_AesXtsSetKey() to call wc_AesXtsSetKeyNoInit() and clean up on failure; readability tweak in wolfSSL_EVP_CipherFinal().
|
retest this please. |
|
retest this please. |
philljj
reviewed
Dec 7, 2023
SparkiDev
approved these changes
Dec 7, 2023
philljj
approved these changes
Dec 7, 2023
JacobBarthelmeh
approved these changes
Dec 9, 2023
douzzer
added a commit
to douzzer/wolfssl
that referenced
this pull request
Dec 11, 2023
…ate out AES-XTS functionality that depends on new APIs added in wolfSSL#7031 (b14aba4 and 931ac4e) (AES-XTS is non-FIPS in FIPS <5.3).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fix AES-related code, in both crypto and TLS layers, for various uninitialized data and resource leak defects around
wc_AesInit()andwc_AesFree():followup to 20231128-misc-fixes #7009 and Add missing wc_AesInit calls. #7011
adds
WC_DEBUG_CIPHER_LIFECYCLE, which embeds asserts in low-level AES implementations for proper usage ofwc_AesInit()andwc_AesFree().fixes native CMAC, AES-EAX, and AES-XTS implementations to assure resource release.
adds missing
wc_AesXtsInit()API, and adds a newwc_AesXtsSetKey_NoInit().fixes misspellings in EVP that unconditionally gated out AES-OFB and AES-XTS.
fixes misspellings in EVP that unconditionally gated out AES-CBC and AES-CFB code in
wolfSSL_EVP_CIPHER_CTX_cleanup_cipher().openssl compat AES low level cipher API has no counterpart to
wc_AesFree(), so these compat APIs will now be gated out in configurations where they would otherwise leak memory or file descriptors (WOLFSSL_AFALG,WOLFSSL_DEVCRYPTO,WOLF_CRYPTO_CB, etc.). A new macro,WC_AESFREE_IS_MANDATORY, is defined inwolfcrypt/aes.hto streamline this dependency.fixes 40 missing
EVP_CIPHER_CTX_cleanup()s and 11wc_AesFree()s insrc/ssl.c,src/ssl_crypto.c,tests/api.c, andwolfcrypt/test/test.c.tested with
wolfssl-multi-test.sh ... super-quick-check fips-140-3-dev-kcapi cross-armv7a-armasm-fips-140-3-ready-sp-all-testsuite-sanitizer cross-armv7a-all-armasm-testwolfcrypt-sanitizer sp-all-asm-smallstack-sanitizer-fips-140-3-dev sanitizer-all-asm-smallstack-async all-sp-all-Os-sanitizer cross-aarch64-all-armasm-unittest-sanitizer linuxkm-aesni-ksanitize-insmod sanitizer-defaults all-afalg, with-DWC_DEBUG_CIPHER_LIFECYCLEadded to all non-fips sanitizer scenarios.