Merge branch 'openssl-ecp'

Uses the EVP interface for ECDH with newer OpenSSL versions, which,
compared to the previous low-level use of EC_POINT_mul() supports
hardware offloading.  We used this because of the ecp_x_coordinate_only
option, which is now removed as it's been obsolete for a long time and
complicated the code.  There is still some legacy code for OpenSSL 1.0
and the old BoringSSL version we currently use for the Android app.

Closes strongswan/strongswan#186.

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

src/libstrongswan/plugins/openssl/openssl_ec_diffie_hellman.c

@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2008-2013 Tobias Brunner
+ * Copyright (C) 2008-2021 Tobias Brunner
  * HSR Hochschule fuer Technik Rapperswil
  *
  * This program is free software; you can redistribute it and/or modify it
@@ -17,10 +17,13 @@
 
 #ifndef OPENSSL_NO_EC
 
-#include <openssl/bn.h>
+#include <openssl/evp.h>
 #include <openssl/ec.h>
 #include <openssl/objects.h>
+
+#if OPENSSL_VERSION_NUMBER < 0x1010000fL || defined(OPENSSL_IS_BORINGSSL)
 #include <openssl/bn.h>
+#endif
 
 #include "openssl_ec_diffie_hellman.h"
 #include "openssl_util.h"
@@ -46,17 +49,12 @@ struct private_openssl_ec_diffie_hellman_t {
 	/**
 	 * EC private (public) key
 	 */
-	EC_KEY *key;
+	EVP_PKEY *key;
 
 	/**
 	 * EC group
 	 */
-	const EC_GROUP *ec_group;
+	EC_GROUP *ec_group;
-
-	/**
-	 * Other public key
-	 */
-	EC_POINT *pub_key;
 
 	/**
 	 * Shared secret
@@ -69,12 +67,15 @@ struct private_openssl_ec_diffie_hellman_t {
 	bool computed;
 };
 
+#if OPENSSL_VERSION_NUMBER < 0x1010000fL || defined(OPENSSL_IS_BORINGSSL)
 /**
- * Convert a chunk to an EC_POINT (which must already exist). The x and y
+ * Convert a chunk to an EC_POINT and set it on the given key. The x and y
  * coordinates of the point have to be concatenated in the chunk.
  */
-static bool chunk2ecp(const EC_GROUP *group, chunk_t chunk, EC_POINT *point)
+static bool chunk2ecp(const EC_GROUP *group, chunk_t chunk, EVP_PKEY *key)
 {
+	EC_POINT *point = NULL;
+	EC_KEY *pub = NULL;
 	BN_CTX *ctx;
 	BIGNUM *x, *y;
 	bool ret = FALSE;
@@ -98,7 +99,8 @@ static bool chunk2ecp(const EC_GROUP *group, chunk_t chunk, EC_POINT *point)
 		goto error;
 	}
 
-	if (!EC_POINT_set_affine_coordinates_GFp(group, point, x, y, ctx))
+	point = EC_POINT_new(group);
+	if (!point || !EC_POINT_set_affine_coordinates_GFp(group, point, x, y, ctx))
 	{
 		goto error;
 	}
@@ -108,20 +110,40 @@ static bool chunk2ecp(const EC_GROUP *group, chunk_t chunk, EC_POINT *point)
 		goto error;
 	}
 
+	pub = EC_KEY_new();
+	if (!pub || !EC_KEY_set_group(pub, group))
+	{
+		goto error;
+	}
+
+	if (EC_KEY_set_public_key(pub, point) != 1)
+	{
+		goto error;
+	}
+
+	if (EVP_PKEY_set1_EC_KEY(key, pub) != 1)
+	{
+		goto error;
+	}
+
 	ret = TRUE;
+
 error:
+	EC_POINT_clear_free(point);
+	EC_KEY_free(pub);
 	BN_CTX_end(ctx);
 	BN_CTX_free(ctx);
 	return ret;
 }
 
 /**
- * Convert an EC_POINT to a chunk by concatenating the x and y coordinates of
+ * Convert a key to a chunk by concatenating the x and y coordinates of
- * the point. This function allocates memory for the chunk.
+ * the underlying EC point. This function allocates memory for the chunk.
  */
