MDL-64075 antivirus_clamav: refactor scan_data for tcpsocket

- Fix whitespace accross scanner_test for codechecker
- Deprecate \antivirus_clamav\scanner::scan_data_execute_unixsocket()

lib/antivirus/clamav/classes/scanner.php

@@ -34,6 +34,7 @@ define('ANTIVIRUS_CLAMAV_SOCKET_CHUNKSIZE', 1024);
 /**
  * Class implementing ClamAV antivirus.
  * @copyright  2015 Ruslan Kabalin, Lancaster University.
+ * @copyright  2019 Didier Raboud, Liip AG.
  * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
 class scanner extends \core\antivirus\scanner {
@@ -47,6 +48,8 @@ class scanner extends \core\antivirus\scanner {
             return (bool)$this->get_config('pathtoclam');
         } else if ($this->get_config('runningmethod') === 'unixsocket') {
             return (bool)$this->get_config('pathtounixsocket');
+        } else if ($this->get_config('runningmethod') === 'tcpsocket') {
+            return (bool)$this->get_config('tcpsockethost') && (bool)$this->get_config('tcpsocketport');
         }
         return false;
     }
@@ -92,10 +95,11 @@ class scanner extends \core\antivirus\scanner {
      * @return int Scanning result constant.
      */
     public function scan_data($data) {
-        // We can do direct stream scanning if unixsocket running method is in use,
+        // We can do direct stream scanning if unixsocket or tcpsocket running methods are in use,
         // if not, use default process.
-        if ($this->get_config('runningmethod') === 'unixsocket') {
+        $runningmethod = $this->get_config('runningmethod');
-            $return = $this->scan_data_execute_unixsocket($data);
+        if (in_array($runningmethod, array('unixsocket', 'tcpsocket'))) {
+            $return = $this->scan_data_execute_socket($data, $runningmethod);
 
             if ($return === self::SCAN_RESULT_ERROR) {
                 $this->message_admins($this->get_scanning_notice());
@@ -120,6 +124,15 @@ class scanner extends \core\antivirus\scanner {
         return 'unix://' . $this->get_config('pathtounixsocket');
     }
 
+    /**
+     * Returns a Internet domain socket destination url
+     *
+     * @return string The socket url, fit for stream_socket_client()
+     */
+    private function get_tcpsocket_destination() {
+        return 'tcp://' . $this->get_config('tcpsockethost') . ':' . $this->get_config('tcpsocketport');
+    }
+
     /**
      * Returns the string equivalent of a numeric clam error code
      *
@@ -226,12 +239,12 @@ class scanner extends \core\antivirus\scanner {
             // After scanning we revert permissions to initial ones.
             chmod($file, $perms);
             // Parse the output.
-            return $this->parse_unixsocket_response($output);
+            return $this->parse_socket_response($output);
         }
     }
 
     /**
-     * Scan data using unix socket.
+     * Scan data socket.
      *
      * We are running INSTREAM command and passing data stream in chunks.
      * The format of the chunk is: <length><data> where <length> is the size of the following
@@ -240,11 +253,25 @@ class scanner extends \core\antivirus\scanner {
      * Do not exceed StreamMaxLength as defined in clamd.conf, otherwise clamd will
      * reply with INSTREAM size limit exceeded and close the connection.
      *
-     * @param string $data The varaible containing the data to scan.
+     * @param string $data The variable containing the data to scan.
+     * @param string $type Either 'tcpsocket' or 'unixsocket'
      * @return int Scanning result constant.
      */
-    public function scan_data_execute_unixsocket($data) {
+    public function scan_data_execute_socket($data, $type) {
-        $socket = stream_socket_client($this->get_unixsocket_destination(), $errno, $errstr, ANTIVIRUS_CLAMAV_SOCKET_TIMEOUT);
+        switch ($type) {
+            case "tcpsocket":
+                $socketurl = $this->get_tcpsocket_destination();
+                break;
+            case "unixsocket":
+                $socketurl = $this->get_unixsocket_destination();
+                break;
+            default;
+                // This should not happen.
+                debugging('Unknown socket type!');
+                return self::SCAN_RESULT_ERROR;
+        }
+
+        $socket = stream_socket_client($socketurl, $errno, $errstr, ANTIVIRUS_CLAMAV_SOCKET_TIMEOUT);
         if (!$socket) {
             // Can't open socket for some reason, notify admins.
             $notice = get_string('errorcantopensocket', 'antivirus_clamav', "$errstr ($errno)");
@@ -270,17 +297,17 @@ class scanner extends \core\antivirus\scanner {
             fclose($socket);
 
             // Parse the output.
-            return $this->parse_unixsocket_response($output);
+            return $this->parse_socket_response($output);
         }
     }
 
     /**
-     * Parse unix socket command response.
+     * Parse socket command response.
      *
-     * @param string $output The unix socket command response.
+     * @param string $output The socket response.
      * @return int Scanning result constant.
      */
-    private function parse_unixsocket_response($output) {
+    private function parse_socket_response($output) {
         $splitoutput = explode(': ', $output);
         $message = trim($splitoutput[1]);
         if ($message === 'OK') {
@@ -298,4 +325,19 @@ class scanner extends \core\antivirus\scanner {
             }
         }
     }
+
+    /**
+     * Scan data using Unix domain socket.
+     *
+     * @deprecated since Moodle 3.9 MDL-64075 - please do not use this function any more.
+     * @see antivirus_clamav\scanner::scan_data_execute_socket()
+     *
+     * @param string $data The variable containing the data to scan.
+     * @return int Scanning result constant.
+     */
+    public function scan_data_execute_unixsocket($data) {
+        debugging('antivirus_clamav\scanner::scan_data_execute_unixsocket() is deprecated. ' .
+                  'Use antivirus_clamav\scanner::scan_data_execute_socket() instead.', DEBUG_DEVELOPER);
+        return $this->scan_data_execute_socket($data, "unixsocket");
+    }
 }

lib/antivirus/clamav/tests/scanner_test.php

@@ -44,8 +44,8 @@ class antivirus_clamav_scanner_testcase extends advanced_testcase {
 
     public function test_scan_file_not_exists() {
         $antivirus = $this->getMockBuilder('\antivirus_clamav\scanner')
-                ->setMethods(array('scan_file_execute_commandline', 'message_admins'))
+            ->setMethods(array('scan_file_execute_commandline', 'message_admins'))
-                ->getMock();
+            ->getMock();
 
         // Test specifying file that does not exist.
         $nonexistingfile = $this->tempfile . '_';
@@ -63,8 +63,8 @@ class antivirus_clamav_scanner_testcase extends advanced_testcase {
             'get_config',
         );
         $antivirus = $this->getMockBuilder('\antivirus_clamav\scanner')
-                ->setMethods($methods)
+            ->setMethods($methods)
-                ->getMock();
+            ->getMock();
         // Initiate mock scanning with configuration setting to use commandline.
         $configmap = array(array('runningmethod', 'commandline'));
         $antivirus->method('get_config')->will($this->returnValueMap($configmap));
@@ -97,8 +97,8 @@ class antivirus_clamav_scanner_testcase extends advanced_testcase {
             'get_config',
         );
         $antivirus = $this->getMockBuilder('\antivirus_clamav\scanner')
-                ->setMethods($methods)
+            ->setMethods($methods)
-                ->getMock();
+            ->getMock();
         // Initiate mock scanning with configuration setting to use commandline.
         $configmap = array(array('runningmethod', 'commandline'));
         $antivirus->method('get_config')->will($this->returnValueMap($configmap));
@@ -132,8 +132,8 @@ class antivirus_clamav_scanner_testcase extends advanced_testcase {
             'get_scanning_notice',
         );
         $antivirus = $this->getMockBuilder('\antivirus_clamav\scanner')
-                ->setMethods($methods)
+            ->setMethods($methods)
-                ->getMock();
+            ->getMock();
 
         // Configure scan_file_execute_commandline and scan_file_execute_unixsocket
         // method stubs to behave as if there is a scanning error (SCAN_RESULT_ERROR).
@@ -171,8 +171,8 @@ class antivirus_clamav_scanner_testcase extends advanced_testcase {
             'get_scanning_notice',
         );
         $antivirus = $this->getMockBuilder('\antivirus_clamav\scanner')
-                ->setMethods($methods)
+            ->setMethods($methods)
-                ->getMock();
+            ->getMock();
 
         // Configure scan_file_execute_commandline and scan_file_execute_unixsocket
         // method stubs to behave as if there is a scanning error (SCAN_RESULT_ERROR).
@@ -205,20 +205,30 @@ class antivirus_clamav_scanner_testcase extends advanced_testcase {
 
     public function test_scan_data_no_virus() {
         $methods = array(
-            'scan_data_execute_unixsocket',
+            'scan_data_execute_socket',
             'message_admins',
             'get_config',
         );
         $antivirus = $this->getMockBuilder('\antivirus_clamav\scanner')
-                ->setMethods($methods)
+            ->setMethods($methods)
-                ->getMock();
+            ->getMock();
         // Initiate mock scanning with configuration setting to use unixsocket.
         $configmap = array(array('runningmethod', 'unixsocket'));
         $antivirus->method('get_config')->will($this->returnValueMap($configmap));
 
-        // Configure scan_data_execute_unixsocket method stubs to behave as if
+        // Configure scan_data_execute_socket method stubs to behave as if
         // no virus has been found (SCAN_RESULT_OK).
-        $antivirus->method('scan_data_execute_unixsocket')->willReturn(0);
+        $antivirus->method('scan_data_execute_socket')->willReturn(0);
+
+        // Set expectation that message_admins is NOT called.
+        $antivirus->expects($this->never())->method('message_admins');
+
+        // Run mock scanning.
+        $this->assertEquals(0, $antivirus->scan_data(''));
+
+        // Re-initiate mock scanning with configuration setting to use tcpsocket.
+        $configmap = array(array('runningmethod', 'tcpsocket'));
+        $antivirus->method('get_config')->will($this->returnValueMap($configmap));
 
         // Set expectation that message_admins is NOT called.
         $antivirus->expects($this->never())->method('message_admins');
@@ -229,20 +239,30 @@ class antivirus_clamav_scanner_testcase extends advanced_testcase {
 
     public function test_scan_data_virus() {
         $methods = array(
-            'scan_data_execute_unixsocket',
+            'scan_data_execute_socket',
             'message_admins',
             'get_config',
         );
         $antivirus = $this->getMockBuilder('\antivirus_clamav\scanner')
-                ->setMethods($methods)
+            ->setMethods($methods)
-                ->getMock();
+            ->getMock();
         // Initiate mock scanning with configuration setting to use unixsocket.
         $configmap = array(array('runningmethod', 'unixsocket'));
         $antivirus->method('get_config')->will($this->returnValueMap($configmap));
 
-        // Configure scan_data_execute_unixsocket method stubs to behave as if
+        // Configure scan_data_execute_socket method stubs to behave as if
         // no virus has been found (SCAN_RESULT_FOUND).
-        $antivirus->method('scan_data_execute_unixsocket')->willReturn(1);
+        $antivirus->method('scan_data_execute_socket')->willReturn(1);
+
+        // Set expectation that message_admins is NOT called.
+        $antivirus->expects($this->never())->method('message_admins');
+
+        // Run mock scanning.
+        $this->assertEquals(1, $antivirus->scan_data(''));
+
+        // Re-initiate mock scanning with configuration setting to use tcpsocket.
+        $configmap = array(array('runningmethod', 'tcpsocket'));
+        $antivirus->method('get_config')->will($this->returnValueMap($configmap));
 
         // Set expectation that message_admins is NOT called.
         $antivirus->expects($this->never())->method('message_admins');
@@ -253,22 +273,22 @@ class antivirus_clamav_scanner_testcase extends advanced_testcase {
 
     public function test_scan_data_error_donothing() {
         $methods = array(
-            'scan_data_execute_unixsocket',
+            'scan_data_execute_socket',
             'message_admins',
             'get_config',
             'get_scanning_notice',
         );
         $antivirus = $this->getMockBuilder('\antivirus_clamav\scanner')
-                ->setMethods($methods)
+            ->setMethods($methods)
-                ->getMock();
+            ->getMock();
         // Initiate mock scanning with configuration setting to do nothing on
         // scanning error and using unixsocket.
         $configmap = array(array('clamfailureonupload', 'donothing'), array('runningmethod', 'unixsocket'));
         $antivirus->method('get_config')->will($this->returnValueMap($configmap));
 
-        // Configure scan_data_execute_unixsocket method stubs to behave as if
+        // Configure scan_data_execute_socket method stubs to behave as if
         // there is a scanning error (SCAN_RESULT_ERROR).
-        $antivirus->method('scan_data_execute_unixsocket')->willReturn(2);
+        $antivirus->method('scan_data_execute_socket')->willReturn(2);
         $antivirus->method('get_scanning_notice')->willReturn('someerror');
 
         // Set expectation that message_admins is called.
@@ -276,27 +296,38 @@ class antivirus_clamav_scanner_testcase extends advanced_testcase {
 
         // Run mock scanning.
         $this->assertEquals(2, $antivirus->scan_data(''));
+
+        // Re-initiate mock scanning with configuration setting to do nothing on
+        // scanning error and using tcsocket.
+        $configmap = array(array('clamfailureonupload', 'donothing'), array('runningmethod', 'tcpsocket'));
+        $antivirus->method('get_config')->will($this->returnValueMap($configmap));
+
+        // Set expectation that message_admins is called.
+        $antivirus->expects($this->atLeastOnce())->method('message_admins')->with($this->equalTo('someerror'));
+
+        // Run mock scanning.
+        $this->assertEquals(2, $antivirus->scan_data(''));
     }
 
     public function test_scan_data_error_actlikevirus() {
         $methods = array(
-            'scan_data_execute_unixsocket',
+            'scan_data_execute_socket',
             'message_admins',
             'get_config',
             'get_scanning_notice',
         );
         $antivirus = $this->getMockBuilder('\antivirus_clamav\scanner')
-                ->setMethods($methods)
+            ->setMethods($methods)
-                ->getMock();
+            ->getMock();
 
         // Initiate mock scanning with configuration setting to act like virus on
         // scanning error and using unixsocket.
         $configmap = array(array('clamfailureonupload', 'actlikevirus'), array('runningmethod', 'unixsocket'));
         $antivirus->method('get_config')->will($this->returnValueMap($configmap));
 
-        // Configure scan_data_execute_unixsocket method stubs to behave as if
+        // Configure scan_data_execute_socket method stubs to behave as if
         // there is a scanning error (SCAN_RESULT_ERROR).
-        $antivirus->method('scan_data_execute_unixsocket')->willReturn(2);
+        $antivirus->method('scan_data_execute_socket')->willReturn(2);
         $antivirus->method('get_scanning_notice')->willReturn('someerror');
 
         // Set expectation that message_admins is called.
@@ -305,5 +336,17 @@ class antivirus_clamav_scanner_testcase extends advanced_testcase {
         // Run mock scanning, we expect SCAN_RESULT_FOUND since configuration
         // require us to act like virus.
         $this->assertEquals(1, $antivirus->scan_data(''));
+
+        // Re-initiate mock scanning with configuration setting to act like virus on
+        // scanning error and using tcpsocket.
+        $configmap = array(array('clamfailureonupload', 'actlikevirus'), array('runningmethod', 'tcpsocket'));
+        $antivirus->method('get_config')->will($this->returnValueMap($configmap));
+
+        // Set expectation that message_admins is called.
+        $antivirus->expects($this->atLeastOnce())->method('message_admins')->with($this->equalTo('someerror'));
+
+        // Run mock scanning, we expect SCAN_RESULT_FOUND since configuration
+        // require us to act like virus.
+        $this->assertEquals(1, $antivirus->scan_data(''));
     }
 }