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 e6356ba..ee97713 100644 (file)
--- 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 e6356ba..ee97713 100644 (file)
--- 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);