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.

diff --git a/lib/private/integritycheck/checker.php b/lib/private/integritycheck/checker.php
index edfe6b082e7db73446a5e971d6cd1e06835429ac..8748c3983887588c45ce3b53bedf1fee0f77b455 100644 (file)
--- a/lib/private/integritycheck/checker.php
+++ b/lib/private/integritycheck/checker.php
@@ -81,6 +81,34 @@ class Checker {
                $this->appManager = $appManager;
        }
 
+       /**
+        * 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.
         *
@@ -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 d7747dbb966405e6975dcb2c5b6f0e734bafe078..7cfebdea46df754ef5b02b67cf2c3a0f77324e7c 100644 (file)
--- 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 9ec72bab2f94203d4c2881a981c1b1535b36df06..f2a24976e9a80d62347b79070713ec8e9bb45d01 100644 (file)
--- 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 17117a4274b43b494afdd85ba479d2c270af4042..ed7ca4bceb897a9ac8216c2fb4379334c5cc4013 100644 (file)
--- 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',
@@ -360,6 +430,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')
@@ -391,6 +471,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')
@@ -422,6 +512,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')
@@ -459,6 +559,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')
@@ -511,6 +621,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')
@@ -548,6 +668,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')
@@ -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 4a292188e7538eab2ebd1a803f42dbca45c918eb..a1d1f671b070def9c8b04697487d05aaa2595ab1 100644 (file)
--- 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());
        }
 }