From 7e7821bdf550c42bbb2be0926f32d7f05bc2f7b8 Mon Sep 17 00:00:00 2001 From: Nicolae Iotu Date: Sun, 26 Nov 2023 12:53:38 +0000 Subject: [PATCH 01/10] add: skip hostname checks if CURLRequest options 'verify' is set to false. When CURLRequest options 'verify' is set to false, some CURLOPT_SSL_... options should be disabled in such a way as to allow requests to pass through in case the destination is for example on private networks. Avoids SSL errors: SSL: certificate subject name 'CA' does not match target host name 'localhost' --- system/HTTP/CURLRequest.php | 6 ++++++ tests/system/HTTP/CURLRequestTest.php | 3 +++ 2 files changed, 9 insertions(+) diff --git a/system/HTTP/CURLRequest.php b/system/HTTP/CURLRequest.php index c8b3b9fe9f92..15e8e65d5579 100644 --- a/system/HTTP/CURLRequest.php +++ b/system/HTTP/CURLRequest.php @@ -559,6 +559,12 @@ protected function setCURLOptions(array $curlOptions = [], array $config = []) $curlOptions[CURLOPT_SSL_VERIFYPEER] = 1; } elseif (is_bool($config['verify'])) { $curlOptions[CURLOPT_SSL_VERIFYPEER] = $config['verify']; + + if ($config['verify'] === false) { + $curlOptions[CURLOPT_SSL_VERIFYHOST] = 0; + } else { + $curlOptions[CURLOPT_SSL_VERIFYHOST] = 2; + } } } diff --git a/tests/system/HTTP/CURLRequestTest.php b/tests/system/HTTP/CURLRequestTest.php index 186284da6ba6..25a8c30669d0 100644 --- a/tests/system/HTTP/CURLRequestTest.php +++ b/tests/system/HTTP/CURLRequestTest.php @@ -529,6 +529,9 @@ public function testSSLVerification(): void $this->assertArrayHasKey(CURLOPT_SSL_VERIFYPEER, $options); $this->assertSame(1, $options[CURLOPT_SSL_VERIFYPEER]); + + $this->assertArrayHasKey(CURLOPT_SSL_VERIFYHOST, $options); + $this->assertSame(2, $options[CURLOPT_SSL_VERIFYHOST]); } public function testSSLWithBadKey(): void From 368523263cfa73d80d2114ddbf3122b6c8235fa2 Mon Sep 17 00:00:00 2001 From: Nicolae Iotu Date: Mon, 27 Nov 2023 19:14:21 +0000 Subject: [PATCH 02/10] add: SSL verify options when config.verify is string and boolean --- system/HTTP/CURLRequest.php | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/system/HTTP/CURLRequest.php b/system/HTTP/CURLRequest.php index 15e8e65d5579..2eb4aa388a49 100644 --- a/system/HTTP/CURLRequest.php +++ b/system/HTTP/CURLRequest.php @@ -555,16 +555,17 @@ protected function setCURLOptions(array $curlOptions = [], array $config = []) throw HTTPException::forInvalidSSLKey($config['ssl_key']); } - $curlOptions[CURLOPT_CAINFO] = $file; - $curlOptions[CURLOPT_SSL_VERIFYPEER] = 1; - } elseif (is_bool($config['verify'])) { - $curlOptions[CURLOPT_SSL_VERIFYPEER] = $config['verify']; - - if ($config['verify'] === false) { - $curlOptions[CURLOPT_SSL_VERIFYHOST] = 0; - } else { + $curlOptions[CURLOPT_CAINFO] = $file; + if ($config['verify'] === 'yes') { + $curlOptions[CURLOPT_SSL_VERIFYPEER] = 1; $curlOptions[CURLOPT_SSL_VERIFYHOST] = 2; + } else { + $curlOptions[CURLOPT_SSL_VERIFYPEER] = 0; + $curlOptions[CURLOPT_SSL_VERIFYHOST] = 0; } + } elseif (is_bool($config['verify'])) { + $curlOptions[CURLOPT_SSL_VERIFYPEER] = $config['verify']; + $curlOptions[CURLOPT_SSL_VERIFYHOST] = $config['verify'] ? 2 : 0; } } From e159bfd8a059c19dc55f58a8ea491e111bcb9df6 Mon Sep 17 00:00:00 2001 From: Nicolae Iotu Date: Tue, 28 Nov 2023 07:48:10 +0000 Subject: [PATCH 03/10] fix: CURLOPT_SSL_VERIFYPEER values and tests related with CURLOPT_SSL_VERIFYPEER and CURLOPT_SSL_VERIFYHOST --- system/HTTP/CURLRequest.php | 4 ++-- .../HTTP/CURLRequestDoNotShareOptionsTest.php | 2 +- tests/system/HTTP/CURLRequestTest.php | 17 ++++++++++++++++- 3 files changed, 19 insertions(+), 4 deletions(-) diff --git a/system/HTTP/CURLRequest.php b/system/HTTP/CURLRequest.php index 2eb4aa388a49..5edd9af6859e 100644 --- a/system/HTTP/CURLRequest.php +++ b/system/HTTP/CURLRequest.php @@ -557,10 +557,10 @@ protected function setCURLOptions(array $curlOptions = [], array $config = []) $curlOptions[CURLOPT_CAINFO] = $file; if ($config['verify'] === 'yes') { - $curlOptions[CURLOPT_SSL_VERIFYPEER] = 1; + $curlOptions[CURLOPT_SSL_VERIFYPEER] = true; $curlOptions[CURLOPT_SSL_VERIFYHOST] = 2; } else { - $curlOptions[CURLOPT_SSL_VERIFYPEER] = 0; + $curlOptions[CURLOPT_SSL_VERIFYPEER] = false; $curlOptions[CURLOPT_SSL_VERIFYHOST] = 0; } } elseif (is_bool($config['verify'])) { diff --git a/tests/system/HTTP/CURLRequestDoNotShareOptionsTest.php b/tests/system/HTTP/CURLRequestDoNotShareOptionsTest.php index d8e96424dae5..39dc0409afb9 100644 --- a/tests/system/HTTP/CURLRequestDoNotShareOptionsTest.php +++ b/tests/system/HTTP/CURLRequestDoNotShareOptionsTest.php @@ -545,7 +545,7 @@ 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]); } public function testSSLWithBadKey(): void diff --git a/tests/system/HTTP/CURLRequestTest.php b/tests/system/HTTP/CURLRequestTest.php index 25a8c30669d0..a947acee7163 100644 --- a/tests/system/HTTP/CURLRequestTest.php +++ b/tests/system/HTTP/CURLRequestTest.php @@ -528,12 +528,27 @@ 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 { $file = 'something_obviously_bogus'; From e601954b35db48ebdd8042d24d1a2e6c6fd5a960 Mon Sep 17 00:00:00 2001 From: Nicolae Iotu Date: Thu, 30 Nov 2023 11:18:56 +0000 Subject: [PATCH 04/10] recreate fix: CURLOPT_SSL_VERIFYPEER values and tests related with CURLOPT_SSL_VERIFYPEER and CURLOPT_SSL_VERIFYHOST --- system/HTTP/CURLRequest.php | 25 ++++++++----------- .../HTTP/CURLRequestDoNotShareOptionsTest.php | 9 ++++--- tests/system/HTTP/CURLRequestTest.php | 6 ++--- 3 files changed, 18 insertions(+), 22 deletions(-) diff --git a/system/HTTP/CURLRequest.php b/system/HTTP/CURLRequest.php index 5edd9af6859e..9392e2446b8e 100644 --- a/system/HTTP/CURLRequest.php +++ b/system/HTTP/CURLRequest.php @@ -548,24 +548,21 @@ 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']; + $configVerify = $config['verify']; + + if (is_string($configVerify)) { + $file = realpath($configVerify) ?: $configVerify; if (! is_file($file)) { - throw HTTPException::forInvalidSSLKey($config['ssl_key']); + throw HTTPException::forInvalidSSLKey($configVerify); } - $curlOptions[CURLOPT_CAINFO] = $file; - if ($config['verify'] === 'yes') { - $curlOptions[CURLOPT_SSL_VERIFYPEER] = true; - $curlOptions[CURLOPT_SSL_VERIFYHOST] = 2; - } else { - $curlOptions[CURLOPT_SSL_VERIFYPEER] = false; - $curlOptions[CURLOPT_SSL_VERIFYHOST] = 0; - } - } elseif (is_bool($config['verify'])) { - $curlOptions[CURLOPT_SSL_VERIFYPEER] = $config['verify']; - $curlOptions[CURLOPT_SSL_VERIFYHOST] = $config['verify'] ? 2 : 0; + $curlOptions[CURLOPT_CAINFO] = $file; + $curlOptions[CURLOPT_SSL_VERIFYPEER] = true; + $curlOptions[CURLOPT_SSL_VERIFYHOST] = 2; + } elseif (is_bool($configVerify)) { + $curlOptions[CURLOPT_SSL_VERIFYPEER] = $configVerify; + $curlOptions[CURLOPT_SSL_VERIFYHOST] = $configVerify ? 2 : 0; } } diff --git a/tests/system/HTTP/CURLRequestDoNotShareOptionsTest.php b/tests/system/HTTP/CURLRequestDoNotShareOptionsTest.php index 39dc0409afb9..32e60b1418b3 100644 --- a/tests/system/HTTP/CURLRequestDoNotShareOptionsTest.php +++ b/tests/system/HTTP/CURLRequestDoNotShareOptionsTest.php @@ -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; @@ -546,6 +545,9 @@ public function testSSLVerification(): void $this->assertArrayHasKey(CURLOPT_SSL_VERIFYPEER, $options); $this->assertTrue($options[CURLOPT_SSL_VERIFYPEER]); + + $this->assertArrayHasKey(CURLOPT_SSL_VERIFYHOST, $options); + $this->assertSame(2, $options[CURLOPT_SSL_VERIFYHOST]); } public function testSSLWithBadKey(): void @@ -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, ]); } diff --git a/tests/system/HTTP/CURLRequestTest.php b/tests/system/HTTP/CURLRequestTest.php index a947acee7163..4893266e5009 100644 --- a/tests/system/HTTP/CURLRequestTest.php +++ b/tests/system/HTTP/CURLRequestTest.php @@ -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; @@ -555,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, ]); } From f8dd0b70b23edd315bfd2bc8640073899cb0bf42 Mon Sep 17 00:00:00 2001 From: Nicolae Iotu Date: Thu, 30 Nov 2023 17:49:04 +0000 Subject: [PATCH 05/10] Updated Bugs and Breaking sections Revert to ['verify'] instead of --- CHANGELOG.md | 7 +++++++ system/HTTP/CURLRequest.php | 14 ++++++-------- user_guide_src/source/changelogs/v4.4.4.rst | 6 ++++++ 3 files changed, 19 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9cf10b0f8372..9dbee6dba64c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 + ## [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) diff --git a/system/HTTP/CURLRequest.php b/system/HTTP/CURLRequest.php index 9392e2446b8e..500b425d2dc5 100644 --- a/system/HTTP/CURLRequest.php +++ b/system/HTTP/CURLRequest.php @@ -548,21 +548,19 @@ protected function setCURLOptions(array $curlOptions = [], array $config = []) // SSL Verification if (isset($config['verify'])) { - $configVerify = $config['verify']; - - if (is_string($configVerify)) { - $file = realpath($configVerify) ?: $configVerify; + if (is_string($config['verify'])) { + $file = realpath($config['verify']) ?: $config['verify']; if (! is_file($file)) { - throw HTTPException::forInvalidSSLKey($configVerify); + throw HTTPException::forInvalidSSLKey($config['verify']); } $curlOptions[CURLOPT_CAINFO] = $file; $curlOptions[CURLOPT_SSL_VERIFYPEER] = true; $curlOptions[CURLOPT_SSL_VERIFYHOST] = 2; - } elseif (is_bool($configVerify)) { - $curlOptions[CURLOPT_SSL_VERIFYPEER] = $configVerify; - $curlOptions[CURLOPT_SSL_VERIFYHOST] = $configVerify ? 2 : 0; + } elseif (is_bool($config['verify'])) { + $curlOptions[CURLOPT_SSL_VERIFYPEER] = $config['verify']; + $curlOptions[CURLOPT_SSL_VERIFYHOST] = $config['verify'] ? 2 : 0; } } diff --git a/user_guide_src/source/changelogs/v4.4.4.rst b/user_guide_src/source/changelogs/v4.4.4.rst index aca70ba7874e..f1123a099308 100644 --- a/user_guide_src/source/changelogs/v4.4.4.rst +++ b/user_guide_src/source/changelogs/v4.4.4.rst @@ -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. + *************** Message Changes *************** From 25b6d4022758a1b02be28ce4e7693202e3faaced Mon Sep 17 00:00:00 2001 From: Nicolae Iotu Date: Thu, 30 Nov 2023 17:57:03 +0000 Subject: [PATCH 06/10] Fix sphinx-action warning 'Title underline too short' --- user_guide_src/source/changelogs/v4.4.4.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/user_guide_src/source/changelogs/v4.4.4.rst b/user_guide_src/source/changelogs/v4.4.4.rst index f1123a099308..39b74598c92f 100644 --- a/user_guide_src/source/changelogs/v4.4.4.rst +++ b/user_guide_src/source/changelogs/v4.4.4.rst @@ -28,7 +28,7 @@ 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. From ab25092599263f316fe7b185bf08053e55b2ea83 Mon Sep 17 00:00:00 2001 From: IOTU Nicolae Date: Fri, 1 Dec 2023 07:17:56 +0000 Subject: [PATCH 07/10] Update user_guide_src/source/changelogs/v4.4.4.rst Co-authored-by: Michal Sniatala --- user_guide_src/source/changelogs/v4.4.4.rst | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/user_guide_src/source/changelogs/v4.4.4.rst b/user_guide_src/source/changelogs/v4.4.4.rst index 39b74598c92f..98415f303990 100644 --- a/user_guide_src/source/changelogs/v4.4.4.rst +++ b/user_guide_src/source/changelogs/v4.4.4.rst @@ -27,11 +27,11 @@ 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 -======================================= +The use of the `ssl_key` option in CURLRequest was removed +========================================================== -CURLRequest option 'ssl_key' was eliminated. -Please use CURLRequest option 'verify' to indicate the path to a CA bundle. +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 From 6df07b00acba0379440270feffb9c14c4fd66623 Mon Sep 17 00:00:00 2001 From: Nicolae Iotu Date: Fri, 1 Dec 2023 07:39:46 +0000 Subject: [PATCH 08/10] Added the summary of the bug --- user_guide_src/source/changelogs/v4.4.4.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/user_guide_src/source/changelogs/v4.4.4.rst b/user_guide_src/source/changelogs/v4.4.4.rst index 98415f303990..7ee96a53556f 100644 --- a/user_guide_src/source/changelogs/v4.4.4.rst +++ b/user_guide_src/source/changelogs/v4.4.4.rst @@ -55,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 `_ for a complete list of bugs fixed. From bfb9ab4ef7dcaf43bd9ce1ca0f091e2935a39b33 Mon Sep 17 00:00:00 2001 From: Nicolae Iotu Date: Fri, 1 Dec 2023 07:51:44 +0000 Subject: [PATCH 09/10] Revert manual changes to CHANGELOG.md --- CHANGELOG.md | 7 ------- 1 file changed, 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9dbee6dba64c..9cf10b0f8372 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,12 +1,5 @@ # 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 - ## [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) From 5b4482c86dd7b50af8c67901095bda7f9fc20979 Mon Sep 17 00:00:00 2001 From: Nicolae Iotu Date: Fri, 1 Dec 2023 08:58:51 +0000 Subject: [PATCH 10/10] Added upgrading instructions --- user_guide_src/source/installation/upgrade_444.rst | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/user_guide_src/source/installation/upgrade_444.rst b/user_guide_src/source/installation/upgrade_444.rst index b3773947efd9..77026be6673e 100644 --- a/user_guide_src/source/installation/upgrade_444.rst +++ b/user_guide_src/source/installation/upgrade_444.rst @@ -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 *********************