[openssl] master update
Matt Caswell
matt at openssl.org
Tue Jun 18 12:49:34 UTC 2019
The branch master has been updated
via cd0fb43cbe2774220f2702b8289faec590a72d01 (commit)
via eee2a6a718151336534d15a61d8d11209d4dfb1e (commit)
via 45436e611b3e11c948ea9f3273df971c9bb4c122 (commit)
from e7a4682d0b347f0dfba629f4601a28801e54ad67 (commit)
- Log -----------------------------------------------------------------
commit cd0fb43cbe2774220f2702b8289faec590a72d01
Author: Matt Caswell <matt at openssl.org>
Date: Tue Jun 18 11:45:26 2019 +0100
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 <tmraz at fedoraproject.org>
(Merged from https://github.com/openssl/openssl/pull/9162)
commit eee2a6a718151336534d15a61d8d11209d4dfb1e
Author: Matt Caswell <matt at openssl.org>
Date: Fri Jun 14 14:06:55 2019 +0100
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 at fedoraproject.org>
(Merged from https://github.com/openssl/openssl/pull/9162)
commit 45436e611b3e11c948ea9f3273df971c9bb4c122
Author: Matt Caswell <matt at openssl.org>
Date: Fri Jun 14 12:46:13 2019 +0100
Fix a race condition in supported groups handling
In TLSv1.3 the supported groups can be negotiated each time a handshake
occurs, regardless of whether we are resuming or not. We should not store
the supported groups information in the session because session objects
can be shared between multiple threads and we can end up with race
conditions. For most users this won't be seen because, by default, we
use stateless tickets in TLSv1.3 which don't get shared. However if you
use SSL_OP_NO_TICKET (to get stateful tickets in TLSv1.3) then this can
happen.
The answer is to move the supported the supported group information into
the SSL object instead.
Reviewed-by: Tomas Mraz <tmraz at fedoraproject.org>
(Merged from https://github.com/openssl/openssl/pull/9162)
-----------------------------------------------------------------------
Summary of changes:
ssl/s3_lib.c | 11 +++++------
ssl/ssl_lib.c | 12 +++++++-----
ssl/ssl_locl.h | 23 +++++++++++++----------
ssl/ssl_sess.c | 37 -------------------------------------
ssl/statem/extensions.c | 10 +++++-----
ssl/statem/extensions_clnt.c | 12 ++++++------
ssl/statem/extensions_srvr.c | 16 ++++++++--------
ssl/statem/statem_srvr.c | 16 ++++++++--------
ssl/t1_lib.c | 6 +++---
9 files changed, 55 insertions(+), 88 deletions(-)
diff --git a/ssl/s3_lib.c b/ssl/s3_lib.c
index 6c3f04d..3238fd9 100644
--- a/ssl/s3_lib.c
+++ b/ssl/s3_lib.c
@@ -3586,8 +3586,8 @@ long ssl3_ctrl(SSL *s, int cmd, long larg, void *parg)
if (!s->session)
return 0;
- clist = s->session->ext.supportedgroups;
- clistlen = s->session->ext.supportedgroups_len;
+ clist = s->ext.peer_supportedgroups;
+ clistlen = s->ext.peer_supportedgroups_len;
if (parg) {
size_t i;
int *cptr = parg;
@@ -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 4e4477d..d15b743 100644
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -1169,6 +1169,7 @@ void SSL_free(SSL *s)
sk_SSL_CIPHER_free(s->cipher_list);
sk_SSL_CIPHER_free(s->cipher_list_by_id);
sk_SSL_CIPHER_free(s->tls13_ciphersuites);
+ sk_SSL_CIPHER_free(s->peer_ciphers);
/* Make the next call work :-) */
if (s->session != NULL) {
@@ -1187,7 +1188,9 @@ 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 */
sk_X509_EXTENSION_pop_free(s->ext.ocsp.exts, X509_EXTENSION_free);
#ifndef OPENSSL_NO_OCSP
@@ -2570,9 +2573,9 @@ STACK_OF(SSL_CIPHER) *SSL_get_ciphers(const SSL *s)
STACK_OF(SSL_CIPHER) *SSL_get_client_ciphers(const SSL *s)
{
- if ((s == NULL) || (s->session == NULL) || !s->server)
+ if ((s == NULL) || !s->server)
return NULL;
- return s->session->ciphers;
+ return s->peer_ciphers;
}
STACK_OF(SSL_CIPHER) *SSL_get1_supported_ciphers(SSL *s)
@@ -2711,13 +2714,12 @@ char *SSL_get_shared_ciphers(const SSL *s, char *buf, int size)
int i;
if (!s->server
- || s->session == NULL
- || s->session->ciphers == NULL
+ || s->peer_ciphers == NULL
|| size < 2)
return NULL;
p = buf;
- clntsk = s->session->ciphers;
+ clntsk = s->peer_ciphers;
srvrsk = SSL_get_ciphers(s);
if (clntsk == NULL || srvrsk == NULL)
return NULL;
diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h
index e521152..a61987f 100644
--- a/ssl/ssl_locl.h
+++ b/ssl/ssl_locl.h
@@ -556,7 +556,6 @@ struct ssl_session_st {
const SSL_CIPHER *cipher;
unsigned long cipher_id; /* when ASN.1 loaded, this needs to be used to
* load the 'cipher' structure */
- STACK_OF(SSL_CIPHER) *ciphers; /* ciphers offered by the client */
CRYPTO_EX_DATA ex_data; /* application specific data */
/*
* These are used to make removal of session-ids more efficient and to
@@ -566,13 +565,7 @@ 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 */
- size_t supportedgroups_len;
- uint16_t *supportedgroups; /* peer's list */
- /* RFC4507 info */
+ /* RFC4507 info */
unsigned char *tick; /* Session ticket */
size_t ticklen; /* Session ticket length */
/* Session lifetime hint in seconds */
@@ -1320,6 +1313,7 @@ struct ssl_st {
/* Per connection DANE state */
SSL_DANE dane;
/* crypto */
+ STACK_OF(SSL_CIPHER) *peer_ciphers;
STACK_OF(SSL_CIPHER) *cipher_list;
STACK_OF(SSL_CIPHER) *cipher_list_by_id;
/* TLSv1.3 specific ciphersuites */
@@ -1483,10 +1477,19 @@ 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 */
uint16_t *supportedgroups;
+
+ size_t peer_supportedgroups_len;
+ /* peer's list */
+ uint16_t *peer_supportedgroups;
+
/* TLS Session Ticket extension override */
TLS_SESSION_TICKET_EXT *session_ticket;
/* TLS Session Ticket extension callback */
@@ -2258,8 +2261,8 @@ static ossl_inline int ssl_has_cert(const SSL *s, int idx)
static ossl_inline void tls1_get_peer_groups(SSL *s, const uint16_t **pgroups,
size_t *pgroupslen)
{
- *pgroups = s->session->ext.supportedgroups;
- *pgroupslen = s->session->ext.supportedgroups_len;
+ *pgroups = s->ext.peer_supportedgroups;
+ *pgroupslen = s->ext.peer_supportedgroups_len;
}
# ifndef OPENSSL_UNIT_TEST
diff --git a/ssl/ssl_sess.c b/ssl/ssl_sess.c
index 508182a..f80b8e2 100644
--- a/ssl/ssl_sess.c
+++ b/ssl/ssl_sess.c
@@ -121,12 +121,7 @@ SSL_SESSION *ssl_session_dup(const SSL_SESSION *src, int ticket)
dest->psk_identity_hint = NULL;
dest->psk_identity = NULL;
#endif
- dest->ciphers = NULL;
dest->ext.hostname = NULL;
-#ifndef OPENSSL_NO_EC
- dest->ext.ecpointformats = NULL;
- dest->ext.supportedgroups = NULL;
-#endif
dest->ext.tick = NULL;
dest->ext.alpn_selected = NULL;
#ifndef OPENSSL_NO_SRP
@@ -176,12 +171,6 @@ SSL_SESSION *ssl_session_dup(const SSL_SESSION *src, int ticket)
}
#endif
- if (src->ciphers != NULL) {
- dest->ciphers = sk_SSL_CIPHER_dup(src->ciphers);
- if (dest->ciphers == NULL)
- goto err;
- }
-
if (!CRYPTO_dup_ex_data(CRYPTO_EX_INDEX_SSL_SESSION,
&dest->ex_data, &src->ex_data)) {
goto err;
@@ -193,23 +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;
- }
- if (src->ext.supportedgroups) {
- dest->ext.supportedgroups =
- OPENSSL_memdup(src->ext.supportedgroups,
- src->ext.supportedgroups_len
- * sizeof(*src->ext.supportedgroups));
- if (dest->ext.supportedgroups == NULL)
- goto err;
- }
-#endif
if (ticket != 0 && src->ext.tick != NULL) {
dest->ext.tick =
@@ -790,17 +762,8 @@ void SSL_SESSION_free(SSL_SESSION *ss)
OPENSSL_cleanse(ss->session_id, sizeof(ss->session_id));
X509_free(ss->peer);
sk_X509_pop_free(ss->peer_chain, X509_free);
- sk_SSL_CIPHER_free(ss->ciphers);
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;
- OPENSSL_free(ss->ext.supportedgroups);
- ss->ext.supportedgroups = NULL;
- ss->ext.supportedgroups_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 eacc721..2a9b796 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 b12361f..b6e96ae 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 6181f3f..e16722c 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;
@@ -962,12 +962,12 @@ int tls_parse_ctos_supported_groups(SSL *s, PACKET *pkt, unsigned int context,
}
if (!s->hit || SSL_IS_TLS13(s)) {
- OPENSSL_free(s->session->ext.supportedgroups);
- s->session->ext.supportedgroups = NULL;
- s->session->ext.supportedgroups_len = 0;
+ OPENSSL_free(s->ext.peer_supportedgroups);
+ s->ext.peer_supportedgroups = NULL;
+ s->ext.peer_supportedgroups_len = 0;
if (!tls1_save_u16(&supported_groups_list,
- &s->session->ext.supportedgroups,
- &s->session->ext.supportedgroups_len)) {
+ &s->ext.peer_supportedgroups,
+ &s->ext.peer_supportedgroups_len)) {
SSLfatal(s, SSL_AD_INTERNAL_ERROR,
SSL_F_TLS_PARSE_CTOS_SUPPORTED_GROUPS,
ERR_R_INTERNAL_ERROR);
@@ -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/statem/statem_srvr.c b/ssl/statem/statem_srvr.c
index 6504f4f..79c2aa0 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,
diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index dcae274..95622de 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;
More information about the openssl-commits
mailing list