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

Problem with mutex used in CTR_DRBG and HMAC_DRBG #4017

Closed
MAntoniak opened this issue Jan 12, 2021 · 6 comments · Fixed by #4104
Closed

Problem with mutex used in CTR_DRBG and HMAC_DRBG #4017

MAntoniak opened this issue Jan 12, 2021 · 6 comments · Fixed by #4104
Labels

Comments

@MAntoniak
Copy link

Bug

OS
embedded system

mbed TLS build:
Version: 2.7.17

Hello everyone.

I am trying to upgrade the library from version 2.7.17 to 2.7.18.
In my application I use the mbedtls library together with curl.
After updating, my device stops connecting to servers properly after some time.

My application control the number of mutexes used by mbedtls. Logs show that there are no free mutexes to use and a new one could not be initialized.

This has to do with 'In CTR_DRBG and HMAC_DRBG, don't reset the reseed interval in seed (). Fixes # 2927. '

In the mbedtls_crt_drbg_free and mbedtls_hmac_drbg_free functions, a new mutex is initialized at the end of these functions. What if we don't want to use mbedtls_crt_drbg_context or mbedtls_hmac_drbg_context anymore?

In the library I found this function:
https://github.com/ARMmbed/mbedtls/blob/d0c7b79170dbfa42932523c8b4dd326559cf698e/library/ecp.c#L2241
which uses the local ecp_drbg_context. After executing the function, ecp_drbg_context is released but the mutex is initialized and its address is dropped.

Perhaps this is the real problem. I cannot define it.

@gilles-peskine-arm
Copy link
Contributor

Indeed #3393 is a regression on platforms where mbedtls_mutex_init allocates resources.

Please note that Mbed TLS 2.7 is approaching the end of its support period. Given that this bug is a regression on long-time support branches, I think we'll make one more 2.7 release to fix it, but that will be the last release in the 2.7 series. Everyone who is still using Mbed TLS 2.7 should upgrade to the 2.16 long-time support branch or to the latest release.

@gilles-peskine-arm
Copy link
Contributor

There's a proposed fix in #4090. Please note that it has not been reviewed yet, and we may yet revise how we resolve this issue (it's not fully clear to me how to fix this issue without breaking “unusual” usage of the API). If you want to try it out, you can apply the patches locally: use the branch https://github.com/gilles-peskine-arm/mbedtls/tree/test-mutex-usage-count-2.7, or the individual patches a989b1f for CTR_DRBG and dc3e470 for HMAC_DRBG. The 2.7 patches should work for 2.16 as well.

@giso-c
Copy link

giso-c commented Feb 3, 2021

I did some tests and when calling mbedtls_ssl_handshake
it will call ecp_mul_comb function and this will call ecp_drbg_init which create a mutex and after ecp_drbg_free which delete previous mutex and create a new one.
So at end of handshake I get 1 remaining mutex.
ssl_issue.txt

My server does:

  • init the TLS
  • accept incoming connection
  • call mbedtls_ssl_session_reset and after mbedtls_ssl_handshake
  • uses the secure connection
  • when connection lost go to accept

I attached a log of the handshake process, with extra log when mutex are created or deleted

@gilles-peskine-arm
Copy link
Contributor

(continued from #4071 (comment)) @giso-c There are two implementations of ecp_drbg_init (one with HMAC_DRBG and one with CTR_DRBG), but my patch (either https://github.com/gilles-peskine-arm/mbedtls/tree/test-mutex-usage-count-development or the earlier version https://github.com/gilles-peskine-arm/mbedtls/tree/fix-drbg-mutex-usage-2.25-untested should fix the leak in both implementations. Can you please confirm:

  • Your compile-time configuration (so as to know which implementation of ecp_drbg_init is called)?
  • What commit you tested?

@giso-c
Copy link

giso-c commented Feb 4, 2021

I used this version https://github.com/gilles-peskine-arm/mbedtls/tree/test-mutex-usage-count-development and it seems that the bug is fixed, I was also investigating another memory issue I was having with version 2.14 and this code seems to have fixed it.

@gilles-peskine-arm
Copy link
Contributor

The fix is now merged into the supported branches (https://github.com/ARMmbed/mbedtls/tree/mbedtls-2.7, https://github.com/ARMmbed/mbedtls/tree/mbedtls-2.16 and https://github.com/ARMmbed/mbedtls/tree/development). The critical parts (mutex resource leak through a DRBG context) is identical to the patches I shared earlier.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants