Move the early_data_{offered,reason} logic out of extension callbacks.

ECH requires that we construct two ClientHellos. That means our
add_clienthello callbacks will need to be called multiple times and
should be const. (They already are called multiple times for
HelloRetryRequest, but we currently thread that through the callbacks a
bit. With ECH, I think we need to make them pure serialization.)

Bug: 275
Change-Id: I11f8195fd2ec4b8639f0a2af01a24d4974445580
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/47984
Reviewed-by: Adam Langley <agl@google.com>

ssl/handshake_client.cc

@@ -374,6 +374,54 @@ static bool parse_supported_versions(SSL_HANDSHAKE *hs, uint16_t *version,
   return true;
 }
 
+// should_offer_early_data returns |ssl_early_data_accepted| if |hs| should
+// offer early data, and some other reason code otherwise.
+static ssl_early_data_reason_t should_offer_early_data(
+    const SSL_HANDSHAKE *hs) {
+  const SSL *const ssl = hs->ssl;
+  assert(!ssl->server);
+  if (!ssl->enable_early_data) {
+    return ssl_early_data_disabled;
+  }
+
+  if (hs->max_version < TLS1_3_VERSION) {
+    // We discard inapplicable sessions, so this is redundant with the session
+    // checks below, but reporting that TLS 1.3 was disabled is more useful.
+    return ssl_early_data_protocol_version;
+  }
+
+  if (ssl->session == nullptr) {
+    return ssl_early_data_no_session_offered;
+  }
+
+  if (ssl_session_protocol_version(ssl->session.get()) < TLS1_3_VERSION ||
+      ssl->session->ticket_max_early_data == 0) {
+    return ssl_early_data_unsupported_for_session;
+  }
+
+  if (!ssl->session->early_alpn.empty()) {
+    if (!ssl_is_alpn_protocol_allowed(hs, ssl->session->early_alpn)) {
+      // Avoid reporting a confusing value in |SSL_get0_alpn_selected|.
+      return ssl_early_data_alpn_mismatch;
+    }
+
+    // If the previous connection negotiated ALPS, only offer 0-RTT when the
+    // local are settings are consistent with what we'd offer for this
+    // connection.
+    if (ssl->session->has_application_settings) {
+      Span<const uint8_t> settings;
+      if (!ssl_get_local_application_settings(hs, &settings,
+                                              ssl->session->early_alpn) ||
+          settings != ssl->session->local_application_settings) {
+        return ssl_early_data_alps_mismatch;
+      }
+    }
+  }
+
+  // Early data has not yet been accepted, but we use it as a success code.
+  return ssl_early_data_accepted;
+}
+
 static enum ssl_hs_wait_t do_start_connect(SSL_HANDSHAKE *hs) {
   SSL *const ssl = hs->ssl;
 
@@ -436,6 +484,13 @@ static enum ssl_hs_wait_t do_start_connect(SSL_HANDSHAKE *hs) {
     }
   }
 
+  ssl_early_data_reason_t reason = should_offer_early_data(hs);
+  if (reason != ssl_early_data_accepted) {
+    ssl->s3->early_data_reason = reason;
+  } else {
+    hs->early_data_offered = true;
+  }
+
   if (!ssl_write_client_hello(hs)) {
     return ssl_hs_error;
   }

ssl/internal.h

