wolfssl: Move shared secret calculation to get_shared_secret()

The ECDH implementation gets a bit simpler since we removed the
ecp_x_coordinate_only option a while ago.

Also added calls to verify public keys.

diff --git a/src/libstrongswan/plugins/wolfssl/wolfssl_diffie_hellman.c b/src/libstrongswan/plugins/wolfssl/wolfssl_diffie_hellman.c
index 9856e8d40d925ded53039a512fd0085bf6182d35..2f64c4800050588e888252c383fa69df99bc2472 100644 (file)
--- a/src/libstrongswan/plugins/wolfssl/wolfssl_diffie_hellman.c
+++ b/src/libstrongswan/plugins/wolfssl/wolfssl_diffie_hellman.c
@@ -68,6 +68,11 @@ struct private_wolfssl_diffie_hellman_t {
         */
        chunk_t pub;
 
+       /**
+        * Public key provided by peer
+        */
+       chunk_t other;
+
        /**
         * Shared secret
         */
@@ -84,9 +89,19 @@ METHOD(key_exchange_t, get_public_key, bool,
 METHOD(key_exchange_t, get_shared_secret, bool,
        private_wolfssl_diffie_hellman_t *this, chunk_t *secret)
 {
+       word32 len;
+
        if (!this->shared_secret.len)
        {
-               return FALSE;
+               this->shared_secret = chunk_alloc(this->len);
+               if (wc_DhAgree(&this->dh, this->shared_secret.ptr, &len, this->priv.ptr,
+                                          this->priv.len, this->other.ptr, this->other.len) != 0)
+               {
+                       DBG1(DBG_LIB, "DH shared secret computation failed");
+                       chunk_free(&this->shared_secret);
+                       return FALSE;
+               }
+               this->shared_secret.len = len;
        }
        *secret = chunk_copy_pad(chunk_alloc(this->len), this->shared_secret, 0x00);
        return TRUE;
@@ -95,23 +110,17 @@ METHOD(key_exchange_t, get_shared_secret, bool,
 METHOD(key_exchange_t, set_public_key, bool,
        private_wolfssl_diffie_hellman_t *this, chunk_t value)
 {
-       word32 len;
-
        if (!key_exchange_verify_pubkey(this->group, value))
        {
                return FALSE;
        }
-
-       chunk_clear(&this->shared_secret);
-       this->shared_secret = chunk_alloc(this->len);
-       if (wc_DhAgree(&this->dh, this->shared_secret.ptr, &len, this->priv.ptr,
-                                  this->priv.len, value.ptr, value.len) != 0)
+       if (wc_DhCheckPubKey(&this->dh, value.ptr, value.len) != 0)
        {
-               DBG1(DBG_LIB, "DH shared secret computation failed");
-               chunk_free(&this->shared_secret);
+               DBG1(DBG_LIB, "DH public key value invalid");
                return FALSE;
        }
-       this->shared_secret.len = len;
+       chunk_clear(&this->other);
+       this->other = chunk_clone(value);
        return TRUE;
 }
 
@@ -153,6 +162,7 @@ METHOD(key_exchange_t, destroy, void,
        wc_FreeDhKey(&this->dh);
        chunk_clear(&this->pub);
        chunk_clear(&this->priv);
+       chunk_clear(&this->other);
        chunk_clear(&this->shared_secret);
        free(this);
 }

diff --git a/src/libstrongswan/plugins/wolfssl/wolfssl_ec_diffie_hellman.c b/src/libstrongswan/plugins/wolfssl/wolfssl_ec_diffie_hellman.c
index d622e14dbc2a97c0d10b40f5c73b0901d6d9520c..4ee286f7121e585f3b1f9f86b5db4a4c9f6b164a 100644 (file)
--- a/src/libstrongswan/plugins/wolfssl/wolfssl_ec_diffie_hellman.c
+++ b/src/libstrongswan/plugins/wolfssl/wolfssl_ec_diffie_hellman.c
@@ -62,29 +62,17 @@ struct private_wolfssl_ec_diffie_hellman_t {
         */
        ecc_key key;
 
+       /**
+        * Public key provided by peer
+        */
+       ecc_key pubkey;
+
        /**
         * Shared secret
         */
        chunk_t shared_secret;
 };
 
-/**
- * Convert a chunk to an ecc_point (which must already exist). The x and y
- * coordinates of the point have to be concatenated in the chunk.
- */
-static bool chunk2ecp(chunk_t chunk, ecc_point *point)
-{
-       if (!wolfssl_mp_split(chunk, point->x, point->y))
-       {
-               return FALSE;
-       }
-       if (mp_set(point->z, 1) != 0)
-       {
-               return FALSE;
-       }
-       return TRUE;
-}
-
 /**
  * Convert an ec_point to a chunk by concatenating the x and y coordinates of
  * the point. This function allocates memory for the chunk.
@@ -138,69 +126,30 @@ static bool wolfssl_ecc_multiply(const ecc_set_type *ecc_set, mp_int *scalar,
        return ret == 0;
 }
 
-/**
- * Compute the shared secret.
- *
- * We cannot use the function wc_ecc_shared_secret() 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_wolfssl_ec_diffie_hellman_t *this,
-                                                          ecc_point *pub_key, chunk_t *shared_secret)
-{
-       ecc_point* secret;
-       bool success = FALSE;
-
-       if ((secret = wc_ecc_new_point()) == NULL)
-       {
-               return FALSE;
-       }
-
-       if (wolfssl_ecc_multiply(this->key.dp, &this->key.k, pub_key, secret))
-       {
-               success = ecp2chunk(this->keysize, secret, shared_secret, TRUE);
-       }
-
-       wc_ecc_del_point(secret);
-       return success;
-}
-
 METHOD(key_exchange_t, set_public_key, bool,
        private_wolfssl_ec_diffie_hellman_t *this, chunk_t value)
 {
-       ecc_point *pub_key;
+       chunk_t uncomp;
 
        if (!key_exchange_verify_pubkey(this->group, value))
        {
                return FALSE;
        }
 
-       if ((pub_key = wc_ecc_new_point()) == NULL)
-       {
-               return FALSE;
-       }
-
-       if (!chunk2ecp(value, pub_key))
+       /* prepend 0x04 to indicate uncompressed point format */
+       uncomp = chunk_cata("cc", chunk_from_chars(0x04), value);
+       if (wc_ecc_import_x963_ex(uncomp.ptr, uncomp.len, &this->pubkey,
+                                                         this->curve_id) != 0)
        {
                DBG1(DBG_LIB, "ECDH public value is malformed");
-               wc_ecc_del_point(pub_key);
                return FALSE;
        }
 
-       chunk_clear(&this->shared_secret);
-
-       if (!compute_shared_key(this, pub_key, &this->shared_secret))
+       if (wc_ecc_check_key(&this->pubkey) != 0)
        {
-               DBG1(DBG_LIB, "ECDH shared secret computation failed");
-               chunk_clear(&this->shared_secret);
-               wc_ecc_del_point(pub_key);
+               DBG1(DBG_LIB, "ECDH public value is invalid");
                return FALSE;
        }
-       wc_ecc_del_point(pub_key);
        return TRUE;
 }
 
@@ -248,10 +197,51 @@ METHOD(key_exchange_t, set_private_key, bool,
        return success;
 }
 
+/**
+ * Derive the shared secret
+ */
+static bool compute_shared_key(private_wolfssl_ec_diffie_hellman_t *this)
+{
+       word32 len;
+#ifdef ECC_TIMING_RESISTANT
+       WC_RNG rng;
+
+       if (wc_InitRng(&rng) != 0)
+       {
+               return FALSE;
+       }
+       if (wc_ecc_set_rng(&this->key, &rng) != 0)
+       {
+               wc_FreeRng(&rng);
+               return FALSE;
+       }
+#endif
+
+       this->shared_secret = chunk_alloc(this->keysize);
+       len = this->shared_secret.len;
+
+       if (wc_ecc_shared_secret(&this->key, &this->pubkey, this->shared_secret.ptr,
+                                                        &len) != 0)
+       {
+               DBG1(DBG_LIB, "ECDH shared secret computation failed");
+               chunk_clear(&this->shared_secret);
+#ifdef ECC_TIMING_RESISTANT
+               wc_FreeRng(&rng);
+#endif
+               return FALSE;
+       }
+       this->shared_secret.len = len;
+#ifdef ECC_TIMING_RESISTANT
+       wc_FreeRng(&rng);
+#endif
+       return TRUE;
+}
+
 METHOD(key_exchange_t, get_shared_secret, bool,
        private_wolfssl_ec_diffie_hellman_t *this, chunk_t *secret)
 {
-       if (!this->shared_secret.len)
+       if (!this->shared_secret.len &&
+               !compute_shared_key(this))
        {
                return FALSE;
        }
@@ -269,6 +259,7 @@ METHOD(key_exchange_t, destroy, void,
        private_wolfssl_ec_diffie_hellman_t *this)
 {
        wc_ecc_free(&this->key);
+       wc_ecc_free(&this->pubkey);
        chunk_clear(&this->shared_secret);
        free(this);
 }
@@ -295,10 +286,10 @@ wolfssl_ec_diffie_hellman_t *wolfssl_ec_diffie_hellman_create(key_exchange_metho
                .group = group,
        );
 
-       if (wc_ecc_init(&this->key) != 0)
+       if (wc_ecc_init(&this->key) != 0 || wc_ecc_init(&this->pubkey) != 0)
        {
                DBG1(DBG_LIB, "key init failed, ecdh create failed");
-               free(this);
+               destroy(this);
                return NULL;
        }
 

diff --git a/src/libstrongswan/plugins/wolfssl/wolfssl_x_diffie_hellman.c b/src/libstrongswan/plugins/wolfssl/wolfssl_x_diffie_hellman.c
index f2aca03e4672a7d6d24cfa610d61f00d12c8aa4c..f2361f2efb87b5c33b882facefb064b08559826f 100644 (file)
--- a/src/libstrongswan/plugins/wolfssl/wolfssl_x_diffie_hellman.c
+++ b/src/libstrongswan/plugins/wolfssl/wolfssl_x_diffie_hellman.c
@@ -68,6 +68,18 @@ struct private_diffie_hellman_t {
 #endif
        } key;
 
+       /**
+        * Public key provided by peer
+        */
+       union {
+#ifdef HAVE_CURVE25519
+               curve25519_key key25519;
+#endif
+#ifdef HAVE_CURVE448
+               curve448_key key448;
+#endif
+       } pub;
+
        /**
         * Shared secret
         */
@@ -76,47 +88,42 @@ struct private_diffie_hellman_t {
 
 #ifdef HAVE_CURVE25519
 
-METHOD(key_exchange_t, set_public_key_25519, bool,
-       private_diffie_hellman_t *this, chunk_t value)
+METHOD(key_exchange_t, get_shared_secret_25519, bool,
+       private_diffie_hellman_t *this, chunk_t *secret)
 {
        word32 len = CURVE25519_KEYSIZE;
-       curve25519_key pub;
-       int ret;
 
-       if (!key_exchange_verify_pubkey(this->group, value))
+       if (!this->shared_secret.len)
        {
-               return FALSE;
+               this->shared_secret = chunk_alloc(len);
+               if (wc_curve25519_shared_secret_ex(&this->key.key25519, &this->pub.key25519,
+                                       this->shared_secret.ptr, &len, EC25519_LITTLE_ENDIAN) != 0)
+               {
+                       DBG1(DBG_LIB, "%N shared secret computation failed",
+                                key_exchange_method_names, this->group);
+                       chunk_clear(&this->shared_secret);
+                       return FALSE;
+               }
        }
+       *secret = chunk_clone(this->shared_secret);
+       return TRUE;
+}
 
-       ret = wc_curve25519_init(&pub);
-       if (ret != 0)
+METHOD(key_exchange_t, set_public_key_25519, bool,
+       private_diffie_hellman_t *this, chunk_t value)
+{
+       if (!key_exchange_verify_pubkey(this->group, value))
        {
-               DBG1(DBG_LIB, "%N public key initialization failed",
-                        key_exchange_method_names, this->group);
                return FALSE;
        }
 
-       ret = wc_curve25519_import_public_ex(value.ptr, value.len, &pub,
-                                                                                EC25519_LITTLE_ENDIAN);
-       if (ret != 0)
+       if (wc_curve25519_import_public_ex(value.ptr, value.len, &this->pub.key25519,
+                                                                          EC25519_LITTLE_ENDIAN) != 0)
        {
                DBG1(DBG_LIB, "%N public value is malformed",
                         key_exchange_method_names, this->group);
                return FALSE;
        }
-
-       chunk_clear(&this->shared_secret);
-       this->shared_secret = chunk_alloc(len);
-       if (wc_curve25519_shared_secret_ex(&this->key.key25519, &pub,
-                                       this->shared_secret.ptr, &len, EC25519_LITTLE_ENDIAN) != 0)
-       {
-               DBG1(DBG_LIB, "%N shared secret computation failed",
-                        key_exchange_method_names, this->group);
-               chunk_clear(&this->shared_secret);
-               wc_curve25519_free(&pub);
-               return FALSE;
-       }
-       wc_curve25519_free(&pub);
        return TRUE;
 }
 
@@ -168,47 +175,42 @@ METHOD(key_exchange_t, set_private_key_25519, bool,
 
 #ifdef HAVE_CURVE448
 
-METHOD(key_exchange_t, set_public_key_448, bool,
-       private_diffie_hellman_t *this, chunk_t value)
+METHOD(key_exchange_t, get_shared_secret_448, bool,
+       private_diffie_hellman_t *this, chunk_t *secret)
 {
        word32 len = CURVE448_KEY_SIZE;
-       curve448_key pub;
-       int ret;
 
-       if (!key_exchange_verify_pubkey(this->group, value))
+       if (!this->shared_secret.len)
        {
-               return FALSE;
+               this->shared_secret = chunk_alloc(len);
+               if (wc_curve448_shared_secret_ex(&this->key.key448, &this->pub.key448,
+                                       this->shared_secret.ptr, &len, EC448_LITTLE_ENDIAN) != 0)
+               {
+                       DBG1(DBG_LIB, "%N shared secret computation failed",
+                                key_exchange_method_names, this->group);
+                       chunk_clear(&this->shared_secret);
+                       return FALSE;
+               }
        }
+       *secret = chunk_clone(this->shared_secret);
+       return TRUE;
+}
 
-       ret = wc_curve448_init(&pub);
-       if (ret != 0)
+METHOD(key_exchange_t, set_public_key_448, bool,
+       private_diffie_hellman_t *this, chunk_t value)
+{
+       if (!key_exchange_verify_pubkey(this->group, value))
        {
-               DBG1(DBG_LIB, "%N public key initialization failed",
-                        key_exchange_method_names, this->group);
                return FALSE;
        }
 
-       ret = wc_curve448_import_public_ex(value.ptr, value.len, &pub,
-                                                                          EC448_LITTLE_ENDIAN);
-       if (ret != 0)
+       if (wc_curve448_import_public_ex(value.ptr, value.len, &this->pub.key448,
+                                                                        EC448_LITTLE_ENDIAN) != 0)
        {
                DBG1(DBG_LIB, "%N public value is malformed",
                         key_exchange_method_names, this->group);
                return FALSE;
        }
-
-       chunk_clear(&this->shared_secret);
-       this->shared_secret = chunk_alloc(len);
-       if (wc_curve448_shared_secret_ex(&this->key.key448, &pub,
-                                       this->shared_secret.ptr, &len, EC448_LITTLE_ENDIAN) != 0)
-       {
-               DBG1(DBG_LIB, "%N shared secret computation failed",
-                        key_exchange_method_names, this->group);
-               chunk_clear(&this->shared_secret);
-               wc_curve448_free(&pub);
-               return FALSE;
-       }
-       wc_curve448_free(&pub);
        return TRUE;
 }
 
@@ -258,17 +260,6 @@ METHOD(key_exchange_t, set_private_key_448, bool,
 
 #endif /* HAVE_CURVE448 */
 
-METHOD(key_exchange_t, get_shared_secret, bool,
-       private_diffie_hellman_t *this, chunk_t *secret)
-{
-       if (!this->shared_secret.len)
-       {
-               return FALSE;
-       }
-       *secret = chunk_clone(this->shared_secret);
-       return TRUE;
-}
-
 METHOD(key_exchange_t, get_method, key_exchange_method_t,
        private_diffie_hellman_t *this)
 {
@@ -282,12 +273,14 @@ METHOD(key_exchange_t, destroy, void,
        {
 #ifdef HAVE_CURVE25519
                wc_curve25519_free(&this->key.key25519);
+               wc_curve25519_free(&this->pub.key25519);
 #endif
        }
        else if (this->group == CURVE_448)
        {
 #ifdef HAVE_CURVE448
                wc_curve448_free(&this->key.key448);
+               wc_curve448_free(&this->pub.key448);
 #endif
        }
        chunk_clear(&this->shared_secret);
@@ -305,7 +298,6 @@ key_exchange_t *wolfssl_x_diffie_hellman_create(key_exchange_method_t group)
 
        INIT(this,
                .public = {
-                       .get_shared_secret = _get_shared_secret,
                        .get_method = _get_method,
                        .destroy = _destroy,
                },
@@ -322,14 +314,16 @@ key_exchange_t *wolfssl_x_diffie_hellman_create(key_exchange_method_t group)
        if (group == CURVE_25519)
        {
 #ifdef HAVE_CURVE25519
+               this->public.get_shared_secret = _get_shared_secret_25519;
                this->public.set_public_key = _set_public_key_25519;
                this->public.get_public_key = _get_public_key_25519;
                this->public.set_private_key = _set_private_key_25519;
 
-               if (wc_curve25519_init(&this->key.key25519) != 0)
+               if (wc_curve25519_init(&this->key.key25519) != 0 ||
+                       wc_curve25519_init(&this->pub.key25519) != 0)
                {
                        DBG1(DBG_LIB, "initializing key failed");
-                       free(this);
+                       destroy(this);
                        return NULL;
                }
                ret = wc_curve25519_make_key(&rng, CURVE25519_KEYSIZE,
@@ -339,14 +333,16 @@ key_exchange_t *wolfssl_x_diffie_hellman_create(key_exchange_method_t group)
        else if (group == CURVE_448)
        {
 #ifdef HAVE_CURVE448
+               this->public.get_shared_secret = _get_shared_secret_448;
                this->public.set_public_key = _set_public_key_448;
                this->public.get_public_key = _get_public_key_448;
                this->public.set_private_key = _set_private_key_448;
 
-               if (wc_curve448_init(&this->key.key448) != 0)
+               if (wc_curve448_init(&this->key.key448) != 0 ||
+                       wc_curve448_init(&this->pub.key448) != 0)
                {
                        DBG1(DBG_LIB, "initializing key failed");
-                       free(this);
+                       destroy(this);
                        return NULL;
                }
                ret = wc_curve448_make_key(&rng, CURVE448_KEY_SIZE, &this->key.key448);