From 5463a67785d3cc07fa43b57537b93d1584e29156 Mon Sep 17 00:00:00 2001 From: tb Date: Tue, 22 Oct 2024 21:06:16 +0000 Subject: [PATCH] Start cleaning up oct2point and point2oct The SEC 1 standard defines various ways of encoding an elliptic curve point as ASN.1 octet string. It's also used for the public key, which isn't an octet string but a bit string for whatever historic reason. The public API is incomplete and inconvenient, so we need to jump through a few hoops to support it and to preserve our own sanity. Split a small helper function out of ec_GFp_simple_point2oct() that checks that a uint8_t represents a valid point conversion form. It supports exactly the four possible variants and helps translating from point_conversion_form_t at the API boundary. Reject the form for the point at infinity since the function has historically done that even for the case that the point actually is the point at infinity. ok jsing --- lib/libcrypto/ec/ecp_oct.c | 46 +++++++++++++++++++++++++++++++++----- 1 file changed, 41 insertions(+), 5 deletions(-) diff --git a/lib/libcrypto/ec/ecp_oct.c b/lib/libcrypto/ec/ecp_oct.c index 133704bd7f5..9646e44499c 100644 --- a/lib/libcrypto/ec/ecp_oct.c +++ b/lib/libcrypto/ec/ecp_oct.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ecp_oct.c,v 1.22 2024/10/22 12:09:57 tb Exp $ */ +/* $OpenBSD: ecp_oct.c,v 1.23 2024/10/22 21:06:16 tb Exp $ */ /* Includes code written by Lenka Fibikova * for the OpenSSL project. * Includes code written by Bodo Moeller for the OpenSSL project. @@ -63,6 +63,7 @@ */ #include +#include #include #include @@ -182,17 +183,52 @@ ec_GFp_simple_set_compressed_coordinates(const EC_GROUP *group, return ret; } +/* + * Only the last three bits of the leading octet of a point should be set. + * Bits 3 and 2 encode the conversion form for all points except the point + * at infinity. In compressed and hybrid form bit 1 indicates if the even + * or the odd solution of the quadratic equation for y should be used. + * + * The public point_conversion_t enum lacks the point at infinity, so we + * ignore it except at the API boundary. + */ + +#define EC_OCT_YBIT 0x01 + +#define EC_OCT_POINT_AT_INFINITY 0x00 +#define EC_OCT_POINT_COMPRESSED 0x02 +#define EC_OCT_POINT_UNCOMPRESSED 0x04 +#define EC_OCT_POINT_HYBRID 0x06 +#define EC_OCT_POINT_CONVERSION_MASK 0x06 + +static int +ec_oct_conversion_form_is_valid(uint8_t form) +{ + return (form & EC_OCT_POINT_CONVERSION_MASK) == form; +} + size_t ec_GFp_simple_point2oct(const EC_GROUP *group, const EC_POINT *point, - point_conversion_form_t form, unsigned char *buf, size_t len, BN_CTX *ctx) + point_conversion_form_t conversion_form, unsigned char *buf, size_t len, + BN_CTX *ctx) { + uint8_t form; BIGNUM *x, *y; size_t field_len, i, skip; size_t ret = 0; - if (form != POINT_CONVERSION_COMPRESSED && - form != POINT_CONVERSION_UNCOMPRESSED && - form != POINT_CONVERSION_HYBRID) { + if (conversion_form > UINT8_MAX) { + ECerror(EC_R_INVALID_FORM); + return 0; + } + + form = conversion_form; + + /* + * Established behavior is to reject a request for the form 0 for the + * point at infinity even if it is valid. + */ + if (form == 0 || !ec_oct_conversion_form_is_valid(form)) { ECerror(EC_R_INVALID_FORM); return 0; } -- 2.20.1