Merge pull request #4079 from nextcloud/fix/matterbridge-binary-exec-perm

Make sure matterbridge binary is executable

lib/Controller/MatterbridgeSettingsController.php

@@ -27,6 +27,7 @@ namespace OCA\Talk\Controller;
 
 use OCA\Talk\MatterbridgeManager;
 use OCA\Talk\Exceptions\ImpossibleToKillException;
+use OCA\Talk\Exceptions\WrongPermissionsException;
 use OCP\AppFramework\Http;
 use OCP\AppFramework\Http\DataResponse;
 use OCP\AppFramework\OCSController;
@@ -49,10 +50,16 @@ class MatterbridgeSettingsController extends OCSController {
 	 * @return DataResponse
 	 */
 	public function getMatterbridgeVersion(): DataResponse {
-		$version = $this->bridgeManager->getCurrentVersionFromBinary();
-		if ($version === null) {
+		try {
+			$version = $this->bridgeManager->getCurrentVersionFromBinary();
+			if ($version === null) {
+				return new DataResponse([
+					'error' => 'binary',
+				], Http::STATUS_BAD_REQUEST);
+			}
+		} catch (WrongPermissionsException $e) {
 			return new DataResponse([
-				'error' => 'binary',
+				'error' => 'binary_permissions',
 			], Http::STATUS_BAD_REQUEST);
 		}
 

lib/Exceptions/WrongPermissionsException.php

@@ -0,0 +1,28 @@
+<?php
+
+declare(strict_types=1);
+/**
+ * @copyright Copyright (c) 2020 Julien Veyssier <eneiluj@posteo.net>
+ *
+ * @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\Talk\Exceptions;
+
+class WrongPermissionsException extends \Exception {
+}

lib/MatterbridgeManager.php

@@ -40,6 +40,7 @@ use Doctrine\DBAL\Exception\UniqueConstraintViolationException;
 use Psr\Log\LoggerInterface;
 
 use OCA\Talk\Exceptions\ImpossibleToKillException;
+use OCA\Talk\Exceptions\WrongPermissionsException;
 use OCA\Talk\Exceptions\ParticipantNotFoundException;
 
 class MatterbridgeManager {
@@ -683,6 +684,33 @@ class MatterbridgeManager {
 			return null;
 		}
 
+		// is www user the file owner?
+		$user = posix_getpwuid(posix_getuid());
+		$fileOwner = posix_getpwuid(fileowner($binaryPath));
+		$userIsOwner = $user['name'] === $fileOwner['name'];
+
+		// get file group and user groups
+		$fileGid = filegroup($binaryPath);
+		$myGids = posix_getgroups();
+
+		// check permissions
+		$perms = fileperms($binaryPath);
+		$uExec = (($perms & 0x0040) && !($perms & 0x0800));
+		$gExec = (($perms & 0x0008) && !($perms & 0x0400));
+		$aExec = (($perms & 0x0001) && !($perms & 0x0200));
+
+		// 3 ways to have the permission :
+		// * be the owner and have u+x perm
+		// * not be the owner, be in the file group and have g+x perm
+		// * have o+x perm
+		$execPerm = $aExec
+			|| ($user['name'] === $fileOwner['name'] && $uExec)
+			|| ($user['name'] !== $fileOwner['name'] && in_array($fileGid, $myGids) && $gExec);
+
+		if (!$execPerm) {
+			throw new WrongPermissionsException();
+		}
+
 		$cmd = escapeshellcmd($binaryPath) . ' ' . escapeshellarg('-version');
 		@exec($cmd, $output, $returnCode);
 

lib/Settings/Admin/AdminSettings.php

@@ -29,6 +29,7 @@ use OCA\Talk\Model\Command;
 use OCA\Talk\Participant;
 use OCA\Talk\Room;
 use OCA\Talk\Service\CommandService;
+use OCA\Talk\Exceptions\WrongPermissionsException;
 use OCP\AppFramework\Http\TemplateResponse;
 use OCP\ICacheFactory;
 use OCP\IConfig;
@@ -119,9 +120,21 @@ class AdminSettings implements ISettings {
 	}
 
 	protected function initMatterbridge(): void {
+		$error = '';
+		try {
+			$version = (string) $this->bridgeManager->getCurrentVersionFromBinary();
+			if ($version === '') {
+				$error = 'binary';
+			}
+		} catch (WrongPermissionsException $e) {
+			$version = '';
+			$error = 'binary_permissions';
+		}
+		$this->initialStateService->provideInitialState(
+			'talk', 'matterbridge_error', $error
+		);
 		$this->initialStateService->provideInitialState(
-			'talk', 'matterbridge_version',
-			(string) $this->bridgeManager->getCurrentVersionFromBinary()
+			'talk', 'matterbridge_version', $version
 		);
 
 		$this->initialStateService->provideInitialState(

src/components/AdminSettings/MatterbridgeIntegration.vue

@@ -49,6 +49,10 @@
 		<template v-else>
 			<p class="settings-hint" v-html="description" />
 
+			<p v-if="errorText" class="settings-hint">
+				{{ errorText }}
+			</p>
+
 			<p>
 				<button v-if="isInstalling">
 					<span class="icon icon-loading-small" />
@@ -82,6 +86,7 @@ export default {
 			matterbridgeEnabled: loadState('talk', 'matterbridge_enable'),
 			matterbridgeVersion: loadState('talk', 'matterbridge_version'),
 			isInstalling: false,
+			error: loadState('talk', 'matterbridge_error'),
 		}
 	},
 
@@ -97,6 +102,15 @@ export default {
 				.replace('{linkstart2}', '<a  target="_blank" rel="noreferrer nofollow" class="external" href="https://apps.nextcloud.com/apps/talk_matterbridge">')
 				.replace(/{linkend}/g, ' ↗</a>')
 		},
+		errorText() {
+			if (this.error === 'binary_permissions') {
+				return t('spreed', 'Matterbridge binary has incorrect permissions. Please make sure the Matterbridge binary file is owned by the correct user and can be executed. It can be found in "/.../nextcloud/apps/talk_matterbridge/bin/".')
+			} else if (this.error === 'binary') {
+				return t('spreed', 'Matterbridge binary was not found or couldn\'t be executed.')
+			} else {
+				return ''
+			}
+		},
 	},
 
 	methods: {
@@ -138,12 +152,17 @@ export default {
 				this.matterbridgeVersion = response.data.ocs.data.version
 				this.matterbridgeEnabled = true
 				this.saveMatterbridgeEnabled()
-
-				this.isInstalling = false
+				this.error = ''
 			} catch (e) {
 				console.error(e)
 				showError(t('spreed', 'Failed to execute Matterbridge binary.'))
+				if (e.response && e.response.data && e.response.data.ocs && e.response.data.ocs.data && e.response.data.ocs.data.error) {
+					this.error = e.response.data.ocs.data.error
+				} else {
+					this.error = 'binary'
+				}
 			}
+			this.isInstalling = false
 		},
 	},
 }