Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Should ssl_ciphersuites.c honor MBEDTLS_AES_ONLY_128_BIT_KEY_LENGTH? #9802

Open
sherber opened this issue Nov 25, 2024 · 2 comments
Open

Should ssl_ciphersuites.c honor MBEDTLS_AES_ONLY_128_BIT_KEY_LENGTH? #9802

sherber opened this issue Nov 25, 2024 · 2 comments

Comments

@sherber
Copy link

sherber commented Nov 25, 2024

Summary

I recently ported mbed-TLS to a small embedded system with hardware acceleration for AES. Unfortunately, only AES-128 is supported in hardware.

I have therefore set MBEDTLS_AES_ALT and MBEDTLS_AES_ONLY_128_BIT_KEY_LENGTH for the hardware implementation. The mbedtls_aes_self_test passes for 128b key length. However, TLS1.2 was causing problems which I attribute to the ciphersuite definitions I found in ssl_ciphersuites.c. A whole lot of compile switches are used to build up the ciphersuite_definitions array.
MBEDTLS_AES_ONLY_128_BIT_KEY_LENGTH is not one of them, which I find odd. I guess my setup is a bit unusual, so MBEDTLS_AES_ONLY_128_BIT_KEY_LENGTH might have been overlooked in the past.

However, I know to little about a TLS and mbed-TLS's architecture to be certain. Does TLS1.2 have AES-256 as a prerequisite?

If you agree that MBEDTLS_AES_ONLY_128_BIT_KEY_LENGTH should be included, I can provide a PR for the issue.

System information

Mbed TLS version 3.6.0

@gilles-peskine-arm
Copy link
Contributor

MBEDTLS_AES_ONLY_128_BIT_KEY_LENGTH started out in a code size optimization project which mostly focused on crypto. It derived from an earlier project that encompassed TLS, but given the focus on code size, I expect that interested users just defined a single cipher suite anyway.

MBEDTLS_AES_ONLY_128_BIT_KEY_LENGTH would definitely make sense to have in TLS as well. It's a bug that you can make a sensible configuration where the library will offer cipher suites that it then can't handle.

A patch would definitely be welcome, but I'm afraid I can't promise when we would be able to review it (our review bandwidth is limited). We would want some testing as well, I guess by extending test_suite_ssl.

Is there a bug in TLS 1.3 as well?

@sherber
Copy link
Author

sherber commented Nov 26, 2024

I don't know for certain if the issue presents itself with TLS1.3 as I don't have a test setup to check it. Looking at the code of ssl_ciphersuites.c however I can see that "TLS1-3-AES-256-GCM-SHA384" is added to the ciphersuite definitions independent of MBEDTLS_AES_ONLY_128_BIT_KEY_LENGTH. I would therefore assume the bug is present in TLS1.3 too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Development

No branches or pull requests

2 participants