openssl: Revert to less sensitivity for SYSCALL errors (releases only) #4623
Conversation
|
Fixes #4624 LIBCURL_DEV_BUILD defined: LIBCURL_DEV_BUILD undefined: |
|
I'm for reverting that behavior change. But I don't like the |
|
I considered DEBUGBUILD but I'd guess there are very few people running that in practice. If users are building from the repo I would assume they're almost always building a development build. To me it seems reasonable, as any development build may have issues that would not be in a release build. |
I think we would want that to be true but I fear it is not. There are even cases of operating systems building curl like that and shipping it to billions of users. We have rather large amount of users who build straight from git and deploy to production. |
- 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
force-pushed
the
ossl_rollback_syscall_on_read
branch
from
3eeac3f
to
d741f50
Compare
|
Ok I changed it to DEBUGBUILD. Also I improved on the SYSCALL error message logic for ossl_send and ossl_recv. |
|
This change breaks Facebook oembeds from WordPress. Took us a while to track it down to the exact curl version. Is the curl version that fixes this known at this point? |
|
This regression is (only) present in 7.67.0. The fix is in git and will be part of the pending 7.68.0 release. |
|
@bagder Do you have an ETA on the release? This broke some things for my client too and if the release is soon I can hold off updating the AMI rather than rolling back. |
|
Next release date is always present here. 7.68.0 is set to ship on January 8th 2020. |
|
curl 7.68 is out and fixes this issue. |
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 curl release builds but remains enabled in curl development
builds. If after a period of time there are no major issues then it can
be enabled for OpenSSL 1.1.1+ in curl release builds.
Bug: #4409 (comment)
Reported-by: Bjoern Franke
Closes #xxxx
/cc @bjo81