Skip to content

fix: [CURLRequest] skip hostname checks if options 'verify' false #8258

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

Merged
8 changes: 5 additions & 3 deletions system/HTTP/CURLRequest.php
Original file line number Diff line number Diff line change
Expand Up @@ -549,16 +549,18 @@ protected function setCURLOptions(array $curlOptions = [], array $config = [])
// SSL Verification
if (isset($config['verify'])) {
if (is_string($config['verify'])) {
$file = realpath($config['ssl_key']) ?: $config['ssl_key'];
$file = realpath($config['verify']) ?: $config['verify'];

if (! is_file($file)) {
throw HTTPException::forInvalidSSLKey($config['ssl_key']);
throw HTTPException::forInvalidSSLKey($config['verify']);
}

$curlOptions[CURLOPT_CAINFO] = $file;
$curlOptions[CURLOPT_SSL_VERIFYPEER] = 1;
$curlOptions[CURLOPT_SSL_VERIFYPEER] = true;
$curlOptions[CURLOPT_SSL_VERIFYHOST] = 2;
} elseif (is_bool($config['verify'])) {
$curlOptions[CURLOPT_SSL_VERIFYPEER] = $config['verify'];
$curlOptions[CURLOPT_SSL_VERIFYHOST] = $config['verify'] ? 2 : 0;
}
}

Expand Down
11 changes: 6 additions & 5 deletions tests/system/HTTP/CURLRequestDoNotShareOptionsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -535,8 +535,7 @@ public function testSSLVerification(): void
$file = __FILE__;

$this->request->request('get', 'http://example.com', [
'verify' => 'yes',
'ssl_key' => $file,
'verify' => $file,
]);

$options = $this->request->curl_options;
Expand All @@ -545,7 +544,10 @@ public function testSSLVerification(): void
$this->assertSame($file, $options[CURLOPT_CAINFO]);

$this->assertArrayHasKey(CURLOPT_SSL_VERIFYPEER, $options);
$this->assertSame(1, $options[CURLOPT_SSL_VERIFYPEER]);
$this->assertTrue($options[CURLOPT_SSL_VERIFYPEER]);

$this->assertArrayHasKey(CURLOPT_SSL_VERIFYHOST, $options);
$this->assertSame(2, $options[CURLOPT_SSL_VERIFYHOST]);
}

public function testSSLWithBadKey(): void
Expand All @@ -554,8 +556,7 @@ public function testSSLWithBadKey(): void
$this->expectException(HTTPException::class);

$this->request->request('get', 'http://example.com', [
'verify' => 'yes',
'ssl_key' => $file,
'verify' => $file,
]);
}

Expand Down
26 changes: 21 additions & 5 deletions tests/system/HTTP/CURLRequestTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -518,8 +518,7 @@ public function testSSLVerification(): void
$file = __FILE__;

$this->request->request('get', 'http://example.com', [
'verify' => 'yes',
'ssl_key' => $file,
'verify' => $file,
]);

$options = $this->request->curl_options;
Expand All @@ -528,7 +527,25 @@ public function testSSLVerification(): void
$this->assertSame($file, $options[CURLOPT_CAINFO]);

$this->assertArrayHasKey(CURLOPT_SSL_VERIFYPEER, $options);
$this->assertSame(1, $options[CURLOPT_SSL_VERIFYPEER]);
$this->assertTrue($options[CURLOPT_SSL_VERIFYPEER]);

$this->assertArrayHasKey(CURLOPT_SSL_VERIFYHOST, $options);
$this->assertSame(2, $options[CURLOPT_SSL_VERIFYHOST]);
}

public function testNoSSL(): void
{
$this->request->request('get', 'http://example.com', [
'verify' => false,
]);

$options = $this->request->curl_options;

$this->assertArrayHasKey(CURLOPT_SSL_VERIFYPEER, $options);
$this->assertFalse($options[CURLOPT_SSL_VERIFYPEER]);

$this->assertArrayHasKey(CURLOPT_SSL_VERIFYHOST, $options);
$this->assertSame(0, $options[CURLOPT_SSL_VERIFYHOST]);
}

public function testSSLWithBadKey(): void
Expand All @@ -537,8 +554,7 @@ public function testSSLWithBadKey(): void
$this->expectException(HTTPException::class);

$this->request->request('get', 'http://example.com', [
'verify' => 'yes',
'ssl_key' => $file,
'verify' => $file,
]);
}

Expand Down
8 changes: 8 additions & 0 deletions user_guide_src/source/changelogs/v4.4.4.rst
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,12 @@ Validation rules matches and differs
Bugs have been fixed in the case where ``matches`` and ``differs`` in the Strict
and Traditional rules validate data of non-string types.

The use of the `ssl_key` option in CURLRequest was removed
==========================================================

Due to a bug, we were using the undocumented `ssl_key` config option to define the CA bundle in CURLRequest.
This was fixed and is now working according to documentation. You can define your CA bundle via the `verify` option.

***************
Message Changes
***************
Expand All @@ -49,6 +55,8 @@ Deprecations
Bugs Fixed
**********

- **CURLRequest:** Fixed a bug where the hostname was checked even if options 'verify' was set to *false*.

See the repo's
`CHANGELOG.md <https://github.com/codeigniter4/CodeIgniter4/blob/develop/CHANGELOG.md>`_
for a complete list of bugs fixed.
9 changes: 9 additions & 0 deletions user_guide_src/source/installation/upgrade_444.rst
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,15 @@ changed (fixed).
Note that Traditional Rules should not be used to validate data that is not a
string.

The use of the `ssl_key` option in CURLRequest was removed
==========================================================

CURLRequest option `ssl_key` it's not recognized anymore.
If in use, option `ssl_key` must be replaced with option `verify` in order to define the path
to a CA bundle for CURLRequest.

CURLRequest option `verify` can also take *boolean* values as usual.

*********************
Breaking Enhancements
*********************
Expand Down