File 0001-RT-4242-reject-invalid-EC-point-coordinate... of Package openssl

Overview Repositories Revisions Requests Users Attributes Meta

File 0001-RT-4242-reject-invalid-EC-point-coordinates.patch of Package openssl (Revision ab9e1b647facbbf65a6dc99d207b992a)

Currently displaying revision ab9e1b647facbbf65a6dc99d207b992a , Show latest

From 1e2012b7ff4a5f12273446b281775faa5c8a1858 Mon Sep 17 00:00:00 2001
From: Emilia Kasper <emilia@openssl.org>
Date: Fri, 3 Jun 2016 14:42:04 +0200
Subject: [PATCH] RT 4242: reject invalid EC point coordinates

We already test in EC_POINT_oct2point that points are on the curve. To
be on the safe side, move this check to
EC_POINT_set_affine_coordinates_* so as to also check point coordinates
received through some other method.

We do not check projective coordinates, though, as
- it's unlikely that applications would be receiving this primarily
  internal representation from untrusted sources, and
- it's possible that the projective setters are used in a setting where
  performance matters.

Reviewed-by: Rich Salz <rsalz@openssl.org>
---
 crypto/ec/ec2_oct.c | 10 ++---
 crypto/ec/ec_lib.c  | 20 +++++++++-
 crypto/ec/ecp_oct.c | 10 ++---
 test/ectest.c       | 96 ++++++++++++++++++++++++++++++++++++++++++---
 4 files changed, 116 insertions(+), 20 deletions(-)

Index: openssl-1.0.1i/crypto/ec/ec2_oct.c
===================================================================
--- openssl-1.0.1i.orig/crypto/ec/ec2_oct.c	2019-04-10 14:47:47.514494796 +0200
+++ openssl-1.0.1i/crypto/ec/ec2_oct.c	2019-04-10 14:49:00.002915539 +0200
@@ -387,15 +387,13 @@ int ec_GF2m_simple_oct2point(const EC_GR
 				}
 			}
 
+        /*
+         * EC_POINT_set_affine_coordinates_GF2m is responsible for checking that
+         * the point is on the curve.
+         */
 		if (!EC_POINT_set_affine_coordinates_GF2m(group, point, x, y, ctx)) goto err;
 		}
 	
-	if (!EC_POINT_is_on_curve(group, point, ctx)) /* test required by X9.62 */
-		{
-		ECerr(EC_F_EC_GF2M_SIMPLE_OCT2POINT, EC_R_POINT_IS_NOT_ON_CURVE);
-		goto err;
-		}
-
 	ret = 1;
 	
  err:
Index: openssl-1.0.1i/crypto/ec/ecp_oct.c
===================================================================
--- openssl-1.0.1i.orig/crypto/ec/ecp_oct.c	2019-04-10 14:47:47.518494819 +0200
+++ openssl-1.0.1i/crypto/ec/ecp_oct.c	2019-04-10 14:49:30.107090272 +0200
@@ -413,15 +413,13 @@ int ec_GFp_simple_oct2point(const EC_GRO
 				}
 			}
 
+        /*
+         * EC_POINT_set_affine_coordinates_GFp is responsible for checking that
+         * the point is on the curve.
+         */
 		if (!EC_POINT_set_affine_coordinates_GFp(group, point, x, y, ctx)) goto err;
 		}
 	
-	if (!EC_POINT_is_on_curve(group, point, ctx)) /* test required by X9.62 */
-		{
-		ECerr(EC_F_EC_GFP_SIMPLE_OCT2POINT, EC_R_POINT_IS_NOT_ON_CURVE);
-		goto err;
-		}
-
 	ret = 1;
 	
  err:
Index: openssl-1.0.1i/crypto/ec/ec_lib.c
===================================================================
--- openssl-1.0.1i.orig/crypto/ec/ec_lib.c	2019-04-10 14:47:47.518494819 +0200
+++ openssl-1.0.1i/crypto/ec/ec_lib.c	2019-04-10 14:57:07.397694417 +0200
@@ -853,7 +853,15 @@ int EC_POINT_set_affine_coordinates_GFp(
 		ECerr(EC_F_EC_POINT_SET_AFFINE_COORDINATES_GFP, EC_R_INCOMPATIBLE_OBJECTS);
 		return 0;
 		}
