diff --git a/src/pk_ec.c b/src/pk_ec.c index 2a44ca55de..dabce27918 100644 --- a/src/pk_ec.c +++ b/src/pk_ec.c @@ -1532,7 +1532,7 @@ int wolfSSL_ECPoint_d2i(const unsigned char *in, unsigned int len, ret = 0; } - /* wolfSSL_EC_POINT_set_affine_coordinates_GFp check that the point is + /* wolfSSL_EC_POINT_set_affine_coordinates_GFp checks that the point is * on the curve. */ if (ret == 1 && wolfSSL_EC_POINT_set_affine_coordinates_GFp(group, point, x, y, NULL) != 1) { @@ -1544,6 +1544,18 @@ int wolfSSL_ECPoint_d2i(const unsigned char *in, unsigned int len, "operations later on."); #endif } +#if !defined(HAVE_SELFTEST) && (!defined(HAVE_FIPS) || FIPS_VERSION_GT(2,0)) + /* Validate that the imported point lies on the curve. The Z!=1 path + * above validates via set_affine_coordinates_GFp, but for affine + * imports (Z==1), the common case for uncompressed points, that + * block is skipped. Check unconditionally so no import path can + * bypass validation. */ + if (ret == 1 && wolfSSL_EC_POINT_is_on_curve(group, + (WOLFSSL_EC_POINT *)point, NULL) != 1) { + WOLFSSL_MSG("wolfSSL_ECPoint_d2i: point not on curve"); + ret = 0; + } +#endif if (ret == 1) { /* Dump new point. */ @@ -1750,8 +1762,7 @@ WOLFSSL_BIGNUM *wolfSSL_EC_POINT_point2bn(const WOLFSSL_EC_GROUP* group, return ret; } -#if defined(USE_ECC_B_PARAM) && !defined(HAVE_SELFTEST) && \ - (!defined(HAVE_FIPS) || FIPS_VERSION_GT(2,0)) +#if !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. @@ -1792,7 +1803,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 && !HAVE_SELFTEST && !(FIPS_VERSION <= 2) */ +#endif /* !HAVE_SELFTEST && !(HAVE_FIPS && FIPS_VERSION <= 2) */ #if !defined(WOLFSSL_SP_MATH) && !defined(WOLF_CRYPTO_CB_ONLY_ECC) /* Convert Jacobian ordinates to affine. @@ -1985,8 +1996,7 @@ 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)) +#if !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)) { diff --git a/tests/api/test_ecc.c b/tests/api/test_ecc.c index 5c1ee42115..b0d32d4b8c 100644 --- a/tests/api/test_ecc.c +++ b/tests/api/test_ecc.c @@ -1314,6 +1314,7 @@ int test_wc_ecc_encryptDecrypt(void) #ifdef WOLFSSL_ECIES_OLD tmpKey.dp = cliKey.dp; + tmpKey.idx = cliKey.idx; ExpectIntEQ(wc_ecc_copy_point(&cliKey.pubkey, &tmpKey.pubkey), 0); #endif @@ -1445,7 +1446,6 @@ int test_wc_ecc_pointFns(void) #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 */ ExpectIntEQ(wc_ecc_point_is_on_curve(point, idx), 0); /* Test bad args. */ @@ -1453,7 +1453,6 @@ int test_wc_ecc_pointFns(void) WC_NO_ERR_TRACE(BAD_FUNC_ARG)); ExpectIntEQ(wc_ecc_point_is_on_curve(point, 1000), WC_NO_ERR_TRACE(ECC_BAD_ARG_E)); -#endif /* USE_ECC_B_PARAM */ #endif /* !HAVE_SELFTEST && (!HAVE_FIPS || HAVE_FIPS_VERSION > 2) */ /* Free */ diff --git a/tests/api/test_ossl_ec.c b/tests/api/test_ossl_ec.c index 7f788984cd..5411c46130 100644 --- a/tests/api/test_ossl_ec.c +++ b/tests/api/test_ossl_ec.c @@ -476,8 +476,7 @@ int test_wolfSSL_EC_POINT(void) /* check if point X coordinate is zero */ ExpectIntEQ(BN_is_zero(new_point->X), 0); -#if defined(USE_ECC_B_PARAM) && !defined(HAVE_SELFTEST) && \ - (!defined(HAVE_FIPS) || FIPS_VERSION_GT(2,0)) +#if !defined(HAVE_SELFTEST) && (!defined(HAVE_FIPS) || FIPS_VERSION_GT(2,0)) ExpectIntEQ(EC_POINT_is_on_curve(group, new_point, ctx), 1); #endif diff --git a/wolfcrypt/src/ecc.c b/wolfcrypt/src/ecc.c index f879392939..3b73e08302 100644 --- a/wolfcrypt/src/ecc.c +++ b/wolfcrypt/src/ecc.c @@ -50,8 +50,6 @@ Possible ECC enable options: * SECP160K1 and SECP224K1. These do not work with scalars * that are the length of the order when the order is * longer than the prime. Use wc_ecc_fp_free to free cache. - * USE_ECC_B_PARAM: Enable ECC curve B param default: off - * (on for HAVE_COMP_KEY) * WOLFSSL_ECC_CURVE_STATIC: default off (on for windows) * For the ECC curve parameters `ecc_set_type` use fixed * array for hex string @@ -1522,9 +1520,7 @@ typedef struct ecc_curve_spec { mp_int* prime; mp_int* Af; - #ifdef USE_ECC_B_PARAM - mp_int* Bf; - #endif + mp_int* Bf; mp_int* order; mp_int* Gx; mp_int* Gy; @@ -1532,9 +1528,7 @@ typedef struct ecc_curve_spec { #ifdef ECC_CACHE_CURVE mp_int prime_lcl; mp_int Af_lcl; - #ifdef USE_ECC_B_PARAM - mp_int Bf_lcl; - #endif + mp_int Bf_lcl; mp_int order_lcl; mp_int Gx_lcl; mp_int Gy_lcl; @@ -1554,19 +1548,12 @@ typedef struct ecc_curve_spec { #define ECC_CURVE_FIELD_NONE 0x00 #define ECC_CURVE_FIELD_PRIME 0x01 #define ECC_CURVE_FIELD_AF 0x02 -#ifdef USE_ECC_B_PARAM #define ECC_CURVE_FIELD_BF 0x04 -#endif #define ECC_CURVE_FIELD_ORDER 0x08 #define ECC_CURVE_FIELD_GX 0x10 #define ECC_CURVE_FIELD_GY 0x20 -#ifdef USE_ECC_B_PARAM #define ECC_CURVE_FIELD_ALL 0x3F #define ECC_CURVE_FIELD_COUNT 6 -#else - #define ECC_CURVE_FIELD_ALL 0x3B - #define ECC_CURVE_FIELD_COUNT 5 -#endif #if defined(WOLFSSL_XILINX_CRYPT_VERSAL) static const u32 xil_curve_type[ECC_CURVE_MAX] = { @@ -1710,10 +1697,8 @@ static void wc_ecc_curve_cache_free_spec(ecc_curve_spec* curve) wc_ecc_curve_cache_free_spec_item(curve, curve->prime, ECC_CURVE_FIELD_PRIME); if (curve->load_mask & ECC_CURVE_FIELD_AF) wc_ecc_curve_cache_free_spec_item(curve, curve->Af, ECC_CURVE_FIELD_AF); -#ifdef USE_ECC_B_PARAM if (curve->load_mask & ECC_CURVE_FIELD_BF) wc_ecc_curve_cache_free_spec_item(curve, curve->Bf, ECC_CURVE_FIELD_BF); -#endif if (curve->load_mask & ECC_CURVE_FIELD_ORDER) wc_ecc_curve_cache_free_spec_item(curve, curve->order, ECC_CURVE_FIELD_ORDER); if (curve->load_mask & ECC_CURVE_FIELD_GX) @@ -1847,9 +1832,7 @@ static int wc_ecc_curve_load(const ecc_set_type* dp, ecc_curve_spec** pCurve, #ifdef ECC_CACHE_CURVE curve->prime = &curve->prime_lcl; curve->Af = &curve->Af_lcl; - #ifdef USE_ECC_B_PARAM - curve->Bf = &curve->Bf_lcl; - #endif + curve->Bf = &curve->Bf_lcl; curve->order = &curve->order_lcl; curve->Gx = &curve->Gx_lcl; curve->Gy = &curve->Gy_lcl; @@ -1868,11 +1851,9 @@ static int wc_ecc_curve_load(const ecc_set_type* dp, ecc_curve_spec** pCurve, if (load_items & ECC_CURVE_FIELD_AF) ret += wc_ecc_curve_cache_load_item(curve, dp->Af, &curve->Af, ECC_CURVE_FIELD_AF); -#ifdef USE_ECC_B_PARAM if (load_items & ECC_CURVE_FIELD_BF) ret += wc_ecc_curve_cache_load_item(curve, dp->Bf, &curve->Bf, ECC_CURVE_FIELD_BF); -#endif if (load_items & ECC_CURVE_FIELD_ORDER) ret += wc_ecc_curve_cache_load_item(curve, dp->order, &curve->order, ECC_CURVE_FIELD_ORDER); @@ -4762,6 +4743,7 @@ int wc_ecc_shared_secret(ecc_key* private_key, ecc_key* public_key, byte* out, return ECC_BAD_ARG_E; } + #if defined(WOLFSSL_ATECC508A) || defined(WOLFSSL_ATECC608A) /* For SECP256R1 use hardware */ if (private_key->dp->id == ECC_SECP256R1) { @@ -5274,7 +5256,6 @@ int wc_ecc_shared_secret_ex(ecc_key* private_key, ecc_point* point, #endif /* !WOLFSSL_ATECC508A && !WOLFSSL_CRYPTOCELL && !WOLFSSL_KCAPI_ECC */ #endif /* HAVE_ECC_DHE */ -#ifdef USE_ECC_B_PARAM /* Checks if a point p lies on the curve with index curve_idx */ int wc_ecc_point_is_on_curve(ecc_point *p, int curve_idx) { @@ -5309,7 +5290,6 @@ int wc_ecc_point_is_on_curve(ecc_point *p, int curve_idx) return err; } -#endif /* USE_ECC_B_PARAM */ #if !defined(WOLFSSL_ATECC508A) && !defined(WOLFSSL_ATECC608A) && \ !defined(WOLFSSL_CRYPTOCELL) && \ @@ -9969,8 +9949,6 @@ int wc_ecc_export_x963_ex(ecc_key* key, byte* out, word32* outLen, #endif /* HAVE_ECC_KEY_EXPORT */ -#ifdef HAVE_ECC_CHECK_PUBKEY_ORDER - /* is ecc point on curve described by dp ? */ static int _ecc_is_point(ecc_point* ecp, mp_int* a, mp_int* b, mp_int* prime) { @@ -10147,6 +10125,8 @@ int wc_ecc_is_point(ecc_point* ecp, mp_int* a, mp_int* b, mp_int* prime) return err; } +#ifdef HAVE_ECC_CHECK_PUBKEY_ORDER + #if (FIPS_VERSION_GE(5,0) || defined(WOLFSSL_VALIDATE_ECC_KEYGEN) || \ (defined(WOLFSSL_VALIDATE_ECC_IMPORT) && !defined(WOLFSSL_SP_MATH))) && \ !defined(WOLFSSL_KCAPI_ECC) || defined(WOLFSSL_CAAM) @@ -10514,14 +10494,7 @@ static int _ecc_validate_public_key(ecc_key* key, int partial, int priv) int err = MP_OKAY; #if defined(HAVE_ECC_CHECK_PUBKEY_ORDER) && !defined(WOLFSSL_SP_MATH) mp_int* b = NULL; - #ifdef USE_ECC_B_PARAM - DECLARE_CURVE_SPECS(4); - #else - #ifndef WOLFSSL_SMALL_STACK - mp_int b_lcl; - #endif - DECLARE_CURVE_SPECS(3); - #endif /* USE_ECC_B_PARAM */ + DECLARE_CURVE_SPECS(4); #endif ASSERT_SAVED_VECTOR_REGISTERS(); @@ -10573,21 +10546,7 @@ static int _ecc_validate_public_key(ecc_key* key, int partial, int priv) #endif #ifndef WOLFSSL_SP_MATH - #ifdef USE_ECC_B_PARAM - ALLOC_CURVE_SPECS(4, err); - #else - ALLOC_CURVE_SPECS(3, err); - #ifndef WOLFSSL_SMALL_STACK - b = &b_lcl; - #else - b = (mp_int*)XMALLOC(sizeof(mp_int), key->heap, DYNAMIC_TYPE_ECC); - if (b == NULL) { - FREE_CURVE_SPECS(); - return MEMORY_E; - } - #endif - XMEMSET(b, 0, sizeof(mp_int)); - #endif + ALLOC_CURVE_SPECS(4, err); #ifdef WOLFSSL_CAAM /* keys can be black encrypted ones which can not be checked like plain text @@ -10612,22 +10571,10 @@ static int _ecc_validate_public_key(ecc_key* key, int partial, int priv) /* load curve info */ if (err == MP_OKAY) err = wc_ecc_curve_load(key->dp, &curve, (ECC_CURVE_FIELD_PRIME | - ECC_CURVE_FIELD_AF | ECC_CURVE_FIELD_ORDER -#ifdef USE_ECC_B_PARAM - | ECC_CURVE_FIELD_BF -#endif - )); + ECC_CURVE_FIELD_AF | ECC_CURVE_FIELD_ORDER | ECC_CURVE_FIELD_BF)); -#ifndef USE_ECC_B_PARAM - /* load curve b parameter */ - if (err == MP_OKAY) - err = mp_init(b); - if (err == MP_OKAY) - err = mp_read_radix(b, key->dp->Bf, MP_RADIX_HEX); -#else if (err == MP_OKAY) b = curve->Bf; -#endif /* SP 800-56Ar3, section 5.6.2.3.3, process step 2 */ /* SP 800-56Ar3, section 5.6.2.3.4, process step 2 */ @@ -10684,11 +10631,6 @@ static int _ecc_validate_public_key(ecc_key* key, int partial, int priv) wc_ecc_curve_free(curve); -#ifndef USE_ECC_B_PARAM - mp_clear(b); - WC_FREE_VAR_EX(b, key->heap, DYNAMIC_TYPE_ECC); -#endif - FREE_CURVE_SPECS(); #else @@ -11012,16 +10954,75 @@ int wc_ecc_import_x963_ex2(const byte* in, word32 inLen, ecc_key* key, !defined(WOLFSSL_CRYPTOCELL) && \ (!defined(WOLF_CRYPTO_CB_ONLY_ECC) || defined(WOLFSSL_QNX_CAAM) || \ defined(WOLFSSL_IMXRT1170_CAAM)) - if (untrusted) { - /* Only do quick checks. */ - if ((err == MP_OKAY) && wc_ecc_point_is_at_infinity(&key->pubkey)) { + if ((err == MP_OKAY) && untrusted) { + /* Reject point at infinity. */ + if (wc_ecc_point_is_at_infinity(&key->pubkey)) { err = ECC_INF_E; } - #ifdef USE_ECC_B_PARAM - if ((err == MP_OKAY) && (key->idx != ECC_CUSTOM_IDX)) { + /* Verify the point lies on the curve (y^2 = x^3 + ax + b mod p) */ + if ((err == MP_OKAY) && (key->idx != ECC_CUSTOM_IDX)) { + #ifdef WOLFSSL_HAVE_SP_ECC + #ifndef WOLFSSL_SP_NO_256 + if (ecc_sets[key->idx].id == ECC_SECP256R1) { + err = sp_ecc_is_point_256(key->pubkey.x, key->pubkey.y); + #if defined(WOLFSSL_SM2) && defined(WOLFSSL_SP_SM2) + if (err != MP_OKAY && curve_id < 0) { + /* Retry with SM2 curve when P-256 returns invalid. + * Only when no explicit curve was requested (curve_id < 0). + * Needed because SM2 keys can be mis-identified as + * SECP256R1 during parsing. */ + err = sp_ecc_is_point_sm2_256(key->pubkey.x, + key->pubkey.y); + if (err == MP_OKAY) { + err = wc_ecc_set_curve(key, key->dp->size, + ECC_SM2P256V1); + } + } + #endif + } + else + #endif + #if defined(WOLFSSL_SM2) && defined(WOLFSSL_SP_SM2) + if (ecc_sets[key->idx].id == ECC_SM2P256V1) { + err = sp_ecc_is_point_sm2_256(key->pubkey.x, key->pubkey.y); + } + else + #endif + #ifdef WOLFSSL_SP_384 + if (ecc_sets[key->idx].id == ECC_SECP384R1) { + err = sp_ecc_is_point_384(key->pubkey.x, key->pubkey.y); + } + else + #endif + #ifdef WOLFSSL_SP_521 + if (ecc_sets[key->idx].id == ECC_SECP521R1) { + err = sp_ecc_is_point_521(key->pubkey.x, key->pubkey.y); + } + else + #endif + { + err = wc_ecc_point_is_on_curve(&key->pubkey, key->idx); + } + #else err = wc_ecc_point_is_on_curve(&key->pubkey, key->idx); + #if defined(WOLFSSL_SM2) + if (err != MP_OKAY && curve_id < 0) { + /* Retry with SM2 curve when P-256 returns invalid. + * Only when no explicit curve was requested (curve_id < 0). + * Needed because SM2 keys can be mis-identified as + * SECP256R1 during parsing. */ + int sm2_idx = wc_ecc_get_curve_idx(ECC_SM2P256V1); + if (sm2_idx != ECC_CURVE_INVALID) { + err = wc_ecc_point_is_on_curve(&key->pubkey, sm2_idx); + if (err == MP_OKAY) { + err = wc_ecc_set_curve(key, WOLFSSL_SM2_KEY_BITS / 8, + ECC_SM2P256V1); + } + } + } + #endif + #endif /* WOLFSSL_HAVE_SP_ECC */ } - #endif /* USE_ECC_B_PARAM */ } #endif (void)untrusted; @@ -11048,7 +11049,8 @@ int wc_ecc_import_x963_ex2(const byte* in, word32 inLen, ecc_key* key, int wc_ecc_import_x963_ex(const byte* in, word32 inLen, ecc_key* key, int curve_id) { - return wc_ecc_import_x963_ex2(in, inLen, key, curve_id, 0); + /* treat as untrusted: validate the point is on the curve */ + return wc_ecc_import_x963_ex2(in, inLen, key, curve_id, 1); } WOLFSSL_ABI diff --git a/wolfcrypt/src/evp.c b/wolfcrypt/src/evp.c index b802958a9e..c37363a324 100644 --- a/wolfcrypt/src/evp.c +++ b/wolfcrypt/src/evp.c @@ -3704,6 +3704,10 @@ int wolfSSL_EVP_PKEY_keygen_init(WOLFSSL_EVP_PKEY_CTX *ctx) return WOLFSSL_SUCCESS; } +#ifdef HAVE_ECC +static int ECC_populate_EVP_PKEY(WOLFSSL_EVP_PKEY* pkey, WOLFSSL_EC_KEY *key); +#endif + int wolfSSL_EVP_PKEY_keygen(WOLFSSL_EVP_PKEY_CTX *ctx, WOLFSSL_EVP_PKEY **ppkey) { @@ -3758,6 +3762,8 @@ int wolfSSL_EVP_PKEY_keygen(WOLFSSL_EVP_PKEY_CTX *ctx, ret = wolfSSL_EC_KEY_generate_key(pkey->ecc); if (ret == WOLFSSL_SUCCESS) { pkey->ownEcc = 1; + if (ECC_populate_EVP_PKEY(pkey, pkey->ecc) != WOLFSSL_SUCCESS) + ret = WOLFSSL_FAILURE; } } break; @@ -9516,7 +9522,15 @@ static int ECC_populate_EVP_PKEY(WOLFSSL_EVP_PKEY* pkey, WOLFSSL_EC_KEY *key) else #endif /* HAVE_PKCS8 */ { - if (ecc->type == ECC_PRIVATEKEY_ONLY) { + if (ecc->type == ECC_PRIVATEKEY_ONLY || + (ecc->type == ECC_PRIVATEKEY && + mp_iszero(ecc->pubkey.x))) { + /* Reconstruct public key from private scalar. This covers + * both ECC_PRIVATEKEY_ONLY keys and ECC_PRIVATEKEY keys whose + * public-key point was never populated (e.g. when only + * EC_KEY_set_private_key was called, SetECKeyInternal copies + * the zero-initialized pub_key point and marks the type as + * ECC_PRIVATEKEY, leaving pubkey.x == 0). */ if (wc_ecc_make_pub(ecc, NULL) != MP_OKAY) { return WOLFSSL_FAILURE; } diff --git a/wolfcrypt/test/test.c b/wolfcrypt/test/test.c index 755aa94358..d5e06fbed2 100644 --- a/wolfcrypt/test/test.c +++ b/wolfcrypt/test/test.c @@ -38221,6 +38221,7 @@ static wc_test_ret_t ecc_encrypt_e2e_test(WC_RNG* rng, ecc_key* userA, ecc_key* #ifdef WOLFSSL_ECIES_OLD tmpKey->dp = userA->dp; + tmpKey->idx = userA->idx; ret = wc_ecc_copy_point(&userA->pubkey, &tmpKey->pubkey); if (ret != 0) { ret = WC_TEST_RET_ENC_EC(ret); goto done; diff --git a/wolfssl/wolfcrypt/ecc.h b/wolfssl/wolfcrypt/ecc.h index 8f4c374f6d..3eb6454b8b 100644 --- a/wolfssl/wolfcrypt/ecc.h +++ b/wolfssl/wolfcrypt/ecc.h @@ -84,13 +84,6 @@ WOLFSSL_LOCAL int wolfCrypt_FIPS_ECC_sanity(void); #endif -/* Enable curve B parameter if needed */ -#if defined(HAVE_COMP_KEY) || defined(ECC_CACHE_CURVE) - #ifndef USE_ECC_B_PARAM /* Allow someone to force enable */ - #define USE_ECC_B_PARAM - #endif -#endif - /* Use this as the key->idx if a custom ecc_set is used for key->dp */ #define ECC_CUSTOM_IDX (-1) diff --git a/wolfssl/wolfcrypt/settings.h b/wolfssl/wolfcrypt/settings.h index a0c79c5e84..0208cd45a9 100644 --- a/wolfssl/wolfcrypt/settings.h +++ b/wolfssl/wolfcrypt/settings.h @@ -3149,14 +3149,16 @@ extern void uITRON4_free(void *p) ; #endif #endif /* HAVE_ECC */ -#if (defined(OPENSSL_EXTRA) || defined(OPENSSL_ALL)) && 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 +/* The FIPS-validated ecc.c gates wc_ecc_point_is_on_curve behind + * USE_ECC_B_PARAM. That guard was removed from the non-FIPS tree (the + * function is now unconditionally compiled in). Force-define the flag + * when building with any FIPS module so the certified file still + * provides the symbol. */ + #if defined(HAVE_FIPS) && !defined(USE_ECC_B_PARAM) + #define USE_ECC_B_PARAM #endif + /* Curve25519 Configs */ #ifdef HAVE_CURVE25519 /* By default enable shared secret, key export and import */