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 <kurt@openssl.org>

diff --git a/crypto/ec/ec2_oct.c b/crypto/ec/ec2_oct.c
index 821c3710096aeb70e3c2a86fd388e5e6c2ab7e43..33f703b168bf56e58a98b4c9a3393388a351fe6b 100644 (file)
--- a/crypto/ec/ec2_oct.c
+++ b/crypto/ec/ec2_oct.c
@@ -384,7 +384,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 1d44ad228321c8dcbbe5dcb5aaca8c447e04bccd..bdbf91c4707e4196b4c0e3b922cacfa11aca5759 100644 (file)
--- 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 07c33fe733db60152b429508e0ee21a15181bbe0..620860cad49788187083b3cae7481d2d408ce0c1 100644 (file)
--- a/crypto/ec/ec_key.c
+++ b/crypto/ec/ec_key.c
@@ -296,7 +296,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 9156943e200a0275a6ab5d5ca07f957cb2e93087..3ddaa5d8fba016d3bb8d353adb43f22047a2e882 100644 (file)
--- a/crypto/ec/ec_lib.c
+++ b/crypto/ec/ec_lib.c
@@ -949,6 +949,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 a68b559bc0f3859c9390be5ad6b5f0cfb1f1fe7c..8bb7aa30907694506238fe3665d80ee73645b932 100644 (file)
--- a/crypto/ec/ecp_oct.c
+++ b/crypto/ec/ecp_oct.c
@@ -410,7 +410,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/test/ectest.c b/test/ectest.c
index 977b8d969b5af41aeb6d395e868a31cbf3cc697f..c8893f9f2322bda2264b8bdc0e4fd743a231ee95 100644 (file)
--- a/test/ectest.c
+++ b/test/ectest.c
@@ -319,7 +319,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");
@@ -439,7 +439,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;
@@ -488,7 +488,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;
@@ -541,7 +541,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"))
@@ -600,7 +600,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"))
@@ -656,7 +656,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"))
@@ -715,7 +715,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"
@@ -759,7 +759,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 */
@@ -877,7 +877,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; \
@@ -895,7 +895,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; \
@@ -1024,7 +1024,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))
@@ -1245,7 +1245,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 */