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.

Sign up for GitHub

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

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

Merged
merged 10 commits into from
Dec 2, 2023
Merged

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

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
7e7821b
add: skip hostname checks if CURLRequest options 'verify' is set to f…
NicolaeIotu Nov 26, 2023
3685232
add: SSL verify options when config.verify is string and boolean
NicolaeIotu Nov 27, 2023
e159bfd
fix: CURLOPT_SSL_VERIFYPEER values and tests related with CURLOPT_SSL…
NicolaeIotu Nov 28, 2023
e601954
recreate fix: CURLOPT_SSL_VERIFYPEER values and tests related with CU…
NicolaeIotu Nov 30, 2023
f8dd0b7
Updated Bugs and Breaking sections
NicolaeIotu Nov 30, 2023
25b6d40
Fix sphinx-action warning 'Title underline too short'
NicolaeIotu Nov 30, 2023
ab25092
Update user_guide_src/source/changelogs/v4.4.4.rst
NicolaeIotu Dec 1, 2023
6df07b0
Added the summary of the bug
NicolaeIotu Dec 1, 2023
bfb9ab4
Revert manual changes to CHANGELOG.md
NicolaeIotu Dec 1, 2023
5b4482c
Added upgrading instructions
NicolaeIotu Dec 1, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
# Changelog

## [v4.4.4](https://github.com/codeigniter4/CodeIgniter4/tree/v4.4.4) (Unreleased)
[Full Changelog](https://github.com/codeigniter4/CodeIgniter4/compare/v4.4.3...v4.4.4)

### Fixed Bugs

* fix: [CURLRequest] skip hostname checks if options 'verify' false by @NicolaeIotu in https://github.com/codeigniter4/CodeIgniter4/pull/8258

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that this file is handled automatically when we prepare the release. @kenjis can confirm or deny.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it is generated by GitHub.
Please add in changelog in user guide.
https://github.com/codeigniter4/CodeIgniter4/blob/develop/user_guide_src/source/changelogs/v4.4.4.rst#breaking

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

File user_guide_src/source/changelogs/v4.4.4.rst was updated.
Now I'll revert the changes to CHANGELOG.md.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

## [v4.4.3](https://github.com/codeigniter4/CodeIgniter4/tree/v4.4.3) (2023-10-26)
[Full Changelog](https://github.com/codeigniter4/CodeIgniter4/compare/v4.4.2...v4.4.3)

Expand Down
8 changes: 5 additions & 3 deletions system/HTTP/CURLRequest.php
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
6 changes: 6 additions & 0 deletions user_guide_src/source/changelogs/v4.4.4.rst
View file
Open in desktop
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.

CURLRequest option 'ssl_key' eliminated
=======================================

CURLRequest option 'ssl_key' was eliminated.
Please use CURLRequest option 'verify' to indicate the path to a CA bundle.

NicolaeIotu marked this conversation as resolved.
Show resolved Hide resolved
***************
Message Changes
***************
Expand Down
Loading