commit 4eb95ccfd64d46fad23f36238690594d02518467
author David Benjamin <davidben@google.com> Wed Nov 16 17:08:23 2016 +0900
committer CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org> Wed Nov 16 23:58:02 2016 +0000
tree 8ddc72907ba1a5c16a5227a3a417749b8c2e948d
parent 8d3f130c557fd69ccb241ff781583dedad085542

Parse ClientHello extensions before deciding on resumption.

This simplifies a little code around EMS and PSK KE modes, but requires
tweaking the SNI code.

The extensions that are more tightly integrated with the handshake are
still processed inline for now. It does, however, require an extra state
in 1.2 so the asynchronous session callback does not cause extensions to
be processed twice. Tweak a test enforce this.

This and a follow-up to move cert_cb before resumption are done in
preparation for resolving the cipher suite before resumption and only
resuming on match.

Note this has caller-visible effects:

- The legacy SNI callback happens before resumption.

- The ALPN callback happens before resumption.

- Custom extension ClientHello parsing callbacks also cannot depend on
  resumption state.

- The DoS protection callback now runs after all the extension callbacks
  as it is documented to be called after the resumption decision.

BUG=116

Change-Id: I1281a3b61789b95c370314aaed4f04c1babbc65f
Reviewed-on: https://boringssl-review.googlesource.com/11845
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>

ssl/t1_lib.c

diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index 08c5db0..bf40284 100644
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -705,28 +705,19 @@
     return 0;
   }
 
-  /* TODO(davidben): SNI should be resolved before resumption. We have the
-   * early callback as a replacement, but we should fix the current callback
-   * and avoid the need for |SSL_CTX_set_session_id_context|. */
-  if (ssl->session == NULL) {
-    assert(ssl->s3->new_session->tlsext_hostname == NULL);
-
-    /* Copy the hostname as a string. */
-    if (!CBS_strdup(&host_name, &ssl->s3->new_session->tlsext_hostname)) {
-      *out_alert = SSL_AD_INTERNAL_ERROR;
-      return 0;
-    }
-
-    ssl->s3->hs->should_ack_sni = 1;
+  /* Copy the hostname as a string. */
+  if (!CBS_strdup(&host_name, &ssl->s3->hs->hostname)) {
+    *out_alert = SSL_AD_INTERNAL_ERROR;
+    return 0;
   }
 
+  ssl->s3->hs->should_ack_sni = 1;
   return 1;
 }
 
 static int ext_sni_add_serverhello(SSL *ssl, CBB *out) {
-  if (ssl->session != NULL ||
-      !ssl->s3->hs->should_ack_sni ||
-      ssl->s3->new_session->tlsext_hostname == NULL) {
+  if (ssl->s3->session_reused ||
+      !ssl->s3->hs->should_ack_sni) {
     return 1;
   }
 
@@ -2063,9 +2054,13 @@
   return CBB_flush(out);
 }
 
-int ssl_ext_psk_key_exchange_modes_parse_clienthello(SSL *ssl,
-                                                     uint8_t *out_alert,
-                                                     CBS *contents) {
+static int ext_psk_key_exchange_modes_parse_clienthello(SSL *ssl,
+                                                        uint8_t *out_alert,
+                                                        CBS *contents) {
+  if (contents == NULL) {
+    return 1;
+  }
+
   CBS ke_modes;
   if (!CBS_get_u8_length_prefixed(contents, &ke_modes) ||
       CBS_len(&ke_modes) == 0 ||
@@ -2561,7 +2556,7 @@
     NULL,
     ext_psk_key_exchange_modes_add_clienthello,
     forbid_parse_serverhello,
-    ignore_parse_clienthello,
+    ext_psk_key_exchange_modes_parse_clienthello,
     dont_add_serverhello,
   },
   {