Fix server behaviour when facing backwards-compatible client hellos.

diff --git a/CHANGES b/CHANGES
index 649de47179cb3d458fc89640329d295dcb88a032..f2d1cd8eb35587a66858b012f410771eb1cee73e 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -4,6 +4,11 @@
 
  Changes between 0.9.4 and 0.9.5  [xx XXX 1999]
 
+  *) Use client_version from client hello to select the protocol
+     (s23_srvr.c) and for RSA client key exchange verification
+     (s3_srvr.c), as required by the SSL 3.0/TLS 1.0 specifications.
+     [Bodo Moeller]
+
   *) Add various utility functions to handle SPKACs, these were previously
      handled by poking round in the structure internals. Added new function
      NETSCAPE_SPKI_print() to print out SPKAC and a new utility 'spkac' to

diff --git a/crypto/bio/bio.h b/crypto/bio/bio.h
index a05e0c2eacdb7eb8aaecf9f46d13b5ca364aeaef..5558ac544ca58d53acf3444c2a3e1466faf54324 100644 (file)
--- a/crypto/bio/bio.h
+++ b/crypto/bio/bio.h
@@ -517,10 +517,12 @@ BIO *     BIO_get_retry_BIO(BIO *bio, int *reason);
 int    BIO_get_retry_reason(BIO *bio);
 BIO *  BIO_dup_chain(BIO *in);
 
+#if 0 /* not yet */
 int BIO_nread0(BIO *bio, char **buf);
 int BIO_nread(BIO *bio, char **buf, int num);
 int BIO_nwrite0(BIO *bio, char **buf);
 int BIO_nwrite(BIO *bio, char **buf, int num);
+#endif
 
 #ifndef WIN16
 long BIO_debug_callback(BIO *bio,int cmd,const char *argp,int argi,

diff --git a/ssl/s23_pkt.c b/ssl/s23_pkt.c
index 8370ea508c7eb33719bcd9d6edc941ce70e9505f..f45e1ce3d806ce0baa7c6abe86b1d09ca83a03a1 100644 (file)
--- a/ssl/s23_pkt.c
+++ b/ssl/s23_pkt.c
@@ -89,7 +89,7 @@ int ssl23_write_bytes(SSL *s)
                }
        }
 
