Clean up some of the SSL server code.
authorBodo Möller <bodo@openssl.org>
Tue, 11 Jan 2000 01:07:26 +0000 (01:07 +0000)
committerBodo Möller <bodo@openssl.org>
Tue, 11 Jan 2000 01:07:26 +0000 (01:07 +0000)
CHANGES
ssl/s23_srvr.c
ssl/s3_pkt.c
ssl/s3_srvr.c
ssl/ssl.h
ssl/ssl3.h
ssl/ssl_err.c

diff --git a/CHANGES b/CHANGES
index 49ed7aef5ab35f15c39b7c53349c0e243176d26e..c1edb5c2e32747928b6c881f8ec34ed43f515b38 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -4,6 +4,11 @@
 
  Changes between 0.9.4 and 0.9.5  [xx XXX 1999]
 
+  *) Rewrite ssl3_read_n (ssl/s3_pkt.c) avoiding a couple of bugs,
+     including a possible buffer overflow when the 'read_ahead'
+     flag is set.
+     [Bodo Moeller]
+
   *) New function X509_CTX_rget_chain(), this returns the chain
      from an X509_CTX structure with a dup of the stack and all
      the X509 reference counts upped: so the stack will exist
index a9fefef3543a3c4ce98ef1796cc0e67223a53a55..3aec65dd4f398fb837043a2b6a555a67eaacbec1 100644 (file)
@@ -186,7 +186,18 @@ end:
 
 int ssl23_get_client_hello(SSL *s)
        {
-       char buf_space[11]; /* request this many bytes in initial read */
+       char buf_space[11]; /* Request this many bytes in initial read.
+                            * We can detect SSL 3.0/TLS 1.0 Client Hellos only
+                            * when the following is in a single record
+                            * (not guaranteed by protocol specs):
+                            * Byte  Content
+                            *  0     type            \
+                            *  1/2   version          > record header
+                            *  3/4   length          /
+                            *  5     msg_type        \
+                            *  6-8   length           > Client Hello message
+                            *  9/10  client_version  /
+                            */
        char *buf= &(buf_space[0]);
        unsigned char *p,*d,*dd;
        unsigned int i;
index 1ce30ff5a191238579e5ee3f4da054cfa1eaf1de..38063b50d58d1472195931283f0ce1442d1aecd6 100644 (file)
  * copied and put under another distribution licence
  * [including the GNU Public Licence.]
  */
+/* ====================================================================
+ * Copyright (c) 1998-1999 The OpenSSL Project.  All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer. 
+ *
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in
+ *    the documentation and/or other materials provided with the
+ *    distribution.
+ *
+ * 3. All advertising materials mentioning features or use of this
+ *    software must display the following acknowledgment:
+ *    "This product includes software developed by the OpenSSL Project
+ *    for use in the OpenSSL Toolkit. (http://www.openssl.org/)"
+ *
+ * 4. The names "OpenSSL Toolkit" and "OpenSSL Project" must not be used to
+ *    endorse or promote products derived from this software without
+ *    prior written permission. For written permission, please contact
+ *    openssl-core@openssl.org.
+ *
+ * 5. Products derived from this software may not be called "OpenSSL"
+ *    nor may "OpenSSL" appear in their names without prior written
+ *    permission of the OpenSSL Project.
+ *
+ * 6. Redistributions of any form whatsoever must retain the following
+ *    acknowledgment:
+ *    "This product includes software developed by the OpenSSL Project
+ *    for use in the OpenSSL Toolkit (http://www.openssl.org/)"
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE OpenSSL PROJECT ``AS IS'' AND ANY
+ * EXPRESSED OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL THE OpenSSL PROJECT OR
+ * ITS CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+ * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED
+ * OF THE POSSIBILITY OF SUCH DAMAGE.
+ * ====================================================================
+ *
+ * This product includes cryptographic software written by Eric Young
+ * (eay@cryptsoft.com).  This product includes software written by Tim
+ * Hudson (tjh@cryptsoft.com).
+ *
+ */
 
 #include <stdio.h>
 #include <errno.h>
@@ -71,104 +124,91 @@ static int ssl3_get_record(SSL *s);
 static int do_compress(SSL *ssl);
 static int do_uncompress(SSL *ssl);
 static int do_change_cipher_spec(SSL *ssl);
+
 static int ssl3_read_n(SSL *s, int n, int max, int extend)
        {
+       /* If extend == 0, obtain new n-byte packet; if extend == 1, increase
+        * packet by another n bytes.
+        * The packet will be in the sub-array of s->s3->rbuf.buf specified
+        * by s->packet and s->packet_length.
+        * (If s->read_ahead is set, 'max' bytes may be stored in rbuf
+        * [plus s->packet_length bytes if extend == 1].)
+        */
        int i,off,newb;
 
-       /* if there is stuff still in the buffer from a previous read,
-        * and there is more than we want, take some. */
+       if (!extend)
+               {
+               /* start with empty packet ... */
+               if (s->s3->rbuf.left == 0)
+                       s->s3->rbuf.offset = 0;
+               s->packet = s->s3->rbuf.buf + s->s3->rbuf.offset;
+               s->packet_length = 0;
+               /* ... now we can act as if 'extend' was set */
+               }
+
+       /* if there is enough in the buffer from a previous read, take some */
        if (s->s3->rbuf.left >= (int)n)
                {
-               if (extend)
-                       s->packet_length+=n;
-               else
-                       {
-                       s->packet= &(s->s3->rbuf.buf[s->s3->rbuf.offset]);
-                       s->packet_length=n;
-                       }
+               s->packet_length+=n;
                s->s3->rbuf.left-=n;
                s->s3->rbuf.offset+=n;
                return(n);
                }
 
        /* else we need to read more data */
-       if (!s->read_ahead) max=n;
-       if (max > SSL3_RT_MAX_PACKET_SIZE)
-               max=SSL3_RT_MAX_PACKET_SIZE;
-
-       /* First check if there is some left or we want to extend */
-       off=0;
-       if (    (s->s3->rbuf.left != 0) ||
-               ((s->packet_length != 0) && extend))
-               {
-               newb=s->s3->rbuf.left;
-               if (extend)
-                       {
-                       /* Copy bytes back to the front of the buffer 
-                        * Take the bytes already pointed to by 'packet'
-                        * and take the extra ones on the end. */
-                       off=s->packet_length;
-                       if (s->packet != s->s3->rbuf.buf)
-                               memcpy(s->s3->rbuf.buf,s->packet,newb+off);
-                       }
-               else if (s->s3->rbuf.offset != 0)
-                       { /* so the data is not at the start of the buffer */
-                       memcpy(s->s3->rbuf.buf,
-                               &(s->s3->rbuf.buf[s->s3->rbuf.offset]),newb);
-                       s->s3->rbuf.offset=0;
-                       }
-
-               s->s3->rbuf.left=0;
-               }
-       else
-               newb=0;
+       if (!s->read_ahead)
+               max=n;
 
-       /* So we now have 'newb' bytes at the front of 
-        * s->s3->rbuf.buf and need to read some more in on the end
-        * We start reading into the buffer at 's->s3->rbuf.offset'
-        */
-       s->packet=s->s3->rbuf.buf;
+       {
+               /* avoid buffer overflow */
+               int max_max = SSL3_RT_MAX_PACKET_SIZE - s->packet_length;
+               if (s->options & SSL_OP_MICROSOFT_BIG_SSLV3_BUFFER)
+                       max_max += SSL3_RT_MAX_EXTRA;
+               if (max > max_max)
+                       max = max_max;
+       }
 
+       off = s->packet_length;
+       newb = s->s3->rbuf.left;
+       /* Move any available bytes to front of buffer:
+        * 'off' bytes already pointed to by 'packet',
+        * 'newb' extra ones at the end */
+       if (s->packet != s->s3->rbuf.buf)
+               {
+               /*  off > 0 */
+               memmove(s->s3->rbuf.buf, s->packet, off+newb);
+               s->packet = s->s3->rbuf.buf;
+               }
+       
        while (newb < n)
                {
+               /* Now we have off+newb bytes at the front of s->s3->rbuf.buf and need
+                * to read in more until we have off+n (up to off+max if possible) */
+
                clear_sys_error();
                if (s->rbio != NULL)
                        {
                        s->rwstate=SSL_READING;
-                       i=BIO_read(s->rbio,
-                               (char *)&(s->s3->rbuf.buf[off+newb]),
-                               max-newb);
+                       i=BIO_read(s->rbio,     &(s->s3->rbuf.buf[off+newb]), max-newb);
                        }
                else
                        {
                        SSLerr(SSL_F_SSL3_READ_N,SSL_R_READ_BIO_NOT_SET);
-                       i= -1;
+                       i = -1;
                        }
 
                if (i <= 0)
                        {
-                       s->s3->rbuf.left+=newb;
+                       s->s3->rbuf.left = newb;
                        return(i);
                        }
                newb+=i;
                }
 
-       /* record used data read */
-       if (newb > n)
-               {
-               s->s3->rbuf.offset=n+off;
-               s->s3->rbuf.left=newb-n;
-               }
-       else
-               {
-               s->s3->rbuf.offset=0;
-               s->s3->rbuf.left=0;
-               }
-
-       if (extend)
-               s->packet_length+=n;
-       else
-               s->packet_length+=n;
+       /* done reading, now the book-keeping */
+       s->s3->rbuf.offset = off + n;
+       s->s3->rbuf.left = newb - n;
+       s->packet_length += n;
        return(n);
        }
 
@@ -176,8 +216,8 @@ static int ssl3_read_n(SSL *s, int n, int max, int extend)
  * It will return <= 0 if more data is needed, normally due to an error
  * or non-blocking IO.
  * When it finishes, one packet has been decoded and can be found in
- * ssl->s3->rrec.type  - is the type of record
- * ssl->s3->rrec.data,         - data
+ * ssl->s3->rrec.type    - is the type of record
+ * ssl->s3->rrec.data,          - data
  * ssl->s3->rrec.length, - number of bytes
  */
 static int ssl3_get_record(SSL *s)
@@ -262,7 +302,6 @@ again:
                if (rr->length > (s->packet_length-SSL3_RT_HEADER_LENGTH))
                        {
                        i=rr->length;
-                       /*-(s->packet_length-SSL3_RT_HEADER_LENGTH); */
                        n=ssl3_read_n(s,i,i,1);
                        if (n <= 0) return(n); /* error or non-blocking io */
                        }
@@ -645,7 +684,7 @@ int ssl3_read_bytes(SSL *s, int type, unsigned char *buf, int len)
        void (*cb)()=NULL;
        BIO *bio;
 
-       if (s->s3->rbuf.buf == NULL) /* Not initialize yet */
+       if (s->s3->rbuf.buf == NULL) /* Not initialized yet */
                if (!ssl3_setup_buffers(s))
                        return(-1);
 
@@ -662,10 +701,10 @@ int ssl3_read_bytes(SSL *s, int type, unsigned char *buf, int len)
 start:
        s->rwstate=SSL_NOTHING;
 
-       /* s->s3->rrec.type     - is the type of record
-        * s->s3->rrec.data,    - data
-        * s->s3->rrec.off,     - ofset into 'data' for next read
-        * s->s3->rrec.length,  - number of bytes. */
+       /* s->s3->rrec.type         - is the type of record
+        * s->s3->rrec.data,    - data
+        * s->s3->rrec.off,     - offset into 'data' for next read
+        * s->s3->rrec.length,  - number of bytes. */
        rr= &(s->s3->rrec);
 
        /* get new packet */
@@ -692,16 +731,16 @@ start:
                return(0);
                }
 
