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.

1 files 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);
     }