On 19.05.2014 10:15, Plüm, Rüdiger, Vodafone Group wrote:
> Maybe stupid idea, but can't we do that once and hand it out over
> and over again?
Not a stupid idea at all - I think it's actually the most sensible
solution to this problem. This is what OpenSSL does with the
DH parameters provided by the callback in
s3_srvr.c:ssl3_send_server_key_exchange():
if (type & SSL_kEDH)
{
dhp=cert->dh_tmp;
if ((dhp == NULL) && (s->cert->dh_tmp_cb != NULL))
dhp=s->cert->dh_tmp_cb(s,
SSL_C_IS_EXPORT(s->s3->tmp.new_cipher),
SSL_C_EXPORT_PKEYLENGTH(s->s3->tmp.new_cipher));
if (dhp == NULL)
{
al=SSL_AD_HANDSHAKE_FAILURE;
SSLerr(SSL_F_SSL3_SEND_SERVER_KEY_EXCHANGE,SSL_R_MISSING_TMP_DH_KEY);
goto f_err;
}
if (s->s3->tmp.dh != NULL)
{
SSLerr(SSL_F_SSL3_SEND_SERVER_KEY_EXCHANGE,
ERR_R_INTERNAL_ERROR);
goto err;
}
if ((dh=DHparams_dup(dhp)) == NULL)
{
SSLerr(SSL_F_SSL3_SEND_SERVER_KEY_EXCHANGE,ERR_R_DH_LIB);
goto err;
}
s->s3->tmp.dh=dh;
if ((dhp->pub_key == NULL ||
dhp->priv_key == NULL ||
(s->options & SSL_OP_SINGLE_DH_USE)))
{
if(!DH_generate_key(dh))
{
SSLerr(SSL_F_SSL3_SEND_SERVER_KEY_EXCHANGE,
ERR_R_DH_LIB);
goto err;
}
}
else
{
dh->pub_key=BN_dup(dhp->pub_key);
dh->priv_key=BN_dup(dhp->priv_key);
if ((dh->pub_key == NULL) ||
(dh->priv_key == NULL))
{
SSLerr(SSL_F_SSL3_SEND_SERVER_KEY_EXCHANGE,ERR_R_DH_LIB);
goto err;
}
}
r[0]=dh->p;
r[1]=dh->g;
r[2]=dh->pub_key;
}
I.e., the "dhp" parameters provided by the callback (dh_tmp_cb) are
duplicated, and as Rainer mentioned, we would have to keep track
of dhp ourselves in order to be able to free it ourselves later on.
(In 2.2.x or before 2.4.7, these parameters/keys are part of mod_ssl's
global SSLModConfigRec, that's how the leak is avoided there.)
> It looks like to me that we only generate the parameters, not
> the keys. And reusing the same parameters should be save.
Correct as well, yes. DH_new sets pub_key and priv_key both to NULL,
so DH_generate_key is always called by ssl3_send_server_key_exchange().
> So something like:
Looks good to me. I suggest adding a short comment which explains
the rationale for using dh and dh_tmp (the SSL_CTX_set_tmp_dh_callback(3)
man page e.g. doesn't make it clear that reusing the parameters
within the lifetime of a process is actually a must to prevent memory
from leaking).
Kaspar
