Call cert_cb before resolving resumption.

This is in preparation for determining the cipher suite (which, in TLS
1.2, requires the certificate be known) before resumption.

Note this has caller-visible effects:

- cert_cb is now called whether resumption occurs or not. Our only
  consumer which uses this as a server is Node which will require a
  patch to fix up their mucking about with SSL_get_session. (But the
  patch should be quite upstreamable. More 1.1.0-compatible and
  generally saner.)

- cert_cb is now called before new_session_cb and dos_protection_cb.

BUG=116

Change-Id: I6cc745757f63281fad714d4548f23880570204b0
Reviewed-on: https://boringssl-review.googlesource.com/11846
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/handshake_server.c

@@ -748,84 +748,6 @@ static int ssl3_get_client_hello(SSL *ssl) {
   }
 
   if (ssl->state == SSL3_ST_SR_CLNT_HELLO_D) {
-    /* Determine whether we are doing session resumption. */
-    int tickets_supported = 0, renew_ticket = 0;
-    switch (ssl_get_prev_session(ssl, &session, &tickets_supported,
-                                 &renew_ticket, &client_hello)) {
-      case ssl_session_success:
-        break;
-      case ssl_session_error:
-        goto err;
-      case ssl_session_retry:
-        ssl->rwstate = SSL_PENDING_SESSION;
-        goto err;
-    }
-
-    if (session != NULL) {
-      if (session->extended_master_secret &&
-          !ssl->s3->tmp.extended_master_secret) {
-        /* A ClientHello without EMS that attempts to resume a session with EMS
-         * is fatal to the connection. */
-        al = SSL_AD_HANDSHAKE_FAILURE;
-        OPENSSL_PUT_ERROR(SSL, SSL_R_RESUMED_EMS_SESSION_WITHOUT_EMS_EXTENSION);
-        goto f_err;
-      }
-
-      if (!ssl_session_is_resumable(ssl, session) ||
-          /* If the client offers the EMS extension, but the previous session
-           * didn't use it, then negotiate a new session. */
-          ssl->s3->tmp.extended_master_secret !=
-              session->extended_master_secret) {
-        SSL_SESSION_free(session);
-        session = NULL;
-      }
-    }
-
-    if (session != NULL) {
-      /* Use the old session. */
-      ssl->s3->hs->ticket_expected = renew_ticket;
-      ssl->session = session;
-      session = NULL;
-      ssl->s3->session_reused = 1;
-    } else {
-      ssl->s3->hs->ticket_expected = tickets_supported;
-      ssl_set_session(ssl, NULL);
-      if (!ssl_get_new_session(ssl, 1 /* server */)) {
-        goto err;
-      }
-
-      /* Clear the session ID if we want the session to be single-use. */
-      if (!(ssl->ctx->session_cache_mode & SSL_SESS_CACHE_SERVER)) {
-        ssl->s3->new_session->session_id_length = 0;
-      }
-    }
-
-    if (ssl->ctx->dos_protection_cb != NULL &&
-        ssl->ctx->dos_protection_cb(&client_hello) == 0) {
-      /* Connection rejected for DOS reasons. */
-      al = SSL_AD_INTERNAL_ERROR;
-      OPENSSL_PUT_ERROR(SSL, SSL_R_CONNECTION_REJECTED);
-      goto f_err;
-    }
-
-    ssl->state = SSL3_ST_SR_CLNT_HELLO_E;
-  }
-
-  /* Determine the remaining connection parameters. This is a separate state so
-   * |cert_cb| does not cause earlier logic to run multiple times. */
-  assert(ssl->state == SSL3_ST_SR_CLNT_HELLO_E);
-
-  if (ssl->session != NULL) {
-    /* Check that the cipher is in the list. */
-    if (!ssl_client_cipher_list_contains_cipher(
-            &client_hello, (uint16_t)ssl->session->cipher->id)) {
-      al = SSL_AD_ILLEGAL_PARAMETER;
-      OPENSSL_PUT_ERROR(SSL, SSL_R_REQUIRED_CIPHER_MISSING);
-      goto f_err;
-    }
-
-    ssl->s3->tmp.new_cipher = ssl->session->cipher;
-  } else {
     /* Call |cert_cb| to update server certificates if required. */
     if (ssl->cert->cert_cb != NULL) {
       int rv = ssl->cert->cert_cb(ssl, ssl->cert->cert_cb_arg);
@@ -840,6 +762,82 @@ static int ssl3_get_client_hello(SSL *ssl) {
       }
     }
 
+    ssl->state = SSL3_ST_SR_CLNT_HELLO_E;
+  }
+
+  assert(ssl->state == SSL3_ST_SR_CLNT_HELLO_E);
+
+  /* Determine whether we are doing session resumption. */
+  int tickets_supported = 0, renew_ticket = 0;
+  switch (ssl_get_prev_session(ssl, &session, &tickets_supported, &renew_ticket,
+                               &client_hello)) {
+    case ssl_session_success:
+      break;
+    case ssl_session_error:
+      goto err;
+    case ssl_session_retry:
+      ssl->rwstate = SSL_PENDING_SESSION;
+      goto err;
+  }
+
+  if (session != NULL) {
+    if (session->extended_master_secret &&
+        !ssl->s3->tmp.extended_master_secret) {
+      /* A ClientHello without EMS that attempts to resume a session with EMS
+       * is fatal to the connection. */
+      al = SSL_AD_HANDSHAKE_FAILURE;
+      OPENSSL_PUT_ERROR(SSL, SSL_R_RESUMED_EMS_SESSION_WITHOUT_EMS_EXTENSION);
+      goto f_err;
+    }
+
+    if (!ssl_session_is_resumable(ssl, session) ||
+        /* If the client offers the EMS extension, but the previous session
+         * didn't use it, then negotiate a new session. */
+        ssl->s3->tmp.extended_master_secret !=
+            session->extended_master_secret) {
+      SSL_SESSION_free(session);
+      session = NULL;
+    }
+  }
+
+  if (session != NULL) {
+    /* Use the old session. */
+    ssl->s3->hs->ticket_expected = renew_ticket;
+    ssl->session = session;
+    session = NULL;
+    ssl->s3->session_reused = 1;
+  } else {
+    ssl->s3->hs->ticket_expected = tickets_supported;
+    ssl_set_session(ssl, NULL);
+    if (!ssl_get_new_session(ssl, 1 /* server */)) {
+      goto err;
+    }
+
+    /* Clear the session ID if we want the session to be single-use. */
+    if (!(ssl->ctx->session_cache_mode & SSL_SESS_CACHE_SERVER)) {
+      ssl->s3->new_session->session_id_length = 0;
+    }
+  }
+
+  if (ssl->ctx->dos_protection_cb != NULL &&
+      ssl->ctx->dos_protection_cb(&client_hello) == 0) {
+    /* Connection rejected for DOS reasons. */
+    al = SSL_AD_INTERNAL_ERROR;
+    OPENSSL_PUT_ERROR(SSL, SSL_R_CONNECTION_REJECTED);
+    goto f_err;
+  }
+
+  if (ssl->session != NULL) {
+    /* Check that the cipher is in the list. */
+    if (!ssl_client_cipher_list_contains_cipher(
+            &client_hello, (uint16_t)ssl->session->cipher->id)) {
+      al = SSL_AD_ILLEGAL_PARAMETER;
+      OPENSSL_PUT_ERROR(SSL, SSL_R_REQUIRED_CIPHER_MISSING);
+      goto f_err;
+    }
+
+    ssl->s3->tmp.new_cipher = ssl->session->cipher;
+  } else {
     const SSL_CIPHER *c =
         ssl3_choose_cipher(ssl, &client_hello, ssl_get_cipher_preferences(ssl));
     if (c == NULL) {

ssl/tls13_server.c

@@ -123,6 +123,75 @@ static enum ssl_hs_wait_t do_process_client_hello(SSL *ssl, SSL_HANDSHAKE *hs) {
     return ssl_hs_error;
   }
 
+  hs->state = state_select_parameters;
+  return ssl_hs_ok;
+}
+
+static const SSL_CIPHER *choose_tls13_cipher(
+    const SSL *ssl, const struct ssl_early_callback_ctx *client_hello) {
+  if (client_hello->cipher_suites_len % 2 != 0) {
+    return NULL;
+  }
+
+  CBS cipher_suites;
+  CBS_init(&cipher_suites, client_hello->cipher_suites,
+           client_hello->cipher_suites_len);
+
+  const int aes_is_fine = EVP_has_aes_hardware();
+
+  const SSL_CIPHER *best = NULL;
+  while (CBS_len(&cipher_suites) > 0) {
+    uint16_t cipher_suite;
+    if (!CBS_get_u16(&cipher_suites, &cipher_suite)) {
+      return NULL;
+    }
+
+    const SSL_CIPHER *candidate = SSL_get_cipher_by_value(cipher_suite);
+    if (candidate == NULL || !ssl_is_valid_cipher(ssl, candidate)) {
+      continue;
+    }
+
+    /* TLS 1.3 removes legacy ciphers, so honor the client order, but prefer
+     * ChaCha20 if we do not have AES hardware. */
+    if (aes_is_fine) {
+      return candidate;
+    }
+
+    if (candidate->algorithm_enc == SSL_CHACHA20POLY1305) {
+      return candidate;
+    }
+
+    if (best == NULL) {
+      best = candidate;
+    }
+  }
+
+  return best;
+}
+
+static enum ssl_hs_wait_t do_select_parameters(SSL *ssl, SSL_HANDSHAKE *hs) {
+  /* Call |cert_cb| to update server certificates if required. */
+  if (ssl->cert->cert_cb != NULL) {
+    int rv = ssl->cert->cert_cb(ssl, ssl->cert->cert_cb_arg);
+    if (rv == 0) {
+      OPENSSL_PUT_ERROR(SSL, SSL_R_CERT_CB_ERROR);
+      ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR);
+      return ssl_hs_error;
+    }
+    if (rv < 0) {
+      hs->state = state_select_parameters;
+      return ssl_hs_x509_lookup;
+    }
+  }
+
+  struct ssl_early_callback_ctx client_hello;
+  if (!ssl_early_callback_init(ssl, &client_hello, ssl->init_msg,
+                               ssl->init_num)) {
+    OPENSSL_PUT_ERROR(SSL, SSL_R_CLIENTHELLO_PARSE_FAILED);
+    ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
+    return ssl_hs_error;
+  }
+
   uint8_t alert = SSL_AD_DECODE_ERROR;
   SSL_SESSION *session = NULL;
   CBS pre_shared_key, binders;
@@ -182,77 +251,6 @@ static enum ssl_hs_wait_t do_process_client_hello(SSL *ssl, SSL_HANDSHAKE *hs) {
     return ssl_hs_error;
   }
 
-  hs->state = state_select_parameters;
-  return ssl_hs_ok;
-}
-
-static const SSL_CIPHER *choose_tls13_cipher(
-    const SSL *ssl, const struct ssl_early_callback_ctx *client_hello) {
-  if (client_hello->cipher_suites_len % 2 != 0) {
-    return NULL;
-  }
-
-  CBS cipher_suites;
-  CBS_init(&cipher_suites, client_hello->cipher_suites,
-           client_hello->cipher_suites_len);
-
-  const int aes_is_fine = EVP_has_aes_hardware();
-
-  const SSL_CIPHER *best = NULL;
-  while (CBS_len(&cipher_suites) > 0) {
-    uint16_t cipher_suite;
-    if (!CBS_get_u16(&cipher_suites, &cipher_suite)) {
-      return NULL;
-    }
-
-    const SSL_CIPHER *candidate = SSL_get_cipher_by_value(cipher_suite);
-    if (candidate == NULL || !ssl_is_valid_cipher(ssl, candidate)) {
-      continue;
-    }
-
-    /* TLS 1.3 removes legacy ciphers, so honor the client order, but prefer
-     * ChaCha20 if we do not have AES hardware. */
-    if (aes_is_fine) {
-      return candidate;
-    }
-
-    if (candidate->algorithm_enc == SSL_CHACHA20POLY1305) {
-      return candidate;
-    }
-
-    if (best == NULL) {
-      best = candidate;
-    }
-  }
-
-  return best;
-}
-
-static enum ssl_hs_wait_t do_select_parameters(SSL *ssl, SSL_HANDSHAKE *hs) {
-  if (!ssl->s3->session_reused) {
-    /* Call |cert_cb| to update server certificates if required. */
-    if (ssl->cert->cert_cb != NULL) {
-      int rv = ssl->cert->cert_cb(ssl, ssl->cert->cert_cb_arg);
-      if (rv == 0) {
-        OPENSSL_PUT_ERROR(SSL, SSL_R_CERT_CB_ERROR);
-        ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR);
-        return ssl_hs_error;
-      }
-      if (rv < 0) {
-        hs->state = state_select_parameters;
-        return ssl_hs_x509_lookup;
-      }
-    }
-  }
-
-  struct ssl_early_callback_ctx client_hello;
-  if (!ssl_early_callback_init(ssl, &client_hello, ssl->init_msg,
-                               ssl->init_num)) {
-    OPENSSL_PUT_ERROR(SSL, SSL_R_CLIENTHELLO_PARSE_FAILED);
-    ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
-    return ssl_hs_error;
-  }
-
   if (ssl->s3->session_reused) {
     /* Clients may not offer sessions containing unsupported ciphers. */
     if (!ssl_client_cipher_list_contains_cipher(
@@ -287,7 +285,6 @@ static enum ssl_hs_wait_t do_select_parameters(SSL *ssl, SSL_HANDSHAKE *hs) {
 
   /* Resolve ALPN after the cipher suite is selected. HTTP/2 negotiation depends
    * on the cipher suite. */
-  uint8_t alert;
   if (!ssl_negotiate_alpn(ssl, &alert, &client_hello)) {
     ssl3_send_alert(ssl, SSL3_AL_FATAL, alert);
     return ssl_hs_error;