feat: add option whether to block unscannable files

Signed-off-by: Robin Appelman <robin@icewind.nl>

lib/AppConfig.php

@@ -65,6 +65,7 @@ class AppConfig {
 		'av_icap_chunk_size' => '1048576',
 		'av_icap_connect_timeout' => '5',
 		'av_scan_first_bytes' => -1,
+		'av_block_unscannable' => false,
 	];
 
 	/**
@@ -94,6 +95,14 @@ public function setAvIcapTls(bool $enable): void {
 		$this->setAppValue('av_icap_tls', $enable ? '1' : '0');
 	}
 
+	public function getAvBlockUnscannable(): bool {
+		return (bool)$this->getAppValue('av_block_unscannable');
+	}
+
+	public function setAvBlockUnscannable(bool $block): void {
+		$this->setAppValue('av_block_unscannable', $block ? '1' : '0');
+	}
+
 	/**
 	 * Get full commandline
 	 *

lib/AppInfo/Application.php

@@ -100,6 +100,8 @@ function (string $mountPoint, IStorage $storage) {
 				$activityManager = $container->get(IManager::class);
 				$eventDispatcher = $container->get(IEventDispatcher::class);
 				$appManager = $container->get(IAppManager::class);
+				/** @var AppConfig $appConfig */
+				$appConfig = $container->get(AppConfig::class);
 				return new AvirWrapper([
 					'storage' => $storage,
 					'scannerFactory' => $scannerFactory,
@@ -110,6 +112,7 @@ function (string $mountPoint, IStorage $storage) {
 					'eventDispatcher' => $eventDispatcher,
 					'trashEnabled' => $appManager->isEnabledForUser('files_trashbin'),
 					'mount_point' => $mountPoint,
+					'block_unscannable' => $appConfig->getAvBlockUnscannable(),
 				]);
 			},
 			1

lib/AvirWrapper.php

@@ -32,6 +32,7 @@ class AvirWrapper extends Wrapper {
 	private bool $shouldScan = true;
 	private bool $trashEnabled;
 	private string $mountPoint;
+	private bool $blockUnscannable = false;
 
 	/**
 	 * @param array $parameters
@@ -45,6 +46,7 @@ public function __construct($parameters) {
 		$this->isHomeStorage = $parameters['isHomeStorage'];
 		$this->trashEnabled = $parameters['trashEnabled'];
 		$this->mountPoint = $parameters['mount_point'];
+		$this->blockUnscannable = $parameters['block_unscannable'];
 
 		/** @var IEventDispatcher $eventDispatcher */
 		$eventDispatcher = $parameters['eventDispatcher'];
@@ -107,6 +109,9 @@ function () use ($scanner, $path) {
 					if ($status->getNumericStatus() === Status::SCANRESULT_INFECTED) {
 						$this->handleInfected($path, $status);
 					}
+					if ($this->blockUnscannable && $status->getNumericStatus() === Status::SCANRESULT_UNSCANNABLE) {
+						$this->handleInfected($path, $status);
+					}
 				}
 			);
 		} catch (\Exception $e) {

lib/Command/Scan.php

@@ -80,6 +80,10 @@ protected function execute(InputInterface $input, OutputInterface $output): int
 				$status = "is <error>infected</error>";
 				$exit = 1;
 				break;
+			case \OCA\Files_Antivirus\Status::SCANRESULT_UNSCANNABLE:
+				$status = "is not scannable";
+				$exit = 2;
+				break;
 		}
 		if ($result->getDetails()) {
 			$details = ": " . $result->getDetails();

lib/Command/Test.php

@@ -79,6 +79,8 @@ protected function execute(InputInterface $input, OutputInterface $output): int
 			return 1;
 		} elseif ($result->getNumericStatus() === Status::SCANRESULT_UNCHECKED) {
 			$output->writeln("<comment>- file not scanned or scan still pending</comment>");
+		} elseif ($result->getNumericStatus() === Status::SCANRESULT_UNSCANNABLE) {
+			$output->writeln("<comment>- file could not be scanned</comment>");
 		} else {
 			$output->writeln("<info>✓</info>");
 		}
