Further version negotiation updates

More miscellaneous updates to version negotiation following feedback.

Reviewed-by: Kurt Roeckx <kurt@openssl.org>

2 files changed, 23 insertions, 46 deletions

diff --git a/ssl/Makefile b/ssl/Makefile
index eb3171eafb..ad2f99f367 100644
--- a/ssl/Makefile
+++ b/ssl/Makefile
@@ -17,7 +17,7 @@ GENERAL=Makefile README ssl-lib.com install.com
 LIB=$(TOP)/libssl.a
 SHARED_LIB= libssl$(SHLIB_EXT)
 LIBSRC=	\
-	s3_meth.c   s3_srvr.c s3_clnt.c  s3_lib.c  s3_enc.c record/rec_layer_s3.c \
+	s3_srvr.c s3_clnt.c  s3_lib.c  s3_enc.c record/rec_layer_s3.c \
 	s3_both.c s3_cbc.c s3_msg.c \
 	t1_meth.c   t1_srvr.c t1_clnt.c  t1_lib.c  t1_enc.c t1_ext.c \
 	d1_meth.c   d1_srvr.c d1_clnt.c  d1_lib.c  record/rec_layer_d1.c d1_msg.c \
@@ -28,7 +28,7 @@ LIBSRC=	\
 	bio_ssl.c ssl_err.c t1_reneg.c tls_srp.c t1_trce.c ssl_utst.c \
 	record/ssl3_buffer.c record/ssl3_record.c record/dtls1_bitmap.c
 LIBOBJ= \
-	s3_meth.o  s3_srvr.o  s3_clnt.o  s3_lib.o  s3_enc.o record/rec_layer_s3.o \
+	s3_srvr.o  s3_clnt.o  s3_lib.o  s3_enc.o record/rec_layer_s3.o \
 	s3_both.o s3_cbc.o s3_msg.o \
 	t1_meth.o   t1_srvr.o t1_clnt.o  t1_lib.o  t1_enc.o t1_ext.o \
 	d1_meth.o   d1_srvr.o d1_clnt.o  d1_lib.o  record/rec_layer_d1.o d1_msg.o \
@@ -406,25 +406,6 @@ s3_lib.o: ../include/openssl/ssl2.h ../include/openssl/ssl3.h
 s3_lib.o: ../include/openssl/stack.h ../include/openssl/symhacks.h
 s3_lib.o: ../include/openssl/tls1.h ../include/openssl/x509.h
 s3_lib.o: ../include/openssl/x509_vfy.h record/record.h s3_lib.c ssl_locl.h
-s3_meth.o: ../e_os.h ../include/openssl/asn1.h ../include/openssl/bio.h
-s3_meth.o: ../include/openssl/buffer.h ../include/openssl/comp.h
-s3_meth.o: ../include/openssl/crypto.h ../include/openssl/dsa.h
-s3_meth.o: ../include/openssl/dtls1.h ../include/openssl/e_os2.h
-s3_meth.o: ../include/openssl/ec.h ../include/openssl/ecdh.h
-s3_meth.o: ../include/openssl/ecdsa.h ../include/openssl/err.h
-s3_meth.o: ../include/openssl/evp.h ../include/openssl/hmac.h
-s3_meth.o: ../include/openssl/lhash.h ../include/openssl/obj_mac.h
-s3_meth.o: ../include/openssl/objects.h ../include/openssl/opensslconf.h
-s3_meth.o: ../include/openssl/opensslv.h ../include/openssl/ossl_typ.h
-s3_meth.o: ../include/openssl/pem.h ../include/openssl/pem2.h
-s3_meth.o: ../include/openssl/pkcs7.h ../include/openssl/pqueue.h
-s3_meth.o: ../include/openssl/rsa.h ../include/openssl/safestack.h
-s3_meth.o: ../include/openssl/sha.h ../include/openssl/srtp.h
-s3_meth.o: ../include/openssl/ssl.h ../include/openssl/ssl2.h
-s3_meth.o: ../include/openssl/ssl3.h ../include/openssl/stack.h
-s3_meth.o: ../include/openssl/symhacks.h ../include/openssl/tls1.h
-s3_meth.o: ../include/openssl/x509.h ../include/openssl/x509_vfy.h
-s3_meth.o: record/record.h s3_meth.c ssl_locl.h
 s3_msg.o: ../e_os.h ../include/openssl/asn1.h ../include/openssl/bio.h
 s3_msg.o: ../include/openssl/buffer.h ../include/openssl/comp.h
 s3_msg.o: ../include/openssl/crypto.h ../include/openssl/dsa.h
