From 46ba7f3a92203852e04d79912e008fb0d4a9c5dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Mon, 28 Aug 2017 12:20:39 +0200 Subject: [PATCH] Avoid running useless code in tests With max_ops set to 0 or a very large value, we would always be doing an extra full operation for no testing value. --- tests/suites/test_suite_ecdsa.function | 24 +++++++++----- tests/suites/test_suite_ecp.function | 46 ++++++++++++-------------- tests/suites/test_suite_pk.function | 22 +++++++----- 3 files changed, 50 insertions(+), 42 deletions(-) diff --git a/tests/suites/test_suite_ecdsa.function b/tests/suites/test_suite_ecdsa.function index 9205627be..5db01a6cc 100644 --- a/tests/suites/test_suite_ecdsa.function +++ b/tests/suites/test_suite_ecdsa.function @@ -254,10 +254,14 @@ void ecdsa_read_restart( int id, char *k_str, char *h_str, char *s_str, TEST_ASSERT( ret == MBEDTLS_ERR_ECP_VERIFY_FAILED ); sig[sig_len - 1]--; - /* do we leak memory when aborting? */ - ret = mbedtls_ecdsa_read_signature_restartable( &ctx, - hash, hash_len, sig, sig_len, &rs_ctx ); - TEST_ASSERT( ret == 0 || ret == MBEDTLS_ERR_ECP_IN_PROGRESS ); + /* Do we leak memory when aborting an operation? + * This test only makes sense when we actually restart */ + if( min_restart > 0 ) + { + ret = mbedtls_ecdsa_read_signature_restartable( &ctx, + hash, hash_len, sig, sig_len, &rs_ctx ); + TEST_ASSERT( ret == MBEDTLS_ERR_ECP_IN_PROGRESS ); + } exit: mbedtls_ecdsa_free( &ctx ); @@ -311,10 +315,14 @@ void ecdsa_write_restart( int id, char *d_str, int md_alg, TEST_ASSERT( cnt_restart >= min_restart ); TEST_ASSERT( cnt_restart <= max_restart ); - /* do we leak memory when aborting? */ - ret = mbedtls_ecdsa_write_signature_restartable( &ctx, - md_alg, hash, hlen, sig, &slen, NULL, NULL, &rs_ctx ); - TEST_ASSERT( ret == 0 || ret == MBEDTLS_ERR_ECP_IN_PROGRESS ); + /* Do we leak memory when aborting an operation? + * This test only makes sense when we actually restart */ + if( min_restart > 0 ) + { + ret = mbedtls_ecdsa_write_signature_restartable( &ctx, + md_alg, hash, hlen, sig, &slen, NULL, NULL, &rs_ctx ); + TEST_ASSERT( ret == MBEDTLS_ERR_ECP_IN_PROGRESS ); + } exit: mbedtls_ecdsa_restart_free( &rs_ctx ); diff --git a/tests/suites/test_suite_ecp.function b/tests/suites/test_suite_ecp.function index 3ec7caf54..c60d0d349 100644 --- a/tests/suites/test_suite_ecp.function +++ b/tests/suites/test_suite_ecp.function @@ -103,13 +103,9 @@ void ecp_test_vect_restart( int id, cnt_restarts = 0; do { ret = mbedtls_ecp_mul_restartable( &grp, &R, &dA, &grp.G, NULL, NULL, &ctx ); - TEST_ASSERT( ret == 0 || ret == MBEDTLS_ERR_ECP_IN_PROGRESS ); - - if( ret == MBEDTLS_ERR_ECP_IN_PROGRESS ) - cnt_restarts++; - } - while( ret != 0 ); + } while( ret == MBEDTLS_ERR_ECP_IN_PROGRESS && ++cnt_restarts ); + TEST_ASSERT( ret == 0 ); TEST_ASSERT( mbedtls_mpi_cmp_mpi( &R.X, &xA ) == 0 ); TEST_ASSERT( mbedtls_mpi_cmp_mpi( &R.Y, &yA ) == 0 ); @@ -120,22 +116,22 @@ void ecp_test_vect_restart( int id, cnt_restarts = 0; do { ret = mbedtls_ecp_mul_restartable( &grp, &R, &dB, &R, NULL, NULL, &ctx ); - TEST_ASSERT( ret == 0 || ret == MBEDTLS_ERR_ECP_IN_PROGRESS ); - - if( ret == MBEDTLS_ERR_ECP_IN_PROGRESS ) - cnt_restarts++; - } - while( ret != 0 ); + } while( ret == MBEDTLS_ERR_ECP_IN_PROGRESS && ++cnt_restarts ); + TEST_ASSERT( ret == 0 ); TEST_ASSERT( mbedtls_mpi_cmp_mpi( &R.X, &xZ ) == 0 ); TEST_ASSERT( mbedtls_mpi_cmp_mpi( &R.Y, &yZ ) == 0 ); TEST_ASSERT( cnt_restarts >= min_restarts ); TEST_ASSERT( cnt_restarts <= max_restarts ); - /* Do we leak memory when not finishing an operation? */ - ret = mbedtls_ecp_mul_restartable( &grp, &R, &dB, &R, NULL, NULL, &ctx ); - TEST_ASSERT( ret == 0 || ret == MBEDTLS_ERR_ECP_IN_PROGRESS ); + /* Do we leak memory when aborting an operation? + * This test only makes sense when we actually restart */ + if( min_restarts > 0 ) + { + ret = mbedtls_ecp_mul_restartable( &grp, &R, &dB, &R, NULL, NULL, &ctx ); + TEST_ASSERT( ret == MBEDTLS_ERR_ECP_IN_PROGRESS ); + } exit: mbedtls_ecp_restart_free( &ctx ); @@ -188,23 +184,23 @@ void ecp_muladd_restart( int id, char *xR_str, char *yR_str, do { ret = mbedtls_ecp_muladd_restartable( &grp, &R, &u1, &grp.G, &u2, &Q, &ctx ); - TEST_ASSERT( ret == 0 || ret == MBEDTLS_ERR_ECP_IN_PROGRESS ); - - if( ret == MBEDTLS_ERR_ECP_IN_PROGRESS ) - cnt_restarts++; - } - while( ret != 0 ); + } while( ret == MBEDTLS_ERR_ECP_IN_PROGRESS && ++cnt_restarts ); + TEST_ASSERT( ret == 0 ); TEST_ASSERT( mbedtls_mpi_cmp_mpi( &R.X, &xR ) == 0 ); TEST_ASSERT( mbedtls_mpi_cmp_mpi( &R.Y, &yR ) == 0 ); TEST_ASSERT( cnt_restarts >= min_restarts ); TEST_ASSERT( cnt_restarts <= max_restarts ); - /* Do we leak memory when aborting? */ - ret = mbedtls_ecp_muladd_restartable( &grp, &R, - &u1, &grp.G, &u2, &Q, &ctx ); - TEST_ASSERT( ret == 0 || ret == MBEDTLS_ERR_ECP_IN_PROGRESS ); + /* Do we leak memory when aborting an operation? + * This test only makes sense when we actually restart */ + if( min_restarts > 0 ) + { + ret = mbedtls_ecp_muladd_restartable( &grp, &R, + &u1, &grp.G, &u2, &Q, &ctx ); + TEST_ASSERT( ret == MBEDTLS_ERR_ECP_IN_PROGRESS ); + } exit: mbedtls_ecp_restart_free( &ctx ); diff --git a/tests/suites/test_suite_pk.function b/tests/suites/test_suite_pk.function index 176b08f98..d7edb755c 100644 --- a/tests/suites/test_suite_pk.function +++ b/tests/suites/test_suite_pk.function @@ -347,16 +347,20 @@ void pk_sign_verify_restart( int pk_type, int grp_id, char *d_str, TEST_ASSERT( ret != 0 ); sig[0]--; - /* Do we leak memory when aborting? try verify then sign */ - ret = mbedtls_pk_verify_restartable( &pub, md_alg, - hash, hlen, sig, slen, &rs_ctx ); - TEST_ASSERT( ret == 0 || ret == MBEDTLS_ERR_ECP_IN_PROGRESS ); - mbedtls_pk_restart_free( &rs_ctx ); + /* Do we leak memory when aborting? try verify then sign + * This test only makes sense when we actually restart */ + if( min_restart > 0 ) + { + ret = mbedtls_pk_verify_restartable( &pub, md_alg, + hash, hlen, sig, slen, &rs_ctx ); + TEST_ASSERT( ret == MBEDTLS_ERR_ECP_IN_PROGRESS ); + mbedtls_pk_restart_free( &rs_ctx ); - slen = sizeof( sig ); - ret = mbedtls_pk_sign_restartable( &prv, md_alg, hash, hlen, - sig, &slen, NULL, NULL, &rs_ctx ); - TEST_ASSERT( ret == 0 || ret == MBEDTLS_ERR_ECP_IN_PROGRESS ); + slen = sizeof( sig ); + ret = mbedtls_pk_sign_restartable( &prv, md_alg, hash, hlen, + sig, &slen, NULL, NULL, &rs_ctx ); + TEST_ASSERT( ret == MBEDTLS_ERR_ECP_IN_PROGRESS ); + } exit: mbedtls_pk_restart_free( &rs_ctx );