From 72a5813192a84a507d8810a142774df4c9116ba8 Mon Sep 17 00:00:00 2001 From: Mark Salisbury Date: Wed, 20 Jun 2012 00:51:03 +0200 Subject: schannel SSL: changes in schannel_connect_step2 Process extra data buffer before returning from schannel_connect_step2. Without this change I've seen WinCE hang when schannel_connect_step2 returns and calls Curl_socket_ready. If the encrypted handshake does not fit in the intial buffer (seen with large certificate chain), increasing the encrypted data buffer is necessary. Fixed warning in curl_schannel.c line 1215. --- lib/curl_schannel.c | 223 ++++++++++++++++++++++++++++++---------------------- 1 file changed, 128 insertions(+), 95 deletions(-) diff --git a/lib/curl_schannel.c b/lib/curl_schannel.c index 8b2f5ea72..8f4b11cc5 100644 --- a/lib/curl_schannel.c +++ b/lib/curl_schannel.c @@ -290,6 +290,7 @@ schannel_connect_step2(struct connectdata *conn, int sockindex) SECURITY_STATUS sspi_status = SEC_E_OK; TCHAR *host_name; CURLcode code; + bool doread = TRUE; infof(data, "schannel: SSL/TLS connection with %s port %hu (step 2/3)\n", conn->host.name, conn->remote_port); @@ -305,129 +306,161 @@ schannel_connect_step2(struct connectdata *conn, int sockindex) } } - /* read encrypted handshake data from socket */ - code = Curl_read_plain(conn->sock[sockindex], + /* if we need a bigger buffer to read a full message, increase buffer now */ + if(connssl->encdata_offset == connssl->encdata_length) { + if(connssl->encdata_length >= CURL_SCHANNEL_BUFFER_INIT_SIZE * 16) + return CURLE_OUT_OF_MEMORY; + /* increase internal encrypted data buffer */ + connssl->encdata_length *= 2; + connssl->encdata_buffer = realloc(connssl->encdata_buffer, + connssl->encdata_length); + if(connssl->encdata_buffer == NULL) { + failf(data, "schannel: unable to re-allocate memory"); + return CURLE_OUT_OF_MEMORY; + } + } + + for (;;) { + if(doread) { + /* read encrypted handshake data from socket */ + code = Curl_read_plain(conn->sock[sockindex], (char *) (connssl->encdata_buffer + connssl->encdata_offset), connssl->encdata_length - connssl->encdata_offset, &nread); - if(code == CURLE_AGAIN) { - if(connssl->connecting_state != ssl_connect_2_writing) - connssl->connecting_state = ssl_connect_2_reading; - infof(data, "schannel: failed to receive handshake, " - "need more data\n"); - return CURLE_OK; - } - else if((code != CURLE_OK) || (nread == 0)) { - failf(data, "schannel: failed to receive handshake, " - "SSL/TLS connection failed"); - return CURLE_SSL_CONNECT_ERROR; - } + if(code == CURLE_AGAIN) { + if(connssl->connecting_state != ssl_connect_2_writing) + connssl->connecting_state = ssl_connect_2_reading; + infof(data, "schannel: failed to receive handshake, " + "need more data\n"); + return CURLE_OK; + } + else if((code != CURLE_OK) || (nread == 0)) { + failf(data, "schannel: failed to receive handshake, " + "SSL/TLS connection failed"); + return CURLE_SSL_CONNECT_ERROR; + } - /* increase encrypted data buffer offset */ - connssl->encdata_offset += nread; + /* increase encrypted data buffer offset */ + connssl->encdata_offset += nread; + } - infof(data, "schannel: encrypted data buffer: offset %zu length %zu\n", + infof(data, "schannel: encrypted data buffer: offset %zu length %zu\n", connssl->encdata_offset, connssl->encdata_length); - /* setup input buffers */ - InitSecBuffer(&inbuf[0], SECBUFFER_TOKEN, malloc(connssl->encdata_offset), - curlx_uztoul(connssl->encdata_offset)); - InitSecBuffer(&inbuf[1], SECBUFFER_EMPTY, NULL, 0); - InitSecBufferDesc(&inbuf_desc, inbuf, 2); + /* setup input buffers */ + InitSecBuffer(&inbuf[0], SECBUFFER_TOKEN, malloc(connssl->encdata_offset), + curlx_uztoul(connssl->encdata_offset)); + InitSecBuffer(&inbuf[1], SECBUFFER_EMPTY, NULL, 0); + InitSecBufferDesc(&inbuf_desc, inbuf, 2); - /* setup output buffers */ - InitSecBuffer(&outbuf[0], SECBUFFER_TOKEN, NULL, 0); - InitSecBuffer(&outbuf[1], SECBUFFER_ALERT, NULL, 0); - InitSecBufferDesc(&outbuf_desc, outbuf, 2); + /* setup output buffers */ + InitSecBuffer(&outbuf[0], SECBUFFER_TOKEN, NULL, 0); + InitSecBuffer(&outbuf[1], SECBUFFER_ALERT, NULL, 0); + InitSecBufferDesc(&outbuf_desc, outbuf, 2); - if(inbuf[0].pvBuffer == NULL) { - failf(data, "schannel: unable to allocate memory"); - return CURLE_OUT_OF_MEMORY; - } + if(inbuf[0].pvBuffer == NULL) { + failf(data, "schannel: unable to allocate memory"); + return CURLE_OUT_OF_MEMORY; + } - /* copy received handshake data into input buffer */ - memcpy(inbuf[0].pvBuffer, connssl->encdata_buffer, connssl->encdata_offset); + /* copy received handshake data into input buffer */ + memcpy(inbuf[0].pvBuffer, connssl->encdata_buffer, + connssl->encdata_offset); #ifdef UNICODE - host_name = Curl_convert_UTF8_to_wchar(conn->host.name); - if(!host_name) - return CURLE_OUT_OF_MEMORY; + host_name = Curl_convert_UTF8_to_wchar(conn->host.name); + if(!host_name) + return CURLE_OUT_OF_MEMORY; #else host_name = conn->host.name; #endif - /* http://msdn.microsoft.com/en-us/library/windows/desktop/aa375924.aspx */ + /* http://msdn.microsoft.com/en-us/library/windows/desktop/aa375924.aspx */ - sspi_status = s_pSecFn->InitializeSecurityContext( - &connssl->cred->cred_handle, &connssl->ctxt->ctxt_handle, - host_name, connssl->req_flags, 0, 0, &inbuf_desc, 0, NULL, - &outbuf_desc, &connssl->ret_flags, &connssl->ctxt->time_stamp); + sspi_status = s_pSecFn->InitializeSecurityContext( + &connssl->cred->cred_handle, &connssl->ctxt->ctxt_handle, + host_name, connssl->req_flags, 0, 0, &inbuf_desc, 0, NULL, + &outbuf_desc, &connssl->ret_flags, &connssl->ctxt->time_stamp); #ifdef UNICODE - free(host_name); + free(host_name); #endif - /* free buffer for received handshake data */ - free(inbuf[0].pvBuffer); + /* free buffer for received handshake data */ + free(inbuf[0].pvBuffer); - /* check if the handshake was incomplete */ - if(sspi_status == SEC_E_INCOMPLETE_MESSAGE) { - connssl->connecting_state = ssl_connect_2_reading; - infof(data, "schannel: received incomplete message, need more data\n"); - return CURLE_OK; - } + /* check if the handshake was incomplete */ + if(sspi_status == SEC_E_INCOMPLETE_MESSAGE) { + connssl->connecting_state = ssl_connect_2_reading; + infof(data, "schannel: received incomplete message, need more data\n"); + return CURLE_OK; + } - /* check if the handshake needs to be continued */ - if(sspi_status == SEC_I_CONTINUE_NEEDED || sspi_status == SEC_E_OK) { - for(i = 0; i < 2; i++) { - /* search for handshake tokens that need to be send */ - if(outbuf[i].BufferType == SECBUFFER_TOKEN && outbuf[i].cbBuffer > 0) { - infof(data, "schannel: sending next handshake data: " - "sending %lu bytes...\n", outbuf[i].cbBuffer); - - /* send handshake token to server */ - code = Curl_write_plain(conn, conn->sock[sockindex], - outbuf[i].pvBuffer, outbuf[i].cbBuffer, - &written); - if((code != CURLE_OK) || (outbuf[i].cbBuffer != (size_t)written)) { - failf(data, "schannel: failed to send next handshake data: " - "sent %zd of %lu bytes", written, outbuf[i].cbBuffer); - return CURLE_SSL_CONNECT_ERROR; + /* check if the handshake needs to be continued */ + if(sspi_status == SEC_I_CONTINUE_NEEDED || sspi_status == SEC_E_OK) { + for(i = 0; i < 2; i++) { + /* search for handshake tokens that need to be send */ + if(outbuf[i].BufferType == SECBUFFER_TOKEN && outbuf[i].cbBuffer > 0) { + infof(data, "schannel: sending next handshake data: " + "sending %lu bytes...\n", outbuf[i].cbBuffer); + + /* send handshake token to server */ + code = Curl_write_plain(conn, conn->sock[sockindex], + outbuf[i].pvBuffer, outbuf[i].cbBuffer, + &written); + if((code != CURLE_OK) || (outbuf[i].cbBuffer != (size_t)written)) { + failf(data, "schannel: failed to send next handshake data: " + "sent %zd of %lu bytes", written, outbuf[i].cbBuffer); + return CURLE_SSL_CONNECT_ERROR; + } } - } - /* free obsolete buffer */ - if(outbuf[i].pvBuffer != NULL) { - s_pSecFn->FreeContextBuffer(outbuf[i].pvBuffer); + /* free obsolete buffer */ + if(outbuf[i].pvBuffer != NULL) { + s_pSecFn->FreeContextBuffer(outbuf[i].pvBuffer); + } } } - } - else { - if(sspi_status == SEC_E_WRONG_PRINCIPAL) - failf(data, "schannel: SNI or certificate check failed: %s", - Curl_sspi_strerror(conn, sspi_status)); - else - failf(data, "schannel: next InitializeSecurityContext failed: %s", - Curl_sspi_strerror(conn, sspi_status)); - return CURLE_SSL_CONNECT_ERROR; - } - - /* check if there was additional remaining encrypted data */ - if(inbuf[1].BufferType == SECBUFFER_EXTRA && inbuf[1].cbBuffer > 0) { - infof(data, "schannel: encrypted data length: %lu\n", inbuf[1].cbBuffer); + else { + if(sspi_status == SEC_E_WRONG_PRINCIPAL) + failf(data, "schannel: SNI or certificate check failed: %s", + Curl_sspi_strerror(conn, sspi_status)); + else + failf(data, "schannel: next InitializeSecurityContext failed: %s", + Curl_sspi_strerror(conn, sspi_status)); + return CURLE_SSL_CONNECT_ERROR; + } - /* check if the remaining data is less than the total amount - * and therefore begins after the already processed data - */ - if(connssl->encdata_offset > inbuf[1].cbBuffer) { - memmove(connssl->encdata_buffer, - (connssl->encdata_buffer + connssl->encdata_offset) - - inbuf[1].cbBuffer, inbuf[1].cbBuffer); - connssl->encdata_offset = inbuf[1].cbBuffer; + /* check if there was additional remaining encrypted data */ + if(inbuf[1].BufferType == SECBUFFER_EXTRA && inbuf[1].cbBuffer > 0) { + infof(data, "schannel: encrypted data length: %lu\n", inbuf[1].cbBuffer); + /* + There are two cases where we could be getting extra data here: + 1) If we're renegotiating a connection and the handshake is already + complete (from the server perspective), it can encrypted app data + (not handshake data) in an extra buffer at this point. + 2) (sspi_status == SEC_I_CONTINUE_NEEDED) We are negotiating a + connection and this extra data is part of the handshake. + We should process the data immediately; waiting for the socket to + be ready may fail since the server is done sending handshake data. + */ + /* check if the remaining data is less than the total amount + and therefore begins after the already processed data */ + if(connssl->encdata_offset > inbuf[1].cbBuffer) { + memmove(connssl->encdata_buffer, + (connssl->encdata_buffer + connssl->encdata_offset) - + inbuf[1].cbBuffer, inbuf[1].cbBuffer); + connssl->encdata_offset = inbuf[1].cbBuffer; + if(sspi_status == SEC_I_CONTINUE_NEEDED) { + doread = FALSE; + continue; + } + } } - } - else { - connssl->encdata_offset = 0; + else { + connssl->encdata_offset = 0; + } + break; } /* check if the handshake needs to be continued */ @@ -1229,7 +1262,7 @@ static CURLcode verify_certificate(struct connectdata *conn, int sockindex) failf(data, "schannel: CertGetNameString() certificate hostname " "(%s) did not match connection (%s)", _cert_hostname, conn->host.name); - free(_cert_hostname); + free((void *)_cert_hostname); } free(hostname); } -- cgit v1.2.3