From 7cb79c7afb043e80eb9a4f7ff36f5a7a80cdd8c9 Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Thu, 4 Jun 2015 14:22:00 +0100 Subject: [PATCH] EC_POINT_is_on_curve does not return a boolean The function EC_POINT_is_on_curve does not return a boolean value. It returns 1 if the point is on the curve, 0 if it is not, and -1 on error. Many usages within OpenSSL were incorrectly using this function and therefore not correctly handling error conditions. With thanks to the Open Crypto Audit Project for reporting this issue. Reviewed-by: Kurt Roeckx (cherry picked from commit 68886be7e2cd395a759fcd41d2cede461b68843d) --- crypto/ec/ec2_oct.c | 2 +- crypto/ec/ec_check.c | 2 +- crypto/ec/ec_key.c | 2 +- crypto/ec/ec_lib.c | 7 +++++++ crypto/ec/ecp_oct.c | 2 +- crypto/ec/ectest.c | 24 ++++++++++++------------ 6 files changed, 23 insertions(+), 16 deletions(-) diff --git a/crypto/ec/ec2_oct.c b/crypto/ec/ec2_oct.c index c245d886da..0d04cc6923 100644 --- a/crypto/ec/ec2_oct.c +++ b/crypto/ec/ec2_oct.c @@ -387,7 +387,7 @@ int ec_GF2m_simple_oct2point(const EC_GROUP *group, EC_POINT *point, } /* test required by X9.62 */ - if (!EC_POINT_is_on_curve(group, point, ctx)) { + if (EC_POINT_is_on_curve(group, point, ctx) <= 0) { ECerr(EC_F_EC_GF2M_SIMPLE_OCT2POINT, EC_R_POINT_IS_NOT_ON_CURVE); goto err; } diff --git a/crypto/ec/ec_check.c b/crypto/ec/ec_check.c index d3f534999e..dd6f0ac409 100644 --- a/crypto/ec/ec_check.c +++ b/crypto/ec/ec_check.c @@ -85,7 +85,7 @@ int EC_GROUP_check(const EC_GROUP *group, BN_CTX *ctx) ECerr(EC_F_EC_GROUP_CHECK, EC_R_UNDEFINED_GENERATOR); goto err; } - if (!EC_POINT_is_on_curve(group, group->generator, ctx)) { + if (EC_POINT_is_on_curve(group, group->generator, ctx) <= 0) { ECerr(EC_F_EC_GROUP_CHECK, EC_R_POINT_IS_NOT_ON_CURVE); goto err; } diff --git a/crypto/ec/ec_key.c b/crypto/ec/ec_key.c index ebdffc8212..55ce3fe9be 100644 --- a/crypto/ec/ec_key.c +++ b/crypto/ec/ec_key.c @@ -314,7 +314,7 @@ int EC_KEY_check_key(const EC_KEY *eckey) goto err; /* testing whether the pub_key is on the elliptic curve */ - if (!EC_POINT_is_on_curve(eckey->group, eckey->pub_key, ctx)) { + if (EC_POINT_is_on_curve(eckey->group, eckey->pub_key, ctx) <= 0) { ECerr(EC_F_EC_KEY_CHECK_KEY, EC_R_POINT_IS_NOT_ON_CURVE); goto err; } diff --git a/crypto/ec/ec_lib.c b/crypto/ec/ec_lib.c index 6ffd9fc165..3ffa112cc3 100644 --- a/crypto/ec/ec_lib.c +++ b/crypto/ec/ec_lib.c @@ -970,6 +970,13 @@ int EC_POINT_is_at_infinity(const EC_GROUP *group, const EC_POINT *point) return group->meth->is_at_infinity(group, point); } +/* + * Check whether an EC_POINT is on the curve or not. Note that the return + * value for this function should NOT be treated as a boolean. Return values: + * 1: The point is on the curve + * 0: The point is not on the curve + * -1: An error occurred + */ int EC_POINT_is_on_curve(const EC_GROUP *group, const EC_POINT *point, BN_CTX *ctx) { diff --git a/crypto/ec/ecp_oct.c b/crypto/ec/ecp_oct.c index e5cec8be82..1bc3f39ad1 100644 --- a/crypto/ec/ecp_oct.c +++ b/crypto/ec/ecp_oct.c @@ -413,7 +413,7 @@ int ec_GFp_simple_oct2point(const EC_GROUP *group, EC_POINT *point, } /* test required by X9.62 */ - if (!EC_POINT_is_on_curve(group, point, ctx)) { + if (EC_POINT_is_on_curve(group, point, ctx) <= 0) { ECerr(EC_F_EC_GFP_SIMPLE_OCT2POINT, EC_R_POINT_IS_NOT_ON_CURVE); goto err; } diff --git a/crypto/ec/ectest.c b/crypto/ec/ectest.c index a18b32761a..fede530bc1 100644 --- a/crypto/ec/ectest.c +++ b/crypto/ec/ectest.c @@ -412,7 +412,7 @@ static void prime_field_tests(void) ABORT; if (!EC_POINT_set_compressed_coordinates_GFp(group, Q, x, 1, ctx)) ABORT; - if (!EC_POINT_is_on_curve(group, Q, ctx)) { + if (EC_POINT_is_on_curve(group, Q, ctx) <= 0) { if (!EC_POINT_get_affine_coordinates_GFp(group, Q, x, y, ctx)) ABORT; fprintf(stderr, "Point is not on curve: x = 0x"); @@ -544,7 +544,7 @@ static void prime_field_tests(void) ABORT; if (!EC_POINT_set_affine_coordinates_GFp(group, P, x, y, ctx)) ABORT; - if (!EC_POINT_is_on_curve(group, P, ctx)) + if (EC_POINT_is_on_curve(group, P, ctx) <= 0) ABORT; if (!BN_hex2bn(&z, "0100000000000000000001F4C8F927AED3CA752257")) ABORT; @@ -593,7 +593,7 @@ static void prime_field_tests(void) ABORT; if (!EC_POINT_set_compressed_coordinates_GFp(group, P, x, 1, ctx)) ABORT; - if (!EC_POINT_is_on_curve(group, P, ctx)) + if (EC_POINT_is_on_curve(group, P, ctx) <= 0) ABORT; if (!BN_hex2bn(&z, "FFFFFFFFFFFFFFFFFFFFFFFF99DEF836146BC9B1B4D22831")) ABORT; @@ -646,7 +646,7 @@ static void prime_field_tests(void) ABORT; if (!EC_POINT_set_compressed_coordinates_GFp(group, P, x, 0, ctx)) ABORT; - if (!EC_POINT_is_on_curve(group, P, ctx)) + if (EC_POINT_is_on_curve(group, P, ctx) <= 0) ABORT; if (!BN_hex2bn (&z, "FFFFFFFFFFFFFFFFFFFFFFFFFFFF16A2E0B8F03E13DD29455C5C2A3D")) @@ -705,7 +705,7 @@ static void prime_field_tests(void) ABORT; if (!EC_POINT_set_compressed_coordinates_GFp(group, P, x, 1, ctx)) ABORT; - if (!EC_POINT_is_on_curve(group, P, ctx)) + if (EC_POINT_is_on_curve(group, P, ctx) <= 0) ABORT; if (!BN_hex2bn(&z, "FFFFFFFF00000000FFFFFFFFFFFFFFFFBCE6FAADA7179E" "84F3B9CAC2FC632551")) @@ -761,7 +761,7 @@ static void prime_field_tests(void) ABORT; if (!EC_POINT_set_compressed_coordinates_GFp(group, P, x, 1, ctx)) ABORT; - if (!EC_POINT_is_on_curve(group, P, ctx)) + if (EC_POINT_is_on_curve(group, P, ctx) <= 0) ABORT; if (!BN_hex2bn(&z, "FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF" "FFC7634D81F4372DDF581A0DB248B0A77AECEC196ACCC52973")) @@ -820,7 +820,7 @@ static void prime_field_tests(void) ABORT; if (!EC_POINT_set_compressed_coordinates_GFp(group, P, x, 0, ctx)) ABORT; - if (!EC_POINT_is_on_curve(group, P, ctx)) + if (EC_POINT_is_on_curve(group, P, ctx) <= 0) ABORT; if (!BN_hex2bn(&z, "1FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF" "FFFFFFFFFFFFFFFFFFFFA51868783BF2F966B7FCC0148F709A5D03BB5" @@ -864,7 +864,7 @@ static void prime_field_tests(void) ABORT; if (!EC_POINT_dbl(group, P, P, ctx)) ABORT; - if (!EC_POINT_is_on_curve(group, P, ctx)) + if (EC_POINT_is_on_curve(group, P, ctx) <= 0) ABORT; if (!EC_POINT_invert(group, Q, ctx)) ABORT; /* P = -2Q */ @@ -1008,7 +1008,7 @@ static void prime_field_tests(void) # define CHAR2_CURVE_TEST_INTERNAL(_name, _p, _a, _b, _x, _y, _y_bit, _order, _cof, _degree, _variable) \ if (!BN_hex2bn(&x, _x)) ABORT; \ if (!EC_POINT_set_compressed_coordinates_GF2m(group, P, x, _y_bit, ctx)) ABORT; \ - if (!EC_POINT_is_on_curve(group, P, ctx)) ABORT; \ + if (EC_POINT_is_on_curve(group, P, ctx) <= 0) ABORT; \ if (!BN_hex2bn(&z, _order)) ABORT; \ if (!BN_hex2bn(&cof, _cof)) ABORT; \ if (!EC_GROUP_set_generator(group, P, z, cof)) ABORT; \ @@ -1026,7 +1026,7 @@ static void prime_field_tests(void) if (!BN_hex2bn(&x, _x)) ABORT; \ if (!BN_hex2bn(&y, _y)) ABORT; \ if (!EC_POINT_set_affine_coordinates_GF2m(group, P, x, y, ctx)) ABORT; \ - if (!EC_POINT_is_on_curve(group, P, ctx)) ABORT; \ + if (EC_POINT_is_on_curve(group, P, ctx) <= 0) ABORT; \ if (!BN_hex2bn(&z, _order)) ABORT; \ if (!BN_hex2bn(&cof, _cof)) ABORT; \ if (!EC_GROUP_set_generator(group, P, z, cof)) ABORT; \ @@ -1157,7 +1157,7 @@ static void char2_field_tests(void) if (!EC_POINT_set_affine_coordinates_GF2m(group, Q, x, y, ctx)) ABORT; # endif - if (!EC_POINT_is_on_curve(group, Q, ctx)) { + if (EC_POINT_is_on_curve(group, Q, ctx) <= 0) { /* Change test based on whether binary point compression is enabled or not. */ # ifdef OPENSSL_EC_BIN_PT_COMP if (!EC_POINT_get_affine_coordinates_GF2m(group, Q, x, y, ctx)) @@ -1378,7 +1378,7 @@ static void char2_field_tests(void) ABORT; if (!EC_POINT_dbl(group, P, P, ctx)) ABORT; - if (!EC_POINT_is_on_curve(group, P, ctx)) + if (EC_POINT_is_on_curve(group, P, ctx) <= 0) ABORT; if (!EC_POINT_invert(group, Q, ctx)) ABORT; /* P = -2Q */ -- 2.25.1