Merge pull request #4984 from nextcloud/tmpcertfile

Create a temp certfile

lib/private/Security/CertificateManager.php

@@ -31,6 +31,7 @@ use OC\Files\Filesystem;
 use OCP\ICertificateManager;
 use OCP\IConfig;
 use OCP\ILogger;
+use OCP\Security\ISecureRandom;
 
 /**
  * Manage trusted certificates for users
@@ -56,17 +57,26 @@ class CertificateManager implements ICertificateManager {
 	 */
 	protected $logger;
 
+	/** @var ISecureRandom */
+	protected $random;
+
 	/**
 	 * @param string $uid
 	 * @param \OC\Files\View $view relative to data/
 	 * @param IConfig $config
 	 * @param ILogger $logger
+	 * @param ISecureRandom $random
 	 */
-	public function __construct($uid, \OC\Files\View $view, IConfig $config, ILogger $logger) {
+	public function __construct($uid,
+								\OC\Files\View $view,
+								IConfig $config,
+								ILogger $logger,
+								ISecureRandom $random) {
 		$this->uid = $uid;
 		$this->view = $view;
 		$this->config = $config;
 		$this->logger = $logger;
+		$this->random = $random;
 	}
 
 	/**
@@ -120,7 +130,8 @@ class CertificateManager implements ICertificateManager {
 		}
 
 		$certPath = $path . 'rootcerts.crt';
-		$fhCerts = $this->view->fopen($certPath, 'w');
+		$tmpPath = $certPath . '.tmp' . $this->random->generate(10, ISecureRandom::CHAR_DIGITS);
+		$fhCerts = $this->view->fopen($tmpPath, 'w');
 
 		// Write user certificates
 		foreach ($certs as $cert) {
@@ -143,6 +154,8 @@ class CertificateManager implements ICertificateManager {
 		}
 
 		fclose($fhCerts);
+
+		$this->view->rename($tmpPath, $certPath);
 	}
 
 	/**
@@ -218,7 +231,7 @@ class CertificateManager implements ICertificateManager {
 		}
 		if ($this->needsRebundling($uid)) {
 			if (is_null($uid)) {
-				$manager = new CertificateManager(null, $this->view, $this->config, $this->logger);
+				$manager = new CertificateManager(null, $this->view, $this->config, $this->logger, $this->random);
 				$manager->createCertificateBundle();
 			} else {
 				$this->createCertificateBundle();

lib/private/Server.php

@@ -596,7 +596,13 @@ class Server extends ServerContainer implements IServerContainer {
 			$uid = $user ? $user : null;
 			return new ClientService(
 				$c->getConfig(),
-				new \OC\Security\CertificateManager($uid, new View(), $c->getConfig(), $c->getLogger())
+				new \OC\Security\CertificateManager(
+					$uid,
+					new View(),
+					$c->getConfig(),
+					$c->getLogger(),
+					$c->getSecureRandom()
+				)
 			);
 		});
 		$this->registerAlias('HttpClientService', \OCP\Http\Client\IClientService::class);
@@ -1438,7 +1444,13 @@ class Server extends ServerContainer implements IServerContainer {
 			}
 			$userId = $user->getUID();
 		}
-		return new CertificateManager($userId, new View(), $this->getConfig(), $this->getLogger());
+		return new CertificateManager(
+			$userId,
+			new View(),
+			$this->getConfig(),
+			$this->getLogger(),
+			$this->getSecureRandom()
+		);
 	}
 
 	/**

tests/lib/Security/CertificateManagerTest.php

@@ -12,6 +12,7 @@ use OC\Files\Storage\Temporary;
 use \OC\Security\CertificateManager;
 use OCP\IConfig;
 use OCP\ILogger;
+use OCP\Security\ISecureRandom;
 
 /**
  * Class CertificateManagerTest
@@ -26,6 +27,8 @@ class CertificateManagerTest extends \Test\TestCase {
 	private $certificateManager;
 	/** @var String */
 	private $username;
+	/** @var ISecureRandom */
+	private $random;
 
 	protected function setUp() {
 		parent::setUp();
@@ -45,7 +48,17 @@ 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->createMock(ILogger::class));
+		$this->random = $this->createMock(ISecureRandom::class);
+		$this->random->method('generate')
+			->willReturn('random');
+
+		$this->certificateManager = new CertificateManager(
+			$this->username,
+			new \OC\Files\View(),
+			$config,
+			$this->createMock(ILogger::class),
+			$this->random
+		);
 	}
 
 	protected function tearDown() {
@@ -145,7 +158,7 @@ class CertificateManagerTest extends \Test\TestCase {
 
 		/** @var CertificateManager | \PHPUnit_Framework_MockObject_MockObject $certificateManager */
 		$certificateManager = $this->getMockBuilder('OC\Security\CertificateManager')
-			->setConstructorArgs([$uid, $view, $config, $this->createMock(ILogger::class)])
+			->setConstructorArgs([$uid, $view, $config, $this->createMock(ILogger::class), $this->random])
 			->setMethods(['getFilemtimeOfCaBundle', 'getCertificateBundle'])
 			->getMock();