From cd0fb43cbe2774220f2702b8289faec590a72d01 Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Tue, 18 Jun 2019 11:45:26 +0100 Subject: Following the previous 2 commits also move ecpointformats out of session The previous 2 commits moved supported groups and ciphers out of the session object to avoid race conditions. We now also move ecpointformats for consistency. There does not seem to be a race condition with access to this data since it is only ever set in a non-resumption handshake. However, there is no reason for it to be in the session. Reviewed-by: Tomas Mraz (Merged from https://github.com/openssl/openssl/pull/9162) --- ssl/s3_lib.c | 7 +++---- ssl/ssl_lib.c | 1 + ssl/ssl_locl.h | 8 ++++---- ssl/ssl_sess.c | 17 ----------------- ssl/statem/extensions.c | 10 +++++----- ssl/statem/extensions_clnt.c | 12 ++++++------ ssl/statem/extensions_srvr.c | 6 +++--- ssl/t1_lib.c | 6 +++--- 8 files changed, 25 insertions(+), 42 deletions(-) (limited to 'ssl') diff --git a/ssl/s3_lib.c b/ssl/s3_lib.c index e51877a6b4..3238fd9b7e 100644 --- a/ssl/s3_lib.c +++ b/ssl/s3_lib.c @@ -3702,13 +3702,12 @@ long ssl3_ctrl(SSL *s, int cmd, long larg, void *parg) #ifndef OPENSSL_NO_EC case SSL_CTRL_GET_EC_POINT_FORMATS: { - SSL_SESSION *sess = s->session; const unsigned char **pformat = parg; - if (sess == NULL || sess->ext.ecpointformats == NULL) + if (s->ext.peer_ecpointformats == NULL) return 0; - *pformat = sess->ext.ecpointformats; - return (int)sess->ext.ecpointformats_len; + *pformat = s->ext.peer_ecpointformats; + return (int)s->ext.peer_ecpointformats_len; } #endif diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c index 220168a148..d15b743f50 100644 --- a/ssl/ssl_lib.c +++ b/ssl/ssl_lib.c @@ -1188,6 +1188,7 @@ void SSL_free(SSL *s) SSL_CTX_free(s->session_ctx); #ifndef OPENSSL_NO_EC OPENSSL_free(s->ext.ecpointformats); + OPENSSL_free(s->ext.peer_ecpointformats); OPENSSL_free(s->ext.supportedgroups); OPENSSL_free(s->ext.peer_supportedgroups); #endif /* OPENSSL_NO_EC */ diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h index 9663c7c3bb..a61987f327 100644 --- a/ssl/ssl_locl.h +++ b/ssl/ssl_locl.h @@ -565,10 +565,6 @@ struct ssl_session_st { struct { char *hostname; -# ifndef OPENSSL_NO_EC - size_t ecpointformats_len; - unsigned char *ecpointformats; /* peer's list */ -# endif /* OPENSSL_NO_EC */ /* RFC4507 info */ unsigned char *tick; /* Session ticket */ size_t ticklen; /* Session ticket length */ @@ -1481,6 +1477,10 @@ struct ssl_st { size_t ecpointformats_len; /* our list */ unsigned char *ecpointformats; + + size_t peer_ecpointformats_len; + /* peer's list */ + unsigned char *peer_ecpointformats; # endif /* OPENSSL_NO_EC */ size_t supportedgroups_len; /* our list */ diff --git a/ssl/ssl_sess.c b/ssl/ssl_sess.c index f13c909da7..f80b8e24a7 100644 --- a/ssl/ssl_sess.c +++ b/ssl/ssl_sess.c @@ -122,9 +122,6 @@ SSL_SESSION *ssl_session_dup(const SSL_SESSION *src, int ticket) dest->psk_identity = NULL; #endif dest->ext.hostname = NULL; -#ifndef OPENSSL_NO_EC - dest->ext.ecpointformats = NULL; -#endif dest->ext.tick = NULL; dest->ext.alpn_selected = NULL; #ifndef OPENSSL_NO_SRP @@ -185,15 +182,6 @@ SSL_SESSION *ssl_session_dup(const SSL_SESSION *src, int ticket) goto err; } } -#ifndef OPENSSL_NO_EC - if (src->ext.ecpointformats) { - dest->ext.ecpointformats = - OPENSSL_memdup(src->ext.ecpointformats, - src->ext.ecpointformats_len); - if (dest->ext.ecpointformats == NULL) - goto err; - } -#endif if (ticket != 0 && src->ext.tick != NULL) { dest->ext.tick = @@ -776,11 +764,6 @@ void SSL_SESSION_free(SSL_SESSION *ss) sk_X509_pop_free(ss->peer_chain, X509_free); OPENSSL_free(ss->ext.hostname); OPENSSL_free(ss->ext.tick); -#ifndef OPENSSL_NO_EC - OPENSSL_free(ss->ext.ecpointformats); - ss->ext.ecpointformats = NULL; - ss->ext.ecpointformats_len = 0; -#endif /* OPENSSL_NO_EC */ #ifndef OPENSSL_NO_PSK OPENSSL_free(ss->psk_identity_hint); OPENSSL_free(ss->psk_identity); diff --git a/ssl/statem/extensions.c b/ssl/statem/extensions.c index eacc7215b5..2a9b796c9f 100644 --- a/ssl/statem/extensions.c +++ b/ssl/statem/extensions.c @@ -1039,18 +1039,18 @@ static int final_ec_pt_formats(SSL *s, unsigned int context, int sent) */ if (s->ext.ecpointformats != NULL && s->ext.ecpointformats_len > 0 - && s->session->ext.ecpointformats != NULL - && s->session->ext.ecpointformats_len > 0 + && s->ext.peer_ecpointformats != NULL + && s->ext.peer_ecpointformats_len > 0 && ((alg_k & SSL_kECDHE) || (alg_a & SSL_aECDSA))) { /* we are using an ECC cipher */ size_t i; - unsigned char *list = s->session->ext.ecpointformats; + unsigned char *list = s->ext.peer_ecpointformats; - for (i = 0; i < s->session->ext.ecpointformats_len; i++) { + for (i = 0; i < s->ext.peer_ecpointformats_len; i++) { if (*list++ == TLSEXT_ECPOINTFORMAT_uncompressed) break; } - if (i == s->session->ext.ecpointformats_len) { + if (i == s->ext.peer_ecpointformats_len) { SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER, SSL_F_FINAL_EC_PT_FORMATS, SSL_R_TLS_INVALID_ECPOINTFORMAT_LIST); return 0; diff --git a/ssl/statem/extensions_clnt.c b/ssl/statem/extensions_clnt.c index b12361f36a..b6e96ae56f 100644 --- a/ssl/statem/extensions_clnt.c +++ b/ssl/statem/extensions_clnt.c @@ -1411,19 +1411,19 @@ int tls_parse_stoc_ec_pt_formats(SSL *s, PACKET *pkt, unsigned int context, return 0; } - s->session->ext.ecpointformats_len = 0; - OPENSSL_free(s->session->ext.ecpointformats); - s->session->ext.ecpointformats = OPENSSL_malloc(ecpointformats_len); - if (s->session->ext.ecpointformats == NULL) { + s->ext.peer_ecpointformats_len = 0; + OPENSSL_free(s->ext.peer_ecpointformats); + s->ext.peer_ecpointformats = OPENSSL_malloc(ecpointformats_len); + if (s->ext.peer_ecpointformats == NULL) { SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_PARSE_STOC_EC_PT_FORMATS, ERR_R_INTERNAL_ERROR); return 0; } - s->session->ext.ecpointformats_len = ecpointformats_len; + s->ext.peer_ecpointformats_len = ecpointformats_len; if (!PACKET_copy_bytes(&ecptformatlist, - s->session->ext.ecpointformats, + s->ext.peer_ecpointformats, ecpointformats_len)) { SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_PARSE_STOC_EC_PT_FORMATS, ERR_R_INTERNAL_ERROR); diff --git a/ssl/statem/extensions_srvr.c b/ssl/statem/extensions_srvr.c index 68fb0863c3..e16722cbeb 100644 --- a/ssl/statem/extensions_srvr.c +++ b/ssl/statem/extensions_srvr.c @@ -254,8 +254,8 @@ int tls_parse_ctos_ec_pt_formats(SSL *s, PACKET *pkt, unsigned int context, if (!s->hit) { if (!PACKET_memdup(&ec_point_format_list, - &s->session->ext.ecpointformats, - &s->session->ext.ecpointformats_len)) { + &s->ext.peer_ecpointformats, + &s->ext.peer_ecpointformats_len)) { SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_PARSE_CTOS_EC_PT_FORMATS, ERR_R_INTERNAL_ERROR); return 0; @@ -1379,7 +1379,7 @@ EXT_RETURN tls_construct_stoc_ec_pt_formats(SSL *s, WPACKET *pkt, unsigned long alg_k = s->s3.tmp.new_cipher->algorithm_mkey; unsigned long alg_a = s->s3.tmp.new_cipher->algorithm_auth; int using_ecc = ((alg_k & SSL_kECDHE) || (alg_a & SSL_aECDSA)) - && (s->session->ext.ecpointformats != NULL); + && (s->ext.peer_ecpointformats != NULL); const unsigned char *plist; size_t plistlen; diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c index dcae274879..95622de0cd 100644 --- a/ssl/t1_lib.c +++ b/ssl/t1_lib.c @@ -597,11 +597,11 @@ static int tls1_check_pkey_comp(SSL *s, EVP_PKEY *pkey) * If point formats extension present check it, otherwise everything is * supported (see RFC4492). */ - if (s->session->ext.ecpointformats == NULL) + if (s->ext.peer_ecpointformats == NULL) return 1; - for (i = 0; i < s->session->ext.ecpointformats_len; i++) { - if (s->session->ext.ecpointformats[i] == comp_id) + for (i = 0; i < s->ext.peer_ecpointformats_len; i++) { + if (s->ext.peer_ecpointformats[i] == comp_id) return 1; } return 0; -- cgit v1.2.3