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>
diff --git a/ssl/handshake_client.cc b/ssl/handshake_client.cc
index fbf0ef5..07da255 100644
--- a/ssl/handshake_client.cc
+++ b/ssl/handshake_client.cc
@@ -374,6 +374,54 @@
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 @@
}
}
+ 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;
}
diff --git a/ssl/internal.h b/ssl/internal.h
index cb79494..c5fb263 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -2092,6 +2092,13 @@
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.
diff --git a/ssl/t1_lib.cc b/ssl/t1_lib.cc
index 7155f12..0eec28a 100644
--- a/ssl/t1_lib.cc
+++ b/ssl/t1_lib.cc
@@ -2146,21 +2146,6 @@
//
// 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 @@
return true;
}
- if (!ssl->enable_early_data) {
- ssl->s3->early_data_reason = ssl_early_data_disabled;
+ if (!hs->early_data_offered) {
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 @@
//
// 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.
