Skip to content

Reject negative scalars in ECC point multiplication #12036

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
Frauschi wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Reject negative scalars in ECC point multiplication #12036

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions doc/dox_comments/header_files/ecc.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1027,6 +1027,7 @@ int wc_ecc_point_is_at_infinity(ecc_point *p);
\return MP_OKAY Returns on successful operation.
\return MP_INIT_E Returned if there is an error initializing an integer
for use with the multiple precision integer (mp_int) library.
\return ECC_OUT_OF_RANGE_E Returned if k is negative or too large.

\param k The multiplicand.
\param G Base point to multiply.
Expand Down
64 changes: 64 additions & 0 deletions src/pk_ec.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2484,6 +2484,57 @@ static int wolfssl_ec_point_mul(int curveIdx, ecc_point* r, mp_int* n,
return ret;
}

/* Reduce a scalar that is negative or wider than the prime modulo the number
* of points on the curve, as OpenSSL does.
*
* @param [in] group EC group.
* @param [in, out] s Scalar. May point to NULL. Set to *r when reduced.
* @param [out] r New BN holding the reduced scalar.
* @param [in, out] card Order times cofactor. Computed when first needed.
* @return 1 on success.
* @return 0 on error.
*/
static int ec_scalar_nnmod(const WOLFSSL_EC_GROUP* group,
const WOLFSSL_BIGNUM** s, WOLFSSL_BIGNUM** r, WOLFSSL_BIGNUM** card)
{
int ret = 1;

if ((*s == NULL) || ((!wolfSSL_BN_is_negative(*s)) &&
(wolfSSL_BN_num_bits(*s) <= wolfSSL_EC_GROUP_get_degree(group)))) {
return 1;
}

if (*card == NULL) {
*card = wolfSSL_BN_new();
if ((*card == NULL) ||
(wolfSSL_EC_GROUP_get_order(group, *card, NULL) != 1) ||
(wolfSSL_BN_mul_word(*card,
(WOLFSSL_BN_ULONG)ecc_sets[group->curve_idx].cofactor) !=
1)) {
WOLFSSL_MSG("Getting the number of points failed");
ret = 0;
}
}
if (ret == 1) {
*r = wolfSSL_BN_new();
if ((*r == NULL) || (wolfSSL_BN_mod(*r, *s, *card, NULL) != 1)) {
WOLFSSL_MSG("wolfSSL_BN_mod error");
ret = 0;
}
}
/* The remainder keeps the sign of s with some math libraries. */
if ((ret == 1) && wolfSSL_BN_is_negative(*r) &&
(wolfSSL_BN_add(*r, *r, *card) != 1)) {
WOLFSSL_MSG("wolfSSL_BN_add error");
ret = 0;
}
if (ret == 1) {
*s = *r;
}

return ret;
}

/* Sum the scalar multiplications of the base point and n, and q and m.
*
* r = base point * n + q * m
Expand All @@ -2504,6 +2555,9 @@ int wolfSSL_EC_POINT_mul(const WOLFSSL_EC_GROUP *group, WOLFSSL_EC_POINT *r,
WOLFSSL_BN_CTX *ctx)
{
int ret = 1;
WOLFSSL_BIGNUM* nr = NULL;
WOLFSSL_BIGNUM* mr = NULL;
WOLFSSL_BIGNUM* card = NULL;

/* No BN operations performed. */
(void)ctx;
Expand Down Expand Up @@ -2542,6 +2596,13 @@ int wolfSSL_EC_POINT_mul(const WOLFSSL_EC_GROUP *group, WOLFSSL_EC_POINT *r,
}
#endif

if (ret == 1) {
ret = ec_scalar_nnmod(group, &n, &nr, &card);
}
if (ret == 1) {
ret = ec_scalar_nnmod(group, &m, &mr, &card);
}

