Skip to content

Commit 7f4a9a9

Browse files
Harry Sintonenbagder
Harry Sintonen
authored andcommitted
openssl: associate/detach the transfer from connection
CVE-2021-22901 Bug: https://curl.se/docs/CVE-2021-22901.html
1 parent 39ce47f commit 7f4a9a9

13 files changed

+172
-50
lines changed

Diff for: lib/multi.c

+4-1
Original file line numberDiff line numberDiff line change
@@ -878,8 +878,10 @@ bool Curl_multiplex_wanted(const struct Curl_multi *multi)
878878
void Curl_detach_connnection(struct Curl_easy *data)
879879
{
880880
struct connectdata *conn = data->conn;
881-
if(conn)
881+
if(conn) {
882882
Curl_llist_remove(&conn->easyq, &data->conn_queue, NULL);
883+
Curl_ssl_detach_conn(data, conn);
884+
}
883885
data->conn = NULL;
884886
}
885887

@@ -898,6 +900,7 @@ void Curl_attach_connnection(struct Curl_easy *data,
898900
&data->conn_queue);
899901
if(conn->handler->attach)
900902
conn->handler->attach(data, conn);
903+
Curl_ssl_associate_conn(data, conn);
901904
}
902905

903906
static int waitconnect_getsock(struct connectdata *conn,

Diff for: lib/vtls/gskit.c

+3-1
Original file line numberDiff line numberDiff line change
@@ -1304,7 +1304,9 @@ const struct Curl_ssl Curl_ssl_gskit = {
13041304
Curl_none_set_engine_default, /* set_engine_default */
13051305
Curl_none_engines_list, /* engines_list */
13061306
Curl_none_false_start, /* false_start */
1307-
NULL /* sha256sum */
1307+
NULL, /* sha256sum */
1308+
NULL, /* associate_connection */
1309+
NULL /* disassociate_connection */
13081310
};
13091311

13101312
#endif /* USE_GSKIT */

Diff for: lib/vtls/gtls.c

+3-1
Original file line numberDiff line numberDiff line change
@@ -1656,7 +1656,9 @@ const struct Curl_ssl Curl_ssl_gnutls = {
16561656
Curl_none_set_engine_default, /* set_engine_default */
16571657
Curl_none_engines_list, /* engines_list */
16581658
Curl_none_false_start, /* false_start */
1659-
gtls_sha256sum /* sha256sum */
1659+
gtls_sha256sum, /* sha256sum */
1660+
NULL, /* associate_connection */
1661+
NULL /* disassociate_connection */
16601662
};
16611663

16621664
#endif /* USE_GNUTLS */

Diff for: lib/vtls/mbedtls.c

+3-1
Original file line numberDiff line numberDiff line change
@@ -1093,7 +1093,9 @@ const struct Curl_ssl Curl_ssl_mbedtls = {
10931093
Curl_none_set_engine_default, /* set_engine_default */
10941094
Curl_none_engines_list, /* engines_list */
10951095
Curl_none_false_start, /* false_start */
1096-
mbedtls_sha256sum /* sha256sum */
1096+
mbedtls_sha256sum, /* sha256sum */
1097+
NULL, /* associate_connection */
1098+
NULL /* disassociate_connection */
10971099
};
10981100

10991101
#endif /* USE_MBEDTLS */

Diff for: lib/vtls/mesalink.c

+3-1
Original file line numberDiff line numberDiff line change
@@ -666,7 +666,9 @@ const struct Curl_ssl Curl_ssl_mesalink = {
666666
Curl_none_set_engine_default, /* set_engine_default */
667667
Curl_none_engines_list, /* engines_list */
668668
Curl_none_false_start, /* false_start */
669-
NULL /* sha256sum */
669+
NULL, /* sha256sum */
670+
NULL, /* associate_connection */
671+
NULL /* disassociate_connection */
670672
};
671673

672674
#endif

Diff for: lib/vtls/nss.c

+3-1
Original file line numberDiff line numberDiff line change
@@ -2465,7 +2465,9 @@ const struct Curl_ssl Curl_ssl_nss = {
24652465
Curl_none_set_engine_default, /* set_engine_default */
24662466
Curl_none_engines_list, /* engines_list */
24672467
nss_false_start, /* false_start */
2468-
nss_sha256sum /* sha256sum */
2468+
nss_sha256sum, /* sha256sum */
2469+
NULL, /* associate_connection */
2470+
NULL /* disassociate_connection */
24692471
};
24702472

24712473
#endif /* USE_NSS */

Diff for: lib/vtls/openssl.c

+107-39
Original file line numberDiff line numberDiff line change
@@ -240,6 +240,10 @@ struct ssl_backend_data {
240240
#endif
241241
};
242242

243+
static void ossl_associate_connection(struct Curl_easy *data,
244+
struct connectdata *conn,
245+
int sockindex);
246+
243247
/*
244248
* Number of bytes to read from the random number seed file. This must be
245249
* a finite value (because some entropy "files" like /dev/urandom have
@@ -2581,6 +2585,7 @@ static CURLcode ossl_connect_step1(struct Curl_easy *data,
25812585
curl_socket_t sockfd = conn->sock[sockindex];
25822586
struct ssl_connect_data *connssl = &conn->ssl[sockindex];
25832587
ctx_option_t ctx_options = 0;
2588+
void *ssl_sessionid = NULL;
25842589

25852590
#ifdef SSL_CTRL_SET_TLSEXT_HOSTNAME
25862591
bool sni;
@@ -3225,46 +3230,23 @@ static CURLcode ossl_connect_step1(struct Curl_easy *data,
32253230
}
32263231
#endif
32273232

3228-
/* Check if there's a cached ID we can/should use here! */
3229-
if(SSL_SET_OPTION(primary.sessionid)) {
3230-
void *ssl_sessionid = NULL;
3231-
int data_idx = ossl_get_ssl_data_index();
3232-
int connectdata_idx = ossl_get_ssl_conn_index();
3233-
int sockindex_idx = ossl_get_ssl_sockindex_index();
3234-
int proxy_idx = ossl_get_proxy_index();
3235-
3236-
if(data_idx >= 0 && connectdata_idx >= 0 && sockindex_idx >= 0 &&
3237-
proxy_idx >= 0) {
3238-
/* Store the data needed for the "new session" callback.
3239-
* The sockindex is stored as a pointer to an array element. */
3240-
SSL_set_ex_data(backend->handle, data_idx, data);
3241-
SSL_set_ex_data(backend->handle, connectdata_idx, conn);
3242-
SSL_set_ex_data(backend->handle, sockindex_idx, conn->sock + sockindex);
3243-
#ifndef CURL_DISABLE_PROXY
3244-
SSL_set_ex_data(backend->handle, proxy_idx, SSL_IS_PROXY() ? (void *) 1:
3245-
NULL);
3246-
#else
3247-
SSL_set_ex_data(backend->handle, proxy_idx, NULL);
3248-
#endif
3249-
3250-
}
3233+
ossl_associate_connection(data, conn, sockindex);
32513234

3252-
Curl_ssl_sessionid_lock(data);
3253-
if(!Curl_ssl_getsessionid(data, conn, SSL_IS_PROXY() ? TRUE : FALSE,
3254-
&ssl_sessionid, NULL, sockindex)) {
3255-
/* we got a session id, use it! */
3256-
if(!SSL_set_session(backend->handle, ssl_sessionid)) {
3257-
Curl_ssl_sessionid_unlock(data);
3258-
failf(data, "SSL: SSL_set_session failed: %s",
3259-
ossl_strerror(ERR_get_error(), error_buffer,
3260-
sizeof(error_buffer)));
3261-
return CURLE_SSL_CONNECT_ERROR;
3262-
}
3263-
/* Informational message */
3264-
infof(data, "SSL re-using session ID\n");
3235+
Curl_ssl_sessionid_lock(data);
3236+
if(!Curl_ssl_getsessionid(data, conn, SSL_IS_PROXY() ? TRUE : FALSE,
3237+
&ssl_sessionid, NULL, sockindex)) {
3238+
/* we got a session id, use it! */
3239+
if(!SSL_set_session(backend->handle, ssl_sessionid)) {
3240+
Curl_ssl_sessionid_unlock(data);
3241+
failf(data, "SSL: SSL_set_session failed: %s",
3242+
ossl_strerror(ERR_get_error(), error_buffer,
3243+
sizeof(error_buffer)));
3244+
return CURLE_SSL_CONNECT_ERROR;
32653245
}
3266-
Curl_ssl_sessionid_unlock(data);
3246+
/* Informational message */
3247+
infof(data, "SSL re-using session ID\n");
32673248
}
3249+
Curl_ssl_sessionid_unlock(data);
32683250

32693251
#ifndef CURL_DISABLE_PROXY
32703252
if(conn->proxy_ssl[sockindex].use) {
@@ -4498,6 +4480,90 @@ static void *ossl_get_internals(struct ssl_connect_data *connssl,
44984480
(void *)backend->ctx : (void *)backend->handle;
44994481
}
45004482

4483+
static void ossl_associate_connection(struct Curl_easy *data,
4484+
struct connectdata *conn,
4485+
int sockindex)
4486+
{
4487+
struct ssl_connect_data *connssl = &conn->ssl[sockindex];
4488+
struct ssl_backend_data *backend = connssl->backend;
4489+
4490+
/* If we don't have SSL context, do nothing. */
4491+
if(!backend->handle)
4492+
return;
4493+
4494+
if(SSL_SET_OPTION(primary.sessionid)) {
4495+
int data_idx = ossl_get_ssl_data_index();
4496+
int connectdata_idx = ossl_get_ssl_conn_index();
4497+
int sockindex_idx = ossl_get_ssl_sockindex_index();
4498+
int proxy_idx = ossl_get_proxy_index();
4499+
4500+
if(data_idx >= 0 && connectdata_idx >= 0 && sockindex_idx >= 0 &&
4501+
proxy_idx >= 0) {
4502+
/* Store the data needed for the "new session" callback.
4503+
* The sockindex is stored as a pointer to an array element. */
4504+
SSL_set_ex_data(backend->handle, data_idx, data);
4505+
SSL_set_ex_data(backend->handle, connectdata_idx, conn);
4506+
SSL_set_ex_data(backend->handle, sockindex_idx, conn->sock + sockindex);
4507+
#ifndef CURL_DISABLE_PROXY
4508+
SSL_set_ex_data(backend->handle, proxy_idx, SSL_IS_PROXY() ? (void *) 1:
4509+
NULL);
4510+
#else
4511+
SSL_set_ex_data(backend->handle, proxy_idx, NULL);
4512+
#endif
4513+
}
4514+
}
4515+
}
4516+
4517+
/*
4518+
* Starting with TLS 1.3, the ossl_new_session_cb callback gets called after
4519+
* the handshake. If the transfer that sets up the callback gets killed before
4520+
* this callback arrives, we must make sure to properly clear the data to
4521+
* avoid UAF problems. A future optimization could be to instead store another
4522+
* transfer that might still be using the same connection.
4523+
*/
4524+
4525+
static void ossl_disassociate_connection(struct Curl_easy *data,
4526+
int sockindex)
4527+
{
4528+
struct connectdata *conn = data->conn;
4529+
struct ssl_connect_data *connssl = &conn->ssl[sockindex];
4530+
struct ssl_backend_data *backend = connssl->backend;
4531+
4532+
/* If we don't have SSL context, do nothing. */
4533+
if(!backend->handle)
4534+
return;
4535+
4536+
if(SSL_SET_OPTION(primary.sessionid)) {
4537+
bool isproxy = FALSE;
4538+
bool incache;
4539+
void *old_ssl_sessionid = NULL;
4540+
int data_idx = ossl_get_ssl_data_index();
4541+
int connectdata_idx = ossl_get_ssl_conn_index();
4542+
int sockindex_idx = ossl_get_ssl_sockindex_index();
4543+
int proxy_idx = ossl_get_proxy_index();
4544+
4545+
if(data_idx >= 0 && connectdata_idx >= 0 && sockindex_idx >= 0 &&
4546+
proxy_idx >= 0) {
4547+
/* Invalidate the session cache entry, if any */
4548+
isproxy = SSL_get_ex_data(backend->handle, proxy_idx) ? TRUE : FALSE;
4549+
4550+
/* Disable references to data in "new session" callback to avoid
4551+
* accessing a stale pointer. */
4552+
SSL_set_ex_data(backend->handle, data_idx, NULL);
4553+
SSL_set_ex_data(backend->handle, connectdata_idx, NULL);
4554+
SSL_set_ex_data(backend->handle, sockindex_idx, NULL);
4555+
SSL_set_ex_data(backend->handle, proxy_idx, NULL);
4556+
}
4557+
4558+
Curl_ssl_sessionid_lock(data);
4559+
incache = !(Curl_ssl_getsessionid(data, conn, isproxy,
4560+
&old_ssl_sessionid, NULL, sockindex));
4561+
if(incache)
4562+
Curl_ssl_delsessionid(data, old_ssl_sessionid);
4563+
Curl_ssl_sessionid_unlock(data);
4564+
}
4565+
}
4566+
45014567
const struct Curl_ssl Curl_ssl_openssl = {
45024568
{ CURLSSLBACKEND_OPENSSL, "openssl" }, /* info */
45034569

@@ -4533,10 +4599,12 @@ const struct Curl_ssl Curl_ssl_openssl = {
45334599
ossl_engines_list, /* engines_list */
45344600
Curl_none_false_start, /* false_start */
45354601
#if (OPENSSL_VERSION_NUMBER >= 0x0090800fL) && !defined(OPENSSL_NO_SHA256)
4536-
ossl_sha256sum /* sha256sum */
4602+
ossl_sha256sum, /* sha256sum */
45374603
#else
4538-
NULL /* sha256sum */
4604+
NULL, /* sha256sum */
45394605
#endif
4606+
ossl_associate_connection, /* associate_connection */
4607+
ossl_disassociate_connection /* disassociate_connection */
45404608
};
45414609

45424610
#endif /* USE_OPENSSL */

Diff for: lib/vtls/rustls.c

+3-1
Original file line numberDiff line numberDiff line change
@@ -604,7 +604,9 @@ const struct Curl_ssl Curl_ssl_rustls = {
604604
Curl_none_set_engine_default, /* set_engine_default */
605605
Curl_none_engines_list, /* engines_list */
606606
Curl_none_false_start, /* false_start */
607-
NULL /* sha256sum */
607+
NULL, /* sha256sum */
608+
NULL, /* associate_connection */
609+
NULL /* disassociate_connection */
608610
};
609611

610612
#endif /* USE_RUSTLS */

Diff for: lib/vtls/schannel.c

+4-2
Original file line numberDiff line numberDiff line change
@@ -329,7 +329,7 @@ get_alg_id_by_name(char *name)
329329

330330
static CURLcode
331331
set_ssl_ciphers(SCHANNEL_CRED *schannel_cred, char *ciphers,
332-
int *algIds)
332+
ALG_ID *algIds)
333333
{
334334
char *startCur = ciphers;
335335
int algCount = 0;
@@ -2433,7 +2433,9 @@ const struct Curl_ssl Curl_ssl_schannel = {
24332433
Curl_none_set_engine_default, /* set_engine_default */
24342434
Curl_none_engines_list, /* engines_list */
24352435
Curl_none_false_start, /* false_start */
2436-
schannel_sha256sum /* sha256sum */
2436+
schannel_sha256sum, /* sha256sum */
2437+
NULL, /* associate_connection */
2438+
NULL /* disassociate_connection */
24372439
};
24382440

24392441
#endif /* USE_SCHANNEL */

Diff for: lib/vtls/sectransp.c

+2
Original file line numberDiff line numberDiff line change
@@ -3453,6 +3453,8 @@ const struct Curl_ssl Curl_ssl_sectransp = {
34533453
Curl_none_engines_list, /* engines_list */
34543454
sectransp_false_start, /* false_start */
34553455
sectransp_sha256sum /* sha256sum */
3456+
NULL, /* associate_connection */
3457+
NULL /* disassociate_connection */
34563458
};
34573459

34583460
#ifdef __clang__

Diff for: lib/vtls/vtls.c

+22-1
Original file line numberDiff line numberDiff line change
@@ -586,6 +586,25 @@ CURLcode Curl_ssl_addsessionid(struct Curl_easy *data,
586586
return CURLE_OK;
587587
}
588588

589+
void Curl_ssl_associate_conn(struct Curl_easy *data,
590+
struct connectdata *conn)
591+
{
592+
if(Curl_ssl->associate_connection) {
593+
Curl_ssl->associate_connection(data, conn, FIRSTSOCKET);
594+
if(conn->sock[SECONDARYSOCKET] && conn->bits.sock_accepted)
595+
Curl_ssl->associate_connection(data, conn, SECONDARYSOCKET);
596+
}
597+
}
598+
599+
void Curl_ssl_detach_conn(struct Curl_easy *data,
600+
struct connectdata *conn)
601+
{
602+
if(Curl_ssl->disassociate_connection) {
603+
Curl_ssl->disassociate_connection(data, FIRSTSOCKET);
604+
if(conn->sock[SECONDARYSOCKET] && conn->bits.sock_accepted)
605+
Curl_ssl->disassociate_connection(data, SECONDARYSOCKET);
606+
}
607+
}
589608

590609
void Curl_ssl_close_all(struct Curl_easy *data)
591610
{
@@ -1214,7 +1233,9 @@ static const struct Curl_ssl Curl_ssl_multi = {
12141233
Curl_none_set_engine_default, /* set_engine_default */
12151234
Curl_none_engines_list, /* engines_list */
12161235
Curl_none_false_start, /* false_start */
1217-
NULL /* sha256sum */
1236+
NULL, /* sha256sum */
1237+
NULL, /* associate_connection */
1238+
NULL /* disassociate_connection */
12181239
};
12191240

12201241
const struct Curl_ssl *Curl_ssl =

Diff for: lib/vtls/vtls.h

+12
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,11 @@ struct Curl_ssl {
8484
bool (*false_start)(void);
8585
CURLcode (*sha256sum)(const unsigned char *input, size_t inputlen,
8686
unsigned char *sha256sum, size_t sha256sumlen);
87+
88+
void (*associate_connection)(struct Curl_easy *data,
89+
struct connectdata *conn,
90+
int sockindex);
91+
void (*disassociate_connection)(struct Curl_easy *data, int sockindex);
8792
};
8893

8994
#ifdef USE_SSL
@@ -283,6 +288,11 @@ bool Curl_ssl_cert_status_request(void);
283288

284289
bool Curl_ssl_false_start(void);
285290

291+
void Curl_ssl_associate_conn(struct Curl_easy *data,
292+
struct connectdata *conn);
293+
void Curl_ssl_detach_conn(struct Curl_easy *data,
294+
struct connectdata *conn);
295+
286296
#define SSL_SHUTDOWN_TIMEOUT 10000 /* ms */
287297

288298
#else /* if not USE_SSL */
@@ -309,6 +319,8 @@ bool Curl_ssl_false_start(void);
309319
#define Curl_ssl_cert_status_request() FALSE
310320
#define Curl_ssl_false_start() FALSE
311321
#define Curl_ssl_tls13_ciphersuites() FALSE
322+
#define Curl_ssl_associate_conn(a,b) Curl_nop_stmt
323+
#define Curl_ssl_detach_conn(a,b) Curl_nop_stmt
312324
#endif
313325

314326
#endif /* HEADER_CURL_VTLS_H */

Diff for: lib/vtls/wolfssl.c

+3-1
Original file line numberDiff line numberDiff line change
@@ -1125,7 +1125,9 @@ const struct Curl_ssl Curl_ssl_wolfssl = {
11251125
Curl_none_set_engine_default, /* set_engine_default */
11261126
Curl_none_engines_list, /* engines_list */
11271127
Curl_none_false_start, /* false_start */
1128-
wolfssl_sha256sum /* sha256sum */
1128+
wolfssl_sha256sum, /* sha256sum */
1129+
NULL, /* associate_connection */
1130+
NULL /* disassociate_connection */
11291131
};
11301132

11311133
#endif

0 commit comments

Comments
 (0)