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

Timeout can result in segv in Curl_multiuse_state with nss #8341

Closed
sfc-gh-emusser opened this issue Jan 28, 2022 · 2 comments
Closed

Timeout can result in segv in Curl_multiuse_state with nss #8341

sfc-gh-emusser opened this issue Jan 28, 2022 · 2 comments
Labels
crash TLS

Comments

@sfc-gh-emusser
Copy link

sfc-gh-emusser commented Jan 28, 2022 •
edited

I did this

Using the attached modified curl example program a segv is regularly seen in Curl_multiuse_state when a timeout is forced. This is with NSS (version 3.68.1 RTM). With curl debug assertions enabled the following error is instead emitted:
multi.c:3486: Curl_multiuse_state: Assertion `conn->bundle' failed.

curl-nss-crash.c.txt

The callstack of the assertion failure / crash is:

#4  0x0000000000417a05 in Curl_multiuse_state (data=0x4ea0b8, bundlestate=-1) at multi.c:3486
#5  0x0000000000481ce2 in HandshakeCallback (sock=0x9810e0, arg=0x4ea0b8) at vtls/nss.c:893
#6  0x00007ffff7629eb0 in ssl_FinishHandshake () from /lib64/libssl3.so
#7  0x00007ffff7637fbf in tls13_FinishHandshake () from /lib64/libssl3.so
#8  0x00007ffff763b880 in tls13_SendClientSecondRound () from /lib64/libssl3.so
#9  0x00007ffff763c709 in tls13_HandlePostHelloHandshakeMessage () from /lib64/libssl3.so
#10 0x00007ffff761ca95 in ssl3_HandleHandshakeMessage () from /lib64/libssl3.so
#11 0x00007ffff7620973 in ssl3_HandleNonApplicationData () from /lib64/libssl3.so
#12 0x00007ffff7620f21 in ssl3_HandleRecord () from /lib64/libssl3.so
#13 0x00007ffff76226df in ssl3_GatherCompleteHandshake () from /lib64/libssl3.so
#14 0x00007ffff7623869 in ssl_GatherRecord1stHandshake () from /lib64/libssl3.so
#15 0x00007ffff7629e52 in ssl_Do1stHandshake () from /lib64/libssl3.so
#16 0x00007ffff762ab45 in ssl_SecureRecv () from /lib64/libssl3.so
#17 0x00007ffff762ed11 in ssl_Recv () from /lib64/libssl3.so
#18 0x000000000048318e in close_one (connssl=0x51dac8) at vtls/nss.c:1563
#19 0x0000000000483308 in nss_close (data=0x4ea0b8, conn=0x51d898, sockindex=0) at vtls/nss.c:1620
#20 0x000000000043eff4 in Curl_ssl_close (data=0x4ea0b8, conn=0x51d898, sockindex=0) at vtls/vtls.c:676
#21 0x000000000042fb80 in conn_shutdown (data=0x4ea0b8, conn=0x51d898) at url.c:752
#22 0x000000000043021d in Curl_disconnect (data=0x4ea0b8, conn=0x51d898, dead_connection=true) at url.c:870
#23 0x00000000004124a0 in multi_done (data=0x4ea0b8, status=CURLE_OPERATION_TIMEDOUT, premature=true) at multi.c:664
#24 0x0000000000414a8a in multi_runsingle (multi=0x4fd828, nowp=0x7fffffffdec0, data=0x4ea0b8) at multi.c:2016
#25 0x0000000000415c3c in curl_multi_perform (multi=0x4fd828, running_handles=0x7fffffffdf2c) at multi.c:2559
#26 0x0000000000405fd6 in main () at ../../src/playground.cpp:74

and appears to be due to this change: #7095 which reads any pending close notify alert prior to closing a connection. If there is an issue in how we are calling curl or curl_multi please let us know!

I expected the following

It not to crash.

curl/libcurl version

curl-7_78_0

[curl -V output]
curl 7.78.0-DEV (x86_64-unknown-linux-gnu) libcurl/7.78.0-DEV NSS/3.67 zlib/1.2.7 c-ares/1.17.2 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 Debug HSTS HTTPS-proxy IPv6 Largefile libz NTLM NTLM_WB SSL TrackMemory UnixSockets

operating system

Linux xxxxxxx 3.10.0-1160.49.1.el7.x86_64 #1 SMP Tue Nov 30 15:51:32 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux
bagder added a commit that referenced this issue Jan 28, 2022
@bagder
nss: handshake callback called during shutdown should return early
0ee0606 
It gets called because of the call to PR_Recv() done to attempt to avoid
RST on the TCP connection. This is NSS though so documentation for this
is nowhere to be found, why I opt to side-step this by disabling NPN and
ALPN when the connection is shutting down.

Reported-by: Eric Musser
Fixes #8341
@bagder bagder mentioned this issue Jan 28, 2022
Closed
@bagder
Copy link
Member

bagder commented Jan 28, 2022

I believe #8342 should fix this. Any chance you can try that out and verify that it truly does?

sfc-gh-emusser pushed a commit to sfc-gh-emusser/curl that referenced this issue Jan 28, 2022
@bagder @sfc-gh-emusser
nss: handshake callback called during shutdown should return early
5b5e3a2 
It gets called because of the call to PR_Recv() done to attempt to avoid
RST on the TCP connection. This is NSS though so documentation for this
is nowhere to be found, why I opt to side-step this by disabling NPN and
ALPN when the connection is shutting down.

Reported-by: Eric Musser
Fixes curl#8341
@sfc-gh-emusser
Copy link
Author

sfc-gh-emusser commented Jan 28, 2022 •
edited

Hi Daniel, I've confirmed the issues goes away. Thanks for the incredible turnaround time!

bagder added a commit that referenced this issue Jan 28, 2022
@bagder
nss: handshake callback during shutdown has no conn->bundle
ccbc104 
The callback gets called because of the call to PR_Recv() done to
attempt to avoid RST on the TCP connection. The conn->bundle pointer is
already cleared at this point so avoid dereferencing it.

Reported-by: Eric Musser
Fixes #8341
@bagder bagder closed this as completed in 3267ac4 Jan 28, 2022
sfc-gh-emusser pushed a commit to sfc-gh-emusser/curl that referenced this issue Jan 28, 2022
@bagder @sfc-gh-emusser
nss: handshake callback during shutdown has no conn->bundle
1dcadc8 
The callback gets called because of the call to PR_Recv() done to
attempt to avoid RST on the TCP connection. The conn->bundle pointer is
already cleared at this point so avoid dereferencing it.

Reported-by: Eric Musser
Fixes curl#8341
Closes curl#8342
sfc-gh-emusser pushed a commit to sfc-gh-emusser/curl that referenced this issue Jan 28, 2022
@bagder @sfc-gh-emusser
nss: handshake callback during shutdown has no conn->bundle
a365a6e 
The callback gets called because of the call to PR_Recv() done to
attempt to avoid RST on the TCP connection. The conn->bundle pointer is
already cleared at this point so avoid dereferencing it.

Reported-by: Eric Musser
Fixes curl#8341
Closes curl#8342
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
crash TLS
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

nss: handshake callback called during shutdown should return early curl/curl
2 participants
@bagder @sfc-gh-emusser