Merge pull request #2987 from nextcloud/no-empty-bundle

dont write a certificate bundle if the shipped ca bundle is empty

lib/private/Security/CertificateManager.php

@@ -30,6 +30,7 @@ namespace OC\Security;
 use OC\Files\Filesystem;
 use OCP\ICertificateManager;
 use OCP\IConfig;
+use OCP\ILogger;
 
 /**
  * Manage trusted certificates for users
@@ -50,15 +51,22 @@ class CertificateManager implements ICertificateManager {
 	 */
 	protected $config;
 
+	/**
+	 * @var ILogger
+	 */
+	protected $logger;
+
 	/**
 	 * @param string $uid
 	 * @param \OC\Files\View $view relative to data/
 	 * @param IConfig $config
+	 * @param ILogger $logger
 	 */
-	public function __construct($uid, \OC\Files\View $view, IConfig $config) {
+	public function __construct($uid, \OC\Files\View $view, IConfig $config, ILogger $logger) {
 		$this->uid = $uid;
 		$this->view = $view;
 		$this->config = $config;
+		$this->logger = $logger;
 	}
 
 	/**
@@ -104,6 +112,13 @@ class CertificateManager implements ICertificateManager {
 			$this->view->mkdir($path);
 		}
 
+		$defaultCertificates = file_get_contents(\OC::$SERVERROOT . '/resources/config/ca-bundle.crt');
+		if (strlen($defaultCertificates) < 1024) { // sanity check to verify that we have some content for our bundle
+			// log as exception so we have a stacktrace
+			$this->logger->logException(new \Exception('Shipped ca-bundle is empty, refusing to create certificate bundle'));
+			return;
+		}
+
 		$fhCerts = $this->view->fopen($path . '/rootcerts.crt', 'w');
 
 		// Write user certificates
@@ -117,7 +132,6 @@ class CertificateManager implements ICertificateManager {
 		}
 
 		// Append the default certificates
-		$defaultCertificates = file_get_contents(\OC::$SERVERROOT . '/resources/config/ca-bundle.crt');
 		fwrite($fhCerts, $defaultCertificates);
 
 		// Append the system certificate bundle
@@ -203,7 +217,7 @@ class CertificateManager implements ICertificateManager {
 		}
 		if ($this->needsRebundling($uid)) {
 			if (is_null($uid)) {
-				$manager = new CertificateManager(null, $this->view, $this->config);
+				$manager = new CertificateManager(null, $this->view, $this->config, $this->logger);
 				$manager->createCertificateBundle();
 			} else {
 				$this->createCertificateBundle();

lib/private/Server.php

@@ -482,7 +482,7 @@ class Server extends ServerContainer implements IServerContainer {
 			$uid = $user ? $user : null;
 			return new ClientService(
 				$c->getConfig(),
-				new \OC\Security\CertificateManager($uid, new View(), $c->getConfig())
+				new \OC\Security\CertificateManager($uid, new View(), $c->getConfig(), $c->getLogger())
 			);
 		});
 		$this->registerService('EventLogger', function (Server $c) {
@@ -1220,7 +1220,7 @@ class Server extends ServerContainer implements IServerContainer {
 			}
 			$userId = $user->getUID();
 		}
-		return new CertificateManager($userId, new View(), $this->getConfig());
+		return new CertificateManager($userId, new View(), $this->getConfig(), $this->getLogger());
 	}
 
 	/**

tests/lib/Security/CertificateManagerTest.php

@@ -8,8 +8,10 @@
 
 namespace Test\Security;
 
+use OC\Files\Storage\Temporary;
 use \OC\Security\CertificateManager;
 use OCP\IConfig;
+use OCP\ILogger;
 
 /**
  * Class CertificateManagerTest
@@ -43,7 +45,7 @@ class CertificateManagerTest extends \Test\TestCase {
 		$config->expects($this->any())->method('getSystemValue')
 			->with('installed', false)->willReturn(true);
 
-		$this->certificateManager = new CertificateManager($this->username, new \OC\Files\View(), $config);
+		$this->certificateManager = new CertificateManager($this->username, new \OC\Files\View(), $config, $this->createMock(ILogger::class));
 	}
 
 	protected function tearDown() {
@@ -143,7 +145,7 @@ class CertificateManagerTest extends \Test\TestCase {
 
 		/** @var CertificateManager | \PHPUnit_Framework_MockObject_MockObject $certificateManager */
 		$certificateManager = $this->getMockBuilder('OC\Security\CertificateManager')
-			->setConstructorArgs([$uid, $view, $config])
+			->setConstructorArgs([$uid, $view, $config, $this->createMock(ILogger::class)])
 			->setMethods(['getFilemtimeOfCaBundle', 'getCertificateBundle'])
 			->getMock();
 
@@ -210,5 +212,4 @@ class CertificateManagerTest extends \Test\TestCase {
 			[null, 10, 5, 8, false, true],
 		];
 	}
-
 }