Index | Thread | Search

From:
Theo Buehler <tb@theobuehler.org>
Subject:
Re: ssh: affine_coordinates_GFp() -> affine_coordinates()
To:
tech@openbsd.org
Cc:
jsing@openbsd.org, djm@openbsd.org, dtucker@openbsd.org
Date:
Sat, 3 May 2025 03:17:50 +0200

Download raw body.

Thread
On Fri, Apr 25, 2025 at 09:13:07AM +0200, Theo Buehler wrote:
> The EC_POINT_[sg]et_affine_coordinates_GF(p|2m) API never made much
> sense since the group argument would know whether it represents a prime
> curve or a binary curve and allow calling the appropriate handler from
> within libcrypto.
> 
> OpenSSL 1.1 recognized this and made them wrappers around a new
> EC_POINT_[sg]et_affine_coordinates() API which would do precisely that.
> This API has been available since LibreSSL 3.4 and is equally as
> deprecated as all the "low level" (read "somewhat usable") EC API in
> OpenSSL 3.
> 
> I want to remove the _GFp() API from libcrypto and some ssh-related code
> is in the way of that. I will of course upstream the change to libfido2.
> Does portable openssh still need to care about pre-3.4 libressl?

I know it's a boring diff :)

Index: lib/libfido2/src/es256.c
===================================================================
RCS file: /cvs/src/lib/libfido2/src/es256.c,v
diff -u -p -r1.5 es256.c
--- lib/libfido2/src/es256.c	29 Aug 2022 03:04:29 -0000	1.5
+++ lib/libfido2/src/es256.c	25 Apr 2025 06:56:07 -0000
@@ -298,7 +298,7 @@ es256_pk_to_EVP_PKEY(const es256_pk_t *k
 	}
 
 	if ((q = EC_POINT_new(g)) == NULL ||
-	    EC_POINT_set_affine_coordinates_GFp(g, q, x, y, bnctx) == 0 ||
+	    EC_POINT_set_affine_coordinates(g, q, x, y, bnctx) == 0 ||
 	    EC_KEY_set_public_key(ec, q) == 0) {
 		fido_log_debug("%s: EC_KEY_set_public_key", __func__);
 		goto fail;
@@ -363,10 +363,10 @@ es256_pk_from_EC_KEY(es256_pk_t *pk, con
 		goto fail;
 	}
 
-	if (EC_POINT_get_affine_coordinates_GFp(g, q, x, y, bnctx) == 0 ||
+	if (EC_POINT_get_affine_coordinates(g, q, x, y, bnctx) == 0 ||
 	    (nx = BN_num_bytes(x)) < 0 || (size_t)nx > sizeof(pk->x) ||
 	    (ny = BN_num_bytes(y)) < 0 || (size_t)ny > sizeof(pk->y)) {
-		fido_log_debug("%s: EC_POINT_get_affine_coordinates_GFp",
+		fido_log_debug("%s: EC_POINT_get_affine_coordinates",
 		    __func__);
 		goto fail;
 	}
Index: regress/usr.bin/ssh/misc/ssh-verify-attestation/ssh-verify-attestation.c
===================================================================
RCS file: /cvs/src/regress/usr.bin/ssh/misc/ssh-verify-attestation/ssh-verify-attestation.c,v
diff -u -p -r1.2 ssh-verify-attestation.c
--- regress/usr.bin/ssh/misc/ssh-verify-attestation/ssh-verify-attestation.c	6 Dec 2024 10:37:42 -0000	1.2
+++ regress/usr.bin/ssh/misc/ssh-verify-attestation/ssh-verify-attestation.c	25 Apr 2025 06:57:32 -0000
@@ -162,8 +162,8 @@ get_pubkey_from_cred_ecdsa(const fido_cr
 		error_f("BN_bin2bn failed");
 		goto out;
 	}
-	if (EC_POINT_set_affine_coordinates_GFp(g, q, x, y, NULL) != 1) {
-		error_f("EC_POINT_set_affine_coordinates_GFp failed");
+	if (EC_POINT_set_affine_coordinates(g, q, x, y, NULL) != 1) {
+		error_f("EC_POINT_set_affine_coordinates failed");
 		goto out;
 	}
 	*pubkey_len = EC_POINT_point2oct(g, q,
Index: regress/usr.bin/ssh/unittests/sshbuf/test_sshbuf_getput_crypto.c
===================================================================
RCS file: /cvs/src/regress/usr.bin/ssh/unittests/sshbuf/test_sshbuf_getput_crypto.c,v
diff -u -p -r1.3 test_sshbuf_getput_crypto.c
--- regress/usr.bin/ssh/unittests/sshbuf/test_sshbuf_getput_crypto.c	14 Dec 2021 21:25:27 -0000	1.3
+++ regress/usr.bin/ssh/unittests/sshbuf/test_sshbuf_getput_crypto.c	25 Apr 2025 06:57:18 -0000
@@ -218,7 +218,7 @@ sshbuf_getput_crypto_tests(void)
 	ASSERT_PTR_NE(ecp, NULL);
 	MKBN(ec256_x, bn_x);
 	MKBN(ec256_y, bn_y);
-	ASSERT_INT_EQ(EC_POINT_set_affine_coordinates_GFp(
+	ASSERT_INT_EQ(EC_POINT_set_affine_coordinates(
 	    EC_KEY_get0_group(eck), ecp, bn_x, bn_y, NULL), 1);
 	ASSERT_INT_EQ(EC_KEY_set_public_key(eck, ecp), 1);
 	BN_free(bn_x);
@@ -247,7 +247,7 @@ sshbuf_getput_crypto_tests(void)
 	bn_y = BN_new();
 	ASSERT_PTR_NE(bn_x, NULL);
 	ASSERT_PTR_NE(bn_y, NULL);
-	ASSERT_INT_EQ(EC_POINT_get_affine_coordinates_GFp(
+	ASSERT_INT_EQ(EC_POINT_get_affine_coordinates(
 	    EC_KEY_get0_group(eck), EC_KEY_get0_public_key(eck),
 	    bn_x, bn_y, NULL), 1);
 	MKBN(ec256_x, bn);
Index: usr.bin/ssh/sk-usbhid.c
===================================================================
RCS file: /cvs/src/usr.bin/ssh/sk-usbhid.c,v
diff -u -p -r1.47 sk-usbhid.c
--- usr.bin/ssh/sk-usbhid.c	3 Dec 2024 08:31:49 -0000	1.47
+++ usr.bin/ssh/sk-usbhid.c	25 Apr 2025 06:51:10 -0000
@@ -508,8 +508,8 @@ pack_public_key_ecdsa(const fido_cred_t 
 		skdebug(__func__, "BN_bin2bn failed");
 		goto out;
 	}
-	if (EC_POINT_set_affine_coordinates_GFp(g, q, x, y, NULL) != 1) {
-		skdebug(__func__, "EC_POINT_set_affine_coordinates_GFp failed");
+	if (EC_POINT_set_affine_coordinates(g, q, x, y, NULL) != 1) {
+		skdebug(__func__, "EC_POINT_set_affine_coordinates failed");
 		goto out;
 	}
 	response->public_key_len = EC_POINT_point2oct(g, q,
Index: usr.bin/ssh/sshkey.c
===================================================================
RCS file: /cvs/src/usr.bin/ssh/sshkey.c,v
diff -u -p -r1.148 sshkey.c
--- usr.bin/ssh/sshkey.c	3 Dec 2024 15:53:51 -0000	1.148
+++ usr.bin/ssh/sshkey.c	25 Apr 2025 06:52:51 -0000
@@ -2679,8 +2679,7 @@ sshkey_ec_validate_public(const EC_GROUP
 
 	/* log2(x) > log2(order)/2, log2(y) > log2(order)/2 */
 	if (EC_GROUP_get_order(group, order, NULL) != 1 ||
-	    EC_POINT_get_affine_coordinates_GFp(group, public,
-	    x, y, NULL) != 1) {
+	    EC_POINT_get_affine_coordinates(group, public, x, y, NULL) != 1) {
 		ret = SSH_ERR_LIBCRYPTO_ERROR;
 		goto out;
 	}
@@ -2764,9 +2763,8 @@ sshkey_dump_ec_point(const EC_GROUP *gro
 		fprintf(stderr, "%s: BN_new failed\n", __func__);
 		goto out;
 	}
-	if (EC_POINT_get_affine_coordinates_GFp(group, point,
-	    x, y, NULL) != 1) {
-		fprintf(stderr, "%s: EC_POINT_get_affine_coordinates_GFp\n",
+	if (EC_POINT_get_affine_coordinates(group, point, x, y, NULL) != 1) {
+		fprintf(stderr, "%s: EC_POINT_get_affine_coordinates\n",
 		    __func__);
 		goto out;
 	}