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.

Sign up for GitHub

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

openssl s_server causes curl with Schannel to hang #12894

Closed
jay opened this issue Feb 8, 2024 · 2 comments
Closed

openssl s_server causes curl with Schannel to hang #12894

jay opened this issue Feb 8, 2024 · 2 comments
Labels
TLS Windows Windows-specific

Comments

@jay
Copy link
Member

jay commented Feb 8, 2024 •
edited

I did this

Console 1:

openssl s_server -accept 4433 -cert [removed] -keylogfile keys -timeout -www

Console 2:

curld -k -v https://localhost:4433/

OpenSSL serves an HTTP/1.0 status page without content length so curl keeps reading until the connection is closed.

In the case of Schannel, all data is received at once (including the server's close notify) and schannel_recv processes all records at once (including the close_notify) and then returns the body content. readwrite_data does not make further calls to schannel_recv (which would return 0 if called again) because data_pending returns false.

I found this issue while investigating #12885 and the recent mailing list report which were fixed by #12848. However my report is not the same and reproduces regardless of the fix. It precedes connection filters and I was able to reproduce from master all the way back to early versions of curl such as 7.45.0.

I am not sure if other backends are affected, it depends on how they process records. I could not reproduce with OpenSSL 3.0 as a backend.

I expected the following

I expected it not to hang of course...

I can force readwrite_data to call schannel_recv again by treating connection closed and close notify messages "data pending" even though there's no data. In these cases, schannel_recv would return 0. This seems like an ok solution, unless there is some pitfall I'm not thinking of. I'd like to get some thoughts on it, or maybe there's something more elegant to be done in readwrite_data?

diff --git a/lib/vtls/schannel.c b/lib/vtls/schannel.c
index 45c3373..c1edc29 100644
--- a/lib/vtls/schannel.c
+++ b/lib/vtls/schannel.c
@@ -2443,7 +2443,9 @@ static bool schannel_data_pending(struct Curl_cfilter *cf,
 
   if(backend->ctxt) /* SSL/TLS is in use */
     return (backend->decdata_offset > 0 ||
-            (backend->encdata_offset > 0 && !backend->encdata_is_incomplete));
+            (backend->encdata_offset > 0 && !backend->encdata_is_incomplete) ||
+            backend->recv_connection_closed ||
+            backend->recv_sspi_close_notify);
   else
     return FALSE;
 }

curl/libcurl version

master ef4bd8d 2024-02-07

curl 8.6.1-DEV (i386-pc-win32) libcurl/8.6.1-DEV Schannel WinIDN
Release-Date: [unreleased]
Protocols: dict file ftp ftps gopher gophers http https imap imaps ipfs ipns ldap ldaps mqtt pop3 pop3s rtsp smb smbs smtp smtps telnet tftp
Features: alt-svc AsynchDNS Debug HSTS HTTPS-proxy IDN IPv6 Kerberos Largefile NTLM SPNEGO SSL SSPI Unicode UnixSockets

operating system

Windows, any
@jay jay added the Windows Windows-specific label Feb 8, 2024
@jay
Copy link
Member Author

jay commented Feb 8, 2024

Possibly related: #5284 #7068 #7615 #9161

/cc @JDepooter

@bagder bagder added the TLS label Feb 8, 2024
@bagder
Copy link
Member

bagder commented Feb 8, 2024

This seems like an ok solution

Me too. 👍

jay added a commit to jay/curl that referenced this issue Feb 9, 2024
@jay
schannel: fix hang on unexpected server close
aa6c2e7 
- Treat TLS connection close (either due to a close_notify from the
  server or just closed due to receiving 0) as pending data.

This is because in some cases schannel_recv knows the connection is
closed but has to return actual pending data so it can't return 0 to
indicate no more data. In this case schannel_recv must be called again,
which only happens if readwrite_data sees that there is still pending
data.

Prior to this change if the total size of the body that libcurl expected
to receive from the server was unknown then it was possible under some
network conditions that libcurl would hang waiting to receive more data,
when in fact a close_notify alert indicating no more data would be sent
was already processed.

Fixes curl#12894
Closes #xxxx
@jay jay mentioned this issue Feb 9, 2024
Closed
@jay jay closed this as completed in 24d6c28 Feb 13, 2024
jay added a commit to jay/curl that referenced this issue Mar 3, 2024
@jay
KNOWN_BUGS: FTPS server compatibility on Windows with Schannel
f9f3208 
- Remove "2.12 FTPS with Schannel times out file list operation"

- Remove "7.12 FTPS directory listing hangs on Windows with Schannel"

- Add "7.12 FTPS server compatibility on Windows with Schannel"

This change adds a more generic bug description that explains FTPS with
the latest curl and Schannel is not widely used and may have more bugs
than other TLS backends.

The two removed FTPS Schannel bugs can't be reproduced any longer and
were likely fixed by 24d6c28.

Ref: curl#5284
Ref: curl#9161
Ref: curl#12894

Closes #xxxx
@jay jay mentioned this issue Mar 3, 2024
Closed
jay added a commit that referenced this issue Mar 6, 2024
@jay
KNOWN_BUGS: FTPS server compatibility on Windows with Schannel
2ea178a 
- Remove "2.12 FTPS with Schannel times out file list operation"

- Remove "7.12 FTPS directory listing hangs on Windows with Schannel"

- Add "7.12 FTPS server compatibility on Windows with Schannel"

This change adds a more generic bug description that explains FTPS with
the latest curl and Schannel is not widely used and may have more bugs
than other TLS backends.

The two removed FTPS Schannel bugs can't be reproduced any longer and
were likely fixed by 24d6c28.

Ref: #5284
Ref: #9161
Ref: #12894

Closes #13032
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
TLS Windows Windows-specific
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

schannel: fix hang on unexpected server close jay/curl
2 participants
@bagder @jay