[VOTE] TLS Peer Verification

Thanks for the input -- I'm just happy people are interested in the issue!

Let me address a couple of things ...

you get essentially a configuration that can not use https at all

I wouldn't say this is the really the case. Users still have access to the
same https functionality they've always had. The only difference is that
they now must explicitly acknowledge that, "Yes, what I'm doing is
insecure. I'm aware of it and I choose to continue anyway by specifying
this context option."

But it may be against the spirit of the RFC?

:) Yes ... that's kind of what I'm going for. Basically it's my thought
that many (most?) people using things like file_get_contents('https://')
are completely unaware of this issue in the first place. My thinking here
is that instead of not saying anything and just giving these users a false
sense of security we should at least make mention of the problem instead
of
sweeping it under the rug.

people would still ignore it

Almost certainly. In fact, users do this routinely with curl_* because
they
don't know any better.

Finally, I think this problem can largely be alleviated with appropriate
documentation. Should the RFC pass I'll work to make sure that any peer
verification changes are well-documented to (hopefully) stem the
inevitable storm of bug reports.

On Mon, Dec 16, 2013 at 8:42 PM, Stas Malyshev <smalyshev@sugarcrm.com

wrote:

Hi!

Please throw your votes at the TLS Peer Verification proposal:

https://wiki.php.net/rfc/tls-peer-verification

Voting closes Dec. 24 ... Happy Holidays!

I'm not sure what to vote for here, because I like the ideas in the
patch about having a setting for CAfile, which in many distros would by
default enable peer verification and thus make you more secure, but I
don't like the fact that when you compile PHP, you get essentially a
configuration that can not use https at all, since you have no CA file
configured.
I'd like it more if there was an option where if you set cafile or
capath, you get automatic peer verification, but if you don't, you do
not have it. But it may be against the spirit of the RFC?
I know you propose a warning in this case, but judging from the story of
the datetime timezone warning, people would still ignore it. Also
warning is not much help if for some reason you don't know where to get
a cert file. And there's no way to disable peer verification on ini
level.

Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227

Morning Internalz,

    Daniel, you have to assume that nobody will even read the manual;

because they will not. I'm up for making it safer, but not for breaking
anything at all in a minor version, if we do not bundle the CA file it
would appear to break a bunch of requests that previously worked, that
doesn't seem good enough to me.
We can change the behaviour of the engine, but we cannot change
the behaviour of users code; if a requests works now, regardless of it's
security, it must continue to work with the default settings, therefore, I
suggest that you remove the option to change the behaviour of the engine
without maintaining the behaviour of users code, if the aim is to make
these requests more secure by default, then allowing it to fail, by any
means, defeats the object of making any changes at all.

    If I'm wrong, tell me how :)

Cheers
Joe

--

I'm not taking sides, but given that this is a security related change, one
could argue that security fixes should be ok to go in a minor version, even
if they break BC.

--
Ferenc Kovács
@Tyr43l - http://tyrael.hu

On Tue, Dec 17, 2013 at 2:19 PM, Daniel Lowrey rdlowrey@gmail.com
wrote:

On Tue, Dec 17, 2013 at 7:36 AM, Ferenc Kovacs tyra3l@gmail.com
wrote:

given that this is a security related change, one could argue that

security

fixes should be ok to go in a minor version, even if they break BC.

This was my thought process. In my mind the RFC is about improving
security
for users who don't know any better. I'm hoping to avoid the "Are we
allowed to break BC?" discussion.

Okay, but nobody is asking the question "are we allowed to break
compatiblity for no good reason", because it's a silly question.

Adding a CA file to the distribution is exceedingly simple, but this
is
not
a silver bullet. For example, the Mozilla CA file used by cURL is
usually
updated three or four times a year. Even when bundling a CA file it
would
only be a matter of time before a distribution's version was out of
date.
In the end we can only do so much before users must bear the weight
of
maintaining an acceptable level of security themselves.

