Merge pull request #27097 from owncloud/26655_ComputeChecksum2

Server side checksum verification #26655
owncloud · Mar 10, 2017 · c9f5fce · c9f5fce
2 parents a651f4f + 0496a82
commit c9f5fce
Show file tree

Hide file tree

Showing 11 changed files with 674 additions and 140 deletions.
diff --git a/apps/dav/lib/Connector/Sabre/File.php b/apps/dav/lib/Connector/Sabre/File.php
@@ -34,6 +34,7 @@
 namespace OCA\DAV\Connector\Sabre;
 
 use OC\Files\Filesystem;
+use OC\Files\Storage\Storage;
 use OCA\DAV\Connector\Sabre\Exception\EntityTooLarge;
 use OCA\DAV\Connector\Sabre\Exception\FileLocked;
 use OCA\DAV\Connector\Sabre\Exception\Forbidden as DAVForbiddenException;
@@ -134,6 +135,10 @@ public function put($data) {
 			list($count, $result) = \OC_Helper::streamCopy($data, $target);
 			fclose($target);
 
+			if (!self::isChecksumValid($partStorage, $internalPartPath)) {
+				throw new BadRequest('The computed checksum does not match the one received from the client.');
+			}
+
 			if ($result === false) {
 				$expected = -1;
 				if (isset($_SERVER['CONTENT_LENGTH'])) {
@@ -218,15 +223,17 @@ public function put($data) {
 
 			$this->refreshInfo();
 
-			if (isset($request->server['HTTP_OC_CHECKSUM'])) {
-				$checksum = trim($request->server['HTTP_OC_CHECKSUM']);
-				$this->fileView->putFileInfo($this->path, ['checksum' => $checksum]);
-				$this->refreshInfo();
-			} else if ($this->getChecksum() !== null && $this->getChecksum() !== '') {
-				$this->fileView->putFileInfo($this->path, ['checksum' => '']);
-				$this->refreshInfo();
+			$meta = $partStorage->getMetaData($internalPartPath);
+
+			if (isset($meta['checksum'])) {
+				$this->fileView->putFileInfo(
+					$this->path,
+					['checksum' => $meta['checksum']]
+				);
 			}
 
+			$this->refreshInfo();
+
 		} catch (StorageNotAvailableException $e) {
 			throw new ServiceUnavailable("Failed to check file size: " . $e->getMessage());
 		}
@@ -441,6 +448,10 @@ private function createFileChunked($data) {
 
 					$chunk_handler->file_assemble($partStorage, $partInternalPath);
 
+					if (!self::isChecksumValid($partStorage, $partInternalPath)) {
+						throw new BadRequest('The computed checksum does not match the one received from the client.');
+					}
+
 					// here is the final atomic rename
 					$renameOkay = $targetStorage->moveFromStorage($partStorage, $partInternalPath, $targetInternalPath);
 					$fileExists = $targetStorage->file_exists($targetInternalPath);
@@ -479,13 +490,20 @@ private function createFileChunked($data) {
 				// FIXME: should call refreshInfo but can't because $this->path is not the of the final file
 				$info = $this->fileView->getFileInfo($targetPath);
 
-				if (isset($request->server['HTTP_OC_CHECKSUM'])) {
-					$checksum = trim($request->server['HTTP_OC_CHECKSUM']);
-					$this->fileView->putFileInfo($targetPath, ['checksum' => $checksum]);
-				} else if ($info->getChecksum() !== null && $info->getChecksum() !== '') {
-					$this->fileView->putFileInfo($this->path, ['checksum' => '']);
+
+				if (isset($partStorage) && isset($partInternalPath)) {
+					$checksums = $partStorage->getMetaData($partInternalPath)['checksum'];
+				} else {
+					$checksums = $targetStorage->getMetaData($targetInternalPath)['checksum'];
 				}
 
+				$this->fileView->putFileInfo(
+					$targetPath,
+					['checksum' => $checksums]
+				);
+
+				$this->refreshInfo();
+
 				$this->fileView->unlockFile($targetPath, ILockingProvider::LOCK_SHARED);
 
 				return $info->getEtag();
@@ -500,6 +518,29 @@ private function createFileChunked($data) {
 		return null;
 	}
 
+	/**
+	 * will return true if checksum was not provided in request
+	 *
+	 * @param Storage $storage
+	 * @param $path
+	 * @return bool
+	 */
+	private static function isChecksumValid(Storage $storage, $path) {
+		$meta = $storage->getMetaData($path);
+		$request = \OC::$server->getRequest();
+
+		if (!isset($request->server['HTTP_OC_CHECKSUM']) || !isset($meta['checksum'])) {
+			// No comparison possible, skip the check
+			return true;
+		}
+
+		$expectedChecksum = trim($request->server['HTTP_OC_CHECKSUM']);
+		$computedChecksums = $meta['checksum'];
+
+		return strpos($computedChecksums, $expectedChecksum) !== false;
+
+	}
+
 	/**
 	 * Returns whether a part file is needed for the given storage
 	 * or whether the file can be assembled/uploaded directly on the
@@ -563,12 +604,30 @@ private function convertToSabreException(\Exception $e) {
 		throw new \Sabre\DAV\Exception($e->getMessage(), 0, $e);
 	}
 
+
 	/**
-	 * Get the checksum for this file
-	 *
+	 * Set $algo to get a specific checksum, leave null to get all checksums
+	 * (space seperated)
+	 * @param null $algo
 	 * @return string
 	 */
-	public function getChecksum() {
-		return $this->info->getChecksum();
+	public function getChecksum($algo = null) {
+		$allChecksums = $this->info->getChecksum();
+
+		if (!$algo) {
+			return $allChecksums;
+		}
+
+		$checksums = explode(' ', $allChecksums);
+		$algoPrefix = strtoupper($algo) . ':';
+
+		foreach ($checksums as $checksum) {
+			// starts with $algoPrefix
+			if (substr($checksum, 0, strlen($algoPrefix)) === $algoPrefix) {
+				return $checksum;
+			}
+		}
+
+		return '';
 	}
 }
diff --git a/apps/dav/lib/Connector/Sabre/FilesPlugin.php b/apps/dav/lib/Connector/Sabre/FilesPlugin.php
@@ -252,7 +252,7 @@ function httpGet(RequestInterface $request, ResponseInterface $response) {
 		if ($node instanceof \OCA\DAV\Connector\Sabre\File) {
 			//Add OC-Checksum header
 			/** @var $node File */
-			$checksum = $node->getChecksum();
+			$checksum = $node->getChecksum('sha1');
 			if ($checksum !== null && $checksum !== '') {
 				$response->addHeader('OC-Checksum', $checksum);
 			}
@@ -340,7 +340,7 @@ public function handleGetProperties(PropFind $propFind, \Sabre\DAV\INode $node)
 			});
 
 			$propFind->handle(self::CHECKSUMS_PROPERTYNAME, function() use ($node) {
-				$checksum = $node->getChecksum();
+				$checksum = $node->getChecksum('sha1');
 				if ($checksum === NULL || $checksum === '') {
 					return null;
 				}

diff --git a/apps/files/lib/Capabilities.php b/apps/files/lib/Capabilities.php
@@ -52,6 +52,10 @@ public function __construct(IConfig $config) {
 	 */
 	public function getCapabilities() {
 		return [
+			'checksums' => [
+				'supportedTypes' => ['SHA1'],
+				'preferredUploadType' => 'SHA1'
+			],
 			'files' => [
 				'bigfilechunking' => true,
 				'blacklisted_files' => $this->config->getSystemValue('blacklisted_files', ['.htaccess']),

diff --git a/apps/files_trashbin/tests/TrashbinTest.php b/apps/files_trashbin/tests/TrashbinTest.php
@@ -146,50 +146,8 @@ protected function tearDown() {
 		parent::tearDown();
 	}
 
-	/**
-	 * test expiration of files older then the max storage time defined for the trash
-	 */
-	public function testExpireOldFiles() {
-
-		$currentTime = time();
-		$expireAt = $currentTime - 2 * 24 * 60 * 60;
-		$expiredDate = $currentTime - 3 * 24 * 60 * 60;
-
-		// create some files
-		\OC\Files\Filesystem::file_put_contents('file1.txt', 'file1');
-		\OC\Files\Filesystem::file_put_contents('file2.txt', 'file2');
-		\OC\Files\Filesystem::file_put_contents('file3.txt', 'file3');
-
-		// delete them so that they end up in the trash bin
-		\OC\Files\Filesystem::unlink('file1.txt');
-		\OC\Files\Filesystem::unlink('file2.txt');
-		\OC\Files\Filesystem::unlink('file3.txt');
-
-		//make sure that files are in the trash bin
-		$filesInTrash = OCA\Files_Trashbin\Helper::getTrashFiles('/', self::TEST_TRASHBIN_USER1, 'name');
-		$this->assertSame(3, count($filesInTrash));
 
-		// every second file will get a date in the past so that it will get expired
-		$manipulatedList = $this->manipulateDeleteTime($filesInTrash, $this->trashRoot1, $expiredDate);
-
-		$testClass = new TrashbinForTesting();
-		list($sizeOfDeletedFiles, $count) = $testClass->dummyDeleteExpiredFiles($manipulatedList, $expireAt);
-
-		$this->assertSame(10, $sizeOfDeletedFiles);
-		$this->assertSame(2, $count);
-
-		// only file2.txt should be left
-		$remainingFiles = array_slice($manipulatedList, $count);
-		$this->assertSame(1, count($remainingFiles));
-		$remainingFile = reset($remainingFiles);
-		$this->assertSame('file2.txt', $remainingFile['name']);
 
-		// check that file1.txt and file3.txt was really deleted
-		$newTrashContent = OCA\Files_Trashbin\Helper::getTrashFiles('/', self::TEST_TRASHBIN_USER1);
-		$this->assertSame(1, count($newTrashContent));
-		$element = reset($newTrashContent);
-		$this->assertSame('file2.txt', $element['name']);
-	}
 
 	/**
 	 * test expiration of files older then the max storage time defined for the trash
@@ -267,6 +225,52 @@ public function testExpireOldFilesShared() {
 		$this->verifyArray($filesInTrashUser1AfterDelete, ['user1-2.txt', 'user1-4.txt']);
 	}
 
+	/**
+	 * test expiration of files older then the max storage time defined for the trash
+	 */
+	public function testExpireOldFiles() {
+
+		$currentTime = time();
+		$expireAt = $currentTime - 2 * 24 * 60 * 60;
+		$expiredDate = $currentTime - 3 * 24 * 60 * 60;
+
+		// create some files
+		\OC\Files\Filesystem::file_put_contents('file1.txt', 'file1');
+		\OC\Files\Filesystem::file_put_contents('file2.txt', 'file2');
+		\OC\Files\Filesystem::file_put_contents('file3.txt', 'file3');
+
+		// delete them so that they end up in the trash bin
+		\OC\Files\Filesystem::unlink('file1.txt');
+		\OC\Files\Filesystem::unlink('file2.txt');
+		\OC\Files\Filesystem::unlink('file3.txt');
+
+		//make sure that files are in the trash bin
+		$filesInTrash = OCA\Files_Trashbin\Helper::getTrashFiles('/', self::TEST_TRASHBIN_USER1, 'name');
+		$this->assertSame(3, count($filesInTrash));
+
+		// every second file will get a date in the past so that it will get expired
+		$manipulatedList = $this->manipulateDeleteTime($filesInTrash, $this->trashRoot1, $expiredDate);
+
+		$testClass = new TrashbinForTesting();
+		list($sizeOfDeletedFiles, $count) = $testClass->dummyDeleteExpiredFiles($manipulatedList, $expireAt);
+
+		$this->assertSame(10, $sizeOfDeletedFiles);
+		$this->assertSame(2, $count);
+
+		// only file2.txt should be left
+		$remainingFiles = array_slice($manipulatedList, $count);
+		$this->assertSame(1, count($remainingFiles));
+		$remainingFile = reset($remainingFiles);
+		$this->assertSame('file2.txt', $remainingFile['name']);
+
+		// check that file1.txt and file3.txt was really deleted
+		$newTrashContent = OCA\Files_Trashbin\Helper::getTrashFiles('/', self::TEST_TRASHBIN_USER1);
+		$this->assertSame(1, count($newTrashContent));
+		$element = reset($newTrashContent);
+		$this->assertSame('file2.txt', $element['name']);
+	}
+
+
 	/**
 	 * verify that the array contains the expected results
 	 *

diff --git a/apps/files_versions/tests/VersioningTest.php b/apps/files_versions/tests/VersioningTest.php
@@ -117,6 +117,57 @@ protected function tearDown() {
 		parent::tearDown();
 	}
 
+
+	public function testMoveFileIntoSharedFolderAsRecipient() {
+
+		\OC\Files\Filesystem::mkdir('folder1');
+		$fileInfo = \OC\Files\Filesystem::getFileInfo('folder1');
+
+		$node = \OC::$server->getUserFolder(self::TEST_VERSIONS_USER)->get('folder1');
+		$share = \OC::$server->getShareManager()->newShare();
+		$share->setNode($node)
+			->setShareType(\OCP\Share::SHARE_TYPE_USER)
+			->setSharedBy(self::TEST_VERSIONS_USER)
+			->setSharedWith(self::TEST_VERSIONS_USER2)
+			->setPermissions(\OCP\Constants::PERMISSION_ALL);
+		$share = \OC::$server->getShareManager()->createShare($share);
+
+		self::loginHelper(self::TEST_VERSIONS_USER2);
+		$versionsFolder2 = '/' . self::TEST_VERSIONS_USER2 . '/files_versions';
+		\OC\Files\Filesystem::file_put_contents('test.txt', 'test file');
+
+		$t1 = time();
+		// second version is two weeks older, this way we make sure that no
+		// version will be expired
+		$t2 = $t1 - 60 * 60 * 24 * 14;
+
+		$this->rootView->mkdir($versionsFolder2);
+		// create some versions
+		$v1 = $versionsFolder2 . '/test.txt.v' . $t1;
+		$v2 = $versionsFolder2 . '/test.txt.v' . $t2;
+
+		$this->rootView->file_put_contents($v1, 'version1');
+		$this->rootView->file_put_contents($v2, 'version2');
+
+		// move file into the shared folder as recipient
+		\OC\Files\Filesystem::rename('/test.txt', '/folder1/test.txt');
+
+		$this->assertFalse($this->rootView->file_exists($v1));
+		$this->assertFalse($this->rootView->file_exists($v2));
+
+		self::loginHelper(self::TEST_VERSIONS_USER);
+
+		$versionsFolder1 = '/' . self::TEST_VERSIONS_USER . '/files_versions';
+
+		$v1Renamed = $versionsFolder1 . '/folder1/test.txt.v' . $t1;
+		$v2Renamed = $versionsFolder1 . '/folder1/test.txt.v' . $t2;
+
+		$this->assertTrue($this->rootView->file_exists($v1Renamed));
+		$this->assertTrue($this->rootView->file_exists($v2Renamed));
+
+		\OC::$server->getShareManager()->deleteShare($share);
+	}
+
 	/**
 	 * @medium
 	 * test expire logic
@@ -378,56 +429,6 @@ public function testMoveFolder() {
 	}
 
 
-	public function testMoveFileIntoSharedFolderAsRecipient() {
-
-		\OC\Files\Filesystem::mkdir('folder1');
-		$fileInfo = \OC\Files\Filesystem::getFileInfo('folder1');
-
-		$node = \OC::$server->getUserFolder(self::TEST_VERSIONS_USER)->get('folder1');
-		$share = \OC::$server->getShareManager()->newShare();
-		$share->setNode($node)
-			->setShareType(\OCP\Share::SHARE_TYPE_USER)
-			->setSharedBy(self::TEST_VERSIONS_USER)
-			->setSharedWith(self::TEST_VERSIONS_USER2)
-			->setPermissions(\OCP\Constants::PERMISSION_ALL);
-		$share = \OC::$server->getShareManager()->createShare($share);
-
-		self::loginHelper(self::TEST_VERSIONS_USER2);
-		$versionsFolder2 = '/' . self::TEST_VERSIONS_USER2 . '/files_versions';
-		\OC\Files\Filesystem::file_put_contents('test.txt', 'test file');
-
-		$t1 = time();
-		// second version is two weeks older, this way we make sure that no
-		// version will be expired
-		$t2 = $t1 - 60 * 60 * 24 * 14;
-
-		$this->rootView->mkdir($versionsFolder2);
-		// create some versions
-		$v1 = $versionsFolder2 . '/test.txt.v' . $t1;
-		$v2 = $versionsFolder2 . '/test.txt.v' . $t2;
-
-		$this->rootView->file_put_contents($v1, 'version1');
-		$this->rootView->file_put_contents($v2, 'version2');
-
-		// move file into the shared folder as recipient
-		\OC\Files\Filesystem::rename('/test.txt', '/folder1/test.txt');
-
-		$this->assertFalse($this->rootView->file_exists($v1));
-		$this->assertFalse($this->rootView->file_exists($v2));
-
-		self::loginHelper(self::TEST_VERSIONS_USER);
-
-		$versionsFolder1 = '/' . self::TEST_VERSIONS_USER . '/files_versions';
-
-		$v1Renamed = $versionsFolder1 . '/folder1/test.txt.v' . $t1;
-		$v2Renamed = $versionsFolder1 . '/folder1/test.txt.v' . $t2;
-
-		$this->assertTrue($this->rootView->file_exists($v1Renamed));
-		$this->assertTrue($this->rootView->file_exists($v2Renamed));
-
-		\OC::$server->getShareManager()->deleteShare($share);
-	}
-
 	public function testMoveFolderIntoSharedFolderAsRecipient() {
 
 		\OC\Files\Filesystem::mkdir('folder1');