Fix some bugs and document others
authorBodo Möller <bodo@openssl.org>
Mon, 21 Feb 2000 17:09:54 +0000 (17:09 +0000)
committerBodo Möller <bodo@openssl.org>
Mon, 21 Feb 2000 17:09:54 +0000 (17:09 +0000)
apps/s_server.c
ssl/s3_both.c
ssl/s3_lib.c
ssl/s3_pkt.c
ssl/s3_srvr.c
ssl/ssl_lib.c
ssl/ssl_locl.h

index a92db5c..af19b89 100644 (file)
@@ -813,33 +813,47 @@ static int sv_body(char *hostname, int s, unsigned char *context)
        width=s+1;
        for (;;)
                {
-               FD_ZERO(&readfds);
+               int read_from_terminal;
+               int read_from_sslcon;
+
+               read_from_terminal = 0;
+               read_from_sslcon = SSL_pending(con);
+
+               if (!read_from_sslcon)
+                       {
+                       FD_ZERO(&readfds);
 #ifndef WINDOWS
-               FD_SET(fileno(stdin),&readfds);
+                       FD_SET(fileno(stdin),&readfds);
 #endif
-               FD_SET(s,&readfds);
-               /* Note: under VMS with SOCKETSHR the second parameter is
-                * currently of type (int *) whereas under other systems
-                * it is (void *) if you don't have a cast it will choke
-                * the compiler: if you do have a cast then you can either
-                * go for (int *) or (void *).
-                */
+                       FD_SET(s,&readfds);
+                       /* Note: under VMS with SOCKETSHR the second parameter is
+                        * currently of type (int *) whereas under other systems
+                        * it is (void *) if you don't have a cast it will choke
+                        * the compiler: if you do have a cast then you can either
+                        * go for (int *) or (void *).
+                        */
 #ifdef WINDOWS
-               /* Under Windows we can't select on stdin: only
-                * on sockets. As a workaround we timeout the select every
-                * second and check for any keypress. In a proper Windows
-                * application we wouldn't do this because it is inefficient.
-                */
-               tv.tv_sec = 1;
-               tv.tv_usec = 0;
-               i=select(width,(void *)&readfds,NULL,NULL,&tv);
-               if((i < 0) || (!i && !_kbhit() ) )continue;
-               if(_kbhit())
+                       /* Under Windows we can't select on stdin: only
+                        * on sockets. As a workaround we timeout the select every
+                        * second and check for any keypress. In a proper Windows
+                        * application we wouldn't do this because it is inefficient.
+                        */
+                       tv.tv_sec = 1;
+                       tv.tv_usec = 0;
+                       i=select(width,(void *)&readfds,NULL,NULL,&tv);
+                       if((i < 0) || (!i && !_kbhit() ) )continue;
+                       if(_kbhit())
+                               read_from_terminal = 1;
 #else
-               i=select(width,(void *)&readfds,NULL,NULL,NULL);
-               if (i <= 0) continue;
-               if (FD_ISSET(fileno(stdin),&readfds))
+                       i=select(width,(void *)&readfds,NULL,NULL,NULL);
+                       if (i <= 0) continue;
+                       if (FD_ISSET(fileno(stdin),&readfds))
+                               read_from_terminal = 1;
 #endif
+                       if (FD_ISSET(s,&readfds))
+                               read_from_sslcon = 1;
+                       }
+               if (read_from_terminal)
                        {
                        if (s_crlf)
                                {
@@ -952,7 +966,7 @@ static int sv_body(char *hostname, int s, unsigned char *context)
                                if (i <= 0) break;
                                }
                        }
-               if (FD_ISSET(s,&readfds))
+               if (read_from_sslcon)
                        {
                        if (!SSL_is_init_finished(con))
                                {
index f1a9282..035a937 100644 (file)
@@ -342,14 +342,15 @@ long ssl3_get_message(SSL *s, int st1, int stn, int mt, long max, int *ok)
                        SSLerr(SSL_F_SSL3_GET_MESSAGE,SSL_R_UNEXPECTED_MESSAGE);
                        goto f_err;
                        }
-               if((mt < 0) && (*p == SSL3_MT_CLIENT_HELLO) &&
+               if ((mt < 0) && (*p == SSL3_MT_CLIENT_HELLO) &&
                                        (st1 == SSL3_ST_SR_CERT_A) &&
                                        (stn == SSL3_ST_SR_CERT_B))
                        {
                        /* At this point we have got an MS SGC second client
                         * hello (maybe we should always allow the client to
                         * start a new handshake?). We need to restart the mac.
-                        */
+                        * Don't increment {num,total}_renegotiations because
+                        * we have not completed the handshake. */
                        ssl3_init_finished_mac(s);
                        }
 
index 87525fa..c4b49aa 100644 (file)
@@ -695,6 +695,10 @@ void ssl3_clear(SSL *s)
                Free(s->s3->rrec.comp);
                s->s3->rrec.comp=NULL;
                }
+#ifndef NO_DH
+       if (s->s3->tmp.dh != NULL)
+               DH_free(s->s3->tmp.dh);
+#endif
 
        rp=s->s3->rbuf.buf;
        wp=s->s3->wbuf.buf;
index e95dcd9..32eda4e 100644 (file)
@@ -709,7 +709,6 @@ int ssl3_read_bytes(SSL *s, int type, unsigned char *buf, int len)
        int al,i,j,n,ret;
        SSL3_RECORD *rr;
        void (*cb)()=NULL;
-       BIO *bio;
 
        if (s->s3->rbuf.buf == NULL) /* Not initialized yet */
                if (!ssl3_setup_buffers(s))
@@ -988,9 +987,15 @@ start:
                if (((s->state&SSL_ST_MASK) == SSL_ST_OK) &&
                        !(s->s3->flags & SSL3_FLAGS_NO_RENEGOTIATE_CIPHERS))
                        {
+#if 0 /* worked only because C operator preferences are not as expected (and
+       * because this is not really needed for clients except for detecting
+       * protocol violations): */
                        s->state=SSL_ST_BEFORE|(s->server)
                                ?SSL_ST_ACCEPT
                                :SSL_ST_CONNECT;
+#else
+                       s->state = s->server ? SSL_ST_ACCEPT : SSL_ST_CONNECT;
+#endif
                        s->new_session=1;
                        }
                n=s->handshake_func(s);
@@ -1001,11 +1006,20 @@ start:
                        return(-1);
                        }
 
