Merge pull request #21591 from owncloud/add-code-checking-for-apps

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

4 files changed, 58 insertions, 13 deletions

diff --git a/lib/private/app.php b/lib/private/app.php
index 6c155136fe9..b4856f11860 100644
--- a/lib/private/app.php
+++ b/lib/private/app.php
@@ -313,14 +313,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']);
@@ -856,7 +856,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)) {
@@ -1042,7 +1042,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 46e022b4777..ef5efb09b46 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 770b64c6b9d..0cd01df7fe1 100644
--- a/lib/private/integritycheck/checker.php
+++ b/lib/private/integritycheck/checker.php
@@ -352,6 +352,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:
@@ -382,11 +390,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,
@@ -460,6 +471,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 0cb4ad5a698..a783a1f8425 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;