By fabianx on
Change record status: 
Published (View all published change records)
Project: 
Drupal core
Introduced in branch: 
8.0.x
Introduced in version: 
8.0.0-beta13
Issue links: 
https://www.drupal.org/node/2514136
Description: 

Drupal core now protects against clickjacking by default by emitting the 'X-Frame-Options: SAMEORIGIN' header. This prevents the site from being embedded in an iframe on another domain.

This should not be disruptive for contributed modules, unless a contributed module or site wants to embed the Drupal site somewhere else (e.g. for example in a Facebook application).

In that case a new Response Subscriber needs to be added that has a higher priority as the current FinishResponseSubscriber (see core.services.yml) to overwrite or remove the header - depending on the use case.

https://api.drupal.org/api/drupal/core!lib!Drupal!Core!EventSubscriber!F... is also a good example of how to write such a subscriber.

e.g. as an example for a Facebook application living at the /fb-app path:

    $path = $request->getPathInfo();

    if (strpos($path, '/fb-app/') === 0) {
      $request->headers->remove('X-Frame-Options');
    }

Do not remove the header lightly, as else your Drupal site could be embedded on other sites and then the user tricked into doing actions they don't want.

See also the related Drupal 7 change record.

Impacts: 
Site builders, administrators, editors
Module developers

Comments

geek-merlin’s picture

geek-merlin
Primary language German
Location Freiburg, Germany
commented

In fact in your shiny new facebook app you should not do

if (strpos($path, '/fb-app/') === 0) {
      $request->headers->remove('X-Frame-Options');
    }

but rather something like (untested)

if (strpos($path, '/fb-app/') === 0) {
      $request->headers->set('X-Frame-Options', 'ALLOW-FROM https://www.facebook.com/');
    }

Also see https://developer.mozilla.org/docs/Web/HTTP/Headers/X-Frame-Options

mxh’s picture

mxh commented

... and would make sense as it would be a similar approach to CORS ' s Access-Control-Allow-Origin header, it unfortunately won't work with all browsers. Chrome for example does not use this directive, it only "accepts" DENY and SAMEORIGIN values.
Actually, the only way I know is to remove the X-Frame-Options header when you know it's ok to do so.

mxh’s picture

mxh commented

.. that breaks flexibility of writing custom controllers. Why just not checking inside FinishResponseSubscriber if that header is already set, and if so - do nothing. If it's not set, then set SAMEORIGIN as default - like this change record explains, but actually not really does, as it ALWAYS sets the value, no matter what. Now you need to write more event subscribers, which are always being instantiated on EVERY page request, just for checking if that header is set and if so - reset it to your needs.

Alternative: Write wrapper classes for the ResponseHeaderBag. This would at least save you from writing those annoying event subscribers, but those wrappers are also a complicated way to solve this.

geek-merlin’s picture

geek-merlin
Primary language German
Location Freiburg, Germany
commented

Cool you are digging too into this.

> "x-frame-options: allow-from example.com" not working in chrome

Yes unfortunately. This SO answer suggests to instead leverage X-Content-Security-Policy. We'd have to add a cache context to vary by browser (does this exist?) though.
We might create a core issue to leverage that.

> Why just not checking inside FinishResponseSubscriber if that header is already set, and if so - do nothing.

Fortunately this is the case:

    $response->headers->set('X-Frame-Options', 'SAMEORIGIN', FALSE);

The third parameter is "overwrite"! See \Drupal\render_attached_test\Controller\RenderAttachedTestController::header

    $render['#attached']['http_header'][] = ['X-Test-Teapot-Replace', 'This value gets replaced'];
    $render['#attached']['http_header'][] = ['X-Test-Teapot-Replace', 'Teapot replaced', TRUE];
    $render['#attached']['http_header'][] = ['X-Test-Teapot-No-Replace', 'This value is not replaced'];
    $render['#attached']['http_header'][] = ['X-Test-Teapot-No-Replace', 'This one is added', FALSE];
    $render['#attached']['http_header'][] = ['X-Test-Teapot', 'Teapot Mode Active'];

Which also instructs us how to do this in any render array...
So it's easy to write a trivial module providing a block that sets empty x-frame-options header liks so (and as it is empty and not missing does not get overrideen by FinishResponseSubscriber:

    $render['#attached']['http_header'][] = ['X-Frame-Options', '', TRUE];
mxh’s picture

mxh commented

We'd have to add a cache context to vary by browser (does this exist?) though.

page_cache doesn't support cache contexts, which makes it hard to cache by Vary header for example. Dynamic Page Cache can support it.
https://www.drupal.org/project/drupal/issues/2972483#comment-12918534

<?php
$response->headers->set('X-Frame-Options', 'SAMEORIGIN', FALSE);
?>

It's a month since I investigated this one. But as what I remember is, that your suggestion would still result in SAMEORIGIN being in the X-Frame-Options header key.
The $replace parameter, when set to FALSE, would perform an array merge on the given header values, and when set to TRUE, it would just reset the already existing list of values, then appending the new value(s). See also
https://api.drupal.org/api/drupal/vendor%21symfony%21http-foundation%21R...

As mentioned above, core behavior makes it hard for custom controllers - e.g. Json response controllers - to determine that SAMEORIGIN should not be included as value. Such a controller might not return a render array, but it's own JsonResponse object.