From: deraadt <deraadt@openbsd.org>
Date: Thu, 7 Aug 2014 04:49:53 +0000 (+0000)
Subject: Fix CVE-2014-3511; TLS downgrade, verbatim diff
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=20f15341fcb4f83f1232717df78a93fe38bde346;p=openbsd

Fix CVE-2014-3511; TLS downgrade, verbatim diff
https://git.openssl.org/gitweb/?p=openssl.git;a=commit;h=280b1f1ad12131defcd986676a8fc9717aaa601b
ok guenther miod
---

diff --git a/lib/libssl/s23_srvr.c b/lib/libssl/s23_srvr.c
index e6356ba2a28..ee977130fb4 100644
--- a/lib/libssl/s23_srvr.c
+++ b/lib/libssl/s23_srvr.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: s23_srvr.c,v 1.31 2014/07/11 08:17:36 miod Exp $ */
+/* $OpenBSD: s23_srvr.c,v 1.32 2014/08/07 04:49:53 deraadt Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -358,17 +358,19 @@ ssl23_get_client_hello(SSL *s)
 			 * Client Hello message, this would be difficult, and we'd have
 			 * to read more records to find out.
 			 * No known SSL 3.0 client fragments ClientHello like this,
-			 * so we simply assume TLS 1.0 to avoid protocol version downgrade
-			 * attacks. */
+			 * so we simply reject such connections to avoid
+			 * protocol version downgrade attacks. */
 			if (p[3] == 0 && p[4] < 6) {
-				v[1] = TLS1_VERSION_MINOR;
+				SSLerr(SSL_F_SSL23_GET_CLIENT_HELLO,
+				    SSL_R_RECORD_TOO_SMALL);
+				return -1;
 			}
 			/* if major version number > 3 set minor to a value
 			 * which will use the highest version 3 we support.
 			 * If TLS 2.0 ever appears we will need to revise
 			 * this....
 			 */
-			else if (p[9] > SSL3_VERSION_MAJOR)
+			if (p[9] > SSL3_VERSION_MAJOR)
 				v[1] = 0xff;
 			else
 				v[1] = p[10]; /* minor version according to client_version */
@@ -422,13 +424,33 @@ ssl23_get_client_hello(SSL *s)
 		v[0] = p[3]; /* == SSL3_VERSION_MAJOR */
 		v[1] = p[4];
 
+		/* An SSLv3/TLSv1 backwards-compatible CLIENT-HELLO in an SSLv2
+		 * header is sent directly on the wire, not wrapped as a TLS
+		 * record. It's format is:
+		 * Byte  Content
+		 * 0-1   msg_length
+		 * 2     msg_type
+		 * 3-4   version
+		 * 5-6   cipher_spec_length
+		 * 7-8   session_id_length
+		 * 9-10  challenge_length
+		 * ...   ...
+		 */
 		n = ((p[0] & 0x7f) << 8) | p[1];
 		if (n > (1024 * 4)) {
 			SSLerr(SSL_F_SSL23_GET_CLIENT_HELLO, SSL_R_RECORD_TOO_LARGE);
 			return -1;
 		}
+		if (n < 9) {
+			SSLerr(SSL_F_SSL23_GET_CLIENT_HELLO,
+			    SSL_R_RECORD_LENGTH_MISMATCH);
+			return -1;
+		}
 
 		j = ssl23_read_bytes(s, n + 2);
+		/* We previously read 11 bytes, so if j > 0, we must have
+		 * j == n+2 == s->packet_length. We have at least 11 valid
+		 * packet bytes. */
 		if (j <= 0)
 			return (j);
 
diff --git a/lib/libssl/src/ssl/s23_srvr.c b/lib/libssl/src/ssl/s23_srvr.c
index e6356ba2a28..ee977130fb4 100644
--- a/lib/libssl/src/ssl/s23_srvr.c
+++ b/lib/libssl/src/ssl/s23_srvr.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: s23_srvr.c,v 1.31 2014/07/11 08:17:36 miod Exp $ */
+/* $OpenBSD: s23_srvr.c,v 1.32 2014/08/07 04:49:53 deraadt Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -358,17 +358,19 @@ ssl23_get_client_hello(SSL *s)
 			 * Client Hello message, this would be difficult, and we'd have
 			 * to read more records to find out.
 			 * No known SSL 3.0 client fragments ClientHello like this,
-			 * so we simply assume TLS 1.0 to avoid protocol version downgrade
-			 * attacks. */
+			 * so we simply reject such connections to avoid
+			 * protocol version downgrade attacks. */
 			if (p[3] == 0 && p[4] < 6) {
-				v[1] = TLS1_VERSION_MINOR;
+				SSLerr(SSL_F_SSL23_GET_CLIENT_HELLO,
+				    SSL_R_RECORD_TOO_SMALL);
+				return -1;
 			}
 			/* if major version number > 3 set minor to a value
 			 * which will use the highest version 3 we support.
 			 * If TLS 2.0 ever appears we will need to revise
 			 * this....
 			 */
-			else if (p[9] > SSL3_VERSION_MAJOR)
+			if (p[9] > SSL3_VERSION_MAJOR)
 				v[1] = 0xff;
 			else
 				v[1] = p[10]; /* minor version according to client_version */
@@ -422,13 +424,33 @@ ssl23_get_client_hello(SSL *s)
 		v[0] = p[3]; /* == SSL3_VERSION_MAJOR */
 		v[1] = p[4];
 
+		/* An SSLv3/TLSv1 backwards-compatible CLIENT-HELLO in an SSLv2
+		 * header is sent directly on the wire, not wrapped as a TLS
+		 * record. It's format is:
+		 * Byte  Content
+		 * 0-1   msg_length
+		 * 2     msg_type
+		 * 3-4   version
+		 * 5-6   cipher_spec_length
+		 * 7-8   session_id_length
+		 * 9-10  challenge_length
+		 * ...   ...
+		 */
 		n = ((p[0] & 0x7f) << 8) | p[1];
 		if (n > (1024 * 4)) {
 			SSLerr(SSL_F_SSL23_GET_CLIENT_HELLO, SSL_R_RECORD_TOO_LARGE);
 			return -1;
 		}
+		if (n < 9) {
+			SSLerr(SSL_F_SSL23_GET_CLIENT_HELLO,
+			    SSL_R_RECORD_LENGTH_MISMATCH);
+			return -1;
+		}
 
 		j = ssl23_read_bytes(s, n + 2);
+		/* We previously read 11 bytes, so if j > 0, we must have
+		 * j == n+2 == s->packet_length. We have at least 11 valid
+		 * packet bytes. */
 		if (j <= 0)
 			return (j);