From: Bodo Möller <bodo@openssl.org>
Date: Thu, 25 Oct 2001 06:09:51 +0000 (+0000)
Subject: Assume TLS 1.0 when ClientHello fragment is too short.
X-Git-Tag: OpenSSL-engine-0_9_6c~39^2~41
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=ba1c602281ebe05087b8441b51cf9cf63c066a8c;p=openssl

Assume TLS 1.0 when ClientHello fragment is too short.
---

diff --git a/CHANGES b/CHANGES
index 755efead7d..99d9dd0642 100644
--- a/CHANGES
+++ b/CHANGES
@@ -12,6 +12,15 @@
          *) applies to 0.9.6a/0.9.6b/0.9.6c and 0.9.7
          +) applies to 0.9.7 only
 
+  *) Change ssl23_get_client_hello (ssl/s23_srvr.c) behaviour when
+     faced with a pathologically small ClientHello fragment that does
+     not contain client_version: Instead of aborting with an error,
+     simply choose the highest available protocol version (i.e.,
+     TLS 1.0 unless it is disabled).  In practice, ClientHello
+     messages are never sent like this, but this change gives us
+     strictly correct behaviour at least for TLS.
+     [Bodo Moeller]
+
   +) Change all functions with names starting with des_ to be starting
      with DES_ instead.  This because there are increasing clashes with
      libdes and other des libraries that are currently used by other
diff --git a/ssl/s23_srvr.c b/ssl/s23_srvr.c
index fe8bd33ce7..f2e1dc4173 100644
--- a/ssl/s23_srvr.c
+++ b/ssl/s23_srvr.c
@@ -232,9 +232,9 @@ int ssl23_accept(SSL *s)
 			}
 		}
 end:
+	s->in_handshake--;
 	if (cb != NULL)
 		cb(s,SSL_CB_ACCEPT_EXIT,ret);
-	s->in_handshake--;
 	return(ret);
 	}
 
@@ -339,17 +339,22 @@ int ssl23_get_client_hello(SSL *s)
 			/* We must look at client_version inside the Client Hello message
 			 * to get the correct minor version.
 			 * However if we have only a pathologically small fragment of the
-			 * Client Hello message, this would be difficult, we'd have
-			 * to read at least one additional record to find out.
-			 * This doesn't usually happen in real life, so we just complain
-			 * for now.
-			 */
+			 * 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. */
 			if (p[3] == 0 && p[4] < 6)
 				{
+#if 0
 				SSLerr(SSL_F_SSL23_GET_CLIENT_HELLO,SSL_R_RECORD_TOO_SMALL);
 				goto err;
+#else
+				v[1] = TLS1_VERSION_MINOR;
+#endif
 				}
-			v[1]=p[10]; /* minor version according to client_version */
+			else
+				v[1]=p[10]; /* minor version according to client_version */
 			if (v[1] >= TLS1_VERSION_MINOR)
 				{
 				if (!(s->options & SSL_OP_NO_TLSv1))