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>
(cherry picked from commit 68886be7e2cd395a759fcd41d2cede461b68843d)

Conflicts:
	crypto/ec/ec2_oct.c
	crypto/ec/ecp_oct.c
	crypto/ec/ectest.c

diff --git a/crypto/ec/ec2_smpl.c b/crypto/ec/ec2_smpl.c
index 5df41e2660859d685ff95c2f7c59f0a1b6cfcb7d..df37571bd1952bcf8671e893334fdd3568f7a254 100644 (file)
--- a/crypto/ec/ec2_smpl.c
+++ b/crypto/ec/ec2_smpl.c
@@ -677,7 +677,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 d3f534999e067cdf641ab2dd7764b2902ba641b7..dd6f0ac409942d3a6fb8d3d5d67293697f2b3eb6 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 7e480151dbbcd1903ffa138c61dba1f334cd437e..6104a597cba67d9d96996524a1fd8923c11442c6 100644 (file)
--- a/crypto/ec/ec_key.c
+++ b/crypto/ec/ec_key.c
@@ -304,7 +304,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 8d8b807a0f4c4d9988fcee208d0dcead246fd9b4..33f397b6065ff51f716c4ee33e97630adb435504 100644 (file)
--- a/crypto/ec/ec_lib.c
+++ b/crypto/ec/ec_lib.c
@@ -993,6 +993,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_smpl.c b/crypto/ec/ecp_smpl.c
index a0c1540c459ae4578412131030ef38da49a1febf..cc924bd713a7daed1bf73389328427702a1696da 100644 (file)
--- a/crypto/ec/ecp_smpl.c
+++ b/crypto/ec/ecp_smpl.c
@@ -985,7 +985,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 2df7f338f8c93bf25b124933e5f36052f6380da4..0fa769712c798b03256828fbdda8a1e1a37b99c2 100644 (file)
--- a/crypto/ec/ectest.c
+++ b/crypto/ec/ectest.c
@@ -303,7 +303,7 @@ void prime_field_tests()
         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");
@@ -436,7 +436,7 @@ void prime_field_tests()
         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;
@@ -501,7 +501,7 @@ void prime_field_tests()
         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;
@@ -572,7 +572,7 @@ void prime_field_tests()
         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"))
@@ -649,7 +649,7 @@ void prime_field_tests()
         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"))
@@ -723,7 +723,7 @@ void prime_field_tests()
         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"))
@@ -800,7 +800,7 @@ void prime_field_tests()
         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"
@@ -862,7 +862,7 @@ void prime_field_tests()
         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 */
@@ -1004,7 +1004,7 @@ void prime_field_tests()
 #  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; \
@@ -1022,7 +1022,7 @@ void prime_field_tests()
         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; \
@@ -1161,7 +1161,7 @@ void char2_field_tests()
     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))
@@ -1382,7 +1382,7 @@ void char2_field_tests()
         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 */