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.
This commit is contained in:
Lukas Reschke 2016-01-09 22:15:50 +01:00
parent 656b541899
commit c009d5dcc1
7 changed files with 267 additions and 15 deletions

View File

@ -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

View File

@ -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);

View File

@ -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) {

View File

@ -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;

View File

@ -38,6 +38,10 @@ class Test_Installer extends \Test\TestCase {
$data = array(
'path' => $tmp,
'source' => 'path',
'appdata' => [
'id' => 'Bar',
'level' => 100,
]
);
OC_Installer::installApp($data);
@ -57,6 +61,10 @@ class Test_Installer extends \Test\TestCase {
$oldData = array(
'path' => $oldTmp,
'source' => 'path',
'appdata' => [
'id' => 'Bar',
'level' => 100,
]
);
$pathOfNewTestApp = __DIR__;
@ -69,6 +77,10 @@ class Test_Installer extends \Test\TestCase {
$newData = array(
'path' => $newTmp,
'source' => 'path',
'appdata' => [
'id' => 'Bar',
'level' => 100,
]
);
OC_Installer::installApp($oldData);

View File

@ -23,6 +23,7 @@ namespace Test\IntegrityCheck;
use OC\IntegrityCheck\Checker;
use OC\Memcache\NullCache;
use OCP\ICache;
use phpseclib\Crypt\RSA;
use phpseclib\File\X509;
use Test\TestCase;
@ -253,6 +254,59 @@ class CheckerTest extends TestCase {
$this->assertSame($expected, $this->checker->verifyAppSignature('SomeApp'));
}
public function testVerifyAppSignatureWithTamperedFilesAndAlternatePath() {
$this->appLocator
->expects($this->never())
->method('getAppPath')
->with('SomeApp');
$signatureDataFile = '{
"hashes": {
"AnotherFile.txt": "1570ca9420e37629de4328f48c51da29840ddeaa03ae733da4bf1d854b8364f594aac560601270f9e1797ed4cd57c1aea87bf44cf4245295c94f2e935a2f0112",
"subfolder\/file.txt": "410738545fb623c0a5c8a71f561e48ea69e3ada0981a455e920a5ae9bf17c6831ae654df324f9328ff8453de179276ae51931cca0fa71fe8ccde6c083ca0574b"
},
"signature": "dYoohBaWIFR\/To1FXEbMQB5apUhVYlEauBGSPo12nq84wxWkBx2EM3KDRgkB5Sub2tr0CgmAc2EVjPhKIEzAam26cyUb48bJziz1V6wvW7z4GZAfaJpzLkyHdSfV5117VSf5w1rDcAeZDXfGUaaNEJPWytaF4ZIxVge7f3NGshHy4odFVPADy\/u6c43BWvaOtJ4m3aJQbP6sxCO9dxwcm5yJJJR3n36jfh229sdWBxyl8BhwhH1e1DEv78\/aiL6ckKFPVNzx01R6yDFt3TgEMR97YZ\/R6lWiXG+dsJ305jNFlusLu518zBUvl7g5yjzGN778H29b2C8VLZKmi\/h1CH9jGdD72fCqCYdenD2uZKzb6dsUtXtvBmVcVT6BUGz41W1pkkEEB+YJpMrHILIxAiHRGv1+aZa9\/Oz8LWFd+BEUQjC2LJgojPnpzaG\/msw1nBkX16NNVDWWtJ25Bc\/r\/mG46rwjWB\/cmV6Lwt6KODiqlxgrC4lm9ALOCEWw+23OcYhLwNfQTYevXqHqsFfXOkhUnM8z5vDUb\/HBraB1DjFXN8iLK+1YewD4P495e+SRzrR79Oi3F8SEqRIzRLfN2rnW1BTms\/wYsz0p67cup1Slk1XlNmHwbWX25NVd2PPlLOvZRGoqcKFpIjC5few8THiZfyjiNFwt3RM0AFdZcXY=",
"certificate": "-----BEGIN CERTIFICATE-----\r\nMIIEvjCCAqagAwIBAgIUc\/0FxYrsgSs9rDxp03EJmbjN0NwwDQYJKoZIhvcNAQEF\r\nBQAwIzEhMB8GA1UECgwYb3duQ2xvdWQgQ29kZSBTaWduaW5nIENBMB4XDTE1MTEw\r\nMzIxMDMzM1oXDTE2MTEwMzIxMDMzM1owDzENMAsGA1UEAwwEY29yZTCCAiIwDQYJ\r\nKoZIhvcNAQEBBQADggIPADCCAgoCggIBALb6EgHpkAqZbO5vRO8XSh7G7XGWHw5s\r\niOf4RwPXR6SE9bWZEm\/b72SfWk\/\/J6AbrD8WiOzBuT\/ODy6k5T1arEdHO+Pux0W1\r\nMxYJJI4kH74KKgMpC0SB0Rt+8WrMqV1r3hhJ46df6Xr\/xolP3oD+eLbShPcblhdS\r\nVtkZEkoev8Sh6L2wDCeHDyPxzvj1w2dTdGVO9Kztn0xIlyfEBakqvBWtcxyi3Ln0\r\nklnxlMx3tPDUE4kqvpia9qNiB1AN2PV93eNr5\/2riAzIssMFSCarWCx0AKYb54+d\r\nxLpcYFyqPJ0ydBCkF78DD45RCZet6PNYkdzgbqlUWEGGomkuDoJbBg4wzgzO0D77\r\nH87KFhYW8tKFFvF1V3AHl\/sFQ9tDHaxM9Y0pZ2jPp\/ccdiqnmdkBxBDqsiRvHvVB\r\nCn6qpb4vWGFC7vHOBfYspmEL1zLlKXZv3ezMZEZw7O9ZvUP3VO\/wAtd2vUW8UFiq\r\ns2v1QnNLN6jNh51obcwmrBvWhJy9vQIdtIjQbDxqWTHh1zUSrw9wrlklCBZ\/zrM0\r\ni8nfCFwTxWRxp3H9KoECzO\/zS5R5KIS7s3\/wq\/w9T2Ie4rcecgXwDizwnn0C\/aKc\r\nbDIjujpL1s9HO05pcD\/V3wKcPZ1izymBkmMyIbL52iRVN5FTVHeZdXPpFuq+CTQJ\r\nQ238lC+A\/KOVAgMBAAEwDQYJKoZIhvcNAQEFBQADggIBAGoKTnh8RfJV4sQItVC2\r\nAvfJagkrIqZ3iiQTUBQGTKBsTnAqE1H7QgUSV9vSd+8rgvHkyZsRjmtyR1e3A6Ji\r\noNCXUbExC\/0iCPUqdHZIVb+Lc\/vWuv4ByFMybGPydgtLoEUX2ZrKFWmcgZFDUSRd\r\n9Uj26vtUhCC4bU4jgu6hIrR9IuxOBLQUxGTRZyAcXvj7obqRAEZwFAKQgFpfpqTb\r\nH+kjcbZSaAlLVSF7vBc1syyI8RGYbqpwvtREqJtl5IEIwe6huEqJ3zPnlP2th\/55\r\ncf3Fovj6JJgbb9XFxrdnsOsDOu\/tpnaRWlvv5ib4+SzG5wWFT5UUEo4Wg2STQiiX\r\nuVSRQxK1LE1yg84bs3NZk9FSQh4B8vZVuRr5FaJsZZkwlFlhRO\/\/+TJtXRbyNgsf\r\noMRZGi8DLGU2SGEAHcRH\/QZHq\/XDUWVzdxrSBYcy7GSpT7UDVzGv1rEJUrn5veP1\r\n0KmauAqtiIaYRm4f6YBsn0INcZxzIPZ0p8qFtVZBPeHhvQtvOt0iXI\/XUxEWOa2F\r\nK2EqhErgMK\/N07U1JJJay5tYZRtvkGq46oP\/5kQG8hYST0MDK6VihJoPpvCmAm4E\r\npEYKQ96x6A4EH9Y9mZlYozH\/eqmxPbTK8n89\/p7Ydun4rI+B2iiLnY8REWWy6+UQ\r\nV204fGUkJqW5CrKy3P3XvY9X\r\n-----END CERTIFICATE-----"
}';
$this->fileAccessHelper
->expects($this->at(0))
->method('file_get_contents')
->with(
\OC::$SERVERROOT . '/tests/data/integritycheck/appWithInvalidData//appinfo/signature.json'
)
->will($this->returnValue($signatureDataFile));
$this->fileAccessHelper
->expects($this->at(1))
->method('file_get_contents')
->with(
'/resources/codesigning/root.crt'
)
->will($this->returnValue(file_get_contents(__DIR__ .'/../../data/integritycheck/root.crt')));
$expected = [
'INVALID_HASH' => [
'AnotherFile.txt' => [
'expected' => '1570ca9420e37629de4328f48c51da29840ddeaa03ae733da4bf1d854b8364f594aac560601270f9e1797ed4cd57c1aea87bf44cf4245295c94f2e935a2f0112',
'current' => '7322348ba269c6d5522efe02f424fa3a0da319a7cd9c33142a5afe32a2d9af2da3a411f086fcfc96ff4301ea566f481dba0960c2abeef3594c4d930462f6584c',
],
],
'FILE_MISSING' => [
'subfolder/file.txt' => [
'expected' => '410738545fb623c0a5c8a71f561e48ea69e3ada0981a455e920a5ae9bf17c6831ae654df324f9328ff8453de179276ae51931cca0fa71fe8ccde6c083ca0574b',
'current' => '',
],
],
'EXTRA_FILE' => [
'UnecessaryFile' => [
'expected' => '',
'current' => 'cf83e1357eefb8bdf1542850d66d8007d620e4050b5715dc83f4a921d36ce9ce47d0d13c5d85f2b0ff8318d2877eec2f63b931bd47417a81a538327af927da3e',
],
],
];
$this->assertSame($expected, $this->checker->verifyAppSignature('SomeApp', \OC::$SERVERROOT . '/tests/data/integritycheck/appWithInvalidData/'));
}
public function testVerifyAppWithDifferentScope() {
$this->appLocator
->expects($this->once())
@ -664,6 +718,10 @@ class CheckerTest extends TestCase {
->expects($this->at(3))
->method('verifyAppSignature')
->with('dav');
$this->config
->expects($this->once())
->method('deleteAppValue')
->with('core', 'oc.integritycheck.checker');
$this->checker->runInstanceVerification();
}

View File

@ -748,7 +748,7 @@ class OCSClientTest extends \Test\TestCase {
->expects($this->once())
->method('get')
->with(
'https://api.owncloud.com/v1/content/data/MyId',
'https://api.owncloud.com/v1/content/data/166053',
[
'timeout' => 5,
'query' => ['version' => '8x1x0x7'],
@ -779,8 +779,145 @@ class OCSClientTest extends \Test\TestCase {
'score' => 50,
'level' => 200,
];
$this->assertSame($expected, $this->ocsClient->getApplication('MyId', [8, 1, 0, 7]));
$this->assertSame($expected, $this->ocsClient->getApplication(166053, [8, 1, 0, 7]));
}
public function testGetApplicationSuccessfulWithOldId() {
$this->config
->expects($this->at(0))
->method('getSystemValue')
->with('appstoreenabled', true)
->will($this->returnValue(true));
$this->config
->expects($this->at(1))
->method('getSystemValue')
->with('appstoreurl', 'https://api.owncloud.com/v1')
->will($this->returnValue('https://api.owncloud.com/v1'));
$response = $this->getMock('\OCP\Http\Client\IResponse');
$response
->expects($this->once())
->method('getBody')
->will($this->returnValue('<?xml version="1.0"?>
<ocs>
<meta>
<status>ok</status>
<statuscode>100</statuscode>
<message></message>
</meta>
<data>
<content details="full">
<id>1337</id>
<name>Versioning</name>
<version>0.0.1</version>
<label>recommended</label>
<typeid>925</typeid>
<typename>ownCloud other</typename>
<language></language>
<personid>owncloud</personid>
<profilepage>http://opendesktop.org/usermanager/search.php?username=owncloud</profilepage>
<created>2014-07-07T16:34:40+02:00</created>
<changed>2014-07-07T16:34:40+02:00</changed>
<downloads>140</downloads>
<score>50</score>
<description>Placeholder for future updates</description>
<summary></summary>
<feedbackurl></feedbackurl>
<changelog></changelog>
<homepage></homepage>
<homepagetype></homepagetype>
<homepage2></homepage2>
<homepagetype2></homepagetype2>
<homepage3></homepage3>
<homepagetype3></homepagetype3>
<homepage4></homepage4>
<homepagetype4></homepagetype4>
<homepage5></homepage5>
<homepagetype5></homepagetype5>
<homepage6></homepage6>
<homepagetype6></homepagetype6>
<homepage7></homepage7>
<homepagetype7></homepagetype7>
<homepage8></homepage8>
<homepagetype8></homepagetype8>
<homepage9></homepage9>
<homepagetype9></homepagetype9>
<homepage10></homepage10>
<homepagetype10></homepagetype10>
<licensetype>16</licensetype>
<license>AGPL</license>
<donationpage></donationpage>
<comments>0</comments>
<commentspage>http://apps.owncloud.com/content/show.php?content=166053</commentspage>
<fans>0</fans>
<fanspage>http://apps.owncloud.com/content/show.php?action=fan&amp;content=166053</fanspage>
<knowledgebaseentries>0</knowledgebaseentries>
<knowledgebasepage>http://apps.owncloud.com/content/show.php?action=knowledgebase&amp;content=166053</knowledgebasepage>
<depend>ownCloud 7</depend>
<preview1></preview1>
<preview2></preview2>
<preview3></preview3>
<previewpic1></previewpic1>
<previewpic2></previewpic2>
<previewpic3></previewpic3>
<picsmall1></picsmall1>
<picsmall2></picsmall2>
<picsmall3></picsmall3>
<detailpage>https://apps.owncloud.com/content/show.php?content=166053</detailpage>
<downloadtype1></downloadtype1>
<downloadprice1>0</downloadprice1>
<downloadlink1>http://apps.owncloud.com/content/download.php?content=166053&amp;id=1</downloadlink1>
<downloadname1></downloadname1>
<downloadgpgfingerprint1></downloadgpgfingerprint1>
<downloadgpgsignature1></downloadgpgsignature1>
<downloadpackagename1></downloadpackagename1>
<downloadrepository1></downloadrepository1>
<downloadsize1>1</downloadsize1>
<approved>200</approved>
</content>
</data>
</ocs>
'));
$client = $this->getMock('\OCP\Http\Client\IClient');
$client
->expects($this->once())
->method('get')
->with(
'https://api.owncloud.com/v1/content/data/166053',
[
'timeout' => 5,
'query' => ['version' => '8x1x0x7'],
]
)
->will($this->returnValue($response));
$this->clientService
->expects($this->once())
->method('newClient')
->will($this->returnValue($client));
$expected = [
'id' => 166053,
'name' => 'Versioning',
'version' => '0.0.1',
'type' => '925',
'label' => 'recommended',
'typename' => 'ownCloud other',
'personid' => 'owncloud',
'profilepage' => 'http://opendesktop.org/usermanager/search.php?username=owncloud',
'detailpage' => 'https://apps.owncloud.com/content/show.php?content=166053',
'preview1' => '',
'preview2' => '',
'preview3' => '',
'changed' => 1404743680,
'description' => 'Placeholder for future updates',
'score' => 50,
'level' => 200,
];
$this->assertSame($expected, $this->ocsClient->getApplication(166053, [8, 1, 0, 7]));
}
public function testGetApplicationEmptyXml() {
$this->config
->expects($this->at(0))