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

verify certificates against the SSL backend's default 'capath' #31

Closed
rcanavan opened this issue Mar 21, 2016 · 24 comments
Closed

verify certificates against the SSL backend's default 'capath' #31

rcanavan opened this issue Mar 21, 2016 · 24 comments

Comments

@rcanavan
Copy link

By default, ext-http currently does not verify certificates against the 'capath' that is built into the SSL backend used by curl. Therefore, a hashed directory of certificates isn't checked unless the location is explicitly set.

Since the correct value for CURLOPT_CAPATH depends on the type of SSL backend used and its compile-time and run-time configuration, it's difficult to even make a good educated guess which value to set in ['ssl']['capath'].

With the "plain" curl PHP extension, the default CAPATH of the SSL backend is used to verify a peer's SSL certificate, even if CURLOP_CAINFO is set to a non-default file.

@m6w6
Copy link
Owner

m6w6 commented Mar 22, 2016

Hi, thanks for the report.

I'm not sure I completely understand the issue.
We already do this with CAINFO. https://github.com/m6w6/ext-http/search?utf8=%E2%9C%93&q=PHP_HTTP_CURL_CAINFO

Or... do you want to use CAPATH and CAINFO at the sane time?

@m6w6
Copy link
Owner

m6w6 commented Mar 22, 2016

@rcanavan
Copy link
Author

What I actually want is to disable CAINFO and only check the default CAPATH. With PHP's own curl extension, parsing the ~300k certificate store for every request has a significant performance impact, and we usually set CAINFO to a dummy (e.g. expired) CA certificate.

@m6w6
Copy link
Owner

m6w6 commented Mar 22, 2016

Hm, okay. As far as I understand the issues presented in above article, it seems that the, let's call it, CAPATH is ultimately an OS thing. So even if we only look at openssl, the "CAPATH" might be different depending on the OS?

Do you have any suggestions on how to go forward about this?

@rcanavan
Copy link
Author

The idea is - and casual testing appears to show that the PHP curl extension works like this - that the compiled in defaults of the SSL backend "just work", i.e. neither the user nor the ext-http should be required to guess which CAPATH might be appropriate.

Right now, using the openssl backend, I haven't managed to convince ext-http to use the hashed directories without explicitly setting CAPATH correctly. With PHP curl, I can just set CAINFO to a dummy CA to achieve this.

Using the NSS backend on CentOS or RHEL, ext-http already behaves like this, i.e. running a http\Client\Request with

$request->setOptions(array('ssl' => array('cainfo' => '..../ssl/dummyCA.pem')));

works (assuming you have populated the sqlite PKI db). With an OpenSSL backend, it throws a "Peer certificate cannot be authenticated" exception instead.

@m6w6
Copy link
Owner

m6w6 commented Mar 23, 2016

Okay.

Using the NSS backend on CentOS or RHEL, ext-http already behaves like this, i.e. running a http\Client\Request with

$request->setOptions(array('ssl' => array('cainfo' => '..../ssl/dummyCA.pem')));

Do you mean that it does not work without setting a dummy cainfo?

Right now, using the openssl backend, I haven't managed to convince ext-http to use the hashed directories without explicitly setting CAPATH correctly. With PHP curl, I can just set CAINFO to a dummy CA to achieve this.

ATM pecl_http's configure checks curl-config --ca, /etc/ssl/certs/ca-certificates.crt and /etc/ssl/certs/ca-bundle.crt to use as default for cainfo. Do you say, that this does not work on your system (with libcurl/openssl)?

If so, what does grep CA config.log in pecl_http's build dir give you?

Currently, we reset every used curl option before we use a curl handle, because they may be re-used, so "just letting curl alone with its default CAPATH" would not work out that easily... 😞

Thanks!

@rcanavan
Copy link
Author

$request->setOptions(array('ssl' => array('cainfo' => '..../ssl/dummyCA.pem')));
Do you mean that it does not work without setting a dummy cainfo?

Curl + NSS works both if I set a dummy CA, if I set 'cainfo' to a proper CA bundle and if I leave the setting untouched.

