From f2268d1c171be435e32a7f38d4f6b9b371f5cf41 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 23 Jun 2021 10:14:58 +0200 Subject: [PATCH] Reject low-order points on Curve25519 early MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We were already rejecting them at the end, due to the fact that with the usual (x, z) formulas they lead to the result (0, 0) so when we want to normalize at the end, trying to compute the modular inverse of z will give an error. If we wanted to support those points, we'd a special case in ecp_normalize_mxz(). But it's actually permitted by all sources (RFC 7748 say we MAY reject 0 as a result) and recommended by some to reject those points (either to ensure contributory behaviour, or to protect against timing attack when the underlying field arithmetic is not constant-time). Since our field arithmetic is indeed not constant-time, let's reject those points before they get mixed with sensitive data (in ecp_mul_mxz()), in order to avoid exploitable leaks caused by the special cases they would trigger. (See the "May the Fourth" paper https://eprint.iacr.org/2017/806.pdf) Signed-off-by: Manuel Pégourié-Gonnard --- library/ecp.c | 61 ++++++++++++++++++++++++++++++++ tests/suites/test_suite_ecp.data | 6 ++-- 2 files changed, 64 insertions(+), 3 deletions(-) diff --git a/library/ecp.c b/library/ecp.c index f74870496..5149abb49 100644 --- a/library/ecp.c +++ b/library/ecp.c @@ -2935,6 +2935,56 @@ int mbedtls_ecp_muladd( mbedtls_ecp_group *grp, mbedtls_ecp_point *R, #endif /* MBEDTLS_ECP_SHORT_WEIERSTRASS_ENABLED */ #if defined(MBEDTLS_ECP_MONTGOMERY_ENABLED) +#if defined(MBEDTLS_ECP_DP_CURVE25519_ENABLED) +/* + * Check that the input point is not one of the low-order points. + * This is recommended by the "May the Fourth" paper: + * https://eprint.iacr.org/2017/806.pdf + * Those points are never sent by an honest peer. + */ +static int ecp_check_pubkey_x25519( const mbedtls_mpi *X, const mbedtls_mpi *P ) +{ + int ret; + mbedtls_mpi XmP, bad; + + mbedtls_mpi_init( &XmP ); + mbedtls_mpi_init( &bad ); + + /* Reduce X mod P so that we only need to check values less than P. + * We know X < 2^256 so we can proceed by subtraction. */ + MBEDTLS_MPI_CHK( mbedtls_mpi_copy( &XmP, X ) ); + while( mbedtls_mpi_cmp_mpi( &XmP, P ) >= 0 ) + MBEDTLS_MPI_CHK( mbedtls_mpi_sub_mpi( &XmP, &XmP, P ) ); + + /* Check against the known bad values that are less than P in the + * following list: https://cr.yp.to/ecdh.html#validate */ + if( mbedtls_mpi_cmp_int( &XmP, 1 ) <= 0 ) /* takes care of 0 and 1 */ + return( MBEDTLS_ERR_ECP_INVALID_KEY ); + + MBEDTLS_MPI_CHK( mbedtls_mpi_read_string( &bad, 10, + "325606250916557431795983626356110631294008115727848805560023387167927233504" ) ); + if( mbedtls_mpi_cmp_mpi( &XmP, &bad ) == 0 ) + return( MBEDTLS_ERR_ECP_INVALID_KEY ); + + MBEDTLS_MPI_CHK( mbedtls_mpi_read_string( &bad, 10, + "39382357235489614581723060781553021112529911719440698176882885853963445705823" ) ); + if( mbedtls_mpi_cmp_mpi( &XmP, &bad ) == 0 ) + return( MBEDTLS_ERR_ECP_INVALID_KEY ); + + MBEDTLS_MPI_CHK( mbedtls_mpi_sub_int( &bad, P, 1 ) ); + if( mbedtls_mpi_cmp_mpi( &XmP, &bad ) == 0 ) + return( MBEDTLS_ERR_ECP_INVALID_KEY ); + + ret = 0; + +cleanup: + mbedtls_mpi_free( &XmP ); + mbedtls_mpi_free( &bad ); + + return( ret ); +} +#endif /* MBEDTLS_ECP_DP_CURVE25519_ENABLED */ + /* * Check validity of a public key for Montgomery curves with x-only schemes */ @@ -2946,6 +2996,17 @@ static int ecp_check_pubkey_mx( const mbedtls_ecp_group *grp, const mbedtls_ecp_ if( mbedtls_mpi_size( &pt->X ) > ( grp->nbits + 7 ) / 8 ) return( MBEDTLS_ERR_ECP_INVALID_KEY ); + /* Implicit in all standards (as they don't consider negative numbers): + * X must be non-negative. This is normally ensured by the way it's + * encoded for transmission, but let's be extra sure. */ + if( mbedtls_mpi_cmp_int( &pt->X, 0 ) < 0 ) + return( MBEDTLS_ERR_ECP_INVALID_KEY ); + +#if defined(MBEDTLS_ECP_DP_CURVE25519_ENABLED) + if( grp->id == MBEDTLS_ECP_DP_CURVE25519 ) + return( ecp_check_pubkey_x25519( &pt->X, &grp->P ) ); +#endif + return( 0 ); } #endif /* MBEDTLS_ECP_MONTGOMERY_ENABLED */ diff --git a/tests/suites/test_suite_ecp.data b/tests/suites/test_suite_ecp.data index 5f92ca459..85845c454 100644 --- a/tests/suites/test_suite_ecp.data +++ b/tests/suites/test_suite_ecp.data @@ -476,15 +476,15 @@ ecp_test_mul:MBEDTLS_ECP_DP_CURVE25519:"5AC99F33632E5A768DE7E81BF854C27C46E3FBF2 ECP point multiplication Curve25519 (element of order 2: origin) #3 depends_on:MBEDTLS_ECP_DP_CURVE25519_ENABLED -ecp_test_mul:MBEDTLS_ECP_DP_CURVE25519:"5AC99F33632E5A768DE7E81BF854C27C46E3FBF2ABBACD29EC4AFF517369C660":"00":"00":"01":"00":"01":"00":MBEDTLS_ERR_MPI_NOT_ACCEPTABLE +ecp_test_mul:MBEDTLS_ECP_DP_CURVE25519:"5AC99F33632E5A768DE7E81BF854C27C46E3FBF2ABBACD29EC4AFF517369C660":"00":"00":"01":"00":"01":"00":MBEDTLS_ERR_ECP_INVALID_KEY ECP point multiplication Curve25519 (element of order 4: 1) #4 depends_on:MBEDTLS_ECP_DP_CURVE25519_ENABLED -ecp_test_mul:MBEDTLS_ECP_DP_CURVE25519:"5AC99F33632E5A768DE7E81BF854C27C46E3FBF2ABBACD29EC4AFF517369C660":"01":"00":"01":"00":"01":"00":MBEDTLS_ERR_MPI_NOT_ACCEPTABLE +ecp_test_mul:MBEDTLS_ECP_DP_CURVE25519:"5AC99F33632E5A768DE7E81BF854C27C46E3FBF2ABBACD29EC4AFF517369C660":"01":"00":"01":"00":"01":"00":MBEDTLS_ERR_ECP_INVALID_KEY ECP point multiplication Curve25519 (element of order 8) #5 depends_on:MBEDTLS_ECP_DP_CURVE25519_ENABLED -ecp_test_mul:MBEDTLS_ECP_DP_CURVE25519:"5AC99F33632E5A768DE7E81BF854C27C46E3FBF2ABBACD29EC4AFF517369C660":"B8495F16056286FDB1329CEB8D09DA6AC49FF1FAE35616AEB8413B7C7AEBE0":"00":"01":"00":"01":"00":MBEDTLS_ERR_MPI_NOT_ACCEPTABLE +ecp_test_mul:MBEDTLS_ECP_DP_CURVE25519:"5AC99F33632E5A768DE7E81BF854C27C46E3FBF2ABBACD29EC4AFF517369C660":"B8495F16056286FDB1329CEB8D09DA6AC49FF1FAE35616AEB8413B7C7AEBE0":"00":"01":"00":"01":"00":MBEDTLS_ERR_ECP_INVALID_KEY ECP point multiplication rng fail secp256r1 depends_on:MBEDTLS_ECP_DP_SECP256R1_ENABLED