Make WordPress Core

Opened 8 years ago

Closed 8 years ago

#34794 closed defect (bug) (invalid)

CURLOPT_SSL_VERIFYHOST should be set to 2 or not be set at all

Reported by: friendlygreg's profile FriendlyGreg Owned by: johnbillion's profile johnbillion
Milestone: Priority: normal
Severity: normal Version:
Component: HTTP API Keywords: needs-patch
Focuses: Cc:

Description

This is a follow-up to #16904.

In the case of a local connection (e.g., for cron) to an instance of OS X Server 5 running with RC4 support shut off in the Server's proxy's cipher suites, setting CURLOPT_SSL_VERIFYHOST at all will cause cause WP_Http_Curl to return WP_Error with SSLRead() return error -9841. While this impacts OS X in a particularly disruptive way regardless of certificate trust, it may also impact other platforms when using self-signed certificates.

When making local connections, with $ssl_verify at false, CURLOPT_SSL_VERIFYHOST should not be set at all, rather than being set to false. (Note that CURLOPT_SSL_VERIFYHOST accepts only integer values, so false is not a valid option anyway.) Applying the same check used in 4.4 (#33978) to fix the incorrect setting of CURLOPT_CAINFO in 4.3:

<?php
        if ( $ssl_verify ) {
                curl_setopt( $handle, CURLOPT_SSL_VERIFYHOST, 2 );
        }

Change History (14)

#1 @johnbillion
8 years ago

  • Focuses performance removed
  • Keywords needs-patch needs-testing added

#2 @johnbillion
8 years ago

  • Milestone changed from Awaiting Review to Future Release
  • Owner set to johnbillion
  • Status changed from new to reviewing

#3 @johnbillion
8 years ago

  • Version 4.3 deleted

Thanks for the ticket and the clear explanation @FriendlyGreg.

IIRC the default value of CURLOPT_SSL_VERIFYHOST is 2, so not setting it won't have the desired effect, but I need to double check.

#4 @johnbillion
8 years ago

  • Keywords reporter-feedback added; needs-testing removed

And yes, according to cURL's docs and PHP's docs, the default value for CURLOPT_SSL_VERIFYHOST is indeed 2.

The boolean false value should get cast to 0, but there may be a bug there. @FriendlyGreg are you able to test this on your OS X Server 5 server using 0 instead of false and verify if there's a change in behaviour?

#5 @FriendlyGreg
8 years ago

  • Keywords reporter-feedback removed

Yes, I've confirmed that setting CURLOPT_SSL_VERIFYHOST to 0 causes the same behaviour. As I understand it, CURLOPT_SSL_VERIFYHOST does not support any values except for 2: the docs indicate that support for value 1 was removed in cURL 7.28.1, and they make no mention of either 0 or false as an acceptable value. I take this to mean that the correct way of setting up curl is not to set CURLOPT_SSL_VERIFYHOST at all unless you want the default (and only) value of 2.

In other words, if I understand correctly, it's not that the default behaviour for a fresh cURL is for CURLOPT_SSL_VERIFYHOST to be set, with a value of 2; it's that the default behaviour for cURL is for CURLOPT_SSL_VERIFYHOST to be switched off completely. Only when we do set it does its default value of 2 come into play.

Edit: Sorry, didn't specify that it's the PHP docs, as distinct from the cURL docs, which don't mention any other valid value except for 2. The cURL docs do mention a value of 0. As far as I can tell, though, the default value for a new connection is still for CURLOPT_SSL_VERIFYHOST to be switched off completely, as distinct from being active with its default value of 2.

Last edited 8 years ago by FriendlyGreg (previous) (diff)

This ticket was mentioned in Slack in #core by rmccue. View the logs.


8 years ago

#7 follow-up: @rmccue
8 years ago

false is an acceptable value for this option, and there's a test that verifies this. The cURL extension in PHP checks only for 1/false, and other values are passed through to cURL directly (as the long value).

From checking the cURL source, I'm not sure the description here matches the actual behaviour. As far as I can tell, peer verification and host verification are completely separate.

The default value for both peer and host verification is 2 (true internally though), so we do need to disable it if verification is turned off.

Per Apple's docs, the returned error code just means the certificate was ignored, so this looks like it might actually be a cURL issue in the DarwinSSL layer.

What's the exact OSX version (10.10 or 10.11, presumably)? Did you compile cURL/PHP yourself, or are you using a pre-built version?

#8 @rmccue
8 years ago

(Related issue from Guzzle.)

#9 in reply to: ↑ 7 @rmccue
8 years ago

FWIW, cannot replicate this on OSX 10.10 or 10.11 using this script. This is using self-signed.badssl.com, not a loopback connection, but per SSL Labs doesn't have RC4 enabled either. Can you try this script and see if it replicates the issue for you please? :)

(Per curl_version(), this is SecureTransport x86_64-apple-darwin14.0 (i.e. OSX built-in), cURL 7.43.0.)

Last edited 8 years ago by rmccue (previous) (diff)

#10 follow-up: @FriendlyGreg
8 years ago

@rmccue, the script you mentioned won't reveal the problem except in the case of connecting back locally to the same server, but in that specific case, yes, it replicates the issue exactly as described. Under both 10.10.5 with Server 5.0.15 and 10.11.1 with Server 5.0.15 -- both plain vanilla, and running PHP 5.5.29 in both cases -- the behaviour is identical. Having already verified this with a plugin that makes test calls to wp_remote_get() and wp_remote_post() before submitting the report, I have now also verified the same using your script -- both dropped into a plugin to ensure the WP environment is up and running and simply calling the script directly. Again, the behaviour is as I have described, failing with SSLRead() return error -9841; comment out the CURLOPT_SSL_VERIFYHOST, and everything works as expected.

As far as I can tell, here's the explanation for what's going on...

Contrary to my suggestion earlier that CURLOPT_SSL_VERIFYHOST is not active unless set, you're exactly right that it is set by default. The reason that commenting out the CURLOPT_SSL_VERIFYHOST line makes the problem go away is not that it prevents CURLOPT_SSL_VERIFYHOST from being active; on the contrary, the reason it makes the problem go away is that it ensures CURLOPT_SSL_VERIFYHOST is active, with a value of 2. When I was testing on a live server using a trusted certificate, that is why everything went smoothly. It was only when I did a separate test with an untrusted certificate that the real problem was revealed (and the clue was your having looked up the specific meaning of the returned error code): on OS X, we receive that error message back, but apparently we don't receive back the rest of the results. Since the code appears to have been meant to be advisory rather than fatal, I'm not sure whether this is a PHP problem or an oversight in Darwin.

So the bad news is that the original bug remains: if the user has shut off local verification in WordPress (e.g., because they're using a self-signed certificate) and the server has shut off RC4 support, then local connections will fail under Server 5 on either 10.10 or 10.11. (Whether it's ultimately 'blamed' on PHP or on Apple, from a WordPress user's perspective, it doesn't matter: the connection doesn't work.)

The good news, however, is that restoring local verification will solve the problem. For those users like me who only shut it off in the first place because WordPress 4.3 introduced problems for Server 4 when it incorrectly set CAINFO, we can switch it back on when running Server 5, and bunnies are happy everywhere.

(Edit: See correction in comment 13 below on why it was initially necessary to shut off local verification.)

Last edited 8 years ago by FriendlyGreg (previous) (diff)

#11 in reply to: ↑ 10 ; follow-ups: @rmccue
8 years ago

Replying to FriendlyGreg:

@rmccue, the script you mentioned won't reveal the problem except in the case of connecting back locally to the same server, but in that specific case, yes, it replicates the issue exactly as described.

As far as I can see, there's nothing special about the loopback connection, apart from being able to control the server? The client should still be connecting via HTTP + SSL over port 443 for remote and loopback connections; I don't think the system does any optimisation there.

Since the code appears to have been meant to be advisory rather than fatal, I'm not sure whether this is a PHP problem or an oversight in Darwin.

From what I can see, the code indicates that the SSL handling code either hasn't run the handshake properly (and needs to complete it), or the socket has been exhausted and needs to be reinitialised via another handshake.

My theory of why this is happening: if both peer and host verification are off, the socket doesn't get initialised correctly. Either of these being on would initiate the handshake (and both would do it once), but both being off causes the handshake to never take place. My bet is on this being a bug in cURL, but it's possible this is a problem in SecureTransport/OSX (as evidenced by RC4 affecting it).

(In either case, I'll report this upstream to cURL.)

The good news, however, is that restoring local verification will solve the problem. For those users like me who only shut it off in the first place because WordPress 4.3 introduced problems for Server 4 when it incorrectly set CAINFO, we can switch it back on when running Server 5, and bunnies are happy everywhere.

Given this, should we fix it here, or simply recommend verification is left on (in your opinion)?

#12 in reply to: ↑ 11 @rmccue
8 years ago

Replying to rmccue:

(In either case, I'll report this upstream to cURL.)

Filed upstream.

#13 in reply to: ↑ 11 @FriendlyGreg
8 years ago

Replying to rmccue:

As far as I can see, there's nothing special about the loopback connection, apart from being able to control the server? The client should still be connecting via HTTP + SSL over port 443 for remote and loopback connections; I don't think the system does any optimisation there.

Well, I can't suss why, but there does seem to be something different about the loopback connection; the connection only ever fails when it's back to the same machine.

My theory of why this is happening: if both peer and host verification are off, the socket doesn't get initialised correctly.

We can leave peer verification switched on, though -- it's only when switching off host verification that the problem crops up. (I realise that makes no sense when the host and the peer are the same...)

(In either case, I'll report this upstream to cURL.)

Cool -- thank you very much for that. Fingers crossed that something worthwhile may yet come from my initial hair-pulling with this; I don't have a lot to spare.

The good news, however, is that restoring local verification will solve the problem. For those users like me who only shut it off in the first place because WordPress 4.3 introduced problems for Server 4 when it incorrectly set CAINFO, we can switch it back on when running Server 5, and bunnies are happy everywhere.

Given this, should we fix it here, or simply recommend verification is left on (in your opinion)?

(I just need to correct what I said above about having switched off local verification due to the CAINFO problem: it was necessary to shut it off with earlier WP versions when using Server 4 on Yosemite, and it was the CAINFO problem in 4.3 which actually broke that as a fix, requiring backporting of the 4.4 code I mentioned at the start of this ticket in order to fix the fix.)

People hosting WordPress on OS X have been in for a rough ride the during the last two versions, as various bits and pieces changing in the HTTP API class, combined with the ongoing expunging of OpenSSL from OS X, have created one problem after another. But in this case, I think it would be best just to recommend that folks keep local verification on, even at the cost of leaving a (hopefully narrow) set of users out in the cold.

#14 @dd32
8 years ago

  • Milestone Future Release deleted
  • Resolution set to invalid
  • Status changed from reviewing to closed

Due to the switch to Requests, I suspect this is no longer an issue as it only sets CURLOPT_SSL_VERIFYHOST to 0 specifically when disabling verification.

Any bugs deeper than that are unfortunately out of our control. The upstream cURL issue was closed as palming it off to Apple, but AFAIK no-one has reported it there yet. I have no way of knowing if it's fixed in recent OSX's either.

Either way, I'm marking this as invalid, if it's still an issue it needs to be chased up with cURL on the above ticket or reproduced under Requests and a bug filed there.

Note: See TracTickets for help on using tickets.