From 24a88bccaad11b472394b5b13d20df3b1a200ecd Mon Sep 17 00:00:00 2001 From: Mark Salisbury Date: Wed, 13 Jun 2012 14:47:28 -0600 Subject: [PATCH 4/8] schannel SSL: Made send method handle unexpected cases better Implemented timeout loop in schannel_send while sending data. This is as close as I think we can get to write buffering; I put a big comment in to explain my thinking. Fixed lines that went well past 80 chars. --- lib/curl_schannel.c | 70 ++++++++++++++++++++++++++++++++++++++++++++------ 1 files changed, 61 insertions(+), 9 deletions(-) diff --git a/lib/curl_schannel.c b/lib/curl_schannel.c index f67b1bc..6e2cf5e 100644 --- a/lib/curl_schannel.c +++ b/lib/curl_schannel.c @@ -39,7 +39,6 @@ /* * TODO list for TLS/SSL implementation: - * - implement write buffering * - implement SSL/TLS shutdown * - implement client certificate authentication * - implement custom server certificate validation @@ -247,7 +246,8 @@ schannel_connect_step1(struct connectdata *conn, int sockindex) outbuf.cbBuffer); /* send initial handshake data which is now stored in output buffer */ - Curl_write_plain(conn, conn->sock[sockindex], outbuf.pvBuffer, outbuf.cbBuffer, &write); + Curl_write_plain(conn, conn->sock[sockindex], outbuf.pvBuffer, outbuf.cbBuffer, + &write); s_pSecFn->FreeContextBuffer(outbuf.pvBuffer); if(write != outbuf.cbBuffer) { failf(data, "schannel: failed to send initial handshake data: %d\n", @@ -321,7 +321,8 @@ schannel_connect_step2(struct connectdata *conn, int sockindex) connssl->encdata_offset, connssl->encdata_length); /* setup input buffers */ - InitSecBuffer(&inbuf[0], SECBUFFER_TOKEN, malloc(connssl->encdata_offset), connssl->encdata_offset); + InitSecBuffer(&inbuf[0], SECBUFFER_TOKEN, malloc(connssl->encdata_offset), + connssl->encdata_offset); InitSecBuffer(&inbuf[1], SECBUFFER_EMPTY, NULL, 0); InitSecBufferDesc(&inbuf_desc, inbuf, 2); @@ -378,7 +379,8 @@ schannel_connect_step2(struct connectdata *conn, int sockindex) outbuf[i].cbBuffer); /* send handshake token to server */ - Curl_write_plain(conn, conn->sock[sockindex], outbuf[i].pvBuffer, outbuf[i].cbBuffer, &write); + Curl_write_plain(conn, conn->sock[sockindex], outbuf[i].pvBuffer, + outbuf[i].cbBuffer, &write); if(write != outbuf[i].cbBuffer) { failf(data, "schannel: failed to send next handshake data: %d\n", write); @@ -645,7 +647,8 @@ schannel_send(struct connectdata *conn, int sockindex, /* setup output buffers (header, data, trailer, empty) */ InitSecBuffer(&outbuf[0], SECBUFFER_STREAM_HEADER, data, connssl->stream_sizes.cbHeader); InitSecBuffer(&outbuf[1], SECBUFFER_DATA, data + connssl->stream_sizes.cbHeader, len); - InitSecBuffer(&outbuf[2], SECBUFFER_STREAM_TRAILER, data + connssl->stream_sizes.cbHeader + len, connssl->stream_sizes.cbTrailer); + InitSecBuffer(&outbuf[2], SECBUFFER_STREAM_TRAILER, + data + connssl->stream_sizes.cbHeader + len, connssl->stream_sizes.cbTrailer); InitSecBuffer(&outbuf[3], SECBUFFER_EMPTY, NULL, 0); InitSecBufferDesc(&outbuf_desc, outbuf, 4); @@ -658,10 +661,59 @@ schannel_send(struct connectdata *conn, int sockindex, /* check if the message was encrypted */ if(sspi_status == SEC_E_OK) { + size_t total_written = 0; + /* send the encrypted message including header, data and trailer */ len = outbuf[0].cbBuffer + outbuf[1].cbBuffer + outbuf[2].cbBuffer; - Curl_write_plain(conn, conn->sock[sockindex], data, len, &ret); - /* TODO: implement write buffering */ + + while (total_written < len) { + ssize_t written; + *err = Curl_write_plain(conn, conn->sock[sockindex], data + total_written, + len - total_written, &written); + if (*err == CURLE_OK || *err == CURLE_AGAIN) { + total_written += written; + if (total_written == len) { + /* The return value is the number of unencrypted bytes that were sent. */ + ret = outbuf[1].cbBuffer; + } + else { + /* It's important to send the full message which includes the header, + * encrypted payload, and trailer. Until the client receives all the + * data a coherent message has not been delivered and the client + * can't read any of it. + * If no bytes were written, we can return CURLE_AGAIN now. + + * If we wanted to buffer the unwritten encrypted bytes, we would tell + * the client that all data it has requested to be sent has been sent. + * The unwritten encrypted bytes would be the first bytes to send on the + * next invocation. + * Here's the catch with this - if we tell the client that all the bytes + * have been sent, will the client call this method again to send the + * buffered data? Looking at who calls this function, it seems the + * answer is NO. + */ + if (total_written == 0) { + ret = 0; + *err = CURLE_AGAIN; + break; + } + else { + if (Curl_timeleft(conn->data, NULL, FALSE) < 0) { + /* no need to continue if time already is up */ + failf(data, "SSL connection timeout"); + failf(conn->data, "schannel: timed out sending data (bytes sent: %d)", + total_written); + *err = CURLE_OPERATION_TIMEDOUT; + break; + } + } + } + } + else { + /* *err will be set to CURLE_SEND_ERROR or some error already. */ + break; + } + } } else if(sspi_status == SEC_E_INSUFFICIENT_MEMORY) { *err = CURLE_OUT_OF_MEMORY; @@ -746,8 +798,8 @@ schannel_recv(struct connectdata *conn, int sockindex, /* check if we still have some data in our buffers */ while(connssl->encdata_offset > 0 && sspi_status == SEC_E_OK) { /* prepare data buffer for DecryptMessage call */ - InitSecBuffer(&inbuf[0], SECBUFFER_DATA, connssl->encdata_buffer, connssl->encdata_offset); - + InitSecBuffer(&inbuf[0], SECBUFFER_DATA, connssl->encdata_buffer, + connssl->encdata_offset); /* we need 3 more empty input buffers for possible output */ InitSecBuffer(&inbuf[1], SECBUFFER_EMPTY, NULL, 0); InitSecBuffer(&inbuf[2], SECBUFFER_EMPTY, NULL, 0); -- 1.7.4.msysgit.0