mbedtls: improve debug output and fix AES-ICM issues

[seyednasermoravej]

• aes_gcm: Add debug print when PSA status fails.
• aes_icm: Destroy previous key before importing new key.
• aes_icm: Add missing destination buffer argument to encrypt function.

After testing the new libSRTP on a microcontroller with a limited resources (RAM = 256KB), I faced two issues:

1. We need to check if the key is available, first we need to destroy it, after that we can address its pointer to the NULL pointer.
2. When performing in-place encryption/decryption (input buffer == output buffer), the input length must be divisible by the cipher block size. Please check this:
   https://github.com/Mbed-TLS/TF-PSA-Crypto/blob/76920edddcad00ac41b248e12d937b845df7bedb/drivers/builtin/src/cipher.c#L641

[seyednasermoravej]

I didn't want to touch the cipher.c file.
But if the maximum size of buffer clear, uint8_t buffer[MAX_SIZE] = { 0 }; is much better for the embedded devices.

[pabuhler]

I am not sure I understand why you would want to malloc, or why you need a new output buffer.
This cant be good for performance, but in the end it is up to the uses of this mbedtls implementation to decided.
If this is due to buffer size when doing in-place, than maybe only do it if it is in-place. Or return an error if in place and not the correct size.
At the very least ensure there is some test that prevents someone from changing this in the future.

[seyednasermoravej]

Let me outline the problem and propose potential solutions. Following migration to mbedTLS v4.0, libSRTP ceased functioning on my targets. Debugging revealed the root cause within the cipher.c file. Specifically, the self-test loop (line 435) combined with a randomly generated plaintext length (line 443) triggered an error.

The relevant code path is as follows: srtp_cipher_encrypt -> psa_cipher_update -> psa_driver_wrapper_cipher_update -> mbedtls_psa_cipher_update -> mbedtls_to_psa_error(mbedtls_cipher_update). Within mbedtls_cipher_update, the block size (16 in this test case) is retrieved. An if condition then evaluates: if (input == output && (ctx->unprocessed_len != 0 || ilen % block_size)) { return MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA; }. During the self-test, input == output is always true due to in-place encryption/decryption. However, because the plaintext length (ilen) is randomly generated, it’s possible that ilen % block_size isn't zero, causing the function to return MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA.

This issue has not been observed in the following environments:

1. Ubuntu 22.04 & 24.04 with mbedtls v4.0 (using PSA APIs)
2. Ubuntu 24.04 with mbedtls v2.x (without PSA APIs)
3. Zephyr RTOS with mbedtls v3.x (without PSA APIs)

Here are several potential solutions for addressing this self-test failure:

1. Modify cipher.c to zero-pad the plaintext length, ensuring ilen % block_size == 0.
2. Modify cipher.c to utilize a dedicated buffer for the destination data, preventing the (input == output) == true condition from being evaluated.
3. Within aes_icm_mbedtls.c, implement a buffer for the destination using malloc.
4. Within aes_icm_mbedtls.c, utilize a local variable to manage the destination buffer, requiring an estimate of the maximum required buffer size. (maybe adding an if condition if the length is not dividable to block size, use a buffer otherwise use the same buffer for the source and destination)
   Within aes_icm_mbedtls.c, add a check for plaintext length divisibility by the block size; if not divisible, return an error – however, this would necessitate modifying the self-test procedure as well.

[pabuhler]

hmm, I do not think we want to expose the block size requirement outside of the mbedtls files for now.
I think I would go with combination of option 3 & 4. If in place and not % block_size then allocate a buffer, but store the buffer in the ctx and increase it's size if need. That way the number of allocs will go to zero and the memcpy would only be used when needed.

[seyednasermoravej]

I was fixing it but I think maybe its the MbedTLS bug. So, I opened an issue to see what's their respond.
Mbed-TLS/TF-PSA-Crypto#711

[pabuhler]

I am not sure I understand why you would want to malloc, or why you need a new output buffer.
This cant be good for performance, but in the end it is up to the uses of this mbedtls implementation to decided.
If this is due to buffer size when doing in-place, than maybe only do it if it is in-place. Or return an error if in place and not the correct size.
At the very least ensure there is some test that prevents someone from changing this in the future.