Add Same Site Cookie protection

2 files changed, 343 insertions, 30 deletions

diff --git a/tests/lib/appframework/http/RequestTest.php b/tests/lib/appframework/http/RequestTest.php
index 3f1d09c2a93..787f410e0c4 100644
--- a/tests/lib/appframework/http/RequestTest.php
+++ b/tests/lib/appframework/http/RequestTest.php
@@ -1,15 +1,16 @@
 <?php
 /**
  * @copyright 2013 Thomas Tanghus (thomas@tanghus.net)
- * @copyright 2015 Lukas Reschke lukas@owncloud.com
+ * @copyright 2016 Lukas Reschke lukas@owncloud.com
  *
  * This file is licensed under the Affero General Public License version 3 or
  * later.
  * See the COPYING-README file.
  */
 
-namespace OC\AppFramework\Http;
+namespace Test\AppFramework\Http;
 
+use OC\AppFramework\Http\Request;
 use OC\Security\CSRF\CsrfToken;
 use OC\Security\CSRF\CsrfTokenManager;
 use OCP\Security\ISecureRandom;
@@ -740,15 +741,15 @@ class RequestTest extends \Test\TestCase {
 	 */
 	public function testUserAgent($testAgent, $userAgent, $matches) {
 		$request = new Request(
-				[
-						'server' => [
-								'HTTP_USER_AGENT' => $testAgent,
-						]
-				],
-				$this->secureRandom,
-				$this->config,
-				$this->csrfTokenManager,
-				$this->stream
+			[
+				'server' => [
+					'HTTP_USER_AGENT' => $testAgent,
+				]
+			],
+			$this->secureRandom,
+			$this->config,
+			$this->csrfTokenManager,
+			$this->stream
 		);
 
 		$this->assertSame($matches, $request->isUserAgent($userAgent));
@@ -762,11 +763,11 @@ class RequestTest extends \Test\TestCase {
 	 */
 	public function testUndefinedUserAgent($testAgent, $userAgent, $matches) {
 		$request = new Request(
-				[],
-				$this->secureRandom,
-				$this->config,
-				$this->csrfTokenManager,
-				$this->stream
+			[],
+			$this->secureRandom,
+			$this->config,
+			$this->csrfTokenManager,
+			$this->stream
 		);
 
 		$this->assertFalse($request->isUserAgent($userAgent));
@@ -1322,6 +1323,10 @@ class RequestTest extends \Test\TestCase {
 					'get' => [
 						'requesttoken' => 'AAAHGxsTCTc3BgMQESAcNR0OAR0=:MyTotalSecretShareds',
 					],
+					'cookies' => [
+						'nc_sameSiteCookiestrict' => 'true',
+						'nc_sameSiteCookielax' => 'true',
+					],
 				],
 				$this->secureRandom,
 				$this->config,
@@ -1348,6 +1353,10 @@ class RequestTest extends \Test\TestCase {
 					'post' => [
 						'requesttoken' => 'AAAHGxsTCTc3BgMQESAcNR0OAR0=:MyTotalSecretShareds',
 					],
+					'cookies' => [
+						'nc_sameSiteCookiestrict' => 'true',
+						'nc_sameSiteCookielax' => 'true',
+					],
 				],
 				$this->secureRandom,
 				$this->config,
@@ -1357,10 +1366,10 @@ class RequestTest extends \Test\TestCase {
 			->getMock();
 		$token = new CsrfToken('AAAHGxsTCTc3BgMQESAcNR0OAR0=:MyTotalSecretShareds');
 		$this->csrfTokenManager
-				->expects($this->once())
-				->method('isTokenValid')
-				->with($token)
-				->willReturn(true);
+			->expects($this->once())
+			->method('isTokenValid')
+			->with($token)
+			->willReturn(true);
 
 		$this->assertTrue($request->passesCSRFCheck());
 	}
@@ -1374,6 +1383,10 @@ class RequestTest extends \Test\TestCase {
 					'server' => [
 						'HTTP_REQUESTTOKEN' => 'AAAHGxsTCTc3BgMQESAcNR0OAR0=:MyTotalSecretShareds',
 					],
+					'cookies' => [
+						'nc_sameSiteCookiestrict' => 'true',
+						'nc_sameSiteCookielax' => 'true',
+					],
 				],
 				$this->secureRandom,
 				$this->config,
@@ -1383,14 +1396,225 @@ class RequestTest extends \Test\TestCase {
 			->getMock();
 		$token = new CsrfToken('AAAHGxsTCTc3BgMQESAcNR0OAR0=:MyTotalSecretShareds');
 		$this->csrfTokenManager
-				->expects($this->once())
-				->method('isTokenValid')
-				->with($token)
-				->willReturn(true);
+			->expects($this->once())
+			->method('isTokenValid')
+			->with($token)
+			->willReturn(true);
 
 		$this->assertTrue($request->passesCSRFCheck());
 	}
 
+	public function testFailsCSRFCheckWithGetAndWithoutCookies() {
+		/** @var Request $request */
+		$request = $this->getMockBuilder('\OC\AppFramework\Http\Request')
+			->setMethods(['getScriptName'])
+			->setConstructorArgs([
+				[
+					'get' => [
+						'requesttoken' => 'AAAHGxsTCTc3BgMQESAcNR0OAR0=:MyTotalSecretShareds',
+					],
+				],
+				$this->secureRandom,
+				$this->config,
+				$this->csrfTokenManager,
+				$this->stream
+			])
+			->getMock();
+		$this->csrfTokenManager
+			->expects($this->never())
+			->method('isTokenValid');
+
+		$this->assertFalse($request->passesCSRFCheck());
+	}
+
+	public function testFailsCSRFCheckWithPostAndWithoutCookies() {
+		/** @var Request $request */
+		$request = $this->getMockBuilder('\OC\AppFramework\Http\Request')
+			->setMethods(['getScriptName'])
+			->setConstructorArgs([
+				[
+					'post' => [
+						'requesttoken' => 'AAAHGxsTCTc3BgMQESAcNR0OAR0=:MyTotalSecretShareds',
+					],
+				],
+				$this->secureRandom,
+				$this->config,
+				$this->csrfTokenManager,
+				$this->stream
+			])
+			->getMock();
+		$this->csrfTokenManager
+			->expects($this->never())
+			->method('isTokenValid');
+
+		$this->assertFalse($request->passesCSRFCheck());
+	}
+
+	public function testFailsCSRFCheckWithHeaderAndWithoutCookies() {
+		/** @var Request $request */
+		$request = $this->getMockBuilder('\OC\AppFramework\Http\Request')
+			->setMethods(['getScriptName'])
+			->setConstructorArgs([
+				[
+					'server' => [
+						'HTTP_REQUESTTOKEN' => 'AAAHGxsTCTc3BgMQESAcNR0OAR0=:MyTotalSecretShareds',
+					],
+				],
+				$this->secureRandom,
+				$this->config,
+				$this->csrfTokenManager,
+				$this->stream
+			])
+			->getMock();
+		$this->csrfTokenManager
+			->expects($this->never())
+			->method('isTokenValid');
+
+		$this->assertFalse($request->passesCSRFCheck());
+	}
+
+	public function testFailsCSRFCheckWithHeaderAndNotAllChecksPassing() {
+		/** @var Request $request */
+		$request = $this->getMockBuilder('\OC\AppFramework\Http\Request')
+			->setMethods(['getScriptName'])
+			->setConstructorArgs([
+				[
+					'server' => [
+						'HTTP_REQUESTTOKEN' => 'AAAHGxsTCTc3BgMQESAcNR0OAR0=:MyTotalSecretShareds',
+					],
+					'cookies' => [
+						'nc_sameSiteCookiestrict' => 'true',
+					],
+				],
+				$this->secureRandom,
+				$this->config,
+				$this->csrfTokenManager,
+				$this->stream
+			])
+			->getMock();
+		$this->csrfTokenManager
+			->expects($this->never())
+			->method('isTokenValid');
+
+		$this->assertFalse($request->passesCSRFCheck());
+	}
+
+	public function testPassesStrictCookieCheckWithAllCookies() {
+		/** @var Request $request */
+		$request = $this->getMockBuilder('\OC\AppFramework\Http\Request')
+			->setMethods(['getScriptName'])
+			->setConstructorArgs([
+				[
+					'server' => [
+						'HTTP_REQUESTTOKEN' => 'AAAHGxsTCTc3BgMQESAcNR0OAR0=:MyTotalSecretShareds',
+					],
+					'cookies' => [
+						'nc_sameSiteCookiestrict' => 'true',
+						'nc_sameSiteCookielax' => 'true',
+					],
+				],
+				$this->secureRandom,
+				$this->config,
+				$this->csrfTokenManager,
+				$this->stream
+			])
+			->getMock();
+
+		$this->assertTrue($request->passesStrictCookieCheck());
+	}
+
+	public function testFailStrictCookieCheckWithOnlyLaxCookie() {
+		/** @var Request $request */
+		$request = $this->getMockBuilder('\OC\AppFramework\Http\Request')
+			->setMethods(['getScriptName'])
+			->setConstructorArgs([
+				[
+					'server' => [
+						'HTTP_REQUESTTOKEN' => 'AAAHGxsTCTc3BgMQESAcNR0OAR0=:MyTotalSecretShareds',
+					],
+					'cookies' => [
+						'nc_sameSiteCookielax' => 'true',
+					],
+				],
+				$this->secureRandom,
+				$this->config,
+				$this->csrfTokenManager,
+				$this->stream
+			])
+			->getMock();
+
+		$this->assertFalse($request->passesStrictCookieCheck());
+	}
+
+	public function testFailStrictCookieCheckWithOnlyStrictCookie() {
+		/** @var Request $request */
+		$request = $this->getMockBuilder('\OC\AppFramework\Http\Request')
+			->setMethods(['getScriptName'])
+			->setConstructorArgs([
+				[
+					'server' => [
+						'HTTP_REQUESTTOKEN' => 'AAAHGxsTCTc3BgMQESAcNR0OAR0=:MyTotalSecretShareds',
+					],
+					'cookies' => [
+						'nc_sameSiteCookiestrict' => 'true',
+					],
+				],
+				$this->secureRandom,
+				$this->config,
+				$this->csrfTokenManager,
+				$this->stream
+			])
+			->getMock();
+
+		$this->assertFalse($request->passesStrictCookieCheck());
+	}
+
+	public function testPassesLaxCookieCheck() {
+		/** @var Request $request */
+		$request = $this->getMockBuilder('\OC\AppFramework\Http\Request')
+			->setMethods(['getScriptName'])
+			->setConstructorArgs([
+				[
+					'server' => [
+						'HTTP_REQUESTTOKEN' => 'AAAHGxsTCTc3BgMQESAcNR0OAR0=:MyTotalSecretShareds',
+					],
+					'cookies' => [
+						'nc_sameSiteCookielax' => 'true',
+					],
+				],
+				$this->secureRandom,
+				$this->config,
+				$this->csrfTokenManager,
+				$this->stream
+			])
+			->getMock();
+
+		$this->assertTrue($request->passesLaxCookieCheck());
+	}
+
+	public function testFailsLaxCookieCheckWithOnlyStrictCookie() {
+		/** @var Request $request */
+		$request = $this->getMockBuilder('\OC\AppFramework\Http\Request')
+			->setMethods(['getScriptName'])
+			->setConstructorArgs([
+				[
+					'server' => [
+						'HTTP_REQUESTTOKEN' => 'AAAHGxsTCTc3BgMQESAcNR0OAR0=:MyTotalSecretShareds',
+					],
+					'cookies' => [
+						'nc_sameSiteCookiestrict' => 'true',
+					],
+				],
+				$this->secureRandom,
+				$this->config,
+				$this->csrfTokenManager,
+				$this->stream
+			])
+			->getMock();
+
+		$this->assertFalse($request->passesLaxCookieCheck());
+	}
+
 	/**
 	 * @return array
 	 */
@@ -1426,10 +1650,10 @@ class RequestTest extends \Test\TestCase {
 
 		$token = new CsrfToken($invalidToken);
 		$this->csrfTokenManager
-				->expects($this->any())
-				->method('isTokenValid')
-				->with($token)
-				->willReturn(false);
+			->expects($this->any())
+			->method('isTokenValid')
+			->with($token)
+			->willReturn(false);
 
 		$this->assertFalse($request->passesCSRFCheck());
 	}
@@ -1450,4 +1674,4 @@ class RequestTest extends \Test\TestCase {
 		$this->assertFalse($request->passesCSRFCheck());
 	}
 
-}
+}
\ No newline at end of file
diff --git a/tests/lib/appframework/middleware/security/SecurityMiddlewareTest.php b/tests/lib/appframework/middleware/security/SecurityMiddlewareTest.php
index 9e71a3d0961..f7bb10c6880 100644
--- a/tests/lib/appframework/middleware/security/SecurityMiddlewareTest.php
+++ b/tests/lib/appframework/middleware/security/SecurityMiddlewareTest.php
@@ -31,6 +31,7 @@ use OC\Appframework\Middleware\Security\Exceptions\CrossSiteRequestForgeryExcept
 use OC\Appframework\Middleware\Security\Exceptions\NotAdminException;
 use OC\Appframework\Middleware\Security\Exceptions\NotLoggedInException;
 use OC\AppFramework\Middleware\Security\Exceptions\SecurityException;
+use OC\Appframework\Middleware\Security\Exceptions\StrictCookieMissingException;
 use OC\AppFramework\Utility\ControllerMethodReflector;
 use OC\Security\CSP\ContentSecurityPolicy;
 use OCP\AppFramework\Http\RedirectResponse;
@@ -255,7 +256,9 @@ class SecurityMiddlewareTest extends \Test\TestCase {
 		$this->request->expects($this->once())
 			->method('passesCSRFCheck')
 			->will($this->returnValue(false));
-
+		$this->request->expects($this->once())
+			->method('passesStrictCookieCheck')
+			->will($this->returnValue(true));
 		$this->reader->reflect(__CLASS__, __FUNCTION__);
 		$this->middleware->beforeController(__CLASS__, __FUNCTION__);
 	}
@@ -274,19 +277,81 @@ class SecurityMiddlewareTest extends \Test\TestCase {
 		$this->middleware->beforeController(__CLASS__, __FUNCTION__);
 	}
 
+	/**
+	 * @PublicPage
+	 */
+	public function testPassesCsrfCheck(){
+		$this->request->expects($this->once())
+			->method('passesCSRFCheck')
+			->will($this->returnValue(true));
+		$this->request->expects($this->once())
+			->method('passesStrictCookieCheck')
+			->will($this->returnValue(true));
+
+		$this->reader->reflect(__CLASS__, __FUNCTION__);
+		$this->middleware->beforeController(__CLASS__, __FUNCTION__);
+	}
 
 	/**
 	 * @PublicPage
+	 * @expectedException \OC\AppFramework\Middleware\Security\Exceptions\CrossSiteRequestForgeryException
 	 */
 	public function testFailCsrfCheck(){
 		$this->request->expects($this->once())
 			->method('passesCSRFCheck')
+			->will($this->returnValue(false));
+		$this->request->expects($this->once())
+			->method('passesStrictCookieCheck')
 			->will($this->returnValue(true));
 
 		$this->reader->reflect(__CLASS__, __FUNCTION__);
 		$this->middleware->beforeController(__CLASS__, __FUNCTION__);
 	}
 
+	/**
+	 * @PublicPage
+	 * @StrictCookieRequired
+	 * @expectedException \OC\Appframework\Middleware\Security\Exceptions\StrictCookieMissingException
+	 */
+	public function testStrictCookieRequiredCheck() {
+		$this->request->expects($this->never())
+			->method('passesCSRFCheck');
+		$this->request->expects($this->once())
+			->method('passesStrictCookieCheck')
+			->will($this->returnValue(false));
+
+		$this->reader->reflect(__CLASS__, __FUNCTION__);
+		$this->middleware->beforeController(__CLASS__, __FUNCTION__);
+	}
+
+
+	/**
+	 * @PublicPage
+	 * @NoCSRFRequired
+	 */
+	public function testNoStrictCookieRequiredCheck() {
+		$this->request->expects($this->never())
+			->method('passesStrictCookieCheck')
+			->will($this->returnValue(false));
+
+		$this->reader->reflect(__CLASS__, __FUNCTION__);
+		$this->middleware->beforeController(__CLASS__, __FUNCTION__);
+	}
+
+	/**
+	 * @PublicPage
+	 * @NoCSRFRequired
+	 * @StrictCookieRequired
+	 */
+	public function testPassesStrictCookieRequiredCheck() {
+		$this->request
+			->expects($this->once())
+			->method('passesStrictCookieCheck')
+			->willReturn(true);
+
+		$this->reader->reflect(__CLASS__, __FUNCTION__);
+		$this->middleware->beforeController(__CLASS__, __FUNCTION__);
+	}
 
 	/**
 	 * @NoCSRFRequired
@@ -360,6 +425,30 @@ class SecurityMiddlewareTest extends \Test\TestCase {
 		$this->assertEquals($expected , $response);
 	}
 
+	public function testAfterExceptionRedirectsToWebRootAfterStrictCookieFail() {
+		$this->request = new Request(
+			[
+				'server' => [
+						'HTTP_ACCEPT' => 'text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8',
+						'REQUEST_URI' => 'owncloud/index.php/apps/specialapp',
+					],
+			],
+			$this->getMock('\OCP\Security\ISecureRandom'),
+			$this->getMock('\OCP\IConfig')
+		);
+
+		$this->middleware = $this->getMiddleware(false, false);
+		$response = $this->middleware->afterException(
+			$this->controller,
+			'test',
+			new StrictCookieMissingException()
+		);
+
+		$expected = new RedirectResponse(\OC::$WEBROOT);
+		$this->assertEquals($expected , $response);
+	}
+
+
 	/**
 	 * @return array
 	 */