Merge pull request #26822 from owncloud/fix-23591

Report failures for SignApp and SignCore

core/Command/Integrity/SignApp.php

@@ -100,8 +100,13 @@ class SignApp extends Command {
 		$x509 = new X509();
 		$x509->loadX509($keyBundle);
 		$x509->setPrivateKey($rsa);
-		$this->checker->writeAppSignature($path, $x509, $rsa);
-
-		$output->writeln('Successfully signed "'.$path.'"');
+		try {
+			$this->checker->writeAppSignature($path, $x509, $rsa);
+			$output->writeln('Successfully signed "'.$path.'"');
+		} catch (\Exception $e){
+			$output->writeln('Error: ' . $e->getMessage());
+			return 1;
+		}
+		return 0;
 	}
 }

core/Command/Integrity/SignCore.php

@@ -22,12 +22,10 @@
 namespace OC\Core\Command\Integrity;
 
 use OC\IntegrityCheck\Checker;
-use OC\IntegrityCheck\Helpers\EnvironmentHelper;
 use OC\IntegrityCheck\Helpers\FileAccessHelper;
 use phpseclib\Crypt\RSA;
 use phpseclib\File\X509;
 use Symfony\Component\Console\Command\Command;
-use OCP\IConfig;
 use Symfony\Component\Console\Input\InputInterface;
 use Symfony\Component\Console\Input\InputOption;
 use Symfony\Component\Console\Output\OutputInterface;
@@ -93,8 +91,14 @@ class SignCore extends Command {
 		$x509 = new X509();
 		$x509->loadX509($keyBundle);
 		$x509->setPrivateKey($rsa);
-		$this->checker->writeCoreSignature($x509, $rsa, $path);
 
-		$output->writeln('Successfully signed "core"');
+		try {
+			$this->checker->writeCoreSignature($x509, $rsa, $path);
+			$output->writeln('Successfully signed "core"');
+		} catch (\Exception $e){
+			$output->writeln('Error: ' . $e->getMessage());
+			return 1;
+		}
+		return 0;
 	}
 }

lib/private/IntegrityCheck/Checker.php

@@ -266,16 +266,24 @@ class Checker {
 	public function writeAppSignature($path,
 									  X509 $certificate,
 									  RSA $privateKey) {
-		if(!is_dir($path)) {
-			throw new \Exception('Directory does not exist.');
-		}
+		$appInfoDir = $path . '/appinfo';
+		$this->fileAccessHelper->assertDirectoryExists($path);
+		$this->fileAccessHelper->assertDirectoryExists($appInfoDir);
+
 		$iterator = $this->getFolderIterator($path);
 		$hashes = $this->generateHashes($iterator, $path);
 		$signature = $this->createSignatureData($hashes, $certificate, $privateKey);
-		$this->fileAccessHelper->file_put_contents(
-				$path . '/appinfo/signature.json',
+		try {
+			$this->fileAccessHelper->file_put_contents(
+				$appInfoDir . '/signature.json',
 				json_encode($signature, JSON_PRETTY_PRINT)
-		);
+			);
+		} catch (\Exception $e){
+			if (!$this->fileAccessHelper->is_writeable($appInfoDir)){
+				throw new \Exception($appInfoDir . ' is not writable');
+			}
+			throw $e;
+		}
 	}
 
 	/**
@@ -284,17 +292,29 @@ class Checker {
 	 * @param X509 $certificate
 	 * @param RSA $rsa
 	 * @param string $path
+	 * @throws \Exception
 	 */
 	public function writeCoreSignature(X509 $certificate,
 									   RSA $rsa,
 									   $path) {
+		$coreDir = $path . '/core';
+		$this->fileAccessHelper->assertDirectoryExists($path);
+		$this->fileAccessHelper->assertDirectoryExists($coreDir);
+
 		$iterator = $this->getFolderIterator($path, $path);
 		$hashes = $this->generateHashes($iterator, $path);
 		$signatureData = $this->createSignatureData($hashes, $certificate, $rsa);
-		$this->fileAccessHelper->file_put_contents(
-				$path . '/core/signature.json',
+		try {
+			$this->fileAccessHelper->file_put_contents(
+				$coreDir . '/signature.json',
 				json_encode($signatureData, JSON_PRETTY_PRINT)
-		);
+			);
+		} catch (\Exception $e){
+			if (!$this->fileAccessHelper->is_writeable($coreDir)){
+				throw new \Exception($coreDir . ' is not writable');
+			}
+			throw $e;
+		}
 	}
 
 	/**

lib/private/IntegrityCheck/Helpers/FileAccessHelper.php

@@ -52,10 +52,33 @@ class FileAccessHelper {
 	 * Wrapper around file_put_contents($filename, $data)
 	 *
 	 * @param string $filename
-	 * @param $data
-	 * @return int|false
+	 * @param string $data
+	 * @return int
+	 * @throws \Exception
 	 */
 	public function file_put_contents($filename, $data) {
-		return file_put_contents($filename, $data);
+		$bytesWritten = file_put_contents($filename, $data);
+		if ($bytesWritten === false || $bytesWritten !== strlen($data)){
+			throw new \Exception('Failed to write into ' . $filename);
+		}
+		return $bytesWritten;
+	}
+
+	/**
+	 * @param string $path
+	 * @return bool
+	 */
+	public function is_writeable($path){
+		return is_writeable($path);
+	}
+
+	/**
+	 * @param string $path
+	 * @throws \Exception
+	 */
+	public function assertDirectoryExists($path){
+		if (!is_dir($path)) {
+			throw new \Exception('Directory ' . $path . ' does not exist.');
+		}
 	}
 }

tests/lib/IntegrityCheck/CheckerTest.php

@@ -77,7 +77,6 @@ class CheckerTest extends TestCase {
 
 	/**
 	 * @expectedException \Exception
-	 * @expectedExceptionMessage Directory does not exist.
 	 */
 	public function testWriteAppSignatureOfNotExistingApp() {
 		$keyBundle = file_get_contents(__DIR__ .'/../../data/integritycheck/SomeApp.crt');
@@ -89,6 +88,24 @@ class CheckerTest extends TestCase {
 		$this->checker->writeAppSignature('NotExistingApp', $x509, $rsa);
 	}
 
+	/**
+	 * @expectedException \Exception
+	 */
+	public function testWriteAppSignatureWrongPermissions(){
+		$this->fileAccessHelper
+			->expects($this->once())
+			->method('file_put_contents')
+			->will($this->throwException(new \Exception))
+		;
+		$keyBundle = file_get_contents(__DIR__ .'/../../data/integritycheck/SomeApp.crt');
+		$rsaPrivateKey = file_get_contents(__DIR__ .'/../../data/integritycheck/SomeApp.key');
+		$rsa = new RSA();
+		$rsa->loadKey($rsaPrivateKey);
+		$x509 = new X509();
+		$x509->loadX509($keyBundle);
+		$this->checker->writeAppSignature(\OC::$SERVERROOT . '/tests/data/integritycheck/app/', $x509, $rsa);
+	}
+
 	public function testWriteAppSignature() {
 		$expectedSignatureFileData = '{
     "hashes": {