Make WordPress Core

Opened 5 years ago

Closed 5 years ago

#16904 closed defect (bug) (fixed)

CURLOPT_SSL_VERIFYHOST Should use 2 instead of true

Reported by: sivel Owned by:
Milestone: 3.2 Priority: normal
Severity: normal Version: 3.1
Component: HTTP API Keywords: has-patch 3.2-early
Focuses: Cc:


CURLOPT_SSL_VERIFYHOST Should use an integer of 2 instead of true to enforce proper SSL verification. Setting to true acts like 1, which only checks for the existence of a common name, and not that the common name matches the requested hostname.

This was reported in another ticket #16855

Attachments (3)

16904.diff (668 bytes) - added by sivel 5 years ago.
16904.patch (713 bytes) - added by hakre 5 years ago.
Found by cogmios
16904.2.patch (709 bytes) - added by hakre 5 years ago.
Make setting more visible.

Download all attachments as: .zip

Change History (9)

5 years ago

5 years ago

Found by cogmios

#1 follow-up: @hakre
5 years ago

Related Comment

No need to be that strict on the evaluation of $ssl_verify btw.

5 years ago

Make setting more visible.

#2 @hakre
5 years ago

Hmm, I just ran over the fact that any (bool) is infact an (int), if it's TRUE it is (int) 1, if it is false, it is (int) 0.

So in case we don't change the current implementation, users of the cUrl transport are able to additionally specify 2 here, which is also (bool) TRUE for the second setting. Same for 1, it's (bool) TRUE for the second setting as well.

The unpatched code:

   curl_setopt( $handle, CURLOPT_SSL_VERIFYHOST, $ssl_verify ); 
   curl_setopt( $handle, CURLOPT_SSL_VERIFYPEER, $ssl_verify );

Forcing $ssl_verify to result in either 2 or 0 for CURLOPT_SSL_VERIFYHOST will remove the possibility to set the value to 1.

#3 @hakre
5 years ago

Hmm, and then there is the Streams context:

'ssl' => array(
		'verify_peer' => $ssl_verify,
		'verify_host' => $ssl_verify

I can not find the verify_host documented.

It's probably worth to list parameters and their current meaning.

With such a list we could more easily say which feature should be supported on the abstracted transports interface.

#4 in reply to: ↑ 1 @sivel
5 years ago

Replying to hakre:

Related Comment

No need to be that strict on the evaluation of $ssl_verify btw.

In general we do try to be that strict with our checks in the HTTP API. We want to expect very specific values.

#5 @sivel
5 years ago

  • Milestone changed from Future Release to 3.2

#6 @dd32
5 years ago

  • Resolution set to fixed
  • Status changed from new to closed

(In [17616]) Verify certificate matches both the common name and the provided hostname. WP_HTTP_Curl requires CURLOPT_SSL_VERIFYHOST be either 2 or false. Props sivel. Fixes #16904

Note: See TracTickets for help on using tickets.