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

curl 7.67.0 w/OpenSSL: Missing TLS shutdown from server may cause error #4624

Closed
jay opened this issue Nov 20, 2019 · 11 comments
Closed

curl 7.67.0 w/OpenSSL: Missing TLS shutdown from server may cause error #4624

jay opened this issue Nov 20, 2019 · 11 comments
Labels

Comments

@jay
Copy link
Member

jay commented Nov 20, 2019

If an SSL transfer is made from curl 7.67.0 w/ OpenSSL and the server terminates the connection without giving an acceptable protocol termination point (eg in HTTP would be Content Length or chunked) and without the TLS termination point (close_notify alert), then curl error 56 may occur and the associated error message may contain text such as Success or No error.

Other SSL backends such as Schannel (Windows OS native SSL) have stricter behavior where we require a termination point, but I don't believe it was @bagder's intention to do this yet for OpenSSL (presumably because of the wide user base and compatibility). It has only been reported once so far, so we can assume it affects very few users.

Update: Fixed in 78cef06, the extra sensitivity is now disabled except in debug builds. If you are using 7.67.0 you may either cherry-pick that commit from the repo or apply it as a patch.


I did this

@bjo81 reports error curl: (56) OpenSSL SSL_read: Success at the end of transfer of https://www.presseportal.de

I am able to reproduce in Windows using curl-7_67_0 but the error message is different, likely due to OS differences:

> curld https://www.presseportal.de 1>NUL
curl: (56) OpenSSL SSL_read: No error

(In other words socket error is 0.)

SSL_read fails and then SSL_get_error() returns SSL_ERROR_SYSCALL because the server closed the connection without a proper shutdown (ie != SSL_ERROR_ZERO_RETURN). Prior to 0ab38f5 SSL_ERROR_SYSCALL on its own wasn't an error during SSL_read.

SYSCALL is not directly linked to a socket error, therefore the socket error may be 0. That may happen --as it does in this case-- when a server does not have a known termination point and no close_notify is sent, instead the connection is closed.

I expected the following

Debatable. IMHO technically it is correct to pass on the error, but if we were going to make this change I expected it to be rolled out slowly to only dev builds at first and let it marinate. This may be a breaking change for some users. And I don't like that the error text if the socket error is 0 may be "no error" or "success", that's sure to confuse.

Suggested remedy is disable this behavior for release builds, see #4623.

curl/libcurl version

curl 7.67.0 (i386-pc-win32) libcurl/7.67.0 OpenSSL/1.0.2t nghttp2/1.40.0
curl: (56) OpenSSL SSL_read: No error

curl 7.67.0 (i386-pc-win32) libcurl/7.67.0 wolfSSL/4.2.0 nghttp2/1.40.0
(exit code 0)

curl 7.67.0 (i386-pc-win32) libcurl/7.67.0 Schannel WinIDN
curl: (56) Failure when receiving data from the peer

operating system

Windows 7 Enterprise

@bagder
Copy link
Member

bagder commented Nov 21, 2019

Hm, that change was inadvertent on my behalf back then, as it was only intended to improve the error messages for the syscall case, not actually change behavior...

@afragen
Copy link

afragen commented Nov 22, 2019

Interesting. I think I'm seeing the same error when sending a GET request to api.bitbucket.org with Basic Auth headers. Though the site is hosted on Bluehost, so maybe that's it.

cURL version | 7.67.0 OpenSSL/1.0.2t

@Kailashaghera
Copy link

Getting same error "CURL Error: 56" on Linux server using curl 7.67.0..

jay added a commit to jay/curl that referenced this issue Nov 22, 2019
- Disable the extra sensitivity except in debug builds (--enable-debug).

- Improve SYSCALL error message logic in ossl_send and ossl_recv so that
  "No error" / "Success" socket error text isn't shown on SYSCALL error.

Prior to this change 0ab38f5 (precedes 7.67.0) increased the sensitivity
of OpenSSL's SSL_ERROR_SYSCALL error so that abrupt server closures were
also considered errors. For example, a server that does not send a known
protocol termination point (eg HTTP content length or chunked encoding)
_and_ does not send a TLS termination point (close_notify alert) would
cause an error if it closed the connection.

To be clear that behavior made it into release build 7.67.0
unintentionally. So far there is just one user report due to it.

Ultimately the idea is a good one, and other SSL backends may already
behave similarly (such as Windows native OS SSL Schannel). However much
more of our user base is using OpenSSL and there is a mass of legacy
users in that space, so I think that behavior should be partially
reverted and then rolled out slowly.

This commit changes the behavior so that the increased sensitivity is
disabled in all curl builds except curl debug builds (DEBUGBUILD). If
after a period of time there are no major issues then it can be enabled
in dev and release builds with the newest OpenSSL (1.1.1+).

Bug: curl#4409 (comment)
Reported-by: Bjoern Franke

Fixes curl#4624
Closes curl#4623
@jay
Copy link
Member Author

jay commented Nov 22, 2019

This is a very specific case of error 56 where the error message will say there is "No error" or "Success". For reference please include your curl -V and the URL if it is public.

@marcelwirtz
Copy link

marcelwirtz commented Nov 22, 2019

As is said in the other issue request:

I think it's an error related to WinSockets that the socket is closed while curl / OpenSSH tries to use the socket again. For me the problem only seems to be appearing on POST or custom requests like PUT, DELETE and only if there are two or more requests are send right after the other is finished and got data. If i insert a sleep with at least 0.5 seconds between the return and the second request it works.

If i set the CURLOPT_FRESH_CONNECT option it works for me too. Thats why i think it has something todo with reused sockets which are closed to early.

@jay
Copy link
Member Author

jay commented Nov 22, 2019

This issue is for a very specific error, "No error" / "Success" is shown in the error message. As I mentioned in the other thread SYSCALL errors are almost invariably due to server or network issues.

@afragen
Copy link

afragen commented Nov 22, 2019

This is the error I receive when making a GET request using Basic Auth headers to api.bitbucket.org from my site. cURL error 56: OpenSSL SSL_read: Success

The site is hosted on Bluehost. Similar error if I try the same GET request from another site on same host.

These requests were working fine a few days ago.

@jay
Copy link
Member Author

jay commented Nov 23, 2019

Fix landed but leaving open for now to minimize dupes.

@jay jay reopened this Nov 23, 2019
dscho added a commit to git-for-windows/build-extra that referenced this issue Nov 28, 2019
When reading from their gitweb, cURL exits with exit code 56 and this
error message:

	$ curl -o /tmp/a1 "https://git.gnupg.org/cgi-bin/gitweb.cgi?p=gnupg.git;a=tags"
	  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
					 Dload  Upload   Total   Spent    Left  Speed
	100  206k    0  206k    0     0   346k      0 --:--:-- --:--:-- --:--:--  346k
	curl: (56) OpenSSL SSL_read: No error

There is actually no error, but apparently the server is configured
incorrectly, hitting this bug: curl/curl#4624

Let's just work around this by ignoring the exit code 56 (and leisurely
update cURL to take the fix indicated in the ticket).

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
@jay jay pinned this issue Dec 6, 2019
@gvanem gvanem unpinned this issue Dec 10, 2019
@bagder
Copy link
Member

bagder commented Jan 8, 2020

Closing now as this is now fixed in a shipped release.

@bagder bagder closed this as completed Jan 8, 2020
@WouterFlorijn
Copy link

@bagder in which release is this fix included? 7.68.0?

@bagder
Copy link
Member

bagder commented Feb 26, 2020

yes, 7.68.0

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

No branches or pull requests

6 participants