From: Nicola Tuveri <nic.tuv@gmail.com>
Date: Sun, 31 Mar 2019 13:26:33 +0000 (+0300)
Subject: EC_GROUP_set_curve() might fail for arbitrary params
X-Git-Tag: openssl-3.0.0-alpha1~2192
X-Git-Url: https://git.librecmc.org/?a=commitdiff_plain;h=8d4f150f70d70d6c3e62661ed7cc16c2f751d8a1;p=oweals%2Fopenssl.git

EC_GROUP_set_curve() might fail for arbitrary params

Setting arbitrary `p`, `a` or `b` with `EC_GROUP_set_curve()` might fail
for some `EC_GROUP`s, depending on the internal `EC_METHOD`
implementation, hence the block of tests verifying that
`EC_GROUP_check_named_curve()` fails when any of the curve parameters is
changed is modified to run only if the previous `EC_GROUP_set_curve()`
call succeeds.

`ERR_set_mark()` and `ERR_pop_to_mark()` are used to avoid littering the
thread error stack with unrelated errors happened during
`EC_GROUP_set_curve()`.

Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/8555)
---

diff --git a/test/ectest.c b/test/ectest.c
index 56c6b6f06d..c8cb752301 100644
--- a/test/ectest.c
+++ b/test/ectest.c
@@ -1701,16 +1701,46 @@ static int check_named_curve(int id)
         /* check that restoring the generator passes */
         || !TEST_true(EC_GROUP_set_generator(gtest, group_gen, group_order,
                                              group_cofactor))
-        || !TEST_int_eq(EC_GROUP_check_named_curve(gtest, 0), nid)
-        /* check that changing any curve parameters fail */
-        || !TEST_true(EC_GROUP_set_curve(gtest, other_p, group_a, group_b, NULL))
-        || !TEST_int_le(EC_GROUP_check_named_curve(gtest, 0), 0)
-        || !TEST_true(EC_GROUP_set_curve(gtest, group_p, other_a, group_b, NULL))
-        || !TEST_int_eq(EC_GROUP_check_named_curve(gtest, 0), 0)
-        || !TEST_true(EC_GROUP_set_curve(gtest, group_p, group_a, other_b, NULL))
-        || !TEST_int_eq(EC_GROUP_check_named_curve(gtest, 0), 0)
-        /* Check that restoring the curve parameters pass */
-        || !TEST_true(EC_GROUP_set_curve(gtest, group_p, group_a, group_b, NULL))
+        || !TEST_int_eq(EC_GROUP_check_named_curve(gtest, 0), nid))
+        goto err;
+
+
+    /*-
+     * check that changing any curve parameter fails
+     *
+     * Setting arbitrary p, a or b might fail for some EC_GROUPs
+     * depending on the internal EC_METHOD implementation, hence run
+     * these tests conditionally to the success of EC_GROUP_set_curve().
+     */
+    ERR_set_mark();
+    if (EC_GROUP_set_curve(gtest, other_p, group_a, group_b, NULL)) {
+        if (!TEST_int_le(EC_GROUP_check_named_curve(gtest, 0), 0))
+            goto err;
+    } else {
+        /* clear the error stack if EC_GROUP_set_curve() failed */
+        ERR_pop_to_mark();
+        ERR_set_mark();
+    }
+    if (EC_GROUP_set_curve(gtest, group_p, other_a, group_b, NULL)) {
+        if (!TEST_int_le(EC_GROUP_check_named_curve(gtest, 0), 0))
+            goto err;
+    } else {
+        /* clear the error stack if EC_GROUP_set_curve() failed */
+        ERR_pop_to_mark();
+        ERR_set_mark();
+    }
+    if (EC_GROUP_set_curve(gtest, group_p, group_a, other_b, NULL)) {
+        if (!TEST_int_le(EC_GROUP_check_named_curve(gtest, 0), 0))
+            goto err;
+    } else {
+        /* clear the error stack if EC_GROUP_set_curve() failed */
+        ERR_pop_to_mark();
+        ERR_set_mark();
+    }
+    ERR_pop_to_mark();
+
+    /* Check that restoring the curve parameters pass */
+    if (!TEST_true(EC_GROUP_set_curve(gtest, group_p, group_a, group_b, NULL))
         || !TEST_int_eq(EC_GROUP_check_named_curve(gtest, 0), nid))
         goto err;