commit ef0183c537536b19c2d7352a68cd3c2bbe5572c4
author David Benjamin <davidben@google.com> Sat Jul 20 09:11:05 2019 -0400
committer CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org> Sat Jul 20 14:13:05 2019 +0000
tree b8cbc603a8c628ddbe902f7237aa3f375aa6bf9c
parent 4dfd5af70191b068aebe567b8e29ce108cee85ce

Make SSL_get_servername work in the early callback.

This avoids early callback users writing their own SNI parser and gives us a
place to surface the server name from ESNI in the future.

Update-Note: This isn't a breaking change, but users of
SSL_CTX_set_select_certificate_cb can likely drop a bit of code after this CL.

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

ssl/handshake_server.cc

diff --git a/ssl/handshake_server.cc b/ssl/handshake_server.cc
index 8ee39a2..36aa560 100644
--- a/ssl/handshake_server.cc
+++ b/ssl/handshake_server.cc
@@ -503,6 +503,54 @@
   return true;
 }
 
+static bool extract_sni(SSL_HANDSHAKE *hs, uint8_t *out_alert,
+                        const SSL_CLIENT_HELLO *client_hello) {
+  SSL *const ssl = hs->ssl;
+  CBS sni;
+  if (!ssl_client_hello_get_extension(client_hello, &sni,
+                                      TLSEXT_TYPE_server_name)) {
+    // No SNI extension to parse.
+    return true;
+  }
+
+  CBS server_name_list, host_name;
+  uint8_t name_type;
+  if (!CBS_get_u16_length_prefixed(&sni, &server_name_list) ||
+      !CBS_get_u8(&server_name_list, &name_type) ||
+      // Although the server_name extension was intended to be extensible to
+      // new name types and multiple names, OpenSSL 1.0.x had a bug which meant
+      // different name types will cause an error. Further, RFC 4366 originally
+      // defined syntax inextensibly. RFC 6066 corrected this mistake, but
+      // adding new name types is no longer feasible.
+      //
+      // Act as if the extensibility does not exist to simplify parsing.
+      !CBS_get_u16_length_prefixed(&server_name_list, &host_name) ||
+      CBS_len(&server_name_list) != 0 ||
+      CBS_len(&sni) != 0) {
+    *out_alert = SSL_AD_DECODE_ERROR;
+    return false;
+  }
+
+  if (name_type != TLSEXT_NAMETYPE_host_name ||
+      CBS_len(&host_name) == 0 ||
+      CBS_len(&host_name) > TLSEXT_MAXLEN_host_name ||
+      CBS_contains_zero_byte(&host_name)) {
+    *out_alert = SSL_AD_UNRECOGNIZED_NAME;
+    return false;
+  }
+
+  // Copy the hostname as a string.
+  char *raw = nullptr;
+  if (!CBS_strdup(&host_name, &raw)) {
+    *out_alert = SSL_AD_INTERNAL_ERROR;
+    return false;
+  }
+  ssl->s3->hostname.reset(raw);
+
+  hs->should_ack_sni = true;
+  return true;
+}
+
 static enum ssl_hs_wait_t do_read_client_hello(SSL_HANDSHAKE *hs) {
   SSL *const ssl = hs->ssl;
 
@@ -526,6 +574,12 @@
     return ssl_hs_handoff;
   }
 
+  uint8_t alert = SSL_AD_DECODE_ERROR;
+  if (!extract_sni(hs, &alert, &client_hello)) {
+    ssl_send_alert(ssl, SSL3_AL_FATAL, alert);
+    return ssl_hs_error;
+  }
+
   // Run the early callback.
   if (ssl->ctx->select_certificate_cb != NULL) {
     switch (ssl->ctx->select_certificate_cb(&client_hello)) {
@@ -553,7 +607,6 @@
     hs->apply_jdk11_workaround = true;
   }
 
-  uint8_t alert = SSL_AD_DECODE_ERROR;
   if (!negotiate_version(hs, &alert, &client_hello)) {
     ssl_send_alert(ssl, SSL3_AL_FATAL, alert);
     return ssl_hs_error;