Fix CVE-2014-3511; TLS downgrade, verbatim diff

https://git.openssl.org/gitweb/?p=openssl.git;a=commit;h=280b1f1ad12131defcd986676a8fc9717aaa601b
ok guenther miod

2 files changed, 54 insertions, 10 deletions

diff --git a/src/lib/libssl/s23_srvr.c b/src/lib/libssl/s23_srvr.c
index e6356ba2a2..ee977130fb 100644
--- a/src/lib/libssl/s23_srvr.c
+++ b/src/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/src/lib/libssl/src/ssl/s23_srvr.c b/src/lib/libssl/src/ssl/s23_srvr.c
index e6356ba2a2..ee977130fb 100644
--- a/src/lib/libssl/src/ssl/s23_srvr.c
+++ b/src/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);