commit f350351a9a9f0b11da9d5a2683884e1a1dc2c0fc
author David Benjamin <davidben@google.com> Tue Aug 20 15:55:24 2019 -0400
committer CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org> Thu Aug 22 15:43:08 2019 +0000
tree 003c23e7c223d2ed96ae6afb93c70d875b9225d5
parent e39d13656889639655ceec15ab3a1aa5a4e50652

Align 0-RTT and resumption state machines slightly

QUIC is going to make this flow a bit more complicated, so let's simplify it a
bit. The client flight states already know to skip themselves in 1-RTT
resumption, so just run through the same states. Also remove a redundant
early_data_offered check.

(I think we originally skipped the states because we do half-RTT tickets and
the transcript bits were precomputed.)

Bug: 221
Change-Id: I3a62c864458012e74b46f7ef212abc125760c12d
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/37144
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>

ssl/tls13_server.cc

diff --git a/ssl/tls13_server.cc b/ssl/tls13_server.cc
index d99b5b4..f1891cf 100644
--- a/ssl/tls13_server.cc
+++ b/ssl/tls13_server.cc
@@ -787,42 +787,42 @@
 
 static enum ssl_hs_wait_t do_process_end_of_early_data(SSL_HANDSHAKE *hs) {
   SSL *const ssl = hs->ssl;
-  if (hs->early_data_offered) {
-    // If early data was not accepted, the EndOfEarlyData and ChangeCipherSpec
-    // message will be in the discarded early data.
-    if (hs->ssl->s3->early_data_accepted) {
-      SSLMessage msg;
-      if (!ssl->method->get_message(ssl, &msg)) {
-        return ssl_hs_read_message;
-      }
-
-      if (!ssl_check_message_type(ssl, msg, SSL3_MT_END_OF_EARLY_DATA)) {
-        return ssl_hs_error;
-      }
-      if (CBS_len(&msg.body) != 0) {
-        ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
-        OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR);
-        return ssl_hs_error;
-      }
-      ssl->method->next_message(ssl);
+  // If early data was not accepted, the EndOfEarlyData will be in the discarded
+  // early data.
+  if (hs->ssl->s3->early_data_accepted) {
+    SSLMessage msg;
+    if (!ssl->method->get_message(ssl, &msg)) {
+      return ssl_hs_read_message;
     }
+    if (!ssl_check_message_type(ssl, msg, SSL3_MT_END_OF_EARLY_DATA)) {
+      return ssl_hs_error;
+    }
+    if (CBS_len(&msg.body) != 0) {
+      ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
+      OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR);
+      return ssl_hs_error;
+    }
+    ssl->method->next_message(ssl);
   }
   if (!tls13_set_traffic_key(ssl, ssl_encryption_handshake, evp_aead_open,
                              hs->client_handshake_secret())) {
     return ssl_hs_error;
   }
-  hs->tls13_state = ssl->s3->early_data_accepted
-                        ? state_read_client_finished
-                        : state_read_client_certificate;
+  hs->tls13_state = state_read_client_certificate;
   return ssl_hs_ok;
 }
 
 static enum ssl_hs_wait_t do_read_client_certificate(SSL_HANDSHAKE *hs) {
   SSL *const ssl = hs->ssl;
   if (!hs->cert_request) {
-    // OpenSSL returns X509_V_OK when no certificates are requested. This is
-    // classed by them as a bug, but it's assumed by at least NGINX.
-    hs->new_session->verify_result = X509_V_OK;
+    if (!ssl->s3->session_reused) {
+      // OpenSSL returns X509_V_OK when no certificates are requested. This is
+      // classed by them as a bug, but it's assumed by at least NGINX. (Only do
+      // this in full handshakes as resumptions should carry over the previous
+      // |verify_result|, though this is a no-op because servers do not
+      // implement the client's odd soft-fail mode.)
+      hs->new_session->verify_result = X509_V_OK;
+    }
 
     // Skip this state.
     hs->tls13_state = state_read_channel_id;