- Revision
- 222374
- Author
- [email protected]
- Date
- 2017-09-21 17:42:49 -0700 (Thu, 21 Sep 2017)
Log Message
REGRESSION (r221017): iCloud mail logs me out after looking at a few messages
https://bugs.webkit.org/show_bug.cgi?id=177328
Reviewed by Daniel Bates.
<rdar://problem/34288629>
Reviewed by Dan Bates.
Source/WebCore:
This site was triggering a log-out because the page was loading insecure images. We don't treat that as a security
issue for deciding to display mixed content since it can only affect pixels on screen, not trigger a change in
program logic or persistent storage.
Consequently, we can correct thsi compatibility problem without relaxing the security fix by not blocking Secure cookies
when we have merely displayed mixed content (i.e., encountered 'Inactive' mixed content), as opposed to executing mixed
content (i.e., loaded 'Active' mixed content).
* Modules/geolocation/Geolocation.cpp:
(WebCore::Geolocation::shouldBlockGeolocationRequests): Revise for new API.
* dom/SecurityContext.h:
(WebCore::SecurityContext::foundMixedContent const): Change to return an OptionSet of
mixed content types.
(WebCore::SecurityContext::setFoundMixedContent): Accept an enum stating the type of mixed content found.
* loader/CookieJar.cpp:
(WebCore::cookies): Only block Secure cookies for Active mixed content.
(WebCore::cookieRequestHeaderFieldValue): Ditto.
* loader/MixedContentChecker.cpp:
(WebCore::MixedContentChecker::canDisplayInsecureContent const): Mark the context as having found 'Inactive'
mixed content.
(WebCore::MixedContentChecker::canRunInsecureContent const): Mark the context as having found 'Active'
mixed content.
LayoutTests:
Rebaseline tests for revised behavior.
* http/tests/security/mixedContent/insecure-image-with-securecookie-block-expected.txt:
* http/tests/security/mixedContent/insecure-image-with-securecookie-block.html:
* http/tests/security/mixedContent/redirect-https-to-http-image-secure-cookies-block-expected.txt:
Modified Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (222373 => 222374)
--- trunk/LayoutTests/ChangeLog 2017-09-22 00:40:47 UTC (rev 222373)
+++ trunk/LayoutTests/ChangeLog 2017-09-22 00:42:49 UTC (rev 222374)
@@ -1,3 +1,19 @@
+2017-09-21 Brent Fulgham <[email protected]>
+
+ REGRESSION (r221017): iCloud mail logs me out after looking at a few messages
+ https://bugs.webkit.org/show_bug.cgi?id=177328
+
+ Reviewed by Daniel Bates.
+ <rdar://problem/34288629>
+
+ Reviewed by Dan Bates.
+
+ Rebaseline tests for revised behavior.
+
+ * http/tests/security/mixedContent/insecure-image-with-securecookie-block-expected.txt:
+ * http/tests/security/mixedContent/insecure-image-with-securecookie-block.html:
+ * http/tests/security/mixedContent/redirect-https-to-http-image-secure-cookies-block-expected.txt:
+
2017-09-21 Ryosuke Niwa <[email protected]>
DataTransfer.items should contain text/html and text/uri-list
Modified: trunk/LayoutTests/http/tests/security/mixedContent/insecure-image-with-securecookie-block-expected.txt (222373 => 222374)
--- trunk/LayoutTests/http/tests/security/mixedContent/insecure-image-with-securecookie-block-expected.txt 2017-09-22 00:40:47 UTC (rev 222373)
+++ trunk/LayoutTests/http/tests/security/mixedContent/insecure-image-with-securecookie-block-expected.txt 2017-09-22 00:42:49 UTC (rev 222374)
@@ -1,4 +1,4 @@
CONSOLE MESSAGE: line 4: The page at https://127.0.0.1:8443/security/mixedContent/resources/frame-with-insecure-image-secure-cookie-block.html was allowed to display insecure content from http://127.0.0.1:8080/security/resources/compass.jpg.
-CONSOLE MESSAGE: line 6:
-This test opens a window that tries to read a secure cookie after an insecure image has been loaded. This should block reading of the secure cookie since insecure content was loaded on this page.
+CONSOLE MESSAGE: line 6: secureCookie=yes
+This test opens a window that tries to read a secure cookie after an insecure image has been loaded. This should allow reading of the secure cookie since only 'inactive' insecure content was loaded on this page.
Modified: trunk/LayoutTests/http/tests/security/mixedContent/insecure-image-with-securecookie-block.html (222373 => 222374)
--- trunk/LayoutTests/http/tests/security/mixedContent/insecure-image-with-securecookie-block.html 2017-09-22 00:40:47 UTC (rev 222373)
+++ trunk/LayoutTests/http/tests/security/mixedContent/insecure-image-with-securecookie-block.html 2017-09-22 00:42:49 UTC (rev 222374)
@@ -15,7 +15,7 @@
}, false);
</script>
-<p>This test opens a window that tries to read a secure cookie after an insecure image has been loaded. This should block reading of the secure cookie since insecure content was loaded on this page.</p>
+<p>This test opens a window that tries to read a secure cookie after an insecure image has been loaded. This should allow reading of the secure cookie since only 'inactive' insecure content was loaded on this page.</p>
<script>
_onload_ = function() {
window.open("https://127.0.0.1:8443/security/mixedContent/resources/frame-with-insecure-image-secure-cookie-block.html");
Modified: trunk/LayoutTests/http/tests/security/mixedContent/redirect-https-to-http-image-secure-cookies-block-expected.txt (222373 => 222374)
--- trunk/LayoutTests/http/tests/security/mixedContent/redirect-https-to-http-image-secure-cookies-block-expected.txt 2017-09-22 00:40:47 UTC (rev 222373)
+++ trunk/LayoutTests/http/tests/security/mixedContent/redirect-https-to-http-image-secure-cookies-block-expected.txt 2017-09-22 00:42:49 UTC (rev 222374)
@@ -7,7 +7,7 @@
CONSOLE MESSAGE: The page at https://127.0.0.1:8443/security/mixedContent/resources/frame-with-redirect-https-to-http-image-secure-cookie-block.html was allowed to display insecure content from http://127.0.0.1:8080/security/resources/compass.jpg.
didDisplayInsecureContent
-CONSOLE MESSAGE: line 8:
+CONSOLE MESSAGE: line 8: secureCookie=yes
main frame - didHandleOnloadEventsForFrame
main frame - didFinishLoadForFrame
This test opens a window that loads an insecure image (via a tricky redirect) and then tries to read a secure cookie. This should block the secure cookie from being read because insecure content was loaded while loading a main frame.
Modified: trunk/Source/WebCore/ChangeLog (222373 => 222374)
--- trunk/Source/WebCore/ChangeLog 2017-09-22 00:40:47 UTC (rev 222373)
+++ trunk/Source/WebCore/ChangeLog 2017-09-22 00:42:49 UTC (rev 222374)
@@ -1,3 +1,36 @@
+2017-09-21 Brent Fulgham <[email protected]>
+
+ REGRESSION (r221017): iCloud mail logs me out after looking at a few messages
+ https://bugs.webkit.org/show_bug.cgi?id=177328
+
+ Reviewed by Daniel Bates.
+ <rdar://problem/34288629>
+
+ Reviewed by Dan Bates.
+
+ This site was triggering a log-out because the page was loading insecure images. We don't treat that as a security
+ issue for deciding to display mixed content since it can only affect pixels on screen, not trigger a change in
+ program logic or persistent storage.
+
+ Consequently, we can correct thsi compatibility problem without relaxing the security fix by not blocking Secure cookies
+ when we have merely displayed mixed content (i.e., encountered 'Inactive' mixed content), as opposed to executing mixed
+ content (i.e., loaded 'Active' mixed content).
+
+ * Modules/geolocation/Geolocation.cpp:
+ (WebCore::Geolocation::shouldBlockGeolocationRequests): Revise for new API.
+ * dom/SecurityContext.h:
+ (WebCore::SecurityContext::foundMixedContent const): Change to return an OptionSet of
+ mixed content types.
+ (WebCore::SecurityContext::setFoundMixedContent): Accept an enum stating the type of mixed content found.
+ * loader/CookieJar.cpp:
+ (WebCore::cookies): Only block Secure cookies for Active mixed content.
+ (WebCore::cookieRequestHeaderFieldValue): Ditto.
+ * loader/MixedContentChecker.cpp:
+ (WebCore::MixedContentChecker::canDisplayInsecureContent const): Mark the context as having found 'Inactive'
+ mixed content.
+ (WebCore::MixedContentChecker::canRunInsecureContent const): Mark the context as having found 'Active'
+ mixed content.
+
2017-09-21 Simon Fraser <[email protected]>
Clean up RenderLayer z-order traversal code
Modified: trunk/Source/WebCore/Modules/geolocation/Geolocation.cpp (222373 => 222374)
--- trunk/Source/WebCore/Modules/geolocation/Geolocation.cpp 2017-09-22 00:40:47 UTC (rev 222373)
+++ trunk/Source/WebCore/Modules/geolocation/Geolocation.cpp 2017-09-22 00:42:49 UTC (rev 222374)
@@ -361,7 +361,7 @@
bool Geolocation::shouldBlockGeolocationRequests()
{
bool isSecure = SecurityOrigin::isSecure(document()->url());
- bool hasMixedContent = document()->foundMixedContent();
+ bool hasMixedContent = !document()->foundMixedContent().isEmpty();
bool isLocalOrigin = securityOrigin()->isLocal();
if (securityOrigin()->canRequestGeolocation()) {
if (isLocalOrigin || (isSecure && !hasMixedContent) || isRequestFromIBooks())
Modified: trunk/Source/WebCore/dom/SecurityContext.h (222373 => 222374)
--- trunk/Source/WebCore/dom/SecurityContext.h 2017-09-22 00:40:47 UTC (rev 222373)
+++ trunk/Source/WebCore/dom/SecurityContext.h 2017-09-22 00:42:49 UTC (rev 222374)
@@ -29,6 +29,7 @@
#include <memory>
#include <wtf/Forward.h>
+#include <wtf/OptionSet.h>
#include <wtf/RefPtr.h>
namespace WebCore {
@@ -82,8 +83,13 @@
static SandboxFlags parseSandboxPolicy(const String& policy, String& invalidTokensErrorMessage);
static bool isSupportedSandboxPolicy(StringView);
- bool foundMixedContent() const { return m_foundMixedContent; }
- void setFoundMixedContent() { m_foundMixedContent = true; }
+ enum MixedContentType {
+ Inactive = 1 << 0,
+ Active = 1 << 1,
+ };
+
+ const OptionSet<MixedContentType>& foundMixedContent() const { return m_mixedContentTypes; }
+ void setFoundMixedContent(MixedContentType type) { m_mixedContentTypes |= type; }
bool geolocationAccessed() const { return m_geolocationAccessed; }
void setGeolocationAccessed() { m_geolocationAccessed = true; }
bool secureCookiesAccessed() const { return m_secureCookiesAccessed; }
@@ -113,8 +119,8 @@
RefPtr<SecurityOriginPolicy> m_securityOriginPolicy;
std::unique_ptr<ContentSecurityPolicy> m_contentSecurityPolicy;
SandboxFlags m_sandboxFlags { SandboxNone };
+ OptionSet<MixedContentType> m_mixedContentTypes;
bool m_haveInitializedSecurityOrigin { false };
- bool m_foundMixedContent { false };
bool m_geolocationAccessed { false };
bool m_secureCookiesAccessed { false };
bool m_isStrictMixedContentMode { false };
Modified: trunk/Source/WebCore/loader/CookieJar.cpp (222373 => 222374)
--- trunk/Source/WebCore/loader/CookieJar.cpp 2017-09-22 00:40:47 UTC (rev 222373)
+++ trunk/Source/WebCore/loader/CookieJar.cpp 2017-09-22 00:42:49 UTC (rev 222374)
@@ -58,7 +58,7 @@
{
TraceScope scope(FetchCookiesStart, FetchCookiesEnd);
- auto includeSecureCookies = (url.protocolIs("https") && !document.foundMixedContent()) ? IncludeSecureCookies::Yes : IncludeSecureCookies::No;
+ auto includeSecureCookies = (url.protocolIs("https") && !document.foundMixedContent().contains(SecurityContext::MixedContentType::Active)) ? IncludeSecureCookies::Yes : IncludeSecureCookies::No;
auto result = platformStrategies()->cookiesStrategy()->cookiesForDOM(storageSession(document), document.firstPartyForCookies(), url, includeSecureCookies);
if (result.second)
document.setSecureCookiesAccessed();
@@ -78,7 +78,7 @@
String cookieRequestHeaderFieldValue(Document& document, const URL& url)
{
- auto includeSecureCookies = (url.protocolIs("https") && !document.foundMixedContent()) ? IncludeSecureCookies::Yes : IncludeSecureCookies::No;
+ auto includeSecureCookies = (url.protocolIs("https") && !document.foundMixedContent().contains(SecurityContext::MixedContentType::Active)) ? IncludeSecureCookies::Yes : IncludeSecureCookies::No;
auto result = platformStrategies()->cookiesStrategy()->cookieRequestHeaderFieldValue(storageSession(document), document.firstPartyForCookies(), url, includeSecureCookies);
if (result.second)
document.setSecureCookiesAccessed();
Modified: trunk/Source/WebCore/loader/MixedContentChecker.cpp (222373 => 222374)
--- trunk/Source/WebCore/loader/MixedContentChecker.cpp 2017-09-22 00:40:47 UTC (rev 222373)
+++ trunk/Source/WebCore/loader/MixedContentChecker.cpp 2017-09-22 00:42:49 UTC (rev 222374)
@@ -78,7 +78,7 @@
logWarning(allowed, "display", url);
if (allowed) {
- m_frame.document()->setFoundMixedContent();
+ m_frame.document()->setFoundMixedContent(SecurityContext::MixedContentType::Inactive);
client().didDisplayInsecureContent();
}
@@ -97,7 +97,7 @@
logWarning(allowed, "run", url);
if (allowed) {
- m_frame.document()->setFoundMixedContent();
+ m_frame.document()->setFoundMixedContent(SecurityContext::MixedContentType::Active);
client().didRunInsecureContent(securityOrigin, url);
}
