From 8d4f150f70d70d6c3e62661ed7cc16c2f751d8a1 Mon Sep 17 00:00:00 2001 From: Nicola Tuveri Date: Sun, 31 Mar 2019 16:26:33 +0300 Subject: [PATCH] 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 (Merged from https://github.com/openssl/openssl/pull/8555) --- test/ectest.c | 50 ++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 40 insertions(+), 10 deletions(-) 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; -- 2.25.1