-
-
Notifications
You must be signed in to change notification settings - Fork 6.9k
HTTP/3 proxy CONNECT and MASQUE CONNECT-UDP support for OpenSSL QUIC #18331
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Thanks for this! What's your thoughts and plans for tests for these new features? The OpenSSL-QUIC backend is still marked experimental in curl while the ngtcp2 one is not. How come you focus your implementation on that? |
## CURLOPT_HTTPPROXYUDPTUNNEL | ||
UDP Tunnel through the HTTP proxy. CURLOPT_HTTPPROXYUDPTUNNEL(3) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we elaborate on this? When or why would a user want to enable this and tunnel UDP explicitly?
I am missing test cases for this functionality without which we would be unable to maintain this much code. |
Wouldn't it make sense to have at least these tests as a start?
|
*pbio_addr = ba; | ||
return result; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Above this point there is OpenSSL-QUIC specific code. Since we want to have h3 support used for either backend, it should probably be moved over to the appropriate h3 backend source file. curl_osslq.c
for OpenSSL-QUIC code.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as outdated.
This comment was marked as outdated.
(2) MASQUE CONNECT-UDP support in cURL for OpenSSL QUIC stack Signed-off-by: Aritra Basu <aritrbas@cisco.com>
Signed-off-by: Aritra Basu <aritrbas@cisco.com>
Signed-off-by: Aritra Basu <aritrbas@cisco.com>
59eb27e
to
e4ec622
Compare
All the testing I had done prior to raising the PR involved manual testing on Ubuntu22.04 using the following setups:
I was more familiar with the OpenSSL code (such as the use of OpenSSL BIO) and never worked on the ngtcp2 stack before 😅. That's why I picked this up on the OpenSSL-QUIC backend. I can try making the changes on ngtcp2, but given my unfamiliarity with the code, it might take some time to get it in a working state.
Yes, I have added these tests for a start.
Addressed the merge conflicts and updated the basic tests.
Sorry for the delayed response on this, got busy with multiple other things. As things stand now, I still need to make some fixes to limit the code to OpenSSL specific HTTP/3 builds - that's the cause of majority of the CI failures now on this branch. Following that, I will try to look at porting the changes to the ngtcp2 backend and working on further review comments / testing requirements. |
I think it would be useful to focus the efforts there. I think there is a non-zero risk that OpenSSL-QUIC ends up a dead end and never becomes production ready in curl. Also for performance reasons.
That seems like a reasonable path to go down, at least for the moment. It should support the necessary mechanisms. We just need to work a little to get that baby into the CI builds as well. (It could be worth noticing however that h2o performs worse than some of the other HTTP/3 servers in raw transfer performance but I don't think it should matter for this testing) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some suggestions!
|
||
result = curlx_dyn_addn(dyn, &ctx_id, sizeof(ctx_id)); | ||
|
||
result = curlx_dyn_addn(dyn, buf, blen); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This sequence overwrites the result
value, risking to hide an error reported in an earlier call.
capsule_length = capsule_decode_varint(&decode_ptr); | ||
|
||
if(capsule_length == HTTP_INVALID_VARINT) { | ||
infof(data, "Error! Invalid varint length encoding"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
infof(data, "Error! Invalid varint length encoding"); | |
failf(data, "Error! Invalid varint length encoding" 8000 ); |
break; | ||
|
||
if(*context_id) { | ||
infof(data, "Error! Invalid context ID: %02x", *context_id); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
infof(data, "Error! Invalid context ID: %02x", *context_id); | |
failf(data, "Error! Invalid context ID: %02x", *context_id); |
bytes_to_read, &bytes_read); | ||
|
||
if(bytes_read != bytes_to_read) { | ||
infof(data, "Error! Read less than expected %zu %zu", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
infof(data, "Error! Read less than expected %zu %zu", | |
failf(data, "Error! Read less than expected %zu %zu", |
|
||
/* Verify we read the entire capsule */ | ||
if(remaining_capsule_length > 0) { | ||
infof(data, "Error! Could not fit entire capsule: remaining=%zu", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
infof(data, "Error! Could not fit entire capsule: remaining=%zu", | |
failf(data, "Error! Could not fit entire capsule: remaining=%zu", |
if(cf->conn->bits.udp_tunnel_proxy) | ||
infof(data, "CONNECT-UDP phase completed for HTTP proxy"); | ||
else | ||
infof(data, "CONNECT phase completed for HTTP proxy"); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if(cf->conn->bits.udp_tunnel_proxy) | |
infof(data, "CONNECT-UDP phase completed for HTTP proxy"); | |
else | |
infof(data, "CONNECT phase completed for HTTP proxy"); | |
infof(data, "CONNECT%s phase completed for HTTP proxy". | |
cf->conn->bits.udp_tunnel_proxy ? "-UDP": ""); |
nread = nghttp3_conn_read_stream(ctx->h3.conn, s->id, | ||
buf, blen, 0); | ||
if(nread < 0) { | ||
failf(data, "nghttp3_conn_read_stream(len=%zu) error: %s", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As it sets result to CURLE_OK below here, should this perhaps be a plain infof() instead?
free(ctx->poll_items); | ||
ctx->poll_items = NULL; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
free(ctx->poll_items); | |
ctx->poll_items = NULL; | |
Curl_safefree(ctx->poll_items); |
free(ctx->curl_items); | ||
ctx->curl_items = NULL; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
free(ctx->curl_items); | |
ctx->curl_items = NULL; | |
Curl_safefree(ctx->curl_items); |
size_t Curl_capsule_process_udp(struct Curl_cfilter *cf, | ||
struct Curl_easy *data, | ||
struct bufq *recvbufq, | ||
char *buf, size_t len, CURLcode *err); | ||
|
||
#endif /* !CURL_DISABLE_PROXY && !CURL_DISABLE_HTTP && | ||
USE_NGHTTP3 && USE_OPENSSL_QUIC */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
size_t Curl_capsule_process_udp(struct Curl_cfilter *cf, | |
struct Curl_easy *data, | |
struct bufq *recvbufq, | |
char *buf, size_t len, CURLcode *err); | |
#endif /* !CURL_DISABLE_PROXY && !CURL_DISABLE_HTTP && | |
USE_NGHTTP3 && USE_OPENSSL_QUIC */ | |
size_t Curl_capsule_process_udp(struct Curl_cfilter *cf, | |
struct Curl_easy *data, | |
struct bufq *recvbufq, | |
char *buf, size_t len, CURLcode *err); | |
#endif /* !CURL_DISABLE_PROXY && !CURL_DISABLE_HTTP && | |
USE_NGHTTP3 && USE_OPENSSL_QUIC */ |
#define foreach_http_capsule_type _ (0, DATAGRAM) | ||
typedef enum http_capsule_type_ | ||
{ | ||
#define _(n, s) HTTP_CAPSULE_TYPE_##s = n, | ||
foreach_http_capsule_type | ||
#undef _ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#define foreach_http_capsule_type _ (0, DATAGRAM) | |
typedef enum http_capsule_type_ | |
{ | |
#define _(n, s) HTTP_CAPSULE_TYPE_##s = n, | |
foreach_http_capsule_type | |
#undef _ | |
#define foreach_http_capsule_type CURL_HTTP_CAPSULE_ENUM(0, DATAGRAM) | |
typedef enum http_capsule_type_ | |
{ | |
#define CURL_HTTP_CAPSULE_ENUM(n, s) HTTP_CAPSULE_TYPE_##s = n, | |
foreach_http_capsule_type | |
#undef CURL_HTTP_CAPSULE_ENUM |
avoid reserved macro namespace
infof(data, "Establishing HTTP proxy UDP tunnel to %s:%s", | ||
data->state.up.hostname, data->state.up.port); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
infof(data, "Establishing HTTP proxy UDP tunnel to %s:%s", | |
data->state.up.hostname, data->state.up.port); | |
infof(data, "Establishing HTTP proxy UDP tunnel to %s:%s", | |
data->state.up.hostname, data->state.up.port); |
if(Curl_compareheader(header, | ||
STRCONST("Transfer-Encoding:"), | ||
STRCONST("chunked"))) { | ||
CURL_TRC_CF(data, cf, "CONNECT-UDP Response --> " | ||
"Transfer-Encoding: chunked"); | ||
ts->chunked_encoding = TRUE; | ||
/* reset our chunky engine */ | ||
Curl_httpchunk_reset(data, &ts->ch, TRUE); | ||
} | ||
} | ||
else if(checkprefix("Capsule-protocol:", header)) { | ||
if(Curl_compareheader(header, | ||
STRCONST("Capsule-protocol:"), | ||
STRCONST("?1"))) { | ||
CURL_TRC_CF(data, cf, "CONNECT-UDP Response --> Capsule-protocol: ?1"); | ||
} | ||
} | ||
else if(Curl_compareheader(header, | ||
STRCONST("Connection:"), STRCONST("close"))) { | ||
ts->close_connection = TRUE; | ||
CURL_TRC_CF(data, cf, "CONNECT-UDP Response --> Connection: close"); | ||
} | ||
else if(!strncmp(header, "HTTP/1.", 7) && | ||
((header[7] == '0') || (header[7] == '1')) && | ||
(header[8] == ' ') && | ||
ISDIGIT(header[9]) && ISDIGIT(header[10]) && ISDIGIT(header[11]) && | ||
!ISDIGIT(header[12])) { | ||
/* store the HTTP code from the proxy */ | ||
data->info.httpproxycode = k->httpcode = (header[9] - '0') * 100 + | ||
(header[10] - '0') * 10 + (header[11] - '0'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if(Curl_compareheader(header, | |
STRCONST("Transfer-Encoding:"), | |
STRCONST("chunked"))) { | |
CURL_TRC_CF(data, cf, "CONNECT-UDP Response --> " | |
"Transfer-Encoding: chunked"); | |
ts->chunked_encoding = TRUE; | |
/* reset our chunky engine */ | |
Curl_httpchunk_reset(data, &ts->ch, TRUE); | |
} | |
} | |
else if(checkprefix("Capsule-protocol:", header)) { | |
if(Curl_compareheader(header, | |
STRCONST("Capsule-protocol:"), | |
STRCONST("?1"))) { | |
CURL_TRC_CF(data, cf, "CONNECT-UDP Response --> Capsule-protocol: ?1"); | |
} | |
} | |
else if(Curl_compareheader(header, | |
STRCONST("Connection:"), STRCONST("close"))) { | |
ts->close_connection = TRUE; | |
CURL_TRC_CF(data, cf, "CONNECT-UDP Response --> Connection: close"); | |
} | |
else if(!strncmp(header, "HTTP/1.", 7) && | |
((header[7] == '0') || (header[7] == '1')) && | |
(header[8] == ' ') && | |
ISDIGIT(header[9]) && ISDIGIT(header[10]) && ISDIGIT(header[11]) && | |
!ISDIGIT(header[12])) { | |
/* store the HTTP code from the proxy */ | |
data->info.httpproxycode = k->httpcode = (header[9] - '0') * 100 + | |
(header[10] - '0') * 10 + (header[11] - '0'); | |
if(Curl_compareheader(header, | |
STRCONST("Transfer-Encoding:"), | |
STRCONST("chunked"))) { | |
CURL_TRC_CF(data, cf, "CONNECT-UDP Response --> " | |
"Transfer-Encoding: chunked"); | |
ts->chunked_encoding = TRUE; | |
/* reset our chunky engine */ | |
Curl_httpchunk_reset(data, &ts->ch, TRUE); | |
} | |
} | |
else if(checkprefix("Capsule-protocol:", header)) { | |
if(Curl_compareheader(header, | |
STRCONST("Capsule-protocol:"), | |
STRCONST("?1"))) { | |
CURL_TRC_CF(data, cf, "CONNECT-UDP Response --> Capsule-protocol: ?1"); | |
} | |
} | |
else if(Curl_compareheader(header, | |
STRCONST("Connection:"), STRCONST("close"))) { | |
ts->close_connection = TRUE; | |
CURL_TRC_CF(data, cf, "CONNECT-UDP Response --> Connection: close"); | |
} | |
else if(!strncmp(header, "HTTP/1.", 7) && | |
((header[7] == '0') || (header[7] == '1')) && | |
(header[8] == ' ') && | |
ISDIGIT(header[9]) && ISDIGIT(header[10]) && ISDIGIT(header[11]) && | |
!ISDIGIT(header[12])) { | |
/* store the HTTP code from the proxy */ | |
data->info.httpproxycode = k->httpcode = (header[9] - '0') * 100 + | |
(header[10] - '0') * 10 + (header[11] - '0'); |
indent
failf(data, "CONNECT-UDP tunnel failed, response %d", | ||
data->req.httpcode); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
failf(data, "CONNECT-UDP tunnel failed, response %d", | |
data->req.httpcode); | |
failf(data, "CONNECT-UDP tunnel failed, response %d", | |
data->req.httpcode); |
indent
result = Curl_bufq_write(&ctx->tunnel.sendbuf, | ||
(const unsigned char *)curlx_dyn_ptr(&dyn), | ||
curlx_dyn_len(&dyn), pnwritten); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
result = Curl_bufq_write(&ctx->tunnel.sendbuf, | |
(const unsigned char *)curlx_dyn_ptr(&dyn), | |
curlx_dyn_len(&dyn), pnwritten); | |
result = Curl_bufq_write(&ctx->tunnel.sendbuf, | |
(const unsigned char *)curlx_dyn_ptr(&dyn), | |
curlx_dyn_len(&dyn), pnwritten); |
indent
#ifndef ARRAYSIZE | ||
#define ARRAYSIZE(A) (sizeof(A) / sizeof((A)[0])) | ||
#endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#ifndef ARRAYSIZE | |
#define ARRAYSIZE(A) (sizeof(A) / sizeof((A)[0])) | |
#endif |
Please use CURL_ARRAYSIZE()
.
#ifdef SSL_ERROR_WANT_EARLY | ||
case SSL_ERROR_WANT_EARLY: | ||
return "SSL_ERROR_WANT_EARLY"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be SSL_ERROR_WANT_CLIENT_HELLO_CB
. Fixed in openssl.c via 80c10c5.
return "SSL_ERROR_WANT_CONNECT"; | ||
case SSL_ERROR_WANT_ACCEPT: | ||
return "SSL_ERROR_WANT_ACCEPT"; | ||
#ifdef SSL_ERROR_WANT_ASYNC |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#ifdef SSL_ERROR_WANT_ASYNC | |
#ifdef SSL_ERROR_WANT_ASYNC /* OpenSSL 1.1.0+, LibreSSL 3.6.0+ */ |
case SSL_ERROR_WANT_ASYNC: | ||
return "SSL_ERROR_WANT_ASYNC"; | ||
#endif | ||
#ifdef SSL_ERROR_WANT_ASYNC_JOB |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#ifdef SSL_ERROR_WANT_ASYNC_JOB | |
#ifdef SSL_ERROR_WANT_ASYNC_JOB /* OpenSSL 1.1.0+, LibreSSL 3.6.0+ */ |
if(!BIO_ADDR_rawmake(ba, AF_INET6, &sin->sin6_addr, | ||
sizeof(sin->sin6_addr), sin->sin6_port)) { | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
empty if()
block?
CURL_TRC_CF(data, cf, "[%" FMT_PRId64 "] reject remote non-uni-read" | ||
" stream", | ||
stream_id); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CURL_TRC_CF(data, cf, "[%" FMT_PRId64 "] reject remote non-uni-read" | |
" stream", | |
stream_id); | |
CURL_TRC_CF(data, cf, "[%" FMT_PRId64 "] reject remote non-uni-read" | |
" stream", stream_id); |
else if((lib == ERR_LIB_SSL) && | ||
(reason == SSL_R_TLSV13_ALERT_CERTIFICATE_REQUIRED)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
else if((lib == ERR_LIB_SSL) && | |
(reason == SSL_R_TLSV13_ALERT_CERTIFICATE_REQUIRED)) { | |
else if((lib == ERR_LIB_SSL) && | |
(reason == SSL_R_TLSV13_ALERT_CERTIFICATE_REQUIRED)) { |
struct cf_osslq_ctx *ctx = proxy_ctx->osslq_ctx; | ||
curl_int64_t stream_id = (curl_int64_t)SSL_get_stream_id(stream_ssl); | ||
|
||
if(h3->remote_ctrl_n >= ARRAYSIZE(h3->remote_ctrl)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if(h3->remote_ctrl_n >= ARRAYSIZE(h3->remote_ctrl)) { | |
if(h3->remote_ctrl_n >= CURL_ARRAYSIZE(h3->remote_ctrl)) { |
static CURLcode cf_h3_proxy_recv(struct Curl_cfilter *cf, | ||
struct Curl_easy *data, | ||
char *buf, size_t len, size_t *pnread) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
static CURLcode cf_h3_proxy_recv(struct Curl_cfilter *cf, | |
struct Curl_easy *data, | |
char *buf, size_t len, size_t *pnread) | |
static CURLcode cf_h3_proxy_recv(struct Curl_cfilter *cf, | |
struct Curl_easy *data, | |
char *buf, size_t len, size_t *pnread) |
result = Curl_bufq_cread(&proxy_ctx->inbufq, | ||
buf, len, pnread); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
result = Curl_bufq_cread(&proxy_ctx->inbufq, | |
buf, len, pnread); | |
result = Curl_bufq_cread(&proxy_ctx->inbufq, | |
buf, len, pnread); |
struct Curl_easy *data, | ||
int query, int *pres1, void *pres2) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
struct Curl_easy *data, | |
int query, int *pres1, void *pres2) | |
struct Curl_easy *data, | |
int query, int *pres1, void *pres2) |
This adds (1) HTTP/3 proxy CONNECT support and (2) MASQUE CONNECT-UDP support for OpenSSL QUIC stack.
Usage:
curl -k -4 --http1.1 --proxy-insecure --proxytunnel --proxy-http3 --proxy https://<proxy> https://<target>
curl -k -4 --http2 --proxy-insecure --proxytunnel --proxy-http3 --proxy https://<proxy> https://<target>
curl -k -4 --http3-only --proxy-insecure --proxyudptunnel --proxy https://<proxy> https://<target>
curl -k -4 --http3-only --proxy-insecure --proxyudptunnel --proxy-http2 --proxy https://<proxy> https://<target>
curl -k -4 --http3-only --proxy-insecure --proxyudptunnel --proxy-http3 --proxy https://<proxy> https://<target>
References:
RFC 9297- HTTP Datagrams and the Capsule Protocol
RFC 9298 - Proxying UDP in HTTP