>> Date: Mon, 9 Nov 2009 00:28:55 +0100
>> From: daniel_at_haxx.se
>> To: curl-library_at_cool.haxx.se
>> Subject: RE: Libcurl stops when sending lots of https messages
>>
>> On Fri, 6 Nov 2009, L S wrote:
>>
>>> if((k->keepon & KEEP_RECV) &&
>>> ((select_res & CURL_CSELECT_IN) || conn->bits.stream_was_rewound)) {
>>>
>>> result = readwrite_data(data, conn, k, &didwhat, done);
>>> if(result || *done)
>>> return result;
>>>
>>> The upper if statement isn't fulfilled after a SSL_ERROR_WANT_WRITE
>>
>> Why isn't it fulfilled anymore? I mean, what exactly makes this condition no
>> longer match after a SSL_ERROR_WANT_WRITE? I can't see any particular
>> treatment of SSL_ERROR_WANT_WRITE in the code so I don't see how your
>> described problem happens.
>
> The problem seems to be that the Curl_socket_ready(fd_read, fd_write, 0); invoked on line 1673 in file transfer.c returns 2 after the SSL_ERROR_WANT_WRITE error code. The reason that my simple fix workes for me is that this function isn't called at all if conn->cselect_bits is non zero. Se code below:
>
> int select_res = conn->cselect_bits;
> .
> .
> .
>
> if(!select_res) { /* Call for select()/poll() only, if read/write/error
> status is not known. */
> select_res = Curl_socket_ready(fd_read, fd_write, 0);
> }
>
>
>
> This return value tells libcurl to upload data - and therefore the readwrite_upload(data, conn, k, &didwhat) function is called insted of readwrite_data(data, conn, k, &didwhat, done). Se code sample (line 1684 in transfer.c):
>
>
> if((k->keepon & KEEP_RECV) &&
> ((select_res & CURL_CSELECT_IN) || conn->bits.stream_was_rewound)) {
>
> result = readwrite_data(data, conn, k, &didwhat, done);
> if(result || *done)
> return result;
> }
>
> /* If we still have writing to do, we check if we have a writable socket. */
> if((k->keepon & KEEP_SEND) && (select_res & CURL_CSELECT_OUT)) {
> /* write */
>
> result = readwrite_upload(data, conn, k, &didwhat);
> if(result)
> return result;
> }
>
> where CURL_CSELECT_IN = 0x01 and CURL_CSELECT_OUT = 0x02.
>
> During the next iterations of the the Curl_readwrite function Curl_socket_ready returns zero and none of the if statment abowe is satisfied.
>
>
>
>>
>>> If this is considered to be an bug and not a feature ( there might be some
>>> reason for this that isn't libcurls fault. There are winsock calls involved
>>> that I don't know what they do )
>>
>> Your described problem certainly sounds like a bug, and the fact that you can
>> make it work again even more suggests that this is a libcurl bug. To me the
>> question is mostly how the fix should be made.
>>
>>> then this little fix seems to work out for me. In file Transfer.c line 449:
>>
>>> conn->cselect_bits |= CURL_CSELECT_IN; //Remember that connection is of type
>>> CURL_CSELECT_IN
>>
>> That's unfortunately too naive. In fact, when OpenSSL has returned
>> SSL_ERROR_WANT_WRITE we shouldn't in fact wait for input anymore but we should
>> rather wait for the socket to become writable since OpenSSL wants to _write_
>> data at that point. We have however never addressed that problem and yet we've
>> done quite well, so I'm not sure we really need to care about that detail
>> right now.
>>
>
> Yes, that is correct. To quote the authors of o'reillys "Network security with openSSL" (which in no way wields the absolute truth):
>
> "Conceptually, SSL_read and SSL_write read and write data from the peer. Actually, a
> call to SSL_read may write data to the underlying I/O layer, and a call to SSL_write
> may read. This usually occurs during a renegotiation. Since a renegotiation may
> occur at any time, this behavior can cause unexpected results when using a blocking
> I/O layer; namely, an I/O call may require a retry. Thus, our implementation must
> handle this.
>
> In order to handle a blocking call correctly, we need to retry the SSL I/O operation if
> we receive SSL_ERROR_WANT_READ or SSL_ERROR_WANT_WRITE. Don’t let the names of
> these errors confuse you. Even though they tell us the SSL connection needs to wait
> until we can read or write, respectively, we just need to retry whatever operation
> caused the condition. For instance, if an SSL_read caused the SSL_ERROR_WANT_WRITE
> error, we must retry the SSL_read rather than making the call to SSL_write. It is worth
> taking a moment to understand the potential errors for a single I/O call. Though
> nonintuitive, a call to SSL_read may indeed return the error SSL_ERROR_WANT_WRITE
> due to the possibility of renegotiations at any point."
>
>
>
>
>> This extra fiddling with those bits was added in 7.19.7 to allow protocols to
>> actually set the bits for writing while it is in fact downloading and vice
>> versa, so we can't unconditionally set downloading to use the CURL_CSELECT_IN
>> bit.
>>
>>> If you want to reproduce the bug i suggest that you sent a couple of
>>> thousand https messages to a server running on a VMWare running on the same
>>> computer. I believe that SSL_ERROR_WANT_WRITE only occur from SSL_read when
>>> using a very fast connection.
>>
>> I'm seriously backlogged with libcurl stuff, but I'll see if I can work on
>> repeating this problem soonish.
>
> I understand! You guys are doing a terrific job with libcurl. It seems to me that Curl_socket_ready might be the function to attack in order to fix this problem. I will se what I can do and return with suggestions on how to fix the problem. It would be nice if the problem could be solved correctly within libcurl in order to minimize trouble with future upgrades.

These are my conclusions:

Curl_socket_ready do return the correct value. Since openSSL want us to write information to the socket after a SSL_ERROR_WANT_WRITE error code it seems reasonable that Curl_socket_ready should return 2 (CURL_CSELECT_OUT). But Curl_readwrite only calls the recieve function if we have a readable socket. And therein lies the fault. We still want to call the recieve function because SSL_read is the function that performs the writing that openSSL needs.

Insted of using the return value from the Curl_socket_ready (which isn't a good measure of determine what to do since both SSL_write and SSL_read might use both readable and writeable sockets during a renegotiation ) we need some other way of knowing what we are doing. But I don't really see an easy way of doing this.

One way ( but a quite ugly way I would say ) of solving the problem is to solve the problem in the Curl_ossl_recv function by creating a loop until we don't have a SSL_ERROR_WANT_WRITE or SSL_ERROR_WANT_READ error code ( or until things times out ). The big gain of this is that we reduce the number of effected protocols which might be a good thing. This loop seems to be working for me (timeout functionallity should be added): ( file: ssluse.c )

/*
 * If the read would block we return -1 and set 'wouldblock' to TRUE.
 * Otherwise we return the amount of data read. Other errors should return -1
 * and set 'wouldblock' to FALSE.
 */

ssize_t Curl_ossl_recv(struct connectdata *conn, /* connection data */
                       int num,                  /* socketindex */
                       char *buf,                /* store read data here */
                       size_t buffersize,        /* max amount to read */
                       bool *wouldblock)
{
  char error_buffer[120]; /* OpenSSL documents that this must be at
                             least 120 bytes long. */
  unsigned long sslerror;
  ssize_t nread;
  int buffsize;

  bool retry;
  int it;

  buffsize = (buffersize> (size_t)INT_MAX) ? INT_MAX : (int)buffersize;
  //printf("innan SSL_Read\n\n");

  do
  {

      nread = (ssize_t)SSL_read(conn->ssl[num].handle, buf, buffsize);
      //printf("efter SSL_Read\n\n");
      *wouldblock = FALSE;

      retry = 0;

     
      if(nread < 0) {
        /* failed SSL_read */
        int err = SSL_get_error(conn->ssl[num].handle, (int)nread);
        //printf("nread : %i, err: %i\n\n", nread, err);

        switch(err) {
        case SSL_ERROR_NONE: /* this is not an error */
        case SSL_ERROR_ZERO_RETURN: /* no more data */
          break;
        case SSL_ERROR_WANT_WRITE:
        case SSL_ERROR_WANT_READ:
          /* there's data pending, re-invoke SSL_read() */
          *wouldblock = TRUE;
         
          //
          retry = 1;
          break;   
        default:
          /* openssl/ssl.h says "look at error stack/return value/errno" */
          sslerror = ERR_get_error();
          failf(conn->data, "SSL read: %s, errno %d",
                ERR_error_string(sslerror, error_buffer),
                SOCKERRNO);
          return -1;
        }
      }
    }
    while( retry );
  return nread;
 
}

Any thoughts?

>
>
>>
>> --
>>
>> / daniel.haxx.se
>> -------------------------------------------------------------------
>> List admin: http://cool.haxx.se/list/listinfo/curl-library
>> Etiquette: http://curl.haxx.se/mail/etiquette.html
>
> ________________________________
> Windows Live: Make it easier for your friends to see what you’re up to on Facebook.
                                               
_________________________________________________________________
Windows Live: Keep your friends up to date with what you do online.
http://www.microsoft.com/middleeast/windows/windowslive/see-it-in-action/social-network-basics.aspx?ocid=PID23461::T:WLMTAGL:ON:WL:en-xm:SI_SB_1:092010
-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette: http://curl.haxx.se/mail/etiquette.html
Received on 2009-11-09