[openssl] OpenSSL_1_1_1-stable update
nic.tuv at gmail.com
nic.tuv at gmail.com
Sat Sep 7 01:05:10 UTC 2019
The branch OpenSSL_1_1_1-stable has been updated
via a6186f39802f94937a46f7a41ef0c86b6334b592 (commit)
via eb1ec38b266340710cb97c90b08fc90edd06262c (commit)
via 30c22fa8b1d840036b8e203585738df62a03cec8 (commit)
from ed0ac119506ac8cbbaa23a1a1347d74a7bf4da47 (commit)
- Log -----------------------------------------------------------------
commit a6186f39802f94937a46f7a41ef0c86b6334b592
Author: Billy Brumley <bbrumley at gmail.com>
Date: Fri Sep 6 17:26:40 2019 +0300
CHANGES entry: for ECC parameters with NULL or zero cofactor, compute it
Reviewed-by: Matt Caswell <matt at openssl.org>
Reviewed-by: Tomas Mraz <tmraz at fedoraproject.org>
Reviewed-by: Nicola Tuveri <nic.tuv at gmail.com>
(Merged from https://github.com/openssl/openssl/pull/9781)
commit eb1ec38b266340710cb97c90b08fc90edd06262c
Author: Billy Brumley <bbrumley at gmail.com>
Date: Thu Sep 5 21:25:52 2019 +0300
[test] computing ECC cofactors: regression test
Reviewed-by: Matt Caswell <matt at openssl.org>
Reviewed-by: Tomas Mraz <tmraz at fedoraproject.org>
Reviewed-by: Nicola Tuveri <nic.tuv at gmail.com>
(Merged from https://github.com/openssl/openssl/pull/9781)
commit 30c22fa8b1d840036b8e203585738df62a03cec8
Author: Billy Brumley <bbrumley at gmail.com>
Date: Thu Sep 5 21:25:37 2019 +0300
[crypto/ec] for ECC parameters with NULL or zero cofactor, compute it
The cofactor argument to EC_GROUP_set_generator is optional, and SCA
mitigations for ECC currently use it. So the library currently falls
back to very old SCA-vulnerable code if the cofactor is not present.
This PR allows EC_GROUP_set_generator to compute the cofactor for all
curves of cryptographic interest. Steering scalar multiplication to more
SCA-robust code.
This issue affects persisted private keys in explicit parameter form,
where the (optional) cofactor field is zero or absent.
It also affects curves not built-in to the library, but constructed
programatically with explicit parameters, then calling
EC_GROUP_set_generator with a nonsensical value (NULL, zero).
The very old scalar multiplication code is known to be vulnerable to
local uarch attacks, outside of the OpenSSL threat model. New results
suggest the code path is also vulnerable to traditional wall clock
timing attacks.
CVE-2019-1547
Reviewed-by: Matt Caswell <matt at openssl.org>
Reviewed-by: Tomas Mraz <tmraz at fedoraproject.org>
Reviewed-by: Nicola Tuveri <nic.tuv at gmail.com>
(Merged from https://github.com/openssl/openssl/pull/9781)
-----------------------------------------------------------------------
Summary of changes:
CHANGES | 6 ++++
crypto/ec/ec_lib.c | 103 +++++++++++++++++++++++++++++++++++++++++++++++++----
test/ectest.c | 84 +++++++++++++++++++++++++++++++++++++++++++
3 files changed, 186 insertions(+), 7 deletions(-)
diff --git a/CHANGES b/CHANGES
index 8f732cb303..d34bba83fe 100644
--- a/CHANGES
+++ b/CHANGES
@@ -9,6 +9,12 @@
Changes between 1.1.1c and 1.1.1d [xx XXX xxxx]
+ *) Compute ECC cofactors if not provided during EC_GROUP construction. Before
+ this change, EC_GROUP_set_generator would accept order and/or cofactor as
+ NULL. After this change, only the cofactor parameter can be NULL. It also
+ does some minimal sanity checks on the passed order.
+ [Billy Bob Brumley]
+
*) Early start up entropy quality from the DEVRANDOM seed source has been
improved for older Linux systems. The RAND subsystem will wait for
/dev/random to be producing output before seeding from /dev/urandom.
diff --git a/crypto/ec/ec_lib.c b/crypto/ec/ec_lib.c
index 8cab5a5061..1289c8608e 100644
--- a/crypto/ec/ec_lib.c
+++ b/crypto/ec/ec_lib.c
@@ -265,6 +265,67 @@ int EC_METHOD_get_field_type(const EC_METHOD *meth)
static int ec_precompute_mont_data(EC_GROUP *);
+/*-
+ * Try computing cofactor from the generator order (n) and field cardinality (q).
+ * This works for all curves of cryptographic interest.
+ *
+ * Hasse thm: q + 1 - 2*sqrt(q) <= n*h <= q + 1 + 2*sqrt(q)
+ * h_min = (q + 1 - 2*sqrt(q))/n
+ * h_max = (q + 1 + 2*sqrt(q))/n
+ * h_max - h_min = 4*sqrt(q)/n
+ * So if n > 4*sqrt(q) holds, there is only one possible value for h:
+ * h = \lfloor (h_min + h_max)/2 \rceil = \lfloor (q + 1)/n \rceil
+ *
+ * Otherwise, zero cofactor and return success.
+ */
+static int ec_guess_cofactor(EC_GROUP *group) {
+ int ret = 0;
+ BN_CTX *ctx = NULL;
+ BIGNUM *q = NULL;
+
+ /*-
+ * If the cofactor is too large, we cannot guess it.
+ * The RHS of below is a strict overestimate of lg(4 * sqrt(q))
+ */
+ if (BN_num_bits(group->order) <= (BN_num_bits(group->field) + 1) / 2 + 3) {
+ /* default to 0 */
+ BN_zero(group->cofactor);
+ /* return success */
+ return 1;
+ }
+
+ if ((ctx = BN_CTX_new()) == NULL)
+ return 0;
+
+ BN_CTX_start(ctx);
+ if ((q = BN_CTX_get(ctx)) == NULL)
+ goto err;
+
+ /* set q = 2**m for binary fields; q = p otherwise */
+ if (group->meth->field_type == NID_X9_62_characteristic_two_field) {
+ BN_zero(q);
+ if (!BN_set_bit(q, BN_num_bits(group->field) - 1))
+ goto err;
+ } else {
+ if (!BN_copy(q, group->field))
+ goto err;
+ }
+
+ /* compute h = \lfloor (q + 1)/n \rceil = \lfloor (q + 1 + n/2)/n \rfloor */
+ if (!BN_rshift1(group->cofactor, group->order) /* n/2 */
+ || !BN_add(group->cofactor, group->cofactor, q) /* q + n/2 */
+ /* q + 1 + n/2 */
+ || !BN_add(group->cofactor, group->cofactor, BN_value_one())
+ /* (q + 1 + n/2)/n */
+ || !BN_div(group->cofactor, NULL, group->cofactor, group->order, ctx))
+ goto err;
+ ret = 1;
+ err:
+ BN_CTX_end(ctx);
+ BN_CTX_free(ctx);
+ return ret;
+}
+
int EC_GROUP_set_generator(EC_GROUP *group, const EC_POINT *generator,
const BIGNUM *order, const BIGNUM *cofactor)
{
@@ -273,6 +334,34 @@ int EC_GROUP_set_generator(EC_GROUP *group, const EC_POINT *generator,
return 0;
}
+ /* require group->field >= 1 */
+ if (group->field == NULL || BN_is_zero(group->field)
+ || BN_is_negative(group->field)) {
+ ECerr(EC_F_EC_GROUP_SET_GENERATOR, EC_R_INVALID_FIELD);
+ return 0;
+ }
+
+ /*-
+ * - require order >= 1
+ * - enforce upper bound due to Hasse thm: order can be no more than one bit
+ * longer than field cardinality
+ */
+ if (order == NULL || BN_is_zero(order) || BN_is_negative(order)
+ || BN_num_bits(order) > BN_num_bits(group->field) + 1) {
+ ECerr(EC_F_EC_GROUP_SET_GENERATOR, EC_R_INVALID_GROUP_ORDER);
+ return 0;
+ }
+
+ /*-
+ * Unfortunately the cofactor is an optional field in many standards.
+ * Internally, the lib uses 0 cofactor as a marker for "unknown cofactor".
+ * So accept cofactor == NULL or cofactor >= 0.
+ */
+ if (cofactor != NULL && BN_is_negative(cofactor)) {
+ ECerr(EC_F_EC_GROUP_SET_GENERATOR, EC_R_UNKNOWN_COFACTOR);
+ return 0;
+ }
+
if (group->generator == NULL) {
group->generator = EC_POINT_new(group);
if (group->generator == NULL)
@@ -281,17 +370,17 @@ int EC_GROUP_set_generator(EC_GROUP *group, const EC_POINT *generator,
if (!EC_POINT_copy(group->generator, generator))
return 0;
- if (order != NULL) {
- if (!BN_copy(group->order, order))
- return 0;
- } else
- BN_zero(group->order);
+ if (!BN_copy(group->order, order))
+ return 0;
- if (cofactor != NULL) {
+ /* Either take the provided positive cofactor, or try to compute it */
+ if (cofactor != NULL && !BN_is_zero(cofactor)) {
if (!BN_copy(group->cofactor, cofactor))
return 0;
- } else
+ } else if (!ec_guess_cofactor(group)) {
BN_zero(group->cofactor);
+ return 0;
+ }
/*
* Some groups have an order with
diff --git a/test/ectest.c b/test/ectest.c
index 269ec4ef8f..ddc4ea11d6 100644
--- a/test/ectest.c
+++ b/test/ectest.c
@@ -1561,6 +1561,89 @@ err:
OPENSSL_free(buf);
return r;
}
+
+/*-
+ * For named curves, test that:
+ * - the lib correctly computes the cofactor if passed a NULL or zero cofactor
+ * - a nonsensical cofactor throws an error (negative test)
+ * - nonsensical orders throw errors (negative tests)
+ */
+static int cardinality_test(int n)
+{
+ int ret = 0;
+ int nid = curves[n].nid;
+ BN_CTX *ctx = NULL;
+ EC_GROUP *g1 = NULL, *g2 = NULL;
+ EC_POINT *g2_gen = NULL;
+ BIGNUM *g1_p = NULL, *g1_a = NULL, *g1_b = NULL, *g1_x = NULL, *g1_y = NULL,
+ *g1_order = NULL, *g1_cf = NULL, *g2_cf = NULL;
+
+ TEST_info("Curve %s cardinality test", OBJ_nid2sn(nid));
+
+ if (!TEST_ptr(ctx = BN_CTX_new())
+ || !TEST_ptr(g1 = EC_GROUP_new_by_curve_name(nid))
+ || !TEST_ptr(g2 = EC_GROUP_new(EC_GROUP_method_of(g1)))) {
+ EC_GROUP_free(g1);
+ EC_GROUP_free(g2);
+ BN_CTX_free(ctx);
+ return 0;
+ }
+
+ BN_CTX_start(ctx);
+ g1_p = BN_CTX_get(ctx);
+ g1_a = BN_CTX_get(ctx);
+ g1_b = BN_CTX_get(ctx);
+ g1_x = BN_CTX_get(ctx);
+ g1_y = BN_CTX_get(ctx);
+ g1_order = BN_CTX_get(ctx);
+ g1_cf = BN_CTX_get(ctx);
+
+ if (!TEST_ptr(g2_cf = BN_CTX_get(ctx))
+ /* pull out the explicit curve parameters */
+ || !TEST_true(EC_GROUP_get_curve(g1, g1_p, g1_a, g1_b, ctx))
+ || !TEST_true(EC_POINT_get_affine_coordinates(g1,
+ EC_GROUP_get0_generator(g1), g1_x, g1_y, ctx))
+ || !TEST_true(BN_copy(g1_order, EC_GROUP_get0_order(g1)))
+ || !TEST_true(EC_GROUP_get_cofactor(g1, g1_cf, ctx))
+ /* construct g2 manually with g1 parameters */
+ || !TEST_true(EC_GROUP_set_curve(g2, g1_p, g1_a, g1_b, ctx))
+ || !TEST_ptr(g2_gen = EC_POINT_new(g2))
+ || !TEST_true(EC_POINT_set_affine_coordinates(g2, g2_gen, g1_x, g1_y, ctx))
+ /* pass NULL cofactor: lib should compute it */
+ || !TEST_true(EC_GROUP_set_generator(g2, g2_gen, g1_order, NULL))
+ || !TEST_true(EC_GROUP_get_cofactor(g2, g2_cf, ctx))
+ || !TEST_BN_eq(g1_cf, g2_cf)
+ /* pass zero cofactor: lib should compute it */
+ || !TEST_true(BN_set_word(g2_cf, 0))
+ || !TEST_true(EC_GROUP_set_generator(g2, g2_gen, g1_order, g2_cf))
+ || !TEST_true(EC_GROUP_get_cofactor(g2, g2_cf, ctx))
+ || !TEST_BN_eq(g1_cf, g2_cf)
+ /* negative test for invalid cofactor */
+ || !TEST_true(BN_set_word(g2_cf, 0))
+ || !TEST_true(BN_sub(g2_cf, g2_cf, BN_value_one()))
+ || !TEST_false(EC_GROUP_set_generator(g2, g2_gen, g1_order, g2_cf))
+ /* negative test for NULL order */
+ || !TEST_false(EC_GROUP_set_generator(g2, g2_gen, NULL, NULL))
+ /* negative test for zero order */
+ || !TEST_true(BN_set_word(g1_order, 0))
+ || !TEST_false(EC_GROUP_set_generator(g2, g2_gen, g1_order, NULL))
+ /* negative test for negative order */
+ || !TEST_true(BN_set_word(g2_cf, 0))
+ || !TEST_true(BN_sub(g2_cf, g2_cf, BN_value_one()))
+ || !TEST_false(EC_GROUP_set_generator(g2, g2_gen, g1_order, NULL))
+ /* negative test for too large order */
+ || !TEST_true(BN_lshift(g1_order, g1_p, 2))
+ || !TEST_false(EC_GROUP_set_generator(g2, g2_gen, g1_order, NULL)))
+ goto err;
+ ret = 1;
+ err:
+ EC_POINT_free(g2_gen);
+ EC_GROUP_free(g1);
+ EC_GROUP_free(g2);
+ BN_CTX_end(ctx);
+ BN_CTX_free(ctx);
+ return ret;
+}
#endif
int setup_tests(void)
@@ -1572,6 +1655,7 @@ int setup_tests(void)
return 0;
ADD_TEST(parameter_test);
+ ADD_ALL_TESTS(cardinality_test, crv_len);
ADD_TEST(prime_field_tests);
# ifndef OPENSSL_NO_EC2M
ADD_TEST(char2_field_tests);
More information about the openssl-commits
mailing list