-/* only return when we have read 'n' bytes */
+/* return regularly only when we have read (at least) 'n' bytes */
 int ssl23_read_bytes(SSL *s, int n)
        {
        unsigned char *p;

diff --git a/ssl/s23_srvr.c b/ssl/s23_srvr.c
index 1a9e5fd867066bc670d2a05e673596c4d7015e2a..4735383c7a395f7670688ba83c6ed8081684139f 100644 (file)
--- a/ssl/s23_srvr.c
+++ b/ssl/s23_srvr.c
@@ -186,7 +186,7 @@ end:
 
 int ssl23_get_client_hello(SSL *s)
        {
-       char buf_space[8];
+       char buf_space[10];
        char *buf= &(buf_space[0]);
        unsigned char *p,*d,*dd;
        unsigned int i;
@@ -202,8 +202,8 @@ int ssl23_get_client_hello(SSL *s)
 
                if (!ssl3_setup_buffers(s)) goto err;
 
-               n=ssl23_read_bytes(s,7);
-               if (n != 7) return(n); /* n == -1 || n == 0 */
+               n=ssl23_read_bytes(s,10);
+               if (n != 10) return(n); /* n == -1 || n == 0 */
 
                p=s->packet;
 
@@ -211,7 +211,9 @@ int ssl23_get_client_hello(SSL *s)
 
                if ((p[0] & 0x80) && (p[2] == SSL2_MT_CLIENT_HELLO))
                        {
-                       /* SSLv2 header */
+                       /*
+                        * SSLv2 header
+                        */
                        if ((p[3] == 0x00) && (p[4] == 0x02))
                                {
                                v[0]=p[3]; v[1]=p[4];
@@ -228,10 +230,12 @@ int ssl23_get_client_hello(SSL *s)
                                        if (!(s->options & SSL_OP_NO_TLSv1))
                                                {
                                                tls1=1;
+                                               /* type=2; */ /* done later to survive restarts */
                                                s->state=SSL23_ST_SR_CLNT_HELLO_B;
                                                }
                                        else if (!(s->options & SSL_OP_NO_SSLv3))
                                                {
+                                               /* type=2; */
                                                s->state=SSL23_ST_SR_CLNT_HELLO_B;
                                                }
                                        else if (!(s->options & SSL_OP_NO_SSLv2))
@@ -240,12 +244,23 @@ int ssl23_get_client_hello(SSL *s)
                                                }
                                        }
                                else if (!(s->options & SSL_OP_NO_SSLv3))
+                                       {
+                                       /* type=2; */
                                        s->state=SSL23_ST_SR_CLNT_HELLO_B;
+                                       }
                                else if (!(s->options & SSL_OP_NO_SSLv2))
                                        type=1;
 
                                if (s->options & SSL_OP_NON_EXPORT_FIRST)
-                                       /* not only confusing, but broken! */
+                                       /* Not only utterly confusing, but broken
+                                        * ('fractured programming'?) -- the details
+                                        * of this block nearly make it work
+                                        * as intended in this environment, but on one
+                                        * of the fine points (w.r.t. restarts) it fails.
+                                        * The obvious fix would be even more devastating
+                                        * to program structure; if you want the functionality,
+                                        * throw this away and implement it in a way
+                                        * that makes sense */
                                        {
                                        STACK_OF(SSL_CIPHER) *sk;
                                        SSL_CIPHER *c;
@@ -301,10 +316,20 @@ int ssl23_get_client_hello(SSL *s)
                        }
                else if ((p[0] == SSL3_RT_HANDSHAKE) &&
                         (p[1] == SSL3_VERSION_MAJOR) &&
-                        (p[5] == SSL3_MT_CLIENT_HELLO))
+                        (p[5] == SSL3_MT_CLIENT_HELLO) &&
+                        (p[9] == p[1]))
                        {
-                       v[0]=p[1]; v[1]=p[2];
-                       /* true SSLv3 or tls1 */
+                       /*
+                        * SSLv3 or tls1 header
+                        */
+                       
+                       /* we must look at client_version inside the client hello: */
+                       n=ssl23_read_bytes(s,11);
+                       /* restarts are no problem here, stay in initial state */
+                       if (n != 11)
+                               return(n); /* n == -1 || n == 0 */
+
+                       v[0]=p[9]; v[1]=p[10];
                        if (p[2] >= TLS1_VERSION_MINOR)
                                {
                                if (!(s->options & SSL_OP_NO_TLSv1))
@@ -336,7 +361,8 @@ int ssl23_get_client_hello(SSL *s)
 next_bit:
        if (s->state == SSL23_ST_SR_CLNT_HELLO_B)
                {
-               /* we have a SSLv3/TLSv1 in a SSLv2 header */
+               /* we have a SSLv3/TLSv1 in a SSLv2 header
+                * (other cases skip this state)* */
                type=2;
                p=s->packet;
                v[0] = p[3];
@@ -406,6 +432,9 @@ next_bit:
                s->s3->tmp.message_size=i;
                }
 
+       /* imaginary new state (for program structure): */
+       /* s->state = SSL23_SR_CLNT_HELLO_C */
+
        if (type == 1)
                {
                /* we are talking sslv2 */
@@ -435,7 +464,7 @@ next_bit:
                else
                        s->s2->ssl2_rollback=1;
 
-               /* setup the 5 bytes we have read so we get them from
+               /* setup the n bytes we have read so we get them from
                 * the sslv2 buffer */
                s->rstate=SSL_ST_READ_HEADER;
                s->packet_length=n;
@@ -450,7 +479,7 @@ next_bit:
 
        if ((type == 2) || (type == 3))
                {
-               /* we have SSLv3/TLSv1 */
+               /* we have SSLv3/TLSv1 (type 2: SSL2 style, type 3: SSL3/TLS style) */
 
                if (!ssl_init_wbio_buffer(s,1)) goto err;
 
@@ -485,7 +514,9 @@ next_bit:
                        s->version=SSL3_VERSION;
                        s->method=SSLv3_server_method();
                        }
+#if 0 /* ssl3_get_client_hello does this */
                s->client_version=(v[0]<<8)|v[1];
+#endif
                s->handshake_func=s->method->ssl_accept;
                }
        

diff --git a/ssl/s3_srvr.c b/ssl/s3_srvr.c
index e003d88357461aa7c9a0b661b9af3a215430d80a..5cb20a28a68a8a39a4e40b941cc5c132b9facebe 100644 (file)
--- a/ssl/s3_srvr.c
+++ b/ssl/s3_srvr.c
@@ -531,10 +531,9 @@ static int ssl3_get_client_hello(SSL *s)
        if (!ok) return((int)n);
        d=p=(unsigned char *)s->init_buf->data;
 
-       /* The version number has already been checked in ssl3_get_message.
-        * I a native TLSv1/SSLv3 method, the match must be correct except
-        * perhaps for the first message */
-/*     s->client_version=(((int)p[0])<<8)|(int)p[1]; */
+       /* use version from inside client hello, not from record header
+        * (may differ: see RFC 2246, Appendix E, second paragraph) */
+       s->client_version=(((int)p[0])<<8)|(int)p[1];
        p+=2;
 
        /* load the client random */
@@ -1239,7 +1238,7 @@ static int ssl3_get_client_key_exchange(SSL *s)
 
                i=RSA_private_decrypt((int)n,p,p,rsa,RSA_PKCS1_PADDING);
 
-#if 1
+#if 0
                /* If a bad decrypt, use a random master key */
                if ((i != SSL_MAX_MASTER_KEY_LENGTH) ||
                        ((p[0] != (s->client_version>>8)) ||