-	return group->meth->point_set_affine_coordinates(group, point, x, y, ctx);
+    if (!group->meth->point_set_affine_coordinates(group, point, x, y, ctx))
+        return 0;
+
+    if (EC_POINT_is_on_curve(group, point, ctx) <= 0) {
+        ECerr(EC_F_EC_POINT_SET_AFFINE_COORDINATES_GFP,
+              EC_R_POINT_IS_NOT_ON_CURVE);
+        return 0;
+    }
+    return 1;
 	}
 
 #ifndef OPENSSL_NO_EC2M
@@ -870,7 +878,15 @@ int EC_POINT_set_affine_coordinates_GF2m
 		ECerr(EC_F_EC_POINT_SET_AFFINE_COORDINATES_GF2M, EC_R_INCOMPATIBLE_OBJECTS);
 		return 0;
 		}
-	return group->meth->point_set_affine_coordinates(group, point, x, y, ctx);
+    if (!group->meth->point_set_affine_coordinates(group, point, x, y, ctx))
+        return 0;
+
+    if (EC_POINT_is_on_curve(group, point, ctx) <= 0) {
+        ECerr(EC_F_EC_POINT_SET_AFFINE_COORDINATES_GF2M,
+              EC_R_POINT_IS_NOT_ON_CURVE);
+        return 0;
+    }
+    return 1;
 	}
 #endif
 
Index: openssl-1.0.1i/crypto/ec/ectest.c
===================================================================
--- openssl-1.0.1i.orig/crypto/ec/ectest.c	2019-04-10 14:47:47.518494819 +0200
+++ openssl-1.0.1i/crypto/ec/ectest.c	2019-04-10 15:44:26.019854226 +0200
@@ -277,7 +277,7 @@ static void prime_field_tests(void)
 	EC_GROUP *group;
 	EC_GROUP *P_160 = NULL, *P_192 = NULL, *P_224 = NULL, *P_256 = NULL, *P_384 = NULL, *P_521 = NULL;
 	EC_POINT *P, *Q, *R;
-	BIGNUM *x, *y, *z;
+	BIGNUM *x, *y, *z, *yplusone;
 	unsigned char buf[100];
 	size_t i, len;
 	int k;
@@ -338,7 +338,8 @@ static void prime_field_tests(void)
 	x = BN_new();
 	y = BN_new();
 	z = BN_new();
-	if (!x || !y || !z) ABORT;
+    yplusone = BN_new();
+    if (x == NULL || y == NULL || z == NULL || yplusone == NULL)
 
 	if (!BN_hex2bn(&x, "D")) ABORT;
 	if (!EC_POINT_set_compressed_coordinates_GFp(group, Q, x, 1, ctx)) ABORT;
@@ -437,6 +438,14 @@ static void prime_field_tests(void)
 
 	if (!BN_hex2bn(&x, "4A96B5688EF573284664698968C38BB913CBFC82")) ABORT;
 	if (!BN_hex2bn(&y, "23a628553168947d59dcc912042351377ac5fb32")) ABORT;
+    if (!BN_add(yplusone, y, BN_value_one()))
+        ABORT;
+    /*
+     * When (x, y) is on the curve, (x, y + 1) is, as it happens, not,
+     * and therefore setting the coordinates should fail.
+     */
+    if (EC_POINT_set_affine_coordinates_GFp(group, P, x, yplusone, ctx))
+        ABORT;
 	if (!EC_POINT_set_affine_coordinates_GFp(group, P, x, y, ctx)) ABORT;
 	if (!EC_POINT_is_on_curve(group, P, ctx)) ABORT;
 	if (!BN_hex2bn(&z, "0100000000000000000001F4C8F927AED3CA752257")) ABORT;
@@ -486,6 +495,15 @@ static void prime_field_tests(void)
 	if (!BN_hex2bn(&z, "07192B95FFC8DA78631011ED6B24CDD573F977A11E794811")) ABORT;
 	if (0 != BN_cmp(y, z)) ABORT;
 
+    if (!BN_add(yplusone, y, BN_value_one()))
+        ABORT;
+    /*
+     * When (x, y) is on the curve, (x, y + 1) is, as it happens, not,
+     * and therefore setting the coordinates should fail.
+     */
+    if (EC_POINT_set_affine_coordinates_GFp(group, P, x, yplusone, ctx))
+        ABORT;
+
 	fprintf(stdout, "verify degree ...");
 	if (EC_GROUP_get_degree(group) != 192) ABORT;
 	fprintf(stdout, " ok\n");
@@ -520,6 +538,15 @@ static void prime_field_tests(void)
 	if (!BN_hex2bn(&z, "BD376388B5F723FB4C22DFE6CD4375A05A07476444D5819985007E34")) ABORT;
 	if (0 != BN_cmp(y, z)) ABORT;
 	
