Merge pull request #8258 from NicolaeIotu/add-curlrequest-curlopt_ssl…

…_verifyhost fix: [CURLRequest] skip hostname checks if options 'verify' false
codeigniter4 · Dec 2, 2023 · 54cbc32 · 54cbc32
2 parents bcc9b54 + 5b4482c
commit 54cbc32
Show file tree

Hide file tree

Showing 5 changed files with 49 additions and 13 deletions.
diff --git a/system/HTTP/CURLRequest.php b/system/HTTP/CURLRequest.php
@@ -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;
             }
         }
 

diff --git 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;
@@ -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
@@ -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
@@ -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;
@@ -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
@@ -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,
         ]);
     }
 

diff --git 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.
 
+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
 ***************
@@ -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.
diff --git a/user_guide_src/source/installation/upgrade_444.rst b/user_guide_src/source/installation/upgrade_444.rst
@@ -58,6 +58,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
 *********************