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/9176)

diff --git a/ssl/s3_lib.c b/ssl/s3_lib.c
index dc148bc2b10411a9e7bd8026403bce620683b04f..d7dbf99954cfd71b732420cdaf4e22a2dbef33dc 100644 (file)
--- a/ssl/s3_lib.c
+++ b/ssl/s3_lib.c
@@ -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 ae65014c0c37fc8db87ece663c918afd4556dc03..40ab87480db535d7590ae139253c0bf338bc8aad 100644 (file)
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -1179,6 +1179,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 1b7bf111fd425687e4adb23aa7949fdb7f149365..fa0f6d018cafdb615ad7684bf1853205806a3a1c 100644 (file)
--- a/ssl/ssl_locl.h
+++ b/ssl/ssl_locl.h
@@ -561,10 +561,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 */
@@ -1298,6 +1294,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 2c3bb5cf793b52708944628c2f394bcef3671781..c880ab4e4b2ab70baef80df37a1746fc12bc6986 100644 (file)
--- a/ssl/ssl_sess.c
+++ b/ssl/ssl_sess.c
@@ -122,9 +122,6 @@ SSL_SESSION *ssl_session_dup(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(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 b27608cbb199ef01389383520aa3539afc086022..7aa475038eaf52b2fc87fd077bd0f742fb6c67ff 100644 (file)
--- 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 3c7d84427f082ea8916cc1057e526e4217138de7..0ebaeead9589d23b447aedf17887f6c5663c9a33 100644 (file)
--- 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 5b83c267852d490b436203bbdd64a86a40ebe901..ff4287c584b76fdd2c5ab635b7dde6a755353116 100644 (file)
--- 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;
@@ -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/t1_lib.c b/ssl/t1_lib.c
index 68cb237ea95ee6f60baa4a33178e59cccb68c9ee..2b1b0bdb3bd7639f224e19e955940b921a0f5fc6 100644 (file)
--- 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;