[wp-trac] [WordPress Trac] #26010: SSL via `WP_Http_Curl` breaks on HTTP version mismatch

[WordPress Trac]

#26010: SSL via `WP_Http_Curl` breaks on HTTP version mismatch
--------------------------+------------------------------
 Reporter:  soulseekah    |       Owner:
     Type:  defect (bug)  |      Status:  new
 Priority:  normal        |   Milestone:  Awaiting Review
Component:  HTTP          |     Version:  3.7
 Severity:  minor         |  Resolution:
 Keywords:                |
--------------------------+------------------------------

Comment (by soulseekah):

 Thanks for your input @dd32. My apologies for going offtopic, but I'm
 trying to understand what happened to get to the bottom of things:

 http://www.openssl.org/docs/ssl/SSL_read.html return code 0:

 > The read operation was not successful. The reason may either be a clean
 shutdown due to a `close notify` alert sent by the peer (in which case the
 `SSL_RECEIVED_SHUTDOWN` flag in the ssl shutdown state is set (see
 `SSL_shutdown`(3), `SSL_set_shutdown`(3)). It is also possible, that the
 peer simply shut down the underlying transport and the shutdown is
 incomplete. Call SSL_get_error() with the return value ret to find out,
 whether an error occurred or the connection was shut down cleanly
 (`SSL_ERROR_ZERO_RETURN`).
 >
 > SSLv2 (deprecated) does not support a shutdown alert protocol, so it can
 only be detected, whether the underlying connection was closed. It cannot
 be checked, whether the closure was initiated by the peer or by something
 else.

 Seems clear, but then this is confusing:

 https://github.com/bagder/curl/commit/8a7a277c086199b37c07a8e01165168037866f3e:

 > ...because the OpenSSL documentation is wrong and a zero return code is
 not
 at all an error case in many situations.

 That documentation changed last 5 years ago
 https://github.com/openssl/openssl/commits/master/doc/ssl/SSL_read.pod and
 seems to be correct. It clearly states that `0` is not always right and
 that further checks are due.

 Emphasizing:

 > Call SSL_get_error() with the return value ret to find out, whether an
 error occurred or the connection was shut down cleanly
 (`SSL_ERROR_ZERO_RETURN`).

 So what do the cURL developers do overall? Initially they don't consider
 `0` to be an error at all, things seem to work fine. Then they consider
 `0` to be an error and all hell breaks loose, because sometimes
 `SSL_get_error` returned uncaught errors (beyond `SSL_ERROR_NONE`,
 `SSL_ERROR_ZERO_RETURN`, `SSL_ERROR_WANT_READ` and
 `SSL_ERROR_WANT_WRITE`).

 In
 https://github.com/bagder/curl/blob/8a7a277c086199b37c07a8e01165168037866f3e/lib/ssluse.c#L2595
 they introduce further checks by calling `ERR_get_error`
 (http://www.openssl.org/docs/crypto/ERR_get_error.html) which:

 > ...returns the earliest error code from the thread's error queue and
 removes the entry.

 I'm a bit confused now, though. Since the documentation appears to be
 right all along, did the cURL developers misaccuse and blame OpenSSL for
 their fiasco? Are they now relying on errors returned from the OpenSSL
 library when SSL/TLS returns an unexpected I/O error? How reliable is
 this? This is akin to ignoring the original error, since library call
 errors and I/O call errors are separate and do not depend on one another
 (although a library call error will probably result in an I/O error, the
 reverse is not true). I'm most surely missing something by making some
 assumptions, but how is their strategy of checking a different error stack
 different from ignoring the I/O error code with value 0 to begin with?

 Probably wrong place to ask. Ticket can be closed.

--
Ticket URL: <http://core.trac.wordpress.org/ticket/26010#comment:7>
WordPress Trac <http://core.trac.wordpress.org/>
WordPress blogging software