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

Using the system CA bundle by default when possible #800

Closed
wants to merge 3 commits into from

Conversation

mtdowling
Copy link
Member

This commit updates Guzzle to use the CA bundle packaged on the
system by default when possible. cURL will use the system CA
bundle without issue, however, PHP 5.5 and lower will still
default to the bundled CA cert. You can now pass "bundled" to
use the cacert file bundled with Guzzle.

@klausi / @catch56: I'd love feedback on this if you have time.

I've gotten feedback from some sysadmins at work, and they'd prefer if Guzzle defaulted to the system's CA bundle. That seemed easy enough when Guzzle just used cURL, but now that Guzzle also uses PHP's stream wrapper, it gets a bit complicated. PHP version < 5.6 do not know where the system's default bundles are located, so there's two options: find it on disk or use a Guzzle bundled cert by default.

Edit: I've updated the PR to scan for common ca bundles on disk when one is not present by default for the PHP stream wrapper when using PHP < 5.6. I've removed the bundled CA bundle and am now throwing a very descriptive error if this problem is encountered.

This commit updates Guzzle to use the CA bundle packaged on the
system by default when possible. cURL will use the system CA
bundle without issue, however, PHP 5.5 and lower will still
default to the bundled CA cert. You can now pass "bundled" to
use the cacert file bundled with Guzzle.
@klausi
Copy link

klausi commented Sep 2, 2014

This looks like an improvement, per default there should be always a system CA file in use if possible. See also my reasoning in #623 .

I've now removed the bundled CA bundle and will scan for a
CA bundle when using the PHP stream wrapper on PHP < 5.6.
This change will still work out of the box for most users,
but may require users on Mac or Windows to manually configure
a CA bundle using the `verify` request option or the
openssl.cafile ini setting.
@mtdowling
Copy link
Member Author

I've updated the PR to scan for common ca bundles on disk when one is not present by default for the PHP stream wrapper when using PHP < 5.6. I've removed the bundled CA bundle and am now throwing a very descriptive error if this problem is encountered.

@GrahamCampbell
Copy link
Member

On windows 7 for local dev, my only copy of curl-ca-bundle.crt is located at C:\Program Files (x86)\Git\bin\curl-ca-bundle.crt. Are you willing to add that as a default search location?

@GrahamCampbell
Copy link
Member

32 bit users will have it at C:\Program Files\Git\bin\curl-ca-bundle.crt

@mtdowling
Copy link
Member Author

@GrahamCampbell Is that placed there from installing git?

@GrahamCampbell
Copy link
Member

Yeh. :)

@GrahamCampbell
Copy link
Member

I imagine most people will have git installed because most people install guzzle through composer.

@mtdowling
Copy link
Member Author

I'm not sure about using the bundled cert with Git. Seems like it could get outdated pretty quickly, and it isn't distributed with the intention of being the cert you use on your system. Using the CA bundle provided with wamp, however, would be a great addition. I just don't know where that is. How do you run PHP on windows, @GrahamCampbell?

@GrahamCampbell
Copy link
Member

I use http://www.easyphp.org/easyphp-devserver.php. It doesn't bundle curl-ca-bundle.crt.

@mtdowling
Copy link
Member Author

So as a Windows user, what is your strategy for verifying SSL peer certificates? Did you download a cert, put it on disk, and then configured an ini setting?

@GrahamCampbell
Copy link
Member

Yeh, I'd just download the cert and use the curl.cainfo ini setting. I've never really been too bothered what was going on on my windows laptop though - I use linux everywhere else.

@mtdowling
Copy link
Member Author

This change is going to be rolled into #810

@mtdowling mtdowling closed this Sep 8, 2014
@mtdowling mtdowling deleted the cabundle branch September 14, 2014 01:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants