Skip to content
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

Jump to bottom

http/3: CURL does not send connection close message #8534

Closed
mehatzri opened this issue Mar 3, 2022 · 6 comments
Closed

http/3: CURL does not send connection close message #8534

mehatzri opened this issue Mar 3, 2022 · 6 comments
Assignees
@bagder
Labels
HTTP/3 h3 or quic related

Comments

@mehatzri
Copy link

mehatzri commented Mar 3, 2022 •
edited

I did this

I sent an HTTP3 request with CURL (NGTCP2 version) to my http3 server by:
curl -k --http3 https://1.1.1.1:443

I expected the following

After curl received the file, i expect it to send a CONNECTION_CLOSE message.
However no CONNECTION_CLOSE is sent.
Further health-check packets from server gets "ICMP port unreachable" response.
This suggests that the client disconnected and close the connection,
but without sending close message, as it should.

curl/libcurl version

This happens only on latest curl based on NGTCP2.
issue is NOT seen with QUICHE version.

[curl -V output]
curl 7.82.0-DEV (x86_64-pc-linux-gnu) libcurl/7.82.0-DEV OpenSSL/3.0.0 zlib/1.2.11 ngtcp2/0.3.0-DEV nghttp3/0.3.0-DEV
Release-Date: [unreleased]
Protocols: dict file ftp ftps gopher gophers http https imap imaps mqtt pop3 pop3s rtsp smb smbs smtp smtps telnet tftp
Features: alt-svc AsynchDNS HSTS HTTP3 HTTPS-proxy IPv6 Largefile libz NTLM NTLM_WB SSL TLS-SRP UnixSockets

This looks like a bug in CURL code, in the inegration code with NGTCP2.

Comparing files under curl/lib/vquic
we can see that qs_disconnect function is implemented differently for ngtcp2.c and quiche.c
as the quiche.c implementation calls conn_close while ngtcp.c does not.

operating system

Ubuntu

Linux 23 5.13.0-30-generic #33~20.04.1-Ubuntu SMP Mon Feb 7 14:25:10 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux
@mehatzri mehatzri changed the title CURL does not send connection close message http/3: CURL does not send connection close message Mar 3, 2022
@bagder bagder added the HTTP/3 h3 or quic related label Mar 4, 2022
@bagder
Copy link
Member

bagder commented Mar 5, 2022

@tatsuhiro-t is there any obvious function call missing here?

curl/lib/vquic/ngtcp2.c

Lines 859 to 886 in 64db5c5

static void qs_disconnect(struct quicsocket *qs)
{
if(!qs->conn) /* already closed */
return;
qs->conn = NULL;
if(qs->qlogfd != -1) {
close(qs->qlogfd);
qs->qlogfd = -1;
}
if(qs->ssl)
#ifdef USE_OPENSSL
SSL_free(qs->ssl);
#elif defined(USE_GNUTLS)
gnutls_deinit(qs->ssl);
#endif
qs->ssl = NULL;
#ifdef USE_GNUTLS
if(qs->cred) {
gnutls_certificate_free_credentials(qs->cred);
qs->cred = NULL;
}
#endif
nghttp3_conn_del(qs->h3conn);
ngtcp2_conn_del(qs->qconn);
#ifdef USE_OPENSSL
SSL_CTX_free(qs->sslctx);
#endif
}

@tatsuhiro-t
Copy link
Contributor

yes, ngtcp2_conn_write_connection_close needs to be called there and send the packet with send(2).
To send the appropriate error code, we need a bit more code, but I have a plan to add new construct to ngtcp2 to make it slightly easier for this, so the simplest fix for now is call ngtcp2_conn_write_connection_close with error_code = 0 and send the packet with send(2).

@tatsuhiro-t
Copy link
Contributor

I tried to send CONNECTION_CLOSE in that function, but the socket has already been closed (conn->tempsock[0]).
Is there any hook that is called before closing sockets?

@mehatzri
Copy link
Author

mehatzri commented Mar 7, 2022

I tried to send CONNECTION_CLOSE in that function, but the socket has already been closed (conn->tempsock[0]). Is there any hook that is called before closing sockets?

@bagder ..?

@bagder
Copy link
Member

bagder commented Mar 9, 2022

I tried to send CONNECTION_CLOSE in that function, but the socket has already been closed (conn->tempsock[0]).

conn->sock[FIRSTSOCKET] is the socket in use after the connection has been established.

Is there any hook that is called before closing sockets?

No, as it hasn't been necessary before. It feels like the ngtcp2.c:Curl_quic_disconnect function should be enough. It will (should) be called without the socket having been closed first, as it would be kind of pointless to try any disconnect action if the socket is already closed...

bagder added a commit that referenced this issue Mar 9, 2022
@bagder
ngtcp2: disconnect the QUIC connection proper
7424179 
Fixes #8534
Reported-by: mehatzri on github
@bagder bagder mentioned this issue Mar 9, 2022
Closed
@bagder bagder self-assigned this Mar 9, 2022
@bagder bagder closed this as completed in 96edc79 Mar 10, 2022
@mehatzri
Copy link
Author

thank you!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@bagder bagder

Labels
HTTP/3 h3 or quic related
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

ngtcp2: disconnect the QUIC connection proper curl/curl
3 participants
@bagder @tatsuhiro-t @mehatzri