search

cisco / libsrtp

Library for SRTP (Secure Realtime Transport Protocol)
Other
1.19k stars 470 forks source link

Make mbedtls hmac less restrictive in line with RFC and library capabilities #636

Closed olishenton closed 1 year ago

olishenton commented 1 year ago

Libsrtp is unnecessarily restrictive with what keys it allows for HMAC with mbedtls . The HMAC RFC permits for keys here to have length between the SHA1_DIGEST_SIZE 20 and the block size of 64.

Unlike libsrtp's internal implementation, mbedtls has no limitations on the key that can be used, as seen in its hmac functions. As far as I can see this is also true historically in mbedtls. The same is true of openssl, for which libsrtp doesn't enforce the key length check. It seems sensible that mbedtls should get the same behaviour.

pabuhler commented 1 year ago

macos CI build fails are fixed in main if you want to merge / rebase.

pabuhler commented 1 year ago

@olishenton I see that the nss implementation also does not have this check so I am ok with removing it. How was this discovered, did it cause some issues ?

olishenton commented 1 year ago

Rebased, thanks!

@olishenton I see that the nss implementation also does not have this check so I am ok with removing it. How was this discovered, did it cause some issues ?

We changed from using MbedTLS with libsrtp to OpenSSL in a different build environment, so the difference in behaviour caused unexpected issues for our implementation. I could see libsrtp's hmac.c not supporting key lengths > 20 causing similar API compatibility problems for others, but it is not a cryptographic problem for HMAC that consumers of libsrtp couldn't deal with as I understand it.