Skip to content

F-4287: check public point is on curve #461

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
padelsbach wants to merge 2 commits into
base: master
Choose a base branch
from
Open

F-4287: check public point is on curve #461

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
201eb0f
F-4287: check public point is on curve
padelsbach Aug 7, 2026
849dbeb
Fix test issue
padelsbach Aug 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions include/wolfprovider/alg_funcs.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,8 @@ ecc_key* wp_ecc_get_key(wp_Ecc* ecc);
WC_RNG* wp_ecc_get_rng(wp_Ecc* ecc);
int wp_ecc_get_size(wp_Ecc* ecc);
int wp_ecc_check_usage(wp_Ecc* ecc);
int wp_ecc_check_pub_point(wp_Ecc* ecc);
int wp_ecc_check_pub_key(wp_Ecc* ecc);
wolfSSL_Mutex* wp_ecc_get_mutex(wp_Ecc* ecc);

/* Internal ECX types and functions. */
Expand Down
166 changes: 107 additions & 59 deletions src/wp_ecc_kmgmt.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,110 @@ int wp_ecc_check_usage(wp_Ecc* ecc)
return ret;
}

/**
* Check a point is usable with a curve.
*
* Rejects the point at infinity and, where wolfSSL provides the check, any
* point that is not on the curve. A point off the curve lies on a group whose
* order may be small enough to recover the other party's private key from a
* key exchange.
*
* @param [in] point Public key point.
* @param [in] curveId wolfSSL identifier of curve point must be on.
* @return 1 on success.
* @return 0 on failure.
*/
static int wp_ecc_check_point(ecc_point* point, int curveId)
{
int ok = 1;
int idx = ECC_CURVE_INVALID;
#ifdef USE_ECC_B_PARAM
int rc;
#endif

WOLFPROV_ENTER(WP_LOG_COMP_ECC, "wp_ecc_check_point");

idx = wc_ecc_get_curve_idx(curveId);
if (idx == ECC_CURVE_INVALID) {
ok = 0;
}
if (ok && wc_ecc_point_is_at_infinity(point)) {
ok = 0;
}
#ifdef USE_ECC_B_PARAM
/* wolfSSL only builds the on-curve check when the curve b parameter is
* available. */
if (ok) {
rc = wc_ecc_point_is_on_curve(point, idx);
if (rc != 0) {
WOLFPROV_MSG_DEBUG_RETCODE(WP_LOG_LEVEL_DEBUG,
"wc_ecc_point_is_on_curve", rc);
ok = 0;
}
}
#endif

WOLFPROV_LEAVE(WP_LOG_COMP_ECC, __FILE__ ":" WOLFPROV_STRINGIZE(__LINE__), ok);
return ok;
}

/**
* Check the public key point is usable with the key's curve.
*
* @param [in] ecc ECC key object.
* @return 1 on success.
* @return 0 on failure.
*/
int wp_ecc_check_pub_point(wp_Ecc* ecc)
{
return (ecc != NULL) && wp_ecc_check_point(&ecc->key.pubkey, ecc->curveId);
}

/**
* Check the public key is valid for use with the key's curve.
*
* As well as the point being on the curve, the point must have the order of
* the curve. Any private key held alongside is ignored.
*
* @param [in] ecc ECC key object.
* @return 1 on success.
* @return 0 on failure.
*/
int wp_ecc_check_pub_key(wp_Ecc* ecc)
{
int ok = 1;
int origType;
int rc;

WOLFPROV_ENTER(WP_LOG_COMP_ECC, "wp_ecc_check_pub_key");

if ((ecc == NULL) || (!ecc->hasPub)) {
ok = 0;
}
/* Fail closed if the key mutex can't be held for the check. */
if (ok && (wp_lock(wp_ecc_get_mutex(ecc)) != 1)) {
ok = 0;
}
if (ok) {
/* A private key may be present that does not match the public key,
* which is OK. Override the internal type to force a public key only
* check. */
origType = ecc->key.type;
ecc->key.type = ECC_PUBLICKEY;
rc = wc_ecc_check_key(&ecc->key);
ecc->key.type = origType;
wp_unlock(wp_ecc_get_mutex(ecc));
if (rc != 0) {
WOLFPROV_MSG_DEBUG_RETCODE(WP_LOG_LEVEL_DEBUG, "wc_ecc_check_key",
rc);
ok = 0;
}
}

WOLFPROV_LEAVE(WP_LOG_COMP_ECC, __FILE__ ":" WOLFPROV_STRINGIZE(__LINE__), ok);
return ok;
}

/*
* ECC key
*/
Expand Down Expand Up @@ -1134,37 +1238,6 @@ static int wp_ecc_match(wp_Ecc* ecc1, wp_Ecc* ecc2, int selection)
return ok;
}

