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@fedoraproject.org>
(Merged from https://github.com/openssl/openssl/pull/9162)
This commit is contained in:
Matt Caswell 2019-06-18 11:45:26 +01:00
parent eee2a6a718
commit cd0fb43cbe
8 changed files with 25 additions and 42 deletions

View file

@ -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

View file

@ -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 */

View file

@ -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 */

View file

@ -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);

View file

@ -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;

View file

@ -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);

View file

@ -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;

View file

@ -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;