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 <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/8305)

diff --git a/engines/e_dasync.c b/engines/e_dasync.c
index f8a52e9da290399d3262bbbccdacb7c56ff833cc..74a62b86e0799566d9db771247630fd1b8a6cff4 100644 (file)
--- 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());
 }