ATM pecl_http's configure checks curl-config --ca, /etc/ssl/certs/ca-certificates.crt and /etc/ssl/certs/ca-bundle.crt to use as default for cainfo. Do you say, that this does not work on your system (with libcurl/openssl)?

It does work, but I don't want to use the CA bundle, I want to use the hashed directory the default CAPATH points to (probably: the default built into openssl, which is used if CAPATH isn't set). This is for performance reasons, as stated in the curl manpage: "Using --capath can allow OpenSSL-powered curl to make SSL-connections much more efficiently).

If so, what does grep CA config.log in pecl_http's build dir give you?

It correctly identifies the location of the bundle:

configure:6600: checking for bundled SSL CA info
#define PHP_HTTP_CURL_CAINFO "/etc/ssl/certs/ca-certificates.crt"

Currently, we reset every used curl option before we use a curl handle, because they may be re-used, so "just letting curl alone with its default CAPATH" would not work out that easily... 

The man page for SSL_CTX_load_verify_locations (https://www.openssl.org/docs/manmaster/ssl/SSL_CTX_load_verify_locations.html), which is used internally by curl if CAPATH !=NULL || CAINFO != NULL seems to indicat that just setting the capath to NULL should result in the desired behavior, however, the following doesn't work for me:

$request->setSslOptions(array("cainfo" => '.../dummyCA.pem', "capath" => NULL));

@m6w6
Copy link
Owner

m6w6 commented Mar 24, 2016

The man page for SSL_CTX_load_verify_locations

Doesn't seem so... it says that SSL_CTX_set_default_verify_paths et al. (note the default) do so, but not SSL_CTX_set_verify_locations and I can't find a reference to the former function in curl's sources.

@rcanavan
Copy link
Author

Looks like you're correct. SSL_CTX_load_verify_locations() doesn't make any efforts to set the defaults if any of the parameters is NULL, but it also doesn't destroy the default values, if they were previously set, which appears to be the case initially. As a result, that doesn't help at all if you want to reset the curl handle to a known state and reuse the defaults.

@m6w6
Copy link
Owner

m6w6 commented Mar 24, 2016

if they were previously set, which appears to be the case initially

Yes, if curl was built with either --with-ca-bundle or --with-ca-path, e.g. on Arch it's built --with-ca-bundle=/etc/ssl/certs/ca-certificates.crt while on Debian it seems to be --with-ca-path=/etc/ssl/certs. Gotta have to do some testing on Debian, then.

As a result, that doesn't help at all if you want to reset the curl handle to a known state and reuse the defaults.

Exactly :(

@m6w6
Copy link
Owner

m6w6 commented Mar 24, 2016

Oookay, actually, I guess I see a possible solution. We should actually only use curl-config --ca's output for CAINFO if it's not a directory. If it is a directory it should be used as default for CAPATH.

Looks like a valid bug, after all.

@m6w6
Copy link
Owner

m6w6 commented Mar 24, 2016

Would be awesome, if you checked the default_capath branch to see if that works out for you?

@rcanavan
Copy link
Author

I've applied d8907d9 and a0c554e on top of 3.0.1, and that doesn't seem to help:

$ grep CA config.log
configure:6600: checking for default SSL CA info/path
#define PHP_HTTP_CURL_CAINFO "/etc/ssl/certs/ca-certificates.crt"

The code inside the #ifdef PHP_HTTP_CURL_CAPATH doesn't get compiled.

@m6w6
Copy link
Owner

m6w6 commented Mar 24, 2016

Did you re-phpize and re-configure and make sure that php_http_client_curl.c got re-compiled (make clean, or something)?

I get this with a fresh build (2.5.x series, though):

# php -r 'var_dump((new http\Client)->getAvailableOptions()["ssl"]);'
array(22) {
  ["cert"]=>
  NULL
  ["certtype"]=>
  string(3) "PEM"
  ["key"]=>
  NULL
  ["keytype"]=>
  string(3) "PEM"
  ["keypasswd"]=>
  NULL
  ["engine"]=>
  NULL
  ["version"]=>
  int(0)
  ["verifypeer"]=>
  bool(true)
  ["verifyhost"]=>
  bool(true)
  ["cipher_list"]=>
  NULL
  ["cainfo"]=>
  NULL
  ["capath"]=>
  string(14) "/etc/ssl/certs"
  ["random_file"]=>
  NULL
  ["egdsocket"]=>
  NULL
  ["issuercert"]=>
  NULL
  ["crlfile"]=>
  NULL
  ["certinfo"]=>
  bool(false)
  ["enable_npn"]=>
  bool(true)
  ["enable_alpn"]=>
  bool(true)
  ["tlsauthtype"]=>
  int(0)
  ["tlsauthuser"]=>
  NULL
  ["tlsauthpass"]=>
  NULL
}

@rcanavan
Copy link
Author

I think the checking for default SSL CA info/path Message in config.log is a good indicator that configure was re-built and re-run. Just to make sure, I've deleted everything and started from scratch.

["cainfo"]=> string(34) "/etc/ssl/certs/ca-certificates.crt"
["capath"]=> NULL

Since curl-config --ca returns /etc/ssl/certs/ca-certificates.crt, the test -e check breaks out of the loop, so the new test returns the same result as the old.

@m6w6
Copy link
Owner

m6w6 commented Mar 24, 2016

facepalm, okay, please give ffee4ca a try!

Thank you.

@rcanavan
Copy link
Author

With ffee4ca (+ adjustment in ZVAL_STRING for PHP 7), it configures and builds:

$ grep CA pecl_http-3.0.1/config*log
configure:6600: checking for default SSL CA info/path
#define PHP_HTTP_CURL_CAPATH "/etc/ssl/certs"
#define PHP_HTTP_CURL_CAINFO "/etc/ssl/certs/ca-certificates.crt"

but a small test program that just makes a new requst to https://google.com/, sets options:

$request->setOptions(array('ssl' => array('cainfo' => '...../dummyCA.pem')));

and prints the result still fails with "Peer certificate cannot be authenticated with given CA certificates; SSL certificate problem: unable to get local issuer certificate".

@m6w6
Copy link
Owner

m6w6 commented Mar 24, 2016

Hmm... what about setting cainfo to NULL instead of a dummy?

@m6w6
Copy link
Owner

m6w6 commented Mar 24, 2016

Ha, guess what's in curl 7.48 :)

@m6w6
Copy link
Owner

m6w6 commented Mar 25, 2016

Hey, I just verified that it's using the hashed capath for me on Debian:

strace -e open -f -- php -r 'var_dump((new http\Client)->enqueue(new http\Client\Request("GET", "https://www.google.de/"))->send()->getResponse()->getTransferInfo());'

...
open("/dev/urandom", O_RDONLY)          = 5
open("/dev/urandom", O_RDONLY|O_NOCTTY|O_NONBLOCK) = 5
open("/etc/ssl/certs/578d5c04.0", O_RDONLY) = 5
...

@rcanavan
Copy link
Author

Hmm... what about setting cainfo to NULL instead of a dummy?

I've tried NULL, false, true, '', and finally '/dev/null'. The latter displays invalid (random) characters in the error message:

PHP Fatal error: Uncaught http\Exception\RuntimeException: http\Client::send(): Problem with the SSL CA cert (path? access rights?); error setting certificate verify locations:
CAfile: /dev/null
CApath: xt`�� (https://google.com/) in /home/canavan/src/pecl_http.php:17

 

Hey, I just verified that it's using the hashed capath for me on Debian:
[...]

On Ubuntu 15.10, the same command line never touches the hashed directory:

open("/dev/urandom", O_RDONLY) = 5
open("/dev/urandom", O_RDONLY|O_NOCTTY|O_NONBLOCK) = 5
open("/etc/ssl/certs/ca-certificates.crt", O_RDONLY) = 5
open("/proc/meminfo", O_RDONLY|O_CLOEXEC) = 5

@m6w6
Copy link
Owner

m6w6 commented Mar 30, 2016

So, did make any progress with this? Did you fetch the latest commits, too?

m6w6 added a commit that referenced this issue Apr 1, 2016
@rcanavan
Copy link
Author

rcanavan commented Apr 1, 2016

I've finally found the time to look closer into this. You patch actually works as well as one could expect.

@m6w6
Copy link
Owner

m6w6 commented Apr 1, 2016

Yay, thanks for verifying!

@m6w6 m6w6 closed this as completed Apr 1, 2016
@m6w6 m6w6 removed the in progress label Jun 21, 2016
m6w6 added a commit that referenced this issue Oct 4, 2016
+ Added http\Client\Curl\User interface for userland event loops
+ Added http\Url::IGNORE_ERRORS, http\Url::SILENT_ERRORS and
http\Url::STDFLAGS
+ Added http\Client::setDebug(callable $debug)
+ Added http\Client\Curl\FEATURES constants and namespace
+ Added http\Client\Curl\VERSIONS constants and namespace
+ Added share_cookies and share_ssl (libcurl >= 7.23.0) options to
http\Client::configure()
+ http\Client uses curl_share handles to properly share cookies and
SSL/TLS sessions between requests
+ Improved configure checks for default CA bundles
+ Improved negotiation precision
* Fixed regression introduced by http\Params::PARSE_RFC5987: negotiation
using the params parser would receive param keys without the trailing
asterisk, stripped by http\Params::PARSE_RFC5987.
* Fix gh-issue #50: http\Client::dequeue() within
http\Client::setDebug() causes segfault (Mike, Maik Wagner)
* Fix gh-issue #47: http\Url: Null pointer deref in sanitize_value()
(Mike, @rc0r)
* Fix gh-issue #45: HTTP/2 response message parsing broken with libcurl
>= 7.49.1 (Mike)
* Fix gh-issue #43: Joining query with empty original variable in query
(Mike, Sander Backus)
* Fix gh-issue #42: fatal error when using punycode in URLs (Mike,
Sebastian Thielen)
* Fix gh-issue #41: Use curl_version_info_data.features when
initializing options (Mike)
* Fix gh-issue #40: determinde the SSL backend used by curl at runtime
(Mike, @rcanavan)
* Fix gh-issue #39: Notice: http\Client::enqueue(): Could not set option
proxy_service_name (Mike, @rcanavan)
* Fix gh-issue #38: Persistent curl handles: error code not properly
reset (Mike, @afflerbach)
* Fix gh-issue #36: Unexpected cookies sent if persistent_handle_id is
used (Mike, @rcanavan, @afflerbach)
* Fix gh-issue #34: allow setting multiple headers with the same name
(Mike, @rcanavan)
* Fix gh-issue #33: allow setting prodyhost request option to NULL
(Mike, @rcanavan)
* Fix gh-issue #31: add/improve configure checks for default CA
bundle/path (Mike, @rcanavan)

Changes from beta1:
* Fixed PHP-5.3 compatibility
* Fixed recursive calls to the event loop dispatcher

Changes from beta2:
* Fix bug #73055: crash in http\QueryString (Mike, @rc0r)
(CVE-2016-7398)
* Fix bug #73185: Buffer overflow in HTTP parse_hostinfo() (Mike, @rc0r)
* Fix HTTP/2 version parser for older libcurl versions (Mike)
m6w6 added a commit that referenced this issue Oct 4, 2016
+ Added http\Client\Curl\User interface for userland event loops
+ Added http\Url::IGNORE_ERRORS, http\Url::SILENT_ERRORS and
http\Url::STDFLAGS
+ Added http\Client::setDebug(callable $debug)
+ Added http\Client\Curl\FEATURES constants and namespace
+ Added http\Client\Curl\VERSIONS constants and namespace
+ Added share_cookies and share_ssl (libcurl >= 7.23.0) options to
http\Client::configure()
+ http\Client uses curl_share handles to properly share cookies and
SSL/TLS sessions between requests
+ Improved configure checks for default CA bundles
+ Improved negotiation precision
* Fixed regression introduced by http\Params::PARSE_RFC5987: negotiation
using the params parser would receive param keys without the trailing
asterisk, stripped by http\Params::PARSE_RFC5987.
* Fix gh-issue #50: http\Client::dequeue() within
http\Client::setDebug() causes segfault (Mike, Maik Wagner)
* Fix gh-issue #47: http\Url: Null pointer deref in sanitize_value()
(Mike, @rc0r)
* Fix gh-issue #45: HTTP/2 response message parsing broken with libcurl
>= 7.49.1 (Mike)
* Fix gh-issue #43: Joining query with empty original variable in query
(Mike, Sander Backus)
* Fix gh-issue #42: fatal error when using punycode in URLs (Mike,
Sebastian Thielen)
* Fix gh-issue #41: Use curl_version_info_data.features when
initializing options (Mike)
* Fix gh-issue #40: determinde the SSL backend used by curl at runtime
(Mike, @rcanavan)
* Fix gh-issue #39: Notice: http\Client::enqueue(): Could not set option
proxy_service_name (Mike, @rcanavan)
* Fix gh-issue #38: Persistent curl handles: error code not properly
reset (Mike, @afflerbach)
* Fix gh-issue #36: Unexpected cookies sent if persistent_handle_id is
used (Mike, @rcanavan, @afflerbach)
* Fix gh-issue #34: allow setting multiple headers with the same name
(Mike, @rcanavan)
* Fix gh-issue #33: allow setting prodyhost request option to NULL
(Mike, @rcanavan)
* Fix gh-issue #31: add/improve configure checks for default CA
bundle/path (Mike, @rcanavan)

Changes from beta1:
* Fixed recursive calls to the event loop dispatcher

Changes from beta2:
+ Improved configure checks for IDNA libraries (added
--with-http-libicu-dir, --with-http-libidnkit{,2}-dir,
--with-http-libidn2-dir)
* Fix bug #73055: crash in http\QueryString (Mike, @rc0r)
(CVE-2016-7398)
* Fix bug #73185: Buffer overflow in HTTP parse_hostinfo() (Mike, @rc0r)
* Fix HTTP/2 version parser for older libcurl versions (Mike)
* Fix gh-issue #52: Underscores in host names: libidn Failed to parse
IDN (Mike, @canavan)
m6w6 added a commit that referenced this issue Dec 12, 2016
+ Added http\Client\Curl\User interface for userland event loops
+ Added http\Url::IGNORE_ERRORS, http\Url::SILENT_ERRORS and
  http\Url::STDFLAGS
+ Added http\Client::setDebug(callable $debug)
+ Added http\Client\Curl\FEATURES constants and namespace
+ Added http\Client\Curl\VERSIONS constants and namespace
+ Added share_cookies and share_ssl (libcurl >= 7.23.0) options to
  http\Client::configure()
+ http\Client uses curl_share handles to properly share cookies and
  SSL/TLS sessions between requests
+ Improved configure checks for default CA bundles
+ Improved negotiation precision
* Fixed regression introduced by http\Params::PARSE_RFC5987: negotiation
  using the params parser would receive param keys without the trailing
  asterisk, stripped by http\Params::PARSE_RFC5987.
* Fix gh-issue #50: http\Client::dequeue() within
  http\Client::setDebug() causes segfault (Mike, Maik Wagner)
* Fix gh-issue #47: http\Url: Null pointer deref in sanitize_value()
  (Mike, @rc0r)
* Fix gh-issue #45: HTTP/2 response message parsing broken with
  libcurl >= 7.49.1 (Mike)
* Fix gh-issue #43: Joining query with empty original variable in query
  (Mike, Sander Backus)
* Fix gh-issue #42: fatal error when using punycode in URLs
  (Mike, Sebastian Thielen)
* Fix gh-issue #41: Use curl_version_info_data.features when
  initializing options (Mike)
* Fix gh-issue #40: determinde the SSL backend used by curl at runtime
  (Mike, @rcanavan)
* Fix gh-issue #39: Notice: http\Client::enqueue(): Could not set option
  proxy_service_name (Mike, @rcanavan)
* Fix gh-issue #38: Persistent curl handles: error code not properly
  reset (Mike, @afflerbach)
* Fix gh-issue #36: Unexpected cookies sent if persistent_handle_id is
  used (Mike, @rcanavan, @afflerbach)
* Fix gh-issue #34: allow setting multiple headers with the same name
  (Mike, @rcanavan)
* Fix gh-issue #33: allow setting prodyhost request option to NULL
  (Mike, @rcanavan)
* Fix gh-issue #31: add/improve configure checks for default
  CA bundle/path (Mike, @rcanavan)

Changes from beta1:
* Fixed PHP-5.3 compatibility
* Fixed recursive calls to the event loop dispatcher

Changes from beta2:
* Fix bug #73055: crash in http\QueryString (Mike, @rc0r)
  (CVE-2016-7398)
* Fix bug #73185: Buffer overflow in HTTP parse_hostinfo() (Mike, @rc0r)
  (CVE-2016-7961)
* Fix HTTP/2 version parser for older libcurl versions (Mike)
m6w6 added a commit that referenced this issue Dec 12, 2016
+ Added http\Client\Curl\User interface for userland event loops
+ Added http\Url::IGNORE_ERRORS, http\Url::SILENT_ERRORS and
  http\Url::STDFLAGS
+ Added http\Client::setDebug(callable $debug)
+ Added http\Client\Curl\FEATURES constants and namespace
+ Added http\Client\Curl\VERSIONS constants and namespace
+ Added share_cookies and share_ssl (libcurl >= 7.23.0) options to
  http\Client::configure()
+ http\Client uses curl_share handles to properly share cookies and
  SSL/TLS sessions between requests
+ Improved configure checks for default CA bundles
+ Improved negotiation precision
* Fixed regression introduced by http\Params::PARSE_RFC5987: negotiation
  using the params parser would receive param keys without the trailing
  asterisk, stripped by http\Params::PARSE_RFC5987.
* Fix gh-issue #50: http\Client::dequeue() within
  http\Client::setDebug() causes segfault (Mike, Maik Wagner)
* Fix gh-issue #47: http\Url: Null pointer deref in sanitize_value()
  (Mike, @rc0r)
* Fix gh-issue #45: HTTP/2 response message parsing broken with
  libcurl >= 7.49.1 (Mike)
* Fix gh-issue #43: Joining query with empty original variable in query
  (Mike, Sander Backus)
* Fix gh-issue #42: fatal error when using punycode in URLs
  (Mike, Sebastian Thielen)
* Fix gh-issue #41: Use curl_version_info_data.features when
  initializing options (Mike)
* Fix gh-issue #40: determinde the SSL backend used by curl at runtime
  (Mike, @rcanavan)
* Fix gh-issue #39: Notice: http\Client::enqueue(): Could not set option
  proxy_service_name (Mike, @rcanavan)
* Fix gh-issue #38: Persistent curl handles: error code not properly
  reset (Mike, @afflerbach)
* Fix gh-issue #36: Unexpected cookies sent if persistent_handle_id is
  used (Mike, @rcanavan, @afflerbach)
* Fix gh-issue #34: allow setting multiple headers with the same name
  (Mike, @rcanavan)
* Fix gh-issue #33: allow setting prodyhost request option to NULL
  (Mike, @rcanavan)
* Fix gh-issue #31: add/improve configure checks for default
  CA bundle/path (Mike, @rcanavan)

Changes from beta1:
* Fixed recursive calls to the event loop dispatcher

Changes from beta2:
+ Improved configure checks for IDNA libraries (added
  --with-http-libicu-dir, --with-http-libidnkit{,2}-dir,
  --with-http-libidn2-dir)
* Fix bug #73055: crash in http\QueryString (Mike, @rc0r)
  (CVE-2016-7398)
* Fix bug #73185: Buffer overflow in HTTP parse_hostinfo() (Mike, @rc0r)
  (CVE-2016-7961)
* Fix HTTP/2 version parser for older libcurl versions (Mike)
* Fix gh-issue #52: Underscores in host names: libidn Failed to parse
  IDN (Mike, @canavan)
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