From fddbadcba9dd220d965fa7a9ff32d8c9e4103349 Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Tue, 9 Aug 2016 19:53:00 -0400 Subject: [PATCH] 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 Commit-Queue: David Benjamin CQ-Verified: CQ bot account: commit-bot@chromium.org --- ssl/handshake_server.c | 36 ++++++++---------------------------- ssl/internal.h | 4 ++-- ssl/s3_lib.c | 10 ++++++++-- ssl/tls13_server.c | 8 +------- 4 files changed, 19 insertions(+), 39 deletions(-) diff --git a/ssl/handshake_server.c b/ssl/handshake_server.c index 43abf39f..aad28f90 100644 --- a/ssl/handshake_server.c +++ b/ssl/handshake_server.c @@ -553,7 +553,6 @@ int ssl_client_cipher_list_contains_cipher( 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 @@ static int ssl3_get_client_hello(SSL *ssl) { 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 @@ static int ssl3_get_client_hello(SSL *ssl) { 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 @@ static int ssl3_get_client_hello(SSL *ssl) { } err: - sk_SSL_CIPHER_free(ciphers); SSL_SESSION_free(session); return ret; } diff --git a/ssl/internal.h b/ssl/internal.h index 827e5fdc..4fcf4b96 100644 --- a/ssl/internal.h +++ b/ssl/internal.h @@ -1306,8 +1306,8 @@ int ssl3_write_app_data(SSL *ssl, const void *buf, int len); 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); diff --git a/ssl/s3_lib.c b/ssl/s3_lib.c index fa29e691..408d5de1 100644 --- a/ssl/s3_lib.c +++ b/ssl/s3_lib.c @@ -248,8 +248,8 @@ struct ssl_cipher_preference_list_st *ssl_get_cipher_preferences(SSL *ssl) { } 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 @@ const SSL_CIPHER *ssl3_choose_cipher( * 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 @@ const SSL_CIPHER *ssl3_choose_cipher( } } + sk_SSL_CIPHER_free(clnt); return ret; } diff --git a/ssl/tls13_server.c b/ssl/tls13_server.c index 5964233b..ec444364 100644 --- a/ssl/tls13_server.c +++ b/ssl/tls13_server.c @@ -166,14 +166,8 @@ static enum ssl_hs_wait_t do_process_client_hello(SSL *ssl, SSL_HANDSHAKE *hs) { } } - 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);