HTTP requests fails with "SSL read: error:00000000:lib(0):func(0):reason( 0), errno 0"

[ocean90]

I'm having a user where the HTTP requests fails via SSL.

$request contains a WP_Error object:

object(WP_Error)#6595 (2) {
  ["errors"]=>
  array(1) {
    ["http_request_failed"]=>
    array(1) {
      [0]=>
      string(58) "SSL read: error:00000000:lib(0):func(0):reason(0), errno 0"
    }
  }
  ["error_data"]=>
  array(0) {
  }

Used transport: cURL, curl_errno() is 56.

PHP Version: 5.3.27
SSL Version: OpenSSL/0.9.8q
Curl Version: 7.31.0
Curl SSL: yes
OpenSSL support: enabled
OpenSSL Library Version: OpenSSL 0.9.8q 2 Dec 2010
OpenSSL Header Version: OpenSSL 0.9.8q 2 Dec 2010

[nacin]

This is not good, because then no updates work at all. In fact, they are trapped on 3.7 and won't be able to update. (Something in the Hotfix plugin will be necessary.)

We *need* an escape hatch for this. If necessary, we can simply double-up on the request — if the SSL one fails with http_request_failed, try again with non-SSL.

[rmccue]

This makes me sad. :(

I'll try and replicate ASAP, but agreed with nacin, we should try without SSL if SSL fails, but not always. The only way to detect if it's from SSL is to search the error string for "SSL" (booo; I'm working on a way of improving this for both cURL and sockets).

Note that if we silently send a non-SSL request, we lose any benefit that SSL gives us. IMO, the best option is to warn with an AYS.

[nacin]

Replying to rmccue:

Note that if we silently send a non-SSL request, we lose any benefit that SSL gives us. IMO, the best option is to warn with an AYS.

Well, not entirely. We still only serve auto-updates from the WP.org API if the request doesn't come through SSL. For 3.7, we need to contain the damage by always trying non-SSL when a request fails.

[rmccue]

Replying to nacin:

Well, not entirely. We still only serve auto-updates from the WP.org API if the request doesn't come through SSL. For 3.7, we need to contain the damage by always trying non-SSL when a request fails.

If you're on a site without SSL support and you attempt an update, without an AYS you'll never know if you got MITM'd. That's a huge security issue, which is why IMO we should at least issue a warning.

[nacin]

25716.2.diff​ introduces a new string (we may just keep it in English for 3.7.1, as core language packs are not up-and-running yet) and reuses an existing one.

The only problem with a warning is that it could cause headers-already-sent issues. Probably a few situations in updates and such where that can be a problem.

Issuing just a notice when headers_sent() || WP_DEBUG ? E_USER_WARNING : E_USER_NOTICE is cheap but it works.

This patch was lightly tested, but I probably screwed something up. Each piece must be tested.

My testing code:

add_filter( 'pre_http_request', function( $result, $args, $url ) {
	if ( 0 === strpos( $url, 'https://' ) )
		return new WP_Error( 'ssl_failed' );
	return $result;
}, 10, 3 );

Also, here's some code someone can use to fix their site:

add_filter( 'use_curl_transport', 'wp_ticket_25716_ssl_not_supported', 10, 2 );
add_filter( 'use_streams_transport', 'wp_ticket_25716_ssl_not_supported', 10, 2 );
function wp_ticket_25716_ssl_not_supported( $test, $args ) {
    if ( ! empty( $args['ssl'] ) )
        return false;
    return $test;
}

[nacin]

rmccue approved this approach before he went off to bed.

[nacin]

Only issue E_WARNING if WP_DEBUG or headers_sent(), otherwise E_NOTICE.

[nacin]

Could use some more eyes and testing, but 25716.3.diff​ works for me. This patch falls back to E_USER_NOTICE if headers aren't set (and ! WP_DEBUG).

[nacin]

You'll not that 25716.3.diff​ is not DRY. We can make it DRY in 3.8. In a point release, I want to limit cross-file dependencies. If a critical update error occurs and not all files copy, I don't want it to end up having a fatal error because the new function's file was what wasn't copied over. Ideally, *any* file can not update and everything will still be OK.

[nacin]

Hotfix...

[ocean90]

Back to the original error code:
It seems to be related to a specific curl version, 7.31.0, see ​http://sourceforge.net/p/curl/bugs/1249/. The related commit is ​https://github.com/bagder/curl/commit/8a7a277c086199b3 which should be in cURL 7.32.0.

25716.3.diff​ looks good so far and works on the broken site.

Hotfix: The broken site drives me crazy.

• wp_remote_get( 'https://api.wordpress.org/' ) works
• wp_remote_post('https://api.wordpress.org/core/checksums/1.0/?version=3.7') works
• wp_remote_post( 'https://api.wordpress.org/', array( 'body' => array( 'foo' => 'bar' ) ) ) doesn't work

Because of this, wp_http_supports( array( 'ssl' ) ) && is_wp_error( wp_remote_get( 'https://api.wordpress.org/' ) ) is false.

[dd32]

As a quick fix, 25716.3.diff​ looks OK.

There's a thread in the support forums for something similar:
​http://wordpress.org/support/topic/37-search-for-plugins-in-cp-gives-unexpected-error?replies=15#post-4807261

I had thrown together a quick plugin to output some info about HTTPS connections too:
​http://dd32.id.au/uploads/2013/10/https-debugger.zip

cURL 7.31.0 appears affected from that thread, but potentially also 7.33.0 (I'm about to ask for clarification)

[ocean90]

Replying to dd32:

I had thrown together a quick plugin to output some info about HTTPS connections too:
​http://dd32.id.au/uploads/2013/10/https-debugger.zip

cURL 7.31.0 appears affected from that thread, but potentially also 7.33.0 (I'm about to ask for clarification)

That's cool! All tests passed. Maybe we/you should add a test for each method with and without a body.

[dd32]

That's cool! All tests passed. Maybe we/you should add a test for each method with and without a body.

Now that we know that's a issue, I've added a check.
If I can get homebrew to play nicely with OSX 10.9 I'll be able to test those curl versions directly.

[dd32]

If I can get homebrew to play nicely with OSX 10.9 I'll be able to test those curl versions directly.

7.30.0, 7.31.0, 7.32.0, and, 7.33.0 CLI doesn't appear to trigger it for me with a simple payload of foo=bar, it might have something to do with the version of OpenSSL it's compiled against too.

I think the good news here, is that limited environments will be affected, as it's requires a version of cURL released between 3 and 4 months ago, so it'll probably only affect a limited set of PHP 5.5 users (and only those who use a distro that actually keeps up to date with packages, none of my systems that are up to date have curl version more recent than 3 years old..)

[dd32]

A few alternative approaches:

If we just want to blacklist that version of cURL for HTTPS requests, we can do that easily with 25716.wp_http.diff​.

Of course that doesn't protect against future (or still unknown, past) bugs. To cover that we could put a counter in WP_HTTP that increments on SSL failures, 3 or more within a 24hr period (core,plugin,theme update checks twice daily, or a user hitting a failure page) to a known good domain *.wordpress.org, disables that transports SSL abilities for a larger-than-24hr period (1 week?) - If it was caused by cURL, hopefully then the Streams transport would be able to pick up the slack for HTTPS requests. If it can't, then the update checks will revert to HTTP anyway.

Now that we only have 2 HTTP transports, we could also make WP_HTTP::_dispatch_request() failover to the 2nd transport in the case where the first request fails, potentially only for a whitelisted set of domains (ie. *.wordpress.org). That is also not ideal though as it doubles the length of the HTTP timeouts which users might see.

[dd32]

Replying to dd32:

cURL 7.31.0 appears affected from that thread, but potentially also 7.33.0 (I'm about to ask for clarification)

Good news from the thread is that 7.33.0 fixes it, so it's most likely just 7.31.0 that's affected, which confirms ocean90's comments in comment:10

[dd32]

Also appears to affect Ubuntu versions of PHP where cURL is compiled with certain versions of GnuTLS (instead of OpenSSL), one again simple HTTPS requests succeed, post with a body fails:
​https://bugs.launchpad.net/ubuntu/+source/gnutls26/+bug/1111882

[PASS]: cURL is installed and supports SSL communication, cURL Details: version_number=464384; age=3; features=50749; ssl_version_number=0; version=7.22.0; host=x86_64-pc-linux-gnu; ssl_version=GnuTLS/2.12.14; libz_version=1.2.3.4; protocols=dict,file,ftp,ftps,gopher,http,https,imap,imaps,ldap,pop3,pop3s,rtmp,rtsp,smtp,smtps,telnet,tftp
[PASS]: [cURL] Communication with WordPress.org suceeded, it took 0.908 seconds
[FAIL]: [cURL with a POST body] Communication with WordPress.org failed with error: [http_request_failed]: GnuTLS recv error (-9): A TLS packet with unexpected length was received.

It appears that GnuTLS will emit that failure when it encounters a server that it doesn't believe adhere to the SSL spec, so the above patch that tests for the known-bad cURL version won't help.

[dd32]

FWIW I'm not against 25716.3.diff​ either, I think that's a good choice for patching over this for now in a .1 release.

What I don't like is that the cURL handler claims to support SSL when it can't make successful requests, yet, we have a streams handler which most likely could've (in some environments at least).

[nacin]

In 25956:

When an HTTPS request to api.wordpress.org fails, try an insecure HTTP request and issue a warning.

Certain versions of cURL appear to claim OpenSSL support but fail to work. We need to not trap users on older versions while we work this out, and instead fall back to an insecure request.

see #25716 for trunk.

[nacin]

In 25957:

When an HTTPS request to api.wordpress.org fails, try an insecure HTTP request and issue a warning.

Certain versions of cURL appear to claim OpenSSL support but fail to work. We need to not trap users on older versions while we work this out, and instead fall back to an insecure request.

Merges [25956] to the 3.7 branch.
fixes #25716 for the 3.7 branch.

[nacin]

This is still better than 3.6. And, we still only serve auto-updates when an API request works over SSL.

Let's open a new ticket to track adjustments to this for 3.8.

[scribu]

Certain versions of cURL appear to claim OpenSSL support but fail to work.

I don't think that's the whole story. At least for environments with GnuTLS, requests made by WP 3.6 work, while requests made by WP 3.7 fail. More context: ​https://github.com/wp-cli/wp-cli/issues/846

[dd32]

Replying to scribu:

Certain versions of cURL appear to claim OpenSSL support but fail to work.

I don't think that's the whole story. At least for environments with GnuTLS, requests made by WP 3.6 work, while requests made by WP 3.7 fail. More context: ​https://github.com/wp-cli/wp-cli/issues/846

GnuTLS refuses to talk to a standard-configured nginx server over SSL, that's the problem with cURL on the travis system. Put basically, when cURL is used with OpenSSL it ignores non-fatal SSL errors that nginx can trigger, when compiled with GnuTLS however, it cannot determine if it's a fatal ssl error or not, and just fails 100% of the time.
(If you have cURL compiled with GnuTLS on the command line, I doubt that can make a POST to ​https://api.wordpress.org either)

The context that you're missing, is that in 3.6 all API requests went over HTTP, in 3.7 all API requests go over HTTPS if supported else HTTP, and in 3.7.1 all API requests first go over HTTPS if supported, and fall back to HTTP in event of failure (but triggers a PHP Notice/Warning).

---

The tech details:
The cause problem with cURL 7.31.0 w/ OpenSSL, and with cURL w/ GnuTLS in general is caused by the connection shutting down before the SSL connection has been properly shutdown, this isn't a fatal issue as the data has already been received and sent by both parties - it's simply like your great aunt saying goodbye and hugging you another dozen times, nginx cuts her off and says get in the car.

The root cause is that nginx applies a set of heuristics to open connections to determine when it believes it can close the connection, and closes it before that SSL shutdown handshake is done, so really, it's that nginx takes a shortcut that works with every client except GnuTLS (which sticks to the standard to a dot - which can be the wrong thing to do when dealing with 3rd party software (web servers) ).

nginx can be configured to not close connections early by using it's lingering_close off; directive, the problem is that it can affect performance in a few unknown cases since it's not documented very well.. but we're looking into it.

One can also say it's a bug in nginx's heuristics, and I'd argue that's true, since standard HTTPS GET's work, it's just HTTPS POST's that fail.

[barry]

lingering_close disabled on WordPress.org. Can you please re-test? Thanks.

[dd32]

Replying to barry:

lingering_close disabled on WordPress.org. Can you please re-test? Thanks.

FWIW, my local PHP+Curl-GnuTLS install now works.

[ocean90]

Replying to barry:

lingering_close disabled on WordPress.org. Can you please re-test? Thanks.

On users host it works now too.

[nacin]

Thanks barry! Closing this out for 3.7.1. Related: #25751. New problems can move into new tickets.

[jewelry1]

[spam]