if (ret == 1) {
mp_int* ni = (n != NULL) ? (mp_int*)n->internal : NULL;
ecc_point* qi = (q != NULL) ? (ecc_point*)q->internal : NULL;
Expand All @@ -2568,6 +2629,9 @@ int wolfSSL_EC_POINT_mul(const WOLFSSL_EC_GROUP *group, WOLFSSL_EC_POINT *r,
}
#endif

wolfSSL_BN_free(card);
wolfSSL_BN_clear_free(mr);
wolfSSL_BN_clear_free(nr);
return ret;
}
#endif /* !WOLFSSL_ATECC508A && !WOLFSSL_ATECC608A && !HAVE_SELFTEST &&
Expand Down
104 changes: 104 additions & 0 deletions tests/api/test_ecc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4170,6 +4170,110 @@ int test_wc_ecc_mulmod_zero(void)
return EXPECT_RESULT();
} /* END test_wc_ecc_mulmod_zero */

#if defined(HAVE_ECC) && defined(WOLFSSL_PUBLIC_MP) && \
defined(WOLFSSL_PUBLIC_ECC_ADD_DBL) && \
!defined(NO_ECC256) && !defined(NO_ECC_SECP) && \
(!defined(ECC_MIN_KEY_SZ) || (ECC_MIN_KEY_SZ <= 256)) && \
!(defined(WOLFSSL_ATECC508A) || defined(WOLFSSL_ATECC608A) || \
defined(WOLFSSL_MICROCHIP_TA100) || \
defined(WOLFSSL_VALIDATE_ECC_IMPORT)) && \
!defined(WOLF_CRYPTO_CB_ONLY_ECC) && !defined(HAVE_SELFTEST) && \
!defined(WOLFSSL_KCAPI_ECC) && !defined(WOLFSSL_STM32_PKA) && \
!defined(FREESCALE_LTC_ECC) && \
!defined(HAVE_FIPS) && \
!defined(WOLFSSL_NO_ECC_SW) && (defined(USE_FAST_MATH) || \
defined(USE_INTEGER_HEAP_MATH) || defined(WOLFSSL_SP_INT_NEGATIVE))
#define TEST_ECC_MULMOD_NEGATIVE
#endif

