diff options
author | Lukas Reschke <lukas@owncloud.com> | 2016-01-12 18:41:11 +0100 |
---|---|---|
committer | Lukas Reschke <lukas@owncloud.com> | 2016-01-12 18:48:36 +0100 |
commit | 08e73d2c8f3c9cbed39a5b632907f8570d23440a (patch) | |
tree | a8746ac0139d24dec71846551ef88b14960767f9 | |
parent | eac5d9fb3a52932fafdb200a2cf5d50fe9f1c759 (diff) | |
download | nextcloud-server-08e73d2c8f3c9cbed39a5b632907f8570d23440a.tar.gz nextcloud-server-08e73d2c8f3c9cbed39a5b632907f8570d23440a.zip |
Add hidden config switch to disable code integrity checking
This adds a hidden config flag that allows somebody to disable the code integrity check. If `integrity.check.disabled` is set to `true` in the config file:
1. The integrity check functions will return always an empty result
2. The integrity check is not performed when installing apps
3. The integrity check is not performed when updating apps
4. The integrity check is not performed when updating the core
Furthermore this adds support for a list of channels that the code checker will run on. At the moment this is only stable because I didn't want to break any build scripts that we have. Once we have a proper CA setup and updated the build process to sign the releases we can add the RC, alpha, beta as well as daily releases. So everything except "git" basically.
-rw-r--r-- | lib/private/integritycheck/checker.php | 32 | ||||
-rw-r--r-- | lib/private/integritycheck/helpers/environmenthelper.php | 9 | ||||
-rw-r--r-- | lib/private/updater.php | 4 | ||||
-rw-r--r-- | tests/lib/integritycheck/checkertest.php | 192 | ||||
-rw-r--r-- | tests/lib/integritycheck/helpers/EnvironmentHelperTest.php | 15 |
5 files changed, 248 insertions, 4 deletions
diff --git a/lib/private/integritycheck/checker.php b/lib/private/integritycheck/checker.php index edfe6b082e7..8748c398388 100644 --- a/lib/private/integritycheck/checker.php +++ b/lib/private/integritycheck/checker.php @@ -82,6 +82,34 @@ class Checker { } /** + * Whether code signing is enforced or not. + * + * @return bool + */ + public function isCodeCheckEnforced() { + // FIXME: Once the signing server is instructed to sign daily, beta and + // RCs as well these need to be included also. + $signedChannels = [ + 'stable', + ]; + if(!in_array($this->environmentHelper->getChannel(), $signedChannels, true)) { + return false; + } + + /** + * This config option is undocumented and supposed to be so, it's only + * applicable for very specific scenarios and we should not advertise it + * too prominent. So please do not add it to config.sample.php. + */ + $isIntegrityCheckDisabled = $this->config->getSystemValue('integrity.check.disabled', false); + if($isIntegrityCheckDisabled === true) { + return false; + } + + return true; + } + + /** * Enumerates all files belonging to the folder. Sensible defaults are excluded. * * @param string $folderToIterate @@ -209,6 +237,10 @@ class Checker { * @throws \Exception */ private function verify($signaturePath, $basePath, $certificateCN) { + if(!$this->isCodeCheckEnforced()) { + return []; + } + $signatureData = json_decode($this->fileAccessHelper->file_get_contents($signaturePath), true); if(!is_array($signatureData)) { throw new InvalidSignatureException('Signature data not found.'); diff --git a/lib/private/integritycheck/helpers/environmenthelper.php b/lib/private/integritycheck/helpers/environmenthelper.php index d7747dbb966..7cfebdea46d 100644 --- a/lib/private/integritycheck/helpers/environmenthelper.php +++ b/lib/private/integritycheck/helpers/environmenthelper.php @@ -36,4 +36,13 @@ class EnvironmentHelper { public function getServerRoot() { return \OC::$SERVERROOT; } + + /** + * Provides \OC_Util::getChannel() + * + * @return string + */ + public function getChannel() { + return \OC_Util::getChannel(); + } } diff --git a/lib/private/updater.php b/lib/private/updater.php index 9ec72bab2f9..f2a24976e9a 100644 --- a/lib/private/updater.php +++ b/lib/private/updater.php @@ -345,8 +345,8 @@ class Updater extends BasicEmitter { //Invalidate update feed $this->config->setAppValue('core', 'lastupdatedat', 0); - // Check for code integrity on the stable channel - if(\OC_Util::getChannel() === 'stable') { + // Check for code integrity if not disabled + if(\OC::$server->getIntegrityCodeChecker()->isCodeCheckEnforced()) { $this->emit('\OC\Updater', 'startCheckCodeIntegrity'); $this->checker->runInstanceVerification(); $this->emit('\OC\Updater', 'finishedCheckCodeIntegrity'); diff --git a/tests/lib/integritycheck/checkertest.php b/tests/lib/integritycheck/checkertest.php index 17117a4274b..ed7ca4bceb8 100644 --- a/tests/lib/integritycheck/checkertest.php +++ b/tests/lib/integritycheck/checkertest.php @@ -120,6 +120,16 @@ class CheckerTest extends TestCase { } public function testVerifyAppSignatureWithoutSignatureData() { + $this->environmentHelper + ->expects($this->once()) + ->method('getChannel') + ->will($this->returnValue('stable')); + $this->config + ->expects($this->any()) + ->method('getSystemValue') + ->with('integrity.check.disabled', false) + ->will($this->returnValue(false)); + $expected = [ 'EXCEPTION' => [ 'class' => 'OC\IntegrityCheck\Exceptions\InvalidSignatureException', @@ -130,6 +140,16 @@ class CheckerTest extends TestCase { } public function testVerifyAppSignatureWithValidSignatureData() { + $this->environmentHelper + ->expects($this->once()) + ->method('getChannel') + ->will($this->returnValue('stable')); + $this->config + ->expects($this->any()) + ->method('getSystemValue') + ->with('integrity.check.disabled', false) + ->will($this->returnValue(false)); + $this->appLocator ->expects($this->once()) ->method('getAppPath') @@ -162,6 +182,16 @@ class CheckerTest extends TestCase { } public function testVerifyAppSignatureWithTamperedSignatureData() { + $this->environmentHelper + ->expects($this->once()) + ->method('getChannel') + ->will($this->returnValue('stable')); + $this->config + ->expects($this->any()) + ->method('getSystemValue') + ->with('integrity.check.disabled', false) + ->will($this->returnValue(false)); + $this->appLocator ->expects($this->once()) ->method('getAppPath') @@ -200,6 +230,16 @@ class CheckerTest extends TestCase { } public function testVerifyAppSignatureWithTamperedFiles() { + $this->environmentHelper + ->expects($this->once()) + ->method('getChannel') + ->will($this->returnValue('stable')); + $this->config + ->expects($this->any()) + ->method('getSystemValue') + ->with('integrity.check.disabled', false) + ->will($this->returnValue(false)); + $this->appLocator ->expects($this->once()) ->method('getAppPath') @@ -254,6 +294,16 @@ class CheckerTest extends TestCase { } public function testVerifyAppWithDifferentScope() { + $this->environmentHelper + ->expects($this->once()) + ->method('getChannel') + ->will($this->returnValue('stable')); + $this->config + ->expects($this->any()) + ->method('getSystemValue') + ->with('integrity.check.disabled', false) + ->will($this->returnValue(false)); + $this->appLocator ->expects($this->once()) ->method('getAppPath') @@ -290,6 +340,16 @@ class CheckerTest extends TestCase { } public function testVerifyAppWithDifferentScopeAndAlwaysTrustedCore() { + $this->environmentHelper + ->expects($this->once()) + ->method('getChannel') + ->will($this->returnValue('stable')); + $this->config + ->expects($this->any()) + ->method('getSystemValue') + ->with('integrity.check.disabled', false) + ->will($this->returnValue(false)); + $this->appLocator ->expects($this->once()) ->method('getAppPath') @@ -350,6 +410,16 @@ class CheckerTest extends TestCase { } public function testVerifyCoreSignatureWithoutSignatureData() { + $this->environmentHelper + ->expects($this->once()) + ->method('getChannel') + ->will($this->returnValue('stable')); + $this->config + ->expects($this->any()) + ->method('getSystemValue') + ->with('integrity.check.disabled', false) + ->will($this->returnValue(false)); + $expected = [ 'EXCEPTION' => [ 'class' => 'OC\\IntegrityCheck\\Exceptions\\InvalidSignatureException', @@ -361,6 +431,16 @@ class CheckerTest extends TestCase { public function testVerifyCoreSignatureWithValidSignatureData() { $this->environmentHelper + ->expects($this->once()) + ->method('getChannel') + ->will($this->returnValue('stable')); + $this->config + ->expects($this->any()) + ->method('getSystemValue') + ->with('integrity.check.disabled', false) + ->will($this->returnValue(false)); + + $this->environmentHelper ->expects($this->any()) ->method('getServerRoot') ->will($this->returnValue(\OC::$SERVERROOT . '/tests/data/integritycheck/app/')); @@ -392,6 +472,16 @@ class CheckerTest extends TestCase { public function testVerifyCoreSignatureWithValidSignatureDataAndNotAlphabeticOrder() { $this->environmentHelper + ->expects($this->once()) + ->method('getChannel') + ->will($this->returnValue('stable')); + $this->config + ->expects($this->any()) + ->method('getSystemValue') + ->with('integrity.check.disabled', false) + ->will($this->returnValue(false)); + + $this->environmentHelper ->expects($this->any()) ->method('getServerRoot') ->will($this->returnValue(\OC::$SERVERROOT . '/tests/data/integritycheck/app/')); @@ -423,6 +513,16 @@ class CheckerTest extends TestCase { public function testVerifyCoreSignatureWithTamperedSignatureData() { $this->environmentHelper + ->expects($this->once()) + ->method('getChannel') + ->will($this->returnValue('stable')); + $this->config + ->expects($this->any()) + ->method('getSystemValue') + ->with('integrity.check.disabled', false) + ->will($this->returnValue(false)); + + $this->environmentHelper ->expects($this->any()) ->method('getServerRoot') ->will($this->returnValue(\OC::$SERVERROOT . '/tests/data/integritycheck/appWithInvalidData/')); @@ -460,6 +560,16 @@ class CheckerTest extends TestCase { public function testVerifyCoreSignatureWithTamperedFiles() { $this->environmentHelper + ->expects($this->once()) + ->method('getChannel') + ->will($this->returnValue('stable')); + $this->config + ->expects($this->any()) + ->method('getSystemValue') + ->with('integrity.check.disabled', false) + ->will($this->returnValue(false)); + + $this->environmentHelper ->expects($this->any()) ->method('getServerRoot') ->will($this->returnValue(\OC::$SERVERROOT . '/tests/data/integritycheck/appWithInvalidData/')); @@ -512,6 +622,16 @@ class CheckerTest extends TestCase { public function testVerifyCoreWithInvalidCertificate() { $this->environmentHelper + ->expects($this->once()) + ->method('getChannel') + ->will($this->returnValue('stable')); + $this->config + ->expects($this->any()) + ->method('getSystemValue') + ->with('integrity.check.disabled', false) + ->will($this->returnValue(false)); + + $this->environmentHelper ->expects($this->any()) ->method('getServerRoot') ->will($this->returnValue(\OC::$SERVERROOT . '/tests/data/integritycheck/app/')); @@ -549,6 +669,16 @@ class CheckerTest extends TestCase { public function testVerifyCoreWithDifferentScope() { $this->environmentHelper + ->expects($this->once()) + ->method('getChannel') + ->will($this->returnValue('stable')); + $this->config + ->expects($this->any()) + ->method('getSystemValue') + ->with('integrity.check.disabled', false) + ->will($this->returnValue(false)); + + $this->environmentHelper ->expects($this->any()) ->method('getServerRoot') ->will($this->returnValue(\OC::$SERVERROOT . '/tests/data/integritycheck/app/')); @@ -668,4 +798,66 @@ class CheckerTest extends TestCase { $this->checker->runInstanceVerification(); } + public function testVerifyAppSignatureWithoutSignatureDataAndCodeCheckerDisabled() { + $this->environmentHelper + ->expects($this->once()) + ->method('getChannel') + ->will($this->returnValue('stable')); + $this->config + ->expects($this->any()) + ->method('getSystemValue') + ->with('integrity.check.disabled', false) + ->will($this->returnValue(true)); + + $expected = []; + $this->assertSame($expected, $this->checker->verifyAppSignature('SomeApp')); + } + + /** + * @return array + */ + public function channelDataProvider() { + return [ + ['stable', true], + ['git', false], + ]; + } + + /** + * @param string $channel + * @param bool $isCodeSigningEnforced + * @dataProvider channelDataProvider + */ + public function testIsCodeCheckEnforced($channel, $isCodeSigningEnforced) { + $this->environmentHelper + ->expects($this->once()) + ->method('getChannel') + ->will($this->returnValue($channel)); + $this->config + ->expects($this->any()) + ->method('getSystemValue') + ->with('integrity.check.disabled', false) + ->will($this->returnValue(false)); + + $this->assertSame($isCodeSigningEnforced, $this->checker->isCodeCheckEnforced()); + } + + /** + * @param string $channel + * @dataProvider channelDataProvider + */ + public function testIsCodeCheckEnforcedWithDisabledConfigSwitch($channel) { + $this->environmentHelper + ->expects($this->once()) + ->method('getChannel') + ->will($this->returnValue($channel)); + $this->config + ->expects($this->any()) + ->method('getSystemValue') + ->with('integrity.check.disabled', false) + ->will($this->returnValue(true)); + + $result = $this->invokePrivate($this->checker, 'isCodeCheckEnforced'); + $this->assertSame(false, $result); + } } diff --git a/tests/lib/integritycheck/helpers/EnvironmentHelperTest.php b/tests/lib/integritycheck/helpers/EnvironmentHelperTest.php index 4a292188e75..a1d1f671b07 100644 --- a/tests/lib/integritycheck/helpers/EnvironmentHelperTest.php +++ b/tests/lib/integritycheck/helpers/EnvironmentHelperTest.php @@ -25,8 +25,19 @@ use OC\IntegrityCheck\Helpers\EnvironmentHelper; use Test\TestCase; class EnvironmentHelperTest extends TestCase { + /** @var EnvironmentHelper */ + private $environmentHelper; + + public function setUp() { + $this->environmentHelper = new EnvironmentHelper(); + return parent::setUp(); + } + public function testGetServerRoot() { - $factory = new EnvironmentHelper(); - $this->assertSame(\OC::$SERVERROOT, $factory->getServerRoot()); + $this->assertSame(\OC::$SERVERROOT, $this->environmentHelper->getServerRoot()); + } + + public function testGetChannel() { + $this->assertSame(\OC_Util::getChannel(), $this->environmentHelper->getChannel()); } } |