From 90a27dd2630fc2f77d90209add109a18beb0f2e2 Mon Sep 17 00:00:00 2001
From: jsing <>
Date: Thu, 29 May 2014 11:28:18 +0000
Subject: Fix another two cases where the return value of ssl_replace_hash() is
 unchecked.

In the case of tls1_change_cipher_state(), it is fairly pointless to use
ssl_replace_hash(), since it does not initialise the hash and there is
special handling required in the DTLS write case. Instead, just inline
the part of ssl_replace_hash() that is needed and only
ssl_clear_hash_ctx() the write hash in the non-DTLS case.

Also add a detailed comment explaining why there needs to be specialised
handling for DTLS write context and where the contexts are actually freed.

ok miod@
---
 src/lib/libssl/src/ssl/t1_enc.c | 26 ++++++++++++++++++--------
 src/lib/libssl/t1_enc.c         | 26 ++++++++++++++++++--------
 2 files changed, 36 insertions(+), 16 deletions(-)

(limited to 'src')

diff --git a/src/lib/libssl/src/ssl/t1_enc.c b/src/lib/libssl/src/ssl/t1_enc.c
index 894b521e71..87860feda9 100644
--- a/src/lib/libssl/src/ssl/t1_enc.c
+++ b/src/lib/libssl/src/ssl/t1_enc.c
@@ -386,7 +386,11 @@ tls1_change_cipher_state(SSL *s, int which)
 			EVP_CIPHER_CTX_init(s->enc_read_ctx);
 		}
 		dd = s->enc_read_ctx;
-		mac_ctx = ssl_replace_hash(&s->read_hash, NULL);
+
+		ssl_clear_hash_ctx(&s->read_hash);
+		if ((mac_ctx = EVP_MD_CTX_create()) == NULL)
+			goto err;
+		s->read_hash = mac_ctx;
 
 		/* this is done by dtls1_reset_seq_numbers for DTLS1_VERSION */
 		if (s->version != DTLS1_VERSION)
@@ -403,13 +407,19 @@ tls1_change_cipher_state(SSL *s, int which)
 		else if ((s->enc_write_ctx = EVP_CIPHER_CTX_new()) == NULL)
 			goto err;
 		dd = s->enc_write_ctx;
-		if (SSL_IS_DTLS(s)) {
-			mac_ctx = EVP_MD_CTX_create();
-			if (!mac_ctx)
-				goto err;
-			s->write_hash = mac_ctx;
-		} else
-			mac_ctx = ssl_replace_hash(&s->write_hash, NULL);
+
+		/*
+		 * DTLS fragments retain a pointer to the compression, cipher
+		 * and hash contexts, so that it can restore state in order
+		 * to perform retransmissions. As such, we cannot free write
+		 * contexts that are used for DTLS - these are instead freed
+		 * by DTLS when its frees a ChangeCipherSpec fragment.
+		 */
+		if (!SSL_IS_DTLS(s))
+			ssl_clear_hash_ctx(&s->write_hash);
+		if ((mac_ctx = EVP_MD_CTX_create()) == NULL)
+			goto err;
+		s->write_hash = mac_ctx;
 
 		/* this is done by dtls1_reset_seq_numbers for DTLS1_VERSION */
 		if (s->version != DTLS1_VERSION)
diff --git a/src/lib/libssl/t1_enc.c b/src/lib/libssl/t1_enc.c
index 894b521e71..87860feda9 100644
--- a/src/lib/libssl/t1_enc.c
+++ b/src/lib/libssl/t1_enc.c
@@ -386,7 +386,11 @@ tls1_change_cipher_state(SSL *s, int which)
 			EVP_CIPHER_CTX_init(s->enc_read_ctx);
 		}
 		dd = s->enc_read_ctx;
-		mac_ctx = ssl_replace_hash(&s->read_hash, NULL);
+
+		ssl_clear_hash_ctx(&s->read_hash);
+		if ((mac_ctx = EVP_MD_CTX_create()) == NULL)
+			goto err;
+		s->read_hash = mac_ctx;
 
 		/* this is done by dtls1_reset_seq_numbers for DTLS1_VERSION */
 		if (s->version != DTLS1_VERSION)
@@ -403,13 +407,19 @@ tls1_change_cipher_state(SSL *s, int which)
 		else if ((s->enc_write_ctx = EVP_CIPHER_CTX_new()) == NULL)
 			goto err;
 		dd = s->enc_write_ctx;
-		if (SSL_IS_DTLS(s)) {
-			mac_ctx = EVP_MD_CTX_create();
-			if (!mac_ctx)
-				goto err;
-			s->write_hash = mac_ctx;
-		} else
-			mac_ctx = ssl_replace_hash(&s->write_hash, NULL);
+
+		/*
+		 * DTLS fragments retain a pointer to the compression, cipher
+		 * and hash contexts, so that it can restore state in order
+		 * to perform retransmissions. As such, we cannot free write
+		 * contexts that are used for DTLS - these are instead freed
+		 * by DTLS when its frees a ChangeCipherSpec fragment.
+		 */
+		if (!SSL_IS_DTLS(s))
+			ssl_clear_hash_ctx(&s->write_hash);
+		if ((mac_ctx = EVP_MD_CTX_create()) == NULL)
+			goto err;
+		s->write_hash = mac_ctx;
 
 		/* this is done by dtls1_reset_seq_numbers for DTLS1_VERSION */
 		if (s->version != DTLS1_VERSION)
-- 
cgit v1.2.3-55-g6feb