Remove support for X-Frame-Options in `<meta>`

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Mike West]

Firefox and Edge follow the RFC's suggestion (https://tools.ietf.org/html/rfc7034#section-4) to ignore the 'X-Frame-Options' header when delivered as `<meta http-equiv="...">` (and have done so from their initial implementations).

Blink has just removed this functionality (https://crbug.com/603002). The risks were outlined in https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/R1gkjKZI0J8, and seem minimal (~150 domains use the feature, period).

Perhaps WebKit could consider removing support as well?

[Radar WebKit Bug Importer]

<rdar://problem/25748714>

[Brent Fulgham]

I think this seems reasonable, and easy to do. I'll propose a patch.

[Brent Fulgham]

Created attachment 276480 [details]
Patch

[Build Bot]

Comment on attachment 276480 [details]
Patch

Attachment 276480 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/1164210

New failing tests:
inspector/console/x-frame-options-message.html

[Build Bot]

Created attachment 276484 [details]
Archive of layout-test-results from ews101 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Build Bot]

Comment on attachment 276480 [details]
Patch

Attachment 276480 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/1164213

New failing tests:
inspector/console/x-frame-options-message.html

[Build Bot]

Created attachment 276485 [details]
Archive of layout-test-results from ews104 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5

[Brent Fulgham]

Created attachment 276486 [details]
Patch v2

[Brent Fulgham]

Comment on attachment 276486 [details]
Patch v2

Rebaselined console test output.

[Brent Fulgham]

John and I discussed this offline, and he agrees that we should change to match Blink here because our current behavior is against spec, and this quirk is rarely used.

[Darin Adler]

Comment on attachment 276486 [details]
Patch v2

View in context: https://bugs.webkit.org/attachment.cgi?id=276486&action=review

> Source/WebCore/dom/Document.cpp:3308
> +            addConsoleMessage(MessageSource::Security, MessageLevel::Error, "X-Frame-Options may only be set via an HTTP header sent along with a document. It may not be set inside <meta>.", requestIdentifier);

This message seems a bit oblique. It states the rule but never explicitly mentions why the rule is being cited; because there is a meta element that has X-Frame-Options in it. It also doesn’t seem like this will point the developer right the meta element with the X-Frame-Options value that is being ignored. Maybe our tools have some way of doing that.

[Brent Fulgham]

Comment on attachment 276486 [details]
Patch v2

View in context: https://bugs.webkit.org/attachment.cgi?id=276486&action=review

>> Source/WebCore/dom/Document.cpp:3308
>> +            addConsoleMessage(MessageSource::Security, MessageLevel::Error, "X-Frame-Options may only be set via an HTTP header sent along with a document. It may not be set inside <meta>.", requestIdentifier);
> 
> This message seems a bit oblique. It states the rule but never explicitly mentions why the rule is being cited; because there is a meta element that has X-Frame-Options in it. It also doesn’t seem like this will point the developer right the meta element with the X-Frame-Options value that is being ignored. Maybe our tools have some way of doing that.

Our tools are capable of identifying the correct <meta> element that caused the console message. You can try it out by using any of the 'http/tests/security/XFrameOptions" tests in this bug. Clicking on this new console messages takes you to the offending <meta> element.

[Brent Fulgham]

Committed r199605: <http://trac.webkit.org/changeset/199605>

[Darin Adler]

Comment on attachment 276486 [details]
Patch v2

View in context: https://bugs.webkit.org/attachment.cgi?id=276486&action=review

>>> Source/WebCore/dom/Document.cpp:3308
>>> +            addConsoleMessage(MessageSource::Security, MessageLevel::Error, "X-Frame-Options may only be set via an HTTP header sent along with a document. It may not be set inside <meta>.", requestIdentifier);
>> 
>> This message seems a bit oblique. It states the rule but never explicitly mentions why the rule is being cited; because there is a meta element that has X-Frame-Options in it. It also doesn’t seem like this will point the developer right the meta element with the X-Frame-Options value that is being ignored. Maybe our tools have some way of doing that.
> 
> Our tools are capable of identifying the correct <meta> element that caused the console message. You can try it out by using any of the 'http/tests/security/XFrameOptions" tests in this bug. Clicking on this new console messages takes you to the offending <meta> element.

That addresses the issue I raised in the third sentence (I am happy that I was wrong), but does not address the other issue I raised in the first two sentences. Can the message be reworded to be less oblique?

[Brent Fulgham]

Comment on attachment 276486 [details]
Patch v2

View in context: https://bugs.webkit.org/attachment.cgi?id=276486&action=review

>>>> Source/WebCore/dom/Document.cpp:3308
>>>> +            addConsoleMessage(MessageSource::Security, MessageLevel::Error, "X-Frame-Options may only be set via an HTTP header sent along with a document. It may not be set inside <meta>.", requestIdentifier);
>>> 
>>> This message seems a bit oblique. It states the rule but never explicitly mentions why the rule is being cited; because there is a meta element that has X-Frame-Options in it. It also doesn’t seem like this will point the developer right the meta element with the X-Frame-Options value that is being ignored. Maybe our tools have some way of doing that.
>> 
>> Our tools are capable of identifying the correct <meta> element that caused the console message. You can try it out by using any of the 'http/tests/security/XFrameOptions" tests in this bug. Clicking on this new console messages takes you to the offending <meta> element.
> 
> That addresses the issue I raised in the third sentence (I am happy that I was wrong), but does not address the other issue I raised in the first two sentences. Can the message be reworded to be less oblique?

Hmm. Let me think about the wording and land something better in a follow-up patch. Do you have any suggestions?

How about:
"The X-Frame-Options '{Fill-in-the-blank} provided in a <meta> element was ignored because they may only be provided in an HTTP header sent with a document."

[Brent Fulgham]

> "The X-Frame-Options '{Fill-in-the-blank} provided in a <meta> element was
> ignored because they may only be provided in an HTTP header sent with a
> document."

Or maybe:

"The X-Frame-Option {fill in blank} supplied in a <meta> element was ignored. X-Frame-Options may only be provided by an HTTP header sent with the document."

[Brent Fulgham]

I spoke with Darin in person, and he agreed with the second choice. I'll land a patch with that change shortly.

[Brent Fulgham]

Revised language of message:

Committed r199696: <http://trac.webkit.org/changeset/199696>