Stunnel stall and SSL_want_*, SSL_pending?

I've been away from the stunnel stall problem for a couple of days,
but here is another attempt at fixing the stunnel transfer loop, after
a look at Ben Laurie's recent example code in state_machine.c. I have
attempted to preserve the different structure of stunnel and
incorporate the IO logic of the example.

Since I am not familiar with the innards of OpenSSL and since I
haven't found descriptions of the relevant OpenSSL functions, this is
unfortunately based on assumptions derived from the names of functions
and some peeks at the code. I was aiming to minimise the work needed
to fix stunnel, so I may well have formulated unwarranted assumptions
;-) I'll try to summarise the most important ones, and hope that some
expert can find some time to have a look. Please point out mistakes
since the proposed fix depends on them. Actual code for the new
stunnel transfer routine is below. (I've chosen to quote it
completely, not just the differences, to facilitate analysis. My
apologies if it is considered inappropriate to quote about 200 lines
of code.)

Note: stunnel uses SSL_set_fd to supply a socket to the SSL library
for reading and writing on the SSL connection.

Major assumptions:

    SSL_want_read and SSL_want_write can be used to see if a previous
    operation was not completed and what kind of IO on the underlying
    file descriptor is needed to continue.

    The SSL library does NOT require an identical retry as the next
    SSL IO operation. If an SSL_read was not completed, one can also
    retry with a larger count, or even do an SSL_write first, then
    retry SSL_read again.  If an SSL_write was not completed, one can
    also retry with a larger count, or even do an SSL_read first, then
    retry SSL_write again.

    SSL_pending can be used to see if the SSL library has buffered
    application data available, already read and decrypted from the
    underlying socket.

With these assumptions the proposed stunnel SSL IO behaviour is as
follows, in each iteration of the select loop:

    The SSL file descriptor is selected for read if

          stunnel has buffer space to read
      or  stunnel has data to write to SSL and SSL_want_read is true

    The SSL file descriptor is selected for write if

          stunnel has data to write to SSL
      or  stunnel has buffer space to read and SSL_want_write is true 

    After the select, SSL_read and SSL_write are done as follows:

        attempt SSL_write if stunnel has data to write (maybe
            increased from previous amount) and

         either  SSL file descriptor ready for write
           or    SSL_want_read and SSL file descriptor ready for read

        attempt SSL_read if stunnel has buffer space to read and:

         either  SSL file descriptor ready for read
           or    just made room in read buffer and SSL_pending
           or    SSL_want_write and SSL file descriptor ready for write


By the way, the code runs my applications (on Solaris 2.5.1 and NT 4)
just as well as the previous fix. It only attempts to take the
possibilities of SSL_want_* into account. I guess it isn't easy to
create conditions where the new code is actually relevant.

Regards,

Peter Wagemans

------------------------------------------------------------------------