@@ -100,6 +102,8 @@ protected function execute(InputInterface $input, OutputInterface $output): int
 			return 1;
 		} elseif ($result->getNumericStatus() === Status::SCANRESULT_UNCHECKED) {
 			$output->writeln("<comment>- file not scanned or scan still pending</comment>");
+		} elseif ($result->getNumericStatus() === Status::SCANRESULT_UNSCANNABLE) {
+			$output->writeln("<comment>- file could not be scanned</comment>");
 		} else {
 			$output->writeln("<info>✓</info>");
 		}

lib/Controller/SettingsController.php

@@ -52,6 +52,7 @@ public function __construct($appName, IRequest $request, AppConfig $appconfig, I
 	 * @param int $avScanFirstBytes - scan size limit
 	 * @param string $avIcapMode
 	 * @param bool $avIcapTls
+	 * @param bool $avBlockUnscannable
 	 * @return JSONResponse
 	 */
 	public function save(
@@ -68,7 +69,8 @@ public function save(
 		$avIcapMode,
 		$avIcapRequestService,
 		$avIcapResponseHeader,
-		$avIcapTls
+		$avIcapTls,
+		$avBlockUnscannable
 	) {
 		$this->settings->setAvMode($avMode);
 		$this->settings->setAvSocket($avSocket);
@@ -84,6 +86,7 @@ public function save(
 		$this->settings->setAvIcapRequestService($avIcapRequestService);
 		$this->settings->setAvIcapResponseHeader($avIcapResponseHeader);
 		$this->settings->setAvIcapTls((bool)$avIcapTls);
+		$this->settings->setAvBlockUnscannable((bool)$avBlockUnscannable);
 
 		try {
 			$scanner = $this->scannerFactory->getScanner('/self-test.txt');

lib/Scanner/ExternalKaspersky.php

@@ -85,7 +85,7 @@ protected function scanBuffer(): void {
 		} elseif (substr($scanResult, 0, 11) === 'NON_SCANNED' && $this->status->getNumericStatus() != Status::SCANRESULT_INFECTED) {
 			if ($scanResult === 'NON_SCANNED (PASSWORD PROTECTED)') {
 				// if we can't scan the file at all, there is no use in trying to scan it again later
-				$this->status->setNumericStatus(Status::SCANRESULT_CLEAN);
+				$this->status->setNumericStatus(Status::SCANRESULT_UNSCANNABLE);
 			} else {
 				$this->status->setNumericStatus(Status::SCANRESULT_UNCHECKED);
 			}

lib/Status.php

@@ -12,23 +12,28 @@
 use Psr\Log\LoggerInterface;
 
 class Status {
-	/*
+	/**
 	 *  The file was not checked (e.g. because the AV daemon wasn't running).
 	 */
 	public const SCANRESULT_UNCHECKED = -1;
 
-	/*
+	/**
 	 *  The file was checked and found to be clean.
 	 */
 	public const SCANRESULT_CLEAN = 0;
 
-	/*
+	/**
 	 *  The file was checked and found to be infected.
 	 */
 	public const SCANRESULT_INFECTED = 1;
 
-	/*
-	 * Should be SCANRESULT_UNCHECKED | SCANRESULT_INFECTED | SCANRESULT_CLEAN
+	/**
+	 * The file cannot be checked (e.g. because it is password protected)
+	 */
+	public const SCANRESULT_UNSCANNABLE = 2;
+
+	/**
+	 * Should be SCANRESULT_UNCHECKED | SCANRESULT_INFECTED | SCANRESULT_CLEAN | SCANRESULT_UNSCANNABLE
 	 */
 	protected $numericStatus = self::SCANRESULT_UNCHECKED;
 
@@ -37,12 +42,15 @@ class Status {
 	 */
 	protected $details = '';
 
+	private bool $blockUnscannable = false;
+
 	protected RuleMapper $ruleMapper;
 	protected LoggerInterface $logger;
 
-	public function __construct(RuleMapper $ruleMapper, LoggerInterface $logger) {
+	public function __construct(RuleMapper $ruleMapper, LoggerInterface $logger, AppConfig $config) {
 		$this->ruleMapper = $ruleMapper;
 		$this->logger = $logger;
+		$this->blockUnscannable = $config->getAvBlockUnscannable();
 	}
 
 	/**
@@ -71,11 +79,11 @@ public function setDetails(string $details): void {
 
 	/**
 	 * @param string $rawResponse
-	 * @param integer $result
+	 * @param ?integer $result
 	 *
 	 * @return void
 	 */
-	public function parseResponse($rawResponse, $result = null) {
+	public function parseResponse(string $rawResponse, int $result = null) {
 		$matches = [];
 
 		if (is_null($result)) { // Daemon or socket mode
@@ -165,6 +173,13 @@ public function dispatch(Item $item): void {
 			case self::SCANRESULT_CLEAN:
 				$item->processClean();
 				break;
+			case self::SCANRESULT_UNSCANNABLE:
+				if ($this->blockUnscannable) {
+					$item->processInfected($this);
+				} else {
+					$item->processClean();
+				}
+				break;
 		}
 	}
 }

lib/StatusFactory.php

@@ -12,16 +12,23 @@
 class StatusFactory {
 	private RuleMapper $ruleMapper;
 	private LoggerInterface $logger;
+	private AppConfig $config;
 
-	public function __construct(RuleMapper $ruleMapper, LoggerInterface $logger) {
+	public function __construct(
+		RuleMapper $ruleMapper,
+		LoggerInterface $logger,
+		AppConfig $config
+	) {
 		$this->ruleMapper = $ruleMapper;
 		$this->logger = $logger;
+		$this->config = $config;
 	}
 
 	public function newStatus(): Status {
 		return new Status(
 			$this->ruleMapper,
-			$this->logger
+			$this->logger,
+			$this->config,
 		);
 	}
 }

templates/settings.php

@@ -121,6 +121,15 @@
 					<td><select id="av_infected_action" name="avInfectedAction"><?php print_unescaped(html_select_options(['only_log' => $l->t('Only log'), 'delete' => $l->t('Delete file')], $_['avInfectedAction'])) ?></select></td>
 					<td></td>
 				</tr>
+				<tr class="av_block_unscannable">
+					<td><label for="av_block_unscannable"><?php p($l->t('Block unscannable files (such as encrypted archives)'));?></label></td>
+					<td>
+						<input type="checkbox" id="av_block_unscannable" name="avBlockUnscannable"  <?php p($_['avBlockUnscannable'] ? 'checked="checked"' : ''); ?>""
+							   title="<?php p($l->t('Block unscannable files (such as encrypted archives)'));?>"
+						/>
+					</td>
+					<td></td>
+				</tr>
 			</table>
 			<input id="av_submit" type="submit" value="<?php p($l->t('Save'));?>" />
 			<span id="antivirus_save_msg"></span>

tests/AvirWrapperTest.php

@@ -76,6 +76,7 @@ protected function setUp(): void {
 			'eventDispatcher' => $this->createMock(EventDispatcherInterface::class),
 			'trashEnabled' => true,
 			'mount_point' => '/' . self::UID . '/files/',
+			'block_unscannable' => false,
 		]);
 
 		$this->config->expects($this->any())

tests/StatusTest.php

@@ -8,53 +8,59 @@
 
 namespace OCA\Files_Antivirus\Tests;
 
+use OCA\Files_Antivirus\AppConfig;
 use OCA\Files_Antivirus\Db\RuleMapper;
 use Psr\Log\LoggerInterface;
 
 /**
  * @group DB
  */
 class StatusTest extends TestBase {
-	
+
 	// See OCA\Files_Antivirus\Status::init for details
 	public const TEST_CLEAN = 0;
 	public const TEST_INFECTED = 1;
 	public const TEST_ERROR = 40;
-
-	protected $ruleMapper;
 
+	protected RuleMapper $ruleMapper;
+	private bool $blockUnscannable = false;
 
 	protected function setUp(): void {
 		parent::setUp();
 		$this->ruleMapper = new RuleMapper($this->db);
 		$this->ruleMapper->deleteAll();
 		$this->ruleMapper->populate();
+		$this->config->method('getAvBlockUnscannable')
+			->willReturnCallback(function() {
+				return $this->blockUnscannable;
+			});
 	}
-	
+
 	public function testParseResponse() {
 		// Testing status codes
 		$testStatus = new \OCA\Files_Antivirus\Status(
 			$this->ruleMapper,
-			$this->createMock(LoggerInterface::class)
+			$this->createMock(LoggerInterface::class),
+			$this->config,
 		);
-		
+
 		$testStatus->parseResponse('dummy : OK', self::TEST_CLEAN);
 		$cleanScan = $testStatus->getNumericStatus();
 		$this->assertEquals(\OCA\Files_Antivirus\Status::SCANRESULT_CLEAN, $cleanScan);
 		$this->assertEquals("", $testStatus->getDetails());
-		
+
 		$scanOutput = "Thu Oct 28 13:02:19 2010 -> /tmp/kitten: Heuristics.Broken.Executable FOUND ";
 		$testStatus->parseResponse($scanOutput, self::TEST_INFECTED);
 		$infectedScan = $testStatus->getNumericStatus();
 		$this->assertEquals(\OCA\Files_Antivirus\Status::SCANRESULT_INFECTED, $infectedScan);
 		$this->assertEquals('Heuristics.Broken.Executable', $testStatus->getDetails());
-		
+
 		$testStatus->parseResponse('dummy', self::TEST_ERROR);
 		$failedScan = $testStatus->getNumericStatus();
 		$this->assertEquals(\OCA\Files_Antivirus\Status::SCANRESULT_UNCHECKED, $failedScan);
 		$this->assertEquals('Unknown option passed.', $testStatus->getDetails());
-		
-		
+
+
 		// Testing raw output (e.g. daemon mode)
 		$assertDetailsWithResponse = function ($response) use ($testStatus) {
 			$expected = "No matching rule for response [$response]. Please check antivirus rules configuration.";
@@ -66,32 +72,32 @@ public function testParseResponse() {
 		$failedScan2 = $testStatus->getNumericStatus();
 		$this->assertEquals(\OCA\Files_Antivirus\Status::SCANRESULT_UNCHECKED, $failedScan2);
 		$assertDetailsWithResponse('');
-		
+
 		// No rules matched result is unknown too
 		$testStatus->parseResponse('123dc');
 		$failedScan3 = $testStatus->getNumericStatus();
 		$this->assertEquals(\OCA\Files_Antivirus\Status::SCANRESULT_UNCHECKED, $failedScan3);
 		$assertDetailsWithResponse('123dc');
-		
+
 		// Raw result is added to details when no rule matched (only ASCII text range 32..126 excluding '`').
 		for ($c = 0; $c < 256; $c++) {
 			$testStatus->parseResponse(chr($c));
 			$expected = $c < 32 || $c > 126 || chr($c) == '`' ? '' : chr($c);
 			$assertDetailsWithResponse($expected);
 		}
-		
+
 		// Raw result in details is truncated at 512 chars.
 		$testStatus->parseResponse(str_repeat('a', 512));
 		$assertDetailsWithResponse(str_repeat('a', 512));
 		$testStatus->parseResponse(str_repeat('a', 513));
 		$assertDetailsWithResponse(str_repeat('a', 509) . '...');
-		
+
 		// File is clean
 		$testStatus->parseResponse('Thu Oct 28 13:02:19 2010 -> /tmp/kitten : OK');
 		$cleanScan2 = $testStatus->getNumericStatus();
 		$this->assertEquals(\OCA\Files_Antivirus\Status::SCANRESULT_CLEAN, $cleanScan2);
 		$this->assertEquals('', $testStatus->getDetails());
-		
+
 		// File is infected
 		$testStatus->parseResponse('Thu Oct 28 13:02:19 2010 -> /tmp/kitten: Heuristics.Broken.Kitten FOUND');
 		$infectedScan2 = $testStatus->getNumericStatus();

tests/TestBase.php

@@ -39,7 +39,7 @@ protected function setUp(): void {
 
 		$this->config = $this->getMockBuilder(AppConfig::class)
 			->disableOriginalConstructor()
-			->setMethods(['getAvPath', 'getAvChunkSize', 'getAvMode', 'getAppValue', 'getAvHost', 'getAvPort'])
+			->setMethods(['getAvPath', 'getAvChunkSize', 'getAvMode', 'getAppValue', 'getAvHost', 'getAvPort',  'getAvBlockUnscannable'])
 			->getMock();
 
 		$this->config->expects($this->any())