Merge pull request #294 from nextcloud/icap-tls

add support for using tls with icap
nextcloud · Oct 27, 2023 · 1e11342 · 1e11342
2 parents 736857f + cb37c9f
commit 1e11342
Show file tree

Hide file tree

Showing 13 changed files with 137 additions and 15 deletions.
diff --git a/.github/workflows/scanner.yml b/.github/workflows/scanner.yml
@@ -22,12 +22,13 @@ jobs:
         php-versions: ['8.0']
         databases: ['sqlite']
         server-versions: [ 'stable27' ]
+        transport: ['plain', 'tls']
 
-    name: icap-clamav
+    name: icap-clamav-${{ matrix.transport }}
 
     services:
       clam:
-        image: deepdiver/icap-clamav-service
+        image: ghcr.io/icewind1991/icap-clamav-service-tls
         ports:
           - 1344:1344
 
@@ -66,7 +67,8 @@ jobs:
         working-directory: apps/${{ env.APP_NAME }}
         env:
           ICAP_HOST: localhost
-          ICAP_PORT: 1344
+          ICAP_PORT: ${{ matrix.transport == 'tls' && '1345' || '1344' }}
+          ICAP_TRANSPORT: ${{ matrix.transport }}
           ICAP_REQUEST: avscan
           ICAP_HEADER: X-Infection-Found
           ICAP_MODE: reqmod
@@ -75,7 +77,8 @@ jobs:
         working-directory: apps/${{ env.APP_NAME }}
         env:
           ICAP_HOST: localhost
-          ICAP_PORT: 1344
+          ICAP_PORT: ${{ matrix.transport == 'tls' && '1345' || '1344' }}
+          ICAP_TRANSPORT: ${{ matrix.transport }}
           ICAP_REQUEST: avscan
           ICAP_HEADER: X-Infection-Found
           ICAP_MODE: respmod

diff --git a/README.md b/README.md
@@ -58,3 +58,13 @@ Additionally, the Kaspersky scan engine needs some additional configuration:
 
 - ["Allow204"](https://support.kaspersky.com/ScanEngine/1.0/en-US/201151.htm) should be enabled.
 - For version 2.0 and later, the [virus response header](https://support.kaspersky.com/ScanEngine/1.0/en-US/201214.htm) needs to be configured
+
+### TLS Encryption
+
+Using TLS encryption for the ICAP connection is supported, this requires the ICAP server to use a valid certificate.
+If the certificate isn't signed by a trusted certificate authority, you can import the certificate into Nextcloud's
+certificate bundle using
+
+```shell
+occ security:certificates:import /path/to/certificate
+```
diff --git a/appinfo/info.xml b/appinfo/info.xml
@@ -14,7 +14,7 @@
 This application inspects files that are uploaded to Nextcloud for viruses before they are written to the Nextcloud storage. If a file is identified as a virus, it is either logged or not uploaded to the server. The application relies on the underlying ClamAV virus scanning engine, which the admin points Nextcloud to when configuring the application. Alternatively, a Kaspersky Scan Engine can be configured, which has to run on a separate server.
 For this app to be effective, the ClamAV virus definitions should be kept up to date. Also note that enabling this app will impact system performance as additional processing is required for every upload. More information is available in the Antivirus documentation.
 	]]></description>
-	<version>5.2.3</version>
+	<version>5.3.0</version>
 	<licence>agpl</licence>
 
 	<author>Manuel Delgado</author>

diff --git a/css/settings.css b/css/settings.css
@@ -17,6 +17,11 @@
 	padding-left: 6px;
 }
 
+.section-antivirus table input[type="checkbox"] {
+	width: 18px;
+	height: 18px;
+}
+
 .section-antivirus table td:last-child {
 	padding-right: 6px;
 	padding-left: 6px;

diff --git a/js/settings.js b/js/settings.js
@@ -141,9 +141,9 @@ function av_mode_show_options(str, mode = 'slow') {
 		$('tr.av_path').show(mode);
 	}
 	if (str === 'icap'){
-		$('tr.av_icap_service, tr.av_icap_header, tr.av_icap_preset, tr.av_icap_mode').show(mode);
+		$('tr.av_icap_service, tr.av_icap_header, tr.av_icap_preset, tr.av_icap_mode, tr.av_icap_tls').show(mode);
 	} else {
-		$('tr.av_icap_service, tr.av_icap_header, tr.av_icap_preset, tr.av_icap_mode').hide(mode);
+		$('tr.av_icap_service, tr.av_icap_header, tr.av_icap_preset, tr.av_icap_mode, tr.av_icap_tls').hide(mode);
 	}
 	if (str === 'kaspersky' || str === 'icap') {
 		$('#antivirus-advanced-wrapper').hide(mode);

diff --git a/lib/AppConfig.php b/lib/AppConfig.php
@@ -60,6 +60,7 @@ class AppConfig {
 		'av_infected_action' => 'only_log',
 		'av_background_scan' => 'on',
 		'av_icap_mode' => ICAPClient::MODE_REQ_MOD,
+		'av_icap_tls' => false,
 		'av_icap_request_service' => 'avscan',
 		'av_icap_response_header' => 'X-Infection-Found',
 		'av_icap_chunk_size' => '1048576',
@@ -86,6 +87,14 @@ public function getAvMaxFileSize(): int {
 		return (int)$this->getAppValue('av_max_file_size');
 	}
 
+	public function getAvIcapTls(): bool {
+		return (bool)$this->getAppValue('av_icap_tls');
+	}
+
+	public function setAvIcapTls(bool $enable): void {
+		$this->setAppValue('av_icap_tls', $enable ? '1' : '0');
+	}
+
 	/**
 	 * Get full commandline
 	 *

diff --git a/lib/Controller/SettingsController.php b/lib/Controller/SettingsController.php
@@ -52,6 +52,7 @@ public function __construct($appName, IRequest $request, AppConfig $appconfig, I
 	 * @param int $avMaxFileSize - file size limit
 	 * @param int $avScanFirstBytes - scan size limit
 	 * @param string $avIcapMode
+	 * @param bool $avIcapTls
 	 * @return JSONResponse
 	 */
 	public function save(
@@ -67,7 +68,8 @@ public function save(
 		$avScanFirstBytes,
 		$avIcapMode,
 		$avIcapRequestService,
-		$avIcapResponseHeader
+		$avIcapResponseHeader,
+		$avIcapTls
 	) {
 		$this->settings->setAvMode($avMode);
 		$this->settings->setAvSocket($avSocket);
@@ -82,6 +84,7 @@ public function save(
 		$this->settings->setAvIcapMode($avIcapMode);
 		$this->settings->setAvIcapRequestService($avIcapRequestService);
 		$this->settings->setAvIcapResponseHeader($avIcapResponseHeader);
+		$this->settings->setAvIcapTls((bool)$avIcapTls);
 
 		try {
 			$scanner = $this->scannerFactory->getScanner();

diff --git a/lib/ICAP/ICAPClient.php b/lib/ICAP/ICAPClient.php
@@ -30,11 +30,11 @@ class ICAPClient {
 	public const MODE_RESP_MOD = 'respmod';
 
 	/** @var string */
-	private $host;
+	protected $host;
 	/** @var int */
-	private $port;
+	protected $port;
 	/** @var int */
-	private $connectTimeout;
+	protected $connectTimeout;
 
 	public function __construct(string $host, int $port, int $connectTimeout) {
 		$this->host = $host;
@@ -47,7 +47,7 @@ public function __construct(string $host, int $port, int $connectTimeout) {
 	 *
 	 * @return resource
 	 */
-	private function connect() {
+	protected function connect() {
 		$stream = @\stream_socket_client(
 			"tcp://{$this->host}:{$this->port}",
 			$errorCode,

diff --git a/lib/ICAP/ICAPTlsClient.php b/lib/ICAP/ICAPTlsClient.php
@@ -0,0 +1,70 @@
+<?php
+
+declare(strict_types=1);
+/**
+ * @copyright Copyright (c) 2022 Robin Appelman <robin@icewind.nl>
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+namespace OCA\Files_Antivirus\ICAP;
+
+use OCP\ICertificateManager;
+use RuntimeException;
+
+class ICAPTlsClient extends ICAPClient {
+	private ICertificateManager $certificateManager;
+
+	public function __construct(
+		string $host,
+		int $port,
+		int $connectTimeout,
+		ICertificateManager $certificateManager
+	) {
+		parent::__construct($host, $port, $connectTimeout);
+		$this->certificateManager = $certificateManager;
+	}
+
+	/**
+	 * Connect to ICAP server
+	 *
+	 * @return resource
+	 */
+	protected function connect() {
+		$ctx = stream_context_create([
+			'ssl' => [
+				'cafile' => $this->certificateManager->getAbsoluteBundlePath()
+			],
+		]);
+		$stream = \stream_socket_client(
+			"tls://{$this->host}:{$this->port}",
+			$errorCode,
+			$errorMessage,
+			$this->connectTimeout,
+			STREAM_CLIENT_CONNECT,
+			$ctx
+		);
+
+		if (!$stream) {
+			throw new RuntimeException(
+				"Cannot connect to \"tls://{$this->host}:{$this->port}\": $errorMessage (code $errorCode)"
+			);
+		}
+
+		return $stream;
+	}
+}
diff --git a/lib/ICAP/ResponseParser.php b/lib/ICAP/ResponseParser.php
@@ -53,7 +53,11 @@ public function read_response($stream): IcapResponse {
 	 * @return IcapResponseStatus
 	 */
 	private function readIcapStatusLine($stream): IcapResponseStatus {
-		$icapHeader = \trim(\fgets($stream));
+		$rawHeader = \fgets($stream);
+		if (!$rawHeader) {
+			throw new RuntimeException("Empty ICAP response");
+		}
+		$icapHeader = \trim($rawHeader);
 		$numValues = \sscanf($icapHeader, "ICAP/%d.%d %d %s", $v1, $v2, $code, $status);
 		if ($numValues !== 4) {
 			throw new RuntimeException("Unknown ICAP response: \"$icapHeader\"");

diff --git a/lib/Scanner/ICAP.php b/lib/Scanner/ICAP.php
@@ -27,8 +27,10 @@
 use OCA\Files_Antivirus\AppConfig;
 use OCA\Files_Antivirus\ICAP\ICAPClient;
 use OCA\Files_Antivirus\ICAP\ICAPRequest;
+use OCA\Files_Antivirus\ICAP\ICAPTlsClient;
 use OCA\Files_Antivirus\Status;
 use OCA\Files_Antivirus\StatusFactory;
+use OCP\ICertificateManager;
 use Psr\Log\LoggerInterface;
 
 class ICAP extends ScannerBase {
@@ -39,11 +41,13 @@ class ICAP extends ScannerBase {
 	private string $service;
 	private string $virusHeader;
 	private int $chunkSize;
+	private bool $tls;
 
 	public function __construct(
 		AppConfig $config,
 		LoggerInterface $logger,
-		StatusFactory $statusFactory
+		StatusFactory $statusFactory,
+		ICertificateManager $certificateManager
 	) {
 		parent::__construct($config, $logger, $statusFactory);
 
@@ -53,11 +57,16 @@ public function __construct(
 		$this->virusHeader = $config->getAvIcapResponseHeader();
 		$this->chunkSize = (int)$config->getAvIcapChunkSize();
 		$this->mode = $config->getAvIcapMode();
+		$this->tls = $config->getAvIcapTls();
 
 		if (!($avHost && $avPort)) {
 			throw new \RuntimeException('The ICAP port and host are not set up.');
 		}
-		$this->icapClient = new ICAPClient($avHost, (int)$avPort, (int)$config->getAvIcapConnectTimeout());
+		if ($this->tls) {
+			$this->icapClient = new ICAPTlsClient($avHost, (int)$avPort, (int)$config->getAvIcapConnectTimeout(), $certificateManager);
+		} else {
+			$this->icapClient = new ICAPClient($avHost, (int)$avPort, (int)$config->getAvIcapConnectTimeout());
+		}
 	}
 
 	public function initScanner() {

diff --git a/templates/settings.php b/templates/settings.php
@@ -35,6 +35,13 @@
 					<td><input type="text" id="av_port" name="avPort" value="<?php p($_['avPort']); ?>" title="<?php p($l->t('Port number of Antivirus Host.')). ' ' .$l->t('Not required in Executable Mode.');?>"></td>
 					<td></td>
 				</tr>
+				<tr class="av_icap_tls">
+					<td><label for="av_icap_tls"><?php p($l->t('Tls'));?></label></td>
+					<td>
+						<input type="checkbox" id="av_icap_tls" name="avIcapTls" <?php p($_['avIcapTls'] ? 'checked="checked"' : ''); ?>" title="<?php p($l->t('Use TLS encryption.'));?>">
+					</td>
+					<td></td>
+				</tr>
 				<tr class="av_icap_preset">
 					<td><?php p($l->t('ICAP preset'));?></td>
 					<td><select id="av_icap_preset">

diff --git a/tests/Scanner/ICAPTest.php b/tests/Scanner/ICAPTest.php
@@ -53,6 +53,8 @@ protected function getScanner(): ScannerBase {
 						return getenv('ICAP_HEADER');
 					case 'av_icap_mode':
 						return getenv('ICAP_MODE');
+					case 'av_icap_tls':
+						return getenv('ICAP_TRANSPORT') === 'tls';
 					case 'av_stream_max_length':
 						return '26214400';
 					case 'av_icap_chunk_size':