-static bool ecp2chunk(const EC_GROUP *group, const EC_POINT *point,
+static bool ecp2chunk(const EC_GROUP *group, EVP_PKEY *key, chunk_t *chunk)
-					  chunk_t *chunk, bool x_coordinate_only)
 {
+	EC_KEY *ec_key = NULL;
+	const EC_POINT *point;
 	BN_CTX *ctx;
 	BIGNUM *x, *y;
 	bool ret = FALSE;
@@ -140,118 +162,101 @@ static bool ecp2chunk(const EC_GROUP *group, const EC_POINT *point,
 		goto error;
 	}
 
-	if (!EC_POINT_get_affine_coordinates_GFp(group, point, x, y, ctx))
+	ec_key = EVP_PKEY_get1_EC_KEY(key);
+	point = EC_KEY_get0_public_key(ec_key);
+	if (!point || !EC_POINT_get_affine_coordinates_GFp(group, point, x, y, ctx))
 	{
 		goto error;
 	}
 
-	if (x_coordinate_only)
-	{
-		y = NULL;
-	}
 	if (!openssl_bn_cat(EC_FIELD_ELEMENT_LEN(group), x, y, chunk))
 	{
 		goto error;
 	}
 
-	ret = TRUE;
+	ret = chunk->len != 0;
 error:
+	EC_KEY_free(ec_key);
 	BN_CTX_end(ctx);
 	BN_CTX_free(ctx);
 	return ret;
 }
-
+#endif /* OPENSSL_VERSION_NUMBER < ... */
-/**
- * Compute the shared secret.
- *
- * We cannot use the function ECDH_compute_key() because that returns only the
- * x coordinate of the shared secret point (which is defined, for instance, in
- * 'NIST SP 800-56A').
- * However, we need both coordinates as RFC 4753 says: "The Diffie-Hellman
- *   public value is obtained by concatenating the x and y values. The format
- *   of the Diffie-Hellman shared secret value is the same as that of the
- *   Diffie-Hellman public value."
- */
-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;
-
-	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;
-	}
-
-	/*
-	 * 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);
-	if (!ecp2chunk(this->ec_group, secret, shared_secret, x_coordinate_only))
-	{
-		goto error;
-	}
-
-	ret = TRUE;
-error:
-	if (secret)
-	{
-		EC_POINT_clear_free(secret);
-	}
-	return ret;
-}
 
 METHOD(diffie_hellman_t, set_other_public_value, bool,
 	private_openssl_ec_diffie_hellman_t *this, chunk_t value)
 {
+	EVP_PKEY *pub = NULL;
+
+	chunk_clear(&this->shared_secret);
+	this->computed = FALSE;
+
 	if (!diffie_hellman_verify_value(this->group, value))
 	{
 		return FALSE;
 	}
 
-	if (!chunk2ecp(this->ec_group, value, this->pub_key))
+	pub = EVP_PKEY_new();
+	if (!pub)
+	{
+		goto error;
+	}
+
+#if OPENSSL_VERSION_NUMBER < 0x1010000fL || defined(OPENSSL_IS_BORINGSSL)
+	if (!chunk2ecp(this->ec_group, value, pub))
 	{
 		DBG1(DBG_LIB, "ECDH public value is malformed");
-		return FALSE;
+		goto error;
 	}
+#else
+	/* OpenSSL expects the pubkey in the format specified in section 2.3.4 of
+	 * SECG SEC 1, i.e. prefixed with 0x04 to indicate an uncompressed point */
+	value = chunk_cata("cc", chunk_from_chars(0x04), value);
+	if (EVP_PKEY_copy_parameters(pub, this->key) <= 0 ||
+		EVP_PKEY_set1_tls_encodedpoint(pub, value.ptr, value.len) <= 0)
+	{
+		DBG1(DBG_LIB, "ECDH public value is malformed");
+		goto error;
+	}
+#endif
 
