Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug #65538 #592

Merged
merged 2 commits into from Feb 19, 2014
Merged

Bug #65538 #592

merged 2 commits into from Feb 19, 2014

Conversation

@rdlowrey
Copy link
Contributor

rdlowrey commented Feb 16, 2014

Bug #65538

This patch provides support for stream wrapper usage in the "cafile" SSL context option. Previously this value was passed directly to the underlying OpenSSL libs and necessarily required an absolute filesystem path. The following behavior now functions as expected:

<?php
$clientCtx = stream_context_create(['ssl' => [
    'cafile' => 'phar://myphar/ca.pem',
]]);
$html = file_get_contents('https://github.com', false, $clientCtx);

Security Note

Note that all stream wrappers ARE NOT supported. This is intentional. Consider the security implications of allowing the following code (which users would almost certainly attempt if allowed to do so):

<?php
$clientCtx = stream_context_create(['ssl' => [
    'cafile' => 'http://curl.haxx.se/ca/cacert.pem',
]]);
$html = file_get_contents('https://github.com', false, $clientCtx);

The use of any stream wrapper that would be disallowed by the allow_url_fopen=0 php.ini directive will fail with an E_WARNING of the form shown below.

Warning: remote cafile streams are disabled for security purposes in %s on line %d

This prohibition IS NOT dependent upon the allow_url_fopen directive. Stream wrappers identifying themselves with the internal is_url flag are disallowed across the board even if allow_url_fopen=1.

Version Note

I intend to merge this for 5.6 and master as it's more a "feature request" than a bug IMO. If anyone really wants this backported for 5.4/5.5 I might be able to find the time to do so eventually.

@php-pulls php-pulls merged commit 2a83295 into php:PHP-5.6 Feb 19, 2014
1 check failed
1 check failed
default The Travis CI build failed
Details
@rdlowrey rdlowrey deleted the rdlowrey:bug-65538 branch Feb 19, 2014
--SKIPIF--
<?php
if (!extension_loaded('openssl')) die('skip, openssl required');
if (!extension_loaded('pcntl')) die('skip, pcntl required');

This comment has been minimized.

Copy link
@KalleZ

KalleZ Aug 30, 2014

Member

Hmm, does this test really need pcntl to work? I can't spot any obvious reference to it

This comment has been minimized.

Copy link
@rdlowrey

rdlowrey Sep 9, 2014

Author Contributor

It does not. All the old openssl tests depended on pcntl to fork but that dependency has since been removed. This is just a copy/paste artifact -- I'll remove it when I have a chance. Thanks for pointing this out 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
3 participants
You can’t perform that action at this time.