Ensure the end of first server flight processing is done

There is a set of miscellaneous processing for OCSP, CT etc at the end of
the ServerDone processing. In TLS1.3 we don't have a ServerDone, so this
needs to move elsewhere.

Reviewed-by: Rich Salz <rsalz@openssl.org>

diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index 2fd0e9fb42e7248fe04b5f03d6ab17cc48f5a1a5..8769f46ba2e34b3a1f7594df74b506a0ab554cf9 100644 (file)
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -2294,6 +2294,7 @@ int ERR_load_SSL_strings(void);
 # define SSL_F_TLS_PROCESS_CLIENT_HELLO                   381
 # define SSL_F_TLS_PROCESS_CLIENT_KEY_EXCHANGE            382
 # define SSL_F_TLS_PROCESS_FINISHED                       364
+# define SSL_F_TLS_PROCESS_INITIAL_SERVER_FLIGHT          442
 # define SSL_F_TLS_PROCESS_KEY_EXCHANGE                   365
 # define SSL_F_TLS_PROCESS_NEW_SESSION_TICKET             366
 # define SSL_F_TLS_PROCESS_NEXT_PROTO                     383

diff --git a/ssl/ssl_err.c b/ssl/ssl_err.c
index 825c5638b843ae106e4fac30d4205f3f287f5a96..49a9d44b3306f4321db52155f3fe6f30ddf6b69e 100644 (file)
--- a/ssl/ssl_err.c
+++ b/ssl/ssl_err.c
@@ -313,6 +313,8 @@ static ERR_STRING_DATA SSL_str_functs[] = {
     {ERR_FUNC(SSL_F_TLS_PROCESS_CLIENT_KEY_EXCHANGE),
      "tls_process_client_key_exchange"},
     {ERR_FUNC(SSL_F_TLS_PROCESS_FINISHED), "tls_process_finished"},
+    {ERR_FUNC(SSL_F_TLS_PROCESS_INITIAL_SERVER_FLIGHT),
+     "tls_process_initial_server_flight"},
     {ERR_FUNC(SSL_F_TLS_PROCESS_KEY_EXCHANGE), "tls_process_key_exchange"},
     {ERR_FUNC(SSL_F_TLS_PROCESS_NEW_SESSION_TICKET),
      "tls_process_new_session_ticket"},

diff --git a/ssl/statem/statem_clnt.c b/ssl/statem/statem_clnt.c
index be3148df0f7db1cb0231a52836291f48768e8eca..97458503873bfb823fbd179cf98ea0c9759bdc56 100644 (file)
--- a/ssl/statem/statem_clnt.c
+++ b/ssl/statem/statem_clnt.c
@@ -2186,34 +2186,22 @@ MSG_PROCESS_RETURN tls_process_cert_status(SSL *s, PACKET *pkt)
     return MSG_PROCESS_ERROR;
 }
 
