search

Mbed-TLS / mbedtls

An open source, portable, easy to use, readable and flexible TLS library, and reference implementation of the PSA Cryptography API. Releases are on a varying cadence, typically around 3 - 6 months between releases.
https://www.trustedfirmware.org/projects/mbed-tls/
Other
5.53k stars 2.6k forks source link

Wrong usage of mbedtls_gcm_crypt_and_tag() during TLS message decryption #9599

Open ASeidelt opened 1 month ago

ASeidelt commented 1 month ago

mbedTLS version: 3.6.0 Platform: ARM STM32

While hunting a bug we noticed that the TLS buffer decryption does not follow the calling convention of mbedtls_gcm_crypt_and_tag() and mbedtls_gcm_auth_decrypt() (which calls mbedtls_gcm_crypt_and_tag() with the parameter MBEDTLS_GCM_DECRYPT).

Both function state that:

For decryption, the output buffer cannot be the same as input buffer. If the buffers overlap, the output buffer must trail at least 8 Bytes behind the input buffer.

mbedtls_ssl_decrypt_buf() calls mbedtls_cipher_auth_decrypt_ext() in line 1638 with 'data' as input and output buffer. These pointers are passed down to the aforementioned functions mostly unaltered as 'input' and 'output', but end up as the same pointer in mbedtls_gcm_crypt_and_tag() (as confirmed during debugging).

Please have a look at the attached StackTrace for details. image

davidhorstmann-arm commented 1 month ago

@ASeidelt Thanks for raising this. I have done some digging and it looks like this is an incorrect part of our documentation (rather than an incorrect usage of the function). It turns out that we even have a PR #7977 to fix it.

The limitation in mbedtls_gcm_crypt_and_tag() was a limitation in the original implementation that was fixed many years ago but the documentation was not updated.

Apologies for taking up your debugging time with an entirely preventable error in our docs, and I will make efforts to ensure that the fix is merged as soon as possible.

Many thanks again!

gilles-peskine-arm commented 1 month ago

Can you please share the Mbed TLS configuration (mbedtls_config.h)? In particular, MBEDTLS_GCM_ALT, MBEDTLS_AES_ALT and PSA-driver AES could change how GCM APIs behave.

ASeidelt commented 1 month ago

@davidhorstmann-arm Thanks for the heads up. Shall I leave this issue open until the PR is merged?

@gilles-peskine-arm This seems not to be necessary anymore with the comment from @davidhorstmann-arm? If it is a bug in mbedTLS documentation I'm not sure how my config would provide any additional insights (we are not using any ALT implementations)?

Thanks to both of you for your fast responses!

davidhorstmann-arm commented 1 month ago

Shall I leave this issue open until the PR is merged?

Yes, it'd be just as well to do that. In fact I'll link it so that it auto-closes when that PR is merged.