commit fddbadcba9dd220d965fa7a9ff32d8c9e4103349
author David Benjamin <davidben@google.com> Tue Aug 09 19:53:00 2016 -0400
committer CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org> Thu Aug 11 05:11:39 2016 +0000
tree b249e58005ca60c6fe8767af9b4aa9a9afac5b3a
parent 1deb41bb2de8c4c99842b2868772b1931b80d315

Pass a ClientHello into ssl3_choose_cipher.

Now that ssl_bytes_to_cipher_list is uninteresting, it can be an
implementation detail of ssl3_choose_cipher. This removes a tiny amount
of duplicated TLS 1.2 / TLS 1.3 code.

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

ssl/handshake_server.c

diff --git a/ssl/handshake_server.c b/ssl/handshake_server.c
index 43abf39..aad28f9 100644
--- a/ssl/handshake_server.c
+++ b/ssl/handshake_server.c
@@ -553,7 +553,6 @@
 static int ssl3_get_client_hello(SSL *ssl) {
   int al = SSL_AD_INTERNAL_ERROR, ret = -1;
   const SSL_CIPHER *c;
-  STACK_OF(SSL_CIPHER) *ciphers = NULL;
   SSL_SESSION *session = NULL;
 
   if (ssl->state == SSL3_ST_SR_CLNT_HELLO_A) {
@@ -729,32 +728,13 @@
     goto f_err;
   }
 
-  ciphers = ssl_parse_client_cipher_list(&client_hello);
-  if (ciphers == NULL) {
-    goto err;
-  }
-
   /* If it is a hit, check that the cipher is in the list. */
-  if (ssl->session != NULL) {
-    size_t j;
-    int found_cipher = 0;
-    uint32_t id = ssl->session->cipher->id;
-
-    for (j = 0; j < sk_SSL_CIPHER_num(ciphers); j++) {
-      c = sk_SSL_CIPHER_value(ciphers, j);
-      if (c->id == id) {
-        found_cipher = 1;
-        break;
-      }
-    }
-
-    if (!found_cipher) {
-      /* we need to have the cipher in the cipher list if we are asked to reuse
-       * it */
-      al = SSL_AD_ILLEGAL_PARAMETER;
-      OPENSSL_PUT_ERROR(SSL, SSL_R_REQUIRED_CIPHER_MISSING);
-      goto f_err;
-    }
+  if (ssl->session != NULL &&
+      !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;
   }
 
   /* Only null compression is supported. */
@@ -792,13 +772,14 @@
         goto err;
       }
     }
-    c = ssl3_choose_cipher(ssl, ciphers, ssl_get_cipher_preferences(ssl));
 
+    c = ssl3_choose_cipher(ssl, &client_hello, ssl_get_cipher_preferences(ssl));
     if (c == NULL) {
       al = SSL_AD_HANDSHAKE_FAILURE;
       OPENSSL_PUT_ERROR(SSL, SSL_R_NO_SHARED_CIPHER);
       goto f_err;
     }
+
     ssl->s3->new_session->cipher = c;
     ssl->s3->tmp.new_cipher = c;
 
@@ -837,7 +818,6 @@
   }
 
 err:
-  sk_SSL_CIPHER_free(ciphers);
   SSL_SESSION_free(session);
   return ret;
 }

ssl/internal.h

diff --git a/ssl/internal.h b/ssl/internal.h
index 827e5fd..4fcf4b9 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -1306,8 +1306,8 @@
 int ssl3_write_bytes(SSL *ssl, int type, const void *buf, int len);
 int ssl3_output_cert_chain(SSL *ssl);
 const SSL_CIPHER *ssl3_choose_cipher(
-    SSL *ssl, STACK_OF(SSL_CIPHER) *clnt,
-    struct ssl_cipher_preference_list_st *srvr);
+    SSL *ssl, const struct ssl_early_callback_ctx *client_hello,
+    const struct ssl_cipher_preference_list_st *srvr);
 
 int ssl3_new(SSL *ssl);
 void ssl3_free(SSL *ssl);

ssl/s3_lib.c

diff --git a/ssl/s3_lib.c b/ssl/s3_lib.c
index fa29e69..408d5de 100644
--- a/ssl/s3_lib.c
+++ b/ssl/s3_lib.c
@@ -248,8 +248,8 @@
 }
 
 const SSL_CIPHER *ssl3_choose_cipher(
-    SSL *ssl, STACK_OF(SSL_CIPHER) *clnt,
-    struct ssl_cipher_preference_list_st *server_pref) {
+    SSL *ssl, const struct ssl_early_callback_ctx *client_hello,
+    const struct ssl_cipher_preference_list_st *server_pref) {
   const SSL_CIPHER *c, *ret = NULL;
   STACK_OF(SSL_CIPHER) *srvr = server_pref->ciphers, *prio, *allow;
   size_t i;
@@ -265,6 +265,11 @@
    * such value exists yet. */
   int group_min = -1;
 
+  STACK_OF(SSL_CIPHER) *clnt = ssl_parse_client_cipher_list(client_hello);
+  if (clnt == NULL) {
+    return NULL;
+  }
+
   if (ssl->options & SSL_OP_CIPHER_SERVER_PREFERENCE) {
     prio = srvr;
     in_group_flags = server_pref->in_group_flags;
@@ -317,6 +322,7 @@
     }
   }
 
+  sk_SSL_CIPHER_free(clnt);
   return ret;
 }
 

ssl/tls13_server.c

diff --git a/ssl/tls13_server.c b/ssl/tls13_server.c
index 5964233..ec44436 100644
--- a/ssl/tls13_server.c
+++ b/ssl/tls13_server.c
@@ -166,14 +166,8 @@
     }
   }
 
-  STACK_OF(SSL_CIPHER) *ciphers = ssl_parse_client_cipher_list(&client_hello);
-  if (ciphers == NULL) {
-    return ssl_hs_error;
-  }
-
   const SSL_CIPHER *cipher =
-      ssl3_choose_cipher(ssl, ciphers, ssl_get_cipher_preferences(ssl));
-  sk_SSL_CIPHER_free(ciphers);
+      ssl3_choose_cipher(ssl, &client_hello, ssl_get_cipher_preferences(ssl));
   if (cipher == NULL) {
     OPENSSL_PUT_ERROR(SSL, SSL_R_NO_SHARED_CIPHER);
     ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_HANDSHAKE_FAILURE);