Merge pull request #21668 from owncloud/add-super-evil-and-hidden-config-switch-to-disable-integrity-checks-really-just-do-not-use-this

Add hidden config switch to disable code integrity checking
2024-07-24 13:24:51 +02:00 · 2016-01-13 10:30:02 +01:00 · 2016-01-13 10:30:02 +01:00 · 3fd976995a
commit 3fd976995a
parent 4129480eb0 08e73d2c8f
5 changed files with 248 additions and 4 deletions
--- 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.');
--- 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();
+	}
 }
--- 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');
--- 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);
+	}
 }
--- 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());
 	}
 }