From fc2c5fe41420c2c43c6718a5a2e287afde5bdf49 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Tue, 23 Feb 2016 11:54:22 +0100 Subject: [PATCH 1/2] Add header for attachment disposition only once Recent refactorings have resulted in the header being added twice, this makes browsers ignore the header which removes any security gains. This changeset adds the header only once and adds integration tests ensuring the correct header in future. https://github.com/owncloud/core/issues/22577 --- apps/dav/lib/server.php | 13 ------ .../integration/features/bootstrap/WebDav.php | 44 +++++++++++++++++++ .../features/webdav-related.feature | 33 ++++++++++++-- 3 files changed, 73 insertions(+), 17 deletions(-) diff --git a/apps/dav/lib/server.php b/apps/dav/lib/server.php index 74be318fe5..377f76fd50 100644 --- a/apps/dav/lib/server.php +++ b/apps/dav/lib/server.php @@ -114,19 +114,6 @@ class Server { $this->server->addPlugin(new \OCA\DAV\Connector\Sabre\FakeLockerPlugin()); } - // Serve all files with an Content-Disposition of type "attachment" - $this->server->on('beforeMethod', function (RequestInterface $requestInterface, ResponseInterface $responseInterface) { - if ($requestInterface->getMethod() === 'GET') { - $path = $requestInterface->getPath(); - if ($this->server->tree->nodeExists($path)) { - $node = $this->server->tree->getNodeForPath($path); - if (($node instanceof IFile)) { - $responseInterface->addHeader('Content-Disposition', 'attachment'); - } - } - } - }); - // wait with registering these until auth is handled and the filesystem is setup $this->server->on('beforeMethod', function () { // custom properties plugin must be the last one diff --git a/build/integration/features/bootstrap/WebDav.php b/build/integration/features/bootstrap/WebDav.php index 58fdfed171..be87a09731 100644 --- a/build/integration/features/bootstrap/WebDav.php +++ b/build/integration/features/bootstrap/WebDav.php @@ -12,6 +12,8 @@ require __DIR__ . '/../../vendor/autoload.php'; trait WebDav { /** @var string*/ private $davPath = "remote.php/webdav"; + /** @var ResponseInterface */ + private $response; /** * @Given /^using dav path "([^"]*)"$/ @@ -104,6 +106,48 @@ trait WebDav { $this->downloadedContentShouldBe($content); } + /** + * @When Downloading file :fileName + */ + public function downloadingFile($fileName) { + $this->response = $this->makeDavRequest($this->currentUser, 'GET', $fileName, []); + } + + /** + * @Then The following headers should be set + */ + public function theFollowingHeadersShouldBeSet(\Behat\Gherkin\Node\TableNode $table) { + foreach($table->getTable() as $header) { + $headerName = $header[0]; + $expectedHeaderValue = $header[1]; + $returnedHeader = $this->response->getHeader($headerName); + if($returnedHeader !== $expectedHeaderValue) { + throw new \Exception( + sprintf( + "Expected value '%s' for header '%s', got '%s'", + $expectedHeaderValue, + $headerName, + $returnedHeader + ) + ); + } + } + } + + /** + * @Then Downloaded content should start with :start + */ + public function downloadedContentShouldStartWith($start) { + if(strpos($this->response->getBody()->getContents(), $start) !== 0) { + throw new \Exception( + sprintf( + "Expected '%s', got '%s'", + $start, + $this->response->getBody()->getContents() + ) + ); + } + } /*Returns the elements of a propfind, $folderDepth requires 1 to see elements without children*/ public function listFolder($user, $path, $folderDepth){ diff --git a/build/integration/features/webdav-related.feature b/build/integration/features/webdav-related.feature index 8be2c19630..c424f77afd 100644 --- a/build/integration/features/webdav-related.feature +++ b/build/integration/features/webdav-related.feature @@ -15,7 +15,6 @@ Feature: sharing When Downloading file "/welcome.txt" with range "bytes=51-77" Then Downloaded content should be "example file for developers" - Scenario: Upload forbidden if quota is 0 Given using dav path "remote.php/webdav" And As an "admin" @@ -33,9 +32,35 @@ Feature: sharing And Downloading last public shared file with range "bytes=51-77" Then Downloaded content should be "example file for developers" - - - + Scenario: Downloading a file on the old endpoint should serve security headers + Given using dav path "remote.php/webdav" + And As an "admin" + When Downloading file "/welcome.txt" + Then The following headers should be set + |Content-Disposition|attachment| + |Content-Security-Policy|default-src 'self'; script-src 'self' 'unsafe-eval'; style-src 'self' 'unsafe-inline'; frame-src *; img-src * data: blob:; font-src 'self' data:; media-src *; connect-src *| + |X-Content-Type-Options |nosniff| + |X-Download-Options|noopen| + |X-Frame-Options|Sameorigin| + |X-Permitted-Cross-Domain-Policies|none| + |X-Robots-Tag|none| + |X-XSS-Protection|1; mode=block| + And Downloaded content should start with "Welcome to your ownCloud account!" + + Scenario: Downloading a file on the new endpoint should serve security headers + Given using dav path "remote.php/dav/files/admin/" + And As an "admin" + When Downloading file "/welcome.txt" + Then The following headers should be set + |Content-Disposition|attachment| + |Content-Security-Policy|default-src 'self'; script-src 'self' 'unsafe-eval'; style-src 'self' 'unsafe-inline'; frame-src *; img-src * data: blob:; font-src 'self' data:; media-src *; connect-src *| + |X-Content-Type-Options |nosniff| + |X-Download-Options|noopen| + |X-Frame-Options|Sameorigin| + |X-Permitted-Cross-Domain-Policies|none| + |X-Robots-Tag|none| + |X-XSS-Protection|1; mode=block| + And Downloaded content should start with "Welcome to your ownCloud account!" From 403f11633c50b087fee8ffc7ce0dd9ac3e5f1263 Mon Sep 17 00:00:00 2001 From: Morris Jobke Date: Tue, 23 Feb 2016 14:53:12 +0100 Subject: [PATCH 2/2] Drop unneeded use statements --- apps/dav/lib/server.php | 4 ---- 1 file changed, 4 deletions(-) diff --git a/apps/dav/lib/server.php b/apps/dav/lib/server.php index 377f76fd50..55ae6c62d3 100644 --- a/apps/dav/lib/server.php +++ b/apps/dav/lib/server.php @@ -31,10 +31,6 @@ use OCA\DAV\Files\CustomPropertiesBackend; use OCP\IRequest; use OCP\SabrePluginEvent; use Sabre\DAV\Auth\Plugin; -use Sabre\DAV\IFile; -use Sabre\HTTP\RequestInterface; -use Sabre\HTTP\ResponseInterface; -use Sabre\HTTP\Util; class Server {