[openssl] OpenSSL_1_0_2-stable update
Matt Caswell
matt at openssl.org
Thu Apr 25 12:20:16 UTC 2019
The branch OpenSSL_1_0_2-stable has been updated
via cea83f9f7825309379db3fea77f19edf0c5b1e13 (commit)
from f937540ec40a5e838460b8f19d2eb722529126b8 (commit)
- Log -----------------------------------------------------------------
commit cea83f9f7825309379db3fea77f19edf0c5b1e13
Author: Emilia Kasper <emilia at openssl.org>
Date: Fri Jun 3 14:42:04 2016 +0200
RT 4242: reject invalid EC point coordinates
This is a backport of commit 1e2012b7 to 1.0.2. This hardening change
was made to 1.1.0 but was not backported to 1.0.2. Recent CVEs in user
applications have shown this additional hardening in 1.0.2 would be
beneficial.
E.g. see the patch for CVE-2019-9498
https://w1.fi/security/2019-4/0011-EAP-pwd-server-Verify-received-scalar-and-element.patch
and CVE-2019-9499
https://w1.fi/security/2019-4/0013-EAP-pwd-client-Verify-received-scalar-and-element.patch
The original commit had this description:
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: Paul Dale <paul.dale at oracle.com>
Reviewed-by: Matt Caswell <matt at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/8750)
-----------------------------------------------------------------------
Summary of changes:
crypto/ec/ec2_oct.c | 10 +++---
crypto/ec/ec_lib.c | 20 +++++++++--
crypto/ec/ecp_oct.c | 10 +++---
crypto/ec/ectest.c | 96 +++++++++++++++++++++++++++++++++++++++++++++++++----
4 files changed, 116 insertions(+), 20 deletions(-)
diff --git a/crypto/ec/ec2_oct.c b/crypto/ec/ec2_oct.c
index 6f2f7ca..b3e71c4 100644
--- a/crypto/ec/ec2_oct.c
+++ b/crypto/ec/ec2_oct.c
@@ -383,16 +383,14 @@ int ec_GF2m_simple_oct2point(const EC_GROUP *group, EC_POINT *point,
}
}
+ /*
+ * 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;
}
- /* test required by X9.62 */
- 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;
- }
-
ret = 1;
err:
diff --git a/crypto/ec/ec_lib.c b/crypto/ec/ec_lib.c
index df56484..c01e0f0 100644
--- a/crypto/ec/ec_lib.c
+++ b/crypto/ec/ec_lib.c
@@ -872,7 +872,15 @@ int EC_POINT_set_affine_coordinates_GFp(const EC_GROUP *group,
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
@@ -890,7 +898,15 @@ int EC_POINT_set_affine_coordinates_GF2m(const EC_GROUP *group,
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
diff --git a/crypto/ec/ecp_oct.c b/crypto/ec/ecp_oct.c
index 1bc3f39..941f0ec 100644
--- a/crypto/ec/ecp_oct.c
+++ b/crypto/ec/ecp_oct.c
@@ -408,16 +408,14 @@ int ec_GFp_simple_oct2point(const EC_GROUP *group, EC_POINT *point,
}
}
+ /*
+ * 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;
}
- /* test required by X9.62 */
- 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;
- }
-
ret = 1;
err:
diff --git a/crypto/ec/ectest.c b/crypto/ec/ectest.c
index 5e1ef50..c3cdac1 100644
--- a/crypto/ec/ectest.c
+++ b/crypto/ec/ectest.c
@@ -325,7 +325,7 @@ static void prime_field_tests(void)
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;
@@ -405,7 +405,8 @@ static void prime_field_tests(void)
x = BN_new();
y = BN_new();
z = BN_new();
- if (!x || !y || !z)
+ yplusone = BN_new();
+ if (x == NULL || y == NULL || z == NULL || yplusone == NULL)
ABORT;
if (!BN_hex2bn(&x, "D"))
@@ -542,6 +543,14 @@ static void prime_field_tests(void)
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) <= 0)
@@ -613,6 +622,15 @@ static void prime_field_tests(void)
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;
@@ -668,6 +686,15 @@ static void prime_field_tests(void)
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;
@@ -728,6 +755,15 @@ static void prime_field_tests(void)
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;
@@ -783,6 +819,15 @@ static void prime_field_tests(void)
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;
@@ -844,6 +889,15 @@ static void prime_field_tests(void)
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;
@@ -858,6 +912,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))
@@ -987,6 +1045,7 @@ static void prime_field_tests(void)
BN_free(x);
BN_free(y);
BN_free(z);
+ BN_free(yplusone);
if (P_160)
EC_GROUP_free(P_160);
@@ -1007,6 +1066,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) <= 0) ABORT; \
if (!BN_hex2bn(&z, _order)) ABORT; \
@@ -1025,6 +1091,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) <= 0) ABORT; \
if (!BN_hex2bn(&z, _order)) ABORT; \
@@ -1062,7 +1134,7 @@ static void char2_field_tests(void)
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;
@@ -1076,7 +1148,7 @@ static void char2_field_tests(void)
p = BN_new();
a = BN_new();
b = BN_new();
- if (!p || !a || !b)
+ if (p == NULL || a == NULL || b == NULL)
ABORT;
if (!BN_hex2bn(&p, "13"))
@@ -1142,7 +1214,8 @@ static void char2_field_tests(void)
y = BN_new();
z = BN_new();
cof = BN_new();
- if (!x || !y || !z || !cof)
+ yplusone = BN_new();
+ if (x == NULL || y == NULL || z == NULL || cof == NULL || yplusone == NULL)
ABORT;
if (!BN_hex2bn(&x, "6"))
@@ -1504,6 +1577,7 @@ static void char2_field_tests(void)
BN_free(y);
BN_free(z);
BN_free(cof);
+ BN_free(yplusone);
if (C2_K163)
EC_GROUP_free(C2_K163);
@@ -1672,7 +1746,7 @@ static const struct nistp_test_params nistp_tests_params[] = {
static 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;
@@ -1687,6 +1761,7 @@ static void nistp_single_test(const struct nistp_test_params *test)
m = BN_new();
n = BN_new();
order = BN_new();
+ yplusone = BN_new();
NISTP = EC_GROUP_new(test->meth());
if (!NISTP)
@@ -1709,6 +1784,14 @@ static void nistp_single_test(const struct nistp_test_params *test)
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))
@@ -1811,6 +1894,7 @@ static void nistp_single_test(const struct nistp_test_params *test)
BN_free(x);
BN_free(y);
BN_free(order);
+ BN_free(yplusone);
BN_CTX_free(ctx);
}
More information about the openssl-commits
mailing list