/*
* Testing that scalar multiplication rejects a negative or oversized scalar.
*/
int test_wc_ecc_mulmod_negative(void)
{
EXPECT_DECLS;
#ifdef TEST_ECC_MULMOD_NEGATIVE
const ecc_set_type* dp = NULL;
ecc_key base;
ecc_key curve;
ecc_point* r = NULL;
mp_int neg;
mp_int five;
mp_int order;
mp_int wide;

XMEMSET(&base, 0, sizeof(ecc_key));
XMEMSET(&curve, 0, sizeof(ecc_key));
XMEMSET(&neg, 0, sizeof(mp_int));
XMEMSET(&five, 0, sizeof(mp_int));
XMEMSET(&order, 0, sizeof(mp_int));
XMEMSET(&wide, 0, sizeof(mp_int));

ExpectNotNull(dp = wc_ecc_get_curve_params(
wc_ecc_get_curve_idx(ECC_SECP256R1)));
ExpectIntEQ(wc_ecc_init(&base), 0);
ExpectIntEQ(wc_ecc_init(&curve), 0);
ExpectIntEQ(mp_init_multi(&neg, &five, &order, &wide, NULL, NULL), 0);
ExpectIntEQ(mp_read_radix(&neg, "-5", MP_RADIX_HEX), 0);
/* 2^258 */
ExpectIntEQ(mp_read_radix(&wide, "4"
"0000000000000000000000000000000000000000000000000000000000000000",
MP_RADIX_HEX), 0);
ExpectIntEQ(mp_set(&five, 5), 0);
ExpectNotNull(r = wc_ecc_new_point());

if (dp != NULL) {
ExpectIntEQ(wc_ecc_import_raw_ex(&base, dp->Gx, dp->Gy, NULL,
ECC_SECP256R1), 0);
ExpectIntEQ(wc_ecc_import_raw_ex(&curve, dp->Af, dp->prime, NULL,
ECC_SECP256R1), 0);
ExpectIntEQ(mp_read_radix(&order, dp->order, MP_RADIX_HEX), 0);
}
ExpectTrue(mp_isneg(&neg));

ExpectIntEQ(wc_ecc_mulmod(&neg, &base.pubkey, r, curve.pubkey.x,
curve.pubkey.y, 1), WC_NO_ERR_TRACE(ECC_OUT_OF_RANGE_E));
ExpectIntEQ(wc_ecc_mulmod_ex(&neg, &base.pubkey, r, curve.pubkey.x,
curve.pubkey.y, 0, NULL), WC_NO_ERR_TRACE(ECC_OUT_OF_RANGE_E));
ExpectIntEQ(wc_ecc_mulmod_ex2(&neg, &base.pubkey, r, curve.pubkey.x,
curve.pubkey.y, &order, NULL, 1, NULL),
WC_NO_ERR_TRACE(ECC_OUT_OF_RANGE_E));
ExpectIntEQ(wc_ecc_mulmod(&wide, &base.pubkey, r, curve.pubkey.x,
curve.pubkey.y, 1), WC_NO_ERR_TRACE(ECC_OUT_OF_RANGE_E));
ExpectIntEQ(wc_ecc_mulmod_ex2(&wide, &base.pubkey, r, curve.pubkey.x,
curve.pubkey.y, &order, NULL, 1, NULL),
WC_NO_ERR_TRACE(ECC_OUT_OF_RANGE_E));
#if defined(ECC_SHAMIR) && !defined(WOLFSSL_SP_MATH)
ExpectIntEQ(ecc_mul2add(&base.pubkey, &neg, &base.pubkey, &five, r,
curve.pubkey.x, curve.pubkey.y, NULL),
WC_NO_ERR_TRACE(ECC_OUT_OF_RANGE_E));
ExpectIntEQ(ecc_mul2add(&base.pubkey, &five, &base.pubkey, &neg, r,
curve.pubkey.x, curve.pubkey.y, NULL),
WC_NO_ERR_TRACE(ECC_OUT_OF_RANGE_E));
ExpectIntEQ(ecc_mul2add(&base.pubkey, NULL, &base.pubkey, &five, r,
curve.pubkey.x, curve.pubkey.y, NULL),
WC_NO_ERR_TRACE(ECC_BAD_ARG_E));
ExpectIntEQ(ecc_mul2add(&base.pubkey, &five, &base.pubkey, &five, r,
curve.pubkey.x, curve.pubkey.y, NULL), 0);
#endif
/* Positive scalars are still accepted. */
ExpectIntEQ(wc_ecc_mulmod(&five, &base.pubkey, r, curve.pubkey.x,
curve.pubkey.y, 1), 0);

wc_ecc_del_point(r);
mp_free(&wide);
mp_free(&order);
mp_free(&five);
mp_free(&neg);
wc_ecc_free(&curve);
wc_ecc_free(&base);
#ifdef FP_ECC
wc_ecc_fp_free();
#endif
#endif
return EXPECT_RESULT();
} /* END test_wc_ecc_mulmod_negative */

