From: Matt Caswell <matt@openssl.org>
Date: Wed, 14 Oct 2020 14:13:28 +0000 (+0100)
Subject: Pass an EVP_PKEY for SSL_SECOP_TMP_DH in the security callback
X-Git-Tag: openssl-3.0.0-alpha8~61
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=47e81a1bfa5ebd21f23c19745c7f5f93d141b02f;p=thirdparty%2Fopenssl.git

Pass an EVP_PKEY for SSL_SECOP_TMP_DH in the security callback

The security operation SSL_SECOP_TMP_DH is defined to take an EVP_PKEY
in the "other" parameter:

 /* Temporary DH key */
 # define SSL_SECOP_TMP_DH                (7 | SSL_SECOP_OTHER_PKEY)

In most places this is what is passed. All these places occur server side.
However there is one client side call of this security operation and it
passes a DH object instead. This is incorrect according to the
definition of SSL_SECOP_TMP_DH, and is inconsistent with all of the other
locations.

Our own default security callback, and the debug callback in the apps,
never look at this value and therefore this issue was never noticed
previously. In theory a client side application could be relying on this
behaviour and could be broken by this change. This is probably fairly
unlikely but can't be ruled out.

Reviewed-by: Paul Dale <paul.dale@oracle.com>
Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
Reviewed-by: Ben Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/13136)
---

diff --git a/ssl/statem/statem_clnt.c b/ssl/statem/statem_clnt.c
index bda6f48f462..ba57d31428c 100644
--- a/ssl/statem/statem_clnt.c
+++ b/ssl/statem/statem_clnt.c
@@ -2163,7 +2163,7 @@ static int tls_process_ske_dhe(SSL *s, PACKET *pkt, EVP_PKEY **pkey)
     dh = NULL;
 
     if (!ssl_security(s, SSL_SECOP_TMP_DH, EVP_PKEY_security_bits(peer_tmp),
-                      0, EVP_PKEY_get0_DH(peer_tmp))) {
+                      0, peer_tmp)) {
         SSLfatal(s, SSL_AD_HANDSHAKE_FAILURE, SSL_F_TLS_PROCESS_SKE_DHE,
                  SSL_R_DH_KEY_TOO_SMALL);
         goto err;