-MSG_PROCESS_RETURN tls_process_server_done(SSL *s, PACKET *pkt)
+/*
+ * Perform miscellaneous checks and processing after we have received the
+ * server's initial flight. In TLS1.3 this is after the Server Finished message.
+ * In <=TLS1.2 this is after the ServerDone message.
+ *
+ * Returns 1 on success or 0 on failure.
+ */
+int tls_process_initial_server_flight(SSL *s, int *al)
 {
-    if (PACKET_remaining(pkt) > 0) {
-        /* should contain no data */
-        ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
-        SSLerr(SSL_F_TLS_PROCESS_SERVER_DONE, SSL_R_LENGTH_MISMATCH);
-        ossl_statem_set_error(s);
-        return MSG_PROCESS_ERROR;
-    }
-#ifndef OPENSSL_NO_SRP
-    if (s->s3->tmp.new_cipher->algorithm_mkey & SSL_kSRP) {
-        if (SRP_Calc_A_param(s) <= 0) {
-            SSLerr(SSL_F_TLS_PROCESS_SERVER_DONE, SSL_R_SRP_A_CALC);
-            ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR);
-            ossl_statem_set_error(s);
-            return MSG_PROCESS_ERROR;
-        }
-    }
-#endif
-
     /*
      * at this point we check that we have the required stuff from
      * the server
      */
     if (!ssl3_check_cert_and_algorithm(s)) {
-        ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_HANDSHAKE_FAILURE);
-        ossl_statem_set_error(s);
-        return MSG_PROCESS_ERROR;
+        *al = SSL_AD_HANDSHAKE_FAILURE;
+        return 0;
     }
 
     /*
@@ -2225,28 +2213,56 @@ MSG_PROCESS_RETURN tls_process_server_done(SSL *s, PACKET *pkt)
         int ret;
         ret = s->ctx->tlsext_status_cb(s, s->ctx->tlsext_status_arg);
         if (ret == 0) {
-            ssl3_send_alert(s, SSL3_AL_FATAL,
-                            SSL_AD_BAD_CERTIFICATE_STATUS_RESPONSE);
-            SSLerr(SSL_F_TLS_PROCESS_SERVER_DONE,
+            *al = SSL_AD_BAD_CERTIFICATE_STATUS_RESPONSE;
+            SSLerr(SSL_F_TLS_PROCESS_INITIAL_SERVER_FLIGHT,
                    SSL_R_INVALID_STATUS_RESPONSE);
-            return MSG_PROCESS_ERROR;
+            return 0;
         }
         if (ret < 0) {
-            ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR);
-            SSLerr(SSL_F_TLS_PROCESS_SERVER_DONE, ERR_R_MALLOC_FAILURE);
-            return MSG_PROCESS_ERROR;
+            *al = SSL_AD_INTERNAL_ERROR;
+            SSLerr(SSL_F_TLS_PROCESS_INITIAL_SERVER_FLIGHT,
+                   ERR_R_MALLOC_FAILURE);
+            return 0;
         }
     }
 #ifndef OPENSSL_NO_CT
     if (s->ct_validation_callback != NULL) {
         /* Note we validate the SCTs whether or not we abort on error */
         if (!ssl_validate_ct(s) && (s->verify_mode & SSL_VERIFY_PEER)) {
-            ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_HANDSHAKE_FAILURE);
-            return MSG_PROCESS_ERROR;
+            *al = SSL_AD_HANDSHAKE_FAILURE;
+            return 0;
+        }
+    }
+#endif
+
+    return 1;
+}
+
+MSG_PROCESS_RETURN tls_process_server_done(SSL *s, PACKET *pkt)
+{
+    int al = SSL_AD_INTERNAL_ERROR;
+
+    if (PACKET_remaining(pkt) > 0) {
+        /* should contain no data */
+        al = SSL_AD_DECODE_ERROR;
+        SSLerr(SSL_F_TLS_PROCESS_SERVER_DONE, SSL_R_LENGTH_MISMATCH);
+        goto err;
+    }
+#ifndef OPENSSL_NO_SRP
+    if (s->s3->tmp.new_cipher->algorithm_mkey & SSL_kSRP) {
+        if (SRP_Calc_A_param(s) <= 0) {
+            SSLerr(SSL_F_TLS_PROCESS_SERVER_DONE, SSL_R_SRP_A_CALC);
+            goto err;
         }
     }
 #endif
 
+    /*
+     * Error queue messages are generated directly by this function
+     */
+    if (!tls_process_initial_server_flight(s, &al))
+        goto err;
+
 #ifndef OPENSSL_NO_SCTP
     /* Only applies to renegotiation */
     if (SSL_IS_DTLS(s) && BIO_dgram_is_sctp(SSL_get_wbio(s))
@@ -2255,6 +2271,11 @@ MSG_PROCESS_RETURN tls_process_server_done(SSL *s, PACKET *pkt)
     else
 #endif
         return MSG_PROCESS_FINISHED_READING;
+
+ err:
+    ssl3_send_alert(s, SSL3_AL_FATAL, al);
+    ossl_statem_set_error(s);
+    return MSG_PROCESS_ERROR;
 }
 
 static int tls_construct_cke_psk_preamble(SSL *s, WPACKET *pkt, int *al)

diff --git a/ssl/statem/statem_lib.c b/ssl/statem/statem_lib.c
index 6d32666e43f431caa89c9a179cb966199cee1cca..a971c51631f4fa437d79cc871f55f73803a08fd7 100644 (file)
--- a/ssl/statem/statem_lib.c
+++ b/ssl/statem/statem_lib.c
@@ -326,7 +326,7 @@ MSG_PROCESS_RETURN tls_process_change_cipher_spec(SSL *s, PACKET *pkt)
 
 MSG_PROCESS_RETURN tls_process_finished(SSL *s, PACKET *pkt)
 {
-    int al;
+    int al = SSL_AD_INTERNAL_ERROR;
     size_t md_len;
 
     /* If this occurs, we have missed a message */
@@ -367,12 +367,14 @@ MSG_PROCESS_RETURN tls_process_finished(SSL *s, PACKET *pkt)
         s->s3->previous_server_finished_len = md_len;
     }
 
-    /* In TLS1.3 we also have to change cipher state */
+    /*
+     * In TLS1.3 we also have to change cipher state and do any final processing
+     * of the initial server flight (if we are a client)
+     */
     if (SSL_IS_TLS13(s)) {
         if (s->server) {
             if (!s->method->ssl3_enc->change_cipher_state(s,
                     SSL3_CC_APPLICATION | SSL3_CHANGE_CIPHER_SERVER_READ)) {
-                al = SSL_AD_INTERNAL_ERROR;
                 SSLerr(SSL_F_TLS_PROCESS_FINISHED, SSL_R_CANNOT_CHANGE_CIPHER);
                 goto f_err;
             }
@@ -380,16 +382,16 @@ MSG_PROCESS_RETURN tls_process_finished(SSL *s, PACKET *pkt)
             if (!s->method->ssl3_enc->generate_master_secret(s,
                     s->session->master_key, s->handshake_secret, 0,
                     &s->session->master_key_length)) {
-                al = SSL_AD_INTERNAL_ERROR;
                 SSLerr(SSL_F_TLS_PROCESS_FINISHED, SSL_R_CANNOT_CHANGE_CIPHER);
                 goto f_err;
             }
             if (!s->method->ssl3_enc->change_cipher_state(s,
                     SSL3_CC_APPLICATION | SSL3_CHANGE_CIPHER_CLIENT_READ)) {
-                al = SSL_AD_INTERNAL_ERROR;
                 SSLerr(SSL_F_TLS_PROCESS_FINISHED, SSL_R_CANNOT_CHANGE_CIPHER);
                 goto f_err;
             }
+            if (!tls_process_initial_server_flight(s, &al))
+                goto f_err;
         }
     }
 

diff --git a/ssl/statem/statem_locl.h b/ssl/statem/statem_locl.h
index 740595bba0aff8181013d778cf14244d6e2dd45f..f6c76ab3a2120d270f9898019f0cc3f46bff072c 100644 (file)
--- a/ssl/statem/statem_locl.h
+++ b/ssl/statem/statem_locl.h
@@ -77,6 +77,7 @@ __owur int tls_get_message_body(SSL *s, size_t *len);
 __owur int dtls_get_message(SSL *s, int *mt, size_t *len);
 
 /* Message construction and processing functions */
+__owur int tls_process_initial_server_flight(SSL *s, int *al);
 __owur MSG_PROCESS_RETURN tls_process_change_cipher_spec(SSL *s, PACKET *pkt);
 __owur MSG_PROCESS_RETURN tls_process_finished(SSL *s, PACKET *pkt);
 __owur int tls_construct_change_cipher_spec(SSL *s, WPACKET *pkt);