-       /* Check for an incoming 'Client Request' message */
+       /* Check for an incoming 'Hello Request' message from client */
        if ((rr->type == SSL3_RT_HANDSHAKE) && (rr->length == 4) &&
-               (rr->data[0] == SSL3_MT_CLIENT_REQUEST) &&
+               (rr->data[0] == SSL3_MT_HELLO_REQUEST) &&
                (s->session != NULL) && (s->session->cipher != NULL))
                {
                if ((rr->data[1] != 0) || (rr->data[2] != 0) ||
                        (rr->data[3] != 0))
                        {
                        al=SSL_AD_DECODE_ERROR;
-                       SSLerr(SSL_F_SSL3_READ_BYTES,SSL_R_BAD_CLIENT_REQUEST);
+                       SSLerr(SSL_F_SSL3_READ_BYTES,SSL_R_BAD_HELLO_REQUEST);
                        goto err;
                        }
 
@@ -1038,4 +1077,3 @@ int ssl3_dispatch_alert(SSL *s)
                }
        return(i);
        }
-
index 85b0f5e9ef5fb54592abc5feea367a20bc27f12a..5ba3a28e63e1ea80ff371885495dc894e02dbcd1 100644 (file)
@@ -503,7 +503,7 @@ static int ssl3_send_hello_request(SSL *s)
        if (s->state == SSL3_ST_SW_HELLO_REQ_A)
                {
                p=(unsigned char *)s->init_buf->data;
-               *(p++)=SSL3_MT_CLIENT_REQUEST;
+               *(p++)=SSL3_MT_HELLO_REQUEST;
                *(p++)=0;
                *(p++)=0;
                *(p++)=0;
index 636b8bc2a2a82caced6c1c284c27a11e7978b226..94a06572a2740f517a915d7e29d9445cfec092a4 100644 (file)
--- a/ssl/ssl.h
+++ b/ssl/ssl.h
@@ -441,7 +441,7 @@ struct ssl_ctx_st
        /* get client cert callback */
 /**/   int (*client_cert_cb)(/* SSL *ssl, X509 **x509, EVP_PKEY **pkey */);
 
-       /* what we put in client requests */
+       /* what we put in client cert requests */
        STACK_OF(X509_NAME) *client_CA;
 
 /**/   int quiet_shutdown;
@@ -575,7 +575,8 @@ struct ssl_st
        struct ssl2_ctx_st *s2; /* SSLv2 variables */
        struct ssl3_ctx_st *s3; /* SSLv3 variables */
 
-       int read_ahead;         /* Read as many input bytes as possible */
+       int read_ahead;         /* Read as many input bytes as possible
+                                * (for non-blocking reads) */
        int hit;                /* reusing a previous session */
 
        int purpose;            /* Purpose setting */
@@ -1310,7 +1311,7 @@ int SSL_COMP_add_compression_method(int id,char *cm);
 #define SSL_R_BAD_AUTHENTICATION_TYPE                   102
 #define SSL_R_BAD_CHANGE_CIPHER_SPEC                    103
 #define SSL_R_BAD_CHECKSUM                              104
-#define SSL_R_BAD_CLIENT_REQUEST                        105
+#define SSL_R_BAD_HELLO_REQUEST                                 105
 #define SSL_R_BAD_DATA_RETURNED_BY_CALLBACK             106
 #define SSL_R_BAD_DECOMPRESSION                                 107
 #define SSL_R_BAD_DH_G_LENGTH                           108
index 322acaf0b6dd2a642c507e2f8218a426883f7af5..da47377e5785a8f8be6c7fd32b6b75bef4e1e465 100644 (file)
@@ -158,22 +158,6 @@ extern "C" {
 #define SSL3_RT_MAX_PACKET_SIZE                (SSL3_RT_MAX_ENCRYPTED_LENGTH+SSL3_RT_HEADER_LENGTH)
 #define SSL3_RT_MAX_DATA_SIZE                  (1024*1024)
 
-/* the states that a SSL3_RECORD can be in
- * For SSL_read it goes
- * rbuf->ENCODED       -> read 
- * ENCODED             -> we need to decode everything - call decode_record
- */
-#define SSL3_RS_BLANK                  1
-#define SSL3_RS_DATA
-
-#define SSL3_RS_ENCODED                        2
-#define SSL3_RS_READ_MORE              3
-#define SSL3_RS_WRITE_MORE
-#define SSL3_RS_PLAIN                  3
-#define SSL3_RS_PART_READ              4
-#define SSL3_RS_PART_WRITE             5
-
 #define SSL3_MD_CLIENT_FINISHED_CONST  "\x43\x4C\x4E\x54"
 #define SSL3_MD_SERVER_FINISHED_CONST  "\x53\x52\x56\x52"
 
@@ -205,7 +189,6 @@ extern "C" {
 typedef struct ssl3_record_st
        {
 /*r */ int type;               /* type of record */
-/*  */ /*int state;*/          /* any data in it? */
 /*rw*/ unsigned int length;    /* How many bytes available */
 /*r */ unsigned int off;       /* read/write offset into 'buf' */
 /*rw*/ unsigned char *data;    /* pointer to the record data */
@@ -215,11 +198,10 @@ typedef struct ssl3_record_st
 
 typedef struct ssl3_buffer_st
        {
-/*r */ int total;              /* used in non-blocking writes */
-/*r */ int wanted;             /* how many more bytes we need */
-/*rw*/ int left;               /* how many bytes left */
-/*rw*/ int offset;             /* where to 'copy from' */
-/*rw*/ unsigned char *buf;     /* SSL3_RT_MAX_PACKET_SIZE bytes */
+       unsigned char *buf;     /* SSL3_RT_MAX_PACKET_SIZE bytes (more if
+                                * SSL_OP_MICROSOFT_BIG_SSLV3_BUFFER is set) */
+       int offset;             /* where to 'copy from' */
+       int left;               /* how many bytes left */
        } SSL3_BUFFER;
 
 #define SSL3_CT_RSA_SIGN                       1
@@ -436,7 +418,7 @@ typedef struct ssl3_ctx_st
 #define SSL3_ST_SW_FINISHED_A          (0x1E0|SSL_ST_ACCEPT)
 #define SSL3_ST_SW_FINISHED_B          (0x1E1|SSL_ST_ACCEPT)
 
-#define SSL3_MT_CLIENT_REQUEST                 0
+#define SSL3_MT_HELLO_REQUEST                  0
 #define SSL3_MT_CLIENT_HELLO                   1
 #define SSL3_MT_SERVER_HELLO                   2
 #define SSL3_MT_CERTIFICATE                    11
index 7c611650f33591e82908249227a696c2dc0cd80a..9dd483d1121b64be9fd2f6e967e1ed6530076b20 100644 (file)
@@ -205,7 +205,7 @@ static ERR_STRING_DATA SSL_str_reasons[]=
 {SSL_R_BAD_AUTHENTICATION_TYPE           ,"bad authentication type"},
 {SSL_R_BAD_CHANGE_CIPHER_SPEC            ,"bad change cipher spec"},
 {SSL_R_BAD_CHECKSUM                      ,"bad checksum"},
-{SSL_R_BAD_CLIENT_REQUEST                ,"bad client request"},
+{SSL_R_BAD_HELLO_REQUEST                 ,"bad hello request"},
 {SSL_R_BAD_DATA_RETURNED_BY_CALLBACK     ,"bad data returned by callback"},
 {SSL_R_BAD_DECOMPRESSION                 ,"bad decompression"},
 {SSL_R_BAD_DH_G_LENGTH                   ,"bad dh g length"},