diff options
author | Matt Caswell <matt@openssl.org> | 2019-06-14 14:06:55 +0100 |
---|---|---|
committer | Matt Caswell <matt@openssl.org> | 2019-06-18 13:36:25 +0100 |
commit | eee2a6a718151336534d15a61d8d11209d4dfb1e (patch) | |
tree | 0ec41e4336d7400c672d6663e751be8522bbb828 /ssl/statem | |
parent | 45436e611b3e11c948ea9f3273df971c9bb4c122 (diff) |
Fix a race condition in ciphers handling
Similarly to the previous commit we were storing the peer offered list
of ciphers in the session. In practice there is no need for this
information to be avilable from one resumption to the next since this
list is specific to a particular handshake. Since the session object is
supposed to be immutable we should not be updating it once we have decided
to resume. The solution is to remove the session list out of the session
object.
Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
(Merged from https://github.com/openssl/openssl/pull/9162)
Diffstat (limited to 'ssl/statem')
-rw-r--r-- | ssl/statem/statem_srvr.c | 16 |
1 files changed, 8 insertions, 8 deletions
diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c index 6504f4f74e..79c2aa0ede 100644 --- a/ssl/statem/statem_srvr.c +++ b/ssl/statem/statem_srvr.c @@ -1924,14 +1924,14 @@ static int tls_early_post_process_client_hello(SSL *s) && master_key_length > 0) { s->session->master_key_length = master_key_length; s->hit = 1; - s->session->ciphers = ciphers; + s->peer_ciphers = ciphers; s->session->verify_result = X509_V_OK; ciphers = NULL; /* check if some cipher was preferred by call back */ if (pref_cipher == NULL) - pref_cipher = ssl3_choose_cipher(s, s->session->ciphers, + pref_cipher = ssl3_choose_cipher(s, s->peer_ciphers, SSL_get_ciphers(s)); if (pref_cipher == NULL) { SSLfatal(s, SSL_AD_HANDSHAKE_FAILURE, @@ -1942,9 +1942,9 @@ static int tls_early_post_process_client_hello(SSL *s) s->session->cipher = pref_cipher; sk_SSL_CIPHER_free(s->cipher_list); - s->cipher_list = sk_SSL_CIPHER_dup(s->session->ciphers); + s->cipher_list = sk_SSL_CIPHER_dup(s->peer_ciphers); sk_SSL_CIPHER_free(s->cipher_list_by_id); - s->cipher_list_by_id = sk_SSL_CIPHER_dup(s->session->ciphers); + s->cipher_list_by_id = sk_SSL_CIPHER_dup(s->peer_ciphers); } } @@ -2044,12 +2044,12 @@ static int tls_early_post_process_client_hello(SSL *s) #endif /* - * Given s->session->ciphers and SSL_get_ciphers, we must pick a cipher + * Given s->peer_ciphers and SSL_get_ciphers, we must pick a cipher */ if (!s->hit || SSL_IS_TLS13(s)) { - sk_SSL_CIPHER_free(s->session->ciphers); - s->session->ciphers = ciphers; + sk_SSL_CIPHER_free(s->peer_ciphers); + s->peer_ciphers = ciphers; if (ciphers == NULL) { SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_EARLY_POST_PROCESS_CLIENT_HELLO, @@ -2256,7 +2256,7 @@ WORK_STATE tls_post_process_client_hello(SSL *s, WORK_STATE wst) /* In TLSv1.3 we selected the ciphersuite before resumption */ if (!SSL_IS_TLS13(s)) { cipher = - ssl3_choose_cipher(s, s->session->ciphers, SSL_get_ciphers(s)); + ssl3_choose_cipher(s, s->peer_ciphers, SSL_get_ciphers(s)); if (cipher == NULL) { SSLfatal(s, SSL_AD_HANDSHAKE_FAILURE, |