Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>tags/v13.0.0beta1
@@ -559,24 +559,20 @@ class OC { | |||
if($currentUrl === '/index.php/apps/user_saml/saml/acs' || $currentUrl === '/apps/user_saml/saml/acs') { | |||
return; | |||
} | |||
// For the "index.php" endpoint only a lax cookie is required. | |||
// index.php routes are handled in the middleware | |||
if($processingScript === 'index.php') { | |||
if(!$request->passesLaxCookieCheck()) { | |||
self::sendSameSiteCookies(); | |||
header('Location: '.$_SERVER['REQUEST_URI']); | |||
return; | |||
} | |||
// All other endpoints require the lax and the strict cookie | |||
if(!$request->passesStrictCookieCheck()) { | |||
self::sendSameSiteCookies(); | |||
// Debug mode gets access to the resources without strict cookie | |||
// due to the fact that the SabreDAV browser also lives there. | |||
if(!\OC::$server->getConfig()->getSystemValue('debug', false)) { | |||
http_response_code(\OCP\AppFramework\Http::STATUS_SERVICE_UNAVAILABLE); | |||
exit(); | |||
} | |||
} else { | |||
// All other endpoints require the lax and the strict cookie | |||
if(!$request->passesStrictCookieCheck()) { | |||
self::sendSameSiteCookies(); | |||
// Debug mode gets access to the resources without strict cookie | |||
// due to the fact that the SabreDAV browser also lives there. | |||
if(!\OC::$server->getConfig()->getSystemValue('debug', false)) { | |||
http_response_code(\OCP\AppFramework\Http::STATUS_SERVICE_UNAVAILABLE); | |||
exit(); | |||
} | |||
} | |||
} | |||
} elseif(!isset($_COOKIE['nc_sameSiteCookielax']) || !isset($_COOKIE['nc_sameSiteCookiestrict'])) { | |||
self::sendSameSiteCookies(); |
@@ -306,12 +306,14 @@ return array( | |||
'OC\\AppFramework\\Middleware\\Security\\CORSMiddleware' => $baseDir . '/lib/private/AppFramework/Middleware/Security/CORSMiddleware.php', | |||
'OC\\AppFramework\\Middleware\\Security\\Exceptions\\AppNotEnabledException' => $baseDir . '/lib/private/AppFramework/Middleware/Security/Exceptions/AppNotEnabledException.php', | |||
'OC\\AppFramework\\Middleware\\Security\\Exceptions\\CrossSiteRequestForgeryException' => $baseDir . '/lib/private/AppFramework/Middleware/Security/Exceptions/CrossSiteRequestForgeryException.php', | |||
'OC\\AppFramework\\Middleware\\Security\\Exceptions\\LaxSameSiteCookieFailedException' => $baseDir . '/lib/private/AppFramework/Middleware/Security/Exceptions/LaxSameSiteCookieFailedException.php', | |||
'OC\\AppFramework\\Middleware\\Security\\Exceptions\\NotAdminException' => $baseDir . '/lib/private/AppFramework/Middleware/Security/Exceptions/NotAdminException.php', | |||
'OC\\AppFramework\\Middleware\\Security\\Exceptions\\NotConfirmedException' => $baseDir . '/lib/private/AppFramework/Middleware/Security/Exceptions/NotConfirmedException.php', | |||
'OC\\AppFramework\\Middleware\\Security\\Exceptions\\NotLoggedInException' => $baseDir . '/lib/private/AppFramework/Middleware/Security/Exceptions/NotLoggedInException.php', | |||
'OC\\AppFramework\\Middleware\\Security\\Exceptions\\SecurityException' => $baseDir . '/lib/private/AppFramework/Middleware/Security/Exceptions/SecurityException.php', | |||
'OC\\AppFramework\\Middleware\\Security\\Exceptions\\StrictCookieMissingException' => $baseDir . '/lib/private/AppFramework/Middleware/Security/Exceptions/StrictCookieMissingException.php', | |||
'OC\\AppFramework\\Middleware\\Security\\RateLimitingMiddleware' => $baseDir . '/lib/private/AppFramework/Middleware/Security/RateLimitingMiddleware.php', | |||
'OC\\AppFramework\\Middleware\\Security\\SameSiteCookieMiddleware' => $baseDir . '/lib/private/AppFramework/Middleware/Security/SameSiteCookieMiddleware.php', | |||
'OC\\AppFramework\\Middleware\\Security\\SecurityMiddleware' => $baseDir . '/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php', | |||
'OC\\AppFramework\\Middleware\\SessionMiddleware' => $baseDir . '/lib/private/AppFramework/Middleware/SessionMiddleware.php', | |||
'OC\\AppFramework\\OCS\\BaseResponse' => $baseDir . '/lib/private/AppFramework/OCS/BaseResponse.php', |
@@ -336,12 +336,14 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c | |||
'OC\\AppFramework\\Middleware\\Security\\CORSMiddleware' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/Security/CORSMiddleware.php', | |||
'OC\\AppFramework\\Middleware\\Security\\Exceptions\\AppNotEnabledException' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/Security/Exceptions/AppNotEnabledException.php', | |||
'OC\\AppFramework\\Middleware\\Security\\Exceptions\\CrossSiteRequestForgeryException' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/Security/Exceptions/CrossSiteRequestForgeryException.php', | |||
'OC\\AppFramework\\Middleware\\Security\\Exceptions\\LaxSameSiteCookieFailedException' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/Security/Exceptions/LaxSameSiteCookieFailedException.php', | |||
'OC\\AppFramework\\Middleware\\Security\\Exceptions\\NotAdminException' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/Security/Exceptions/NotAdminException.php', | |||
'OC\\AppFramework\\Middleware\\Security\\Exceptions\\NotConfirmedException' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/Security/Exceptions/NotConfirmedException.php', | |||
'OC\\AppFramework\\Middleware\\Security\\Exceptions\\NotLoggedInException' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/Security/Exceptions/NotLoggedInException.php', | |||
'OC\\AppFramework\\Middleware\\Security\\Exceptions\\SecurityException' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/Security/Exceptions/SecurityException.php', | |||
'OC\\AppFramework\\Middleware\\Security\\Exceptions\\StrictCookieMissingException' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/Security/Exceptions/StrictCookieMissingException.php', | |||
'OC\\AppFramework\\Middleware\\Security\\RateLimitingMiddleware' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/Security/RateLimitingMiddleware.php', | |||
'OC\\AppFramework\\Middleware\\Security\\SameSiteCookieMiddleware' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/Security/SameSiteCookieMiddleware.php', | |||
'OC\\AppFramework\\Middleware\\Security\\SecurityMiddleware' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php', | |||
'OC\\AppFramework\\Middleware\\SessionMiddleware' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/SessionMiddleware.php', | |||
'OC\\AppFramework\\OCS\\BaseResponse' => __DIR__ . '/../../..' . '/lib/private/AppFramework/OCS/BaseResponse.php', |
@@ -291,9 +291,17 @@ class DIContainer extends SimpleContainer implements IAppContainer { | |||
); | |||
}); | |||
$this->registerService(OC\AppFramework\Middleware\Security\SameSiteCookieMiddleware::class, function (SimpleContainer $c) { | |||
return new OC\AppFramework\Middleware\Security\SameSiteCookieMiddleware( | |||
$c['Request'], | |||
$c['ControllerMethodReflector'] | |||
); | |||
}); | |||
$middleWares = &$this->middleWares; | |||
$this->registerService('MiddlewareDispatcher', function($c) use (&$middleWares) { | |||
$dispatcher = new MiddlewareDispatcher(); | |||
$dispatcher->registerMiddleware($c[OC\AppFramework\Middleware\Security\SameSiteCookieMiddleware::class]); | |||
$dispatcher->registerMiddleware($c['CORSMiddleware']); | |||
$dispatcher->registerMiddleware($c['OCSMiddleware']); | |||
$dispatcher->registerMiddleware($c['SecurityMiddleware']); |
@@ -507,7 +507,7 @@ class Request implements \ArrayAccess, \Countable, IRequest { | |||
* | |||
* @return array | |||
*/ | |||
protected function getCookieParams() { | |||
public function getCookieParams() { | |||
return session_get_cookie_params(); | |||
} | |||
@@ -0,0 +1,38 @@ | |||
<?php | |||
/** | |||
* @copyright 2017, Roeland Jago Douma <roeland@famdouma.nl> | |||
* | |||
* @author Roeland Jago Douma <roeland@famdouma.nl> | |||
* | |||
* @license GNU AGPL version 3 or any later version | |||
* | |||
* This program is free software: you can redistribute it and/or modify | |||
* it under the terms of the GNU Affero General Public License as | |||
* published by the Free Software Foundation, either version 3 of the | |||
* License, or (at your option) any later version. | |||
* | |||
* This program is distributed in the hope that it will be useful, | |||
* but WITHOUT ANY WARRANTY; without even the implied warranty of | |||
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | |||
* GNU Affero General Public License for more details. | |||
* | |||
* You should have received a copy of the GNU Affero General Public License | |||
* along with this program. If not, see <http://www.gnu.org/licenses/>. | |||
* | |||
*/ | |||
namespace OC\AppFramework\Middleware\Security\Exceptions; | |||
use OCP\AppFramework\Http; | |||
/** | |||
* Class LaxSameSiteCookieFailedException is thrown when a request doesn't pass | |||
* the required LaxCookie check on index.php | |||
* | |||
* @package OC\AppFramework\Middleware\Security\Exceptions | |||
*/ | |||
class LaxSameSiteCookieFailedException extends SecurityException { | |||
public function __construct() { | |||
parent::__construct('Lax Same Site Cookie is invalid in request.', Http::STATUS_PRECONDITION_FAILED); | |||
} | |||
} |
@@ -0,0 +1,106 @@ | |||
<?php | |||
/** | |||
* @copyright 2017, Roeland Jago Douma <roeland@famdouma.nl> | |||
* | |||
* @author Roeland Jago Douma <roeland@famdouma.nl> | |||
* | |||
* @license GNU AGPL version 3 or any later version | |||
* | |||
* This program is free software: you can redistribute it and/or modify | |||
* it under the terms of the GNU Affero General Public License as | |||
* published by the Free Software Foundation, either version 3 of the | |||
* License, or (at your option) any later version. | |||
* | |||
* This program is distributed in the hope that it will be useful, | |||
* but WITHOUT ANY WARRANTY; without even the implied warranty of | |||
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | |||
* GNU Affero General Public License for more details. | |||
* | |||
* You should have received a copy of the GNU Affero General Public License | |||
* along with this program. If not, see <http://www.gnu.org/licenses/>. | |||
* | |||
*/ | |||
namespace OC\AppFramework\Middleware\Security; | |||
use OC\AppFramework\Http\Request; | |||
use OC\AppFramework\Middleware\Security\Exceptions\LaxSameSiteCookieFailedException; | |||
use OC\AppFramework\Utility\ControllerMethodReflector; | |||
use OCP\AppFramework\Http; | |||
use OCP\AppFramework\Http\Response; | |||
use OCP\AppFramework\Middleware; | |||
class SameSiteCookieMiddleware extends Middleware { | |||
/** @var Request */ | |||
private $request; | |||
/** @var ControllerMethodReflector */ | |||
private $reflector; | |||
public function __construct(Request $request, | |||
ControllerMethodReflector $reflector) { | |||
$this->request = $request; | |||
$this->reflector = $reflector; | |||
} | |||
public function beforeController($controller, $methodName) { | |||
$requestUri = $this->request->getScriptName(); | |||
$processingScript = explode('/', $requestUri); | |||
$processingScript = $processingScript[count($processingScript)-1]; | |||
if ($processingScript !== 'index.php') { | |||
return; | |||
} | |||
$noSSC = $this->reflector->hasAnnotation('NoSameSiteCookieRequired'); | |||
if ($noSSC) { | |||
return; | |||
} | |||
if (!$this->request->passesLaxCookieCheck()) { | |||
throw new LaxSameSiteCookieFailedException(); | |||
} | |||
} | |||
public function afterException($controller, $methodName, \Exception $exception) { | |||
if ($exception instanceof LaxSameSiteCookieFailedException) { | |||
$respone = new Response(); | |||
$respone->setStatus(Http::STATUS_FOUND); | |||
$respone->addHeader('Location', $this->request->getRequestUri()); | |||
$this->setSameSiteCookie(); | |||
return $respone; | |||
} | |||
throw $exception; | |||
} | |||
protected function setSameSiteCookie() { | |||
$cookieParams = $this->request->getCookieParams(); | |||
$secureCookie = ($cookieParams['secure'] === true) ? 'secure; ' : ''; | |||
$policies = [ | |||
'lax', | |||
'strict', | |||
]; | |||
// Append __Host to the cookie if it meets the requirements | |||
$cookiePrefix = ''; | |||
if($cookieParams['secure'] === true && $cookieParams['path'] === '/') { | |||
$cookiePrefix = '__Host-'; | |||
} | |||
foreach($policies as $policy) { | |||
header( | |||
sprintf( | |||
'Set-Cookie: %snc_sameSiteCookie%s=true; path=%s; httponly;' . $secureCookie . 'expires=Fri, 31-Dec-2100 23:59:59 GMT; SameSite=%s', | |||
$cookiePrefix, | |||
$policy, | |||
$cookieParams['path'], | |||
$policy | |||
), | |||
false | |||
); | |||
} | |||
} | |||
} |
@@ -1568,8 +1568,18 @@ class RequestTest extends \Test\TestCase { | |||
} | |||
public function testGetCookieParams() { | |||
$request = $this->createMock(Request::class); | |||
$actual = $this->invokePrivate($request, 'getCookieParams'); | |||
/** @var Request $request */ | |||
$request = $this->getMockBuilder(Request::class) | |||
->setMethods(['getScriptName']) | |||
->setConstructorArgs([ | |||
[], | |||
$this->secureRandom, | |||
$this->config, | |||
$this->csrfTokenManager, | |||
$this->stream | |||
]) | |||
->getMock(); | |||
$actual = $request->getCookieParams(); | |||
$this->assertSame(session_get_cookie_params(), $actual); | |||
} | |||
@@ -0,0 +1,133 @@ | |||
<?php | |||
/** | |||
* @copyright 2017, Roeland Jago Douma <roeland@famdouma.nl> | |||
* | |||
* @author Roeland Jago Douma <roeland@famdouma.nl> | |||
* | |||
* @license GNU AGPL version 3 or any later version | |||
* | |||
* This program is free software: you can redistribute it and/or modify | |||
* it under the terms of the GNU Affero General Public License as | |||
* published by the Free Software Foundation, either version 3 of the | |||
* License, or (at your option) any later version. | |||
* | |||
* This program is distributed in the hope that it will be useful, | |||
* but WITHOUT ANY WARRANTY; without even the implied warranty of | |||
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | |||
* GNU Affero General Public License for more details. | |||
* | |||
* You should have received a copy of the GNU Affero General Public License | |||
* along with this program. If not, see <http://www.gnu.org/licenses/>. | |||
* | |||
*/ | |||
namespace Test\AppFramework\Middleware\Security; | |||
use OC\AppFramework\Http\Request; | |||
use OC\AppFramework\Middleware\Security\Exceptions\LaxSameSiteCookieFailedException; | |||
use OC\AppFramework\Middleware\Security\Exceptions\SecurityException; | |||
use OC\AppFramework\Middleware\Security\SameSiteCookieMiddleware; | |||
use OC\AppFramework\Utility\ControllerMethodReflector; | |||
use OCP\AppFramework\Controller; | |||
use OCP\AppFramework\Http; | |||
use Test\TestCase; | |||
class SameSiteCookieMiddlewareTest extends TestCase { | |||
/** @var SameSiteCookieMiddleware */ | |||
private $middleware; | |||
/** @var Request|\PHPUnit_Framework_MockObject_MockObject */ | |||
private $request; | |||
/** @var ControllerMethodReflector|\PHPUnit_Framework_MockObject_MockObject */ | |||
private $reflector; | |||
public function setUp() { | |||
parent::setUp(); | |||
$this->request = $this->createMock(Request::class); | |||
$this->reflector = $this->createMock(ControllerMethodReflector::class); | |||
$this->middleware = new SameSiteCookieMiddleware($this->request, $this->reflector); | |||
} | |||
public function testBeforeControllerNoIndex() { | |||
$this->request->method('getScriptName') | |||
->willReturn('/ocs/v2.php'); | |||
$this->middleware->beforeController($this->createMock(Controller::class), 'foo'); | |||
} | |||
public function testBeforeControllerIndexHasAnnotation() { | |||
$this->request->method('getScriptName') | |||
->willReturn('/index.php'); | |||
$this->reflector->method('hasAnnotation') | |||
->with('NoSameSiteCookieRequired') | |||
->willReturn(true); | |||
$this->middleware->beforeController($this->createMock(Controller::class), 'foo'); | |||
} | |||
public function testBeforeControllerIndexNoAnnotationPassingCheck() { | |||
$this->request->method('getScriptName') | |||
->willReturn('/index.php'); | |||
$this->reflector->method('hasAnnotation') | |||
->with('NoSameSiteCookieRequired') | |||
->willReturn(false); | |||
$this->request->method('passesLaxCookieCheck') | |||
->willReturn(true); | |||
$this->middleware->beforeController($this->createMock(Controller::class), 'foo'); | |||
} | |||
public function testBeforeControllerIndexNoAnnotationFailingCheck() { | |||
$this->expectException(LaxSameSiteCookieFailedException::class); | |||
$this->request->method('getScriptName') | |||
->willReturn('/index.php'); | |||
$this->reflector->method('hasAnnotation') | |||
->with('NoSameSiteCookieRequired') | |||
->willReturn(false); | |||
$this->request->method('passesLaxCookieCheck') | |||
->willReturn(false); | |||
$this->middleware->beforeController($this->createMock(Controller::class), 'foo'); | |||
} | |||
public function testAfterExceptionNoLaxCookie() { | |||
$ex = new SecurityException(); | |||
try { | |||
$this->middleware->afterException($this->createMock(Controller::class), 'foo', $ex); | |||
$this->fail(); | |||
} catch (\Exception $e) { | |||
$this->assertSame($ex, $e); | |||
} | |||
} | |||
public function testAfterExceptionLaxCookie() { | |||
$ex = new LaxSameSiteCookieFailedException(); | |||
$this->request->method('getRequestUri') | |||
->willReturn('/myrequri'); | |||
$middleware = $this->getMockBuilder(SameSiteCookieMiddleware::class) | |||
->setConstructorArgs([$this->request, $this->reflector]) | |||
->setMethods(['setSameSiteCookie']) | |||
->getMock(); | |||
$middleware->expects($this->once()) | |||
->method('setSameSiteCookie'); | |||
$resp = $middleware->afterException($this->createMock(Controller::class), 'foo', $ex); | |||
$this->assertSame(Http::STATUS_FOUND, $resp->getStatus()); | |||
$headers = $resp->getHeaders(); | |||
$this->assertSame('/myrequri', $headers['Location']); | |||
} | |||
} |