Merge pull request #44 from nextcloud/stable9-add-same-site-cookies

[stable9] Add Same Site Cookie protection

Author: LukasReschke

apps/files/controller/apicontroller.php

@@ -80,6 +80,7 @@ public function __construct($appName,
 	 *
 	 * @NoAdminRequired
 	 * @NoCSRFRequired
+	 * @StrictCookieRequired
 	 *
 	 * @param int $x
 	 * @param int $y

lib/base.php

@@ -480,6 +480,70 @@ 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];
+
+			// 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));
@@ -589,6 +653,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) {

lib/private/appframework/http/request.php

@@ -448,6 +448,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'])) {
@@ -463,6 +467,35 @@ public function passesCSRFCheck() {
 		return $this->csrfTokenManager->isTokenValid($token);
 	}
 
+	/**
+	 * Checks if the strict cookie has been sent with the request
+	 *
+	 * @return bool
+	 * @since 9.1.0
+	 */
+	public function passesStrictCookieCheck() {
+		if($this->getCookie('nc_sameSiteCookiestrict') === 'true'
+			&& $this->passesLaxCookieCheck()) {
+			return true;
+		}
+
+		return false;
+	}
+
+	/**
+	 * Checks if the lax cookie has been sent with the request
+	 *
+	 * @return bool
+	 * @since 9.1.0
+	 */
+	public function passesLaxCookieCheck() {
+		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.

lib/private/appframework/middleware/security/exceptions/strictcookiemissingexception.php

@@ -0,0 +1,36 @@
+<?php
+/**
+ * @author Lukas Reschke <lukas@statuscode.ch>
+ *
+ * @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);
+	}
+}

lib/private/appframework/middleware/security/securitymiddleware.php

@@ -30,6 +30,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;
@@ -132,6 +133,13 @@ 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')) {
@@ -184,6 +192,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(

lib/private/eventsource.php

@@ -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();

lib/private/json.php

@@ -77,6 +77,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' )));

lib/public/irequest.php

@@ -143,6 +143,22 @@ public function getCookie($key);
 	 */
 	public function passesCSRFCheck();
 
+	/**
+	* Checks if the strict cookie has been sent with the request
+	*
+	* @return bool
+	* @since 9.0.0
+	*/
+	public function passesStrictCookieCheck();
+
+	/**
+	 * Checks if the lax cookie has been sent with the request
+	 *
+	 * @return bool
+	 * @since 9.1.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.

lib/public/util.php

@@ -504,6 +504,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();
 		}