Implement TLSv1.3 style CertificateStatus

We remove the separate CertificateStatus message for TLSv1.3, and instead
send back the response in the appropriate Certificate message extension.

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2020)

7 files changed, 90 insertions, 64 deletions

diff --git a/ssl/statem/extensions.c b/ssl/statem/extensions.c
index 7fe80b52ee..9a76d8b1fa 100644
--- a/ssl/statem/extensions.c
+++ b/ssl/statem/extensions.c
@@ -22,8 +22,6 @@ static int final_ec_pt_formats(SSL *s, unsigned int context, int sent,
 static int init_session_ticket(SSL *s, unsigned int context);
 #ifndef OPENSSL_NO_OCSP
 static int init_status_request(SSL *s, unsigned int context);
-static int final_status_request(SSL *s, unsigned int context, int sent,
-                                        int *al);
 #endif
 #ifndef OPENSSL_NO_NEXTPROTONEG
 static int init_npn(SSL *s, unsigned int context);
@@ -162,7 +160,7 @@ static const EXTENSION_DEFINITION ext_defs[] = {
         | EXT_TLS1_3_CERTIFICATE,
         init_status_request, tls_parse_ctos_status_request,
         tls_parse_stoc_status_request, tls_construct_stoc_status_request,
-        tls_construct_ctos_status_request, final_status_request
+        tls_construct_ctos_status_request, NULL
     },
 #else
     INVALID_EXTENSION,
@@ -792,25 +790,17 @@ static int init_session_ticket(SSL *s, unsigned int context)
 #ifndef OPENSSL_NO_OCSP
 static int init_status_request(SSL *s, unsigned int context)
 {
-    if (s->server)
+    if (s->server) {
         s->tlsext_status_type = TLSEXT_STATUSTYPE_nothing;
-
-    return 1;
-}
-
-static int final_status_request(SSL *s, unsigned int context, int sent,
-                                        int *al)
-{
-    if (s->server)
-        return 1;
-
-    /*
-     * Ensure we get sensible values passed to tlsext_status_cb in the event
-     * that we don't receive a status message
-     */
-    OPENSSL_free(s->tlsext_ocsp_resp);
-    s->tlsext_ocsp_resp = NULL;
-    s->tlsext_ocsp_resplen = 0;
+    } else {
+        /*
+         * Ensure we get sensible values passed to tlsext_status_cb in the event
+         * that we don't receive a status message
+         */
+        OPENSSL_free(s->tlsext_ocsp_resp);
+        s->tlsext_ocsp_resp = NULL;
+        s->tlsext_ocsp_resplen = 0;
+    }
 
     return 1;
 }
diff --git a/ssl/statem/extensions_clnt.c b/ssl/statem/extensions_clnt.c
index 1fd4d84f73..2a9a182409 100644
--- a/ssl/statem/extensions_clnt.c
+++ b/ssl/statem/extensions_clnt.c
@@ -776,14 +776,24 @@ int tls_parse_stoc_status_request(SSL *s, PACKET *pkt, X509 *x, size_t chain,
                                   int *al)
 {
     /*
-     * MUST be empty and only sent if we've requested a status
-     * request message.
+     * MUST only be sent if we've requested a status
+     * request message. In TLS <= 1.2 it must also be empty.
      */
     if (s->tlsext_status_type == TLSEXT_STATUSTYPE_nothing
-            || PACKET_remaining(pkt) > 0) {
+            || (!SSL_IS_TLS13(s) && PACKET_remaining(pkt) > 0)) {
         *al = SSL_AD_UNSUPPORTED_EXTENSION;
         return 0;
     }
+
+    if (SSL_IS_TLS13(s)) {
+        /* We only know how to handle this if it's for the first Certificate in
+         * the chain. We ignore any other repsonses.
+         */
+        if (chain != 0)
+            return 1;
+        return tls_process_cert_status_body(s, pkt, al);
+    }
+
     /* Set flag to expect CertificateStatus message */
     s->tlsext_status_expected = 1;
 
diff --git a/ssl/statem/extensions_srvr.c b/ssl/statem/extensions_srvr.c
index 0bdfce8c7d..f2fc979d34 100644
--- a/ssl/statem/extensions_srvr.c
+++ b/ssl/statem/extensions_srvr.c
@@ -760,7 +760,18 @@ int tls_construct_stoc_status_request(SSL *s, WPACKET *pkt, X509 *x,
         return 1;
 
     if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_status_request)
-            || !WPACKET_put_bytes_u16(pkt, 0)) {
+            || !WPACKET_start_sub_packet_u16(pkt)) {
+        SSLerr(SSL_F_TLS_CONSTRUCT_STOC_STATUS_REQUEST, ERR_R_INTERNAL_ERROR);
+        return 0;
+    }
+
+    /*
+     * In TLSv1.3 we include the certificate status itself. In <= TLSv1.2 we
+     * send back an empty extension, with the certificate status appearing as a
+     * separate message
+     */
+    if ((SSL_IS_TLS13(s) && !tls_construct_cert_status_body(s, pkt))
+            || !WPACKET_close(pkt)) {
         SSLerr(SSL_F_TLS_CONSTRUCT_STOC_STATUS_REQUEST, ERR_R_INTERNAL_ERROR);
         return 0;
     }
diff --git a/ssl/statem/statem_clnt.c b/ssl/statem/statem_clnt.c
index 00062ff5ce..98e19b50ba 100644
--- a/ssl/statem/statem_clnt.c
+++ b/ssl/statem/statem_clnt.c
@@ -169,17 +169,6 @@ static int ossl_statem_client13_read_transition(SSL *s, int mt)
         break;
 
     case TLS_ST_CR_CERT:
-        /*
-         * The CertificateStatus message is optional even if
-         * |tlsext_status_expected| is set
-         */
-        if (s->tlsext_status_expected && mt == SSL3_MT_CERTIFICATE_STATUS) {
-            st->hand_state = TLS_ST_CR_CERT_STATUS;
-            return 1;
-        }
-        /* Fall through */
-
-    case TLS_ST_CR_CERT_STATUS:
         if (mt == SSL3_MT_FINISHED) {
             st->hand_state = TLS_ST_CR_FINISHED;
             return 1;
@@ -2191,41 +2180,57 @@ MSG_PROCESS_RETURN tls_process_new_session_ticket(SSL *s, PACKET *pkt)
     return MSG_PROCESS_ERROR;
 }
 
-MSG_PROCESS_RETURN tls_process_cert_status(SSL *s, PACKET *pkt)
+/*
+ * In TLSv1.3 this is called from the extensions code, otherwise it is used to
+ * parse a separate message. Returns 1 on success or 0 on failure. On failure
+ * |*al| is populated with a suitable alert code.
+ */
+int tls_process_cert_status_body(SSL *s, PACKET *pkt, int *al)
 {
-    int al;
     size_t resplen;
     unsigned int type;
 
     if (!PACKET_get_1(pkt, &type)
         || type != TLSEXT_STATUSTYPE_ocsp) {
-        al = SSL_AD_DECODE_ERROR;
-        SSLerr(SSL_F_TLS_PROCESS_CERT_STATUS, SSL_R_UNSUPPORTED_STATUS_TYPE);
-        goto f_err;
+        *al = SSL_AD_DECODE_ERROR;
+        SSLerr(SSL_F_TLS_PROCESS_CERT_STATUS_BODY,
+               SSL_R_UNSUPPORTED_STATUS_TYPE);
+        return 0;
     }
     if (!PACKET_get_net_3_len(pkt, &resplen)
         || PACKET_remaining(pkt) != resplen) {
-        al = SSL_AD_DECODE_ERROR;
-        SSLerr(SSL_F_TLS_PROCESS_CERT_STATUS, SSL_R_LENGTH_MISMATCH);
-        goto f_err;
+        *al = SSL_AD_DECODE_ERROR;
+        SSLerr(SSL_F_TLS_PROCESS_CERT_STATUS_BODY, SSL_R_LENGTH_MISMATCH);
+        return 0;
     }
     s->tlsext_ocsp_resp = OPENSSL_malloc(resplen);
     if (s->tlsext_ocsp_resp == NULL) {
-        al = SSL_AD_INTERNAL_ERROR;
-        SSLerr(SSL_F_TLS_PROCESS_CERT_STATUS, ERR_R_MALLOC_FAILURE);
-        goto f_err;
+        *al = SSL_AD_INTERNAL_ERROR;
+        SSLerr(SSL_F_TLS_PROCESS_CERT_STATUS_BODY, ERR_R_MALLOC_FAILURE);
+        return 0;
     }
     if (!PACKET_copy_bytes(pkt, s->tlsext_ocsp_resp, resplen)) {
-        al = SSL_AD_DECODE_ERROR;
-        SSLerr(SSL_F_TLS_PROCESS_CERT_STATUS, SSL_R_LENGTH_MISMATCH);
-        goto f_err;
+        *al = SSL_AD_DECODE_ERROR;
+        SSLerr(SSL_F_TLS_PROCESS_CERT_STATUS_BODY, SSL_R_LENGTH_MISMATCH);
+        return 0;
     }
     s->tlsext_ocsp_resplen = resplen;
+
+    return 1;
+}
+    
+
+MSG_PROCESS_RETURN tls_process_cert_status(SSL *s, PACKET *pkt)
+{
+    int al;
+
+    if (!tls_process_cert_status_body(s, pkt, &al)) {
+        ssl3_send_alert(s, SSL3_AL_FATAL, al);
+        ossl_statem_set_error(s);
+        return MSG_PROCESS_ERROR;
+    }
+
     return MSG_PROCESS_CONTINUE_READING;
- f_err:
-    ssl3_send_alert(s, SSL3_AL_FATAL, al);
-    ossl_statem_set_error(s);
-    return MSG_PROCESS_ERROR;
 }
 
 /*
diff --git a/ssl/statem/statem_lib.c b/ssl/statem/statem_lib.c
index f41da103a4..cc5ca67b97 100644
--- a/ssl/statem/statem_lib.c
+++ b/ssl/statem/statem_lib.c
@@ -317,13 +317,13 @@ static int ssl_add_cert_to_wpacket(SSL *s, WPACKET *pkt, X509 *x, int chain,
 
     len = i2d_X509(x, NULL);
     if (len < 0) {
-        SSLerr(SSL_F_SSL_ADD_CERT_TO_BUF, ERR_R_BUF_LIB);
+        SSLerr(SSL_F_SSL_ADD_CERT_TO_WPACKET, ERR_R_BUF_LIB);
         *al = SSL_AD_INTERNAL_ERROR;
         return 0;
     }
     if (!WPACKET_sub_allocate_bytes_u24(pkt, len, &outbytes)
             || i2d_X509(x, &outbytes) != len) {
-        SSLerr(SSL_F_SSL_ADD_CERT_TO_BUF, ERR_R_INTERNAL_ERROR);
+        SSLerr(SSL_F_SSL_ADD_CERT_TO_WPACKET, ERR_R_INTERNAL_ERROR);
         *al = SSL_AD_INTERNAL_ERROR;
         return 0;
     }
diff --git a/ssl/statem/statem_locl.h b/ssl/statem/statem_locl.h
index 5fa190aada..35115c9e93 100644
--- a/ssl/statem/statem_locl.h
+++ b/ssl/statem/statem_locl.h
@@ -115,6 +115,7 @@ __owur int tls_construct_client_hello(SSL *s, WPACKET *pkt);
 __owur MSG_PROCESS_RETURN tls_process_server_hello(SSL *s, PACKET *pkt);
 __owur MSG_PROCESS_RETURN tls_process_certificate_request(SSL *s, PACKET *pkt);
 __owur MSG_PROCESS_RETURN tls_process_new_session_ticket(SSL *s, PACKET *pkt);
+__owur int tls_process_cert_status_body(SSL *s, PACKET *pkt, int *al);
 __owur MSG_PROCESS_RETURN tls_process_cert_status(SSL *s, PACKET *pkt);
 __owur MSG_PROCESS_RETURN tls_process_server_done(SSL *s, PACKET *pkt);
 __owur int tls_construct_client_verify(SSL *s, WPACKET *pkt);
@@ -123,6 +124,7 @@ __owur int tls_construct_client_certificate(SSL *s, WPACKET *pkt);
 __owur int ssl_do_client_cert_cb(SSL *s, X509 **px509, EVP_PKEY **ppkey);
 __owur int tls_construct_client_key_exchange(SSL *s, WPACKET *pkt);
 __owur int tls_client_key_exchange_post_work(SSL *s);
+__owur int tls_construct_cert_status_body(SSL *s, WPACKET *pkt);
 __owur int tls_construct_cert_status(SSL *s, WPACKET *pkt);
 __owur MSG_PROCESS_RETURN tls_process_key_exchange(SSL *s, PACKET *pkt);
 __owur MSG_PROCESS_RETURN tls_process_server_certificate(SSL *s, PACKET *pkt);
diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c
index 5e230f086b..8b765a9aae 100644
--- a/ssl/statem/statem_srvr.c
+++ b/ssl/statem/statem_srvr.c
@@ -427,12 +427,7 @@ static WRITE_TRAN ossl_statem_server13_write_transition(SSL *s)
         return WRITE_TRAN_CONTINUE;
 
     case TLS_ST_SW_CERT:
-            st->hand_state = s->tlsext_status_expected ? TLS_ST_SW_CERT_STATUS
-                                                       : TLS_ST_SW_FINISHED;
-        return WRITE_TRAN_CONTINUE;
-
-    case TLS_ST_SW_CERT_STATUS:
-        st->hand_state = TLS_ST_SW_FINISHED;
+            st->hand_state = TLS_ST_SW_FINISHED;
         return WRITE_TRAN_CONTINUE;
 
     case TLS_ST_SW_FINISHED:
@@ -3464,12 +3459,25 @@ int tls_construct_new_session_ticket(SSL *s, WPACKET *pkt)
     return 0;
 }
 
-int tls_construct_cert_status(SSL *s, WPACKET *pkt)
+/*
+ * In TLSv1.3 this is called from the extensions code, otherwise it is used to
+ * create a separate message. Returns 1 on success or 0 on failure.
+ */
+int tls_construct_cert_status_body(SSL *s, WPACKET *pkt)
 {
     if (!WPACKET_put_bytes_u8(pkt, s->tlsext_status_type)
             || !WPACKET_sub_memcpy_u24(pkt, s->tlsext_ocsp_resp,
                                        s->tlsext_ocsp_resplen)) {
-        SSLerr(SSL_F_TLS_CONSTRUCT_CERT_STATUS, ERR_R_INTERNAL_ERROR);
+        SSLerr(SSL_F_TLS_CONSTRUCT_CERT_STATUS_BODY, ERR_R_INTERNAL_ERROR);
+        return 0;
+    }
+
+    return 1;
+}
+
+int tls_construct_cert_status(SSL *s, WPACKET *pkt)
+{
+    if (!tls_construct_cert_status_body(s, pkt)) {
         ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR);
         return 0;
     }