From 00d3a749d0d94fd2dfc21f55fc6b648f31242e4c Mon Sep 17 00:00:00 2001
From: Ere Maijala <ere.maijala@helsinki.fi>
Date: Mon, 6 Jul 2020 22:01:30 +0300
Subject: [PATCH] Fix MySQLi SSL flags handling to explicitly set
 MYSQLI_CLIENT_SSL if use_ssl is set.

Signed-off-by: Ere Maijala <ere.maijala@helsinki.fi>
---
 src/Adapter/Driver/Mysqli/Connection.php      |  19 ++-
 .../Adapter/Driver/Mysqli/ConnectionTest.php  | 115 ++++++++++++++++++
 2 files changed, 131 insertions(+), 3 deletions(-)

diff --git a/src/Adapter/Driver/Mysqli/Connection.php b/src/Adapter/Driver/Mysqli/Connection.php
index ac278d6e8..abf4d09cc 100644
--- a/src/Adapter/Driver/Mysqli/Connection.php
+++ b/src/Adapter/Driver/Mysqli/Connection.php
@@ -119,7 +119,7 @@ public function connect()
         $caPath = (isset($p['ca_path'])) ? $p['ca_path'] : null;
         $cipher = (isset($p['cipher'])) ? $p['cipher'] : null;
 
-        $this->resource = new \mysqli();
+        $this->resource = $this->createResource();
         $this->resource->init();
 
         if (! empty($p['driver_options'])) {
@@ -138,16 +138,19 @@ public function connect()
         $flags = null;
 
         if ($useSSL && ! $socket) {
+            // Even though mysqli docs are not quite clear on this, MYSQLI_CLIENT_SSL
+            // needs to be set to make sure SSL is used. ssl_set can also cause it to
+            // be implicitly set, but only when any of the parameters is non-empty.
+            $flags = MYSQLI_CLIENT_SSL;
             $this->resource->ssl_set($clientKey, $clientCert, $caCert, $caPath, $cipher);
             //MYSQLI_CLIENT_SSL_DONT_VERIFY_SERVER_CERT is not valid option, needs to be set as flag
             if (isset($p['driver_options'])
                 && isset($p['driver_options'][MYSQLI_CLIENT_SSL_DONT_VERIFY_SERVER_CERT])
             ) {
-                $flags = MYSQLI_CLIENT_SSL_DONT_VERIFY_SERVER_CERT;
+                $flags |= MYSQLI_CLIENT_SSL_DONT_VERIFY_SERVER_CERT;
             }
         }
 
-
         try {
             $this->resource->real_connect($hostname, $username, $password, $database, $port, $socket, $flags);
         } catch (GenericException $e) {
@@ -281,4 +284,14 @@ public function getLastGeneratedValue($name = null)
     {
         return $this->resource->insert_id;
     }
+
+    /**
+     * Create a new mysqli resource
+     *
+     * @return \mysqli
+     */
+    protected function createResource()
+    {
+        return new \mysqli();
+    }
 }
diff --git a/test/unit/Adapter/Driver/Mysqli/ConnectionTest.php b/test/unit/Adapter/Driver/Mysqli/ConnectionTest.php
index 830aa5f7c..ccd13cda5 100644
--- a/test/unit/Adapter/Driver/Mysqli/ConnectionTest.php
+++ b/test/unit/Adapter/Driver/Mysqli/ConnectionTest.php
@@ -64,6 +64,62 @@ public function testGetConnectionParameters()
         self::assertEquals(['foo' => 'bar'], $this->connection->getConnectionParameters());
     }
 
+    public function testNonSecureConnection()
+    {
+        $mysqli = $this->createMockMysqli(0);
+        $connection = $this->createMockConnection(
+            $mysqli,
+            [
+                'hostname' => 'localhost',
+                'username' => 'superuser',
+                'password' => '1234',
+                'database' => 'main',
+                'port' => 123,
+            ]
+        );
+
+        $connection->connect();
+    }
+
+    public function testSslConnection()
+    {
+        $mysqli = $this->createMockMysqli(MYSQLI_CLIENT_SSL);
+        $connection = $this->createMockConnection(
+            $mysqli,
+            [
+                'hostname' => 'localhost',
+                'username' => 'superuser',
+                'password' => '1234',
+                'database' => 'main',
+                'port' => 123,
+                'use_ssl' => true,
+            ]
+        );
+
+        $connection->connect();
+    }
+
+    public function testSslConnectionNoVerify()
+    {
+        $mysqli = $this->createMockMysqli(MYSQLI_CLIENT_SSL | MYSQLI_CLIENT_SSL_DONT_VERIFY_SERVER_CERT);
+        $connection = $this->createMockConnection(
+            $mysqli,
+            [
+                'hostname' => 'localhost',
+                'username' => 'superuser',
+                'password' => '1234',
+                'database' => 'main',
+                'port' => 123,
+                'use_ssl' => true,
+                'driver_options' => [
+                    MYSQLI_CLIENT_SSL_DONT_VERIFY_SERVER_CERT => true
+                ],
+            ]
+        );
+
+        $connection->connect();
+    }
+
     public function testConnectionFails()
     {
         $connection = new Connection([]);
@@ -72,4 +128,63 @@ public function testConnectionFails()
         $this->expectExceptionMessage('Connection error');
         $connection->connect();
     }
+
+    /**
+     * Create a mock mysqli
+     *
+     * @param int $flags Expected flags to real_connect
+     *
+     * @return \PHPUnit\Framework\MockObject\MockObject
+     */
+    protected function createMockMysqli($flags)
+    {
+        $mysqli = $this->getMockBuilder('\mysqli')->getMock();
+        $mysqli->expects($this->once())
+            ->method('init');
+        $mysqli->expects($flags ? $this->once() : $this->never())
+            ->method('ssl_set')
+            ->with(
+                $this->equalTo(null),
+                $this->equalTo(null),
+                $this->equalTo(null),
+                $this->equalTo(null),
+                $this->equalTo(null)
+            );
+
+        $mysqli->expects($this->once())
+            ->method('real_connect')
+            ->with(
+                $this->equalTo('localhost'),
+                $this->equalTo('superuser'),
+                $this->equalTo('1234'),
+                $this->equalTo('main'),
+                $this->equalTo(123),
+                $this->equalTo(null),
+                $this->equalTo($flags)
+            )
+            ->willReturn(null);
+
+        return $mysqli;
+    }
+
+    /**
+     * Create a mock connection
+     *
+     * @param \PHPUnit\Framework\MockObject\MockObject $mysqli Mock mysqli object
+     * @param array                                    $params Connection params
+     *
+     * @return \PHPUnit\Framework\MockObject\MockObject
+     */
+    protected function createMockConnection($mysqli, $params)
+    {
+        $connection = $this->getMockBuilder('\Laminas\Db\Adapter\Driver\Mysqli\Connection')
+            ->setMethods(['createResource'])
+            ->setConstructorArgs([$params])
+            ->getMock();
+        $connection->expects($this->once())
+            ->method('createResource')
+            ->willReturn($mysqli);
+
+        return $connection;
+    }
 }