]> git.ipfire.org Git - thirdparty/strongswan.git/commitdiff
openssl: Move shared secret calculation to get_shared_secret()
authorTobias Brunner <tobias@strongswan.org>
Fri, 23 Jul 2021 14:44:07 +0000 (16:44 +0200)
committerTobias Brunner <tobias@strongswan.org>
Thu, 14 Apr 2022 17:05:44 +0000 (19:05 +0200)
This is a change from the multi-KE branch.

src/libstrongswan/plugins/openssl/openssl_diffie_hellman.c
src/libstrongswan/plugins/openssl/openssl_ec_diffie_hellman.c
src/libstrongswan/plugins/openssl/openssl_util.c
src/libstrongswan/plugins/openssl/openssl_x_diffie_hellman.c

index bee56a4259122dfff144cdb3e4ff832a9b909b18..876742f35b6136716ea33a235581d70ca0458493 100644 (file)
@@ -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)
        {
index d591b0517189c2b2873f5b426abe31effc297611..ff00ecb80137bbcfdc8da523bf97bffd0d012df1 100644 (file)
@@ -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);
 }
index 55fe80ae8840fbd28d3269e542da514103f50e03..225f16ad8571f3adc2c5e5fe951defadce12a485 100644 (file)
@@ -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;
        }
 
index 3d41eb95aeeb25930ad6fc28ce8b96efb22c5cf0..ad36b3dd4cdacce003c76817becb2a0c6f32af63 100644 (file)
@@ -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);
 }