EC_GROUP_set_curve() might fail for arbitrary params
authorNicola Tuveri <nic.tuv@gmail.com>
Sun, 31 Mar 2019 13:26:33 +0000 (16:26 +0300)
committerNicola Tuveri <nic.tuv@gmail.com>
Thu, 11 Apr 2019 09:05:38 +0000 (12:05 +0300)
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)

test/ectest.c

index 56c6b6f06d9b2dfcc01ae6530b62ea061df5758a..c8cb752301cb4a703f1fa64b902a004387d4f1e4 100644 (file)
@@ -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;