diff mbox series

[v2,1/2] ecc: fix incorrect derivation of compressed points

Message ID 20231010142506.261152-1-prestwoj@gmail.com (mailing list archive)
State New
Headers show
Series [v2,1/2] ecc: fix incorrect derivation of compressed points | expand

Checks

Context Check Description
tedd_an/pre-ci_am success Success
prestwoj/iwd-ci-build success Build - Configure
prestwoj/iwd-ci-clang success clang PASS
prestwoj/iwd-ci-makecheckvalgrind success Make Check w/Valgrind
prestwoj/iwd-ci-makecheck success Make Check
prestwoj/iwd-ci-makedistcheck success Make Distcheck
prestwoj/iwd-ci-testrunner success test-runner PASS

Commit Message

James Prestwood Oct. 10, 2023, 2:25 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(-)

Comments

Denis Kenzior Oct. 11, 2023, 2:55 p.m. UTC | #1
Hi James,

On 10/10/23 09:25, James Prestwood wrote:
> 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(-)
> 

Both applied, thanks.

Regards,
-Denis
diff mbox series

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);