Error finalizing cipher loop" when running openssl speed -evp -decrypt for AES-GCM about openssl HOT 7 OPEN

@nhorman In general when I've written tests like this for other systems, I've gone in the direction you state (perform a genuine encryption and use that to test decryption performance). However, OpenSSL hasn't done that, so we are purely testing performance here, not correctness (which is why my original fix here, that didn't quite fix it). Since the algorithms we are testing always perform the decryption and tag computations, and only diverge in behaviour when checking the tag at the very last step, performance is still representative. We might want a note to that effect, either as a comment in the code or "elsewhere" (but I not sure where that else might be!)

from openssl.

@tom-cosgrove-arm can you comment on the fix you made in #23757? I'm having trouble seeing how a fake tag can be used with aes in ccm or gcm mode? I think it makes sense in aead mode because (again I think) the tag is validated independently of the decrypt operation, but in ccm mode the tag is never checked in the decrypt operation. Its only checked in gcm mode, and I can't see how an arbitrary tag will pass that check. It seems like instead for gcm mode, we should do an initial encryption of a buffer to obtain a tag and use that computed value consistently during the decrypt operation

from openssl.

The original failure was due to there not being a tag, and the change I made did fix that. However, it looks rather embarrassingly as if I did the work on an out-of-date branch and then rebased without re-testing. I made a comment about "We don't check the return value" which used to be true, but which certainly isn't true these days.

I think the best fix would be something like this - what do you think?

diff --git a/apps/speed.c b/apps/speed.c
index 1fd7eb26b6..ca87302e22 100644
--- a/apps/speed.c
+++ b/apps/speed.c
@@ -882,10 +882,12 @@ static int EVP_Update_loop(void *args)
             }
         }
     }
-    if (decrypt)
-        rc = EVP_DecryptFinal_ex(ctx, buf, &outl);
-    else
+    if (decrypt) {
+        (void)EVP_DecryptFinal_ex(ctx, buf, &outl);
+        rc = 1; /* DecryptFinal will fail because the tag won't verify */
+    } else {
         rc = EVP_EncryptFinal_ex(ctx, buf, &outl);
+    }
 
     if (rc == 0)
         BIO_printf(bio_err, "Error finalizing cipher loop\n");

from openssl.

It seems like instead for gcm mode, we should do an initial encryption of a buffer to obtain a tag and use that computed value consistently during the decrypt operation

That would definitely be the best option

from openssl.

@ravulapx it seems like the consensus here is that the appropriate fix is, for gcm mode speed tests:

• Modify the speed_main function such that, when loopfunction is set to EVP_Update_loop, and -decrypt is selected, that we preform an initial encryption of a data buffer, and extract the tag from that result
• Modify EVP_Update_loop such that if a tag is assigned in speed_main, we use that as the input tag for decryption

that should avoid this problem. Unfortunately I don't know when we are going to be able to get to this. Would you be willing to create a PR for this effort?

from openssl.

@tom-cosgrove-arm your solution would certainly work. I guess the question is "is it ok for us to test algorithm speed on failing inputs"? If so, that would be a faster solution

from openssl.

@ravulapx are you interested in attempting to write a patch for this and submit it as a pull request, guided by the conversation above?

from openssl.