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

ECDSA signature verification is missing a modular operation #35

Closed
DualTachyon opened this issue Oct 21, 2013 · 3 comments
Closed

ECDSA signature verification is missing a modular operation #35

DualTachyon opened this issue Oct 21, 2013 · 3 comments

Comments

@DualTachyon
Copy link

Hi,

I think that a mpi_mod_mpi is missing from ecdsa_signature as shown below.
Without that line addition, ECDSA verification in DTCP-IP doesn't work.
I'm no ECP expert, so let me know if you disagree with this change.
For reference, I compared the values of each MPI at every line of code with my own reference ECDSA implementation, and that is the only place that differs in PolarSSL.

Thanks.

if( ecp_is_zero( &R ) )
{
    ret = POLARSSL_ERR_ECP_VERIFY_FAILED;
    goto cleanup;
}

// Make sure R.X is mod n.
MPI_CHK( mpi_mod_mpi( &R.X, &R.X, &grp->N ) );

/*
 * Step 6: check that xR == r
 */
if( mpi_cmp_mpi( &R.X, r ) != 0 )
@DualTachyon
Copy link
Author

I forgot to mention I'm using the 1.3.1 release of PolarSSL.

@mpg
Copy link
Contributor

mpg commented Oct 29, 2013

Thanks for your report! Indeed you're right, and reduction mod n should be performed. This just got fixed in the internal development branch and will be in the next minor release.

I'm rather surprised you ran into this issue, since heuristically it seems to me it should happen roughly with probability 2^(-l/2) where l is the bit length of "the curve" (that is, of P or N), so it should be very unlikely with the kind of curve size used in cryptographic applications.

@pjbakker
Copy link
Contributor

pjbakker commented Nov 4, 2013

Updated in private branch ready for 1.3.2 release

@pjbakker pjbakker closed this as completed Nov 4, 2013
gilles-peskine-arm added a commit to gilles-peskine-arm/mbedtls that referenced this issue Sep 5, 2017
Added missing dependencies on PEM parsing in PK test suite
hanno-becker pushed a commit to hanno-becker/mbedtls that referenced this issue Oct 12, 2020
Fixed incorrect length handling in ssl_calc_verify_tls_sha256() and ssl_calc_verify_tls_sha384()
iameli pushed a commit to livepeer/mbedtls that referenced this issue Dec 5, 2023
add -fPIC to CFLAGS by default, use pkg-config to get LDFLAGS and CFLAGS...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants