From bcaba030ec8fe5162f99eb1553c361a5b9482210 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 20 Apr 2022 10:34:22 +0200 Subject: [PATCH 1/2] Expand negative coverage of ECDSA verification MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Motivated by CVE-2022-21449, to which we're not vulnerable, but we didn't have a test for it. Now we do. Signed-off-by: Manuel Pégourié-Gonnard --- tests/suites/test_suite_ecdsa.function | 74 ++++++++++++++++++++++---- 1 file changed, 63 insertions(+), 11 deletions(-) diff --git a/tests/suites/test_suite_ecdsa.function b/tests/suites/test_suite_ecdsa.function index d47bb4ed8..3182b98bf 100644 --- a/tests/suites/test_suite_ecdsa.function +++ b/tests/suites/test_suite_ecdsa.function @@ -314,20 +314,72 @@ void ecdsa_prim_test_vectors( int id, char * d_str, char * xQ_str, if ( result == 0) { - TEST_ASSERT( mbedtls_mpi_cmp_mpi( &r, &r_check ) == 0 ); - TEST_ASSERT( mbedtls_mpi_cmp_mpi( &s, &s_check ) == 0 ); + /* save correct values; we'll generate incorrect ones below */ + TEST_EQUAL( mbedtls_mpi_cmp_mpi( &r, &r_check ), 0 ); + TEST_EQUAL( mbedtls_mpi_cmp_mpi( &s, &s_check ), 0 ); - TEST_ASSERT( mbedtls_ecdsa_verify( &grp, hash->x, hash->len, &Q, &r_check, &s_check ) == 0 ); + /* Valid signature */ + TEST_EQUAL( mbedtls_ecdsa_verify( &grp, hash->x, hash->len, + &Q, &r_check, &s_check ), 0 ); - TEST_ASSERT( mbedtls_mpi_sub_int( &r, &r, 1 ) == 0 ); - TEST_ASSERT( mbedtls_mpi_add_int( &s, &s, 1 ) == 0 ); + /* Invalid signature: wrong public key (G instead of Q) */ + TEST_EQUAL( mbedtls_ecdsa_verify( &grp, hash->x, hash->len, + &grp.G, &r_check, &s_check ), MBEDTLS_ERR_ECP_VERIFY_FAILED ); - TEST_ASSERT( mbedtls_ecdsa_verify( &grp, hash->x, hash->len, - &Q, &r, &s_check ) == MBEDTLS_ERR_ECP_VERIFY_FAILED ); - TEST_ASSERT( mbedtls_ecdsa_verify( &grp, hash->x, hash->len, - &Q, &r_check, &s ) == MBEDTLS_ERR_ECP_VERIFY_FAILED ); - TEST_ASSERT( mbedtls_ecdsa_verify( &grp, hash->x, hash->len, - &grp.G, &r_check, &s_check ) == MBEDTLS_ERR_ECP_VERIFY_FAILED ); + /* Invalid signatures: r or s or both one off */ + TEST_EQUAL( mbedtls_mpi_sub_int( &r, &r, 1 ), 0 ); + TEST_EQUAL( mbedtls_mpi_add_int( &s, &s, 1 ), 0 ); + + TEST_EQUAL( mbedtls_ecdsa_verify( &grp, hash->x, hash->len, &Q, + &r, &s_check ), MBEDTLS_ERR_ECP_VERIFY_FAILED ); + TEST_EQUAL( mbedtls_ecdsa_verify( &grp, hash->x, hash->len, &Q, + &r_check, &s ), MBEDTLS_ERR_ECP_VERIFY_FAILED ); + TEST_EQUAL( mbedtls_ecdsa_verify( &grp, hash->x, hash->len, &Q, + &r, &s ), MBEDTLS_ERR_ECP_VERIFY_FAILED ); + + /* Invalid signatures: r, s or both (CVE-2022-21449) are zero */ + TEST_EQUAL( mbedtls_mpi_lset( &r, 0 ), 0 ); + TEST_EQUAL( mbedtls_mpi_lset( &s, 0 ), 0 ); + + TEST_EQUAL( mbedtls_ecdsa_verify( &grp, hash->x, hash->len, &Q, + &r, &s_check ), MBEDTLS_ERR_ECP_VERIFY_FAILED ); + TEST_EQUAL( mbedtls_ecdsa_verify( &grp, hash->x, hash->len, &Q, + &r_check, &s ), MBEDTLS_ERR_ECP_VERIFY_FAILED ); + TEST_EQUAL( mbedtls_ecdsa_verify( &grp, hash->x, hash->len, &Q, + &r, &s ), MBEDTLS_ERR_ECP_VERIFY_FAILED ); + + /* Invalid signatures: r, s or both are negative */ + TEST_EQUAL( mbedtls_mpi_lset( &r, -1 ), 0 ); + TEST_EQUAL( mbedtls_mpi_lset( &s, -1 ), 0 ); + + TEST_EQUAL( mbedtls_ecdsa_verify( &grp, hash->x, hash->len, &Q, + &r, &s_check ), MBEDTLS_ERR_ECP_VERIFY_FAILED ); + TEST_EQUAL( mbedtls_ecdsa_verify( &grp, hash->x, hash->len, &Q, + &r_check, &s ), MBEDTLS_ERR_ECP_VERIFY_FAILED ); + TEST_EQUAL( mbedtls_ecdsa_verify( &grp, hash->x, hash->len, &Q, + &r, &s ), MBEDTLS_ERR_ECP_VERIFY_FAILED ); + + /* Invalid signatures: r, s or both are == N */ + TEST_EQUAL( mbedtls_mpi_copy( &r, &grp.N ), 0 ); + TEST_EQUAL( mbedtls_mpi_copy( &s, &grp.N ), 0 ); + + TEST_EQUAL( mbedtls_ecdsa_verify( &grp, hash->x, hash->len, &Q, + &r, &s_check ), MBEDTLS_ERR_ECP_VERIFY_FAILED ); + TEST_EQUAL( mbedtls_ecdsa_verify( &grp, hash->x, hash->len, &Q, + &r_check, &s ), MBEDTLS_ERR_ECP_VERIFY_FAILED ); + TEST_EQUAL( mbedtls_ecdsa_verify( &grp, hash->x, hash->len, &Q, + &r, &s ), MBEDTLS_ERR_ECP_VERIFY_FAILED ); + + /* Invalid signatures: r or s or both are > N */ + TEST_EQUAL( mbedtls_mpi_add_int( &r, &grp.N, 1 ), 0 ); + TEST_EQUAL( mbedtls_mpi_add_int( &s, &grp.N, 1 ), 0 ); + + TEST_EQUAL( mbedtls_ecdsa_verify( &grp, hash->x, hash->len, &Q, + &r, &s_check ), MBEDTLS_ERR_ECP_VERIFY_FAILED ); + TEST_EQUAL( mbedtls_ecdsa_verify( &grp, hash->x, hash->len, &Q, + &r_check, &s ), MBEDTLS_ERR_ECP_VERIFY_FAILED ); + TEST_EQUAL( mbedtls_ecdsa_verify( &grp, hash->x, hash->len, &Q, + &r, &s ), MBEDTLS_ERR_ECP_VERIFY_FAILED ); } exit: From 5aeb61ccb46d92734216453df681b5952dac0ba7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 21 Apr 2022 09:25:23 +0200 Subject: [PATCH 2/2] Improve readability and relevance of values MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- tests/suites/test_suite_ecdsa.function | 46 ++++++++++++-------------- 1 file changed, 22 insertions(+), 24 deletions(-) diff --git a/tests/suites/test_suite_ecdsa.function b/tests/suites/test_suite_ecdsa.function index 3182b98bf..6b45aaba2 100644 --- a/tests/suites/test_suite_ecdsa.function +++ b/tests/suites/test_suite_ecdsa.function @@ -279,13 +279,14 @@ void ecdsa_prim_test_vectors( int id, char * d_str, char * xQ_str, { mbedtls_ecp_group grp; mbedtls_ecp_point Q; - mbedtls_mpi d, r, s, r_check, s_check; + mbedtls_mpi d, r, s, r_check, s_check, zero; mbedtls_test_rnd_buf_info rnd_info; mbedtls_ecp_group_init( &grp ); mbedtls_ecp_point_init( &Q ); mbedtls_mpi_init( &d ); mbedtls_mpi_init( &r ); mbedtls_mpi_init( &s ); mbedtls_mpi_init( &r_check ); mbedtls_mpi_init( &s_check ); + mbedtls_mpi_init( &zero ); TEST_ASSERT( mbedtls_ecp_group_load( &grp, id ) == 0 ); TEST_ASSERT( mbedtls_ecp_point_read_string( &Q, 16, xQ_str, yQ_str ) == 0 ); @@ -314,7 +315,7 @@ void ecdsa_prim_test_vectors( int id, char * d_str, char * xQ_str, if ( result == 0) { - /* save correct values; we'll generate incorrect ones below */ + /* Check we generated the expected values */ TEST_EQUAL( mbedtls_mpi_cmp_mpi( &r, &r_check ), 0 ); TEST_EQUAL( mbedtls_mpi_cmp_mpi( &s, &s_check ), 0 ); @@ -327,8 +328,8 @@ void ecdsa_prim_test_vectors( int id, char * d_str, char * xQ_str, &grp.G, &r_check, &s_check ), MBEDTLS_ERR_ECP_VERIFY_FAILED ); /* Invalid signatures: r or s or both one off */ - TEST_EQUAL( mbedtls_mpi_sub_int( &r, &r, 1 ), 0 ); - TEST_EQUAL( mbedtls_mpi_add_int( &s, &s, 1 ), 0 ); + TEST_EQUAL( mbedtls_mpi_sub_int( &r, &r_check, 1 ), 0 ); + TEST_EQUAL( mbedtls_mpi_add_int( &s, &s_check, 1 ), 0 ); TEST_EQUAL( mbedtls_ecdsa_verify( &grp, hash->x, hash->len, &Q, &r, &s_check ), MBEDTLS_ERR_ECP_VERIFY_FAILED ); @@ -338,30 +339,26 @@ void ecdsa_prim_test_vectors( int id, char * d_str, char * xQ_str, &r, &s ), MBEDTLS_ERR_ECP_VERIFY_FAILED ); /* Invalid signatures: r, s or both (CVE-2022-21449) are zero */ - TEST_EQUAL( mbedtls_mpi_lset( &r, 0 ), 0 ); - TEST_EQUAL( mbedtls_mpi_lset( &s, 0 ), 0 ); + TEST_EQUAL( mbedtls_mpi_lset( &zero, 0 ), 0 ); TEST_EQUAL( mbedtls_ecdsa_verify( &grp, hash->x, hash->len, &Q, - &r, &s_check ), MBEDTLS_ERR_ECP_VERIFY_FAILED ); + &zero, &s_check ), MBEDTLS_ERR_ECP_VERIFY_FAILED ); TEST_EQUAL( mbedtls_ecdsa_verify( &grp, hash->x, hash->len, &Q, - &r_check, &s ), MBEDTLS_ERR_ECP_VERIFY_FAILED ); + &r_check, &zero ), MBEDTLS_ERR_ECP_VERIFY_FAILED ); TEST_EQUAL( mbedtls_ecdsa_verify( &grp, hash->x, hash->len, &Q, - &r, &s ), MBEDTLS_ERR_ECP_VERIFY_FAILED ); - - /* Invalid signatures: r, s or both are negative */ - TEST_EQUAL( mbedtls_mpi_lset( &r, -1 ), 0 ); - TEST_EQUAL( mbedtls_mpi_lset( &s, -1 ), 0 ); - - TEST_EQUAL( mbedtls_ecdsa_verify( &grp, hash->x, hash->len, &Q, - &r, &s_check ), MBEDTLS_ERR_ECP_VERIFY_FAILED ); - TEST_EQUAL( mbedtls_ecdsa_verify( &grp, hash->x, hash->len, &Q, - &r_check, &s ), MBEDTLS_ERR_ECP_VERIFY_FAILED ); - TEST_EQUAL( mbedtls_ecdsa_verify( &grp, hash->x, hash->len, &Q, - &r, &s ), MBEDTLS_ERR_ECP_VERIFY_FAILED ); + &zero, &zero ), MBEDTLS_ERR_ECP_VERIFY_FAILED ); /* Invalid signatures: r, s or both are == N */ - TEST_EQUAL( mbedtls_mpi_copy( &r, &grp.N ), 0 ); - TEST_EQUAL( mbedtls_mpi_copy( &s, &grp.N ), 0 ); + TEST_EQUAL( mbedtls_ecdsa_verify( &grp, hash->x, hash->len, &Q, + &grp.N, &s_check ), MBEDTLS_ERR_ECP_VERIFY_FAILED ); + TEST_EQUAL( mbedtls_ecdsa_verify( &grp, hash->x, hash->len, &Q, + &r_check, &grp.N ), MBEDTLS_ERR_ECP_VERIFY_FAILED ); + TEST_EQUAL( mbedtls_ecdsa_verify( &grp, hash->x, hash->len, &Q, + &grp.N, &grp.N ), MBEDTLS_ERR_ECP_VERIFY_FAILED ); + + /* Invalid signatures: r, s or both are negative */ + TEST_EQUAL( mbedtls_mpi_sub_mpi( &r, &r_check, &grp.N ), 0 ); + TEST_EQUAL( mbedtls_mpi_sub_mpi( &s, &s_check, &grp.N ), 0 ); TEST_EQUAL( mbedtls_ecdsa_verify( &grp, hash->x, hash->len, &Q, &r, &s_check ), MBEDTLS_ERR_ECP_VERIFY_FAILED ); @@ -371,8 +368,8 @@ void ecdsa_prim_test_vectors( int id, char * d_str, char * xQ_str, &r, &s ), MBEDTLS_ERR_ECP_VERIFY_FAILED ); /* Invalid signatures: r or s or both are > N */ - TEST_EQUAL( mbedtls_mpi_add_int( &r, &grp.N, 1 ), 0 ); - TEST_EQUAL( mbedtls_mpi_add_int( &s, &grp.N, 1 ), 0 ); + TEST_EQUAL( mbedtls_mpi_add_mpi( &r, &r_check, &grp.N ), 0 ); + TEST_EQUAL( mbedtls_mpi_add_mpi( &s, &s_check, &grp.N ), 0 ); TEST_EQUAL( mbedtls_ecdsa_verify( &grp, hash->x, hash->len, &Q, &r, &s_check ), MBEDTLS_ERR_ECP_VERIFY_FAILED ); @@ -387,6 +384,7 @@ exit: mbedtls_ecp_point_free( &Q ); mbedtls_mpi_free( &d ); mbedtls_mpi_free( &r ); mbedtls_mpi_free( &s ); mbedtls_mpi_free( &r_check ); mbedtls_mpi_free( &s_check ); + mbedtls_mpi_free( &zero ); } /* END_CASE */