From: Tobias Brunner <tobias@strongswan.org>
Date: Tue, 1 Dec 2020 09:13:30 +0000 (+0100)
Subject: Remove the ecp_x_coordinate_only option
X-Git-Tag: 5.9.2dr2~22^2~2
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=86fb24c2c51c42ec9340196fe7078f4d37c58b15;p=thirdparty%2Fstrongswan.git

Remove the ecp_x_coordinate_only option

This was for compatibility with very old releases and only complicates
things unnecessarily nowadays.
---

diff --git a/conf/options/charon.opt b/conf/options/charon.opt
index dc052a89a3..d57fb4cedf 100644
--- a/conf/options/charon.opt
+++ b/conf/options/charon.opt
@@ -129,9 +129,6 @@ charon.dns2
 charon.dos_protection = yes
 	Enable Denial of Service protection using cookies and aggressiveness checks.
 
-charon.ecp_x_coordinate_only = yes
-	Compliance with the errata for RFC 4753.
-
 charon.flush_auth_cfg = no
 	Free objects during authentication (might conflict with plugins).
 
diff --git a/src/libstrongswan/plugins/openssl/openssl_ec_diffie_hellman.c b/src/libstrongswan/plugins/openssl/openssl_ec_diffie_hellman.c
index e3b4ca7116..6f58c2ceb0 100644
--- a/src/libstrongswan/plugins/openssl/openssl_ec_diffie_hellman.c
+++ b/src/libstrongswan/plugins/openssl/openssl_ec_diffie_hellman.c
@@ -120,7 +120,7 @@ error:
  * the point. This function allocates memory for the chunk.
  */
 static bool ecp2chunk(const EC_GROUP *group, const EC_POINT *point,
-					  chunk_t *chunk, bool x_coordinate_only)
+					  chunk_t *chunk)
 {
 	BN_CTX *ctx;
 	BIGNUM *x, *y;
@@ -145,10 +145,6 @@ static bool ecp2chunk(const EC_GROUP *group, const EC_POINT *point,
 		goto error;
 	}
 
-	if (x_coordinate_only)
-	{
-		y = NULL;
-	}
 	if (!openssl_bn_cat(EC_FIELD_ELEMENT_LEN(group), x, y, chunk))
 	{
 		goto error;
@@ -167,66 +163,18 @@ error:
 static bool compute_shared_key(private_openssl_ec_diffie_hellman_t *this,
 							   chunk_t *shared_secret)
 {
-	const BIGNUM *priv_key;
-	EC_POINT *secret = NULL;
-	bool x_coordinate_only, ret = FALSE;
 	int len;
 
-	/*
-	 * The default setting ecp_x_coordinate_only = TRUE
-	 * applies the following errata for RFC 4753:
-	 * http://www.rfc-editor.org/errata_search.php?eid=9
-	 * ECDH_compute_key() is used under this setting as
-	 * it also facilitates hardware offload through the use of
-	 * dynamic engines in OpenSSL.
-	 */
-	x_coordinate_only = lib->settings->get_bool(lib->settings,
-									"%s.ecp_x_coordinate_only", TRUE, lib->ns);
-	if (x_coordinate_only)
-	{
-		*shared_secret = chunk_alloc(EC_FIELD_ELEMENT_LEN(this->ec_group));
-		len = ECDH_compute_key(shared_secret->ptr, shared_secret->len,
-							   this->pub_key, this->key, NULL);
-		if (len <= 0)
-		{
-			chunk_free(shared_secret);
-			goto error;
-		}
-		shared_secret->len = len;
-	}
-	else
+	*shared_secret = chunk_alloc(EC_FIELD_ELEMENT_LEN(this->ec_group));
+	len = ECDH_compute_key(shared_secret->ptr, shared_secret->len,
+						   this->pub_key, this->key, NULL);
+	if (len <= 0)
 	{
-		priv_key = EC_KEY_get0_private_key(this->key);
-		if (!priv_key)
-		{
-			goto error;
-		}
-
-		secret = EC_POINT_new(this->ec_group);
-		if (!secret)
-		{
-			goto error;
-		}
-
-		if (!EC_POINT_mul(this->ec_group, secret, NULL, this->pub_key, priv_key,
-						  NULL))
-		{
-			goto error;
-		}
-
-		if (!ecp2chunk(this->ec_group, secret, shared_secret, x_coordinate_only))
-		{
-			goto error;
-		}
-	}
-
-	ret = TRUE;
-error:
-	if (secret)
-	{
-		EC_POINT_clear_free(secret);
+		chunk_free(shared_secret);
+		return FALSE;
 	}
-	return ret;
+	shared_secret->len = len;
+	return TRUE;
 }
 
 METHOD(diffie_hellman_t, set_other_public_value, bool,
@@ -257,7 +205,7 @@ METHOD(diffie_hellman_t, set_other_public_value, bool,
 METHOD(diffie_hellman_t, get_my_public_value, bool,
 	private_openssl_ec_diffie_hellman_t *this,chunk_t *value)
 {
-	ecp2chunk(this->ec_group, EC_KEY_get0_public_key(this->key), value, FALSE);
+	ecp2chunk(this->ec_group, EC_KEY_get0_public_key(this->key), value);
 	return TRUE;
 }
 
diff --git a/src/libstrongswan/plugins/pkcs11/pkcs11_dh.c b/src/libstrongswan/plugins/pkcs11/pkcs11_dh.c
index 565a57f9a6..747dc62d0f 100644
--- a/src/libstrongswan/plugins/pkcs11/pkcs11_dh.c
+++ b/src/libstrongswan/plugins/pkcs11/pkcs11_dh.c
@@ -139,12 +139,6 @@ METHOD(diffie_hellman_t, set_other_public_value, bool,
 				pubkey.len,
 				pubkey.ptr,
 			};
-
-			if (!lib->settings->get_bool(lib->settings,
-									"%s.ecp_x_coordinate_only", TRUE, lib->ns))
-			{	/* we only get the x coordinate back */
-				return FALSE;
-			}
 			value = chunk_from_thing(params);
 			break;
 		}
diff --git a/src/libstrongswan/plugins/wolfssl/wolfssl_ec_diffie_hellman.c b/src/libstrongswan/plugins/wolfssl/wolfssl_ec_diffie_hellman.c
index ba65006948..4d3e8e21cf 100644
--- a/src/libstrongswan/plugins/wolfssl/wolfssl_ec_diffie_hellman.c
+++ b/src/libstrongswan/plugins/wolfssl/wolfssl_ec_diffie_hellman.c
@@ -153,7 +153,6 @@ static bool compute_shared_key(private_wolfssl_ec_diffie_hellman_t *this,
 							   ecc_point *pub_key, chunk_t *shared_secret)
 {
 	ecc_point* secret;
-	bool x_coordinate_only;
 	bool success = FALSE;
 
 	if ((secret = wc_ecc_new_point()) == NULL)
@@ -163,15 +162,7 @@ static bool compute_shared_key(private_wolfssl_ec_diffie_hellman_t *this,
 
 	if (wolfssl_ecc_multiply(this->key.dp, &this->key.k, pub_key, secret))
 	{
-		/*
-		 * The default setting ecp_x_coordinate_only = TRUE
-		 * applies the following errata for RFC 4753:
-		 * http://www.rfc-editor.org/errata_search.php?eid=9
-		 */
-		x_coordinate_only = lib->settings->get_bool(lib->settings,
-									 "%s.ecp_x_coordinate_only", TRUE, lib->ns);
-		success = ecp2chunk(this->keysize, secret, shared_secret,
-							x_coordinate_only);
+		success = ecp2chunk(this->keysize, secret, shared_secret, TRUE);
 	}
 
 	wc_ecc_del_point(secret);