Issue #1891052 by klausi: Fixed Protect write operations from CRSF by...

Issue #1891052 by klausi: Fixed Protect write operations from CRSF by requiring an token (when using cookie/session-based authentication).

core/modules/rest/lib/Drupal/rest/Access/CSRFAccessCheck.php

+<?php
+
+/**
+ * @file
+ * Contains Drupal\rest\Access\CSRFAccessCheck.
+ */
+
+namespace Drupal\rest\Access;
+
+use Drupal\Core\Access\AccessCheckInterface;
+use Symfony\Component\Routing\Route;
+use Symfony\Component\HttpFoundation\Request;
+
+/**
+ * Access protection against CSRF attacks.
+ */
+class CSRFAccessCheck implements AccessCheckInterface {
+
+  /**
+   * Implements AccessCheckInterface::applies().
+   */
+  public function applies(Route $route) {
+    $requirements = $route->getRequirements();
+    if (array_key_exists('_access_rest_csrf', $requirements)) {
+      if (isset($requirements['_method'])) {
+        // There could be more than one method requirement separated with '|'.
+        $methods = explode('|', $requirements['_method']);
+        // CSRF protection only applies to write operations, so we can filter
+        // out any routes that require reading methods only.
+        $write_methods = array_diff($methods, array('GET', 'HEAD', 'OPTIONS', 'TRACE'));
+        if (empty($write_methods)) {
+          return FALSE;
+        }
+      }
+      // No method requirement given, so we run this access check to be on the
+      // safe side.
+      return TRUE;
+    }
+    return FALSE;
+  }
+
+  /**
+   * Implements AccessCheckInterface::access().
+   */
+  public function access(Route $route, Request $request) {
+    $method = $request->getMethod();
+    $cookie = $request->cookies->get(session_name(), FALSE);
+    // This check only applies if
+    // 1. this is a write operation
+    // 2. the user was successfully authenticated and
+    // 3. the request comes with a session cookie.
+    if (!in_array($method, array('GET', 'HEAD', 'OPTIONS', 'TRACE'))
+      && user_is_logged_in()
+      && $cookie
+    ) {
+      $csrf_token = $request->headers->get('X-CSRF-Token');
+      if (!drupal_valid_token($csrf_token, 'rest')) {
+        return FALSE;
+      }
+    }
+    // As we do not perform any authorization here we always return NULL to
+    // indicate that other access checkers should decide if the request is
+    // legit.
+    return NULL;
+  }
+}

core/modules/rest/lib/Drupal/rest/EventSubscriber/RouteSubscriber.php

@@ -60,8 +60,8 @@ public function dynamicRoutes(RouteBuildEvent $event) {
       foreach ($enabled as $key => $resource) {
         $plugin = $this->manager->getInstance(array('id' => $key));
 
-        // @todo Switch to ->addCollection() once http://drupal.org/node/1819018 is resolved.
         foreach ($plugin->routes() as $name => $route) {
+          $route->setRequirement('_access_rest_csrf',  'TRUE');
           $collection->add("rest.$name", $route);
         }
       }

core/modules/rest/lib/Drupal/rest/RequestHandler.php

@@ -32,6 +32,7 @@ class RequestHandler extends ContainerAware {
   public function handle(Request $request, $id = NULL) {
     $plugin = $request->attributes->get(RouteObjectInterface::ROUTE_OBJECT)->getDefault('_plugin');
     $method = strtolower($request->getMethod());
+
     $resource = $this->container
       ->get('plugin.manager.rest')
       ->getInstance(array('id' => $plugin));
@@ -67,4 +68,14 @@ public function handle(Request $request, $id = NULL) {
     }
     return $response;
   }
+
+  /**
+   * Generates a CSRF protecting session token.
+   *
+   * @return \Symfony\Component\HttpFoundation\Response
+   *   The response object.
+   */
+  public function csrfToken() {
+    return new Response(drupal_get_token('rest'), 200, array('Content-Type' => 'text/plain'));
+  }
 }

core/modules/rest/lib/Drupal/rest/RestBundle.php

@@ -28,5 +28,8 @@ public function build(ContainerBuilder $container) {
       ->addArgument(new Reference('plugin.manager.rest'))
       ->addArgument(new Reference('config.factory'))
       ->addTag('event_subscriber');
+
+    $container->register('access_check.rest.csrf', 'Drupal\rest\Access\CSRFAccessCheck')
+      ->addTag('access_check');
   }
 }

core/modules/rest/lib/Drupal/rest/Tests/CreateTest.php

