PR comments; add hybrid sanity check

crypto/fipsmodule/ec/oct.c

@@ -72,6 +72,26 @@
 
 #include "internal.h"
 
+// TODO: Actually create |ec_felem_is_odd|, so we don't need the conversion to
+//       |BN| below.
+static int ec_felem_is_odd(const EC_GROUP *group, const int form_bit,
+                           const EC_FELEM *y) {
+  int ret = 0;
+  const int y_bit = form_bit & 1;
+  BIGNUM *y_check = BN_new();
+  if (y_check == NULL || !ec_felem_to_bignum(group, y_check, y)) {
+    goto end;
+  }
+  if (BN_is_odd(y_check) != y_bit) {
+    OPENSSL_PUT_ERROR(EC, EC_R_INVALID_ENCODING);
+    goto end;
+  }
+
+  ret = 1;
+end:
+  BN_free(y_check);
+  return ret;
+}
 
 size_t ec_point_byte_len(const EC_GROUP *group, point_conversion_form_t form) {
   if (form != POINT_CONVERSION_COMPRESSED &&
@@ -115,6 +135,10 @@ size_t ec_point_to_bytes(const EC_GROUP *group, const EC_AFFINE *point,
     if (form == POINT_CONVERSION_HYBRID) {
       // |POINT_CONVERSION_HYBRID| specifies y's solution of the quadratic
       // equation, but also encodes the y coordinate along with it.
+      if (!ec_felem_is_odd(group, buf[0], &point->Y)) {
+        OPENSSL_PUT_ERROR(EC, EC_R_INVALID_ENCODING);
+        return 0;
+      }
       OPENSSL_memcpy(buf + 1 + field_len, y_buf, field_len);
     }
   }
@@ -125,10 +149,31 @@ size_t ec_point_to_bytes(const EC_GROUP *group, const EC_AFFINE *point,
 int ec_point_from_uncompressed(const EC_GROUP *group, EC_AFFINE *out,
                                const uint8_t *in, size_t len) {
   const size_t field_len = BN_num_bytes(&group->field.N);
+  if (len != 1 + 2 * field_len || in[0] != POINT_CONVERSION_UNCOMPRESSED) {
+    OPENSSL_PUT_ERROR(EC, EC_R_INVALID_ENCODING);
+    return 0;
+  }
+
+  EC_FELEM x, y;
+  if (!ec_felem_from_bytes(group, &x, in + 1, field_len) ||
+      !ec_felem_from_bytes(group, &y, in + 1 + field_len, field_len) ||
+      !ec_point_set_affine_coordinates(group, out, &x, &y)) {
+    return 0;
+  }
+
+  return 1;
+}
+
+static int is_point_conversion_form_hybrid(int form_bit) {
+  return POINT_CONVERSION_HYBRID == (form_bit & ~1u);
+}
+
+static int ec_point_from_hybrid(const EC_GROUP *group, EC_AFFINE *out,
+                               const uint8_t *in, size_t len) {
+  const size_t field_len = BN_num_bytes(&group->field.N);
   // |POINT_CONVERSION_HYBRID| has the solution of y encoded in the first byte
   // as well.
-  if (len != 1 + 2 * field_len || (in[0] != POINT_CONVERSION_UNCOMPRESSED &&
-                                   (in[0] & ~1u) != POINT_CONVERSION_HYBRID)) {
+  if (len != 1 + 2 * field_len || !is_point_conversion_form_hybrid(in[0])) {
     OPENSSL_PUT_ERROR(EC, EC_R_INVALID_ENCODING);
     return 0;
   }
@@ -140,9 +185,11 @@ int ec_point_from_uncompressed(const EC_GROUP *group, EC_AFFINE *out,
     return 0;
   }
 
-  return 1;
+  // Check that the encoded solution aligns with the computed point.
+  return ec_felem_is_odd(group, in[0], &y);
 }
 
+
 static int ec_GFp_simple_oct2point(const EC_GROUP *group, EC_POINT *point,
                                    const uint8_t *buf, size_t len,
                                    BN_CTX *ctx) {
@@ -165,8 +212,8 @@ static int ec_GFp_simple_oct2point(const EC_GROUP *group, EC_POINT *point,
 
   const int y_bit = form & 1;
   form = form & ~1u;
-  if (form == POINT_CONVERSION_UNCOMPRESSED ||
-      form == POINT_CONVERSION_HYBRID) {
+
+  if (form == POINT_CONVERSION_UNCOMPRESSED) {
     EC_AFFINE affine;
     if (!ec_point_from_uncompressed(group, &affine, buf, len)) {
       // In the event of an error, defend against the caller not checking the
@@ -178,6 +225,16 @@ static int ec_GFp_simple_oct2point(const EC_GROUP *group, EC_POINT *point,
     return 1;
   }
 
+  if (form == POINT_CONVERSION_HYBRID) {
+    EC_AFFINE affine;
+    if (!ec_point_from_hybrid(group, &affine, buf, len)) {
+      ec_set_to_safe_point(group, &point->raw);
+      return 0;
+    }
+    ec_affine_to_jacobian(group, &point->raw, &affine);
+    return 1;
+  }
+
   const size_t field_len = BN_num_bytes(&group->field.N);
   if (form != POINT_CONVERSION_COMPRESSED ||
       len != 1 /* type byte */ + field_len) {