Verify signature of apps with level "Official" coming from the appstore

This change will verify the signature of all apps with the level "Official" coming from the appstore or if they have been signed before.

4 files changed, 58 insertions, 13 deletions

diff --git a/lib/private/app.php b/lib/private/app.php
index 500a60060e6..74a89b8bc78 100644
--- a/lib/private/app.php
+++ b/lib/private/app.php
@@ -312,14 +312,14 @@ class OC_App {
 	 * @param string $app
 	 * @return int
 	 */
-	public static function downloadApp($app) {
+	private static function downloadApp($app) {
 		$ocsClient = new OCSClient(
 			\OC::$server->getHTTPClientService(),
 			\OC::$server->getConfig(),
 			\OC::$server->getLogger()
 		);
 		$appData = $ocsClient->getApplication($app, \OCP\Util::getVersion());
-		$download= $ocsClient->getApplicationDownload($app, \OCP\Util::getVersion());
+		$download = $ocsClient->getApplicationDownload($app, \OCP\Util::getVersion());
 		if(isset($download['downloadlink']) and $download['downloadlink']!='') {
 			// Replace spaces in download link without encoding entire URL
 			$download['downloadlink'] = str_replace(' ', '%20', $download['downloadlink']);
@@ -855,7 +855,7 @@ class OC_App {
 	 * @param string $ocsID
 	 * @return string|false
 	 */
-	protected static function getInternalAppIdByOcs($ocsID) {
+	public static function getInternalAppIdByOcs($ocsID) {
 		if(is_numeric($ocsID)) {
 			$idArray = \OC::$server->getAppConfig()->getValues(false, 'ocsid');
 			if(array_search($ocsID, $idArray)) {
@@ -1036,7 +1036,7 @@ class OC_App {
 
 
 	/**
-	 * @param mixed $app
+	 * @param string $app
 	 * @return bool
 	 * @throws Exception if app is not compatible with this version of ownCloud
 	 * @throws Exception if no app-name was specified
diff --git a/lib/private/installer.php b/lib/private/installer.php
index ed50503b3dc..3072cfd3b28 100644
--- a/lib/private/installer.php
+++ b/lib/private/installer.php
@@ -82,9 +82,10 @@ class OC_Installer{
 		$l = \OC::$server->getL10N('lib');
 
 		list($extractDir, $path) = self::downloadApp($data);
-		$info = self::checkAppsIntegrity($data, $extractDir, $path);
 
-		$basedir=OC_App::getInstallPath().'/'.$info['id'];
+		$info = self::checkAppsIntegrity($data, $extractDir, $path);
+		$appId = OC_App::cleanAppId($info['id']);
+		$basedir = OC_App::getInstallPath().'/'.$appId;
 		//check if the destination directory already exists
 		if(is_dir($basedir)) {
 			OC_Helper::rmdirr($extractDir);
@@ -163,6 +164,8 @@ class OC_Installer{
 	 * @brief Update an application
 	 * @param array $info
 	 * @param bool $isShipped
+	 * @throws Exception
+	 * @return bool
 	 *
 	 * This function could work like described below, but currently it disables and then
 	 * enables the app again. This does result in an updated app.
@@ -191,7 +194,7 @@ class OC_Installer{
 	 * upgrade.php can determine the current installed version of the app using
 	 * "\OC::$server->getAppConfig()->getValue($appid, 'installed_version')"
 	 */
-	public static function updateApp( $info=array(), $isShipped=false) {
+	public static function updateApp($info=array(), $isShipped=false) {
 		list($extractDir, $path) = self::downloadApp($info);
 		$info = self::checkAppsIntegrity($info, $extractDir, $path, $isShipped);
 
@@ -307,11 +310,12 @@ class OC_Installer{
 	 * check an app's integrity
 	 * @param array $data
 	 * @param string $extractDir
+	 * @param string $path
 	 * @param bool $isShipped
 	 * @return array
 	 * @throws \Exception
 	 */
-	public static function checkAppsIntegrity($data, $extractDir, $path, $isShipped=false) {
+	public static function checkAppsIntegrity($data, $extractDir, $path, $isShipped = false) {
 		$l = \OC::$server->getL10N('lib');
 		//load the info.xml file of the app
 		if(!is_file($extractDir.'/appinfo/info.xml')) {
@@ -329,12 +333,41 @@ class OC_Installer{
 		}
 		if(!is_file($extractDir.'/appinfo/info.xml')) {
 			OC_Helper::rmdirr($extractDir);
-			if($data['source']=='http') {
+			if($data['source'] === 'http') {
 				unlink($path);
 			}
 			throw new \Exception($l->t("App does not provide an info.xml file"));
 		}
-		$info=OC_App::getAppInfo($extractDir.'/appinfo/info.xml', true);
+
+		$info = OC_App::getAppInfo($extractDir.'/appinfo/info.xml', true);
+
+		// We can't trust the parsed info.xml file as it may have been tampered
+		// with by an attacker and thus we need to use the local data to check
+		// whether the application needs to be signed.
+		$appId = OC_App::cleanAppId($data['appdata']['id']);
+		$appBelongingToId = OC_App::getInternalAppIdByOcs($appId);
+		if(is_string($appBelongingToId)) {
+			$previouslySigned = \OC::$server->getConfig()->getAppValue($appBelongingToId, 'signed', 'false');
+		} else {
+			$appBelongingToId = $info['id'];
+			$previouslySigned = 'false';
+		}
+		if($data['appdata']['level'] === OC_App::officialApp || $previouslySigned === 'true') {
+			\OC::$server->getConfig()->setAppValue($appBelongingToId, 'signed', 'true');
+			$integrityResult = \OC::$server->getIntegrityCodeChecker()->verifyAppSignature(
+					$appBelongingToId,
+					$extractDir
+			);
+			if($integrityResult !== []) {
+				$e = new \Exception(
+						$l->t(
+								'Signature could not get checked. Please contact the app developer and check your admin screen.'
+						)
+				);
+				throw $e;
+			}
+		}
+
 		// check the code for not allowed calls
 		if(!$isShipped && !OC_Installer::checkCode($extractDir)) {
 			OC_Helper::rmdirr($extractDir);
diff --git a/lib/private/integritycheck/checker.php b/lib/private/integritycheck/checker.php
index edfe6b082e7..baffa9f735b 100644
--- a/lib/private/integritycheck/checker.php
+++ b/lib/private/integritycheck/checker.php
@@ -320,6 +320,14 @@ class Checker {
 		$this->cache->set(self::CACHE_KEY, json_encode($resultArray));
 	}
 
+	/**
+	 *
+	 * Clean previous results for a proper rescanning. Otherwise
+	 */
+	private function cleanResults() {
+		$this->config->deleteAppValue('core', self::CACHE_KEY);
+		$this->cache->remove(self::CACHE_KEY);
+	}
 
 	/**
 	 * Verify the signature of $appId. Returns an array with the following content:
@@ -350,11 +358,14 @@ class Checker {
 	 * Array may be empty in case no problems have been found.
 	 *
 	 * @param string $appId
+	 * @param string $path Optional path. If none is given it will be guessed.
 	 * @return array
 	 */
-	public function verifyAppSignature($appId) {
+	public function verifyAppSignature($appId, $path = '') {
 		try {
-			$path = $this->appLocator->getAppPath($appId);
+			if($path === '') {
+				$path = $this->appLocator->getAppPath($appId);
+			}
 			$result = $this->verify(
 					$path . '/appinfo/signature.json',
 					$path,
@@ -428,6 +439,7 @@ class Checker {
 	 * and store the results.
 	 */
 	public function runInstanceVerification() {
+		$this->cleanResults();
 		$this->verifyCoreSignature();
 		$appIds = $this->appLocator->getAllApps();
 		foreach($appIds as $appId) {
diff --git a/lib/private/ocsclient.php b/lib/private/ocsclient.php
index 81c9abee058..84845896e90 100644
--- a/lib/private/ocsclient.php
+++ b/lib/private/ocsclient.php
@@ -284,7 +284,7 @@ class OCSClient {
 		}
 
 		$app = [];
-		$app['id'] = (int)$tmp->id;
+		$app['id'] = (int)$id;
 		$app['name'] = (string)$tmp->name;
 		$app['version'] = (string)$tmp->version;
 		$app['type'] = (string)$tmp->typeid;