From: Tobias Brunner <tobias@strongswan.org>
Date: Fri, 23 Jul 2021 14:44:07 +0000 (+0200)
Subject: openssl: Move shared secret calculation to get_shared_secret()
X-Git-Tag: 5.9.6rc1~1^2~14
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=1c1213f4b660b947821d4eaa26f7f3d8d09fd677;p=thirdparty%2Fstrongswan.git

openssl: Move shared secret calculation to get_shared_secret()

This is a change from the multi-KE branch.
---

diff --git a/src/libstrongswan/plugins/openssl/openssl_diffie_hellman.c b/src/libstrongswan/plugins/openssl/openssl_diffie_hellman.c
index bee56a4259..876742f35b 100644
--- a/src/libstrongswan/plugins/openssl/openssl_diffie_hellman.c
+++ b/src/libstrongswan/plugins/openssl/openssl_diffie_hellman.c
@@ -63,11 +63,6 @@ struct private_openssl_diffie_hellman_t {
 	 * Shared secret
 	 */
 	chunk_t shared_secret;
-
-	/**
-	 * True if shared secret is computed
-	 */
-	bool computed;
 };
 
 METHOD(diffie_hellman_t, get_my_public_value, bool,
@@ -85,15 +80,24 @@ METHOD(diffie_hellman_t, get_my_public_value, bool,
 METHOD(diffie_hellman_t, get_shared_secret, bool,
 	private_openssl_diffie_hellman_t *this, chunk_t *secret)
 {
-	if (!this->computed)
+	int len;
+
+	if (!this->shared_secret.len)
 	{
-		return FALSE;
+		this->shared_secret = chunk_alloc(DH_size(this->dh));
+		memset(this->shared_secret.ptr, 0xFF, this->shared_secret.len);
+		len = DH_compute_key(this->shared_secret.ptr, this->pub_key, this->dh);
+		if (len < 0)
+		{
+			DBG1(DBG_LIB, "DH shared secret computation failed");
+			chunk_clear(&this->shared_secret);
+			return FALSE;
+		}
+		this->shared_secret.len = len;
 	}
-	/* shared secret should requires a len according the DH group */
-	*secret = chunk_alloc(DH_size(this->dh));
-	memset(secret->ptr, 0, secret->len);
-	memcpy(secret->ptr + secret->len - this->shared_secret.len,
-		   this->shared_secret.ptr, this->shared_secret.len);
+	/* shared secret requires a length according to the DH group */
+	*secret = chunk_copy_pad(chunk_alloc(DH_size(this->dh)),
+							 this->shared_secret, 0);
 	return TRUE;
 }
 
@@ -101,25 +105,16 @@ METHOD(diffie_hellman_t, get_shared_secret, bool,
 METHOD(diffie_hellman_t, set_other_public_value, bool,
 	private_openssl_diffie_hellman_t *this, chunk_t value)
 {
-	int len;
-
 	if (!diffie_hellman_verify_value(this->group, value))
 	{
 		return FALSE;
 	}
 
-	BN_bin2bn(value.ptr, value.len, this->pub_key);
-	chunk_clear(&this->shared_secret);
-	this->shared_secret.ptr = malloc(DH_size(this->dh));
-	memset(this->shared_secret.ptr, 0xFF, this->shared_secret.len);
-	len = DH_compute_key(this->shared_secret.ptr, this->pub_key, this->dh);
-	if (len < 0)
+	if (!BN_bin2bn(value.ptr, value.len, this->pub_key))
 	{
-		DBG1(DBG_LIB, "DH shared secret computation failed");
 		return FALSE;
 	}
-	this->shared_secret.len = len;
-	this->computed = TRUE;
+	chunk_clear(&this->shared_secret);
 	return TRUE;
 }
 
@@ -136,7 +131,6 @@ METHOD(diffie_hellman_t, set_private_value, bool,
 			return FALSE;
 		}
 		chunk_clear(&this->shared_secret);
-		this->computed = FALSE;
 		return DH_generate_key(this->dh);
 	}
 	return FALSE;
@@ -220,9 +214,7 @@ openssl_diffie_hellman_t *openssl_diffie_hellman_create(
 	}
 
 	this->group = group;
-	this->computed = FALSE;
 	this->pub_key = BN_new();
-	this->shared_secret = chunk_empty;
 
 	if (group == MODP_CUSTOM)
 	{
diff --git a/src/libstrongswan/plugins/openssl/openssl_ec_diffie_hellman.c b/src/libstrongswan/plugins/openssl/openssl_ec_diffie_hellman.c
index d591b05171..ff00ecb801 100644
--- a/src/libstrongswan/plugins/openssl/openssl_ec_diffie_hellman.c
+++ b/src/libstrongswan/plugins/openssl/openssl_ec_diffie_hellman.c
@@ -51,6 +51,11 @@ struct private_openssl_ec_diffie_hellman_t {
 	 */
 	EVP_PKEY *key;
 
+	/**
+	 * Public key provided by peer
+	 */
+	EVP_PKEY *pub;
+
 	/**
 	 * EC group
 	 */
@@ -186,50 +191,35 @@ error:
 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;
 	}
 
-	pub = EVP_PKEY_new();
-	if (!pub)
+	if (!this->pub)
 	{
-		goto error;
+		this->pub = EVP_PKEY_new();
 	}
 
 #if OPENSSL_VERSION_NUMBER < 0x1010000fL
-	if (!chunk2ecp(this->ec_group, value, pub))
+	if (!chunk2ecp(this->ec_group, value, this->pub))
 	{
 		DBG1(DBG_LIB, "ECDH public value is malformed");
-		goto error;
+		return FALSE;
 	}
 #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)
+	if (EVP_PKEY_copy_parameters(this->pub, this->key) <= 0 ||
+		EVP_PKEY_set1_tls_encodedpoint(this->pub, value.ptr, value.len) <= 0)
 	{
 		DBG1(DBG_LIB, "ECDH public value is malformed");
-		goto error;
+		return FALSE;
 	}
 #endif
-
-	if (!openssl_compute_shared_key(this->key, pub, &this->shared_secret))
-	{
-		DBG1(DBG_LIB, "ECDH shared secret computation failed");
-		goto error;
-	}
-	this->computed = TRUE;
-
-error:
-	EVP_PKEY_free(pub);
-	return this->computed;
+	chunk_clear(&this->shared_secret);
+	return TRUE;
 }
 
 METHOD(diffie_hellman_t, get_my_public_value, bool,
@@ -304,8 +294,10 @@ error:
 METHOD(diffie_hellman_t, get_shared_secret, bool,
 	private_openssl_ec_diffie_hellman_t *this, chunk_t *secret)
 {
-	if (!this->computed)
+	if (!this->shared_secret.len &&
+		!openssl_compute_shared_key(this->key, this->pub, &this->shared_secret))
 	{
+		DBG1(DBG_LIB, "ECDH shared secret computation failed");
 		return FALSE;
 	}
 	*secret = chunk_clone(this->shared_secret);
@@ -323,6 +315,7 @@ METHOD(diffie_hellman_t, destroy, void,
 {
 	EC_GROUP_free(this->ec_group);
 	EVP_PKEY_free(this->key);
+	EVP_PKEY_free(this->pub);
 	chunk_clear(&this->shared_secret);
 	free(this);
 }
diff --git a/src/libstrongswan/plugins/openssl/openssl_util.c b/src/libstrongswan/plugins/openssl/openssl_util.c
index 55fe80ae88..225f16ad85 100644
--- a/src/libstrongswan/plugins/openssl/openssl_util.c
+++ b/src/libstrongswan/plugins/openssl/openssl_util.c
@@ -62,6 +62,7 @@ bool openssl_compute_shared_key(EVP_PKEY *priv, EVP_PKEY *pub, chunk_t *shared)
 
 	if (EVP_PKEY_derive(ctx, shared->ptr, &shared->len) <= 0)
 	{
+		chunk_clear(shared);
 		goto error;
 	}
 
diff --git a/src/libstrongswan/plugins/openssl/openssl_x_diffie_hellman.c b/src/libstrongswan/plugins/openssl/openssl_x_diffie_hellman.c
index 3d41eb95ae..ad36b3dd4c 100644
--- a/src/libstrongswan/plugins/openssl/openssl_x_diffie_hellman.c
+++ b/src/libstrongswan/plugins/openssl/openssl_x_diffie_hellman.c
@@ -46,14 +46,14 @@ struct private_diffie_hellman_t {
 	EVP_PKEY *key;
 
 	/**
-	 * Shared secret
+	 * Public key provided by peer
 	 */
-	chunk_t shared_secret;
+	EVP_PKEY *pub;
 
 	/**
-	 * True if shared secret is computed
+	 * Shared secret
 	 */
-	bool computed;
+	chunk_t shared_secret;
 };
 
 /**
@@ -75,33 +75,21 @@ static int map_key_type(diffie_hellman_group_t group)
 METHOD(diffie_hellman_t, set_other_public_value, bool,
 	private_diffie_hellman_t *this, chunk_t value)
 {
-	EVP_PKEY *pub;
-
 	if (!diffie_hellman_verify_value(this->group, value))
 	{
 		return FALSE;
 	}
 
-	pub =  EVP_PKEY_new_raw_public_key(map_key_type(this->group), NULL,
-                                       value.ptr, value.len);
-	if (!pub)
+	EVP_PKEY_free(this->pub);
+	this->pub = EVP_PKEY_new_raw_public_key(map_key_type(this->group), NULL,
+											value.ptr, value.len);
+	if (!this->pub)
 	{
 		DBG1(DBG_LIB, "%N public value is malformed",
 			 diffie_hellman_group_names, this->group);
 		return FALSE;
 	}
-
 	chunk_clear(&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);
-		EVP_PKEY_free(pub);
-		return FALSE;
-	}
-	this->computed = TRUE;
-	EVP_PKEY_free(pub);
 	return TRUE;
 }
 
@@ -141,8 +129,11 @@ METHOD(diffie_hellman_t, set_private_value, bool,
 METHOD(diffie_hellman_t, get_shared_secret, bool,
 	private_diffie_hellman_t *this, chunk_t *secret)
 {
-	if (!this->computed)
+	if (!this->shared_secret.len &&
+		!openssl_compute_shared_key(this->key, this->pub, &this->shared_secret))
 	{
+		DBG1(DBG_LIB, "%N shared secret computation failed",
+			 diffie_hellman_group_names, this->group);
 		return FALSE;
 	}
 	*secret = chunk_clone(this->shared_secret);
@@ -159,6 +150,7 @@ METHOD(diffie_hellman_t, destroy, void,
 	private_diffie_hellman_t *this)
 {
 	EVP_PKEY_free(this->key);
+	EVP_PKEY_free(this->pub);
 	chunk_clear(&this->shared_secret);
 	free(this);
 }