diff --git a/ssl/s3_srvr.c b/ssl/s3_srvr.c
index 3e7f44ada9..ce092a70a3 100644
--- a/ssl/s3_srvr.c
+++ b/ssl/s3_srvr.c
@@ -883,7 +883,7 @@ int ssl3_send_hello_request(SSL *s)
 
 int ssl3_get_client_hello(SSL *s)
 {
-    int i, j, ok, al = SSL_AD_INTERNAL_ERROR, ret = -1;
+    int i, complen, j, ok, al = SSL_AD_INTERNAL_ERROR, ret = -1;
     unsigned int cookie_len;
     long n;
     unsigned long id;
@@ -921,13 +921,7 @@ int ssl3_get_client_hello(SSL *s)
     d = p = (unsigned char *)s->init_msg;
 
     /* First lets get s->client_version set correctly */
-    if (!s->read_hash && !s->enc_read_ctx
-            && RECORD_LAYER_is_sslv2_record(&s->rlayer)) {
-        if (n < MIN_SSL2_RECORD_LEN) {
-            SSLerr(SSL_F_SSL3_GET_CLIENT_HELLO, SSL_R_RECORD_LENGTH_MISMATCH);
-            al = SSL_AD_DECODE_ERROR;
-            goto f_err;
-        }
+    if (RECORD_LAYER_is_sslv2_record(&s->rlayer)) {
         /*-
          * An SSLv3/TLSv1 backwards-compatible CLIENT-HELLO in an SSLv2
          * header is sent directly on the wire, not wrapped as a TLS
@@ -986,11 +980,10 @@ int ssl3_get_client_hello(SSL *s)
     /* Do SSL/TLS version negotiation if applicable */
     if (!SSL_IS_DTLS(s)) {
         if (s->version != TLS_ANY_VERSION) {
-            if (s->client_version >= s->version
-                && (((s->client_version >> 8) & 0xff) == SSL3_VERSION_MAJOR)) {
+            if (s->client_version >= s->version) {
                 protverr = 0;
             }
-        } else if (((s->client_version >> 8) & 0xff) == SSL3_VERSION_MAJOR) {
+        } else if (s->client_version >= SSL3_VERSION) {
             switch(s->client_version) {
             default:
             case TLS1_2_VERSION:
@@ -1018,17 +1011,20 @@ int ssl3_get_client_hello(SSL *s)
                 }
                 /* Deliberately fall through */
             case SSL3_VERSION:
+#ifndef OPENSSL_NO_SSL3
                 if(!(s->options & SSL_OP_NO_SSLv3)) {
                     s->version = SSL3_VERSION;
                     s->method = SSLv3_server_method();
                     protverr = 0;
                     break;
                 }
+#else
+                break;
+#endif
             }
         }
-    } else if (((s->client_version >> 8) & 0xff) == DTLS1_VERSION_MAJOR &&
-                (s->client_version <= s->version
-                || s->method->version == DTLS_ANY_VERSION)) {
+    } else if (s->client_version <= s->version
+                || s->method->version == DTLS_ANY_VERSION) {
         /*
          * For DTLS we just check versions are potentially compatible. Version
          * negotiation comes later.
@@ -1096,8 +1092,8 @@ int ssl3_get_client_hello(SSL *s)
         /* Set p to end of packet to ensure we don't look for extensions */
         p = d + n;
 
-        /* No compression, so set i to 0 */
-        i = 0;
+        /* No compression, so set complen to 0 */
+        complen = 0;
     } else {
         /* If we get here we've got SSLv3+ in an SSLv3+ record */
 
@@ -1341,8 +1337,8 @@ int ssl3_get_client_hello(SSL *s)
         }
 
         /* compression */
-        i = *(p++);
-        if ((p + i) > (d + n)) {
+        complen = *(p++);
+        if ((p + complen) > (d + n)) {
             /* not enough data */
             al = SSL_AD_DECODE_ERROR;
             SSLerr(SSL_F_SSL3_GET_CLIENT_HELLO, SSL_R_LENGTH_MISMATCH);
@@ -1351,13 +1347,13 @@ int ssl3_get_client_hello(SSL *s)
 #ifndef OPENSSL_NO_COMP
         q = p;
 #endif
-        for (j = 0; j < i; j++) {
+        for (j = 0; j < complen; j++) {
             if (p[j] == 0)
                 break;
         }
 
-        p += i;
-        if (j >= i) {
+        p += complen;
+        if (j >= complen) {
             /* no compress */
             al = SSL_AD_DECODE_ERROR;
             SSLerr(SSL_F_SSL3_GET_CLIENT_HELLO, SSL_R_NO_COMPRESSION_SPECIFIED);
@@ -1426,7 +1422,7 @@ int ssl3_get_client_hello(SSL *s)
 
     /*
      * Worst case, we will use the NULL compression, but if we have other
-     * options, we will now look for them.  We have i-1 compression
+     * options, we will now look for them.  We have complen-1 compression
      * algorithms from the client, starting at q.
      */
     s->s3->tmp.new_compression = NULL;
@@ -1455,11 +1451,11 @@ int ssl3_get_client_hello(SSL *s)
             goto f_err;
         }
         /* Look for resumed method in compression list */
-        for (m = 0; m < i; m++) {
+        for (m = 0; m < complen; m++) {
             if (q[m] == comp_id)
                 break;
         }
-        if (m >= i) {
+        if (m >= complen) {
             al = SSL_AD_ILLEGAL_PARAMETER;
             SSLerr(SSL_F_SSL3_GET_CLIENT_HELLO,
                    SSL_R_REQUIRED_COMPRESSSION_ALGORITHM_MISSING);
@@ -1475,7 +1471,7 @@ int ssl3_get_client_hello(SSL *s)
         for (m = 0; m < nn; m++) {
             comp = sk_SSL_COMP_value(s->ctx->comp_methods, m);
             v = comp->id;
-            for (o = 0; o < i; o++) {
+            for (o = 0; o < complen; o++) {
                 if (v == q[o]) {
                     done = 1;
                     break;