From e45620140fce22c3251440063bc17440289d730c Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Thu, 11 Oct 2018 17:01:06 +0100 Subject: [PATCH] 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 (Merged from https://github.com/openssl/openssl/pull/7413) --- ssl/statem/statem_clnt.c | 12 ++++++++++++ ssl/statem/statem_lib.c | 13 ++++++++++++- 2 files changed, 24 insertions(+), 1 deletion(-) 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; -- 2.25.1