#if LIBWOLFSSL_VERSION_HEX >= 0x05000000
/**
* Quick validate the ECC public key.
*
* Check for infinity and point is on curve.
*
* @param [in] ecc ECC key object.
* @return 1 on success.
* @return 0 on failure.
*/
static int wp_ecc_validate_public_key_quick(const wp_Ecc* ecc)
{
int ok = 1;

WOLFPROV_ENTER(WP_LOG_COMP_ECC, "wp_ecc_validate_public_key_quick");

if (wc_ecc_point_is_at_infinity((ecc_point*)&ecc->key.pubkey)) {
ok = 0;
}
#ifdef USE_ECC_B_PARAM
if (ok && (!wc_ecc_point_is_on_curve((ecc_point*)&ecc->key.pubkey,
ecc->curveId))) {
ok = 0;
}
#endif

WOLFPROV_LEAVE(WP_LOG_COMP_ECC, __FILE__ ":" WOLFPROV_STRINGIZE(__LINE__), ok);
return ok;
}
#endif

/**
* Validate the ECC key.
*
Expand All @@ -1179,7 +1252,6 @@ static int wp_ecc_validate_public_key_quick(const wp_Ecc* ecc)
static int wp_ecc_validate(const wp_Ecc* ecc, int selection, int checkType)
{
int ok = 1;
int origType;
int rc;

WOLFPROV_ENTER(WP_LOG_COMP_ECC, "wp_ecc_validate");
Expand All @@ -1193,37 +1265,13 @@ static int wp_ecc_validate(const wp_Ecc* ecc, int selection, int checkType)
ok = 0;
}
if (ok && (selection & OSSL_KEYMGMT_SELECT_PUBLIC_KEY) != 0) {
#if LIBWOLFSSL_VERSION_HEX >= 0x05000000
/* TODO: Quick check for older versions? */
if (checkType == OSSL_KEYMGMT_VALIDATE_QUICK_CHECK) {
if (!wp_ecc_validate_public_key_quick(ecc)) {
if (!wp_ecc_check_pub_point((wp_Ecc*)ecc)) {
ok = 0;
}
}
else
#else
(void)checkType;
#endif
{
/* Fail closed if the key mutex can't be held for the check. */
if (wp_lock(wp_ecc_get_mutex((wp_Ecc*)ecc)) != 1) {
ok = 0;
}
if (ok) {
/* We may have a private key inside that does not match the
* public key that has been set, which is OK. Override the
* internal type to force a public key only check */
origType = ecc->key.type;
((wp_Ecc*)ecc)->key.type = ECC_PUBLICKEY;
rc = wc_ecc_check_key((ecc_key*)&ecc->key);
((wp_Ecc*)ecc)->key.type = origType;
wp_unlock(wp_ecc_get_mutex((wp_Ecc*)ecc));
if (rc != 0) {
WOLFPROV_MSG_DEBUG_RETCODE(WP_LOG_LEVEL_DEBUG,
"wc_ecc_check_key", rc);
ok = 0;
}
}
else if (!wp_ecc_check_pub_key((wp_Ecc*)ecc)) {
ok = 0;
}
}
if (ok && ((selection & OSSL_KEYMGMT_SELECT_PRIVATE_KEY) != 0) &&
Expand Down
24 changes: 20 additions & 4 deletions src/wp_ecdh_exch.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -281,12 +281,22 @@ static int wp_ecdh_derive_secret(wp_EcdhCtx* ctx, unsigned char* secret,

WOLFPROV_ENTER(WP_LOG_COMP_ECDH, "wp_ecdh_derive_secret");

if (ctx->peer == NULL) {
ok = 0;
}
#ifdef HAVE_ECC_CDH
if (ctx->cofactor) {
if (ok && ctx->cofactor) {
wc_ecc_set_flags(wp_ecc_get_key(ctx->key), WC_ECC_FLAG_COFACTOR);
}
#endif
if ((ok = wp_ecc_check_usage(ctx->key))) {
if (ok && (!wp_ecc_check_usage(ctx->key))) {
ok = 0;
}
/* Peer's key may have been changed since it was set - check it again. */
if (ok && (!wp_ecc_check_pub_point(ctx->peer))) {
ok = 0;
}
if (ok) {
/* Calculate secret. */
PRIVATE_KEY_UNLOCK();
rc = wc_ecc_shared_secret(wp_ecc_get_key(ctx->key),
Expand Down Expand Up @@ -325,13 +335,16 @@ static int wp_ecdh_derive(wp_EcdhCtx* ctx, unsigned char* secret,
unsigned char* out;
size_t outLen;
unsigned char* tmp = NULL;
size_t maxLen = (size_t)wp_ecc_get_size(ctx->key);
size_t maxLen = 0;

WOLFPROV_ENTER(WP_LOG_COMP_ECDH, "wp_ecdh_derive");

if (!wolfssl_prov_is_running()) {
if ((!wolfssl_prov_is_running()) || (ctx->key == NULL)) {
ok = 0;
}
if (ok) {
maxLen = (size_t)wp_ecc_get_size(ctx->key);
}

/* No output buffer, return maximum size only. */
if (ok && (secret == NULL)) {
Expand Down Expand Up @@ -405,6 +418,9 @@ static int wp_ecdh_set_peer(wp_EcdhCtx* ctx, wp_Ecc* peer)
if (!wolfssl_prov_is_running()) {
ok = 0;
}
if (ok && (!wp_ecc_check_pub_key(peer))) {
ok = 0;
}
if (ok && (ctx->peer != peer)) {
/* Dispose of the old peer and up reference ECC key. */
wp_ecc_free(ctx->peer);
Expand Down
Loading
Loading