From: Matt Caswell <matt@openssl.org>
Date: Thu, 11 Oct 2018 16:01:06 +0000 (+0100)
Subject: Don't call the client_cert_cb immediately in TLSv1.3
X-Git-Tag: OpenSSL_1_1_1a~61
X-Git-Url: https://git.openssl.org/gitweb/?p=openssl.git;a=commitdiff_plain;h=a2388b50afc5136a1b65d0bf794f0398c31a1acb

Don't call the client_cert_cb immediately in TLSv1.3

In TLSv1.2 and below a CertificateRequest is sent after the Certificate
from the server. This means that by the time the client_cert_cb is called
on receipt of the CertificateRequest a call to SSL_get_peer_certificate()
will return the server certificate as expected. In TLSv1.3 a
CertificateRequest is sent before a Certificate message so calling
SSL_get_peer_certificate() returns NULL.

To workaround this we delay calling the client_cert_cb until after we
have processed the CertificateVerify message, when we are doing TLSv1.3.

Fixes #7384

Reviewed-by: Ben Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/7413)

(cherry picked from commit e45620140fce22c3251440063bc17440289d730c)
---

diff --git a/ssl/statem/statem_clnt.c b/ssl/statem/statem_clnt.c
index 8c658da899..0a11b88183 100644
--- a/ssl/statem/statem_clnt.c
+++ b/ssl/statem/statem_clnt.c
@@ -1095,6 +1095,7 @@ WORK_STATE ossl_statem_client_post_process_message(SSL *s, WORK_STATE wst)
                  ERR_R_INTERNAL_ERROR);
         return WORK_ERROR;
 
+    case TLS_ST_CR_CERT_VRFY:
     case TLS_ST_CR_CERT_REQ:
         return tls_prepare_client_certificate(s, wst);
     }
@@ -2563,6 +2564,17 @@ MSG_PROCESS_RETURN tls_process_certificate_request(SSL *s, PACKET *pkt)
     /* we should setup a certificate to return.... */
     s->s3->tmp.cert_req = 1;
 
+    /*
+     * In TLSv1.3 we don't prepare the client certificate yet. We wait until
+     * after the CertificateVerify message has been received. This is because
+     * in TLSv1.3 the CertificateRequest arrives before the Certificate message
+     * but in TLSv1.2 it is the other way around. We want to make sure that
+     * SSL_get_peer_certificate() returns something sensible in
+     * client_cert_cb.
+     */
+    if (SSL_IS_TLS13(s) && s->post_handshake_auth != SSL_PHA_REQUESTED)
+        return MSG_PROCESS_CONTINUE_READING;
+
     return MSG_PROCESS_CONTINUE_PROCESSING;
 }
 
diff --git a/ssl/statem/statem_lib.c b/ssl/statem/statem_lib.c
index e6e61f7876..75cf321b98 100644
--- a/ssl/statem/statem_lib.c
+++ b/ssl/statem/statem_lib.c
@@ -495,7 +495,18 @@ MSG_PROCESS_RETURN tls_process_cert_verify(SSL *s, PACKET *pkt)
         }
     }
 
-    ret = MSG_PROCESS_CONTINUE_READING;
+    /*
+     * In TLSv1.3 on the client side we make sure we prepare the client
+     * certificate after the CertVerify instead of when we get the
+     * CertificateRequest. This is because in TLSv1.3 the CertificateRequest
+     * comes *before* the Certificate message. In TLSv1.2 it comes after. We
+     * want to make sure that SSL_get_peer_certificate() will return the actual
+     * server certificate from the client_cert_cb callback.
+     */
+    if (!s->server && SSL_IS_TLS13(s) && s->s3->tmp.cert_req == 1)
+        ret = MSG_PROCESS_CONTINUE_PROCESSING;
+    else
+        ret = MSG_PROCESS_CONTINUE_READING;
  err:
     BIO_free(s->s3->handshake_buffer);
     s->s3->handshake_buffer = NULL;