Defense against caching attacks when using HW acceleration can be wrong

[lucperneel]

Description

• Type: Bug

---

Bug

OS
RTOS using mbetls with HW acceleration suppport

mbed TLS build:
Version: 2.13.1 , but checked the master branch and issue is still present there

Expected behavior
Dummy hash (or any other calculation) against cache/timing attacks should be effective also when hardware acceleration is used.

Therefore all calculation loops have to follow the rule:

• starts
• loop ( update )
• finish

Also the dummy ones just used as a defense against these attacks.
The thing is that this order is expected by hardware accelerated builds.
As an example, say that the hardware does a hash calculation on 64-byte
blocks:

• start will assure that in your context the buffer is reset and clear
• on update the message data will:
  • be handled, but only in blocks of this 64-byte size.
    Rest will be stored in context temporary buffer
• on finish, the temporary buffer is pushed to the hardware, which will now
  pad this internally to this 64-bytes.

Without the start/finish code, there is nothing sure about how the update
will be done. Because of invalid context state, just an error could be returned.
If the context state would still be valid (due to previous unrelated calls)
you can still not guarantee that it will do actually something: it could
be just a memcpy operation to the temporary context buffer.
As such, this construction will not solve anything against timing/cache attacks,
as there is no guarantee at all that these functions do something.

Actual behavior
not done in ssl_decrypt_buf() function

Solution (proposed bugfix)

diff --git a/mbedTLS/library/ssl_tls.c b/mbedTLS/library/ssl_tls.c
index 8bd74db..336eeff 100644
--- a/mbedTLS/library/ssl_tls.c
+++ b/mbedTLS/library/ssl_tls.c
@@ -2269,8 +2269,21 @@ static int ssl_decrypt_buf( mbedtls_ssl_context *ssl )

             /* Call mbedtls_md_process at least once due to cache attacks
              * that observe whether md_process() was called of not */
-            for( j = 0; j < extra_run + 1; j++ )
-                mbedtls_md_process( &ssl->transform_in->md_ctx_dec, ssl->in_msg );
+            {
+                /* Even in this "dummy" case the normal sequence starts -> N*update -> finish should be used.
+                   Not doing so is problematic when using a hardware acceleration library which expect
+                   this order (on start it inits a hardware related context, which is then used on
+                   subsequent updates and cleaned up on finish).
+                   In fact the update (mbetls_md_process) will fail (due to not having
+                   this acceleration context inited) and as the return value is silently ignored,
+                   the load against attacks would do nothing.... and thus not be effective at all.
+                   So also here the starts -> N*update -> finish logic should be followed! */
+                unsigned char dummy_hash[64];
+                mbedtls_md_starts( &ssl->transform_in->md_ctx_dec );
+                for( j = 0; j < extra_run + 1; j++ )
+                    mbedtls_md_process( &ssl->transform_in->md_ctx_dec, ssl->in_msg );
+                mbedtls_md_finish( &ssl->transform_in->md_ctx_dec, dummy_hash );
+            }

             mbedtls_md_hmac_reset( &ssl->transform_in->md_ctx_dec );

[mpg]

Hi @lucperneel ! Thanks for reporting this. We agree with your analysis of the problem, and are currently discussing aspects of the solution, which relate to other aspects of the issue that had been found internally. Given the security aspects of the issue, unfortunately most of the discussion is likely to remain private for the time being, but I wanted to let you know that we're working on it, so you know your report hasn't been ignored!

[lucperneel]

Hi Manual, No problem. I also just added a fix that worked for us (where a hardware acceleration unit is used). Indeed, looking in the security aspects of such changes is not that trivial ;-) Best Regards, Luc   ----- Original Message ----- From: Manuel Pégourié-Gonnard (notifications@github.com) Date: 05/04/20 12:52 To: ARMmbed/mbedtls (mbedtls@noreply.github.com) Cc: lucperneel (l.perneel@luperco.com), Mention (mention@noreply.github.com) Subject: Re: [ARMmbed/mbedtls] Defense against caching attacks when using HW acceleration can be wrong (#3246) Hi @lucperneel ! Thanks for reporting this. We agree with your analysis of the problem, and are currently discussing aspects of the solution, which relate to other aspects of the issue that had been found internally. Given the security aspects of the issue, unfortunately most of the discussion is likely to remain private for the time being, but I wanted to let you know that we're working on it, so you know your report hasn't been ignored! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.