static int transfer(SSL *ssl, int sock_fd) /* transfer data */
{
    fd_set rd_set, wr_set;
    int num, fdno, ssl_fd, ssl_bytes, sock_bytes, retval;
    char sock_buff[BUFFSIZE], ssl_buff[BUFFSIZE];
    int sock_ptr, ssl_ptr, sock_open, ssl_open;
#if defined FIONBIO && defined USE_NBIO
    unsigned long l;
#endif

    int check_SSL_pending;

    ssl_fd=SSL_get_fd(ssl);
    fdno=(ssl_fd>sock_fd ? ssl_fd : sock_fd)+1;
    sock_ptr=0;
    ssl_ptr=0;
    sock_open=1;
    ssl_open=1;
    sock_bytes=0;
    ssl_bytes=0;

#if defined FIONBIO && defined USE_NBIO
    l=1; /* ON */
    if(ioctlsocket(sock_fd, FIONBIO, &l)<0)
        sockerror("ioctlsocket (sock)"); /* non-critical */
    if(ioctlsocket(ssl_fd, FIONBIO, &l)<0)
        sockerror("ioctlsocket (ssl)"); /* non-critical */
    log(LOG_DEBUG, "Sockets set to non-blocking mode");
#endif

    while((sock_open||sock_ptr) && (ssl_open||ssl_ptr)) {

        FD_ZERO(&rd_set);

        if(sock_open && sock_ptr<BUFFSIZE) /* can read from socket */
            FD_SET(sock_fd, &rd_set);

        if (   ssl_open
            && (    (ssl_ptr<BUFFSIZE) /* I want to read from SSL */
                || (sock_ptr && SSL_want_read(ssl) )
                  /* I want to SSL_write but read from the underlying
                   * socket needed for the SSL protocol. */
               )
           ) {
          FD_SET(ssl_fd, &rd_set);
        }

        FD_ZERO(&wr_set);

        if(sock_open && ssl_ptr) /* can write to socket */
            FD_SET(sock_fd, &wr_set);

        if (   ssl_open
            && (   (sock_ptr) /* can write to SSL */
                || ( (ssl_ptr<BUFFSIZE) && SSL_want_write( ssl ) )
                   /* I want to SSL_read but write to the underlying
                    * socket needed for the SSL protocol. */
               )
           ) {
          FD_SET(ssl_fd, &wr_set);
        }

        if(select(fdno, &rd_set, &wr_set, NULL, NULL)<0) {
            sockerror("select");
            goto error;
        }

        /* Set flag to try and read any buffered SSL data if we made
         * room in the buffer by writing to the socket. */

        check_SSL_pending = 0;

        if(sock_open && FD_ISSET(sock_fd, &wr_set)) {
            num=writesocket(sock_fd, ssl_buff, ssl_ptr);
            if(num<0) {
                sockerror("write");
                goto error;
            }
            if(num) {
                memcpy(ssl_buff, ssl_buff+num, ssl_ptr-num);

                if (ssl_ptr ==BUFFSIZE) check_SSL_pending = 1;

                ssl_ptr-=num;
                sock_bytes+=num;
            } else { /* close */
                log(LOG_DEBUG, "Socket closed on write");
                sock_open=0;
            }
        }


        if (   ssl_open
            && sock_ptr
            && (   FD_ISSET(ssl_fd, &wr_set)
                  /* See if application data can be written. */

                || ( SSL_want_read(ssl) && FD_ISSET(ssl_fd, &rd_set) )
                   /* I want to SSL_write but read from the underlying
                    * socket needed for the SSL protocol. */
               )
           ) {

            num=SSL_write(ssl, sock_buff, sock_ptr);

            switch(SSL_get_error(ssl, num)) {
            case SSL_ERROR_NONE:
                memcpy(sock_buff, sock_buff+num, sock_ptr-num);
                sock_ptr-=num;
                ssl_bytes+=num;
                break;
            case SSL_ERROR_WANT_WRITE:
            case SSL_ERROR_WANT_READ:
            case SSL_ERROR_WANT_X509_LOOKUP:
                log(LOG_DEBUG, "SSL_write returned WANT_ - retry");
                break;
            case SSL_ERROR_SYSCALL:
                if(num) { /* not EOF */
                    sockerror("SSL_write (socket)");
                    goto error;
                }
            case SSL_ERROR_ZERO_RETURN:
                log(LOG_DEBUG, "SSL closed on write");
                ssl_open=0;
                break;
            case SSL_ERROR_SSL:
                sslerror("SSL_write");
                goto error;
            }
        }

        if(sock_open && FD_ISSET(sock_fd, &rd_set)) {
            num=readsocket(sock_fd, sock_buff+sock_ptr, BUFFSIZE-sock_ptr);

            if(num<0 && errno==ECONNRESET) {
                log(LOG_NOTICE, "IPC reset (child died)");
                break; /* close connection */
            }
            if (num < 0 && errno != EIO) {
                sockerror("read");
                goto error;
            } else if (num > 0) {
                sock_ptr += num;
            } else { /* close */
                log(LOG_DEBUG, "Socket closed on read");
                sock_open = 0;
            }
        }


        if(   ssl_open

           && (ssl_ptr<BUFFSIZE)

           && (   FD_ISSET(ssl_fd, &rd_set)
                  /* See if there's any application data coming in. */

               || ( SSL_want_write( ssl ) && FD_ISSET(ssl_fd, &wr_set) )
                  /* I want to SSL_read but write to the underlying
                   * socket needed for the SSL protocol. */

               || ( check_SSL_pending && SSL_pending(ssl) )
                  /* Write made space from full buffer. */
              )
          ) {

            num=SSL_read(ssl, ssl_buff+ssl_ptr, BUFFSIZE-ssl_ptr);

            switch(SSL_get_error(ssl, num)) {
            case SSL_ERROR_NONE:
                ssl_ptr+=num;
                break;
            case SSL_ERROR_WANT_WRITE:
            case SSL_ERROR_WANT_READ:
            case SSL_ERROR_WANT_X509_LOOKUP:
                log(LOG_DEBUG, "SSL_read returned WANT_ - retry");
                break;
            case SSL_ERROR_SYSCALL:
                if(num) { /* not EOF */
                    sockerror("SSL_read (socket)");
                    goto error;
                }
            case SSL_ERROR_ZERO_RETURN:
                log(LOG_DEBUG, "SSL closed on read");
                ssl_open=0;
                break;
            case SSL_ERROR_SSL:
                sslerror("SSL_read");
                goto error;
            }
        }
    }
    retval=0;
    goto done;
error:
    retval=-1;
done:

#if defined FIONBIO && defined USE_NBIO
    l=0; /* OFF */
    if(ioctlsocket(sock_fd, FIONBIO, &l)<0)
        sockerror("ioctlsocket (sock)"); /* non-critical */
    if(ioctlsocket(ssl_fd, FIONBIO, &l)<0)
        sockerror("ioctlsocket (ssl)"); /* non-critical */
#endif

    log(LOG_NOTICE,
        "Connection %s: %d bytes sent to SSL, %d bytes sent to socket",
        retval<0 ? "reset" : "closed", ssl_bytes, sock_bytes);
    return retval;
}
------------------------------------------------------------------------
______________________________________________________________________
OpenSSL Project                                 http://www.openssl.org
User Support Mailing List                    [EMAIL PROTECTED]
Automated List Manager                           [EMAIL PROTECTED]

Reply via email to