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: NRich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2020)

include/openssl/ssl.h

@@ -2136,6 +2136,7 @@ int ERR_load_SSL_strings(void);
 # define SSL_F_SSL3_WRITE_PENDING                         159
 # define SSL_F_SSL_ADD_CERT_CHAIN                         316
 # define SSL_F_SSL_ADD_CERT_TO_BUF                        319
+# define SSL_F_SSL_ADD_CERT_TO_WPACKET                    493
 # define SSL_F_SSL_ADD_CLIENTHELLO_RENEGOTIATE_EXT        298
 # define SSL_F_SSL_ADD_CLIENTHELLO_TLSEXT                 277
 # define SSL_F_SSL_ADD_CLIENTHELLO_USE_SRTP_EXT           307
@@ -2261,6 +2262,7 @@ int ERR_load_SSL_strings(void);
 # define SSL_F_TLS_COLLECT_EXTENSIONS                     435
 # define SSL_F_TLS_CONSTRUCT_CERTIFICATE_REQUEST          372
 # define SSL_F_TLS_CONSTRUCT_CERT_STATUS                  429
+# define SSL_F_TLS_CONSTRUCT_CERT_STATUS_BODY             494
 # define SSL_F_TLS_CONSTRUCT_CHANGE_CIPHER_SPEC           427
 # define SSL_F_TLS_CONSTRUCT_CKE_DHE                      404
 # define SSL_F_TLS_CONSTRUCT_CKE_ECDHE                    405
@@ -2332,6 +2334,7 @@ int ERR_load_SSL_strings(void);
 # define SSL_F_TLS_PREPARE_CLIENT_CERTIFICATE             360
 # define SSL_F_TLS_PROCESS_CERTIFICATE_REQUEST            361
 # define SSL_F_TLS_PROCESS_CERT_STATUS                    362
+# define SSL_F_TLS_PROCESS_CERT_STATUS_BODY               495
 # define SSL_F_TLS_PROCESS_CERT_VERIFY                    379
 # define SSL_F_TLS_PROCESS_CHANGE_CIPHER_SPEC             363
 # define SSL_F_TLS_PROCESS_CKE_DHE                        411

ssl/ssl_err.c

@@ -92,6 +92,7 @@ static ERR_STRING_DATA SSL_str_functs[] = {
     {ERR_FUNC(SSL_F_SSL3_WRITE_PENDING), "ssl3_write_pending"},
     {ERR_FUNC(SSL_F_SSL_ADD_CERT_CHAIN), "ssl_add_cert_chain"},
     {ERR_FUNC(SSL_F_SSL_ADD_CERT_TO_BUF), "ssl_add_cert_to_buf"},
+    {ERR_FUNC(SSL_F_SSL_ADD_CERT_TO_WPACKET), "ssl_add_cert_to_wpacket"},
     {ERR_FUNC(SSL_F_SSL_ADD_CLIENTHELLO_RENEGOTIATE_EXT),
      "ssl_add_clienthello_renegotiate_ext"},
     {ERR_FUNC(SSL_F_SSL_ADD_CLIENTHELLO_TLSEXT),
@@ -259,6 +260,8 @@ static ERR_STRING_DATA SSL_str_functs[] = {
     {ERR_FUNC(SSL_F_TLS_CONSTRUCT_CERTIFICATE_REQUEST),
      "tls_construct_certificate_request"},
     {ERR_FUNC(SSL_F_TLS_CONSTRUCT_CERT_STATUS), "tls_construct_cert_status"},
+    {ERR_FUNC(SSL_F_TLS_CONSTRUCT_CERT_STATUS_BODY),
+     "tls_construct_cert_status_body"},
     {ERR_FUNC(SSL_F_TLS_CONSTRUCT_CHANGE_CIPHER_SPEC),
      "tls_construct_change_cipher_spec"},
     {ERR_FUNC(SSL_F_TLS_CONSTRUCT_CKE_DHE), "tls_construct_cke_dhe"},
@@ -373,6 +376,8 @@ static ERR_STRING_DATA SSL_str_functs[] = {
     {ERR_FUNC(SSL_F_TLS_PROCESS_CERTIFICATE_REQUEST),
      "tls_process_certificate_request"},
     {ERR_FUNC(SSL_F_TLS_PROCESS_CERT_STATUS), "tls_process_cert_status"},
+    {ERR_FUNC(SSL_F_TLS_PROCESS_CERT_STATUS_BODY),
+     "tls_process_cert_status_body"},
     {ERR_FUNC(SSL_F_TLS_PROCESS_CERT_VERIFY), "tls_process_cert_verify"},
     {ERR_FUNC(SSL_F_TLS_PROCESS_CHANGE_CIPHER_SPEC),
      "tls_process_change_cipher_spec"},

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;
 }

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;
 

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;
     }

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;
 }
 
 /*

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;
     }

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);

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;
     }

test/recipes/70-test_tls13messages.t

@@ -43,8 +43,6 @@ $ENV{CTLOG_FILE} = srctop_file("test", "ct", "log_list.conf");
         checkhandshake::CLIENT_AUTH_HANDSHAKE],
     [TLSProxy::Message::MT_CERTIFICATE,
         checkhandshake::ALL_HANDSHAKES & ~checkhandshake::RESUME_HANDSHAKE],
-    [TLSProxy::Message::MT_CERTIFICATE_STATUS,
-        checkhandshake::OCSP_HANDSHAKE],
     [TLSProxy::Message::MT_FINISHED,
         checkhandshake::ALL_HANDSHAKES],
     [TLSProxy::Message::MT_CERTIFICATE,
@@ -148,7 +146,7 @@ $proxy->clientflags("-status");
 $proxy->serverflags("-status_file "
                     .srctop_file("test", "recipes", "ocsp-response.der"));
 $proxy->start();
-checkhandshake($proxy, checkhandshake::OCSP_HANDSHAKE,
+checkhandshake($proxy, checkhandshake::DEFAULT_HANDSHAKE,
                checkhandshake::DEFAULT_EXTENSIONS
                | checkhandshake::STATUS_REQUEST_CLI_EXTENSION
                | checkhandshake::STATUS_REQUEST_SRV_EXTENSION,
@@ -232,7 +230,7 @@ $proxy->clientflags("-ct");
 $proxy->serverflags("-status_file "
                     .srctop_file("test", "recipes", "ocsp-response.der"));
 $proxy->start();
-checkhandshake($proxy, checkhandshake::OCSP_HANDSHAKE,
+checkhandshake($proxy, checkhandshake::DEFAULT_HANDSHAKE,
                checkhandshake::DEFAULT_EXTENSIONS
                | checkhandshake::SCT_CLI_EXTENSION
                | checkhandshake::STATUS_REQUEST_CLI_EXTENSION