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
This commit is contained in:
Jay Satiro 2022-11-14 03:30:30 -05:00
parent 856b133f5d
commit 12e1def51a
5 changed files with 59 additions and 40 deletions

View File

@ -454,14 +454,14 @@ static int ftp_send_command(struct Curl_easy *data, const char *message, ...)
/* Read |len| from the socket |fd| and store it in |to|. Return a CURLcode
saying whether an error occurred or CURLE_OK if |len| was read. */
static CURLcode
socket_read(curl_socket_t fd, void *to, size_t len)
socket_read(struct Curl_easy *data, curl_socket_t fd, void *to, size_t len)
{
char *to_p = to;
CURLcode result;
ssize_t nread = 0;
while(len > 0) {
result = Curl_read_plain(fd, to_p, len, &nread);
result = Curl_read_plain(data, fd, to_p, len, &nread);
if(!result) {
len -= nread;
to_p += nread;
@ -502,15 +502,15 @@ socket_write(struct Curl_easy *data, curl_socket_t fd, const void *to,
return CURLE_OK;
}
static CURLcode read_data(struct connectdata *conn,
curl_socket_t fd,
static CURLcode read_data(struct Curl_easy *data, curl_socket_t fd,
struct krb5buffer *buf)
{
struct connectdata *conn = data->conn;
int len;
CURLcode result;
int nread;
result = socket_read(fd, &len, sizeof(len));
result = socket_read(data, fd, &len, sizeof(len));
if(result)
return result;
@ -525,7 +525,7 @@ static CURLcode read_data(struct connectdata *conn,
if(!len || !buf->data)
return CURLE_OUT_OF_MEMORY;
result = socket_read(fd, buf->data, len);
result = socket_read(data, fd, buf->data, len);
if(result)
return result;
nread = conn->mech->decode(conn->app_data, buf->data, len,
@ -560,7 +560,7 @@ static ssize_t sec_recv(struct Curl_easy *data, int sockindex,
/* Handle clear text response. */
if(conn->sec_complete == 0 || conn->data_prot == PROT_CLEAR)
return sread(fd, buffer, len);
return Curl_recv_plain(data, sockindex, buffer, len, err);
if(conn->in_buffer.eof_flag) {
conn->in_buffer.eof_flag = 0;
@ -573,7 +573,7 @@ static ssize_t sec_recv(struct Curl_easy *data, int sockindex,
buffer += bytes_read;
while(len > 0) {
if(read_data(conn, fd, &conn->in_buffer))
if(read_data(data, fd, &conn->in_buffer))
return -1;
if(conn->in_buffer.size == 0) {
if(bytes_read > 0)

View File

@ -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;
}
/*

View File

@ -61,9 +61,10 @@ CURLcode Curl_client_write(struct Curl_easy *data, int type, char *ptr,
bool Curl_recv_has_postponed_data(struct connectdata *conn, int sockindex);
/* internal read-function, does plain socket only */
CURLcode Curl_read_plain(curl_socket_t sockfd,
CURLcode Curl_read_plain(struct Curl_easy *data,
curl_socket_t sockfd,
char *buf,
size_t bytesfromsocket,
size_t sizerequested,
ssize_t *n);
ssize_t Curl_recv_plain(struct Curl_easy *data, int num, char *buf,

View File

@ -112,7 +112,7 @@ int Curl_blockread_all(struct Curl_easy *data, /* transfer */
result = ~CURLE_OK;
break;
}
result = Curl_read_plain(sockfd, buf, buffersize, &nread);
result = Curl_read_plain(data, sockfd, buf, buffersize, &nread);
if(CURLE_AGAIN == result)
continue;
if(result)
@ -396,7 +396,7 @@ static CURLproxycode do_SOCKS4(struct Curl_cfilter *cf,
/* FALLTHROUGH */
case CONNECT_SOCKS_READ:
/* Receive response */
result = Curl_read_plain(sockfd, (char *)sx->outp,
result = Curl_read_plain(data, sockfd, (char *)sx->outp,
sx->outstanding, &actualread);
if(result && (CURLE_AGAIN != result)) {
failf(data, "SOCKS4: Failed receiving connect request ack: %s",
@ -599,7 +599,7 @@ static CURLproxycode do_SOCKS5(struct Curl_cfilter *cf,
sx->outp = socksreq; /* store it here */
/* FALLTHROUGH */
case CONNECT_SOCKS_READ:
result = Curl_read_plain(sockfd, (char *)sx->outp,
result = Curl_read_plain(data, sockfd, (char *)sx->outp,
sx->outstanding, &actualread);
if(result && (CURLE_AGAIN != result)) {
failf(data, "Unable to receive initial SOCKS5 response.");
@ -729,7 +729,7 @@ static CURLproxycode do_SOCKS5(struct Curl_cfilter *cf,
sxstate(sx, data, CONNECT_AUTH_READ);
/* FALLTHROUGH */
case CONNECT_AUTH_READ:
result = Curl_read_plain(sockfd, (char *)sx->outp,
result = Curl_read_plain(data, sockfd, (char *)sx->outp,
sx->outstanding, &actualread);
if(result && (CURLE_AGAIN != result)) {
failf(data, "Unable to receive SOCKS5 sub-negotiation response.");
@ -931,7 +931,7 @@ static CURLproxycode do_SOCKS5(struct Curl_cfilter *cf,
sxstate(sx, data, CONNECT_REQ_READ);
/* FALLTHROUGH */
case CONNECT_REQ_READ:
result = Curl_read_plain(sockfd, (char *)sx->outp,
result = Curl_read_plain(data, sockfd, (char *)sx->outp,
sx->outstanding, &actualread);
if(result && (CURLE_AGAIN != result)) {
failf(data, "Failed to receive SOCKS5 connect request ack.");
@ -1030,7 +1030,7 @@ static CURLproxycode do_SOCKS5(struct Curl_cfilter *cf,
#endif
/* FALLTHROUGH */
case CONNECT_REQ_READ_MORE:
result = Curl_read_plain(sockfd, (char *)sx->outp,
result = Curl_read_plain(data, sockfd, (char *)sx->outp,
sx->outstanding, &actualread);
if(result && (CURLE_AGAIN != result)) {
failf(data, "Failed to receive SOCKS5 connect request ack.");

View File

@ -1413,7 +1413,7 @@ schannel_connect_step2(struct Curl_easy *data, struct connectdata *conn,
for(;;) {
if(doread) {
/* read encrypted handshake data from socket */
result = Curl_read_plain(conn->sock[sockindex],
result = Curl_read_plain(data, conn->sock[sockindex],
(char *) (backend->encdata_buffer +
backend->encdata_offset),
backend->encdata_length -
@ -2190,7 +2190,7 @@ schannel_recv(struct Curl_easy *data, int sockindex,
backend->encdata_offset, backend->encdata_length));
/* read encrypted data from socket */
*err = Curl_read_plain(conn->sock[sockindex],
*err = Curl_read_plain(data, conn->sock[sockindex],
(char *)(backend->encdata_buffer +
backend->encdata_offset),
size, &nread);