+#if 1 /* probably nonsense (does not work with readahead),
+          * but keep it for now anyway ... s_server relies on this */
+               {
+               BIO *bio;
                /* In the case where we try to read application data
                 * the first time, but we trigger an SSL handshake, we
                 * return -1 with the retry option set.  I do this
                 * otherwise renegotiation can cause nasty problems 
-                * in the non-blocking world */
+                * in the non-blocking world */ /* That's "non-non-blocking",
+                                                * I guess? When receiving a
+                                                * Hello Request, we have the
+                                                * same problem (e.g. in s_client),
+                                                * but it's really an application bug.
+                                                */
 
                s->rwstate=SSL_READING;
                bio=SSL_get_rbio(s);
@@ -1013,6 +1027,10 @@ start:
                BIO_set_retry_read(bio);
                return(-1);
                }
+#else
+               goto start;
+#endif         
+               }
 
        switch (rr->type)
                {
@@ -1041,7 +1059,7 @@ start:
                 * but have application data.  If the library was
                 * running inside ssl3_read() (i.e. in_read_app_data
                 * is set) and it makes sense to read application data
-                * at this point (session renegotation not yet started),
+                * at this point (session renegotiation not yet started),
                 * we will indulge it.
                 */
                if (s->s3->in_read_app_data &&
index b5882d5..2a9e115 100644 (file)
@@ -77,9 +77,9 @@ static int ssl3_send_server_hello(SSL *s);
 static int ssl3_send_server_key_exchange(SSL *s);
 static int ssl3_send_certificate_request(SSL *s);
 static int ssl3_send_server_done(SSL *s);
-static int ssl3_get_cert_verify(SSL *s);
 static int ssl3_get_client_key_exchange(SSL *s);
 static int ssl3_get_client_certificate(SSL *s);
+static int ssl3_get_cert_verify(SSL *s);
 static int ssl3_send_hello_request(SSL *s);
 
 static SSL_METHOD *ssl3_get_server_method(int ver)
@@ -154,7 +154,6 @@ int ssl3_accept(SSL *s)
 
                        if ((s->version>>8) != 3)
                                abort();
-                       /* s->version=SSL3_VERSION; */
                        s->type=SSL_ST_ACCEPT;
 
                        if (s->init_buf == NULL)
@@ -539,7 +538,21 @@ static int ssl3_check_client_hello(SSL *s)
                &ok);
        if (!ok) return((int)n);
        s->s3->tmp.reuse_message = 1;
-       if(s->s3->tmp.message_type == SSL3_MT_CLIENT_HELLO) return 2;
+       if (s->s3->tmp.message_type == SSL3_MT_CLIENT_HELLO)
+               {
+               /* Throw away what we have done so far in the current handshake,
+                * which will now be aborted. (A full SSL_clear would be too much.)
+                * I hope that tmp.dh is the only thing that may need to be cleared
+                * when a handshake is not completed ... */
+#ifndef NO_DH
+               if (s->s3->tmp.dh != NULL)
+                       {
+                       DH_free(s->s3->tmp.dh);
+                       s->s3->tmp.dh = NULL;
+                       }
+#endif
+               return 2;
+               }
        return 1;
 }
 
@@ -1300,31 +1313,6 @@ static int ssl3_get_client_key_exchange(SSL *s)
 
                i=RSA_private_decrypt((int)n,p,p,rsa,RSA_PKCS1_PADDING);
 
-#if 0
-               /* If a bad decrypt, use a random master key */
-               if ((i != SSL_MAX_MASTER_KEY_LENGTH) ||
-                       ((p[0] != (s->client_version>>8)) ||
-                        (p[1] != (s->client_version & 0xff))))
-                       {
-                       int bad=1;
-
-                       if ((i == SSL_MAX_MASTER_KEY_LENGTH) &&
-                               (p[0] == (s->version>>8)) &&
-                               (p[1] == 0))
-                               {
-                               if (s->options & SSL_OP_TLS_ROLLBACK_BUG)
-                                       bad=0;
-                               }
-                       if (bad)
-                               {
-                               p[0]=(s->version>>8);
-                               p[1]=(s->version & 0xff);
-                               RAND_pseudo_bytes(&(p[2]),SSL_MAX_MASTER_KEY_LENGTH-2);
-                               i=SSL_MAX_MASTER_KEY_LENGTH;
-                               }
-                       /* else, an SSLeay bug, ssl only server, tls client */
-                       }
-#else
                if (i != SSL_MAX_MASTER_KEY_LENGTH)
                        {
                        al=SSL_AD_DECODE_ERROR;
@@ -1347,7 +1335,6 @@ static int ssl3_get_client_key_exchange(SSL *s)
                memset(p,0,i);
                }
        else
-#endif
 #ifndef NO_DH
                if (l & (SSL_kEDH|SSL_kDHr|SSL_kDHd))
                {
index 8a9d289..f09e46c 100644 (file)
@@ -107,10 +107,17 @@ int SSL_clear(SSL *s)
        s->hit=0;
        s->shutdown=0;
 
-#if 0
+#if 0 /* Disabled since version 1.10 of this file (early return not
+       * needed because SSL_clear is not called when doing renegotiation) */
        /* This is set if we are doing dynamic renegotiation so keep
         * the old cipher.  It is sort of a SSL_clear_lite :-) */
        if (s->new_session) return(1);
+#else
+       if (s->new_session)
+               {
+               SSLerr(SSL_F_SSL_CLEAR,SSL_R_INTERNAL_ERROR);
+               return 0;
+               }
 #endif
 
        state=s->state; /* Keep to check if we throw away the session-id */
index 0f81902..9a52bab 100644 (file)
@@ -306,8 +306,8 @@ typedef struct cert_st
        {
        /* Current active set */
        CERT_PKEY *key; /* ALWAYS points to an element of the pkeys array
-                                        * Probably it would make more sense to store
-                                        * an index, not a pointer. */
+                        * Probably it would make more sense to store
+                        * an index, not a pointer. */
  
        /* The following masks are for the key and auth
         * algorithms that are supported by the certs below */