/*
* Testing that wc_ecc_mulmod() of the private key and the base point gives
* the public key, on every curve compiled in. This covers curves the SP code
Expand Down
2 changes: 2 additions & 0 deletions tests/api/test_ecc.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ int test_wc_ecc_verify_hash_ex(void);
int test_wc_ecc_mulmod(void);
int test_wc_ecc_mulmod_unreduced(void);
int test_wc_ecc_mulmod_zero(void);
int test_wc_ecc_mulmod_negative(void);
int test_wc_ecc_mulmod_curves(void);
int test_wc_ecc_is_valid_idx(void);
int test_wc_ecc_get_curve_id_from_oid(void);
Expand Down Expand Up @@ -140,6 +141,7 @@ int test_wc_ecc_make_key_id_dev(void);
TEST_DECL_GROUP("ecc", test_wc_ecc_mulmod), \
TEST_DECL_GROUP("ecc", test_wc_ecc_mulmod_unreduced), \
TEST_DECL_GROUP("ecc", test_wc_ecc_mulmod_zero), \
TEST_DECL_GROUP("ecc", test_wc_ecc_mulmod_negative), \
TEST_DECL_GROUP("ecc", test_wc_ecc_mulmod_curves), \
TEST_DECL_GROUP("ecc", test_wc_ecc_is_valid_idx), \
TEST_DECL_GROUP("ecc", test_wc_ecc_get_curve_id_from_oid), \
Expand Down
90 changes: 90 additions & 0 deletions tests/api/test_ossl_ec.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -897,6 +897,96 @@ int test_wolfSSL_EC_POINT(void)
return EXPECT_RESULT();
}

/* EC_POINT_mul() reduces a negative or over-wide scalar modulo the order, as
* OpenSSL does. */
int test_wolfSSL_EC_POINT_mul_negative(void)
{
EXPECT_DECLS;
#if defined(OPENSSL_EXTRA) && !defined(WOLFSSL_SP_MATH) && \
(!defined(WOLFSSL_SP_MATH_ALL) || defined(WOLFSSL_SP_INT_NEGATIVE)) && \
!defined(WOLFSSL_ATECC508A) && !defined(WOLFSSL_ATECC608A) && \
!defined(WOLFSSL_MICROCHIP_TA100) && !defined(HAVE_SELFTEST) && \
!defined(WOLF_CRYPTO_CB_ONLY_ECC) && !defined(WOLFSSL_NO_ECC_SW) && \
(!defined(HAVE_FIPS) || FIPS_VERSION_GE(7,0)) && !defined(NO_ECC256) && \
!defined(NO_ECC_SECP) && \
(!defined(ECC_MIN_KEY_SZ) || (ECC_MIN_KEY_SZ <= 256))
EC_GROUP* group = NULL;
EC_POINT* g = NULL;
EC_POINT* r1 = NULL;
EC_POINT* r2 = NULL;
BIGNUM* order = NULL;
BIGNUM* neg = NULL;
BIGNUM* pos = NULL;

ExpectNotNull(group = EC_GROUP_new_by_curve_name(NID_X9_62_prime256v1));
ExpectNotNull(g = EC_POINT_new(group));
ExpectNotNull(r1 = EC_POINT_new(group));
ExpectNotNull(r2 = EC_POINT_new(group));
ExpectNotNull(order = BN_new());
ExpectNotNull(neg = BN_new());
ExpectNotNull(pos = BN_new());
ExpectIntEQ(EC_GROUP_get_order(group, order, NULL), 1);
ExpectIntEQ(EC_POINT_mul(group, g, BN_value_one(), NULL, NULL, NULL), 1);

/* -5 and order - 5 */
ExpectIntGT(BN_hex2bn(&neg, "-5"), 0);
ExpectIntEQ(BN_is_negative(neg), 1);
ExpectIntEQ(BN_add(pos, order, neg), 1);

ExpectIntEQ(EC_POINT_mul(group, r1, neg, NULL, NULL, NULL), 1);
ExpectIntEQ(EC_POINT_mul(group, r2, pos, NULL, NULL, NULL), 1);
ExpectIntEQ(EC_POINT_cmp(group, r1, r2, NULL), 0);

ExpectIntEQ(EC_POINT_mul(group, r1, NULL, g, neg, NULL), 1);
ExpectIntEQ(EC_POINT_mul(group, r2, NULL, g, pos, NULL), 1);
ExpectIntEQ(EC_POINT_cmp(group, r1, r2, NULL), 0);

ExpectIntEQ(EC_POINT_mul(group, r1, neg, g, BN_value_one(), NULL), 1);
ExpectIntEQ(EC_POINT_mul(group, r2, pos, g, BN_value_one(), NULL), 1);
ExpectIntEQ(EC_POINT_cmp(group, r1, r2, NULL), 0);

ExpectIntEQ(EC_POINT_mul(group, r1, BN_value_one(), g, neg, NULL), 1);
ExpectIntEQ(EC_POINT_cmp(group, r1, r2, NULL), 0);

/* The caller's scalar is left as it was. */
ExpectIntEQ(BN_is_negative(neg), 1);

/* -order is 0 modulo the order. */
BN_zero(neg);
ExpectIntEQ(BN_sub(neg, neg, order), 1);
ExpectIntEQ(EC_POINT_mul(group, r1, neg, NULL, NULL, NULL), 1);
ExpectIntEQ(EC_POINT_is_at_infinity(group, r1), 1);

/* order * 2^64 + 5 is 5 modulo the order. */
ExpectIntEQ(BN_lshift(pos, order, 64), 1);
ExpectIntEQ(BN_add_word(pos, 5), 1);
ExpectIntEQ(BN_set_word(neg, 5), 1);
ExpectIntEQ(EC_POINT_mul(group, r1, pos, NULL, NULL, NULL), 1);
ExpectIntEQ(EC_POINT_mul(group, r2, neg, NULL, NULL, NULL), 1);
ExpectIntEQ(EC_POINT_cmp(group, r1, r2, NULL), 0);
ExpectIntEQ(EC_POINT_mul(group, r1, NULL, g, pos, NULL), 1);
ExpectIntEQ(EC_POINT_cmp(group, r1, r2, NULL), 0);

/* 2^256 + 5 is one bit wider than the prime. */
ExpectIntEQ(BN_set_word(pos, 1), 1);
ExpectIntEQ(BN_lshift(pos, pos, 256), 1);
ExpectIntEQ(BN_add_word(pos, 5), 1);
ExpectIntEQ(BN_mod(neg, pos, order, NULL), 1);
ExpectIntEQ(EC_POINT_mul(group, r1, pos, NULL, NULL, NULL), 1);
ExpectIntEQ(EC_POINT_mul(group, r2, neg, NULL, NULL, NULL), 1);
ExpectIntEQ(EC_POINT_cmp(group, r1, r2, NULL), 0);

BN_free(pos);
BN_free(neg);
BN_free(order);
EC_POINT_free(r2);
EC_POINT_free(r1);
EC_POINT_free(g);
EC_GROUP_free(group);
#endif
return EXPECT_RESULT();
}

