Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions apps/files/lib/Controller/ApiController.php
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ public function __construct($appName,
*
* @NoAdminRequired
* @NoCSRFRequired
* @StrictCookieRequired
*
* @param int $x
* @param int $y
Expand Down
4 changes: 4 additions & 0 deletions core/js/config.php
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@
*
*/

if(!\OC::$server->getRequest()->passesStrictCookieCheck()) {
die();
}

// Set the content type to Javascript
header("Content-type: text/javascript");

Expand Down
80 changes: 80 additions & 0 deletions lib/base.php
Original file line number Diff line number Diff line change
Expand Up @@ -469,6 +469,84 @@ public static function setRequiredIniValues() {
@ini_set('gd.jpeg_ignore_warning', 1);
}

/**
* Send the same site cookies
*/
private static function sendSameSiteCookies() {
$cookieParams = session_get_cookie_params();
$secureCookie = ($cookieParams['secure'] === true) ? 'secure; ' : '';
$policies = [
'lax',
'strict',
];
foreach($policies as $policy) {
header(
sprintf(
'Set-Cookie: nc_sameSiteCookie%s=true; path=%s; httponly;' . $secureCookie . 'expires=Fri, 31-Dec-2100 23:59:59 GMT; SameSite=%s',
$policy,
$cookieParams['path'],
$policy
),
false
);
}
}

/**
* Same Site cookie to further mitigate CSRF attacks. This cookie has to
* be set in every request if cookies are sent to add a second level of
* defense against CSRF.
*
* If the cookie is not sent this will set the cookie and reload the page.
* We use an additional cookie since we want to protect logout CSRF and
* also we can't directly interfere with PHP's session mechanism.
*/
private static function performSameSiteCookieProtection() {
if(count($_COOKIE) > 0) {
$request = \OC::$server->getRequest();
$requestUri = $request->getScriptName();
$processingScript = explode('/', $requestUri);
$processingScript = $processingScript[count($processingScript)-1];
// FIXME: In a SAML scenario we don't get any strict or lax cookie
// send for the ACS endpoint. Since we have some legacy code in Nextcloud
// (direct PHP files) the enforcement of lax cookies is performed here
// instead of the middleware.
//
// This means we cannot exclude some routes from the cookie validation,
// which normally is not a problem but is a little bit cumbersome for
// this use-case.
// Once the old legacy PHP endpoints have been removed we can move
// the verification into a middleware and also adds some exemptions.
//
// Questions about this code? Ask Lukas ;-)
$currentUrl = substr(explode('?',$request->getRequestUri(), 2)[0], strlen(\OC::$WEBROOT));
if($currentUrl === '/index.php/apps/user_saml/saml/acs') {
return;
}
// For the "index.php" endpoint only a lax cookie is required.
if($processingScript === 'index.php') {
if(!$request->passesLaxCookieCheck()) {
self::sendSameSiteCookies();
header('Location: '.$_SERVER['REQUEST_URI']);
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();
}
}

public static function init() {
// calculate the root directories
OC::$SERVERROOT = str_replace("\\", '/', substr(__DIR__, 0, -4));
Expand Down Expand Up @@ -572,6 +650,8 @@ public static function init() {
ini_set('session.cookie_secure', true);
}

self::performSameSiteCookieProtection();

if (!defined('OC_CONSOLE')) {
$errors = OC_Util::checkServer(\OC::$server->getConfig());
if (count($errors) > 0) {
Expand Down
40 changes: 40 additions & 0 deletions lib/private/AppFramework/Http/Request.php
Original file line number Diff line number Diff line change
Expand Up @@ -465,6 +465,10 @@ public function passesCSRFCheck() {
return false;
}

if(!$this->passesStrictCookieCheck()) {
return false;
}

if (isset($this->items['get']['requesttoken'])) {
$token = $this->items['get']['requesttoken'];
} elseif (isset($this->items['post']['requesttoken'])) {
Expand All @@ -480,6 +484,42 @@ public function passesCSRFCheck() {
return $this->csrfTokenManager->isTokenValid($token);
}

/**
* Checks if the strict cookie has been sent with the request if the request
* is including any cookies.
*
* @return bool
* @since 9.1.0
*/
public function passesStrictCookieCheck() {
if(count($this->cookies) === 0) {
return true;
}
if($this->getCookie('nc_sameSiteCookiestrict') === 'true'
&& $this->passesLaxCookieCheck()) {
return true;
}
return false;
}

/**
* Checks if the lax cookie has been sent with the request if the request
* is including any cookies.
*
* @return bool
* @since 9.1.0
*/
public function passesLaxCookieCheck() {
if(count($this->cookies) === 0) {
return true;
}
if($this->getCookie('nc_sameSiteCookielax') === 'true') {
return true;
}
return false;
}


/**
* Returns an ID for the request, value is not guaranteed to be unique and is mostly meant for logging
* If `mod_unique_id` is installed this value will be taken.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
<?php
/**
* @author Lukas Reschke <[email protected]>
*
* @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 StrictCookieMissingException is thrown when the strict cookie has not
* been sent with the request but is required.
*
* @package OC\AppFramework\Middleware\Security\Exceptions
*/
class StrictCookieMissingException extends SecurityException {
public function __construct() {
parent::__construct('Strict Cookie has not been found in request.', Http::STATUS_PRECONDITION_FAILED);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
use OC\AppFramework\Middleware\Security\Exceptions\CrossSiteRequestForgeryException;
use OC\AppFramework\Middleware\Security\Exceptions\NotAdminException;
use OC\AppFramework\Middleware\Security\Exceptions\NotLoggedInException;
use OC\AppFramework\Middleware\Security\Exceptions\StrictCookieMissingException;
use OC\AppFramework\Utility\ControllerMethodReflector;
use OC\Security\CSP\ContentSecurityPolicyManager;
use OCP\AppFramework\Http\ContentSecurityPolicy;
Expand Down Expand Up @@ -134,6 +135,12 @@ public function beforeController($controller, $methodName) {
}
}

// Check for strict cookie requirement
if($this->reflector->hasAnnotation('StrictCookieRequired') || !$this->reflector->hasAnnotation('NoCSRFRequired')) {
if(!$this->request->passesStrictCookieCheck()) {
throw new StrictCookieMissingException();
}
}
// CSRF check - also registers the CSRF token since the session may be closed later
Util::callRegister();
if(!$this->reflector->hasAnnotation('NoCSRFRequired')) {
Expand Down Expand Up @@ -186,7 +193,9 @@ public function afterController($controller, $methodName, Response $response) {
*/
public function afterException($controller, $methodName, \Exception $exception) {
if($exception instanceof SecurityException) {

if($exception instanceof StrictCookieMissingException) {
return new RedirectResponse(\OC::$WEBROOT);
}
if (stripos($this->request->getHeader('Accept'),'html') === false) {
$response = new JSONResponse(
array('message' => $exception->getMessage()),
Expand Down
4 changes: 4 additions & 0 deletions lib/private/legacy/eventsource.php
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,10 @@ protected function init() {
} else {
header("Content-Type: text/event-stream");
}
if(!\OC::$server->getRequest()->passesStrictCookieCheck()) {
header('Location: '.\OC::$WEBROOT);
exit();
}
if (!(\OC::$server->getRequest()->passesCSRFCheck())) {
$this->send('error', 'Possible CSRF attack. Connection will be closed.');
$this->close();
Expand Down
5 changes: 5 additions & 0 deletions lib/private/legacy/json.php
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,11 @@ public static function checkLoggedIn() {
* @deprecated Use annotation based CSRF checks from the AppFramework instead
*/
public static function callCheck() {
if(!\OC::$server->getRequest()->passesStrictCookieCheck()) {
header('Location: '.\OC::$WEBROOT);
exit();
}

if( !(\OC::$server->getRequest()->passesCSRFCheck())) {
$l = \OC::$server->getL10N('lib');
self::error(array( 'data' => array( 'message' => $l->t('Token expired. Please reload page.'), 'error' => 'token_expired' )));
Expand Down
18 changes: 18 additions & 0 deletions lib/public/IRequest.php
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,24 @@ public function getCookie($key);
*/
public function passesCSRFCheck();

/**
* Checks if the strict cookie has been sent with the request if the request
* is including any cookies.
*
* @return bool
* @since 9.0.0
*/
public function passesStrictCookieCheck();

/**
* Checks if the lax cookie has been sent with the request if the request
* is including any cookies.
*
* @return bool
* @since 9.0.0
*/
public function passesLaxCookieCheck();

/**
* Returns an ID for the request, value is not guaranteed to be unique and is mostly meant for logging
* If `mod_unique_id` is installed this value will be taken.
Expand Down
5 changes: 5 additions & 0 deletions lib/public/Util.php
Original file line number Diff line number Diff line change
Expand Up @@ -513,6 +513,11 @@ public static function callRegister() {
* @deprecated 9.0.0 Use annotations based on the app framework.
*/
public static function callCheck() {
if(!\OC::$server->getRequest()->passesStrictCookieCheck()) {
header('Location: '.\OC::$WEBROOT);
exit();
}

if (!(\OC::$server->getRequest()->passesCSRFCheck())) {
exit();
}
Expand Down
Loading