[openssl] OpenSSL_1_1_1-stable update
Matt Caswell
matt at openssl.org
Tue Jun 18 13:30:35 UTC 2019
The branch OpenSSL_1_1_1-stable has been updated
via d8bb277f76612ec9a659ef7b3df75a99d912662b (commit)
via 860fed97aafd30948a05ae8e90ec3fd43324866a (commit)
via 2813852d7111ad0a49a963bdc49d944d453e52e7 (commit)
from 2459dc1bd09468c83f1767b6b6a1ddc45ba60d36 (commit)
- Log -----------------------------------------------------------------
commit d8bb277f76612ec9a659ef7b3df75a99d912662b
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/9176)
commit 860fed97aafd30948a05ae8e90ec3fd43324866a
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/9176)
commit 2813852d7111ad0a49a963bdc49d944d453e52e7
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/9176)
-----------------------------------------------------------------------
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 99ae481..d7dbf99 100644
--- a/ssl/s3_lib.c
+++ b/ssl/s3_lib.c
@@ -3601,8 +3601,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;
@@ -3716,13 +3716,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 f559bc1..40ab874 100644
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -1160,6 +1160,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) {
@@ -1178,7 +1179,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
@@ -2437,9 +2440,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)
@@ -2578,13 +2581,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 0cf3893..fa0f6d0 100644
--- a/ssl/ssl_locl.h
+++ b/ssl/ssl_locl.h
@@ -552,7 +552,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
@@ -562,13 +561,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 */
@@ -1137,6 +1130,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 */
@@ -1300,10 +1294,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 */
@@ -2240,8 +2243,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 5ad2792..c880ab4 100644
--- a/ssl/ssl_sess.c
+++ b/ssl/ssl_sess.c
@@ -121,12 +121,7 @@ SSL_SESSION *ssl_session_dup(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(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(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 b27608c..7aa4750 100644
--- a/ssl/statem/extensions.c
+++ b/ssl/statem/extensions.c
@@ -1040,18 +1040,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 3c7d844..0ebaeea 100644
--- a/ssl/statem/extensions_clnt.c
+++ b/ssl/statem/extensions_clnt.c
@@ -1371,19 +1371,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 6301b4e..ff4287c 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);
@@ -1376,7 +1376,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 04a2332..e7e95c7 100644
--- a/ssl/statem/statem_srvr.c
+++ b/ssl/statem/statem_srvr.c
@@ -1921,14 +1921,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,
@@ -1939,9 +1939,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);
}
}
@@ -2041,12 +2041,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,
@@ -2253,7 +2253,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 68cb237..2b1b0bd 100644
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -465,11 +465,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