Reject point at infinity in EC_KEY_set_public_key (#3101)

Add an explicit check in EC_KEY_set_public_key to reject the point at infinity. The point at infinity is not a valid public key.

This matches the hardening added in BoringSSL
google/boringssl@a135fe1.

Call-outs:
Parsing (EC_POINT_oct2point) continues to accept the infinity encoding
for OpenSSL compatibility, as added in
de33f5e, but it can no longer be
installed as a public key on an EC_KEY.

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license and the ISC license.

Author: nebeid

crypto/ec_extra/ec_asn1.c

@@ -562,6 +562,10 @@ EC_KEY *o2i_ECPublicKey(EC_KEY **keyp, const uint8_t **inp, long len) {
     OPENSSL_PUT_ERROR(EC, ERR_R_EC_LIB);
     return NULL;
   }
+  if (EC_POINT_is_at_infinity(ret->group, ret->pub_key)) {
+    OPENSSL_PUT_ERROR(EC, EC_R_POINT_AT_INFINITY);
+    return NULL;
+  }
   // save the point conversion form
   ret->conv_form = (point_conversion_form_t)(*inp[0] & ~0x01);
   *inp += len;

crypto/fipsmodule/ec/ec_key.c

@@ -206,6 +206,11 @@ int EC_KEY_set_public_key(EC_KEY *key, const EC_POINT *pub_key) {
     return 0;
   }
 
+  if (pub_key != NULL && EC_POINT_is_at_infinity(key->group, pub_key)) {
+    OPENSSL_PUT_ERROR(EC, EC_R_POINT_AT_INFINITY);
+    return 0;
+  }
+
   if (pub_key != NULL && EC_GROUP_cmp(key->group, pub_key->group, NULL) != 0) {
     OPENSSL_PUT_ERROR(EC, EC_R_GROUP_MISMATCH);
     return 0;

crypto/fipsmodule/ec/ec_test.cc

@@ -957,6 +957,12 @@ struct InvalidECPublicKey {
   size_t input_key_len;
   int nid;
 } kInvalidECPublicKeyInputs[] = {
+  /* Test 0: point at infinity. */
+  {
+    (const uint8_t *)"\x00",
+    1,
+    NID_X9_62_prime256v1
+  },
   /* Test 1: incorrect compresion representation. */
   {
     kP224PublicKey_wrong_compressed_byte,
@@ -1354,6 +1360,19 @@ TEST(ECTest, SetNULLKey) {
   EXPECT_FALSE(EC_KEY_get0_public_key(key.get()));
 }
 
+TEST(ECTest, PointAtInfinity) {
+  bssl::UniquePtr<EC_KEY> key(EC_KEY_new_by_curve_name(NID_X9_62_prime256v1));
+  ASSERT_TRUE(key);
+
+  bssl::UniquePtr<EC_POINT> inf(
+      EC_POINT_new(EC_KEY_get0_group(key.get())));
+  ASSERT_TRUE(inf);
+  ASSERT_TRUE(
+      EC_POINT_set_to_infinity(EC_KEY_get0_group(key.get()), inf.get()));
+  // Configuring a public key with the point at infinity is invalid.
+  EXPECT_FALSE(EC_KEY_set_public_key(key.get(), inf.get()));
+}
+
 TEST(ECTest, GroupMismatch) {
   bssl::UniquePtr<EC_KEY> key(EC_KEY_new_by_curve_name(NID_secp384r1));
   ASSERT_TRUE(key);

include/openssl/ec_key.h

@@ -82,7 +82,7 @@ OPENSSL_EXPORT const EC_POINT *EC_KEY_get0_public_key(const EC_KEY *key);
 // EC_KEY_set_public_key sets the public key of |key| to |pub|, by copying it.
 // It returns one on success and zero otherwise. |key| must already have had a
 // group configured (see |EC_KEY_set_group| and |EC_KEY_new_by_curve_name|), and
-// |pub| must also belong to that group.
+// |pub| must also belong to that group, and must not be the point at infinity.
 OPENSSL_EXPORT int EC_KEY_set_public_key(EC_KEY *key, const EC_POINT *pub);
 
 #define EC_PKEY_NO_PARAMETERS 0x001
@@ -273,8 +273,9 @@ OPENSSL_EXPORT int i2d_ECPKParameters_bio(BIO *bio, const EC_GROUP *group);
 
 // o2i_ECPublicKey parses an EC point from |len| bytes at |*inp| into
 // |*out_key|. Note that this differs from the d2i format in that |*out_key|
-// must be non-NULL with a group set. On successful exit, |*inp| is advanced by
-// |len| bytes. It returns |*out_key| or NULL on error.
+// must be non-NULL with a group set. The point must not be the point at
+// infinity. On successful exit, |*inp| is advanced by |len| bytes. It returns
+// |*out_key| or NULL on error.
 //
 // Use |EC_POINT_oct2point| instead.
 OPENSSL_EXPORT EC_KEY *o2i_ECPublicKey(EC_KEY **out_key, const uint8_t **inp,