diff options
author | Theo Buehler <tb@cvs.openbsd.org> | 2024-11-17 06:33:36 +0000 |
---|---|---|
committer | Theo Buehler <tb@cvs.openbsd.org> | 2024-11-17 06:33:36 +0000 |
commit | a414f4fd85a1d33eadebe173c4e901f9e7f47d26 (patch) | |
tree | acc4a0f930a303efd0f65f3e542ecb17707bc1a2 /lib | |
parent | afbc99ff524cabd5e10bcba2fc5856a67709d5d6 (diff) |
Rewrite EC_GROUP_cmp()
Use better variable names (cf. https://jmilne.org/math/tips.html#4) and
avoid the weird style of assigning to r (what does r stand for anyway?)
and short circuiting subsequent tests using if (r || ...). Also, do not
reuse the variables for order and cofactor that were previously used for
the curve coefficients.
ok jsing
Diffstat (limited to 'lib')
-rw-r--r-- | lib/libcrypto/ec/ec_lib.c | 126 |
1 files changed, 75 insertions, 51 deletions
diff --git a/lib/libcrypto/ec/ec_lib.c b/lib/libcrypto/ec/ec_lib.c index d61dea9f125..01e21e9eb83 100644 --- a/lib/libcrypto/ec/ec_lib.c +++ b/lib/libcrypto/ec/ec_lib.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ec_lib.c,v 1.85 2024/11/08 13:55:45 tb Exp $ */ +/* $OpenBSD: ec_lib.c,v 1.86 2024/11/17 06:33:35 tb Exp $ */ /* * Originally written by Bodo Moeller for the OpenSSL project. */ @@ -662,77 +662,101 @@ EC_GROUP_check(const EC_GROUP *group, BN_CTX *ctx_in) } LCRYPTO_ALIAS(EC_GROUP_check); +/* + * Returns -1 on error, 0 if the groups are equal, 1 if they are distinct. + */ int -EC_GROUP_cmp(const EC_GROUP *a, const EC_GROUP *b, BN_CTX *ctx) +EC_GROUP_cmp(const EC_GROUP *group1, const EC_GROUP *group2, BN_CTX *ctx_in) { - int r = 0; - BIGNUM *a1, *a2, *a3, *b1, *b2, *b3; - BN_CTX *ctx_new = NULL; - - /* compare the field types */ - if (ec_group_get_field_type(a) != ec_group_get_field_type(b)) - return 1; - /* compare the curve name (if present in both) */ - if (EC_GROUP_get_curve_name(a) && EC_GROUP_get_curve_name(b) && - EC_GROUP_get_curve_name(a) != EC_GROUP_get_curve_name(b)) - return 1; + BN_CTX *ctx = NULL; + BIGNUM *p1, *a1, *b1, *p2, *a2, *b2; + const EC_POINT *generator1, *generator2; + const BIGNUM *order1, *order2, *cofactor1, *cofactor2; + int nid1, nid2; + int cmp = 1; + int ret = -1; - if (!ctx) - ctx_new = ctx = BN_CTX_new(); - if (!ctx) - return -1; + if ((ctx = ctx_in) == NULL) + ctx = BN_CTX_new(); + if (ctx == NULL) + goto err; BN_CTX_start(ctx); + + if (ec_group_get_field_type(group1) != ec_group_get_field_type(group2)) + goto distinct; + if ((nid1 = EC_GROUP_get_curve_name(group1)) != NID_undef && + (nid2 = EC_GROUP_get_curve_name(group2)) != NID_undef) { + if (nid1 != nid2) + goto distinct; + } + + if ((p1 = BN_CTX_get(ctx)) == NULL) + goto err; if ((a1 = BN_CTX_get(ctx)) == NULL) goto err; - if ((a2 = BN_CTX_get(ctx)) == NULL) + if ((b1 = BN_CTX_get(ctx)) == NULL) goto err; - if ((a3 = BN_CTX_get(ctx)) == NULL) + if ((p2 = BN_CTX_get(ctx)) == NULL) goto err; - if ((b1 = BN_CTX_get(ctx)) == NULL) + if ((a2 = BN_CTX_get(ctx)) == NULL) goto err; if ((b2 = BN_CTX_get(ctx)) == NULL) goto err; - if ((b3 = BN_CTX_get(ctx)) == NULL) + + /* + * If we ever support curves in non-Weierstrass form, this check needs + * to be adjusted. The comparison of the generators will fail anyway. + */ + if (!EC_GROUP_get_curve(group1, p1, a1, b1, ctx)) + goto err; + if (!EC_GROUP_get_curve(group2, p2, a2, b2, ctx)) + goto err; + + if (BN_cmp(p1, p2) != 0 || BN_cmp(a1, a2) != 0 || BN_cmp(b1, b2) != 0) + goto distinct; + + if ((generator1 = EC_GROUP_get0_generator(group1)) == NULL) + goto err; + if ((generator2 = EC_GROUP_get0_generator(group2)) == NULL) goto err; /* - * XXX This approach assumes that the external representation of - * curves over the same field type is the same. + * It does not matter whether group1 or group2 is used: both points must + * have a matching method for this to succeed. */ - if (!a->meth->group_get_curve(a, a1, a2, a3, ctx) || - !b->meth->group_get_curve(b, b1, b2, b3, ctx)) - r = 1; - - if (r || BN_cmp(a1, b1) || BN_cmp(a2, b2) || BN_cmp(a3, b3)) - r = 1; - - /* XXX EC_POINT_cmp() assumes that the methods are equal */ - if (r || EC_POINT_cmp(a, EC_GROUP_get0_generator(a), - EC_GROUP_get0_generator(b), ctx)) - r = 1; - - if (!r) { - /* compare the order and cofactor */ - if (!EC_GROUP_get_order(a, a1, ctx) || - !EC_GROUP_get_order(b, b1, ctx) || - !EC_GROUP_get_cofactor(a, a2, ctx) || - !EC_GROUP_get_cofactor(b, b2, ctx)) - goto err; - if (BN_cmp(a1, b1) || BN_cmp(a2, b2)) - r = 1; - } - BN_CTX_end(ctx); - if (ctx_new) - BN_CTX_free(ctx); + if ((cmp = EC_POINT_cmp(group1, generator1, generator2, ctx)) < 0) + goto err; + if (cmp == 1) + goto distinct; + cmp = 1; + + if ((order1 = EC_GROUP_get0_order(group1)) == NULL) + goto err; + if ((order2 = EC_GROUP_get0_order(group2)) == NULL) + goto err; + + if ((cofactor1 = EC_GROUP_get0_cofactor(group1)) == NULL) + goto err; + if ((cofactor2 = EC_GROUP_get0_cofactor(group2)) == NULL) + goto err; + + if (BN_cmp(order1, order2) != 0 || BN_cmp(cofactor1, cofactor2) != 0) + goto distinct; - return r; + /* All parameters match: the groups are equal. */ + cmp = 0; + + distinct: + ret = cmp; err: BN_CTX_end(ctx); - if (ctx_new) + + if (ctx != ctx_in) BN_CTX_free(ctx); - return -1; + + return ret; } LCRYPTO_ALIAS(EC_GROUP_cmp); |