Changeset 146257 in webkit

Timestamp:

Mar 19, 2013 2:12:24 PM (11 years ago)

Author:

mkwst@chromium.org

Message:

FeatureObserver: Measure X-Frame-Options usage.
​https://bugs.webkit.org/show_bug.cgi?id=112670

Reviewed by Adam Barth.

This patch adds three FeatureObserver entries to gather information
about 'X-Frame-Options' usage in general, and in particular usage
and potential misunderstanding of the 'SAMEORIGIN' value.

Three entries are added:

• XFrameOptions measures the raw number of 'X-Frame-Options' headers.

• XFrameOptionsSameOrigin measures the number of those headers that set the value to 'SAMEORIGIN'.

• XFrameOptionsSameOriginWithBadAncestorChain measures the number of occasions in which the frame passed the "top-only" origin check we're currently performing, but would have failed a more strict check against poisoned ancestor chains (that is, an ancestor chain that looks like 'example.com' -> 'evil.com' -> 'example.com').

Mozilla is considering changing 'SAMEORIGIN's behavior to block the
latter loophole[1], and the UI Safety spec is considering dropping
'top-only' entirely[2]. This data will inform those decisions.

[1]: ​https://bugzilla.mozilla.org/show_bug.cgi?id=725490
[2]: ​http://lists.w3.org/Archives/Public/public-webappsec/2013Mar/0007.html

This doesn't change web-visible behavior; it only adds histograms

• loader/FrameLoader.cpp:

(WebCore::FrameLoader::shouldInterruptLoadForXFrameOptions):

When processing an 'X-Frame-Options' header's value, call out to
FeatureObserver when relevant to increment the correct histogram
entries.

• page/FeatureObserver.h:

Added three entries to the FeatureObserver enum: XFrameOptions,
XFrameOptionsSameOrigin, and XFrameOptionsSameOriginWithBadAncestorChain.
Each is explained above.

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• loader/FrameLoader.cpp (modified) (2 diffs)
• page/FeatureObserver.h (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2013-03-19  Mike West  <mkwst@chromium.org>
+
+        FeatureObserver: Measure X-Frame-Options usage.
+        https://bugs.webkit.org/show_bug.cgi?id=112670
+
+        Reviewed by Adam Barth.
+
+        This patch adds three FeatureObserver entries to gather information
+        about 'X-Frame-Options' usage in general, and in particular usage
+        and potential misunderstanding of the 'SAMEORIGIN' value.
+
+        Three entries are added:
+
+        - XFrameOptions measures the raw number of 'X-Frame-Options' headers.
+
+        - XFrameOptionsSameOrigin measures the number of those headers that
+          set the value to 'SAMEORIGIN'.
+
+        - XFrameOptionsSameOriginWithBadAncestorChain measures the number of
+          occasions in which the frame passed the "top-only" origin check we're
+          currently performing, but would have failed a more strict check
+          against poisoned ancestor chains (that is, an ancestor chain that
+          looks like 'example.com' -> 'evil.com' -> 'example.com').
+
+        Mozilla is considering changing 'SAMEORIGIN's behavior to block the
+        latter loophole[1], and the UI Safety spec is considering dropping
+        'top-only' entirely[2]. This data will inform those decisions.
+
+        [1]: https://bugzilla.mozilla.org/show_bug.cgi?id=725490
+        [2]: http://lists.w3.org/Archives/Public/public-webappsec/2013Mar/0007.html
+
+        This doesn't change web-visible behavior; it only adds histograms
+
+        * loader/FrameLoader.cpp:
+        (WebCore::FrameLoader::shouldInterruptLoadForXFrameOptions):
+            When processing an 'X-Frame-Options' header's value, call out to
+            FeatureObserver when relevant to increment the correct histogram
+            entries.
+        * page/FeatureObserver.h:
+            Added three entries to the FeatureObserver enum: XFrameOptions,
+            XFrameOptionsSameOrigin, and XFrameOptionsSameOriginWithBadAncestorChain.
+            Each is explained above.
+
 2013-03-19  Adam Barth  <abarth@webkit.org>
 

trunk/Source/WebCore/loader/FrameLoader.cpp

@@ -2951 +2951 @@
 bool FrameLoader::shouldInterruptLoadForXFrameOptions(const String& content, const KURL& url, unsigned long requestIdentifier)
 {
+    FeatureObserver::observe(m_frame->document(), FeatureObserver::XFrameOptions);
+
     Frame* topFrame = m_frame->tree()->top();
     if (m_frame == topFrame)
@@ -2958 +2960 @@
         return true;
     else if (equalIgnoringCase(content, "sameorigin")) {
+        FeatureObserver::observe(m_frame->document(), FeatureObserver::XFrameOptionsSameOrigin);
         RefPtr<SecurityOrigin> origin = SecurityOrigin::create(url);
         if (!origin->isSameSchemeHostPort(topFrame->document()->securityOrigin()))
             return true;
+        for (Frame* frame = m_frame->tree()->parent(); frame; frame = frame->tree()->parent()) {
+            if (!origin->isSameSchemeHostPort(frame->document()->securityOrigin())) {
+                FeatureObserver::observe(m_frame->document(), FeatureObserver::XFrameOptionsSameOriginWithBadAncestorChain);
+                break;
+            }
+        }
     } else if (!equalIgnoringCase(content, "allowall")) {
         String message = "Invalid 'X-Frame-Options' header encountered when loading '" + url.elidedString() + "': '" + content + "' is not a recognized directive. The header will be ignored.";

trunk/Source/WebCore/page/FeatureObserver.h

@@ -102 +102 @@
         CursorVisibility,
         StorageInfo,
+        XFrameOptions,
+        XFrameOptionsSameOrigin,
+        XFrameOptionsSameOriginWithBadAncestorChain,
         // Add new features above this line. Don't change assigned numbers of each items.
         NumberOfFeatures, // This enum value must be last.