Move freeing of an old enc_write_ctx/write_hash to dtls1_clear_sent_buffer

When we are clearing the sent messages queue we should ensure we free any
old enc_write_ctx/write_hash that are no longer in use. Previously this
logic was in dtls1_hm_fragment_free() - but this can end up freeing the
current enc_write_ctx/write_hash under certain error conditions.

Fixes #22664

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Tim Hudson <tjh@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2261)

diff --git a/ssl/d1_lib.c b/ssl/d1_lib.c
index 871c187a9dae85f2bc7d0312cf3777dc139be3de..6c2d4ba73e769120b874f6fd7826c7aa54a6055e 100644 (file)
--- a/ssl/d1_lib.c
+++ b/ssl/d1_lib.c
@@ -130,6 +130,23 @@ void dtls1_clear_sent_buffer(SSL *s)
 
     while ((item = pqueue_pop(s->d1->sent_messages)) != NULL) {
         frag = (hm_fragment *)item->data;
+
+        if (frag->msg_header.is_ccs) {
+            /*
+             * If we're freeing the CCS then we're done with the old
+             * enc_write_ctx/write_hash and they can be freed
+             */
+            if (s->enc_write_ctx
+                    != frag->msg_header.saved_retransmit_state.enc_write_ctx)
+                EVP_CIPHER_CTX_free(frag->msg_header.saved_retransmit_state
+                                                    .enc_write_ctx);
+
+            if (s->write_hash
+                    != frag->msg_header.saved_retransmit_state.write_hash)
+                EVP_MD_CTX_free(frag->msg_header.saved_retransmit_state
+                                                .write_hash);
+        }
+
         dtls1_hm_fragment_free(frag);
         pitem_free(item);
     }

diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index 26cbcd2c32edfc3a99afe123467cc3198a29d899..a0185cce07fc93e1c4a9c2b813d000c9c6caaaf2 100644 (file)
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -1216,8 +1216,6 @@ void SSL_free(SSL *s)
     SSL_SESSION_free(s->psksession);
     OPENSSL_free(s->psksession_id);
 
-    clear_ciphers(s);
-
     ssl_cert_free(s->cert);
     OPENSSL_free(s->shared_sigalgs);
     /* Free up if allocated */
@@ -1253,6 +1251,12 @@ void SSL_free(SSL *s)
     if (s->method != NULL)
         s->method->ssl_free(s);
 
+    /*
+     * Must occur after s->method->ssl_free(). The DTLS sent_messages queue
+     * may reference the EVP_CIPHER_CTX/EVP_MD_CTX that are freed here.
+     */
+    clear_ciphers(s);
+
     SSL_CTX_free(s->ctx);
 
     ASYNC_WAIT_CTX_free(s->waitctx);

diff --git a/ssl/statem/statem_dtls.c b/ssl/statem/statem_dtls.c
index 2e98df6235db9065bef2f87a322c4f0a63791a2d..040c23035c99441f29e1a6787615d422e2f3f9c2 100644 (file)
--- a/ssl/statem/statem_dtls.c
+++ b/ssl/statem/statem_dtls.c
@@ -95,11 +95,7 @@ void dtls1_hm_fragment_free(hm_fragment *frag)
 {
     if (!frag)
         return;
-    if (frag->msg_header.is_ccs) {
-        EVP_CIPHER_CTX_free(frag->msg_header.
-                            saved_retransmit_state.enc_write_ctx);
-        EVP_MD_CTX_free(frag->msg_header.saved_retransmit_state.write_hash);
-    }
+
     OPENSSL_free(frag->fragment);
     OPENSSL_free(frag->reassembly);
     OPENSSL_free(frag);