From 4d4c98b1b959a2f1ac270806cfa36d849d286889 Mon Sep 17 00:00:00 2001 From: Minos Galanakis Date: Thu, 27 Oct 2022 15:58:02 +0100 Subject: [PATCH] bignum_mod: `mbedtls_mpi_mod_modulus_setup()` refactoring. This patch addresses more review comments, and fixes a circular depedency in the `mbedtls_mpi_mod_modulus_setup()`. Signed-off-by: Minos Galanakis --- library/bignum_mod.c | 28 +++++++++++++-------- tests/suites/test_suite_bignum_mod.function | 7 +----- 2 files changed, 18 insertions(+), 17 deletions(-) diff --git a/library/bignum_mod.c b/library/bignum_mod.c index 705b2dbae6..60c073ac4e 100644 --- a/library/bignum_mod.c +++ b/library/bignum_mod.c @@ -77,10 +77,13 @@ void mbedtls_mpi_mod_modulus_free( mbedtls_mpi_mod_modulus *m ) switch( m->int_rep ) { case MBEDTLS_MPI_MOD_REP_MONTGOMERY: - mbedtls_platform_zeroize( (mbedtls_mpi_uint *) m->rep.mont.rr, - m->limbs ); - mbedtls_free( (mbedtls_mpi_uint *)m->rep.mont.rr ); - m->rep.mont.rr = NULL; + if (m->rep.mont.rr != NULL) + { + mbedtls_platform_zeroize( (mbedtls_mpi_uint *) m->rep.mont.rr, + m->limbs ); + mbedtls_free( (mbedtls_mpi_uint *)m->rep.mont.rr ); + m->rep.mont.rr = NULL; + } m->rep.mont.mm = 0; break; case MBEDTLS_MPI_MOD_REP_OPT_RED: @@ -104,6 +107,7 @@ static int set_mont_const_square( const mbedtls_mpi_uint **X, int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; mbedtls_mpi N; mbedtls_mpi RR; + *X = NULL; mbedtls_mpi_init( &N ); mbedtls_mpi_init( &RR ); @@ -111,16 +115,18 @@ static int set_mont_const_square( const mbedtls_mpi_uint **X, if ( A == NULL || limbs == 0 || limbs >= ( MBEDTLS_MPI_MAX_LIMBS / 2 ) - 2 ) goto cleanup; - if ( mbedtls_mpi_grow( &N, limbs )) + if ( mbedtls_mpi_grow( &N, limbs ) ) goto cleanup; - memcpy( N.p, A, sizeof(mbedtls_mpi_uint) * limbs ); + memcpy( N.p, A, sizeof(mbedtls_mpi_uint) * limbs ); - mbedtls_mpi_core_get_mont_r2_unsafe(&RR, &N); + ret = mbedtls_mpi_core_get_mont_r2_unsafe(&RR, &N); - *X = RR.p; - RR.p = NULL; - ret = 0; + if ( ret == 0 ) + { + *X = RR.p; + RR.p = NULL; + } cleanup: mbedtls_mpi_free(&N); @@ -157,7 +163,7 @@ int mbedtls_mpi_mod_modulus_setup( mbedtls_mpi_mod_modulus *m, case MBEDTLS_MPI_MOD_REP_MONTGOMERY: m->int_rep = int_rep; m->rep.mont.mm = mbedtls_mpi_core_montmul_init( m->p ); - set_mont_const_square( &m->rep.mont.rr, m->p, m->limbs ); + ret = set_mont_const_square( &m->rep.mont.rr, m->p, m->limbs ); break; case MBEDTLS_MPI_MOD_REP_OPT_RED: m->int_rep = int_rep; diff --git a/tests/suites/test_suite_bignum_mod.function b/tests/suites/test_suite_bignum_mod.function index 2e0377a703..ad89bdf3e7 100644 --- a/tests/suites/test_suite_bignum_mod.function +++ b/tests/suites/test_suite_bignum_mod.function @@ -17,10 +17,9 @@ void mpi_mod_setup( int ext_rep, int int_rep, int iret ) #define MLIMBS 8 mbedtls_mpi_uint mp[MLIMBS]; mbedtls_mpi_mod_modulus m; - const size_t mp_size = sizeof(mbedtls_mpi_uint); int ret; - memset( mp, 0xFF, mp_size ); + memset( mp, 0xFF, sizeof(mp) ); mbedtls_mpi_mod_modulus_init( &m ); ret = mbedtls_mpi_mod_modulus_setup( &m, mp, MLIMBS, ext_rep, int_rep ); @@ -44,13 +43,9 @@ void mpi_mod_setup( int ext_rep, int int_rep, int iret ) /* Only test if the constants have been set-up */ if ( ret == 0 && int_rep == MBEDTLS_MPI_MOD_REP_MONTGOMERY ) { - /* Reuse the allocated buffer for a zeroization check */ - memset( mp, 0x00, mp_size ); - /* Verify the data and pointers allocated have been properly wiped */ TEST_ASSERT( m.rep.mont.rr == NULL ); TEST_ASSERT( m.rep.mont.mm == 0 ); - } exit: /* It should be safe to call an mbedtls free several times */