Deprecate Mbed Crypto #395
Merged
Conversation
This file contains hidden or 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
the
needs: review
|
I have confirmed that all currently available security fixes have been backported, checked spelling and I am happy with the transition period and how it is implemented and documented. I have only one minor question: In |
danh-arm
requested changes
Apr 23, 2020
| scripts/config.py set MBEDTLS_DEPRECATED_REMOVED | ||
| # Build with -O -Wextra to catch a maximum of issues. | ||
| make CC=clang CFLAGS='-O -Werror -Wall -Wextra' lib programs | ||
| make CC=clang CFLAGS='-O -Werror -Wall -Wextra -Wno-unused-function' tests |
There was a problem hiding this comment.
Sorry, the rationale for doing this is not clear to me. Is it to avoid CI issues? Can you elaborate in the commit message?
README.md
Outdated
| We gratefully accept bug reports and contributions from the community. Please see the [contributing guidelines](CONTRIBUTING.md) for details on how to do this. | ||
|
|
||
| ## Feedback welcome | ||
|
|
||
| Arm welcomes feedback on the design of the API. If you think something could be improved, please open an issue on our Github repository. Alternatively, if you prefer to provide your feedback privately, please email us at [`[email protected]`](mailto:[email protected]). All feedback received by email is treated confidentially. |
There was a problem hiding this comment.
As @yanesca said, we should not say "open an issue on our Github repository". We should refer to the psa-crypto mailing list. Actually, we have the same issue on development.
The reference [email protected] is fine though (that's still active)
Fix side channel in ECC code that allowed an adversary with access to precise enough timing and memory access information (typically an untrusted operating system attacking a secure enclave) to fully recover an ECDSA private key. Found and reported by Alejandro Cabrera Aldaya, Billy Brumley and Cesar Pereida Garcia. CVE-2020-10932 See the comments in the code for how an attack would go. For ECDSA, leaking a few bits of the scalar over several signatures translates to full private key recovery using a lattice attack. Signed-off-by: Manuel Pégourié-Gonnard <[email protected]>
State this prominently in README.md and provide a little migration guidance. Signed-off-by: Gilles Peskine <[email protected]>
At this point in time, Mbed Crypto has no known security issues. But in the future, it will not be updated if security issues are discovered. So add warnings about the migration to Mbed TLS, but don't break the build yet. Signed-off-by: Gilles Peskine <[email protected]>
gilles-peskine-arm
force-pushed
the
crypto-backport-2.22.0-and-deprecate
branch
from
4491000 to
42b7d35
Compare
danh-arm
previously approved these changes
Apr 23, 2020
Mbed Crypto is no longer being updated, so using it is deprecated. Don't test with MBEDTLS_DEPRECATED_REMOVED anymore since the library deliberately no longer builds when it's enabled. Signed-off-by: Gilles Peskine <[email protected]>
gilles-peskine-arm
force-pushed
the
crypto-backport-2.22.0-and-deprecate
branch
from
83a819e to
5694d9f
Compare
danh-arm
previously approved these changes
Apr 23, 2020
MBEDTLS_DEPRECATED_WARNING now always emits a #warning since this branch as a whole is deprecated. So disable it from most builds. Signed-off-by: Gilles Peskine <[email protected]>
Signed-off-by: Gilles Peskine <[email protected]>
gilles-peskine-arm
force-pushed
the
crypto-backport-2.22.0-and-deprecate
branch
from
5694d9f to
cd2efd6
Compare
|
CI is passing except for the known breakage in Mbed OS. |
danh-arm
approved these changes
Apr 27, 2020
danh-arm
added
ready for merge
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.
Mbed Crypto is no longer being updated, but this is not visible on https://github.com/ARMmbed/mbed-crypto. In this pull request, I update the readme to let people know to use Mbed TLS instead.
To ease the transition, I propose to have a short transition period during which we fix the security issues in Mbed Crypto, and building the library is still possible but shows a warning. After this transition period, we'll make a last patch to cause the library to fail to build.
To verify what security fixes needed to be backported:
I saw only one relevant commit.