@@ -2092,6 +2092,13 @@ bool ssl_is_alpn_protocol_allowed(const SSL_HANDSHAKE *hs,
 bool ssl_negotiate_alpn(SSL_HANDSHAKE *hs, uint8_t *out_alert,
                         const SSL_CLIENT_HELLO *client_hello);
 
+// ssl_get_local_application_settings looks up the configured ALPS value for
+// |protocol|. If found, it sets |*out_settings| to the value and returns true.
+// Otherwise, it returns false.
+bool ssl_get_local_application_settings(const SSL_HANDSHAKE *hs,
+                                        Span<const uint8_t> *out_settings,
+                                        Span<const uint8_t> protocol);
+
 // ssl_negotiate_alps negotiates the ALPS extension, if applicable. It returns
 // true on successful negotiation or if nothing was negotiated. It returns false
 // and sets |*out_alert| to an alert on error.

ssl/t1_lib.cc

@@ -2146,21 +2146,6 @@ static bool ext_psk_key_exchange_modes_parse_clienthello(SSL_HANDSHAKE *hs,
 //
 // https://tools.ietf.org/html/rfc8446#section-4.2.10
 
-// ssl_get_local_application_settings looks up the configured ALPS value for
-// |protocol|. If found, it sets |*out_settings| to the value and returns true.
-// Otherwise, it returns false.
-static bool ssl_get_local_application_settings(
-    const SSL_HANDSHAKE *hs, Span<const uint8_t> *out_settings,
-    Span<const uint8_t> protocol) {
-  for (const ALPSConfig &config : hs->config->alps_configs) {
-    if (protocol == config.protocol) {
-      *out_settings = config.settings;
-      return true;
-    }
-  }
-  return false;
-}
-
 static bool ext_early_data_add_clienthello(SSL_HANDSHAKE *hs, CBB *out) {
   SSL *const ssl = hs->ssl;
   // The second ClientHello never offers early data, and we must have already
@@ -2170,53 +2155,10 @@ static bool ext_early_data_add_clienthello(SSL_HANDSHAKE *hs, CBB *out) {
     return true;
   }
 
-  if (!ssl->enable_early_data) {
+  if (!hs->early_data_offered) {
-    ssl->s3->early_data_reason = ssl_early_data_disabled;
-    return true;
-  }
-
-  if (hs->max_version < TLS1_3_VERSION) {
-    // We discard inapplicable sessions, so this is redundant with the session
-    // checks below, but we check give a more useful reason.
-    ssl->s3->early_data_reason = ssl_early_data_protocol_version;
-    return true;
-  }
-
-  if (ssl->session == nullptr) {
-    ssl->s3->early_data_reason = ssl_early_data_no_session_offered;
-    return true;
-  }
-
-  if (ssl_session_protocol_version(ssl->session.get()) < TLS1_3_VERSION ||
-      ssl->session->ticket_max_early_data == 0) {
-    ssl->s3->early_data_reason = ssl_early_data_unsupported_for_session;
     return true;
   }
 
-  if (!ssl->session->early_alpn.empty()) {
-    if (!ssl_is_alpn_protocol_allowed(hs, ssl->session->early_alpn)) {
-      // Avoid reporting a confusing value in |SSL_get0_alpn_selected|.
-      ssl->s3->early_data_reason = ssl_early_data_alpn_mismatch;
-      return true;
-    }
-
-    // If the previous connection negotiated ALPS, only offer 0-RTT when the
-    // local are settings are consistent with what we'd offer for this
-    // connection.
-    if (ssl->session->has_application_settings) {
-      Span<const uint8_t> settings;
-      if (!ssl_get_local_application_settings(hs, &settings,
-                                              ssl->session->early_alpn) ||
-          settings != ssl->session->local_application_settings) {
-        ssl->s3->early_data_reason = ssl_early_data_alps_mismatch;
-        return true;
-      }
-    }
-  }
-
-  // |early_data_reason| will be filled in later when the server responds.
-  hs->early_data_offered = true;
-
   if (!CBB_add_u16(out, TLSEXT_TYPE_early_data) ||
       !CBB_add_u16(out, 0) ||
       !CBB_flush(out)) {
@@ -2953,6 +2895,18 @@ static bool cert_compression_add_serverhello(SSL_HANDSHAKE *hs, CBB *out) {
 //
 // https://tools.ietf.org/html/draft-vvv-tls-alps-01
 
+bool ssl_get_local_application_settings(const SSL_HANDSHAKE *hs,
+                                        Span<const uint8_t> *out_settings,
+                                        Span<const uint8_t> protocol) {
+  for (const ALPSConfig &config : hs->config->alps_configs) {
+    if (protocol == config.protocol) {
+      *out_settings = config.settings;
+      return true;
+    }
+  }
+  return false;
+}
+
 static bool ext_alps_add_clienthello(SSL_HANDSHAKE *hs, CBB *out) {
   SSL *const ssl = hs->ssl;
   if (// ALPS requires TLS 1.3.