Make it explicit that the EC_KEY setters don't check things

While EC_POINT_set_affine_coordinates() checks that the resulting point
is on the elliptic curve, this is only necessary, but not sufficient, to
ensure that the point can serve as a valid public key. For example, this
does not check for normalized coordinates or exclude that it is zero (the
point at infinity). Such checks, and more, are performed by the similarly
named EC_KEY_set_public_key_affine_coordinates().

This kind of makes sense from the mathematical standpoint as an elliptic
curve point isn't a priori a public key, even if you are not going to use
libcrypto for actual mathematics (or anything really) unless you like pain.
In a cryptographic library such differences are more of a hazard than a
help.

This is exacerbated by the fact that EC_KEY_set_public_key() does almost
no checking (it only checks that the point's EC_POINT method matches the
one of group set of the EC_KEY, which is far from enough). The API expects
that you call EC_KEY_check_key() on your own. This is kind of confusing
since EC_KEY_set_public_key_affine_coordinates() does that for you.

Unfortunately, adding sanity checks to EC_KEY_set_public_key() isn't easy
since it's going to penalize those who already check. Caching the result
of a check is dangerous and fragile if there are a million ways of fiddling
with an EC_KEY.

While the elliptic curve code is really bad, its documentation is worse
(another thing that applies to OpenSSL in general). Try to help that a
little bit by making it more explicit that you are supposed to call
EC_KEY_check_key() after using lower-level EC_KEY setters. Also make it
clearer that the setters copy the data, they don't take ownership (which
isn't obvious from the naming).

If OpenSSL 3 got one thing kind of right, it was to deprecate the EC_KEY
and EC_POINT APIs. But if you are going to deprecate something, you should
either be prepared to remove it or have a reasonable replacement...

Found by Guido Vranken using cryptofuzz
https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=66667

ok jsing

1 files changed, 6 insertions, 2 deletions

diff --git a/lib/libcrypto/man/EC_KEY_new.3 b/lib/libcrypto/man/EC_KEY_new.3
index 06afdd537ce..f415b91d6fc 100644
--- a/lib/libcrypto/man/EC_KEY_new.3
+++ b/lib/libcrypto/man/EC_KEY_new.3
@@ -1,4 +1,4 @@
-.\" $OpenBSD: EC_KEY_new.3,v 1.18 2023/08/29 10:07:42 tb Exp $
+.\" $OpenBSD: EC_KEY_new.3,v 1.19 2024/02/16 06:09:36 tb Exp $
 .\" full merge up to: OpenSSL 3aef36ff Jan 5 13:06:03 2016 -0500
 .\" partial merge up to: OpenSSL e9b77246 Jan 20 19:58:49 2017 +0100
 .\"
@@ -49,7 +49,7 @@
 .\" ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED
 .\" OF THE POSSIBILITY OF SUCH DAMAGE.
 .\"
-.Dd $Mdocdate: August 29 2023 $
+.Dd $Mdocdate: February 16 2024 $
 .Dt EC_KEY_NEW 3
 .Os
 .Sh NAME
@@ -324,6 +324,10 @@ object, the private key and the
 public key for the
 .Fa key ,
 respectively.
+The setters copy the group and key objects without sanity checks
+and it is the caller's responsibility to ensure that
+the resulting key is valid, for example using
+.Fn EC_KEY_check_key .
 .Pp
 The functions
 .Fn EC_KEY_get_enc_flags