Reorder cleanup sequence in SSL_CTX_free() to leave ex_data for remove_cb().

Submitted by:
Reviewed by:
PR: 212

diff --git a/CHANGES b/CHANGES
index eb964236f85ffbe9220d2a3c67b3e9803999f5d2..9d1ba42e91b24e2ece56681c0c24a47501dd51de 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1914,6 +1914,12 @@ des-cbc           3624.96k     5258.21k     5530.91k     5624.30k     5628.26k
 
  Changes between 0.9.6g and 0.9.6h  [xx XXX xxxx]
 
 
  Changes between 0.9.6g and 0.9.6h  [xx XXX xxxx]
 

+  *) Reorder cleanup sequence in SSL_CTX_free(): only remove the ex_data after
+     the cached sessions are flushed, as the remove_cb() might use ex_data
+     contents. Bug found by Sam Varshavchik <mrsam@courier-mta.com>
+     (see [openssl.org #212]).
+     [Geoff Thorpe, Lutz Jaenicke]
+

   *) Fix typo in OBJ_txt2obj which incorrectly passed the content
      length, instead of the encoding length to d2i_ASN1_OBJECT.
      [Steve Henson]
   *) Fix typo in OBJ_txt2obj which incorrectly passed the content
      length, instead of the encoding length to d2i_ASN1_OBJECT.
      [Steve Henson]

diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index eda3cfd11631854399b85f41dbf3e13108b08080..1ddd3380acb15adace6865967bcaeb2c2f65beb8 100644 (file)
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -1409,13 +1409,24 @@ void SSL_CTX_free(SSL_CTX *a)
                abort(); /* ok */
                }
 #endif
                abort(); /* ok */
                }
 #endif

-       CRYPTO_free_ex_data(CRYPTO_EX_INDEX_SSL_CTX, a, &a->ex_data);

 
 

+       /*
+        * Free internal session cache. However: the remove_cb() may reference
+        * the ex_data of SSL_CTX, thus the ex_data store can only be removed
+        * after the sessions were flushed.
+        * As the ex_data handling routines might also touch the session cache,
+        * the most secure solution seems to be: empty (flush) the cache, then
+        * free ex_data, then finally free the cache.
+        * (See ticket [openssl.org #212].)
+        */

        if (a->sessions != NULL)
        if (a->sessions != NULL)

-               {

                SSL_CTX_flush_sessions(a,0);
                SSL_CTX_flush_sessions(a,0);

+
+       CRYPTO_free_ex_data(CRYPTO_EX_INDEX_SSL_CTX, a, &a->ex_data);
+
+       if (a->sessions != NULL)

                lh_free(a->sessions);
                lh_free(a->sessions);

-               }
+

        if (a->cert_store != NULL)
                X509_STORE_free(a->cert_store);
        if (a->cipher_list != NULL)
        if (a->cert_store != NULL)
                X509_STORE_free(a->cert_store);
        if (a->cipher_list != NULL)