sendf: change Curl_read_plain to wrap Curl_recv_plain

Prior to this change Curl_read_plain would attempt to read the
socket directly. On Windows that's a problem because recv data may be
cached by libcurl and that data is only drained using Curl_recv_plain.

Rather than rewrite Curl_read_plain to handle cached recv data, I
changed it to wrap Curl_recv_plain, in much the same way that
Curl_write_plain already wraps Curl_send_plain.

Curl_read_plain -> Curl_recv_plain
Curl_write_plain -> Curl_send_plain

This fixes a bug in the schannel backend where decryption of arbitrary
TLS records fails because cached recv data is never drained. We send
data (TLS records formed by Schannel) using Curl_write_plain, which
calls Curl_send_plain, and that may do a recv-before-send
("pre-receive") to cache received data. The code calls Curl_read_plain
to read data (TLS records from the server), which prior to this change
did not call Curl_recv_plain and therefore cached recv data wasn't
retrieved, resulting in malformed TLS records and decryption failure
(SEC_E_DECRYPT_FAILURE).

The bug has only been observed during Schannel TLS 1.3 handshakes. Refer
to the issue and PR for more information.

Ref: https://github.com/curl/curl/issues/9431#issuecomment-1312420361

Assisted-by: Joel Depooter
Reported-by: Egor Pugin

Fixes https://github.com/curl/curl/issues/9431
Closes https://github.com/curl/curl/pull/9904

1 files changed, 40 insertions, 22 deletions

diff --git a/lib/sendf.c b/lib/sendf.c
index 52bd47c5c..5c331b76e 100644
--- a/lib/sendf.c
+++ b/lib/sendf.c
@@ -338,6 +338,7 @@ CURLcode Curl_write(struct Curl_easy *data,
   }
 }
 
+/* Curl_send_plain sends raw data without a size restriction on 'len'. */
 ssize_t Curl_send_plain(struct Curl_easy *data, int num,
                         const void *mem, size_t len, CURLcode *code)
 {
@@ -403,7 +404,11 @@ ssize_t Curl_send_plain(struct Curl_easy *data, int num,
 /*
  * Curl_write_plain() is an internal write function that sends data to the
  * server using plain sockets only. Otherwise meant to have the exact same
- * proto as Curl_write()
+ * proto as Curl_write().
+ *
+ * This function wraps Curl_send_plain(). The only difference should be the
+ * prototype. 'sockfd' must be one of the connection's two main sockets and
+ * the value of 'len' must not be changed.
  */
 CURLcode Curl_write_plain(struct Curl_easy *data,
                           curl_socket_t sockfd,
@@ -415,6 +420,12 @@ CURLcode Curl_write_plain(struct Curl_easy *data,
   struct connectdata *conn = data->conn;
   int num;
   DEBUGASSERT(conn);
+  DEBUGASSERT(sockfd == conn->sock[FIRSTSOCKET] ||
+              sockfd == conn->sock[SECONDARYSOCKET]);
+  if(sockfd != conn->sock[FIRSTSOCKET] &&
+     sockfd != conn->sock[SECONDARYSOCKET])
+    return CURLE_BAD_FUNCTION_ARGUMENT;
+
   num = (sockfd == conn->sock[SECONDARYSOCKET]);
 
   *written = Curl_send_plain(data, num, mem, len, &result);
@@ -422,6 +433,7 @@ CURLcode Curl_write_plain(struct Curl_easy *data,
   return result;
 }
 
+/* Curl_recv_plain receives raw data without a size restriction on 'len'. */
 ssize_t Curl_recv_plain(struct Curl_easy *data, int num, char *buf,
                         size_t len, CURLcode *code)
 {
@@ -663,30 +675,36 @@ CURLcode Curl_client_write(struct Curl_easy *data,
   return chop_write(data, type, ptr, len);
 }
 
-CURLcode Curl_read_plain(curl_socket_t sockfd,
-                         char *buf,
-                         size_t bytesfromsocket,
-                         ssize_t *n)
+/*
+ * Curl_read_plain() is an internal read function that reads data from the
+ * server using plain sockets only. Otherwise meant to have the exact same
+ * proto as Curl_read().
+ *
+ * This function wraps Curl_recv_plain(). The only difference should be the
+ * prototype. 'sockfd' must be one of the connection's two main sockets and
+ * the value of 'sizerequested' must not be changed.
+ */
+CURLcode Curl_read_plain(struct Curl_easy *data,   /* transfer */
+                         curl_socket_t sockfd,     /* read from this socket */
+                         char *buf,                /* store read data here */
+                         size_t sizerequested,     /* max amount to read */
+                         ssize_t *n)               /* amount bytes read */
 {
-  ssize_t nread = sread(sockfd, buf, bytesfromsocket);
+  CURLcode result;
+  struct connectdata *conn = data->conn;
+  int num;
+  DEBUGASSERT(conn);
+  DEBUGASSERT(sockfd == conn->sock[FIRSTSOCKET] ||
+              sockfd == conn->sock[SECONDARYSOCKET]);
+  if(sockfd != conn->sock[FIRSTSOCKET] &&
+     sockfd != conn->sock[SECONDARYSOCKET])
+    return CURLE_BAD_FUNCTION_ARGUMENT;
 
-  if(-1 == nread) {
-    const int err = SOCKERRNO;
-    const bool return_error =
-#ifdef USE_WINSOCK
-      WSAEWOULDBLOCK == err
-#else
-      EWOULDBLOCK == err || EAGAIN == err || EINTR == err
-#endif
-      ;
-    *n = 0; /* no data returned */
-    if(return_error)
-      return CURLE_AGAIN;
-    return CURLE_RECV_ERROR;
-  }
+  num = (sockfd == conn->sock[SECONDARYSOCKET]);
 
-  *n = nread;
-  return CURLE_OK;
+  *n = Curl_recv_plain(data, num, buf, sizerequested, &result);
+
+  return result;
 }
 
 /*