Page MenuHomePhabricator

PhpHttpRequest should not check host against CN x509 attribute
Closed, ResolvedPublic

Description

In the class PhpHttpRequest (file includes/HttpFunctions.php, used when CURL is not installed), the option 'sslVerifyHost' is translated by checking the 'CN' x509 attribute against the host, which is now deprecated with x509 certificate v3 with subjectAltName and this avoid the operation although it was correct.

In particular, this can be observed with `$wgInstantCommons = true' on an HTTPS wiki without php-curl installed, because the commons.wikimedia.org certificate has a CN attribute *.wikipedia.org and commons.wikimedia.org is only in the subjectAltName attribute.


Version: 1.25-git
Severity: normal
See Also:
https://bugzilla.wikimedia.org/show_bug.cgi?id=68581

Event Timeline

bzimport raised the priority of this task from to Needs Triage.Nov 22 2014, 4:01 AM
bzimport set Reference to bz73199.
bzimport added a subscriber: Unknown Object (MLST).

Adding this as a (soft) blocker of bug 53131 since it will lead to problems for people without CURL.

Aklapper triaged this task as Lowest priority.Mar 21 2015, 10:09 PM
Aklapper subscribed.

This can be solved by changing the options given to stream_context_create in PhpHttpRequest::execute from CN_match to peer_name (see https://secure.php.net/manual/en/context.ssl.php). But I still didn’t find the PHP versions where one or the other can be used; from PHP 5.6 CN_match becomes deprecated -- minimum PHP version for MediaWiki is currently 5.3.3.

I tested peer_name with PHP 5.5.6 and it works correctly (assuming T75203 is solved <=> a capath is set).

Seb35 raised the priority of this task from Lowest to Medium.Jul 1 2015, 12:03 AM
Seb35 set Security to None.

According to https://secure.php.net/manual/en/context.ssl.php SNI is supported since PHP 5.3.2, so there shouldn’t be any problem from this side. Here the issue is more about the fields CN vs. subjectAltName of X.509 certificates.

According to https://secure.php.net/manual/en/context.ssl.php SNI is supported since PHP 5.3.2, so there shouldn’t be any problem from this side. Here the issue is more about the fields CN vs. subjectAltName of X.509 certificates.

We used to send per-domain certs with SNI, so commons would get one that had a CN of *.wikimedia.org, but looks like we don't anymore due to c02fab71422a490dbdcf (ping @BBlack)

I gave the source for php 5.3.4 a very quick look. Its possible I missed something, but appears peer_name option is not there :(

I suspect that 5.6 is the first version it works based on other options that are listed as being added at that time

php sucks :'(

Change 222086 had a related patch set uploaded (by Brian Wolff):
Workaround fopen lack of SubjectAltName support for instantCommons

https://gerrit.wikimedia.org/r/222086

Forgive my ignorance of the nuances of the PHP ecosystem, but if php-curl is what is required for PHP (a language that's all about HTTP-based things) to have a sane HTTP[S] implementation available, why is it that we can't absolutely require the installation of php-curl as a pre-requisite? And why on earth do we try to work around this by shipping a forked pure-PHP implementation of our own in the first place? :/

Also, note that we're nowhere near the bottom of the pile of problems these things are going to cause us eventually with completing all the corner-cases of the transition ... https://gerrit.wikimedia.org/r/#/c/221974/

Well we don't require either http(s) fopen or php-curl support to install, as making http requests is sufficiently a side feature that its ok if it doesn't work.

Personally I'd like instantcommons to work with as many configurations as possible, as its a feature often used by people who are relatively inexperianced (I think, I have no hard data on that). I have no information on what the relative install rate of php5-curl is on various popular web hosts, so I have no idea how worth it it is.

I'm definitely supportive of having the installer recommend to people to use php5-curl

Also, note that we're nowhere near the bottom of the pile of problems these things are going to cause us eventually with completing all the corner-cases of the transition ... https://gerrit.wikimedia.org/r/#/c/221974/

Perhaps, but from the perspective of just enough features to make it work for the common case, this is probably fine.

I tested peer_name with PHP 5.5.6 and it works correctly

Did you check that it correctly validates certificates, or does it just let anything through (Prior to the introduction of "peer_name", php just silently ignores that setting, and if CN_match is not set, will consider all certs valid)

When I do git grep --text peer_name php-5.5.7 on the php source (There doesn't seem to be a tag for 5.5.6), I don't get any results. I only start getting results in php-5.6.0, which makes me think the feature is not available until then

I aggree php-curl is really better than PHP on this case, but, on Debian at least, it is not installed by default so these two patches for PhpHttpRequest can save some users from wondering why InstantCommons doesn’t work, and possibly they are mandatory for some shared hosting (I have no hard data on installation rate).

I also dived into PHP source and also didn’t find the option peer_name before 5.6. I studied the part in 5.6 and I comment this on the gerrit page.

For completeness here (I spoke about it in the gerrit patch), I was wrong about the resolution for PHP 5.5.6: peer_name is not recognized and the domain name check is silently ignored (so not a secure connection since there is no complete cert checking).

Is this ready to merge? into upcoming point releases? we're basically way overdue for giving up on clients that can't do basic HTTPS...

I have no information on what the relative install rate of php5-curl is on various popular web hosts, so I have no idea how worth it it is.

Which is kinda sad; we should have that kind of information. Maybe T56425 could be used to collect status of optional dependencies.

Change 222086 merged by jenkins-bot:
Workaround fopen lack of SubjectAltName support for instantCommons

https://gerrit.wikimedia.org/r/222086

Change 234926 had a related patch set uploaded (by Gergő Tisza):
Workaround fopen lack of SubjectAltName support for instantCommons

https://gerrit.wikimedia.org/r/234926

Change 234927 had a related patch set uploaded (by Gergő Tisza):
Workaround fopen lack of SubjectAltName support for instantCommons

https://gerrit.wikimedia.org/r/234927

Change 234928 had a related patch set uploaded (by Gergő Tisza):
Workaround fopen lack of SubjectAltName support for instantCommons

https://gerrit.wikimedia.org/r/234928

Tgr claimed this task.

Change 234928 merged by jenkins-bot:
Workaround fopen lack of SubjectAltName support for instantCommons

https://gerrit.wikimedia.org/r/234928

Change 234926 merged by jenkins-bot:
Workaround fopen lack of SubjectAltName support for instantCommons

https://gerrit.wikimedia.org/r/234926

Change 234927 merged by jenkins-bot:
Workaround fopen lack of SubjectAltName support for instantCommons

https://gerrit.wikimedia.org/r/234927