int test_wolfSSL_SPAKE(void)
{
EXPECT_DECLS;
Expand Down
2 changes: 2 additions & 0 deletions tests/api/test_ossl_ec.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ int test_wolfSSL_EC_GROUP(void);
int test_wolfSSL_PEM_read_bio_ECPKParameters(void);
int test_wolfSSL_i2d_ECPKParameters(void);
int test_wolfSSL_EC_POINT(void);
int test_wolfSSL_EC_POINT_mul_negative(void);
int test_wolfSSL_SPAKE(void);
int test_wolfSSL_EC_KEY_generate(void);
int test_EC_i2d(void);
Expand All @@ -54,6 +55,7 @@ int test_wolfSSL_EC_POINT_point2hex_oversized(void);
TEST_DECL_GROUP("ossl_ec", test_wolfSSL_PEM_read_bio_ECPKParameters), \
TEST_DECL_GROUP("ossl_ec", test_wolfSSL_i2d_ECPKParameters), \
TEST_DECL_GROUP("ossl_ec", test_wolfSSL_EC_POINT), \
TEST_DECL_GROUP("ossl_ec", test_wolfSSL_EC_POINT_mul_negative), \
TEST_DECL_GROUP("ossl_ec", test_wolfSSL_SPAKE), \
TEST_DECL_GROUP("ossl_ec", test_wolfSSL_EC_KEY_generate), \
TEST_DECL_GROUP("ossl_ec", test_EC_i2d), \
Expand Down
Loading
Loading