-	chunk_clear(&this->shared_secret);
+	if (!openssl_compute_shared_key(this->key, pub, &this->shared_secret))
-
+	{
-	if (!compute_shared_key(this, &this->shared_secret)) {
 		DBG1(DBG_LIB, "ECDH shared secret computation failed");
-		return FALSE;
+		goto error;
 	}
-
 	this->computed = TRUE;
-	return TRUE;
+
+error:
+	EVP_PKEY_free(pub);
+	return this->computed;
 }
 
 METHOD(diffie_hellman_t, get_my_public_value, bool,
-	private_openssl_ec_diffie_hellman_t *this,chunk_t *value)
+	private_openssl_ec_diffie_hellman_t *this, chunk_t *value)
 {
-	ecp2chunk(this->ec_group, EC_KEY_get0_public_key(this->key), value, FALSE);
+#if OPENSSL_VERSION_NUMBER < 0x1010000fL || defined(OPENSSL_IS_BORINGSSL)
-	return TRUE;
+	return ecp2chunk(this->ec_group, this->key, value);
+#else
+	chunk_t pub;
+
+	/* OpenSSL returns the pubkey in the format specified in section 2.3.4 of
+	 * SECG SEC 1, i.e. prefixed with 0x04 to indicate an uncompressed point */
+	pub.len = EVP_PKEY_get1_tls_encodedpoint(this->key, &pub.ptr);
+	if (pub.len != 0)
+	{
+		*value = chunk_clone(chunk_skip(pub, 1));
+		chunk_free(&pub);
+		return value->len != 0;
+	}
+	return FALSE;
+#endif
 }
 
 METHOD(diffie_hellman_t, set_private_value, bool,
 	private_openssl_ec_diffie_hellman_t *this, chunk_t value)
 {
+	EC_KEY *key = NULL;
 	EC_POINT *pub = NULL;
 	BIGNUM *priv = NULL;
 	bool ret = FALSE;
@@ -261,7 +266,7 @@ METHOD(diffie_hellman_t, set_private_value, bool,
 	{
 		goto error;
 	}
-	pub = EC_POINT_new(EC_KEY_get0_group(this->key));
+	pub = EC_POINT_new(this->ec_group);
 	if (!pub)
 	{
 		goto error;
@@ -270,25 +275,29 @@ METHOD(diffie_hellman_t, set_private_value, bool,
 	{
 		goto error;
 	}
-	if (EC_KEY_set_private_key(this->key, priv) != 1)
+	key = EC_KEY_new();
+	if (!key || !EC_KEY_set_group(key, this->ec_group))
 	{
 		goto error;
 	}
-	if (EC_KEY_set_public_key(this->key, pub) != 1)
+	if (EC_KEY_set_private_key(key, priv) != 1)
+	{
+		goto error;
+	}
+	if (EC_KEY_set_public_key(key, pub) != 1)
+	{
+		goto error;
+	}
+	if (EVP_PKEY_set1_EC_KEY(this->key, key) != 1)
 	{
 		goto error;
 	}
 	ret = TRUE;
 
 error:
-	if (pub)
+	EC_POINT_free(pub);
-	{
+	BN_free(priv);
-		EC_POINT_free(pub);
+	EC_KEY_free(key);
-	}
-	if (priv)
-	{
-		BN_free(priv);
-	}
 	return ret;
 }
 
@@ -312,14 +321,8 @@ METHOD(diffie_hellman_t, get_dh_group, diffie_hellman_group_t,
 METHOD(diffie_hellman_t, destroy, void,
 	private_openssl_ec_diffie_hellman_t *this)
 {
-	if (this->pub_key)
+	EC_GROUP_free(this->ec_group);
-	{
+	EVP_PKEY_free(this->key);
-		EC_POINT_clear_free(this->pub_key);
-	}
-	if (this->key)
-	{
-		EC_KEY_free(this->key);
-	}
 	chunk_clear(&this->shared_secret);
 	free(this);
 }
@@ -598,6 +601,39 @@ static EC_KEY *ec_key_new_brainpool(diffie_hellman_group_t group)
 openssl_ec_diffie_hellman_t *openssl_ec_diffie_hellman_create(diffie_hellman_group_t group)
 {
 	private_openssl_ec_diffie_hellman_t *this;
+	EC_KEY *key = NULL;
+
+	switch (group)
+	{
+		case ECP_192_BIT:
+			key = EC_KEY_new_by_curve_name(NID_X9_62_prime192v1);
+			break;
+		case ECP_224_BIT:
+			key = EC_KEY_new_by_curve_name(NID_secp224r1);
+			break;
+		case ECP_256_BIT:
+			key = EC_KEY_new_by_curve_name(NID_X9_62_prime256v1);
+			break;
+		case ECP_384_BIT:
+			key = EC_KEY_new_by_curve_name(NID_secp384r1);
+			break;
+		case ECP_521_BIT:
+			key = EC_KEY_new_by_curve_name(NID_secp521r1);
+			break;
+		case ECP_224_BP:
+		case ECP_256_BP:
+		case ECP_384_BP:
+		case ECP_512_BP:
+			key = ec_key_new_brainpool(group);
+			break;
+		default:
+			break;
+	}
+
+	if (!key)
+	{
+		return NULL;
+	}
 
 	INIT(this,
 		.public = {
@@ -611,59 +647,24 @@ openssl_ec_diffie_hellman_t *openssl_ec_diffie_hellman_create(diffie_hellman_gro
 			},
 		},
 		.group = group,
+		.ec_group = EC_GROUP_dup(EC_KEY_get0_group(key)),
 	);
 
-	switch (group)
-	{
-		case ECP_192_BIT:
-			this->key = EC_KEY_new_by_curve_name(NID_X9_62_prime192v1);
-			break;
-		case ECP_224_BIT:
-			this->key = EC_KEY_new_by_curve_name(NID_secp224r1);
-			break;
-		case ECP_256_BIT:
-			this->key = EC_KEY_new_by_curve_name(NID_X9_62_prime256v1);
-			break;
-		case ECP_384_BIT:
-			this->key = EC_KEY_new_by_curve_name(NID_secp384r1);
-			break;
-		case ECP_521_BIT:
-			this->key = EC_KEY_new_by_curve_name(NID_secp521r1);
-			break;
-		case ECP_224_BP:
-		case ECP_256_BP:
-		case ECP_384_BP:
-		case ECP_512_BP:
-			this->key = ec_key_new_brainpool(group);
-			break;
-		default:
-			this->key = NULL;
-			break;
-	}
-
-	if (!this->key)
-	{
-		free(this);
-		return NULL;
-	}
-
-	/* caching the EC group */
-	this->ec_group = EC_KEY_get0_group(this->key);
-
-	this->pub_key = EC_POINT_new(this->ec_group);
-	if (!this->pub_key)
-	{
-		destroy(this);
-		return NULL;
-	}
-
 	/* generate an EC private (public) key */
-	if (!EC_KEY_generate_key(this->key))
+	if (!EC_KEY_generate_key(key))
 	{
+		EC_KEY_free(key);
 		destroy(this);
 		return NULL;
 	}
 
+	this->key = EVP_PKEY_new();
+	if (!this->key || !EVP_PKEY_assign_EC_KEY(this->key, key))
+	{
+		EC_KEY_free(key);
+		destroy(this);
+		return NULL;
+	}
 	return &this->public;
 }
 #endif /* OPENSSL_NO_EC */

src/libstrongswan/plugins/openssl/openssl_util.c

@@ -29,6 +29,49 @@
 #define ASN1_STRING_get0_data(a) ASN1_STRING_data((ASN1_STRING*)a)
 #endif
 
+/*
+ * Described in header
+ */
+bool openssl_compute_shared_key(EVP_PKEY *priv, EVP_PKEY *pub, chunk_t *shared)
+{
+	EVP_PKEY_CTX *ctx;
+	bool success = FALSE;
+
+	ctx = EVP_PKEY_CTX_new(priv, NULL);
+	if (!ctx)
+	{
+		return FALSE;
+	}
+
+	if (EVP_PKEY_derive_init(ctx) <= 0)
+	{
+		goto error;
+	}
+
+	if (EVP_PKEY_derive_set_peer(ctx, pub) <= 0)
+	{
+		goto error;
+	}
+
+	if (EVP_PKEY_derive(ctx, NULL, &shared->len) <= 0)
+	{
+		goto error;
+	}
+
+	*shared = chunk_alloc(shared->len);
+
+	if (EVP_PKEY_derive(ctx, shared->ptr, &shared->len) <= 0)
+	{
+		goto error;
+	}
+
+	success = TRUE;
+
+error:
+	EVP_PKEY_CTX_free(ctx);
+	return success;
+}
+
 /**
  * Described in header.
  */

src/libstrongswan/plugins/openssl/openssl_util.h

@@ -36,6 +36,16 @@
  */
 #define EC_FIELD_ELEMENT_LEN(group) ((EC_GROUP_get_degree(group) + 7) / 8)
 
+/**
+ * Derives a shared DH secret from the given keys.
+ *
+ * @param priv		private key
+ * @param pub		public key
+ * @param shared	shared secret
+ * @return			TRUE on success, FALSE otherwise
+ */
+bool openssl_compute_shared_key(EVP_PKEY *priv, EVP_PKEY *pub, chunk_t *shared);
+
 /**
  * Creates a hash of a given type of a chunk of data.
  *

src/libstrongswan/plugins/openssl/openssl_x_diffie_hellman.c

@@ -20,6 +20,7 @@
 #if OPENSSL_VERSION_NUMBER >= 0x1010100fL && !defined(OPENSSL_NO_ECDH)
 
 #include "openssl_x_diffie_hellman.h"
+#include "openssl_util.h"
 
 #include <utils/debug.h>
 
@@ -71,50 +72,6 @@ static int map_key_type(diffie_hellman_group_t group)
 	}
 }
 
-/**
- * Compute the shared secret
- */
-static bool compute_shared_key(private_diffie_hellman_t *this, EVP_PKEY *pub,
-							   chunk_t *shared_secret)
-{
-	EVP_PKEY_CTX *ctx;
-	bool success = FALSE;
-
-	ctx = EVP_PKEY_CTX_new(this->key, NULL);
-	if (!ctx)
-	{
-		return FALSE;
-	}
-
-	if (EVP_PKEY_derive_init(ctx) <= 0)
-	{
-		goto error;
-	}
-
-	if (EVP_PKEY_derive_set_peer(ctx, pub) <= 0)
-	{
-		goto error;
-	}
-
-	if (EVP_PKEY_derive(ctx, NULL, &shared_secret->len) <= 0)
-	{
-		goto error;
-	}
-
-	*shared_secret = chunk_alloc(shared_secret->len);
-
-	if (EVP_PKEY_derive(ctx, shared_secret->ptr, &shared_secret->len) <= 0)
-	{
-		goto error;
-	}
-
-	success = TRUE;
-
-error:
-	EVP_PKEY_CTX_free(ctx);
-	return success;
-}
-
 METHOD(diffie_hellman_t, set_other_public_value, bool,
 	private_diffie_hellman_t *this, chunk_t value)
 {
@@ -136,7 +93,7 @@ METHOD(diffie_hellman_t, set_other_public_value, bool,
 
 	chunk_clear(&this->shared_secret);
 
-	if (!compute_shared_key(this, pub, &this->shared_secret))
+	if (!openssl_compute_shared_key(this->key, pub, &this->shared_secret))
 	{
 		DBG1(DBG_LIB, "%N shared secret computation failed",
 			 diffie_hellman_group_names, this->group);

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

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))
 	{
-		/*
+		success = ecp2chunk(this->keysize, secret, shared_secret, TRUE);
-		 * 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);
 	}
 
 	wc_ecc_del_point(secret);