Backport 2.x: fix SHA384 guards in TLS #4503
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
gilles-peskine-arm
added
bug
needs-review
This was referenced May 12, 2021
ronald-cron-arm
previously approved these changes
May 19, 2021
gilles-peskine-arm
force-pushed
the
ciphersuite-sha384-guard-2.x
branch
from
e6cbd7e to
2a98b7e
Compare
Fix Mbed-TLS#4472 Signed-off-by: Gilles Peskine <[email protected]>
They depended on MBEDTLS_SHA512_C only. A check for !MBEDTLS_SHA512_NO_SHA384 was missing. Fix Mbed-TLS#4499. Signed-off-by: Gilles Peskine <[email protected]>
TLS code specific to SHA-384 was gated on MBEDTLS_SHA512_C. But SHA-384 also requires that MBEDTLS_SHA512_NO_SHA384 is not defined. This lead to dead code in TLS when MBEDTLS_SHA512_C and MBEDTLS_SHA512_NO_SHA384 were both defined (i.e. when SHA-512 was enabled but not SHA-384). Signed-off-by: Gilles Peskine <[email protected]>
Signed-off-by: Gilles Peskine <[email protected]>
Signed-off-by: Gilles Peskine <[email protected]>
gilles-peskine-arm
force-pushed
the
ciphersuite-sha384-guard-2.x
branch
from
2a98b7e to
c54010c
Compare
ronald-cron-arm
approved these changes
May 19, 2021
daverodgman
removed
the
needs-reviewer
gilles-peskine-arm
added
approved
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Even after the addition of
MBEDTLS_SHA512_NO_SHA384, the TLS code gated the availability of SHA384 ciphersuites onMBEDTLS_SHA512_Calone, and in one case due to a typo onMBEDTLS_SHA1_C. Fix #4472, #4499.For 3.0 this is fixed in #4304 (the issues were discovered while reviewing that PR).
I have not fixed a similar issue in
ssl_cookie.cbecause I'm not sure what the fix is and it remains partly open in 3.0. It is filed as #4501.There are no non-regression tests because I don't think it's easy to add them. I've filed the lack of tests as #4500. To test manually, build
development_2.xwithMBEDTLS_SHA512_NO_SHA384defined and observe thatprograms/ssl/ssl_client2 helplists SHA384 cipher suites but they do not work. With this patch, the ciphersuites are no longer offered.