@@ -53,7 +53,7 @@ public function testCreate() {
 
     // Get the new entity ID from the location header and try to read it from
     // the database.
-    $location_url = $this->responseHeaders['location'];
+    $location_url = $this->drupalGetHeader('location');
     $url_parts = explode('/', $location_url);
     $id = end($url_parts);
     $loaded_entity = entity_load($entity_type, $id);
@@ -68,16 +68,32 @@ public function testCreate() {
       $this->assertEqual($send_value, $actual_value, 'Created property ' . $property . ' expected: ' . $send_value . ', actual: ' . $actual_value);
     }
 
+    $loaded_entity->delete();
+    // Try to create an entity without the CSRF token.
+    $this->curlExec(array(
+      CURLOPT_HTTPGET => FALSE,
+      CURLOPT_POST => TRUE,
+      CURLOPT_CUSTOMREQUEST => 'POST',
+      CURLOPT_POSTFIELDS => $serialized,
+      CURLOPT_URL => url('entity/' . $entity_type, array('absolute' => TRUE)),
+      CURLOPT_NOBODY => FALSE,
+      CURLOPT_HTTPHEADER => array('Content-Type: application/vnd.drupal.ld+json'),
+    ));
+    $this->assertResponse(403);
+    $this->assertFalse(entity_load_multiple($entity_type, NULL, TRUE), 'No entity has been created in the database.');
+
     // Try to create an entity without proper permissions.
     $this->drupalLogout();
     $this->httpRequest('entity/' . $entity_type, 'POST', $serialized, 'application/vnd.drupal.ld+json');
     $this->assertResponse(403);
+    $this->assertFalse(entity_load_multiple($entity_type, NULL, TRUE), 'No entity has been created in the database.');
 
     // Try to create a resource which is not web API enabled.
     $this->enableService(FALSE);
     $this->drupalLogin($account);
     $this->httpRequest('entity/entity_test', 'POST', $serialized, 'application/vnd.drupal.ld+json');
     $this->assertResponse(404);
+    $this->assertFalse(entity_load_multiple($entity_type, NULL, TRUE), 'No entity has been created in the database.');
 
     // @todo Once EntityNG is implemented for other entity types add a security
     // test. It should not be possible for example to create a test entity on a

core/modules/rest/lib/Drupal/rest/Tests/DeleteTest.php

@@ -40,9 +40,6 @@ public function testDelete() {
       // Create a user account that has the required permissions to delete
       // resources via the web API.
       $account = $this->drupalCreateUser(array('restful delete entity:' . $entity_type));
-      // Reset cURL here because it is confused from our previously used cURL
-      // options.
-      unset($this->curlHandle);
       $this->drupalLogin($account);
 
       // Create an entity programmatically.
@@ -74,9 +71,6 @@ public function testDelete() {
     // Try to delete a resource which is not web API enabled.
     $this->enableService(FALSE);
     $account = $this->drupalCreateUser();
-    // Reset cURL here because it is confused from our previously used cURL
-    // options.
-    unset($this->curlHandle);
     $this->drupalLogin($account);
     $this->httpRequest('entity/user/' . $account->id(), 'DELETE');
     $user = entity_load('user', $account->id(), TRUE);

core/modules/rest/lib/Drupal/rest/Tests/RESTTestBase.php

@@ -14,13 +14,6 @@
  */
 abstract class RESTTestBase extends WebTestBase {
 
-  /**
-   * Stores HTTP response headers from the last HTTP request.
-   *
-   * @var array
-   */
-  protected $responseHeaders;
-
   /**
    * Helper function to issue a HTTP request with simpletest's cURL.
    *
@@ -34,6 +27,10 @@ abstract class RESTTestBase extends WebTestBase {
    *   The MIME type of the transmitted content.
    */
   protected function httpRequest($url, $method, $body = NULL, $format = 'application/ld+json') {
+    if (!in_array($method, array('GET', 'HEAD', 'OPTIONS', 'TRACE'))) {
+      // GET the CSRF token first for writing requests.
+      $token = $this->drupalGet('rest/session/token');
+    }
     switch ($method) {
       case 'GET':
         // Set query if there are additional GET parameters.
@@ -53,7 +50,10 @@ protected function httpRequest($url, $method, $body = NULL, $format = 'applicati
           CURLOPT_POSTFIELDS => $body,
           CURLOPT_URL => url($url, array('absolute' => TRUE)),
           CURLOPT_NOBODY => FALSE,
-          CURLOPT_HTTPHEADER => array('Content-Type: ' . $format),
+          CURLOPT_HTTPHEADER => array(
+            'Content-Type: ' . $format,
+            'X-CSRF-Token: ' . $token,
+          ),
         );
         break;
 
@@ -64,7 +64,10 @@ protected function httpRequest($url, $method, $body = NULL, $format = 'applicati
           CURLOPT_POSTFIELDS => $body,
           CURLOPT_URL => url($url, array('absolute' => TRUE)),
           CURLOPT_NOBODY => FALSE,
-          CURLOPT_HTTPHEADER => array('Content-Type: ' . $format),
+          CURLOPT_HTTPHEADER => array(
+            'Content-Type: ' . $format,
+            'X-CSRF-Token: ' . $token,
+          ),
         );
         break;
 
@@ -75,7 +78,10 @@ protected function httpRequest($url, $method, $body = NULL, $format = 'applicati
           CURLOPT_POSTFIELDS => $body,
           CURLOPT_URL => url($url, array('absolute' => TRUE)),
           CURLOPT_NOBODY => FALSE,
-          CURLOPT_HTTPHEADER => array('Content-Type: ' . $format),
+          CURLOPT_HTTPHEADER => array(
+            'Content-Type: ' . $format,
+            'X-CSRF-Token: ' . $token,
+          ),
         );
         break;
 
@@ -85,29 +91,21 @@ protected function httpRequest($url, $method, $body = NULL, $format = 'applicati
           CURLOPT_CUSTOMREQUEST => 'DELETE',
           CURLOPT_URL => url($url, array('absolute' => TRUE)),
           CURLOPT_NOBODY => FALSE,
+          CURLOPT_HTTPHEADER => array('X-CSRF-Token: ' . $token),
         );
         break;
     }
-    // Include all HTTP headers in the response.
-    $curl_options[CURLOPT_HEADER] = TRUE;
 
     $response = $this->curlExec($curl_options);
-
-    list($header, $body) = explode("\r\n\r\n", $response, 2);
-    $header_lines = explode("\r\n", $header);
-    foreach ($header_lines as $line) {
-      $parts = explode(':', $line, 2);
-      // Store the header keys lower cased to be more robust. Headers are case
-      // insensitive according to RFC 2616.
-      $this->responseHeaders[strtolower($parts[0])] = isset($parts[1]) ? trim($parts[1]) : '';
-    }
+    $headers = $this->drupalGetHeaders();
+    $headers = implode("\n", $headers);
 
     $this->verbose($method . ' request to: ' . $url .
       '<hr />Code: ' . curl_getinfo($this->curlHandle, CURLINFO_HTTP_CODE) .
-      '<hr />Response headers: ' . $header .
-      '<hr />Response body: ' . $body);
+      '<hr />Response headers: ' . $headers .
+      '<hr />Response body: ' . $response);
 
-    return $body;
+    return $response;
   }
 
   /**
@@ -198,7 +196,20 @@ protected function enableService($resource_type) {
    *   TRUE if the assertion succeeded, FALSE otherwise.
    */
   protected function assertHeader($header, $value, $message = '', $group = 'Browser') {
-    $match = isset($this->responseHeaders[$header]) && $this->responseHeaders[$header] == $value;
-    return $this->assertTrue($match, $message ? $message : 'HTTP response header ' . $header . ' with value ' . $value . ' found.', $group);
+    $header_value = $this->drupalGetHeader($header);
+    return $this->assertTrue($header_value == $value, $message ? $message : 'HTTP response header ' . $header . ' with value ' . $value . ' found.', $group);
+  }
+
+  /**
+   * Overrides WebTestBase::drupalLogin().
+   */
+  protected function drupalLogin($user) {
+    if (isset($this->curlHandle)) {
+      // cURL quirk: when setting CURLOPT_CUSTOMREQUEST to anything other than
+      // POST in httpRequest() it has to be restored to POST here. Otherwise the
+      // POST request to login a user will not work.
+      curl_setopt($this->curlHandle, CURLOPT_CUSTOMREQUEST, 'POST');
+    }
+    parent::drupalLogin($user);
   }
 }

core/modules/rest/lib/Drupal/rest/Tests/ReadTest.php

@@ -42,9 +42,6 @@ public function testRead() {
       // Create a user account that has the required permissions to delete
       // resources via the web API.
       $account = $this->drupalCreateUser(array('restful get entity:' . $entity_type));
-      // Reset cURL here because it is confused from our previously used cURL
-      // options.
-      unset($this->curlHandle);
       $this->drupalLogin($account);
 
       // Create an entity programmatically.
@@ -80,9 +77,6 @@ public function testRead() {
     }
     // Try to read a resource which is not web API enabled.
     $account = $this->drupalCreateUser();
-    // Reset cURL here because it is confused from our previously used cURL
-    // options.
-    unset($this->curlHandle);
     $this->drupalLogin($account);
     $response = $this->httpRequest('entity/user/' . $account->id(), 'GET', NULL, 'application/vnd.drupal.ld+json');
     $this->assertResponse(404);

core/modules/rest/lib/Drupal/rest/Tests/UpdateTest.php

@@ -89,9 +89,6 @@ public function testPatchUpdate() {
 
     // Try to update a resource which is not web API enabled.
     $this->enableService(FALSE);
-    // Reset cURL here because it is confused from our previously used cURL
-    // options.
-    unset($this->curlHandle);
     $this->drupalLogin($account);
     $this->httpRequest('entity/' . $entity_type . '/' . $entity->id(), 'PATCH', $serialized, 'application/vnd.drupal.ld+json');
     $this->assertResponse(404);
@@ -162,9 +159,6 @@ public function testPutUpdate() {
 
     // Try to update a resource which is not web API enabled.
     $this->enableService(FALSE);
-    // Reset cURL here because it is confused from our previously used cURL
-    // options.
-    unset($this->curlHandle);
     $this->drupalLogin($account);
     $this->httpRequest('entity/' . $entity_type . '/' . $entity->id(), 'PUT', $serialized, 'application/vnd.drupal.ld+json');
     $this->assertResponse(404);

core/modules/rest/rest.routing.yml

+rest.csrftoken:
+  pattern: '/rest/session/token'
+  defaults:
+    _controller: '\Drupal\rest\RequestHandler::csrfToken'
+  requirements:
+    _access: 'TRUE'