diff --git a/crypto/fipsmodule/ec/ec_test.cc b/crypto/fipsmodule/ec/ec_test.cc index edcfeaac0..c54adb540 100644 --- a/crypto/fipsmodule/ec/ec_test.cc +++ b/crypto/fipsmodule/ec/ec_test.cc @@ -891,6 +891,29 @@ TEST_P(ECCurveTest, GPlusMinusG) { EXPECT_TRUE(EC_POINT_is_at_infinity(group(), sum.get())); } +// Test that we refuse to encode or decode the point at infinity. +TEST_P(ECCurveTest, EncodeInfinity) { + // The point at infinity is encoded as a single zero byte, but we do not + // support it. + static const uint8_t kInfinity[] = {0}; + bssl::UniquePtr inf(EC_POINT_new(group())); + ASSERT_TRUE(inf); + EXPECT_FALSE(EC_POINT_oct2point(group(), inf.get(), kInfinity, + sizeof(kInfinity), nullptr)); + + // Encoding it also fails. + ASSERT_TRUE(EC_POINT_set_to_infinity(group(), inf.get())); + uint8_t buf[128]; + EXPECT_EQ( + 0u, EC_POINT_point2oct(group(), inf.get(), POINT_CONVERSION_UNCOMPRESSED, + buf, sizeof(buf), nullptr)); + + // Measuring the length of the encoding also fails. + EXPECT_EQ( + 0u, EC_POINT_point2oct(group(), inf.get(), POINT_CONVERSION_UNCOMPRESSED, + nullptr, 0, nullptr)); +} + static std::vector AllCurves() { const size_t num_curves = EC_get_builtin_curves(nullptr, 0); std::vector curves(num_curves); diff --git a/crypto/fipsmodule/ec/internal.h b/crypto/fipsmodule/ec/internal.h index f6c8e8a63..0d535468d 100644 --- a/crypto/fipsmodule/ec/internal.h +++ b/crypto/fipsmodule/ec/internal.h @@ -439,11 +439,18 @@ int ec_get_x_coordinate_as_bytes(const EC_GROUP *group, uint8_t *out, size_t *out_len, size_t max_out, const EC_RAW_POINT *p); -// ec_point_to_bytes behaves like |EC_POINT_point2oct| but takes an -// |EC_AFFINE|. +// ec_point_byte_len returns the number of bytes in the byte representation of +// a non-infinity point in |group|, encoded according to |form|, or zero if +// |form| is invalid. +size_t ec_point_byte_len(const EC_GROUP *group, point_conversion_form_t form); + +// ec_point_to_bytes encodes |point| according to |form| and writes the result +// |buf|. It returns the size of the output on success or zero on error. At most +// |max_out| bytes will be written. The buffer should be at least +// |ec_point_byte_len| long to guarantee success. size_t ec_point_to_bytes(const EC_GROUP *group, const EC_AFFINE *point, point_conversion_form_t form, uint8_t *buf, - size_t len); + size_t max_out); // ec_point_from_uncompressed parses |in| as a point in uncompressed form and // sets the result to |out|. It returns one on success and zero if the input was diff --git a/crypto/fipsmodule/ec/oct.c b/crypto/fipsmodule/ec/oct.c index ddd0f37a1..70326355c 100644 --- a/crypto/fipsmodule/ec/oct.c +++ b/crypto/fipsmodule/ec/oct.c @@ -73,9 +73,7 @@ #include "internal.h" -size_t ec_point_to_bytes(const EC_GROUP *group, const EC_AFFINE *point, - point_conversion_form_t form, uint8_t *buf, - size_t len) { +size_t ec_point_byte_len(const EC_GROUP *group, point_conversion_form_t form) { if (form != POINT_CONVERSION_COMPRESSED && form != POINT_CONVERSION_UNCOMPRESSED) { OPENSSL_PUT_ERROR(EC, EC_R_INVALID_FORM); @@ -88,27 +86,30 @@ size_t ec_point_to_bytes(const EC_GROUP *group, const EC_AFFINE *point, // Uncompressed points have a second coordinate. output_len += field_len; } + return output_len; +} - // if 'buf' is NULL, just return required length - if (buf != NULL) { - if (len < output_len) { - OPENSSL_PUT_ERROR(EC, EC_R_BUFFER_TOO_SMALL); - return 0; - } +size_t ec_point_to_bytes(const EC_GROUP *group, const EC_AFFINE *point, + point_conversion_form_t form, uint8_t *buf, + size_t len) { + size_t output_len = ec_point_byte_len(group, form); + if (len < output_len) { + OPENSSL_PUT_ERROR(EC, EC_R_BUFFER_TOO_SMALL); + return 0; + } - size_t field_len_out; - ec_felem_to_bytes(group, buf + 1, &field_len_out, &point->X); - assert(field_len_out == field_len); + size_t field_len; + ec_felem_to_bytes(group, buf + 1, &field_len, &point->X); + assert(field_len == BN_num_bytes(&group->field)); - if (form == POINT_CONVERSION_UNCOMPRESSED) { - ec_felem_to_bytes(group, buf + 1 + field_len, &field_len_out, &point->Y); - assert(field_len_out == field_len); - buf[0] = form; - } else { - uint8_t y_buf[EC_MAX_BYTES]; - ec_felem_to_bytes(group, y_buf, &field_len_out, &point->Y); - buf[0] = form + (y_buf[field_len_out - 1] & 1); - } + if (form == POINT_CONVERSION_UNCOMPRESSED) { + ec_felem_to_bytes(group, buf + 1 + field_len, &field_len, &point->Y); + assert(field_len == BN_num_bytes(&group->field)); + buf[0] = form; + } else { + uint8_t y_buf[EC_MAX_BYTES]; + ec_felem_to_bytes(group, y_buf, &field_len, &point->Y); + buf[0] = form + (y_buf[field_len - 1] & 1); } return output_len; @@ -214,6 +215,15 @@ size_t EC_POINT_point2oct(const EC_GROUP *group, const EC_POINT *point, OPENSSL_PUT_ERROR(EC, EC_R_INCOMPATIBLE_OBJECTS); return 0; } + if (buf == NULL) { + // When |buf| is NULL, just return the number of bytes that would be + // written, without doing an expensive Jacobian-to-affine conversion. + if (ec_GFp_simple_is_at_infinity(group, &point->raw)) { + OPENSSL_PUT_ERROR(EC, EC_R_POINT_AT_INFINITY); + return 0; + } + return ec_point_byte_len(group, form); + } EC_AFFINE affine; if (!ec_jacobian_to_affine(group, &affine, &point->raw)) { return 0; diff --git a/crypto/trust_token/pmbtoken.c b/crypto/trust_token/pmbtoken.c index 68d8909b9..ab09f0199 100644 --- a/crypto/trust_token/pmbtoken.c +++ b/crypto/trust_token/pmbtoken.c @@ -123,8 +123,7 @@ err: static int point_to_cbb(CBB *out, const EC_GROUP *group, const EC_AFFINE *point) { - size_t len = - ec_point_to_bytes(group, point, POINT_CONVERSION_UNCOMPRESSED, NULL, 0); + size_t len = ec_point_byte_len(group, POINT_CONVERSION_UNCOMPRESSED); if (len == 0) { return 0; } diff --git a/crypto/trust_token/voprf.c b/crypto/trust_token/voprf.c index cedee1e80..f8e1c4cfd 100644 --- a/crypto/trust_token/voprf.c +++ b/crypto/trust_token/voprf.c @@ -62,8 +62,7 @@ static int voprf_init_method(VOPRF_METHOD *method, int curve_nid, static int cbb_add_point(CBB *out, const EC_GROUP *group, const EC_AFFINE *point) { - size_t len = - ec_point_to_bytes(group, point, POINT_CONVERSION_UNCOMPRESSED, NULL, 0); + size_t len = ec_point_byte_len(group, POINT_CONVERSION_UNCOMPRESSED); if (len == 0) { return 0; }