Clarify that we perform the point-on-curve check.
Points not on the curve are invalid inputs to EC functions, so EC
implementations should check the curve equation whenever importing
points from the caller. Sadly, a number of implementations, including
older OpenSSLs, miss this important check, so careful callers want this
clarified in the documentation.
Also update the note about OpenSSL to reflect the current behavior.
While I'm here, const-correct EC_KEY_key2buf.
Change-Id: I6fde5c823c4f3f6b141ba1566f427d96cd5881df
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/39364
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/crypto/fipsmodule/ec/ec_key.c b/crypto/fipsmodule/ec/ec_key.c
index fcdc687..0d9ce67 100644
--- a/crypto/fipsmodule/ec/ec_key.c
+++ b/crypto/fipsmodule/ec/ec_key.c
@@ -394,7 +394,7 @@
return ok;
}
-size_t EC_KEY_key2buf(EC_KEY *key, point_conversion_form_t form,
+size_t EC_KEY_key2buf(const EC_KEY *key, point_conversion_form_t form,
unsigned char **out_buf, BN_CTX *ctx) {
if (key == NULL || key->pub_key == NULL || key->group == NULL) {
return 0;
diff --git a/include/openssl/ec.h b/include/openssl/ec.h
index fcecb62..cfad93e 100644
--- a/include/openssl/ec.h
+++ b/include/openssl/ec.h
@@ -223,8 +223,14 @@
// EC_POINT_set_affine_coordinates_GFp sets the value of |point| to be
// (|x|, |y|). The |ctx| argument may be used if not NULL. It returns one
-// on success or zero on error. Note that, unlike with OpenSSL, it's
-// considered an error if the point is not on the curve.
+// on success or zero on error. It's considered an error if the point is not on
+// the curve.
+//
+// Note that the corresponding function in OpenSSL versions prior to 1.0.2s does
+// not check if the point is on the curve. This is a security-critical check, so
+// code additionally supporting OpenSSL should repeat the check with
+// |EC_POINT_is_on_curve| or check for older OpenSSL versions with
+// |OPENSSL_VERSION_NUMBER|.
OPENSSL_EXPORT int EC_POINT_set_affine_coordinates_GFp(const EC_GROUP *group,
EC_POINT *point,
const BIGNUM *x,
@@ -248,8 +254,9 @@
BN_CTX *ctx);
// EC_POINT_oct2point sets |point| from |len| bytes of X9.62 format
-// serialisation in |buf|. It returns one on success and zero otherwise. The
-// |ctx| argument may be used if not NULL.
+// serialisation in |buf|. It returns one on success and zero on error. The
+// |ctx| argument may be used if not NULL. It's considered an error if |buf|
+// does not represent a point on the curve.
OPENSSL_EXPORT int EC_POINT_oct2point(const EC_GROUP *group, EC_POINT *point,
const uint8_t *buf, size_t len,
BN_CTX *ctx);
diff --git a/include/openssl/ec_key.h b/include/openssl/ec_key.h
index 1bc6d30..932ad8e 100644
--- a/include/openssl/ec_key.h
+++ b/include/openssl/ec_key.h
@@ -172,7 +172,8 @@
OPENSSL_EXPORT int EC_KEY_check_fips(const EC_KEY *key);
// EC_KEY_set_public_key_affine_coordinates sets the public key in |key| to
-// (|x|, |y|). It returns one on success and zero otherwise.
+// (|x|, |y|). It returns one on success and zero on error. It's considered an
+// error if |x| and |y| do not represent a point on |key|'s curve.
OPENSSL_EXPORT int EC_KEY_set_public_key_affine_coordinates(EC_KEY *key,
const BIGNUM *x,
const BIGNUM *y);
@@ -180,7 +181,8 @@
// EC_KEY_key2buf encodes the public key in |key| to an allocated octet string
// and sets |*out_buf| to point to it. It returns the length of the encoded
// octet string or zero if an error occurred.
-OPENSSL_EXPORT size_t EC_KEY_key2buf(EC_KEY *key, point_conversion_form_t form,
+OPENSSL_EXPORT size_t EC_KEY_key2buf(const EC_KEY *key,
+ point_conversion_form_t form,
unsigned char **out_buf, BN_CTX *ctx);