So then bundle it, doing something is much better than doing nothing,
there are plenty of opportunities to update the cafile with minor
versions,
the package maintainers will likely solve the stale cafile problem
for us
on the major distributions, when they see we are actually doing
something
about it ...

It really does not seem sensible to purposefully break compatibility
when it can be retained easily, the vote is going to be split with no
clear
outcome doing no good for anyone. Reduce the options to two if you
want to
actually move forward.

That's enough from me, gonna go find something to break :)

Don't forget that bundling a CA file also means that take the burden of
keeping it up-to-date to our shoulders.
I'm not saying that we shouldn't do it, but if we do then we have to
make
sure that we understand the implications.
Even if we take the "easy" path, and select an already existing CA
bundle
(eg. Mozilla), we have to make sure to always ship the up-to-date
version
and there could be events, when we would need to create a release only
because some CA incident (like what happened with DigiNotar in 2011
which
forced the everybody shipping CA bundles to update their bundle to
remove
this CA from it's list of trusted CAs).
As I've said, I'm only stating this so everybody can understand the
implications before voting.

--
Ferenc Kovács
@Tyr43l - http://tyrael.hu

Regarding the alteration of voting options Joe requested ...

I do believe approving the patch without bundling a CA file is a valid
option. As Ferenc mentioned there are potential maintenance issues with
including a CA file and I do not feel comfortable forcing the hand of
the
release managers on this point. The vote exists between "yes" and "yes
with
a bundled file" for this reason to tease out what's best for PHP in this
scenario. Please make sure you understand the ramifications of your
choice
before voting.

I do want to preemptively address the idea that not including a CA file
would somehow cause disastrous BC breakage. In my opinion this could
not be
further from the truth. APIs are not changing. The return value is
already
FALSE with a warning generated on a connection or transfer failure. Code
should already have error checking in place for this potential scenario
because socket transfers are never guaranteed (maybe your ethernet
cable is
unplugged).

The only difference this patch makes is to expand the reasons why a
transfer can return FALSE with an E_WARNING to include "you're doing
something dangerous." As far as BC breakage goes, this is as
backward-compatible as a BC break could possibly be.

I wouldn't say it isn't a BC break, as there are a plenty of code out
there
which could/will break after this change.

• If we don't ship/include a CA bundle, every stream connection for
  ssl://, https:// or ftps:// resource will fail before the user
  changes
  his/her php.ini
• even if we do include a CA bundle, there are a bunch of code which
  connects to https:// urls using self-signed certs, and for some of
  those, there is no CA file available, so you won't be able to get
  your code
  functioning again without rewriting your code to explicitly disable
  peer
  verification.

Those are BC breaks, and I'm pretty sure that this would/will affect a
lot
of users.
But I do think, that in this case the BC break could be acceptable, so
I'm
looking forward to the results.

Note that the distro maintainers tend to hate when we bundle stuff like
this. We have this same situation with the Olsen tz. They much prefer
having a single place to update resources that are needed by multiple
applications like the ca cert bundle and the Olsen tz db. So, for most
of our users (assuming most of our users rely on distro-packaged
versions of php) I don't think it actually matters whether we ship the
ca bundle or not. The distros are going override this decision and point
to the distro-level bundle much like many of them have done with the
timezone file.

-Rasmus

True, distros would prefer their own bundle, what I was saying above, that
even though most people would be covered by the distros, there will be
people, who will notice the BC break: either because they roll their own
php or because they use a distribution which doesn't provide a CA by
default, or ultimately because they are sites out there, which uses certs
not signed by CAs included in the major CA bundles.

--
Ferenc Kovács
@Tyr43l - http://tyrael.hu

given that this is a security related change, one could argue that

security

fixes should be ok to go in a minor version, even if they break BC.

This was my thought process. In my mind the RFC is about improving
security
for users who don't know any better. I'm hoping to avoid the "Are we
allowed to break BC?" discussion.

Okay, but nobody is asking the question "are we allowed to break
compatiblity for no good reason", because it's a silly question.

Adding a CA file to the distribution is exceedingly simple, but this is
not
a silver bullet. For example, the Mozilla CA file used by cURL is
usually
updated three or four times a year. Even when bundling a CA file it
would
only be a matter of time before a distribution's version was out of
date.
In the end we can only do so much before users must bear the weight of
maintaining an acceptable level of security themselves.

So then bundle it, doing something is much better than doing nothing,
there are plenty of opportunities to update the cafile with minor versions,
the package maintainers will likely solve the stale cafile problem for us
on the major distributions, when they see we are actually doing something
about it ...

It really does not seem sensible to purposefully break compatibility
when it can be retained easily, the vote is going to be split with no clear
outcome doing no good for anyone. Reduce the options to two if you want to
actually move forward.

That's enough from me, gonna go find something to break :)

Don't forget that bundling a CA file also means that take the burden of
keeping it up-to-date to our shoulders.
I'm not saying that we shouldn't do it, but if we do then we have to make
sure that we understand the implications.
Even if we take the "easy" path, and select an already existing CA bundle
(eg. Mozilla), we have to make sure to always ship the up-to-date version
and there could be events, when we would need to create a release only
because some CA incident (like what happened with DigiNotar in 2011 which
forced the everybody shipping CA bundles to update their bundle to remove
this CA from it's list of trusted CAs).
As I've said, I'm only stating this so everybody can understand the
implications before voting.

--
Ferenc Kovács
@Tyr43l - http://tyrael.hu

Regarding the alteration of voting options Joe requested ...

I do believe approving the patch without bundling a CA file is a valid
option. As Ferenc mentioned there are potential maintenance issues with
including a CA file and I do not feel comfortable forcing the hand of the
release managers on this point. The vote exists between "yes" and "yes with
a bundled file" for this reason to tease out what's best for PHP in this
scenario. Please make sure you understand the ramifications of your choice
before voting.

I do want to preemptively address the idea that not including a CA file
would somehow cause disastrous BC breakage. In my opinion this could not be
further from the truth. APIs are not changing. The return value is already
FALSE with a warning generated on a connection or transfer failure. Code
should already have error checking in place for this potential scenario
because socket transfers are never guaranteed (maybe your ethernet cable is
unplugged).

The only difference this patch makes is to expand the reasons why a
transfer can return FALSE with an E_WARNING to include "you're doing
something dangerous." As far as BC breakage goes, this is as
backward-compatible as a BC break could possibly be.

I wouldn't say it isn't a BC break, as there are a plenty of code out there
which could/will break after this change.

- If we don't ship/include a CA bundle, every stream connection for
ssl://, https:// or ftps:// resource will fail before the user changes
his/her php.ini
- even if we do include a CA bundle, there are a bunch of code which
connects to https:// urls using self-signed certs, and for some of
those, there is no CA file available, so you won't be able to get your code
functioning again without rewriting your code to explicitly disable peer
verification.

Those are BC breaks, and I'm pretty sure that this would/will affect a lot
of users.
But I do think, that in this case the BC break could be acceptable, so I'm
looking forward to the results.

Note that the distro maintainers tend to hate when we bundle stuff like
this. We have this same situation with the Olsen tz. They much prefer
having a single place to update resources that are needed by multiple
applications like the ca cert bundle and the Olsen tz db. So, for most
of our users (assuming most of our users rely on distro-packaged
versions of php) I don't think it actually matters whether we ship the
ca bundle or not. The distros are going override this decision and point
to the distro-level bundle much like many of them have done with the
timezone file.

-Rasmus

[sorry Rasmus, you will get this twice, accident]

I said it wasn't necessarily our problem, proposed some ways we might
aid those whose problem it actually is, an update command, maybe host it.

Excluding Windows, most users will have it solved by their distros
package maintainers, that's pretty clear. We will have to include the
file for Windows, and I see no harm in not doing the same for the source
distribution either, I do see how not including it can be a problem,
don't see how excluding it solves anything. Including it doesn't make it
harder to override, it doesn't change that package maintainers will
likely be responsible, but it does allow more possibilities of
successful execution wherever the source or binary distribution came from.

Cheers
Joe