Copy DH key (if available) in addition to the bare parameters

in SSL_new.
If SSL_OP_SINGLE_DH_USE is set, don't waste time in SSL_[CTX_]set_tmp_dh
on computing a DH key that will be ignored anyway.

ssltest -dhe1024dsa (w/ 160-bit sub-prime) had an unfair performance
advantage over -dhe1024 (safe prime): SSL_OP_SINGLE_DH_USE was
effectively always enabled because SSL_new ignored the DH key set in
the SSL_CTX.  Now -dhe1024 takes the server only about twice as long
as -dhe1024dsa instead of three times as long (for 1024 bit RSA
with 1024 bit DH).

diff --git a/CHANGES b/CHANGES
index 55db5725b436c1c12aff4166d4f211d4d9099e2b..ae01a2c90f4dd9fd2afeec1539d63bea4c374a6c 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -4,6 +4,15 @@
 
  Changes between 0.9.5 and 0.9.5a  [XX XXX 2000]
 
+  *) For SSL_[CTX_]set_tmp_dh, don't create a DH key if SSL_OP_SINGLE_DH_USE
+     is set; it will be thrown away anyway because each handshake creates
+     its own key.
+     ssl_cert_dup, which is used by SSL_new, now copies DH keys in addition
+     to parameters -- in previous versions (since OpenSSL 0.9.3) the
+     'default key' from SSL_CTX_set_tmp_dh would always be lost, meanining
+     you effectivly got SSL_OP_SINGLE_DH_USE when using this macro.
+     [Bodo Moeller]
+
   *) New s_client option -ign_eof: EOF at stdin is ignored, and
      'Q' and 'R' lose their special meanings (quit/renegotiate).
      This is part of what -quiet does; unlike -quiet, -ign_eof

