gmp: Move shared secret calculation to get_shared_secret()

This avoids doing costly operations when just setting the public key.
For the same reason the optional extended public key check is moved.

diff --git a/src/libstrongswan/plugins/gmp/gmp_diffie_hellman.c b/src/libstrongswan/plugins/gmp/gmp_diffie_hellman.c
index 619c6de662a49b1d4a6386a17196516106a41bc0..6528495a5e0fd701bc271a890915394c462b4d8d 100644 (file)
--- a/src/libstrongswan/plugins/gmp/gmp_diffie_hellman.c
+++ b/src/libstrongswan/plugins/gmp/gmp_diffie_hellman.c
@@ -97,56 +97,30 @@ METHOD(key_exchange_t, set_public_key, bool,
        mpz_init(p_min_1);
        mpz_sub_ui(p_min_1, this->p, 1);
 
+       /* this->computed is not reset in order to prevent reuse of this DH
+        * instance (see below) */
        mpz_import(this->yb, value.len, 1, 1, 1, 0, value.ptr);
 
-       /* check public value:
-        * 1. 0 or 1 is invalid as 0^a = 0 and 1^a = 1
-        * 2. a public value larger or equal the modulus is invalid */
-       if (mpz_cmp_ui(this->yb, 1) > 0 &&
-               mpz_cmp(this->yb, p_min_1) < 0)
-       {
-#ifdef EXTENDED_DH_TEST
-               /* 3. test if y ^ q mod p = 1, where q = (p - 1)/2. */
-               mpz_t q, one;
-               diffie_hellman_params_t *params;
-
-               mpz_init(q);
-               mpz_init(one);
-
-               params = diffie_hellman_get_params(this->group);
-               if (!params->subgroup.len)
-               {
-                       mpz_fdiv_q_2exp(q, p_min_1, 1);
-               }
-               else
-               {
-                       mpz_import(q, params->subgroup.len, 1, 1, 1, 0, params->subgroup.ptr);
-               }
-               mpz_powm(one, this->yb, q, this->p);
-               mpz_clear(q);
-               if (mpz_cmp_ui(one, 1) == 0)
-               {
-                       mpz_powm(this->zz, this->yb, this->xa, this->p);
-                       this->computed = TRUE;
-               }
-               else
-               {
-                       DBG1(DBG_LIB, "public DH value verification failed:"
-                                " y ^ q mod p != 1");
-               }
-               mpz_clear(one);
-#else
-               mpz_powm(this->zz, this->yb, this->xa, this->p);
-               this->computed = TRUE;
-#endif
-       }
-       else
+       /* check that the public value y satisfies 1 < y < p-1.
+        * according to RFC 6989, section 2.1, this is enough for the common safe-
+        * prime DH groups (i.e. with q=(p-1)/2 being prime), only for those with
+        * small subgroups (22, 23, 24) does the RFC require the extended test but
+        * only if private keys are reused. we never do that anyway and it's
+        * explicitly prevented in this implementation. so the extended test, which
+        * optionally happens in get_shared_secret(), is really only useful for full
+        * NIST SP 800-56A compliance, which only allows the partial check for
+        * safe-prime groups.
+        */
+       if (mpz_cmp_ui(this->yb, 1) <= 0 ||
+               mpz_cmp(this->yb, p_min_1) >= 0)
        {
-               DBG1(DBG_LIB, "public DH value verification failed:"
-                        " y < 2 || y > p - 1 ");
+               DBG1(DBG_LIB, "public DH value verification failed: "
+                        "y <= 1 || y >= p - 1");
+               mpz_clear(p_min_1);
+               return FALSE;
        }
        mpz_clear(p_min_1);
-       return this->computed;
+       return TRUE;
 }
 
 METHOD(key_exchange_t, get_public_key, bool,
@@ -175,7 +149,41 @@ METHOD(key_exchange_t, get_shared_secret, bool,
 {
        if (!this->computed)
        {
-               return FALSE;
+#ifdef EXTENDED_DH_TEST
+               /* test if y ^ q mod p = 1, where q = (p - 1)/2 or the actual size of
+                * the subgroup.  as noted above, this check is not really necessary as
+                * the plugin does not reuse private keys */
+               mpz_t q, one, p_min_1;
+               diffie_hellman_params_t *params;
+
+               mpz_init(q);
+               mpz_init(one);
+
+               params = diffie_hellman_get_params(this->group);
+               if (!params->subgroup.len)
+               {
+                       mpz_init(p_min_1);
+                       mpz_sub_ui(p_min_1, this->p, 1);
+                       mpz_fdiv_q_2exp(q, p_min_1, 1);
+                       mpz_clear(p_min_1);
+               }
+               else
+               {
+                       mpz_import(q, params->subgroup.len, 1, 1, 1, 0, params->subgroup.ptr);
+               }
+               mpz_powm(one, this->yb, q, this->p);
+               mpz_clear(q);
+               if (mpz_cmp_ui(one, 1) != 0)
+               {
+                       DBG1(DBG_LIB, "public DH value verification failed: "
+                                "y ^ q mod p != 1");
+                       mpz_clear(one);
+                       return FALSE;
+               }
+               mpz_clear(one);
+#endif
+               mpz_powm(this->zz, this->yb, this->xa, this->p);
+               this->computed = TRUE;
        }
        secret->len = this->p_len;
        secret->ptr = mpz_export(NULL, NULL, 1, secret->len, 1, 0, this->zz);