+    if (!BN_add(yplusone, y, BN_value_one()))
+        ABORT;
+    /*
+     * When (x, y) is on the curve, (x, y + 1) is, as it happens, not,
+     * and therefore setting the coordinates should fail.
+     */
+    if (EC_POINT_set_affine_coordinates_GFp(group, P, x, yplusone, ctx))
+        ABORT;
+
 	fprintf(stdout, "verify degree ...");
 	if (EC_GROUP_get_degree(group) != 224) ABORT;
 	fprintf(stdout, " ok\n");
@@ -555,6 +582,15 @@ static void prime_field_tests(void)
 	if (!BN_hex2bn(&z, "4FE342E2FE1A7F9B8EE7EB4A7C0F9E162BCE33576B315ECECBB6406837BF51F5")) ABORT;
 	if (0 != BN_cmp(y, z)) ABORT;
 	
+    if (!BN_add(yplusone, y, BN_value_one()))
+        ABORT;
+    /*
+     * When (x, y) is on the curve, (x, y + 1) is, as it happens, not,
+     * and therefore setting the coordinates should fail.
+     */
+    if (EC_POINT_set_affine_coordinates_GFp(group, P, x, yplusone, ctx))
+        ABORT;
+
 	fprintf(stdout, "verify degree ...");
 	if (EC_GROUP_get_degree(group) != 256) ABORT;
 	fprintf(stdout, " ok\n");
@@ -595,6 +631,15 @@ static void prime_field_tests(void)
 		"7CE9DA3113B5F0B8C00A60B1CE1D7E819D7A431D7C90EA0E5F")) ABORT;
 	if (0 != BN_cmp(y, z)) ABORT;
 	
+    if (!BN_add(yplusone, y, BN_value_one()))
+        ABORT;
+    /*
+     * When (x, y) is on the curve, (x, y + 1) is, as it happens, not,
+     * and therefore setting the coordinates should fail.
+     */
+    if (EC_POINT_set_affine_coordinates_GFp(group, P, x, yplusone, ctx))
+        ABORT;
+
 	fprintf(stdout, "verify degree ...");
 	if (EC_GROUP_get_degree(group) != 384) ABORT;
 	fprintf(stdout, " ok\n");
@@ -641,6 +686,15 @@ static void prime_field_tests(void)
 		"7086A272C24088BE94769FD16650")) ABORT;
 	if (0 != BN_cmp(y, z)) ABORT;
 	
+    if (!BN_add(yplusone, y, BN_value_one()))
+        ABORT;
+    /*
+     * When (x, y) is on the curve, (x, y + 1) is, as it happens, not,
+     * and therefore setting the coordinates should fail.
+     */
+    if (EC_POINT_set_affine_coordinates_GFp(group, P, x, yplusone, ctx))
+        ABORT;
+
 	fprintf(stdout, "verify degree ...");
 	if (EC_GROUP_get_degree(group) != 521) ABORT;
 	fprintf(stdout, " ok\n");
@@ -653,6 +707,10 @@ static void prime_field_tests(void)
 
 	/* more tests using the last curve */
 
+    /* Restore the point that got mangled in the (x, y + 1) test. */
+    if (!EC_POINT_set_affine_coordinates_GFp(group, P, x, y, ctx))
+        ABORT;
+
 	if (!EC_POINT_copy(Q, P)) ABORT;
 	if (EC_POINT_is_at_infinity(group, Q)) ABORT;
 	if (!EC_POINT_dbl(group, P, P, ctx)) ABORT;
@@ -755,6 +813,7 @@ static void prime_field_tests(void)
 	EC_POINT_free(Q);
 	EC_POINT_free(R);
 	BN_free(x); BN_free(y); BN_free(z);
+    BN_free(yplusone);
 
 	if (P_160) EC_GROUP_free(P_160);
 	if (P_192) EC_GROUP_free(P_192);
@@ -769,6 +828,13 @@ static void prime_field_tests(void)
 #ifdef OPENSSL_EC_BIN_PT_COMP
 #define CHAR2_CURVE_TEST_INTERNAL(_name, _p, _a, _b, _x, _y, _y_bit, _order, _cof, _degree, _variable) \
 	if (!BN_hex2bn(&x, _x)) ABORT; \
