ecc: fix incorrect derivation of compressed points

Commit Message

James Prestwood Oct. 10, 2023, 1:56 p.m. UTC

The logic was inversed here and was performing a subtraction if:
 - Y was even and type == BIT0
 - Y was odd and type == BIT1

This is not correct according to the ANSI spec. IWD relied on this
API but had matching incorrect logic so things "worked" up until
a compressed point needed to be parsed from an source that
explicitly specified the type (e.g. an ASN1 DER in DPP). All other
uses (PWD/SAE) the point type was only used to force a subtraction
so since both locations used the incorrect logic the points would
compute correctly.
---
 ell/ecc.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

Patch

diff --git a/ell/ecc.c b/ell/ecc.c
index 98ef812..73ddb96 100644
--- a/ell/ecc.c
+++ b/ell/ecc.c
@@ -562,8 +562,24 @@  LIB_EXPORT struct l_ecc_point *l_ecc_point_from_data(
 		if (!_ecc_compute_y(curve, p->y, p->x))
 			goto failed;
 
+		/*
+		 * This is determining whether or not to subtract the Y
+		 * coordinate from P. According to ANSI X9.62 an even Y should
+		 * be prefixed with 02 (BIT0) and an odd Y should be prefixed
+		 * with 03 (BIT1). If this is not the case, subtract Y from P.
+		 *
+		 * ANSI X9.62
+		 * 4.3.6 Point-to-Octet-String Conversion
+		 *
+		 * 2. If the compressed form is used, then do the following:
+		 *     2.1. Compute the bit ~Yp . (See Section 4.2.)
+		 *     2.2. Assign the value 02 to the single octet PC if ~Yp
+		 *          is 0, or the value 03 if ~Yp is 1.
+		 *     2.3. The result is the octet string PO = PC || X
+		 */
+
 		sub = secure_select(type == L_ECC_POINT_TYPE_COMPRESSED_BIT0,
-					!(p->y[0] & 1), p->y[0] & 1);
+					p->y[0] & 1, !(p->y[0] & 1));
 
 		_vli_mod_sub(tmp, curve->p, p->y, curve->p, curve->ndigits);