Mbed TLS: Fix wrong MPI N in ECP Curve448 curve #15287
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
ccli8
force-pushed
the
nuvoton_fix_ecp-load-curve448
branch
from
7235ec8
to
82bdf07
Compare
ciarmcom
added
the
release-type: patch
|
@ccli8, thank you for your changes. |
ccli8
added a commit
to ccli8/mbed-os
that referenced
this pull request
May 23, 2022
1. Replace ecp.c full-module, and other ec modules dependent on ecp.c (ecdh.c/ecdsa.c/ecjpake.c) will improve followingly.
2. Recover from Crypto ECC H/W failure:
(1) Enable timed-out wait to escape from ECC H/W trap
(2) On ECC H/W timeout, stop this ECC H/W operation
(3) Fall back to S/W implementation on failure
3. Support Short Weierstrass curve
NOTE: ECC H/W will trap on m*P with SCAP enabled, esp m = 2 or close to (order - 1).
Cannot work around by fallback to S/W, because following operations are easily to fail with data error.
Disable SCAP temporarily.
4. Support Montgomery curve
Montgomery curve has the form: B y^2 = x^3 + A x^2 + x
(1) In S/W impl, A is used as (A + 2) / 4. Figure out its original value for engine.
https://github.com/ARMmbed/mbed-os/blob/2eb06e76208588afc6cb7580a8dd64c5429a10ce/connectivity/mbedtls/include/mbedtls/ecp.h#L219-L220
(2) In S/W impl, B is unused. Actually, B is 1 for Curve25519/Curve448 and needs to configure to engine.
https://github.com/ARMmbed/mbed-os/blob/2eb06e76208588afc6cb7580a8dd64c5429a10ce/connectivity/mbedtls/include/mbedtls/ecp.h#L221-L222
(3) In S/W impl, y-coord is absent, but engine needs it. Deduce it from x-coord following:
https://tools.ietf.org/id/draft-jivsov-ecc-compact-05.html
https://www.rieselprime.de/ziki/Modular_square_root
NOTE: Fix wrong sign flag grp->N.s in ecp_curves_alt.c/ecp_use_curve448()
grp->N is in uninitialized state due to caller's having invoked mbedtls_ecp_group_free(grp) before.
In uninitialized state, grp->N.s is not initialized to 1 to indicate positive.
This can fix by re-initializing through mbedtls_mpi_lset(&grp->N, 0).
Raise one PR for this:
ARMmbed#15287
0xc0170
reviewed
May 23, 2022
| * re-initializing through mbedtls_mpi_lset(&grp->N, 0), following | ||
| * most other code. | ||
| */ | ||
| MBEDTLS_MPI_CHK( mbedtls_mpi_lset( &grp->N, 0 ) ); |
There was a problem hiding this comment.
is this already fixed in the upstream or should it be ?
There was a problem hiding this comment.
The upstream hasn't fixed it.
There was a problem hiding this comment.
Can you send pull request there please? If tls is updated, this would be overwritten.
There was a problem hiding this comment.
Mbed-TLS/mbedtls#5811 is addressing the issue. This PR is updated to follow its solution.
In loading Curve448, MPI N is in uninitialized state and its sign flag N.s isn't initialized to 1. This is fixed by following: Mbed-TLS/mbedtls#5811
ccli8
force-pushed
the
nuvoton_fix_ecp-load-curve448
branch
from
82bdf07
to
9345c8a
Compare
0xc0170
approved these changes
May 30, 2022
|
CI started |
Jenkins CI Test : ✔️ SUCCESSBuild Number: 1 | 🔒 Jenkins CI Job | 🌐 Logs & ArtifactsCLICK for Detailed Summary
|
mbedmain
added
release-version: 6.16.0
Release-pending
and removed
release-type: patch
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.
Summary of changes
In loading ECP Curve448, MPI N is in uninitialized state due to caller having invoked
mbedtls_ecp_group_free(grp)before and its sign flag N.s isn't initialized to 1 to indicate positive. Following most other code, this can be fixed by invokingmbedtls_mpi_lset()on it.Pull request type
Test results