MDL-64075 antivirus_clamav: refactor scan_file for tcpsocket

- Deprecate \antivirus_clamav\scanner::scan_file_execute_unixsocket()

lib/antivirus/clamav/classes/scanner.php

@@ -70,12 +70,22 @@ class scanner extends \core\antivirus\scanner {
             return self::SCAN_RESULT_ERROR;
         }
 
-        // Execute the scan using preferable method.
+        // We can do direct stream scanning if unixsocket or tcpsocket running methods are in use,
-        $method = 'scan_file_execute_' . $this->get_config('runningmethod');
+        // if not, use default process.
-        if (!method_exists($this, $method)) {
+        $runningmethod = $this->get_config('runningmethod');
-            throw new \coding_exception('Attempting to call non-existing method ' . $method);
+        switch ($runningmethod) {
+            case 'unixsocket':
+            case 'tcpsocket':
+                $return = $this->scan_file_execute_socket($file, $runningmethod);
+                break;
+            case 'commandline':
+                $return = $this->scan_file_execute_commandline($file);
+                break;
+            default:
+                // This should not happen.
+                debugging('Unknown running method.');
+                return self::SCAN_RESULT_ERROR;
         }
-        $return = $this->$method($file);
 
         if ($return === self::SCAN_RESULT_ERROR) {
             $this->message_admins($this->get_scanning_notice());
@@ -211,13 +221,27 @@ class scanner extends \core\antivirus\scanner {
     }
 
     /**
-     * Scan file using Unix domain sockets.
+     * Scan file using sockets.
      *
      * @param string $file Full path to the file.
+     * @param string $type Either 'tcpsocket' or 'unixsocket'
      * @return int Scanning result constant.
      */
-    public function scan_file_execute_unixsocket($file) {
+    public function scan_file_execute_socket($file, $type) {
-        $socket = stream_socket_client($this->get_unixsocket_destination(),
+        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.
@@ -225,6 +249,7 @@ class scanner extends \core\antivirus\scanner {
             $this->set_scanning_notice($notice);
             return self::SCAN_RESULT_ERROR;
         } else {
+            if ($type == "unixsocket") {
                 // Execute scanning. We are running SCAN command and passing file as an argument,
                 // it is the fastest option, but clamav user need to be able to access it, so
                 // we give group read permissions first and assume 'clamav' user is in web server
@@ -233,11 +258,41 @@ class scanner extends \core\antivirus\scanner {
                 // this is to avoid unexpected newline characters on different systems.
                 $perms = fileperms($file);
                 chmod($file, 0640);
+
+                // Actual scan.
                 fwrite($socket, "nSCAN ".$file."\n");
+                // Get ClamAV answer.
                 $output = stream_get_line($socket, 4096);
-            fclose($socket);
+
                 // After scanning we revert permissions to initial ones.
                 chmod($file, $perms);
+            } else if ($type == "tcpsocket") {
+                // Execute scanning by passing the entire file through the TCP socket.
+                // This is not fast, but is the only possibility over a network.
+                // Using 'n' as command prefix is forcing clamav to only treat \n as newline delimeter,
+                // this is to avoid unexpected newline characters on different systems.
+
+                // Actual scan.
+                fwrite($socket, "nINSTREAM\n");
+
+                // Open the file for reading.
+                $fhandle = fopen($file, 'rb');
+                while (!feof($fhandle)) {
+                    // Read it by chunks; write them to the TCP socket.
+                    $chunk = fread($fhandle, ANTIVIRUS_CLAMAV_SOCKET_CHUNKSIZE);
+                    $size = pack('N', strlen($chunk));
+                    fwrite($socket, $size);
+                    fwrite($socket, $chunk);
+                }
+                // Terminate streaming.
+                fwrite($socket, pack('N', 0));
+                // Get ClamAV answer.
+                $output = stream_get_line($socket, 4096);
+
+                fclose($fhandle);
+            }
+            // Free up the ClamAV socket.
+            fclose($socket);
             // Parse the output.
             return $this->parse_socket_response($output);
         }
@@ -340,4 +395,20 @@ class scanner extends \core\antivirus\scanner {
                   'Use antivirus_clamav\scanner::scan_data_execute_socket() instead.', DEBUG_DEVELOPER);
         return $this->scan_data_execute_socket($data, "unixsocket");
     }
+
+    /**
+     * Scan file using Unix domain socket.
+     *
+     * @deprecated since Moodle 3.9 MDL-64075 - please do not use this function any more.
+     * @see antivirus_clamav\scanner::scan_file_execute_socket()
+     *
+     * @param string $file Full path to the file.
+     * @return int Scanning result constant.
+     */
+    public function scan_file_execute_unixsocket($file) {
+        debugging('antivirus_clamav\scanner::scan_file_execute_unixsocket() is deprecated. ' .
+                  'Use antivirus_clamav\scanner::scan_file_execute_socket() instead.', DEBUG_DEVELOPER);
+        return $this->scan_file_execute_socket($file, "unixsocket");
+    }
+
 }

lib/antivirus/clamav/tests/scanner_test.php

@@ -58,7 +58,7 @@ class antivirus_clamav_scanner_testcase extends advanced_testcase {
     public function test_scan_file_no_virus() {
         $methods = array(
             'scan_file_execute_commandline',
-            'scan_file_execute_unixsocket',
+            'scan_file_execute_socket',
             'message_admins',
             'get_config',
         );
@@ -69,10 +69,10 @@ class antivirus_clamav_scanner_testcase extends advanced_testcase {
         $configmap = array(array('runningmethod', 'commandline'));
         $antivirus->method('get_config')->will($this->returnValueMap($configmap));
 
-        // Configure scan_file_execute_commandline and scan_file_execute_unixsocket
+        // Configure scan_file_execute_commandline and scan_file_execute_socket
         // method stubs to behave as if no virus has been found (SCAN_RESULT_OK).
         $antivirus->method('scan_file_execute_commandline')->willReturn(0);
-        $antivirus->method('scan_file_execute_unixsocket')->willReturn(0);
+        $antivirus->method('scan_file_execute_socket')->willReturn(0);
 
         // Set expectation that message_admins is NOT called.
         $antivirus->expects($this->never())->method('message_admins');
@@ -87,12 +87,19 @@ class antivirus_clamav_scanner_testcase extends advanced_testcase {
 
         // Run mock scanning.
         $this->assertEquals(0, $antivirus->scan_file($this->tempfile, ''));
+
+        // Initiate mock scanning with configuration setting to use tcpsocket.
+        $configmap = array(array('runningmethod', 'tcpsocket'));
+        $antivirus->method('get_config')->will($this->returnValueMap($configmap));
+
+        // Run mock scanning.
+        $this->assertEquals(0, $antivirus->scan_file($this->tempfile, ''));
     }
 
     public function test_scan_file_virus() {
         $methods = array(
             'scan_file_execute_commandline',
-            'scan_file_execute_unixsocket',
+            'scan_file_execute_socket',
             'message_admins',
             'get_config',
         );
@@ -103,10 +110,10 @@ class antivirus_clamav_scanner_testcase extends advanced_testcase {
         $configmap = array(array('runningmethod', 'commandline'));
         $antivirus->method('get_config')->will($this->returnValueMap($configmap));
 
-        // Configure scan_file_execute_commandline and scan_file_execute_unixsocket
+        // Configure scan_file_execute_commandline and scan_file_execute_socket
         // method stubs to behave as if virus has been found (SCAN_RESULT_FOUND).
         $antivirus->method('scan_file_execute_commandline')->willReturn(1);
-        $antivirus->method('scan_file_execute_unixsocket')->willReturn(1);
+        $antivirus->method('scan_file_execute_socket')->willReturn(1);
 
         // Set expectation that message_admins is NOT called.
         $antivirus->expects($this->never())->method('message_admins');
@@ -121,12 +128,19 @@ class antivirus_clamav_scanner_testcase extends advanced_testcase {
 
         // Run mock scanning.
         $this->assertEquals(1, $antivirus->scan_file($this->tempfile, ''));
+
+        // Initiate mock scanning with configuration setting to use tcpsocket.
+        $configmap = array(array('runningmethod', 'tcpsocket'));
+        $antivirus->method('get_config')->will($this->returnValueMap($configmap));
+
+        // Run mock scanning.
+        $this->assertEquals(1, $antivirus->scan_file($this->tempfile, ''));
     }
 
     public function test_scan_file_error_donothing() {
         $methods = array(
             'scan_file_execute_commandline',
-            'scan_file_execute_unixsocket',
+            'scan_file_execute_socket',
             'message_admins',
             'get_config',
             'get_scanning_notice',
@@ -135,10 +149,10 @@ class antivirus_clamav_scanner_testcase extends advanced_testcase {
             ->setMethods($methods)
             ->getMock();
 
-        // Configure scan_file_execute_commandline and scan_file_execute_unixsocket
+        // Configure scan_file_execute_commandline and scan_file_execute_socket
         // method stubs to behave as if there is a scanning error (SCAN_RESULT_ERROR).
         $antivirus->method('scan_file_execute_commandline')->willReturn(2);
-        $antivirus->method('scan_file_execute_unixsocket')->willReturn(2);
+        $antivirus->method('scan_file_execute_socket')->willReturn(2);
         $antivirus->method('get_scanning_notice')->willReturn('someerror');
 
         // Set expectation that message_admins is called.
@@ -160,12 +174,20 @@ class antivirus_clamav_scanner_testcase extends advanced_testcase {
 
         // Run mock scanning.
         $this->assertEquals(2, $antivirus->scan_file($this->tempfile, ''));
+
+        // Initiate mock scanning with configuration setting to do nothing on
+        // scanning error and using tcpsocket.
+        $configmap = array(array('clamfailureonupload', 'donothing'), array('runningmethod', 'tcpsocket'));
+        $antivirus->method('get_config')->will($this->returnValueMap($configmap));
+
+        // Run mock scanning.
+        $this->assertEquals(2, $antivirus->scan_file($this->tempfile, ''));
     }
 
     public function test_scan_file_error_actlikevirus() {
         $methods = array(
             'scan_file_execute_commandline',
-            'scan_file_execute_unixsocket',
+            'scan_file_execute_socket',
             'message_admins',
             'get_config',
             'get_scanning_notice',
@@ -174,10 +196,10 @@ class antivirus_clamav_scanner_testcase extends advanced_testcase {
             ->setMethods($methods)
             ->getMock();
 
-        // Configure scan_file_execute_commandline and scan_file_execute_unixsocket
+        // Configure scan_file_execute_commandline and scan_file_execute_socket
         // method stubs to behave as if there is a scanning error (SCAN_RESULT_ERROR).
         $antivirus->method('scan_file_execute_commandline')->willReturn(2);
-        $antivirus->method('scan_file_execute_unixsocket')->willReturn(2);
+        $antivirus->method('scan_file_execute_socket')->willReturn(2);
         $antivirus->method('get_scanning_notice')->willReturn('someerror');
 
         // Set expectation that message_admins is called.
@@ -201,6 +223,15 @@ 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_file($this->tempfile, ''));
+
+        // 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));
+
+        // Run mock scanning, we expect SCAN_RESULT_FOUND since configuration
+        // require us to act like virus.
+        $this->assertEquals(1, $antivirus->scan_file($this->tempfile, ''));
     }
 
     public function test_scan_data_no_virus() {