- Revision
- 204521
- Author
- [email protected]
- Date
- 2016-08-16 13:32:57 -0700 (Tue, 16 Aug 2016)
Log Message
Upgrade-Insecure-Request state is improperly retained between navigations
https://bugs.webkit.org/show_bug.cgi?id=160905
<rdar://problem/27075526>
Reviewed by Andy Estes.
Source/WebCore:
Correct the handling of Upgrade-Insecure-Request state to match the specification, so that
performing top-level navigation to sites that do not have the Upgrade-Insecure-Request header
does not automatically upgrade insecure loads. The same loads performed in an iframe should
be upgraded.
The iframe case was already handled in our tests, but a new test is added that models the top-level
navigation and confirms that an upgrade is not performed.
Tests: http/tests/security/contentSecurityPolicy/upgrade-insecure-requests/proper-uir-on-navigation.html
* dom/Document.cpp:
(WebCore::Document::initContentSecurityPolicy): Properly inherit Upgrade-Insecure-Request state for children
of existing frames.
* loader/DocumentWriter.cpp:
(WebCore::DocumentWriter::begin): Retain the history of upgraded resources (per the specification) so that
we continue to upgrade resources that were upgraded during earlier navigations. Note that we do NOT want to
retain the state of the Upgrade-Insecure-Requests header itself.
* page/csp/ContentSecurityPolicy.cpp:
(WebCore::ContentSecurityPolicy::copyStateFrom): Update to use new helper function.
(WebCore::ContentSecurityPolicy::copyUpgradeInsecureRequestStateFrom): New helper function.
* page/csp/ContentSecurityPolicy.h:
LayoutTests:
* http/tests/security/contentSecurityPolicy/upgrade-insecure-requests/proper-uir-on-navigation-expected.txt: Added.
* http/tests/security/contentSecurityPolicy/upgrade-insecure-requests/proper-uir-on-navigation.html: Added.
* http/tests/security/contentSecurityPolicy/upgrade-insecure-requests/resources/insecure-site.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (204520 => 204521)
--- trunk/LayoutTests/ChangeLog 2016-08-16 19:57:00 UTC (rev 204520)
+++ trunk/LayoutTests/ChangeLog 2016-08-16 20:32:57 UTC (rev 204521)
@@ -1,3 +1,15 @@
+2016-08-16 Brent Fulgham <[email protected]>
+
+ Upgrade-Insecure-Request state is improperly retained between navigations
+ https://bugs.webkit.org/show_bug.cgi?id=160905
+ <rdar://problem/27075526>
+
+ Reviewed by Andy Estes.
+
+ * http/tests/security/contentSecurityPolicy/upgrade-insecure-requests/proper-uir-on-navigation-expected.txt: Added.
+ * http/tests/security/contentSecurityPolicy/upgrade-insecure-requests/proper-uir-on-navigation.html: Added.
+ * http/tests/security/contentSecurityPolicy/upgrade-insecure-requests/resources/insecure-site.html: Added.
+
2016-08-16 Chris Dumez <[email protected]>
ctx.drawImage should clip source rect if it is outside the source image
Added: trunk/LayoutTests/http/tests/security/contentSecurityPolicy/upgrade-insecure-requests/proper-uir-on-navigation-expected.txt (0 => 204521)
--- trunk/LayoutTests/http/tests/security/contentSecurityPolicy/upgrade-insecure-requests/proper-uir-on-navigation-expected.txt (rev 0)
+++ trunk/LayoutTests/http/tests/security/contentSecurityPolicy/upgrade-insecure-requests/proper-uir-on-navigation-expected.txt 2016-08-16 20:32:57 UTC (rev 204521)
@@ -0,0 +1,14 @@
+ALERT: PASS
+ALERT: PASS
+This page should be loaded insecurely as a navigation from the secure site 'secure-second-site.html'. The secure domain should have set the Upgrade-Insecure-Requests header, but this new navigation should have cleared this state.
+
+The following script is loaded using 'http', and should NOT be upgraded to 'https'.
+
+If this test passes there should be two PASS messages in the output.
+
+
+
+============== Back Forward List ==============
+ http://127.0.0.1:8000/security/contentSecurityPolicy/upgrade-insecure-requests/proper-uir-on-navigation.html **nav target**
+curr-> http://127.0.0.1:8000/security/contentSecurityPolicy/upgrade-insecure-requests/resources/insecure-site.html **nav target**
+===============================================
Added: trunk/LayoutTests/http/tests/security/contentSecurityPolicy/upgrade-insecure-requests/proper-uir-on-navigation.html (0 => 204521)
--- trunk/LayoutTests/http/tests/security/contentSecurityPolicy/upgrade-insecure-requests/proper-uir-on-navigation.html (rev 0)
+++ trunk/LayoutTests/http/tests/security/contentSecurityPolicy/upgrade-insecure-requests/proper-uir-on-navigation.html 2016-08-16 20:32:57 UTC (rev 204521)
@@ -0,0 +1,21 @@
+<!DOCTYPE html>
+<html>
+<head>
+ <meta http-equiv="Content-Security-Policy" content="upgrade-insecure-requests">
+</head>
+<body>
+ <script>
+ if (window.testRunner)
+ testRunner.dumpAsText();
+ </script>
+ <div>
+ <p>The following script is loaded using 'http', but it should be upgraded to 'https' due to the
+'upgrade-insecure-requests' header.</p>
+ <script src=""
+ <div>
+ <script>
+ if (window.testRunner)
+ testRunner.queueLoad("http://127.0.0.1:8000/security/contentSecurityPolicy/upgrade-insecure-requests/resources/insecure-site.html");
+ </script>
+</body>
+</html>
\ No newline at end of file
Added: trunk/LayoutTests/http/tests/security/contentSecurityPolicy/upgrade-insecure-requests/resources/insecure-site.html (0 => 204521)
--- trunk/LayoutTests/http/tests/security/contentSecurityPolicy/upgrade-insecure-requests/resources/insecure-site.html (rev 0)
+++ trunk/LayoutTests/http/tests/security/contentSecurityPolicy/upgrade-insecure-requests/resources/insecure-site.html 2016-08-16 20:32:57 UTC (rev 204521)
@@ -0,0 +1,16 @@
+<!DOCTYPE html>
+<html>
+<body>
+ <p>This page should be loaded insecurely as a navigation from the secure site 'secure-second-site.html'. The secure domain should have set the Upgrade-Insecure-Requests header,
+ but this new navigation should have cleared this state.</p>
+ <div>
+ <p>The following script is loaded using 'http', and should NOT be upgraded to 'https'.</p>
+ <script src=""
+ <p>If this test passes there should be two PASS messages in the output.</p>
+ <div>
+ <script>
+ if (window.testRunner)
+ testRunner.dumpBackForwardList();
+ </script>
+</body>
+</html>
\ No newline at end of file
Modified: trunk/Source/WebCore/ChangeLog (204520 => 204521)
--- trunk/Source/WebCore/ChangeLog 2016-08-16 19:57:00 UTC (rev 204520)
+++ trunk/Source/WebCore/ChangeLog 2016-08-16 20:32:57 UTC (rev 204521)
@@ -1,3 +1,33 @@
+2016-08-16 Brent Fulgham <[email protected]>
+
+ Upgrade-Insecure-Request state is improperly retained between navigations
+ https://bugs.webkit.org/show_bug.cgi?id=160905
+ <rdar://problem/27075526>
+
+ Reviewed by Andy Estes.
+
+ Correct the handling of Upgrade-Insecure-Request state to match the specification, so that
+ performing top-level navigation to sites that do not have the Upgrade-Insecure-Request header
+ does not automatically upgrade insecure loads. The same loads performed in an iframe should
+ be upgraded.
+
+ The iframe case was already handled in our tests, but a new test is added that models the top-level
+ navigation and confirms that an upgrade is not performed.
+
+ Tests: http/tests/security/contentSecurityPolicy/upgrade-insecure-requests/proper-uir-on-navigation.html
+
+ * dom/Document.cpp:
+ (WebCore::Document::initContentSecurityPolicy): Properly inherit Upgrade-Insecure-Request state for children
+ of existing frames.
+ * loader/DocumentWriter.cpp:
+ (WebCore::DocumentWriter::begin): Retain the history of upgraded resources (per the specification) so that
+ we continue to upgrade resources that were upgraded during earlier navigations. Note that we do NOT want to
+ retain the state of the Upgrade-Insecure-Requests header itself.
+ * page/csp/ContentSecurityPolicy.cpp:
+ (WebCore::ContentSecurityPolicy::copyStateFrom): Update to use new helper function.
+ (WebCore::ContentSecurityPolicy::copyUpgradeInsecureRequestStateFrom): New helper function.
+ * page/csp/ContentSecurityPolicy.h:
+
2016-08-16 Chris Dumez <[email protected]>
ctx.drawImage should clip source rect if it is outside the source image
Modified: trunk/Source/WebCore/dom/Document.cpp (204520 => 204521)
--- trunk/Source/WebCore/dom/Document.cpp 2016-08-16 19:57:00 UTC (rev 204520)
+++ trunk/Source/WebCore/dom/Document.cpp 2016-08-16 20:32:57 UTC (rev 204521)
@@ -5389,9 +5389,18 @@
void Document::initContentSecurityPolicy()
{
- if (!m_frame->tree().parent() || (!shouldInheritSecurityOriginFromOwner(m_url) && !isPluginDocument()))
+ if (!m_frame->tree().parent())
return;
+ if (!shouldInheritSecurityOriginFromOwner(m_url) && !isPluginDocument()) {
+ // Per <http://www.w3.org/TR/upgrade-insecure-requests/>, we need to retain an ongoing set of upgraded
+ // requests in new navigation contexts. Although this information is present when we construct the
+ // Document object, it is discard in the subsequent 'clear' statements below. So, we must capture it
+ ASSERT(m_frame->tree().parent()->document());
+ contentSecurityPolicy()->copyUpgradeInsecureRequestStateFrom(*m_frame->tree().parent()->document()->contentSecurityPolicy());
+ return;
+ }
+
contentSecurityPolicy()->copyStateFrom(m_frame->tree().parent()->document()->contentSecurityPolicy());
}
Modified: trunk/Source/WebCore/loader/DocumentWriter.cpp (204520 => 204521)
--- trunk/Source/WebCore/loader/DocumentWriter.cpp 2016-08-16 19:57:00 UTC (rev 204520)
+++ trunk/Source/WebCore/loader/DocumentWriter.cpp 2016-08-16 20:32:57 UTC (rev 204521)
@@ -1,5 +1,6 @@
/*
* Copyright (C) 2010. Adam Barth. All rights reserved.
+ * Copyright (C) 2016 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -149,11 +150,8 @@
// Document object, it is discard in the subsequent 'clear' statements below. So, we must capture it
// so we can restore it.
HashSet<RefPtr<SecurityOrigin>> insecureNavigationRequestsToUpgrade;
- bool upgradeInsecureRequests = false;
- if (auto* existingDocument = m_frame->document()) {
- upgradeInsecureRequests = existingDocument->contentSecurityPolicy()->upgradeInsecureRequests();
+ if (auto* existingDocument = m_frame->document())
insecureNavigationRequestsToUpgrade = existingDocument->contentSecurityPolicy()->takeNavigationRequestsToUpgrade();
- }
m_frame->loader().clear(document.ptr(), !shouldReuseDefaultView, !shouldReuseDefaultView);
clear();
@@ -169,7 +167,6 @@
m_frame->loader().setOutgoingReferrer(url);
m_frame->setDocument(document.copyRef());
- document->contentSecurityPolicy()->setUpgradeInsecureRequests(upgradeInsecureRequests);
document->contentSecurityPolicy()->setInsecureNavigationRequestsToUpgrade(WTFMove(insecureNavigationRequestsToUpgrade));
if (m_decoder)
Modified: trunk/Source/WebCore/page/csp/ContentSecurityPolicy.cpp (204520 => 204521)
--- trunk/Source/WebCore/page/csp/ContentSecurityPolicy.cpp 2016-08-16 19:57:00 UTC (rev 204520)
+++ trunk/Source/WebCore/page/csp/ContentSecurityPolicy.cpp 2016-08-16 20:32:57 UTC (rev 204521)
@@ -114,10 +114,15 @@
for (auto& policy : other->m_policies)
didReceiveHeader(policy->header(), policy->headerType(), ContentSecurityPolicy::PolicyFrom::Inherited);
- m_upgradeInsecureRequests = other->m_upgradeInsecureRequests;
- m_insecureNavigationRequestsToUpgrade.add(other->m_insecureNavigationRequestsToUpgrade.begin(), other->m_insecureNavigationRequestsToUpgrade.end());
+ copyUpgradeInsecureRequestStateFrom(*other);
}
+void ContentSecurityPolicy::copyUpgradeInsecureRequestStateFrom(const ContentSecurityPolicy& other)
+{
+ m_upgradeInsecureRequests = other.m_upgradeInsecureRequests;
+ m_insecureNavigationRequestsToUpgrade.add(other.m_insecureNavigationRequestsToUpgrade.begin(), other.m_insecureNavigationRequestsToUpgrade.end());
+}
+
void ContentSecurityPolicy::didCreateWindowShell(JSDOMWindowShell& windowShell) const
{
JSDOMWindow* window = windowShell.window();
Modified: trunk/Source/WebCore/page/csp/ContentSecurityPolicy.h (204520 => 204521)
--- trunk/Source/WebCore/page/csp/ContentSecurityPolicy.h 2016-08-16 19:57:00 UTC (rev 204520)
+++ trunk/Source/WebCore/page/csp/ContentSecurityPolicy.h 2016-08-16 20:32:57 UTC (rev 204521)
@@ -69,6 +69,7 @@
~ContentSecurityPolicy();
void copyStateFrom(const ContentSecurityPolicy*);
+ void copyUpgradeInsecureRequestStateFrom(const ContentSecurityPolicy&);
void didCreateWindowShell(JSDOMWindowShell&) const;
