openssl: Tests broken in master with enable-ec_nistp_64_gcc_128

Using secp384r1 instead should fix this, shouldn’t it?

@romen Do you have any objections to me adding a PR to do this for now? - just so the build is not broken. I think we are aware of the issue.

1. EC_GROUP_new_by_curve_name() and EC_GROUP_new_from_ecparameters() are selecting different methods. I assume this is because EC_GROUP_new_from_ecparameters is using a method that has extra functions for loading. When ec_nistp_64_gcc_128 is enabled shouldnt it end up using the same method? This may not be that simple.

Until very recently (#8555) we did not have a “lookup by parameters” function, only by NID. Actually even now the lookup functionality is not entirely there, but can be implemented with wrappers around EC_GROUP_check_named_curve().

Still there are 2 problems left with this:

1. this lookup is a very expensive operation in memory and operations that could open serious DoS consequences if it was done by default whenever a client sends points encoded with explicit parameters rather than named curve. A generic EC_GROUP must be created in EC_GROUP_new_from_ecparameters(), then EC_GROUP_check_named_curve() must be called on it to get the NID for that curve if it is one of the supported named curves doing a lot of BN_bn2hex() conversions and string comparisons, then an entirely new EC_GROUP must be created to replace the generic one if an NID is returned;
2. there are curves with same parameters but different names (aliases), so even doing what detailed above you could end up with 2 different EC_GROUPs, one created using a name and the other using an alias, that will have 2 different EC_GROUP->curve_name values and fail the comparison.

1. It doesnt seem logical to compare methods to see if 2 points are equal (especially for groups).

It makes sense because how to compare 2 points belonging to the same EC_GROUP object is defined in the associated EC_METHOD: https://github.com/openssl/openssl/blob/10c25644e362381844e0089504f0db42f029d855/crypto/ec/ec_lib.c#L893

Different EC_METHODs may store point coordinates in different forms (e.g. affine coordinates in simple methods, different flavours of projective in specialized methods) so only the EC_METHOD that created an EC_POINT knows how to read its coordinates.

Comparing 2 EC_POINTs from the same EC_GROUP and EC_METHOD is in general a relatively inexpensive operation, and in general in most places our code seems to assume it is a programmer’s error to create an EC_POINT from a EC_GROUP/EC_METHOD and then use functions from another EC_GROUP/EC_METHOD to manipulate it.

If we decide that EC_POINT_cmp() has to support mixing-and-matching for EC_POINT objects and EC_GROUP/EC_METHOD objects, then we need the EC_POINT_cmp() wrapper to actually take on that job, convert both points to a canonical form (e.g. affine coordinates) and compare the canonical representations instead. But then we have a catch-22 paradox, because to compare 2 EC_GROUPs we need to eventually compare the 2 generators, but evaluating EC_POINT_cmp() on 2 EC_POINTs created from 2 non-trivially-identical EC_GROUP/EC_METHODs generally should require, in addition to comparing their canonical representations, that they belong to the same elliptic curve group (as in the same equation with the same parameters and the same generator, rather than the same EC_GROUP programming object). When calling EC_POINT_cmp() from within EC_GROUP_cmp() the paradox is trivially solved because EC_GROUP_cmp() checks all the other parameters before comparing the generators, so what EC_GROUP_cmp() should probably do is never call EC_POINT_cmp() but locally do conversion to affine coordinates and compare the resulting BIGNUMs. This comes at the cost of 4 extra BIGNUM allocations, 2 potentially computationally expensive conversions from projective to affine, and probably more computationally expensive BN_cmp() in affine form than in projective form.

Still I believe it’s a reasonable change, but it can happen only after we solve #8615 (potentailly a behavioural change in the public API) and the ambiguity of what exactly EC_GROUP_cmp() should return: are two EC_GROUP objects identical if they represent the same identical mathematical abstraction or if they are compatible from a programming perspective?

In the latter case the programmer can reasonably assume that EC_POINT objects belonging to EC_GROUP objects for which EC_GROUP_cmp() == true are interoperable from a programming perspecitve. In the first case a programmer can only assume that they belong to the same mathematical abstraction, but nothing more, so he still needs to make sure to never mix EC_POINTs belonging (as in created by) to different EC_GROUPs and to do conversions (EC_POINT_get_affine(group_a, point, x, y) + equivalent_point_in_group_b=EC_POINT_new(group_b) + EC_POINT_set_affine(group_b, equivalent_point_in_group_b, x, y)) before comparing points that belong to the same mathematical abstraction but to non-identical EC_GROUP objects.

All of this has implications for internal use in libcrypto, libssl and apps/*, and for external applications as well, because it directly affects key comparisons, which can have unexpected consequences in PKI evaluation (either through dedicated libcrypto functions or through direct manipulation at the EC layer) or on the state machine for handling protocols.

@paulidale pointed out that this could potentially become even more of an issue when ec is moved to providers.

The behaviour of EC_GROUP_cmp() is not clearly defined (#8615): until that is clarified it’s really difficult to understand what needs to be fixed: documentation, implementations or tests? Currently (1.02, 1.1.0, 1.1.1) the EC subsystem is not directly manipulated by ENGINES, which can at most define an alternative EC(DSA|DH)_KEY_METHOD to implement alternative keygen/derive/sign/verify methods but either implement their internal EC stack or reuse the unmodified OpenSSL one (without much flexibility in expanding it as there is no EC_METHOD_new() nor setters no access to inject entries in the lookup tables, OID/ASN1 details, etc. If this is going to change with the transition to providers then yes, we need to clarify all the issues related to this and other documentation problems of EC to make sure we are internally consistent and have a clear contract for external implementations.