From b739a712d1caa5e27fc49b45cf7d71b8636e4053 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 19 Apr 2017 10:11:56 +0200 Subject: [PATCH] Start moving to new design/API Following discussion in the team, it was deemed preferable for the restart context to be explicitly managed by the caller. This commits in the first in a series moving in that directly: it starts by only changing the public API, while still internally using the old design. Future commits in that series will change to the new design internally. The test function was simplified as it no longer makes sense to test for some memory management errors since that responsibility shifted to the caller. --- include/mbedtls/ecp.h | 54 +++++++++++++++++++++++++--- library/ecp.c | 35 ++++++++++++++++++ tests/suites/test_suite_ecp.function | 31 +++++----------- 3 files changed, 93 insertions(+), 27 deletions(-) diff --git a/include/mbedtls/ecp.h b/include/mbedtls/ecp.h index c5664a84b..86d7c5906 100644 --- a/include/mbedtls/ecp.h +++ b/include/mbedtls/ecp.h @@ -164,7 +164,7 @@ typedef struct mbedtls_ecp_point *T; /*!< pre-computed points for ecp_mul_comb() */ size_t T_size; /*!< number for pre-computed points */ #if defined(MBEDTLS_ECP_EARLY_RETURN) - mbedtls_ecp_restart_mul_ctx *rsm; /*!< restart context for ecp_mul() */ + mbedtls_ecp_restart_mul_ctx *rsm; /*!< temporary */ #endif } mbedtls_ecp_group; @@ -184,6 +184,16 @@ typedef struct } mbedtls_ecp_keypair; +#if defined(MBEDTLS_ECP_EARLY_RETURN) +/** + * \brief General context for resuming ECC operations + */ +typedef struct +{ + mbedtls_ecp_restart_mul_ctx *rsm; /*!< restart context for ecp_mul() */ +} mbedtls_ecp_restart_ctx; +#endif /* MBEDTLS_ECP_EARLY_RETURN */ + /** * \name SECTION: Module settings * @@ -377,6 +387,18 @@ void mbedtls_ecp_group_free( mbedtls_ecp_group *grp ); */ void mbedtls_ecp_keypair_free( mbedtls_ecp_keypair *key ); +#if defined(MBEDTLS_ECP_EARLY_RETURN) +/** + * \brief Initialize a restart context + */ +void mbedtls_ecp_restart_init( mbedtls_ecp_restart_ctx *ctx ); + +/** + * \brief Free the components of a restart context + */ +void mbedtls_ecp_restart_free( mbedtls_ecp_restart_ctx *ctx ); +#endif /* MBEDTLS_ECP_EARLY_RETURN */ + /** * \brief Copy the contents of point Q into P * @@ -588,16 +610,40 @@ int mbedtls_ecp_tls_write_group( const mbedtls_ecp_group *grp, size_t *olen, * \return 0 if successful, * MBEDTLS_ERR_ECP_INVALID_KEY if m is not a valid privkey * or P is not a valid pubkey, - * MBEDTLS_ERR_MPI_ALLOC_FAILED if memory allocation failed, + * MBEDTLS_ERR_MPI_ALLOC_FAILED if memory allocation failed. + */ +int mbedtls_ecp_mul( mbedtls_ecp_group *grp, mbedtls_ecp_point *R, + const mbedtls_mpi *m, const mbedtls_ecp_point *P, + int (*f_rng)(void *, unsigned char *, size_t), void *p_rng ); + +#if defined(MBEDTLS_ECP_EARLY_RETURN) +/** + * \brief Restartable version of \c mbedtls_ecp_mul() + * + * \note Performs the same job as \c mbedtls_ecp_mul(), but can + * return early and restart according to the limit set with + * \c mbedtls_ecp_set_max_ops() to reduce blocking. + * + * \param grp ECP group + * \param R Destination point + * \param m Integer by which to multiply + * \param P Point to multiply + * \param f_rng RNG function (see notes) + * \param p_rng RNG parameter + * \param rs_ctx Restart context - must be non-NULL to enable early-return + * + * \return See \c mbedtls_ecp_mul(), or * MBEDTLS_ERR_ECP_IN_PROGRESS if maximum number of * operations was reached (see \c mbedtls_ecp_set_max_ops()), * indicating the function should be called again with the * exact same arguments. * */ -int mbedtls_ecp_mul( mbedtls_ecp_group *grp, mbedtls_ecp_point *R, +int mbedtls_ecp_mul_restartable( mbedtls_ecp_group *grp, mbedtls_ecp_point *R, const mbedtls_mpi *m, const mbedtls_ecp_point *P, - int (*f_rng)(void *, unsigned char *, size_t), void *p_rng ); + int (*f_rng)(void *, unsigned char *, size_t), void *p_rng, + mbedtls_ecp_restart_ctx *rs_ctx ); +#endif /* MBEDTLS_ECP_EARLY_RETURN */ /** * \brief Multiplication and addition of two points by integers: diff --git a/library/ecp.c b/library/ecp.c index 19d6af08c..b2c2f53c2 100644 --- a/library/ecp.c +++ b/library/ecp.c @@ -152,6 +152,27 @@ static void ecp_restart_mul_free( mbedtls_ecp_restart_mul_ctx *ctx ) memset( ctx, 0, sizeof( mbedtls_ecp_restart_mul_ctx ) ); } +/* + * Initialize a restart context + */ +void mbedtls_ecp_restart_init( mbedtls_ecp_restart_ctx *ctx ) +{ + memset( ctx, 0, sizeof( *ctx ) ); +} + +/* + * Free the components of a restart context + */ +void mbedtls_ecp_restart_free( mbedtls_ecp_restart_ctx *ctx ) +{ + if( ctx == NULL ) + return; + + ecp_restart_mul_free( ctx->rsm ); + mbedtls_free( ctx->rsm ); + ctx->rsm = NULL; +} + /* * Operation counts */ @@ -2111,6 +2132,20 @@ cleanup: return( ret ); } +#if defined(MBEDTLS_ECP_EARLY_RETURN) +/* + * Restartable multiplication R = m * P + */ +int mbedtls_ecp_mul_restartable( mbedtls_ecp_group *grp, mbedtls_ecp_point *R, + const mbedtls_mpi *m, const mbedtls_ecp_point *P, + int (*f_rng)(void *, unsigned char *, size_t), void *p_rng, + mbedtls_ecp_restart_ctx *rs_ctx ) +{ + (void) rs_ctx; /* cheating for now */ + return( mbedtls_ecp_mul( grp, R, m, P, f_rng, p_rng ) ); +} +#endif + #if defined(ECP_SHORTWEIERSTRASS) /* * Check that an affine point is valid as a public key, diff --git a/tests/suites/test_suite_ecp.function b/tests/suites/test_suite_ecp.function index 3dfef1877..195146c54 100644 --- a/tests/suites/test_suite_ecp.function +++ b/tests/suites/test_suite_ecp.function @@ -75,12 +75,14 @@ void ecp_test_vect_restart( int id, * With MBEDTLS_ECP_WINDOW_SIZE set to 2 (minimum): * - Random point mult: ~3850M */ + mbedtls_ecp_restart_ctx ctx; mbedtls_ecp_group grp; mbedtls_ecp_point R; mbedtls_mpi dA, xA, yA, dB, xZ, yZ; int cnt_restarts; int ret; + mbedtls_ecp_restart_init( &ctx ); mbedtls_ecp_group_init( &grp ); mbedtls_ecp_point_init( &R ); mbedtls_mpi_init( &dA ); mbedtls_mpi_init( &xA ); mbedtls_mpi_init( &yA ); mbedtls_mpi_init( &dB ); mbedtls_mpi_init( &xZ ); mbedtls_mpi_init( &yZ ); @@ -100,7 +102,7 @@ void ecp_test_vect_restart( int id, /* Base point case */ cnt_restarts = 0; do { - ret = mbedtls_ecp_mul( &grp, &R, &dA, &grp.G, NULL, NULL ); + 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 ) @@ -114,24 +116,13 @@ void ecp_test_vect_restart( int id, TEST_ASSERT( cnt_restarts >= min_restarts ); TEST_ASSERT( cnt_restarts <= max_restarts ); - /* Do we leak memory when doing it twice in a row? */ - do { - ret = mbedtls_ecp_mul( &grp, &R, &dA, &grp.G, NULL, NULL ); - TEST_ASSERT( ret == 0 || ret == MBEDTLS_ERR_ECP_IN_PROGRESS ); - } - while( ret != 0 ); - - /* Ok, now start an operation with some arguments, and drop it. - * We'll see if the result of the next operation, with different args, - * are correct regardless (do we discard old context on new args?). - * This also tests that we don't write to R prematurely */ - ret = mbedtls_ecp_mul( &grp, &R, &dA, &grp.G, NULL, NULL ); - TEST_ASSERT( ret == 0 || ret == MBEDTLS_ERR_ECP_IN_PROGRESS ); + /* Prepare context for new operation */ + mbedtls_ecp_restart_free( &ctx ); /* Non-base point case */ cnt_restarts = 0; do { - ret = mbedtls_ecp_mul( &grp, &R, &dB, &R, NULL, NULL ); + 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 ) @@ -145,18 +136,12 @@ void ecp_test_vect_restart( int id, TEST_ASSERT( cnt_restarts >= min_restarts ); TEST_ASSERT( cnt_restarts <= max_restarts ); - /* Do we leak memory when doing it twice in a row? */ - do { - ret = mbedtls_ecp_mul( &grp, &R, &dB, &R, NULL, NULL ); - TEST_ASSERT( ret == 0 || ret == MBEDTLS_ERR_ECP_IN_PROGRESS ); - } - while( ret != 0 ); - /* Do we leak memory when not finishing an operation? */ - ret = mbedtls_ecp_mul( &grp, &R, &dB, &R, NULL, NULL ); + ret = mbedtls_ecp_mul_restartable( &grp, &R, &dB, &R, NULL, NULL, &ctx ); TEST_ASSERT( ret == 0 || ret == MBEDTLS_ERR_ECP_IN_PROGRESS ); exit: + mbedtls_ecp_restart_free( &ctx ); mbedtls_ecp_group_free( &grp ); mbedtls_ecp_point_free( &R ); mbedtls_mpi_free( &dA ); mbedtls_mpi_free( &xA ); mbedtls_mpi_free( &yA ); mbedtls_mpi_free( &dB ); mbedtls_mpi_free( &xZ ); mbedtls_mpi_free( &yZ );