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

Remove support for the Truncated HMAC extension #4341

Closed
mpg opened this issue Apr 13, 2021 · 4 comments · Fixed by #4664
Closed

Remove support for the Truncated HMAC extension #4341

mpg opened this issue Apr 13, 2021 · 4 comments · Fixed by #4664
Assignees
Labels
size-s Estimated task size: small (~2d)

Comments

@mpg
Copy link
Contributor

mpg commented Apr 13, 2021

Context

In 2003, RFC 3546 introduce a new TLS extension called Truncated HMAC, with the purpose of saving bandwidth in constrained environments by reducind the length of the authentication tag of each message. Nowadays, the extension is defined in RFC 6066 which replace the original RFC, with the same definition.

Support for this extension was added to Mbed TLS (then PolarSSL) in version 1.3.0 back in 2013.

Our original implementation had a bug that (aside from reducing the security) prevented it from interoperating properly with any implementation that didn't have the same bug. This bug was found and fixed five years later in Mbed TLS 2.8.0, suggesting there had been no use of this extension in connections where was side was Mbed TLS and the other side another implementation in the meantime.

In 2011, a paper found some security concerns with the use of truncated HMAC in TLS, but these only concern the case where random-length padding is applied for length-hiding purposes. Mbed TLS does not support such a thing, but other implementations (for example, GnuTLS) do. So this attack doesn't directly apply to Mbed TLS but only to connections using another implementation as well (but then, see the previous paragraph).

In 2012, RFC 6655 introduced CCM-8 ciphersuites ((D)TLS 1.2 only, not available in earlier versions) which address the same concern as truncated HMAC but in a more efficient way (8 bytes instead of 10, and no padding) and using a more modern construct (AEAD).

In 2015, RFC 7525 (current best practice) issued general recommendations about using TLS in a secure way, including that only AEAD-based ciphersuites should be used, and noting that this makes the Truncated HMAC extension irrelevant, which together with the above paper lead to the conclusion that his extension must not be used. (Later RFC 8446 (TLS 1.3, standards track) repeated that conclusion.)

Proposal

Entirely remove support for the Truncated HMAC extension in Mbed TLS 3.0.

Rationale

Nowadays, people who care about minimising overhead are better served by any of the CCM-8 ciphersuites than by using a CBC ciphersuite with Truncated HMAC, both from bandwidth and security / best practices perspectives.

From a maintenance perspective, we shouldn't keep features in the code base if they don't actually serve our users.

Work to do

Minimal (needs to be done before 3.0)

  • Remove the MBEDTLS_SSL_TRUNCATED_HMAC option from config.h and add a check that it's unset in check_config.h
  • Make minimal change to make the CI pass (for example, remove tests that were specific to it in ssl-opt.sh)

Complete (could be done in 3.0 or later)

  • Remove the code related to this extension from the library
  • Remove related options from example programs (ssl_*2.c)
  • Remove tests that had a declared dependency on it
  • In the end git grep MBEDTLS_SSL_TRUNCATED_HMAC should find no match except in the ChangeLog
  • Search for issues related to this feature on github, and close them unless they apply to the LTS branches.

Mailing-list discussion

https://lists.trustedfirmware.org/pipermail/mbed-tls/2021-April/000330.html

@mpg mpg added mbedtls-3 needs-design-approval size-s Estimated task size: small (~2d) labels Apr 14, 2021
@mpg
Copy link
Contributor Author

mpg commented May 4, 2021

There has been no objection on the list, so I'm marking this as approved.

@tom-daubney-arm
Copy link
Contributor

@mpg - I was thinking of linking two PRs for this one. The first to cover the "minimal" work and the second to cover the "complete" work. Would that make sense to you? Or is it better if I just do the "min" and then pick up another 3.0 mandatory task instead?

@mpg
Copy link
Contributor Author

mpg commented Jun 3, 2021

I think it's better for now to just do the minimal 3.0 part move to another 3.0 task instead of doing the "complete" part. Generally speaking the priority order is (1) 3.0 mandatory (2) 3.0 optional (3) 3.x, and the "complete" part in the description is 3.x, so the lowest right now.

However, I think the person doing a task is in the best position to make decisions about scope adjustments. So, if you find the task easier to execute by including some of the points that are listed as "could be done later", or you feel like doing them now is going to be much more efficient than leaving them for later, IMO that's your call, feel free to do what you think is best.

@tom-daubney-arm
Copy link
Contributor

Cool thanks for the info. I will see how I get on then.

@mpg mpg closed this as completed in #4664 Jun 22, 2021
@bensze01 bensze01 modified the milestone: Mbed TLS 4.0 Jul 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size-s Estimated task size: small (~2d)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants