Fix SSL_peek and SSL_pending.
authorBodo Möller <bodo@openssl.org>
Mon, 25 Dec 2000 18:40:46 +0000 (18:40 +0000)
committerBodo Möller <bodo@openssl.org>
Mon, 25 Dec 2000 18:40:46 +0000 (18:40 +0000)
CHANGES
doc/ssl/SSL_get_error.pod
doc/ssl/SSL_pending.pod
ssl/s2_lib.c
ssl/s2_pkt.c
ssl/s3_lib.c
ssl/s3_pkt.c

diff --git a/CHANGES b/CHANGES
index 167048fa6ad037ef03a2ab91061c79012572b735..ccb179ed8337e78d343f995ab7be8c47ea14a14a 100644 (file)
--- a/CHANGES
+++ b/CHANGES
      512 bits], about 30% for larger ones [1024 or 2048 bits].)
      [Bodo Moeller]
 
-  *) Disable ssl2_peek and ssl3_peek (i.e., both implementations
-     of SSL_peek) because they both are completely broken.
-     For fixing this, the internal read functions now have an additional
-     'peek' parameter, but the actual peek functionality has not
-     yet been implemented.
+  *) Fix ssl3_pending: If the record in s->s3->rrec is not of type
+     SSL3_RT_APPLICATION_DATA, return 0.
+     [Bodo Moeller]
+
+  *) Fix SSL_peek:
+     Both ssl2_peek and ssl3_peek, which were totally broken in earlier
+     releases, have been re-implemented by renaming the previous
+     implementations of ssl2_read and ssl3_read to ssl2_read_internal
+     and ssl3_read_internal, respectively, and adding 'peek' parameters
+     to them.  The new ssl[23]_{read,peek} functions are calls to
+     ssl[23]_read_internal with the 'peek' flag set appropriately.
+     A 'peek' parameter has also been added to ssl3_read_bytes, which
+     does the actual work for ssl3_read_internal.
      [Bodo Moeller]
 
   *) New function BN_kronecker.
index bd6872f61f54921f97325aa17911b8479e8a4fcf..fefaf619369a214766ed51cf8dccc0098faaf4ab 100644 (file)
@@ -14,8 +14,8 @@ SSL_get_error - obtain result code for TLS/SSL I/O operation
 
 SSL_get_error() returns a result code (suitable for the C "switch"
 statement) for a preceding call to SSL_connect(), SSL_accept(),
-SSL_read(), or SSL_write() on B<ssl>.  The value returned by that
-TLS/SSL I/O function must be passed to SSL_get_error() in parameter
+SSL_read(), SSL_peek(), or SSL_write() on B<ssl>.  The value returned by
+that TLS/SSL I/O function must be passed to SSL_get_error() in parameter
 B<ret>.
 
 In addition to B<ssl> and B<ret>, SSL_get_error() inspects the
@@ -64,10 +64,10 @@ TLS/SSL I/O function should be retried.
 
 Caveat: Any TLS/SSL I/O function can lead to either of
 B<SSL_ERROR_WANT_READ> and B<SSL_ERROR_WANT_WRITE>.  In particular,
-SSL_read() may want to write data and SSL_write() may want to read
-data.  This is mainly because TLS/SSL handshakes may occur at any time
-during the protocol (initiated by either the client or the server);
-SSL_read() and SSL_write() will handle any pending handshakes.
+SSL_read() or SSL_peek() may want to write data and SSL_write() may want
+to read data.  This is mainly because TLS/SSL handshakes may occur at any
+time during the protocol (initiated by either the client or the server);
+SSL_read(), SSL_peek(), and SSL_write() will handle any pending handshakes.
 
 =item SSL_ERROR_WANT_X509_LOOKUP
 
index 6c03b14caf1f4072ff2110a4ba85e7bc2768eaa6..b4c48598b25dcc08a04b6eb5d457c603f8a681fb 100644 (file)
@@ -33,8 +33,8 @@ I<read_ahead> flag is set, additional protocol bytes may have been
 read containing more TLS/SSL records; these are ignored by
 SSL_pending().
 
-SSL_pending() does not check if the record type of pending data is
-application data.
+Up to OpenSSL 0.9.6, SSL_pending() does not check if the record type
+of pending data is application data.
 
 =head1 SEE ALSO
 
index 52a2b279638940b2192ad700f0833e6fe543a9ce..a89958607cfd2800e03f5d0e90535891fa0ac704 100644 (file)
@@ -260,9 +260,6 @@ SSL_CIPHER *ssl2_get_cipher(unsigned int u)
 
 int ssl2_pending(SSL *s)
        {
-       /* Unlike ssl2_pending, this one probably works (if read-ahead
-        * is disabled), but it should be examined
-        * XXX */
        return(s->s2->ract_data_length);
        }
 
index 2866d61fa40c1c5c371da3e5373bcf1c36d34adc..0ec9ee33932fc516b6c88f734331613fe5c91b24 100644 (file)
@@ -138,7 +138,7 @@ static int ssl2_read_internal(SSL *s, void *buf, int len, int peek)
                return -1;
                }
 
-ssl2_read_again:
+ ssl2_read_again:
        if (SSL_in_init(s) && !s->in_handshake)
                {
                n=s->handshake_func(s);
@@ -162,13 +162,22 @@ ssl2_read_again:
                        n=len;
 
                memcpy(buf,s->s2->ract_data,(unsigned int)n);
-               s->s2->ract_data_length-=n;
-               s->s2->ract_data+=n;
-               if (s->s2->ract_data_length == 0)
-                       s->rstate=SSL_ST_READ_HEADER;
+               if (!peek)
+                       {
+                       s->s2->ract_data_length-=n;
+                       s->s2->ract_data+=n;
+                       if (s->s2->ract_data_length == 0)
+                               s->rstate=SSL_ST_READ_HEADER;
+                       }
+
                return(n);
                }
 
+       /* s->s2->ract_data_length == 0
+        * 
+        * Fill the buffer, then goto ssl2_read_again.
+        */
+
        if (s->rstate == SSL_ST_READ_HEADER)
                {
                if (s->first_packet)
@@ -266,33 +275,24 @@ ssl2_read_again:
                INC32(s->s2->read_sequence); /* expect next number */
                /* s->s2->ract_data is now available for processing */
 
-#if 1
-               /* How should we react when a packet containing 0
-                * bytes is received?  (Note that SSLeay/OpenSSL itself
-                * never sends such packets; see ssl2_write.)
-                * Returning 0 would be interpreted by the caller as
-                * indicating EOF, so it's not a good idea.
-                * Instead, we just continue reading.  Note that using
-                * select() for blocking sockets *never* guarantees
+               /* Possibly the packet that we just read had 0 actual data bytes.
+                * (SSLeay/OpenSSL itself never sends such packets; see ssl2_write.)
+                * In this case, returning 0 would be interpreted by the caller
+                * as indicating EOF, so it's not a good idea.  Instead, we just
+                * continue reading; thus ssl2_read_internal may have to process
+                * multiple packets before it can return.
+                *
+                * [Note that using select() for blocking sockets *never* guarantees
                 * that the next SSL_read will not block -- the available
-                * data may contain incomplete packets, and except for SSL 2
-                * renegotiation can confuse things even more. */
+                * data may contain incomplete packets, and except for SSL 2,
+                * renegotiation can confuse things even more.] */
 
                goto ssl2_read_again; /* This should really be
-                                      * "return ssl2_read(s,buf,len)",
-                                      * but that would allow for
-                                      * denial-of-service attacks if a
-                                      * C compiler is used that does not
-                                      * recognize end-recursion. */
-#else
-               /* If a 0 byte packet was sent, return 0, otherwise
-                * we play havoc with people using select with
-                * blocking sockets.  Let them handle a packet at a time,
-                * they should really be using non-blocking sockets. */
-               if (s->s2->ract_data_length == 0)
-                       return(0);
-               return(ssl2_read(s,buf,len));
-#endif
+                                      * "return ssl2_read(s,buf,len)",
+                                      * but that would allow for
+                                      * denial-of-service attacks if a
+                                      * C compiler is used that does not
+                                      * recognize end-recursion. */
                }
        else
                {
index f2bfb15a5a8ee3a4f72f16ac1c0ae5c6b5f5d091..47768cc281e8fdc6eb81fde8c5f3dc51d2f60947 100644 (file)
@@ -782,8 +782,7 @@ SSL_CIPHER *ssl3_get_cipher(unsigned int u)
 
 int ssl3_pending(SSL *s)
        {
-       /* The problem is that it may not be the correct record type */
-       return(s->s3->rrec.length); /* FIXME */
+       return (s->s3->rrec.type == SSL3_RT_APPLICATION_DATA) ? s->s3->rrec.length : 0;
        }
 
 int ssl3_new(SSL *s)
index 92d9a4ab1b4c166889bae564559e1ef94e3f5fc3..9ab76604a64534192a45569a6f90257487b2135a 100644 (file)
@@ -711,17 +711,12 @@ int ssl3_read_bytes(SSL *s, int type, unsigned char *buf, int len, int peek)
        SSL3_RECORD *rr;
        void (*cb)()=NULL;
 
-       if (peek)
-               {
-               SSLerr(SSL_F_SSL3_READ_BYTES, SSL_R_FIXME); /* proper implementation not yet completed */
-               return -1;
-               }
-
        if (s->s3->rbuf.buf == NULL) /* Not initialized yet */
                if (!ssl3_setup_buffers(s))
                        return(-1);
 
-       if ((type != SSL3_RT_APPLICATION_DATA) && (type != SSL3_RT_HANDSHAKE) && type)
+       if ((type && (type != SSL3_RT_APPLICATION_DATA) && (type != SSL3_RT_HANDSHAKE) && type) ||
+           (peek && (type != SSL3_RT_APPLICATION_DATA)))
                {
                SSLerr(SSL_F_SSL3_READ_BYTES, SSL_R_INTERNAL_ERROR);
                return -1;
@@ -734,6 +729,7 @@ int ssl3_read_bytes(SSL *s, int type, unsigned char *buf, int len, int peek)
                unsigned char *dst = buf;
                unsigned int k;
 
+               /* peek == 0 */
                n = 0;
                while ((len > 0) && (s->s3->handshake_fragment_len > 0))
                        {
@@ -769,7 +765,7 @@ start:
         * s->s3->rrec.length,  - number of bytes. */
        rr = &(s->s3->rrec);
 
-       /* get new packet */
+       /* get new packet if necessary */
        if ((rr->length == 0) || (s->rstate == SSL_ST_READ_BODY))
                {
                ret=ssl3_get_record(s);
@@ -787,7 +783,8 @@ start:
                goto err;
                }
 
-       /* If the other end has shutdown, throw anything we read away */
+       /* If the other end has shut down, throw anything we read away
+        * (even in 'peek' mode) */
        if (s->shutdown & SSL_RECEIVED_SHUTDOWN)
                {
                rr->length=0;
@@ -816,12 +813,15 @@ start:
                        n = (unsigned int)len;
 
                memcpy(buf,&(rr->data[rr->off]),n);
-               rr->length-=n;
-               rr->off+=n;
-               if (rr->length == 0)
+               if (!peek)
                        {
-                       s->rstate=SSL_ST_READ_HEADER;
-                       rr->off=0;
+                       rr->length-=n;
+                       rr->off+=n;
+                       if (rr->length == 0)
+                               {
+                               s->rstate=SSL_ST_READ_HEADER;
+                               rr->off=0;
+                               }
                        }
                return(n);
                }