+        if (!BN_hex2bn(&y, _y)) ABORT; \
+        if (!BN_add(yplusone, y, BN_value_one())) ABORT;        \
+        /* \
+         * When (x, y) is on the curve, (x, y + 1) is, as it happens, not, \
+         * and therefore setting the coordinates should fail. \
+         */ \
+        if (EC_POINT_set_affine_coordinates_GF2m(group, P, x, yplusone, ctx)) 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 (!BN_hex2bn(&z, _order)) ABORT; \
@@ -787,6 +853,12 @@ 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 (!BN_hex2bn(&y, _y)) ABORT; \
+        if (!BN_add(yplusone, y, BN_value_one())) ABORT;        \
+        /* \
+         * When (x, y) is on the curve, (x, y + 1) is, as it happens, not, \
+         * and therefore setting the coordinates should fail. \
+         */ \
+        if (EC_POINT_set_affine_coordinates_GF2m(group, P, x, yplusone, ctx)) 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 (!BN_hex2bn(&z, _order)) ABORT; \
@@ -822,7 +894,7 @@ static void char2_field_tests(void)
 	EC_GROUP *C2_K163 = NULL, *C2_K233 = NULL, *C2_K283 = NULL, *C2_K409 = NULL, *C2_K571 = NULL;
 	EC_GROUP *C2_B163 = NULL, *C2_B233 = NULL, *C2_B283 = NULL, *C2_B409 = NULL, *C2_B571 = NULL;
 	EC_POINT *P, *Q, *R;
-	BIGNUM *x, *y, *z, *cof;
+	BIGNUM *x, *y, *z, *cof, *yplusone;
 	unsigned char buf[100];
 	size_t i, len;
 	int k;
@@ -883,7 +955,8 @@ static void char2_field_tests(void)
 	y = BN_new();
 	z = BN_new();
 	cof = BN_new();
-	if (!x || !y || !z || !cof) ABORT;
+    yplusone = BN_new();
+    if (x == NULL || y == NULL || z == NULL || cof == NULL || yplusone == NULL)
 
 	if (!BN_hex2bn(&x, "6")) ABORT;
 /* Change test based on whether binary point compression is enabled or not. */
@@ -1234,6 +1307,7 @@ static void char2_field_tests(void)
 	EC_POINT_free(Q);
 	EC_POINT_free(R);
 	BN_free(x); BN_free(y); BN_free(z); BN_free(cof);
+    BN_free(yplusone);
 
 	if (C2_K163) EC_GROUP_free(C2_K163);
 	if (C2_B163) EC_GROUP_free(C2_B163);
@@ -1368,7 +1442,7 @@ static const struct nistp_test_params ni
 void nistp_single_test(const struct nistp_test_params *test)
 	{
 	BN_CTX *ctx;
-	BIGNUM *p, *a, *b, *x, *y, *n, *m, *order;
+	BIGNUM *p, *a, *b, *x, *y, *n, *m, *order, *yplusone;
 	EC_GROUP *NISTP;
 	EC_POINT *G, *P, *Q, *Q_CHECK;
 
@@ -1379,6 +1453,7 @@ void nistp_single_test(const struct nist
 	b = BN_new();
 	x = BN_new(); y = BN_new();
 	m = BN_new(); n = BN_new(); order = BN_new();
+    yplusone = BN_new();
 
 	NISTP = EC_GROUP_new(test->meth());
 	if(!NISTP) ABORT;
@@ -1393,6 +1468,14 @@ void nistp_single_test(const struct nist
 	Q_CHECK = EC_POINT_new(NISTP);
 	if(!BN_hex2bn(&x, test->Qx)) ABORT;
 	if(!BN_hex2bn(&y, test->Qy)) ABORT;
+    if (!BN_add(yplusone, y, BN_value_one()))
+        ABORT;
+    /*
+     * When (x, y) is on the curve, (x, y + 1) is, as it happens, not,
+     * and therefore setting the coordinates should fail.
+     */
+    if (EC_POINT_set_affine_coordinates_GFp(NISTP, Q_CHECK, x, yplusone, ctx))
+        ABORT;
 	if(!EC_POINT_set_affine_coordinates_GFp(NISTP, Q_CHECK, x, y, ctx)) ABORT;
 	if (!BN_hex2bn(&x, test->Gx)) ABORT;
 	if (!BN_hex2bn(&y, test->Gy)) ABORT;
@@ -1465,6 +1548,7 @@ void nistp_single_test(const struct nist
 	BN_free(x);
 	BN_free(y);
 	BN_free(order);
+    BN_free(yplusone);
 	BN_CTX_free(ctx);
 	}

Places

File 0001-RT-4242-reject-invalid-EC-point-coordinates.patch of Package openssl (Revision ab9e1b647facbbf65a6dc99d207b992a)

Places