From a4a0a1eb43cfccd128d085932a567e0482fbfe47 Mon Sep 17 00:00:00 2001 From: "Dr. Matthias St. Pierre" Date: Fri, 22 Feb 2019 13:08:54 +0100 Subject: [PATCH] engines/dasync: add explaining comments about AES-128-CBC-HMAC-SHA1 Fixes #7950 It was reported that there might be a null pointer dereference in the implementation of the dasync_aes_128_cbc_hmac_sha1() cipher, because EVP_aes_128_cbc_hmac_sha1() can return a null pointer if AES-NI is not available. It took some analysis to find out that this is not an issue in practice, and these comments explain the reason to comfort further NPD hunters. Detected by GitHub user @wurongxin1987 using the Sourcebrella Pinpoint static analyzer. Reviewed-by: Matt Caswell (Merged from https://github.com/openssl/openssl/pull/8305) --- engines/e_dasync.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/engines/e_dasync.c b/engines/e_dasync.c index f8a52e9da2..74a62b86e0 100644 --- a/engines/e_dasync.c +++ b/engines/e_dasync.c @@ -155,6 +155,14 @@ static const EVP_CIPHER *dasync_aes_128_cbc(void) /* * Holds the EVP_CIPHER object for aes_128_cbc_hmac_sha1 in this engine. Set up * once only during engine bind and can then be reused many times. + * + * This 'stitched' cipher depends on the EVP_aes_128_cbc_hmac_sha1() cipher, + * which is implemented only if the AES-NI instruction set extension is available + * (see OPENSSL_IA32CAP(3)). If that's not the case, then this cipher will not + * be available either. + * + * Note: Since it is a legacy mac-then-encrypt cipher, modern TLS peers (which + * negotiate the encrypt-then-mac extension) won't negotiate it anyway. */ static EVP_CIPHER *_hidden_aes_128_cbc_hmac_sha1 = NULL; static const EVP_CIPHER *dasync_aes_128_cbc_hmac_sha1(void) @@ -765,6 +773,10 @@ static int dasync_aes128_cbc_hmac_sha1_init_key(EVP_CIPHER_CTX *ctx, const unsigned char *iv, int enc) { + /* + * We can safely assume that EVP_aes_128_cbc_hmac_sha1() != NULL, + * see comment before the definition of dasync_aes_128_cbc_hmac_sha1(). + */ return dasync_cipher_init_key_helper(ctx, key, iv, enc, EVP_aes_128_cbc_hmac_sha1()); } @@ -779,5 +791,9 @@ static int dasync_aes128_cbc_hmac_sha1_cipher(EVP_CIPHER_CTX *ctx, static int dasync_aes128_cbc_hmac_sha1_cleanup(EVP_CIPHER_CTX *ctx) { + /* + * We can safely assume that EVP_aes_128_cbc_hmac_sha1() != NULL, + * see comment before the definition of dasync_aes_128_cbc_hmac_sha1(). + */ return dasync_cipher_cleanup_helper(ctx, EVP_aes_128_cbc_hmac_sha1()); }