diff --git a/ssl/s3_lib.c b/ssl/s3_lib.c
index c4b49aaedf4f35cdd72b182f6a959da868bf6e44..892ff753fcddd0f13d550a7b5b3c8f2ea3067569 100644 (file)
--- a/ssl/s3_lib.c
+++ b/ssl/s3_lib.c
@@ -771,14 +771,16 @@ long ssl3_ctrl(SSL *s, int cmd, long larg, char *parg)
        case SSL_CTRL_SET_TMP_RSA:
                {
                        RSA *rsa = (RSA *)parg;
-                       if (rsa == NULL) {
+                       if (rsa == NULL)
+                               {
                                SSLerr(SSL_F_SSL3_CTRL, ERR_R_PASSED_NULL_PARAMETER);
                                return(ret);
-                       }
-                       if ((rsa = RSAPrivateKey_dup(rsa)) == NULL) {
+                               }
+                       if ((rsa = RSAPrivateKey_dup(rsa)) == NULL)
+                               {
                                SSLerr(SSL_F_SSL3_CTRL, ERR_R_RSA_LIB);
                                return(ret);
-                       }
+                               }
                        if (s->cert->rsa_tmp != NULL)
                                RSA_free(s->cert->rsa_tmp);
                        s->cert->rsa_tmp = rsa;
@@ -796,19 +798,25 @@ long ssl3_ctrl(SSL *s, int cmd, long larg, char *parg)
        case SSL_CTRL_SET_TMP_DH:
                {
                        DH *dh = (DH *)parg;
-                       if (dh == NULL) {
+                       if (dh == NULL)
+                               {
                                SSLerr(SSL_F_SSL3_CTRL, ERR_R_PASSED_NULL_PARAMETER);
                                return(ret);
-                       }
-                       if ((dh = DHparams_dup(dh)) == NULL) {
-                               SSLerr(SSL_F_SSL3_CTRL, ERR_R_DH_LIB);
-                               return(ret);
-                       }
-                       if (!DH_generate_key(dh)) {
-                               DH_free(dh);
+                               }
+                       if ((dh = DHparams_dup(dh)) == NULL)
+                               {
                                SSLerr(SSL_F_SSL3_CTRL, ERR_R_DH_LIB);
                                return(ret);
-                       }
+                               }
+                       if (!(s->options & SSL_OP_SINGLE_DH_USE))
+                               {
+                               if (!DH_generate_key(dh))
+                                       {
+                                       DH_free(dh);
+                                       SSLerr(SSL_F_SSL3_CTRL, ERR_R_DH_LIB);
+                                       return(ret);
+                                       }
+                               }
                        if (s->cert->dh_tmp != NULL)
                                DH_free(s->cert->dh_tmp);
                        s->cert->dh_tmp = dh;
@@ -843,7 +851,7 @@ long ssl3_callback_ctrl(SSL *s, int cmd, void (*fp)())
                0)
                {
                if (!ssl_cert_inst(&s->cert))
-                       {
+                       {
                        SSLerr(SSL_F_SSL3_CTRL, ERR_R_MALLOC_FAILURE);
                        return(0);
                        }
@@ -929,23 +937,26 @@ long ssl3_ctx_ctrl(SSL_CTX *ctx, int cmd, long larg, char *parg)
        case SSL_CTRL_SET_TMP_DH:
                {
                DH *new=NULL,*dh;
-               int rret=0;
 
                dh=(DH *)parg;
-               if (    ((new=DHparams_dup(dh)) == NULL) ||
-                       (!DH_generate_key(new)))
+               if ((new=DHparams_dup(dh)) == NULL)
                        {
                        SSLerr(SSL_F_SSL3_CTX_CTRL,ERR_R_DH_LIB);
-                       if (new != NULL) DH_free(new);
+                       return 0;
                        }
-               else
+               if (!(ctx->options & SSL_OP_SINGLE_DH_USE))
                        {
-                       if (cert->dh_tmp != NULL)
-                               DH_free(cert->dh_tmp);
-                       cert->dh_tmp=new;
-                       rret=1;
+                       if (!DH_generate_key(new))
+                               {
+                               SSLerr(SSL_F_SSL3_CTX_CTRL,ERR_R_DH_LIB);
+                               DH_free(new);
+                               return 0;
+                               }
                        }
-               return(rret);
+               if (cert->dh_tmp != NULL)
+                       DH_free(cert->dh_tmp);
+               cert->dh_tmp=new;
+               return 1;
                }
                /*break; */
        case SSL_CTRL_SET_TMP_DH_CB:

diff --git a/ssl/ssl_cert.c b/ssl/ssl_cert.c
index 48f247ceaca6afdf944ae14b41323765b73247ba..a054e0aa44643221f123e106424722cb2f6ba8ab 100644 (file)
--- a/ssl/ssl_cert.c
+++ b/ssl/ssl_cert.c
@@ -191,16 +191,33 @@ CERT *ssl_cert_dup(CERT *cert)
 #ifndef NO_DH
        if (cert->dh_tmp != NULL)
                {
-               /* DH parameters don't have a reference count (and cannot
-                * reasonably be shared anyway, as the secret exponent may
-                * be created just when it is needed -- earlier library
-                * versions did not pay attention to this) */
+               /* DH parameters don't have a reference count */
                ret->dh_tmp = DHparams_dup(cert->dh_tmp);
                if (ret->dh_tmp == NULL)
                        {
                        SSLerr(SSL_F_SSL_CERT_NEW, ERR_R_DH_LIB);
                        goto err;
                        }
+               if (cert->dh_tmp->priv_key)
+                       {
+                       BIGNUM *b = BN_dup(cert->dh_tmp->priv_key);
+                       if (!b)
+                               {
+                               SSLerr(SSL_F_SSL_CERT_NEW, ERR_R_BN_LIB);
+                               goto err;
+                               }
+                       ret->dh_tmp->priv_key = b;
+                       }
+               if (cert->dh_tmp->pub_key)
+                       {
+                       BIGNUM *b = BN_dup(cert->dh_tmp->pub_key);
+                       if (!b)
+                               {
+                               SSLerr(SSL_F_SSL_CERT_NEW, ERR_R_BN_LIB);
+                               goto err;
+                               }
+                       ret->dh_tmp->pub_key = b;
+                       }
                }
        ret->dh_tmp_cb = cert->dh_tmp_cb;
 #endif