Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:72757
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.216.44 as permitted sender)
MIME-Version: 1.0
Sender: are.you.winning@gmail.com
In-Reply-To: <CAEZPtU5hn6TFaQdS1eKbgLzBH93e6A6Rvdqhc0Vnj0izxRRD+g@mail.gmail.com>
References: <CAH8MpnnmKt2byQU8DcWetnK6W7GB9HuF3srea-ECwQ4RRe1SWQ@mail.gmail.com>
	<CAGAGxbaZ5aCdSxsQrw05_xyxKEAhQY6uGaoAu06GkudKOtdqoQ@mail.gmail.com>
	<CAEZPtU5hn6TFaQdS1eKbgLzBH93e6A6Rvdqhc0Vnj0izxRRD+g@mail.gmail.com>
Date: Sat, 22 Feb 2014 10:51:14 +0000
Message-ID: <CAGAGxbZJkYRgt0t+5c4_tYY1K17fsK1VZmy4jiH72QgwZwfTDw@mail.gmail.com>
To: Pierre Joye <pierre.php@gmail.com>
Cc: Chris Wright <daverandom@php.net>, Daniel Lowrey <rdlowrey@gmail.com>, 
	Ferenc Kovacs <tyra3l@gmail.com>, =?ISO-8859-1?Q?P=E1draic_Brady?= <padraic.brady@gmail.com>, 
	"internals@lists.php.net" <internals@lists.php.net>
Content-Type: text/plain; charset=ISO-8859-1
Subject: Re: [PHP-DEV] Windows Peer Verification
From: daverandom@php.net (Chris Wright)

Good morning Pierre,

On 22 February 2014 08:30, Pierre Joye <pierre.php@gmail.com> wrote:
> hi Chris!
>
> On Sat, Feb 22, 2014 at 1:31 AM, Chris Wright <daverandom@php.net> wrote:
>> Following on from this thread and Daniel's excellent work on TLS
>> improvements, and liaising heavily with Daniel off-list, I have
>> created a PR [1] of some work I have done to get peer verification
>> working with Windows native certificate store.
>
> Very good job! I like it! Thanks.
>
>> This is by far and away the most preferable option as it gives "out of
>> the box" support for peer verification by default on Windows, and does
>> not require any additional certificate bundles or configuration. It
>> also allows us to take advantage of trust updates rolled out via MS
>> update systems.
>>
>> The implementation is complete in that it supports all existing
>> features, although it needs a little polishing and some edge cases
>> covering before it can be merged. The only definite known issue at the
>> time of writing is that the method for fetching the CN from the
>> certificate incorrectly assumes that the returned data will always be
>> UTF-8 encoded, a solution for this is planned and will be implemented
>> in the next day or two.
>>
>> I am by no means an expert on the subject matter here in any respect,
>> so I encourage ruthless code review.
>>
>> Note that there are no new features here, it is simply looking to fill
>> in the gaps in the recent work by providing consistency on Windows.
>
>
> I have however some doubts about doing that now (not sure if I raised
> these points already).
>
> Using Windows CA store is a good thing, from a theoretical point of
> view. However it brings a couple of issues with it. The 1st one is
> compatibility, it is a different system using different methods to
> store, add or manage certificate. One of the key point we try to keep
> in PHP is almost 100% compatibility between Windows and other
> platforms. That's one of the main reasons why I did not implement SSL
> support using WInSSL APIs back then for 5.3.0.
>
> I am also not sure about the impact this change will have on existing
> code using more advanced features (context, environment customization,
> etc) of openssl, be with stream or with openssl only. It may impact
> performance as well a little bit but I need to test it first to
> compare, did you try to see if there are performance regressions?

I have not done any performance testing, I was hoping Daniel would be
able to do this as he has already done a lot of work in this general
area (high performance network applications written in PHP) and I
think he has a better idea of how to extract some meaningful numbers
than I do, as well as mature PHP applications that will be able to
expose this as the bottleneck, if it is one.

This patch has been specifically designed to affect *only* streams,
and *not* to affect applications that are using custom certificate
validation environments, the code that verifies against the Windows
cert stores is only ever used if a cafile/capath is not specified, if
the paths to a traditional OpenSSL compatible certificate collection
are specified through any method (context option/ini file) then that
is used instead. Any potential usage of the openssl_* functions in
userland is also completely unaffected.

The main motivation behind this work is to avoid a huge influx of
"this code used to work and now I get errors when I run it on
Windows!" questions/complaints when people upgrade to 5.6, which is
what I see looming at the moment. Anyone who has historically been
setting their environment up correctly will be completely unaffected,
they will never even enter this code path.

> At the end, I would prefer to have full support of the Windows Crypto
> APIs and OpenSSL using one single and unified APIs. The new crypto
> APIs could be a good base for that. We can then provide two builds,
> one for openssl and one for the Windows Crypto API support. This is
> what we do in Curl for example.

Just so I'm clear on what's being suggested here, are you talking
about a new "extension" that would provide a unified abstracted API
that could be build against different back-ends? I generally like this
idea but it is a huge task... just coming up with a sane userland API
that can be fully implemented using both back-ends is hard enough.

I've learned from using PDO that this is not necessarily always as
good an idea as it seems on the face of it, because you end up with an
API that accommodates only the LCD features of all back ends, or is
inconsistent across back ends, or both.

Not saying it's a bad idea just wondering how practical it is. Or did
I misunderstand what you're getting at?

> I hope you do not take this reply as a bad reply, I really appreciate

Not at all :-)

> this patch and the effort you put in it. But I have to say that I am
> not in favor of doing that in 5.6.0 and not only for this area in any
> other major version. A more global approach will be more beneficial
> and may bring less confusion to our users. It will also reduce support
> requests from windows users as well, who are following the numerous
> documentation out there about working with openssl on windows, from
> various areas.

I hear what you are saying and I don't disagree. This is only really
intended to prevent the breakages that the recent changes would cause
for Windows users. I can see how it could be considered a band-aid fix
and may be viewed as undesirable in this respect, but I don't view
this fix as preventing your proposed global approach, merely buying
some time to do it properly.

Thanks for your comments, they are most welcome :-)

Chris