From c9fefe660f30e3a6241b52abace16399782e88b4 Mon Sep 17 00:00:00 2001 From: Sean Parkinson Date: Thu, 2 Feb 2023 12:10:52 +1000 Subject: [PATCH] EC OpenSSL compat: validate point after setting wolfSSL_EC_POINT_set_affine_coordinates_GFp wasn't checking the point is valid for the curve. Added call to check point when setting. Made check available for opensslextra. Fixed test to have valid ordinates to set. --- src/pk.c | 25 ++++++++++++++++++------- tests/api.c | 28 +++++++++++++++++++--------- wolfssl/wolfcrypt/settings.h | 12 ++++++++++-- 3 files changed, 47 insertions(+), 18 deletions(-) diff --git a/src/pk.c b/src/pk.c index d456cf295..572a2727c 100644 --- a/src/pk.c +++ b/src/pk.c @@ -10151,7 +10151,8 @@ WOLFSSL_BIGNUM *wolfSSL_EC_POINT_point2bn(const WOLFSSL_EC_GROUP* group, return ret; } -#if defined(USE_ECC_B_PARAM) && (!defined(HAVE_FIPS) || FIPS_VERSION_GT(2,0)) +#if defined(USE_ECC_B_PARAM) && !defined(HAVE_SELFTEST) && \ + (!defined(HAVE_FIPS) || FIPS_VERSION_GT(2,0)) /* Check if EC point is on the the curve defined by the EC group. * * @param [in] group EC group defining curve. @@ -10192,7 +10193,7 @@ int wolfSSL_EC_POINT_is_on_curve(const WOLFSSL_EC_GROUP *group, /* Return boolean of on curve. No error means on curve. */ return !err; } -#endif /* USE_ECC_B_PARAM && !(FIPS_VERSION <= 2) */ +#endif /* USE_ECC_B_PARAM && !HAVE_SELFTEST && !(FIPS_VERSION <= 2) */ #if !defined(WOLFSSL_SP_MATH) && !defined(WOLF_CRYPTO_CB_ONLY_ECC) /* Convert Jacobian ordinates to affine. @@ -10337,9 +10338,9 @@ int wolfSSL_EC_POINT_get_affine_coordinates_GFp(const WOLFSSL_EC_GROUP* group, * @return 1 on success. * @return 0 on error. */ -int wolfSSL_EC_POINT_set_affine_coordinates_GFp(const WOLFSSL_EC_GROUP *group, - WOLFSSL_EC_POINT *point, const WOLFSSL_BIGNUM *x, const WOLFSSL_BIGNUM *y, - WOLFSSL_BN_CTX *ctx) +int wolfSSL_EC_POINT_set_affine_coordinates_GFp(const WOLFSSL_EC_GROUP* group, + WOLFSSL_EC_POINT* point, const WOLFSSL_BIGNUM* x, const WOLFSSL_BIGNUM* y, + WOLFSSL_BN_CTX* ctx) { int ret = 1; @@ -10396,6 +10397,16 @@ int wolfSSL_EC_POINT_set_affine_coordinates_GFp(const WOLFSSL_EC_GROUP *group, ret = 0; } +#if defined(USE_ECC_B_PARAM) && !defined(HAVE_SELFTEST) && \ + (!defined(HAVE_FIPS) || FIPS_VERSION_GT(2,0)) + /* Check that the point is valid. */ + if ((ret == 1) && (wolfSSL_EC_POINT_is_on_curve(group, + (WOLFSSL_EC_POINT *)point, ctx) != 1)) { + WOLFSSL_MSG("EC_POINT_is_on_curve failed"); + ret = 0; + } +#endif + return ret; } @@ -11020,8 +11031,8 @@ int wolfSSL_EC_POINT_copy(WOLFSSL_EC_POINT *dest, const WOLFSSL_EC_POINT *src) } /* Copy internal EC points. */ - if ((ret == 1) && (wc_ecc_copy_point((ecc_point*) dest->internal, - (ecc_point*) src->internal) != MP_OKAY)) { + if ((ret == 1) && (wc_ecc_copy_point((ecc_point*)src->internal, + (ecc_point*)dest->internal) != MP_OKAY)) { ret = 0; } diff --git a/tests/api.c b/tests/api.c index 2546377d7..d2c9af383 100644 --- a/tests/api.c +++ b/tests/api.c @@ -26331,7 +26331,8 @@ static int test_wc_ecc_pointFns(void) } } -#if !defined(HAVE_FIPS) || (defined(HAVE_FIPS_VERSION) && (HAVE_FIPS_VERSION>2)) +#if !defined(HAVE_SELFTEST) && (!defined(HAVE_FIPS) || \ + (defined(HAVE_FIPS_VERSION) && (HAVE_FIPS_VERSION>2))) #ifdef USE_ECC_B_PARAM /* On curve if ret == 0 */ if (ret == 0) { @@ -26351,7 +26352,7 @@ static int test_wc_ecc_pointFns(void) } } #endif /* USE_ECC_B_PARAM */ -#endif /* !HAVE_FIPS || HAVE_FIPS_VERSION > 2 */ +#endif /* !HAVE_SELFTEST && (!HAVE_FIPS || HAVE_FIPS_VERSION > 2) */ /* Free */ wc_ecc_del_point(point); @@ -56222,14 +56223,10 @@ static int test_wolfSSL_EC_POINT(void) /* check if point X coordinate is zero */ AssertIntEQ(BN_is_zero(new_point->X), 0); -#ifdef USE_ECC_B_PARAM +#if defined(USE_ECC_B_PARAM) && !defined(HAVE_SELFTEST) && \ + (!defined(HAVE_FIPS) || FIPS_VERSION_GT(2,0)) AssertIntEQ(EC_POINT_is_on_curve(group, new_point, ctx), 1); -#endif /* USE_ECC_B_PARAM */ - - /* Force non-affine coordinates */ - AssertIntEQ(BN_add(new_point->Z, (WOLFSSL_BIGNUM*)BN_value_one(), - (WOLFSSL_BIGNUM*)BN_value_one()), 1); - new_point->inSet = 0; +#endif /* extract the coordinates from point */ AssertIntEQ(EC_POINT_get_affine_coordinates_GFp(group, new_point, X, Y, @@ -56264,6 +56261,19 @@ static int test_wolfSSL_EC_POINT(void) AssertIntEQ(EC_POINT_invert(group, NULL, ctx), 0); AssertIntEQ(EC_POINT_invert(group, new_point, ctx), 1); + /* Test getting affine converts from projective. */ + AssertIntEQ(EC_POINT_copy(set_point, new_point), 1); + /* Force non-affine coordinates */ + AssertIntEQ(BN_add(new_point->Z, (WOLFSSL_BIGNUM*)BN_value_one(), + (WOLFSSL_BIGNUM*)BN_value_one()), 1); + new_point->inSet = 0; + /* extract the coordinates from point */ + AssertIntEQ(EC_POINT_get_affine_coordinates_GFp(group, new_point, X, Y, + ctx), WOLFSSL_SUCCESS); + /* check if point ordinates have changed. */ + AssertIntNE(BN_cmp(X, set_point->X), 0); + AssertIntNE(BN_cmp(Y, set_point->Y), 0); + /* Test check for infinity */ #ifndef WOLF_CRYPTO_CB_ONLY_ECC AssertIntEQ(EC_POINT_is_at_infinity(NULL, NULL), 0); diff --git a/wolfssl/wolfcrypt/settings.h b/wolfssl/wolfcrypt/settings.h index 702d5b36d..f888fc369 100644 --- a/wolfssl/wolfcrypt/settings.h +++ b/wolfssl/wolfcrypt/settings.h @@ -1975,7 +1975,6 @@ extern void uITRON4_free(void *p) ; - /* user can specify what curves they want with ECC_USER_CURVES otherwise * all curves are on by default for now */ #ifndef ECC_USER_CURVES @@ -2010,7 +2009,8 @@ extern void uITRON4_free(void *p) ; /* ECC Configs */ #ifdef HAVE_ECC - /* By default enable Sign, Verify, DHE, Key Import and Key Export unless explicitly disabled */ + /* By default enable Sign, Verify, DHE, Key Import and Key Export unless + * explicitly disabled */ #if !defined(NO_ECC_SIGN) && \ (!defined(ECC_TIMING_RESISTANT) || \ (defined(ECC_TIMING_RESISTANT) && !defined(WC_NO_RNG))) @@ -2039,6 +2039,14 @@ extern void uITRON4_free(void *p) ; #endif #endif /* HAVE_ECC */ +#if defined(OPENSSL_EXTRA) && defined(HAVE_ECC) && \ + !defined(WOLFSSL_ATECC508A) && !defined(WOLFSSL_ATECC608A) && \ + !defined(WOLFSSL_CRYPTOCELL) && !defined(WOLFSSL_SE050) && \ + !defined(WOLF_CRYPTO_CB_ONLY_ECC) && !defined(WOLFSSL_STM32_PKA) + #undef USE_ECC_B_PARAM + #define USE_ECC_B_PARAM +#endif + /* Curve25519 Configs */ #ifdef HAVE_CURVE25519 /* By default enable shared secret, key export and import */