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

Enable CURLOPT_SSL_VERIFYPEER option #18

Open
mjaschen opened this issue Jul 12, 2016 · 5 comments
Open

Enable CURLOPT_SSL_VERIFYPEER option #18

mjaschen opened this issue Jul 12, 2016 · 5 comments
Assignees

Comments

@mjaschen
Copy link
Collaborator

CURLOPT_SSL_VERIFYPEER shouldn't be set to false as this possibly weakens security.

@Niellles
Copy link
Contributor

Niellles commented Jul 2, 2018

Any reason for this not to be implemented yet?

@mjaschen
Copy link
Collaborator Author

mjaschen commented Jul 2, 2018

It's not backward compatible because it possibly breaks existing installations. Therefore it's a feature for the next major release (2.0).

@Niellles
Copy link
Contributor

Niellles commented Jul 2, 2018

I don't see how this will break existing installations, except for those that don't have a correct/complete/up-to-date certificate bundle. That's not so much breaking the installation of this repo as it is a poorly configured php/curl installation. The effect will be the same though and this is probably a good reason to wait for a major release.

Or am I missing something here?

@mjaschen
Copy link
Collaborator Author

mjaschen commented Jul 2, 2018

You're right, but we cannot assume that the CA bundle is up to date everywhere.

Enforcing certificate validation without shipping a current CA bundle will possibly break installations.

@Niellles
Copy link
Contributor

Niellles commented Jul 2, 2018

Fair enough.

How about a temporary fix, that won't break anything in existing installations, that will attempt to verify the certificate and will do the request without verification if it fails. Also it can be easily removed in a future release without breaking backwards compatibility.

For testing purposes I've edited my php.ini to the following -- and it indeed throws an Exception now. Since I am running this on a Windows machine it's probably the worst case scenario, although I am not that familiar with certificates and CURL:

[openssl]
; The location of a Certificate Authority (CA) file on the local filesystem
; to use when verifying the identity of SSL/TLS peers. Most users should
; not specify a value for this directive as PHP will attempt to use the
; OS-managed cert stores in its absence. If specified, this value may still
; be overridden on a per-stream basis via the "cafile" SSL stream context
; option.
;openssl.cafile=
; If openssl.cafile is not specified or if the CA file is not found, the
; directory pointed to by openssl.capath is searched for a suitable
; certificate. This value must be a correctly hashed certificate directory.
; Most users should not specify a value for this directive as PHP will
; attempt to use the OS-managed cert stores in its absence. If specified,
; this value may still be overridden on a per-stream basis via the "capath"
; SSL stream context option.
;openssl.capath=

Suggested fixes:

  1. verifypeer as argument for method.
  2. verifypeer as class property (preferred fix.)

CURL doesn't throw an Exception upon error so I compared the error message as a string, not great practice, but should work if this is indeed the error (and the only error) that will be caused by a missing CA bundle. Added commit to tackle this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants