romenskiy2012/curl
1
0
Fork 0
mirror of https://github.com/curl/curl.git synced 2026-05-30 06:47:28 +03:00
Code Issues Projects Releases Packages Wiki Activity

sendf: change Curl_read_plain to wrap Curl_recv_plain (take 2)

Browse source 
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.

--

This is take 2 of the original fix. It preserves the original behavior
of Curl_read_plain to write 0 to the bytes read parameter on error,
since apparently some callers expect that (SOCKS tests were hanging).
The original fix which landed in 12e1def5 and was later reverted in
18383fbf failed to work properly because it did not do that.

Also, it changes Curl_write_plain the same way to complement
Curl_read_plain, and it changes Curl_send_plain to return -1 instead of
0 on CURLE_AGAIN to complement Curl_recv_plain.

Behavior on error with these changes:

Curl_recv_plain returns -1 and *code receives error code.
Curl_send_plain returns -1 and *code receives error code.
Curl_read_plain returns error code and *n (bytes read) receives 0.
Curl_write_plain returns error code and *written receives 0.

--

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/9949
This commit is contained in:
Jay Satiro 2022-11-14 03:30:30 -05:00
parent 8c859cdb69
commit 4f42150d04
5 changed files with 65 additions and 41 deletions
Download patch file Download diff file Expand all files Collapse all files

16
lib/krb5.c
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)

69
lib/sendf.c
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)
{
@ -386,7 +387,6 @@ ssize_t Curl_send_plain(struct Curl_easy *data, int num,
#endif
) {
/* this is just a case of EWOULDBLOCK */
bytes_written = 0;
*code = CURLE_AGAIN;
}
else {
@ -403,7 +403,12 @@ 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 besides the
* prototype is '*written' (bytes written) is set to 0 on error.
* '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,13 +420,22 @@ 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);
if(*written == -1)
*written = 0;
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 +677,39 @@ 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 besides the
* prototype is '*n' (bytes read) is set to 0 on error.
* '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);
if(*n == -1)
*n = 0;
return result;
}
/*

5
lib/sendf.h
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,

12
lib/socks.c
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.");

4
lib/vtls/schannel.c
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);