From 8493f80e65781d1de73bd40c5aa463b8ed0a99a4 Mon Sep 17 00:00:00 2001 From: Ron Eldor Date: Thu, 1 Nov 2018 11:32:15 +0200 Subject: [PATCH 1/9] conditionaly compile ECDH and ECDSA alt functions Return the condition compilation flags surrounding `mbedtls_ecdh_compute_shared()`, `mbedtls_ecdh_gen_public()`, `mbedtls_ecdsa_sign()` and `mbedtls_ecdsa_verify()` that were accidentally removed in a previous merge. Resolves #2163 --- library/ecdh.c | 23 ++++++++++++++++++++++- library/ecdsa.c | 25 ++++++++++++++++++++----- 2 files changed, 42 insertions(+), 6 deletions(-) diff --git a/library/ecdh.c b/library/ecdh.c index 80e9676419..f05e2c06af 100644 --- a/library/ecdh.c +++ b/library/ecdh.c @@ -110,7 +110,6 @@ cleanup: return( ret ); } -#endif /* MBEDTLS_ECDH_COMPUTE_SHARED_ALT */ /* * Compute shared secret (SEC1 3.3.1) @@ -123,6 +122,7 @@ int mbedtls_ecdh_compute_shared( mbedtls_ecp_group *grp, mbedtls_mpi *z, return( ecdh_compute_shared_restartable( grp, z, Q, d, f_rng, p_rng, NULL ) ); } +#endif /* MBEDTLS_ECDH_COMPUTE_SHARED_ALT */ /* * Initialize context @@ -201,9 +201,16 @@ int mbedtls_ecdh_make_params( mbedtls_ecdh_context *ctx, size_t *olen, rs_ctx = &ctx->rs; #endif + +#if defined(MBEDTLS_ECDH_GEN_PUBLIC_ALT) + if( ( ret = mbedtls_ecdh_gen_public( &ctx->grp, &ctx->d, &ctx->Q, + f_rng, p_rng ) ) != 0 ) + return( ret ); +#else if( ( ret = ecdh_gen_public_restartable( &ctx->grp, &ctx->d, &ctx->Q, f_rng, p_rng, rs_ctx ) ) != 0 ) return( ret ); +#endif if( ( ret = mbedtls_ecp_tls_write_group( &ctx->grp, &grp_len, buf, blen ) ) != 0 ) @@ -287,9 +294,15 @@ int mbedtls_ecdh_make_public( mbedtls_ecdh_context *ctx, size_t *olen, rs_ctx = &ctx->rs; #endif +#if defined(MBEDTLS_ECDH_GEN_PUBLIC_ALT) + if( ( ret = mbedtls_ecdh_gen_public( &ctx->grp, &ctx->d, &ctx->Q, + f_rng, p_rng ) ) != 0 ) + return( ret ); +#else if( ( ret = ecdh_gen_public_restartable( &ctx->grp, &ctx->d, &ctx->Q, f_rng, p_rng, rs_ctx ) ) != 0 ) return( ret ); +#endif return mbedtls_ecp_tls_write_point( &ctx->grp, &ctx->Q, ctx->point_format, olen, buf, blen ); @@ -335,11 +348,19 @@ int mbedtls_ecdh_calc_secret( mbedtls_ecdh_context *ctx, size_t *olen, rs_ctx = &ctx->rs; #endif +#if defined(MBEDTLS_ECDH_COMPUTE_SHARED_ALT) + if( ( ret = mbedtls_ecdh_compute_shared( &ctx->grp, &ctx->z, &ctx->Qp, + &ctx->d, f_rng, p_rng ) ) != 0 ) + { + return( ret ); + } +#else if( ( ret = ecdh_compute_shared_restartable( &ctx->grp, &ctx->z, &ctx->Qp, &ctx->d, f_rng, p_rng, rs_ctx ) ) != 0 ) { return( ret ); } +#endif if( mbedtls_mpi_size( &ctx->z ) > blen ) return( MBEDTLS_ERR_ECP_BAD_INPUT_DATA ); diff --git a/library/ecdsa.c b/library/ecdsa.c index abac015ceb..37379bcdc7 100644 --- a/library/ecdsa.c +++ b/library/ecdsa.c @@ -237,7 +237,6 @@ cleanup: return( ret ); } -#if !defined(MBEDTLS_ECDSA_SIGN_ALT) /* * Compute ECDSA signature of a hashed message (SEC1 4.1.3) * Obviously, compared to SEC1 4.1.3, we skip step 4 (hash message) @@ -369,8 +368,8 @@ cleanup: return( ret ); } -#endif /* MBEDTLS_ECDSA_SIGN_ALT */ +#if !defined(MBEDTLS_ECDSA_SIGN_ALT) /* * Compute ECDSA signature of a hashed message */ @@ -381,6 +380,7 @@ int mbedtls_ecdsa_sign( mbedtls_ecp_group *grp, mbedtls_mpi *r, mbedtls_mpi *s, return( ecdsa_sign_restartable( grp, r, s, d, buf, blen, f_rng, p_rng, NULL ) ); } +#endif /* MBEDTLS_ECDSA_SIGN_ALT */ #if defined(MBEDTLS_ECDSA_DETERMINISTIC) /* @@ -432,8 +432,13 @@ static int ecdsa_sign_det_restartable( mbedtls_ecp_group *grp, sign: #endif +#if defined(MBEDTLS_ECDSA_SIGN_ALT) + ret = mbedtls_ecdsa_sign( grp, r, s, d, buf, blen, + mbedtls_hmac_drbg_random, p_rng ); +#else ret = ecdsa_sign_restartable( grp, r, s, d, buf, blen, mbedtls_hmac_drbg_random, p_rng, rs_ctx ); +#endif cleanup: mbedtls_hmac_drbg_free( &rng_ctx ); @@ -455,7 +460,6 @@ int mbedtls_ecdsa_sign_det( mbedtls_ecp_group *grp, mbedtls_mpi *r, mbedtls_mpi } #endif /* MBEDTLS_ECDSA_DETERMINISTIC */ -#if !defined(MBEDTLS_ECDSA_VERIFY_ALT) /* * Verify ECDSA signature of hashed message (SEC1 4.1.4) * Obviously, compared to SEC1 4.1.3, we skip step 2 (hash message) @@ -564,8 +568,8 @@ cleanup: return( ret ); } -#endif /* MBEDTLS_ECDSA_VERIFY_ALT */ +#if !defined(MBEDTLS_ECDSA_VERIFY_ALT) /* * Verify ECDSA signature of hashed message */ @@ -575,6 +579,7 @@ int mbedtls_ecdsa_verify( mbedtls_ecp_group *grp, { return( ecdsa_verify_restartable( grp, buf, blen, Q, r, s, NULL ) ); } +#endif /* MBEDTLS_ECDSA_VERIFY_ALT */ /* * Convert a signature (given by context) to ASN.1 @@ -626,8 +631,13 @@ int mbedtls_ecdsa_write_signature_restartable( mbedtls_ecdsa_context *ctx, #else (void) md_alg; +#if defined(MBEDTLS_ECDSA_SIGN_ALT) + MBEDTLS_MPI_CHK( mbedtls_ecdsa_sign( &ctx->grp, &r, &s, &ctx->d, + hash, hlen, f_rng, p_rng ) ); +#else MBEDTLS_MPI_CHK( ecdsa_sign_restartable( &ctx->grp, &r, &s, &ctx->d, hash, hlen, f_rng, p_rng, rs_ctx ) ); +#endif #endif MBEDTLS_MPI_CHK( ecdsa_signature_to_asn1( &r, &s, sig, slen ) ); @@ -712,10 +722,15 @@ int mbedtls_ecdsa_read_signature_restartable( mbedtls_ecdsa_context *ctx, ret += MBEDTLS_ERR_ECP_BAD_INPUT_DATA; goto cleanup; } - +#if defined(MBEDTLS_ECDSA_VERIFY_ALT) + if( ( ret = mbedtls_ecdsa_verify( &ctx->grp, hash, hlen, + &ctx->Q, &r, &s ) ) != 0 ) + goto cleanup; +#else if( ( ret = ecdsa_verify_restartable( &ctx->grp, hash, hlen, &ctx->Q, &r, &s, rs_ctx ) ) != 0 ) goto cleanup; +#endif /* At this point we know that the buffer starts with a valid signature. * Return 0 if the buffer just contains the signature, and a specific From 936d284f4d803de343f5fb98db86fc046bafdaff Mon Sep 17 00:00:00 2001 From: Ron Eldor Date: Thu, 1 Nov 2018 13:05:52 +0200 Subject: [PATCH 2/9] Minor fixes 1. Fix unused symbols compilation warnings. 2. Add comments for the closing `endif`. --- library/ecdh.c | 16 +++++++++++----- library/ecdsa.c | 16 ++++++++-------- 2 files changed, 19 insertions(+), 13 deletions(-) diff --git a/library/ecdh.c b/library/ecdh.c index f05e2c06af..9fad8e10b0 100644 --- a/library/ecdh.c +++ b/library/ecdh.c @@ -76,7 +76,7 @@ int mbedtls_ecdh_gen_public( mbedtls_ecp_group *grp, mbedtls_mpi *d, mbedtls_ecp { return( ecdh_gen_public_restartable( grp, d, Q, f_rng, p_rng, NULL ) ); } -#endif /* MBEDTLS_ECDH_GEN_PUBLIC_ALT */ +#endif /* !MBEDTLS_ECDH_GEN_PUBLIC_ALT */ #if !defined(MBEDTLS_ECDH_COMPUTE_SHARED_ALT) /* @@ -122,7 +122,7 @@ int mbedtls_ecdh_compute_shared( mbedtls_ecp_group *grp, mbedtls_mpi *z, return( ecdh_compute_shared_restartable( grp, z, Q, d, f_rng, p_rng, NULL ) ); } -#endif /* MBEDTLS_ECDH_COMPUTE_SHARED_ALT */ +#endif /* !MBEDTLS_ECDH_COMPUTE_SHARED_ALT */ /* * Initialize context @@ -191,7 +191,9 @@ int mbedtls_ecdh_make_params( mbedtls_ecdh_context *ctx, size_t *olen, { int ret; size_t grp_len, pt_len; +#if !defined(MBEDTLS_ECDH_GEN_PUBLIC_ALT) mbedtls_ecp_restart_ctx *rs_ctx = NULL; +#endif if( ctx == NULL || ctx->grp.pbits == 0 ) return( MBEDTLS_ERR_ECP_BAD_INPUT_DATA ); @@ -210,7 +212,7 @@ int mbedtls_ecdh_make_params( mbedtls_ecdh_context *ctx, size_t *olen, if( ( ret = ecdh_gen_public_restartable( &ctx->grp, &ctx->d, &ctx->Q, f_rng, p_rng, rs_ctx ) ) != 0 ) return( ret ); -#endif +#endif /* MBEDTLS_ECDH_GEN_PUBLIC_ALT */ if( ( ret = mbedtls_ecp_tls_write_group( &ctx->grp, &grp_len, buf, blen ) ) != 0 ) @@ -284,7 +286,9 @@ int mbedtls_ecdh_make_public( mbedtls_ecdh_context *ctx, size_t *olen, void *p_rng ) { int ret; +#if !defined(MBEDTLS_ECDH_GEN_PUBLIC_ALT) mbedtls_ecp_restart_ctx *rs_ctx = NULL; +#endif if( ctx == NULL || ctx->grp.pbits == 0 ) return( MBEDTLS_ERR_ECP_BAD_INPUT_DATA ); @@ -302,7 +306,7 @@ int mbedtls_ecdh_make_public( mbedtls_ecdh_context *ctx, size_t *olen, if( ( ret = ecdh_gen_public_restartable( &ctx->grp, &ctx->d, &ctx->Q, f_rng, p_rng, rs_ctx ) ) != 0 ) return( ret ); -#endif +#endif /* MBEDTLS_ECDH_GEN_PUBLIC_ALT */ return mbedtls_ecp_tls_write_point( &ctx->grp, &ctx->Q, ctx->point_format, olen, buf, blen ); @@ -338,7 +342,9 @@ int mbedtls_ecdh_calc_secret( mbedtls_ecdh_context *ctx, size_t *olen, void *p_rng ) { int ret; +#if !defined(MBEDTLS_ECDH_COMPUTE_SHARED_ALT) mbedtls_ecp_restart_ctx *rs_ctx = NULL; +#endif if( ctx == NULL || ctx->grp.pbits == 0 ) return( MBEDTLS_ERR_ECP_BAD_INPUT_DATA ); @@ -360,7 +366,7 @@ int mbedtls_ecdh_calc_secret( mbedtls_ecdh_context *ctx, size_t *olen, { return( ret ); } -#endif +#endif /* MBEDTLS_ECDH_COMPUTE_SHARED_ALT */ if( mbedtls_mpi_size( &ctx->z ) > blen ) return( MBEDTLS_ERR_ECP_BAD_INPUT_DATA ); diff --git a/library/ecdsa.c b/library/ecdsa.c index 37379bcdc7..54ecab11e7 100644 --- a/library/ecdsa.c +++ b/library/ecdsa.c @@ -237,6 +237,7 @@ cleanup: return( ret ); } +#if !defined(MBEDTLS_ECDSA_SIGN_ALT) /* * Compute ECDSA signature of a hashed message (SEC1 4.1.3) * Obviously, compared to SEC1 4.1.3, we skip step 4 (hash message) @@ -369,7 +370,6 @@ cleanup: return( ret ); } -#if !defined(MBEDTLS_ECDSA_SIGN_ALT) /* * Compute ECDSA signature of a hashed message */ @@ -380,7 +380,7 @@ int mbedtls_ecdsa_sign( mbedtls_ecp_group *grp, mbedtls_mpi *r, mbedtls_mpi *s, return( ecdsa_sign_restartable( grp, r, s, d, buf, blen, f_rng, p_rng, NULL ) ); } -#endif /* MBEDTLS_ECDSA_SIGN_ALT */ +#endif /* !MBEDTLS_ECDSA_SIGN_ALT */ #if defined(MBEDTLS_ECDSA_DETERMINISTIC) /* @@ -438,7 +438,7 @@ sign: #else ret = ecdsa_sign_restartable( grp, r, s, d, buf, blen, mbedtls_hmac_drbg_random, p_rng, rs_ctx ); -#endif +#endif /* MBEDTLS_ECDSA_SIGN_ALT */ cleanup: mbedtls_hmac_drbg_free( &rng_ctx ); @@ -460,6 +460,7 @@ int mbedtls_ecdsa_sign_det( mbedtls_ecp_group *grp, mbedtls_mpi *r, mbedtls_mpi } #endif /* MBEDTLS_ECDSA_DETERMINISTIC */ +#if !defined(MBEDTLS_ECDSA_VERIFY_ALT) /* * Verify ECDSA signature of hashed message (SEC1 4.1.4) * Obviously, compared to SEC1 4.1.3, we skip step 2 (hash message) @@ -569,7 +570,6 @@ cleanup: return( ret ); } -#if !defined(MBEDTLS_ECDSA_VERIFY_ALT) /* * Verify ECDSA signature of hashed message */ @@ -579,7 +579,7 @@ int mbedtls_ecdsa_verify( mbedtls_ecp_group *grp, { return( ecdsa_verify_restartable( grp, buf, blen, Q, r, s, NULL ) ); } -#endif /* MBEDTLS_ECDSA_VERIFY_ALT */ +#endif /* !MBEDTLS_ECDSA_VERIFY_ALT */ /* * Convert a signature (given by context) to ASN.1 @@ -637,7 +637,7 @@ int mbedtls_ecdsa_write_signature_restartable( mbedtls_ecdsa_context *ctx, #else MBEDTLS_MPI_CHK( ecdsa_sign_restartable( &ctx->grp, &r, &s, &ctx->d, hash, hlen, f_rng, p_rng, rs_ctx ) ); -#endif +#endif /* MBEDTLS_ECDSA_SIGN_ALT */ #endif MBEDTLS_MPI_CHK( ecdsa_signature_to_asn1( &r, &s, sig, slen ) ); @@ -730,7 +730,7 @@ int mbedtls_ecdsa_read_signature_restartable( mbedtls_ecdsa_context *ctx, if( ( ret = ecdsa_verify_restartable( &ctx->grp, hash, hlen, &ctx->Q, &r, &s, rs_ctx ) ) != 0 ) goto cleanup; -#endif +#endif /* MBEDTLS_ECDSA_VERIFY_ALT */ /* At this point we know that the buffer starts with a valid signature. * Return 0 if the buffer just contains the signature, and a specific @@ -755,7 +755,7 @@ int mbedtls_ecdsa_genkey( mbedtls_ecdsa_context *ctx, mbedtls_ecp_group_id gid, return( mbedtls_ecp_group_load( &ctx->grp, gid ) || mbedtls_ecp_gen_keypair( &ctx->grp, &ctx->d, &ctx->Q, f_rng, p_rng ) ); } -#endif /* MBEDTLS_ECDSA_GENKEY_ALT */ +#endif /* !MBEDTLS_ECDSA_GENKEY_ALT */ /* * Set context from an mbedtls_ecp_keypair From 2b161c33bef53b2ac43bfbe7dffe2a8489e6d086 Mon Sep 17 00:00:00 2001 From: Ron Eldor Date: Thu, 1 Nov 2018 16:18:20 +0200 Subject: [PATCH 3/9] Fix compilation issue Fix compilation error when both `MBEDTLS_ECP_RESTARTABLE` and the alternative definition of ECDH function are defined. --- library/ecdh.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/library/ecdh.c b/library/ecdh.c index 9fad8e10b0..b7e25dcc91 100644 --- a/library/ecdh.c +++ b/library/ecdh.c @@ -198,7 +198,7 @@ int mbedtls_ecdh_make_params( mbedtls_ecdh_context *ctx, size_t *olen, if( ctx == NULL || ctx->grp.pbits == 0 ) return( MBEDTLS_ERR_ECP_BAD_INPUT_DATA ); -#if defined(MBEDTLS_ECP_RESTARTABLE) +#if defined(MBEDTLS_ECP_RESTARTABLE) && !defined(MBEDTLS_ECDH_GEN_PUBLIC_ALT) if( ctx->restart_enabled ) rs_ctx = &ctx->rs; #endif @@ -293,7 +293,7 @@ int mbedtls_ecdh_make_public( mbedtls_ecdh_context *ctx, size_t *olen, if( ctx == NULL || ctx->grp.pbits == 0 ) return( MBEDTLS_ERR_ECP_BAD_INPUT_DATA ); -#if defined(MBEDTLS_ECP_RESTARTABLE) +#if defined(MBEDTLS_ECP_RESTARTABLE) && !defined(MBEDTLS_ECDH_GEN_PUBLIC_ALT) if( ctx->restart_enabled ) rs_ctx = &ctx->rs; #endif @@ -349,7 +349,7 @@ int mbedtls_ecdh_calc_secret( mbedtls_ecdh_context *ctx, size_t *olen, if( ctx == NULL || ctx->grp.pbits == 0 ) return( MBEDTLS_ERR_ECP_BAD_INPUT_DATA ); -#if defined(MBEDTLS_ECP_RESTARTABLE) +#if defined(MBEDTLS_ECP_RESTARTABLE) && !defined(MBEDTLS_ECDH_GEN_PUBLIC_ALT) if( ctx->restart_enabled ) rs_ctx = &ctx->rs; #endif From 5ed8c1ededa349cde7cb949171202d151067f9b4 Mon Sep 17 00:00:00 2001 From: Ron Eldor Date: Mon, 5 Nov 2018 14:04:26 +0200 Subject: [PATCH 4/9] Avoid using restartable and alternative ECP imp. 1. Add a check that MBEDTLS_ECP_RESTARTABLE is not defined along any EC* alternative implementation. 2. Add a closing comment foran `#endif`. --- include/mbedtls/check_config.h | 10 ++++++++++ include/mbedtls/config.h | 3 +++ library/ecdsa.c | 4 ++-- 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/include/mbedtls/check_config.h b/include/mbedtls/check_config.h index 9e6bb8a46a..425e3ea589 100644 --- a/include/mbedtls/check_config.h +++ b/include/mbedtls/check_config.h @@ -108,6 +108,16 @@ #error "MBEDTLS_ECJPAKE_C defined, but not all prerequisites" #endif +#if defined(MBEDTLS_ECP_RESTARTABLE) && \ + ( defined(MBEDTLS_ECDH_COMPUTE_SHARED_ALT) || \ + defined(MBEDTLS_ECDH_GEN_PUBLIC_ALT) || \ + defined(MBEDTLS_ECDSA_SIGN_ALT) || \ + defined(MBEDTLS_ECDSA_VERIFY_ALT) || \ + defined(MBEDTLS_ECDSA_GENKEY_ALT) || \ + defined(MBEDTLS_ECP_ALT) ) +#error "MBEDTLS_ECP_RESTARTABLE defined, but it cannot coexist with an alternative ECP implementation" +#endif + #if defined(MBEDTLS_ECDSA_DETERMINISTIC) && !defined(MBEDTLS_HMAC_DRBG_C) #error "MBEDTLS_ECDSA_DETERMINISTIC defined, but not all prerequisites" #endif diff --git a/include/mbedtls/config.h b/include/mbedtls/config.h index 28e860b189..c594b69442 100644 --- a/include/mbedtls/config.h +++ b/include/mbedtls/config.h @@ -694,6 +694,9 @@ * for too long on ECC (and, hence, X.509 or SSL/TLS) operations. * * Uncomment this macro to enable restartable ECC computations. + * + * \note MBEDTLS_ECP_RESTARTABLE cannot be defined if there is an alternative + * implementation for one of the ECP, ECDSA or ECDH functions. * */ //#define MBEDTLS_ECP_RESTARTABLE diff --git a/library/ecdsa.c b/library/ecdsa.c index 54ecab11e7..a62c14cbe0 100644 --- a/library/ecdsa.c +++ b/library/ecdsa.c @@ -638,7 +638,7 @@ int mbedtls_ecdsa_write_signature_restartable( mbedtls_ecdsa_context *ctx, MBEDTLS_MPI_CHK( ecdsa_sign_restartable( &ctx->grp, &r, &s, &ctx->d, hash, hlen, f_rng, p_rng, rs_ctx ) ); #endif /* MBEDTLS_ECDSA_SIGN_ALT */ -#endif +#endif /* MBEDTLS_ECDSA_DETERMINISTIC */ MBEDTLS_MPI_CHK( ecdsa_signature_to_asn1( &r, &s, sig, slen ) ); @@ -662,7 +662,7 @@ int mbedtls_ecdsa_write_signature( mbedtls_ecdsa_context *ctx, mbedtls_md_type_t ctx, md_alg, hash, hlen, sig, slen, f_rng, p_rng, NULL ) ); } -#if ! defined(MBEDTLS_DEPRECATED_REMOVED) && \ +#if !defined(MBEDTLS_DEPRECATED_REMOVED) && \ defined(MBEDTLS_ECDSA_DETERMINISTIC) int mbedtls_ecdsa_write_signature_det( mbedtls_ecdsa_context *ctx, const unsigned char *hash, size_t hlen, From 19779c47397f68837566d7f051fb8aaa17f34792 Mon Sep 17 00:00:00 2001 From: Ron Eldor Date: Mon, 5 Nov 2018 16:58:13 +0200 Subject: [PATCH 5/9] Some style and documentation fixes 1. Change description of of hte `MBEDTLS_ECP_RESTARTABLE` in the configuration file. 2. Change check for compilation of `rs_ctx` to positive flow. --- include/mbedtls/config.h | 5 +++-- library/ecdh.c | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/include/mbedtls/config.h b/include/mbedtls/config.h index c594b69442..950319a102 100644 --- a/include/mbedtls/config.h +++ b/include/mbedtls/config.h @@ -695,8 +695,9 @@ * * Uncomment this macro to enable restartable ECC computations. * - * \note MBEDTLS_ECP_RESTARTABLE cannot be defined if there is an alternative - * implementation for one of the ECP, ECDSA or ECDH functions. * + * \note This option only works with the default software implementation of + * elliptic curve functionality. It is incompatible with + * MBEDTLS_ECP_ALT, MBEDTLS_ECDH_xxx_ALT and MBEDTLS_ECDSA_xxx_ALT. */ //#define MBEDTLS_ECP_RESTARTABLE diff --git a/library/ecdh.c b/library/ecdh.c index b7e25dcc91..307ee1fb20 100644 --- a/library/ecdh.c +++ b/library/ecdh.c @@ -191,14 +191,14 @@ int mbedtls_ecdh_make_params( mbedtls_ecdh_context *ctx, size_t *olen, { int ret; size_t grp_len, pt_len; -#if !defined(MBEDTLS_ECDH_GEN_PUBLIC_ALT) +#if defined(MBEDTLS_ECP_RESTARTABLE) mbedtls_ecp_restart_ctx *rs_ctx = NULL; #endif if( ctx == NULL || ctx->grp.pbits == 0 ) return( MBEDTLS_ERR_ECP_BAD_INPUT_DATA ); -#if defined(MBEDTLS_ECP_RESTARTABLE) && !defined(MBEDTLS_ECDH_GEN_PUBLIC_ALT) +#if defined(MBEDTLS_ECP_RESTARTABLE) if( ctx->restart_enabled ) rs_ctx = &ctx->rs; #endif From b430d9f26209c18102719b9f6ab70af75744b92f Mon Sep 17 00:00:00 2001 From: Ron Eldor Date: Mon, 5 Nov 2018 17:18:29 +0200 Subject: [PATCH 6/9] Change to positive checks 1. Checge to check for `MBEDTLS_ECP_RESTARTABLE` for all definitions of `rs_ctx`. 2. Remove checks for `_ALT` when using `rs_ctx` as they cannot coexist with the Restartable configuration. --- library/ecdh.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/library/ecdh.c b/library/ecdh.c index 307ee1fb20..5fb06cca37 100644 --- a/library/ecdh.c +++ b/library/ecdh.c @@ -212,7 +212,7 @@ int mbedtls_ecdh_make_params( mbedtls_ecdh_context *ctx, size_t *olen, if( ( ret = ecdh_gen_public_restartable( &ctx->grp, &ctx->d, &ctx->Q, f_rng, p_rng, rs_ctx ) ) != 0 ) return( ret ); -#endif /* MBEDTLS_ECDH_GEN_PUBLIC_ALT */ +#endif /* !MBEDTLS_ECDH_GEN_PUBLIC_ALT */ if( ( ret = mbedtls_ecp_tls_write_group( &ctx->grp, &grp_len, buf, blen ) ) != 0 ) @@ -286,14 +286,14 @@ int mbedtls_ecdh_make_public( mbedtls_ecdh_context *ctx, size_t *olen, void *p_rng ) { int ret; -#if !defined(MBEDTLS_ECDH_GEN_PUBLIC_ALT) +#if defined(MBEDTLS_ECP_RESTARTABLE) mbedtls_ecp_restart_ctx *rs_ctx = NULL; #endif if( ctx == NULL || ctx->grp.pbits == 0 ) return( MBEDTLS_ERR_ECP_BAD_INPUT_DATA ); -#if defined(MBEDTLS_ECP_RESTARTABLE) && !defined(MBEDTLS_ECDH_GEN_PUBLIC_ALT) +#if defined(MBEDTLS_ECP_RESTARTABLE) if( ctx->restart_enabled ) rs_ctx = &ctx->rs; #endif @@ -342,14 +342,14 @@ int mbedtls_ecdh_calc_secret( mbedtls_ecdh_context *ctx, size_t *olen, void *p_rng ) { int ret; -#if !defined(MBEDTLS_ECDH_COMPUTE_SHARED_ALT) +#if defined(MBEDTLS_ECP_RESTARTABLE) mbedtls_ecp_restart_ctx *rs_ctx = NULL; #endif if( ctx == NULL || ctx->grp.pbits == 0 ) return( MBEDTLS_ERR_ECP_BAD_INPUT_DATA ); -#if defined(MBEDTLS_ECP_RESTARTABLE) && !defined(MBEDTLS_ECDH_GEN_PUBLIC_ALT) +#if defined(MBEDTLS_ECP_RESTARTABLE) if( ctx->restart_enabled ) rs_ctx = &ctx->rs; #endif From 93ace0199b05cb5d9cd3d3853c54f7c52087fae8 Mon Sep 17 00:00:00 2001 From: Ron Eldor Date: Mon, 5 Nov 2018 17:50:07 +0200 Subject: [PATCH 7/9] Revert positive flow check Revert changes for checking whether `MBEDTLS_ECP_RESTARTABLE` is defined, since it broke the CI. The context is used whether the restartable feature is defined or not. --- library/ecdh.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/library/ecdh.c b/library/ecdh.c index 5fb06cca37..0fed2d19d3 100644 --- a/library/ecdh.c +++ b/library/ecdh.c @@ -191,7 +191,7 @@ int mbedtls_ecdh_make_params( mbedtls_ecdh_context *ctx, size_t *olen, { int ret; size_t grp_len, pt_len; -#if defined(MBEDTLS_ECP_RESTARTABLE) +#if !defined(MBEDTLS_ECDH_GEN_PUBLIC_ALT) mbedtls_ecp_restart_ctx *rs_ctx = NULL; #endif @@ -286,7 +286,7 @@ int mbedtls_ecdh_make_public( mbedtls_ecdh_context *ctx, size_t *olen, void *p_rng ) { int ret; -#if defined(MBEDTLS_ECP_RESTARTABLE) +#if !defined(MBEDTLS_ECDH_GEN_PUBLIC_ALT) mbedtls_ecp_restart_ctx *rs_ctx = NULL; #endif @@ -342,7 +342,7 @@ int mbedtls_ecdh_calc_secret( mbedtls_ecdh_context *ctx, size_t *olen, void *p_rng ) { int ret; -#if defined(MBEDTLS_ECP_RESTARTABLE) +#if !defined(MBEDTLS_ECDH_COMPUTE_SHARED_ALT) mbedtls_ecp_restart_ctx *rs_ctx = NULL; #endif From 2981d8f16183e06a3fae2013ef8731842b1d5f89 Mon Sep 17 00:00:00 2001 From: Ron Eldor Date: Mon, 5 Nov 2018 18:07:10 +0200 Subject: [PATCH 8/9] Change to positive flow for all cases Use the `rs_ctx` only when `MBEDTLS_ECP_RESTARTABLE` is defined. --- library/ecdh.c | 46 +++++++++++++++++++++++----------------------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/library/ecdh.c b/library/ecdh.c index 0fed2d19d3..e6ae99994e 100644 --- a/library/ecdh.c +++ b/library/ecdh.c @@ -191,7 +191,7 @@ int mbedtls_ecdh_make_params( mbedtls_ecdh_context *ctx, size_t *olen, { int ret; size_t grp_len, pt_len; -#if !defined(MBEDTLS_ECDH_GEN_PUBLIC_ALT) +#if defined(MBEDTLS_ECP_RESTARTABLE) mbedtls_ecp_restart_ctx *rs_ctx = NULL; #endif @@ -204,15 +204,15 @@ int mbedtls_ecdh_make_params( mbedtls_ecdh_context *ctx, size_t *olen, #endif -#if defined(MBEDTLS_ECDH_GEN_PUBLIC_ALT) - if( ( ret = mbedtls_ecdh_gen_public( &ctx->grp, &ctx->d, &ctx->Q, - f_rng, p_rng ) ) != 0 ) - return( ret ); -#else +#if defined(MBEDTLS_ECP_RESTARTABLE) if( ( ret = ecdh_gen_public_restartable( &ctx->grp, &ctx->d, &ctx->Q, f_rng, p_rng, rs_ctx ) ) != 0 ) return( ret ); -#endif /* !MBEDTLS_ECDH_GEN_PUBLIC_ALT */ +#else + if( ( ret = mbedtls_ecdh_gen_public( &ctx->grp, &ctx->d, &ctx->Q, + f_rng, p_rng ) ) != 0 ) + return( ret ); +#endif /* MBEDTLS_ECP_RESTARTABLE */ if( ( ret = mbedtls_ecp_tls_write_group( &ctx->grp, &grp_len, buf, blen ) ) != 0 ) @@ -286,7 +286,7 @@ int mbedtls_ecdh_make_public( mbedtls_ecdh_context *ctx, size_t *olen, void *p_rng ) { int ret; -#if !defined(MBEDTLS_ECDH_GEN_PUBLIC_ALT) +#if defined(MBEDTLS_ECP_RESTARTABLE) mbedtls_ecp_restart_ctx *rs_ctx = NULL; #endif @@ -298,15 +298,15 @@ int mbedtls_ecdh_make_public( mbedtls_ecdh_context *ctx, size_t *olen, rs_ctx = &ctx->rs; #endif -#if defined(MBEDTLS_ECDH_GEN_PUBLIC_ALT) - if( ( ret = mbedtls_ecdh_gen_public( &ctx->grp, &ctx->d, &ctx->Q, - f_rng, p_rng ) ) != 0 ) - return( ret ); -#else +#if defined(MBEDTLS_ECP_RESTARTABLE) if( ( ret = ecdh_gen_public_restartable( &ctx->grp, &ctx->d, &ctx->Q, f_rng, p_rng, rs_ctx ) ) != 0 ) return( ret ); -#endif /* MBEDTLS_ECDH_GEN_PUBLIC_ALT */ +#else + if( ( ret = mbedtls_ecdh_gen_public( &ctx->grp, &ctx->d, &ctx->Q, + f_rng, p_rng ) ) != 0 ) + return( ret ); +#endif /* MBEDTLS_ECP_RESTARTABLE */ return mbedtls_ecp_tls_write_point( &ctx->grp, &ctx->Q, ctx->point_format, olen, buf, blen ); @@ -342,7 +342,7 @@ int mbedtls_ecdh_calc_secret( mbedtls_ecdh_context *ctx, size_t *olen, void *p_rng ) { int ret; -#if !defined(MBEDTLS_ECDH_COMPUTE_SHARED_ALT) +#if defined(MBEDTLS_ECP_RESTARTABLE) mbedtls_ecp_restart_ctx *rs_ctx = NULL; #endif @@ -354,19 +354,19 @@ int mbedtls_ecdh_calc_secret( mbedtls_ecdh_context *ctx, size_t *olen, rs_ctx = &ctx->rs; #endif -#if defined(MBEDTLS_ECDH_COMPUTE_SHARED_ALT) - if( ( ret = mbedtls_ecdh_compute_shared( &ctx->grp, &ctx->z, &ctx->Qp, - &ctx->d, f_rng, p_rng ) ) != 0 ) - { - return( ret ); - } -#else +#if defined(MBEDTLS_ECP_RESTARTABLE) if( ( ret = ecdh_compute_shared_restartable( &ctx->grp, &ctx->z, &ctx->Qp, &ctx->d, f_rng, p_rng, rs_ctx ) ) != 0 ) { return( ret ); } -#endif /* MBEDTLS_ECDH_COMPUTE_SHARED_ALT */ +#else + if( ( ret = mbedtls_ecdh_compute_shared( &ctx->grp, &ctx->z, &ctx->Qp, + &ctx->d, f_rng, p_rng ) ) != 0 ) + { + return( ret ); + } +#endif /* MBEDTLS_ECP_RESTARTABLE */ if( mbedtls_mpi_size( &ctx->z ) > blen ) return( MBEDTLS_ERR_ECP_BAD_INPUT_DATA ); From 7213744b076d068820aa35a0325cadf9d98886bd Mon Sep 17 00:00:00 2001 From: Ron Eldor Date: Mon, 5 Nov 2018 22:31:32 +0200 Subject: [PATCH 9/9] Fix typo in comment Change from from lower case to upper case in XXX_ALT comment in `MBEDTLS_ECP_RESTARTABLE` description. --- include/mbedtls/config.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/mbedtls/config.h b/include/mbedtls/config.h index 950319a102..9f8192fd64 100644 --- a/include/mbedtls/config.h +++ b/include/mbedtls/config.h @@ -697,7 +697,7 @@ * * \note This option only works with the default software implementation of * elliptic curve functionality. It is incompatible with - * MBEDTLS_ECP_ALT, MBEDTLS_ECDH_xxx_ALT and MBEDTLS_ECDSA_xxx_ALT. + * MBEDTLS_ECP_ALT, MBEDTLS_ECDH_XXX_ALT and MBEDTLS_ECDSA_XXX_ALT. */ //#define MBEDTLS_ECP_RESTARTABLE