Sites served over insecure connections should not be allowed to use geolocation.
authorbfulgham@apple.com <bfulgham@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 26 May 2016 18:19:30 +0000 (18:19 +0000)
committerbfulgham@apple.com <bfulgham@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 26 May 2016 18:19:30 +0000 (18:19 +0000)
https://bugs.webkit.org/show_bug.cgi?id=157423
<rdar://problem/23751632>

Patch by Pranjal Jumde <pjumde@apple.com> on 2016-05-26
Reviewed by Brent Fulgham.

Source/WebCore:

Tests: http/tests/security/insecure-geolocation.html
       http/tests/security/mixedcontent-geolocation-block-insecure-content.html
       http/tests/security/mixedcontent-geolocation.html

* Modules/geolocation/Geolocation.cpp:
(WebCore::logError):
Logs an error to the console if geolocation is blocked.
(WebCore::Geolocation::startRequest):
Access to Geolocation will be blocked if site is not secure. An error will be logged when access to Geolocation is blocked.
(WebCore::Geolocation::shouldBlockGeolocationRequests)
Returns true if the access to geolocation should be blocked.
* Modules/geolocation/Geolocation.h:
* dom/SecurityContext.h:
(WebCore::SecurityContext::foundMixedContent):
Returns true if insecure content was accessed over secure connection.
(WebCore::SecurityContext::setFoundMixedContent):
Sets m_foundMixedContent to true if insecure content is accessed over secure connection.
(WebCore::SecurityContext::geolocationAccessed):
Returns true if geolocation was accessed
(WebCore::SecurityContext::setGeolocationAccessed):
Sets m_geolocationAccessed to true if geolocation was accessed.
* loader/MixedContentChecker.cpp:
(WebCore::MixedContentChecker::canDisplayInsecureContent):
Insecure content will be blocked if geolocation was accessed by the page. Updates document to keep track of mixed content.
(WebCore::MixedContentChecker::canRunInsecureContent):
Insecure content will be blocked if geolocation was accessed by the page. Updates document to keep track of mixed content.

LayoutTests:

* http/tests/security/geolocation-over-insecure-content.html: Added.
* http/tests/security/geolocation-over-mixed-content-block.html: Added.
* http/tests/security/geolocation-over-mixed-content.html: Added.
* http/tests/security/insecure-geolocation-expected.txt: Added.
* http/tests/security/insecure-geolocation.html: Added.
* http/tests/security/mixedcontent-geolocation-block-insecure-content-expected.txt: Added.
* http/tests/security/mixedcontent-geolocation-block-insecure-content.html: Added.
* http/tests/security/mixedcontent-geolocation-expected.txt: Added.
* http/tests/security/mixedcontent-geolocation.html: Added.
* http/tests/security/sandboxed-iframe-geolocation-watchPosition.html:
  iframe is loaded over secure connection to avoid geolocation failures
* http/tests/security/sandboxed-iframe-geolocation-getCurrentPosition.html:
  iframe is loaded over secure connection to avoid geolocation failures

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@201423 268f45cc-cd09-0410-ab3c-d52691b4dbfc

14 files changed:
LayoutTests/ChangeLog
LayoutTests/http/tests/security/insecure-geolocation-expected.txt [new file with mode: 0644]
LayoutTests/http/tests/security/insecure-geolocation.html [new file with mode: 0644]
LayoutTests/http/tests/security/mixedcontent-geolocation-block-insecure-content-expected.txt [new file with mode: 0644]
LayoutTests/http/tests/security/mixedcontent-geolocation-block-insecure-content.html [new file with mode: 0644]
LayoutTests/http/tests/security/mixedcontent-geolocation-expected.txt [new file with mode: 0644]
LayoutTests/http/tests/security/mixedcontent-geolocation.html [new file with mode: 0644]
LayoutTests/http/tests/security/sandboxed-iframe-geolocation-getCurrentPosition.html
LayoutTests/http/tests/security/sandboxed-iframe-geolocation-watchPosition.html
Source/WebCore/ChangeLog
Source/WebCore/Modules/geolocation/Geolocation.cpp
Source/WebCore/Modules/geolocation/Geolocation.h
Source/WebCore/dom/SecurityContext.h
Source/WebCore/loader/MixedContentChecker.cpp

index 2f437b8..b822820 100644 (file)
@@ -1,3 +1,25 @@
+2016-05-26  Pranjal Jumde  <pjumde@apple.com>
+
+        Sites served over insecure connections should not be allowed to use geolocation.
+        https://bugs.webkit.org/show_bug.cgi?id=157423
+        <rdar://problem/23751632>
+
+        Reviewed by Brent Fulgham.
+
+        * http/tests/security/geolocation-over-insecure-content.html: Added.
+        * http/tests/security/geolocation-over-mixed-content-block.html: Added.
+        * http/tests/security/geolocation-over-mixed-content.html: Added.
+        * http/tests/security/insecure-geolocation-expected.txt: Added.
+        * http/tests/security/insecure-geolocation.html: Added.
+        * http/tests/security/mixedcontent-geolocation-block-insecure-content-expected.txt: Added.
+        * http/tests/security/mixedcontent-geolocation-block-insecure-content.html: Added.
+        * http/tests/security/mixedcontent-geolocation-expected.txt: Added.
+        * http/tests/security/mixedcontent-geolocation.html: Added.
+        * http/tests/security/sandboxed-iframe-geolocation-watchPosition.html:
+          iframe is loaded over secure connection to avoid geolocation failures
+        * http/tests/security/sandboxed-iframe-geolocation-getCurrentPosition.html: 
+          iframe is loaded over secure connection to avoid geolocation failures
+
 2016-05-26  Brady Eidson  <beidson@apple.com>
 
         Implement internals.observeGC to get called back when a Javascript object is GC'ed.
diff --git a/LayoutTests/http/tests/security/insecure-geolocation-expected.txt b/LayoutTests/http/tests/security/insecure-geolocation-expected.txt
new file mode 100644 (file)
index 0000000..99c08e7
--- /dev/null
@@ -0,0 +1,3 @@
+CONSOLE MESSAGE: line 4: [blocked] Access to geolocation was blocked over insecure connection to http://127.0.0.1:8080.
+
+This test loads an insecure frame that tries to access geolocation. Access to geolocation is blocked over insecure connections.
diff --git a/LayoutTests/http/tests/security/insecure-geolocation.html b/LayoutTests/http/tests/security/insecure-geolocation.html
new file mode 100644 (file)
index 0000000..4c4a0b5
--- /dev/null
@@ -0,0 +1,22 @@
+<html>
+<body>
+<script>
+if (window.testRunner) {
+    testRunner.waitUntilDone();
+    testRunner.dumpAsText();
+    testRunner.setCanOpenWindows();
+    testRunner.setCloseRemainingWindowsWhenComplete(true);
+}
+window.addEventListener("message", function (e) {
+    if (window.testRunner)
+        testRunner.notifyDone();
+}, false);
+</script>
+<p>This test loads an insecure frame that tries to access geolocation.  Access to geolocation is blocked over insecure connections.</p>
+<script>
+onload = function() {
+    window.open("http://127.0.0.1:8080/security/resources/geolocation-over-insecure-content.html");
+}
+</script>
+</body>
+</html>
diff --git a/LayoutTests/http/tests/security/mixedcontent-geolocation-block-insecure-content-expected.txt b/LayoutTests/http/tests/security/mixedcontent-geolocation-block-insecure-content-expected.txt
new file mode 100644 (file)
index 0000000..7526f4d
--- /dev/null
@@ -0,0 +1,3 @@
+CONSOLE MESSAGE: line 14: [blocked] The page at https://127.0.0.1:8443/security/resources/geolocation-over-mixed-content-block.html was not allowed to display insecure content from http://127.0.0.1:8080/security/resources/compass.jpg.
+
+This test loads a secure frame with insecure content that tries to access geolocation before loading insecure content. Access to insecure content is blocked over secure connections when geolocation is accessed.
diff --git a/LayoutTests/http/tests/security/mixedcontent-geolocation-block-insecure-content.html b/LayoutTests/http/tests/security/mixedcontent-geolocation-block-insecure-content.html
new file mode 100644 (file)
index 0000000..7e0a1d3
--- /dev/null
@@ -0,0 +1,22 @@
+<html>
+<body>
+<script>
+if (window.testRunner) {
+    testRunner.waitUntilDone();
+    testRunner.dumpAsText();
+    testRunner.setCanOpenWindows();
+    testRunner.setCloseRemainingWindowsWhenComplete(true);
+}
+window.addEventListener("message", function (e) {
+    if (window.testRunner)
+        testRunner.notifyDone();
+}, false);
+</script>
+<p>This test loads a secure frame with insecure content that tries to access geolocation before loading insecure content.  Access to insecure content is blocked over secure connections when geolocation is accessed.</p>
+<script>
+onload = function() {
+    window.open("https://127.0.0.1:8443/security/resources/geolocation-over-mixed-content-block.html");
+}
+</script>
+</body>
+</html>
diff --git a/LayoutTests/http/tests/security/mixedcontent-geolocation-expected.txt b/LayoutTests/http/tests/security/mixedcontent-geolocation-expected.txt
new file mode 100644 (file)
index 0000000..62ba89c
--- /dev/null
@@ -0,0 +1,5 @@
+CONSOLE MESSAGE: line 2: The page at https://127.0.0.1:8443/security/resources/geolocation-over-mixed-content.html was allowed to display insecure content from http://127.0.0.1:8080/security/resources/compass.jpg.
+
+CONSOLE MESSAGE: line 5: [blocked] Access to geolocation was blocked over secure connection with mixed content to https://127.0.0.1:8443.
+
+This test loads a secure frame with mixed content that tries to access geolocation. Access to geolocation is blocked over secure connections with mixed content.
diff --git a/LayoutTests/http/tests/security/mixedcontent-geolocation.html b/LayoutTests/http/tests/security/mixedcontent-geolocation.html
new file mode 100644 (file)
index 0000000..1d25b26
--- /dev/null
@@ -0,0 +1,22 @@
+<html>
+<body>
+<script>
+if (window.testRunner) {
+    testRunner.waitUntilDone();
+    testRunner.dumpAsText();
+    testRunner.setCanOpenWindows();
+    testRunner.setCloseRemainingWindowsWhenComplete(true);
+}
+window.addEventListener("message", function (e) {
+    if (window.testRunner)
+        testRunner.notifyDone();
+}, false);
+</script>
+<p>This test loads a secure frame with mixed content that tries to access geolocation.  Access to geolocation is blocked over secure connections with mixed content.</p>
+<script>
+onload = function() {
+    window.open("https://127.0.0.1:8443/security/resources/geolocation-over-mixed-content.html");
+}
+</script>
+</body>
+</html>
index a1adcb8..05e834f 100644 (file)
@@ -11,6 +11,6 @@ if (window.testRunner) {
 </head>
 <body>
 <p>Tests that navigator.geolocation.getCurrentPosition() returns error POSITION_UNAVAILABLE when called from a document in a sandboxed iframe.</p>
-<iframe src="resources/sandboxed-iframe-geolocation-getCurrentPosition.html" sandbox="allow-scripts" width="100%" height="300"></iframe>
+<iframe src="https://127.0.0.1:8443/security/resources/sandboxed-iframe-geolocation-getCurrentPosition.html" sandbox="allow-scripts" width="100%" height="300"></iframe>
 </body>
 </html>
index ef26568..123fad1 100644 (file)
@@ -11,6 +11,6 @@ if (window.testRunner) {
 </head>
 <body>
 <p>Tests that navigator.geolocation.watchPosition() returns error POSITION_UNAVAILABLE when called from a document in a sandboxed iframe.</p>
-<iframe src="resources/sandboxed-iframe-geolocation-watchPosition.html" sandbox="allow-scripts" width="100%" height="300"></iframe>
+<iframe src="https://127.0.0.1:8443/security/resources/sandboxed-iframe-geolocation-watchPosition.html" sandbox="allow-scripts" width="100%" height="300"></iframe>
 </body>
 </html>
index e11142e..2569353 100644 (file)
@@ -1,3 +1,38 @@
+2016-05-26  Pranjal Jumde  <pjumde@apple.com>
+
+        Sites served over insecure connections should not be allowed to use geolocation.
+        https://bugs.webkit.org/show_bug.cgi?id=157423
+        <rdar://problem/23751632>
+
+        Reviewed by Brent Fulgham.
+
+        Tests: http/tests/security/insecure-geolocation.html
+               http/tests/security/mixedcontent-geolocation-block-insecure-content.html
+               http/tests/security/mixedcontent-geolocation.html
+
+        * Modules/geolocation/Geolocation.cpp:
+        (WebCore::logError):
+        Logs an error to the console if geolocation is blocked.
+        (WebCore::Geolocation::startRequest):
+        Access to Geolocation will be blocked if site is not secure. An error will be logged when access to Geolocation is blocked.
+        (WebCore::Geolocation::shouldBlockGeolocationRequests)
+        Returns true if the access to geolocation should be blocked.
+        * Modules/geolocation/Geolocation.h:
+        * dom/SecurityContext.h:
+        (WebCore::SecurityContext::foundMixedContent):
+        Returns true if insecure content was accessed over secure connection.
+        (WebCore::SecurityContext::setFoundMixedContent):
+        Sets m_foundMixedContent to true if insecure content is accessed over secure connection.
+        (WebCore::SecurityContext::geolocationAccessed):
+        Returns true if geolocation was accessed
+        (WebCore::SecurityContext::setGeolocationAccessed):
+        Sets m_geolocationAccessed to true if geolocation was accessed.
+        * loader/MixedContentChecker.cpp:
+        (WebCore::MixedContentChecker::canDisplayInsecureContent):
+        Insecure content will be blocked if geolocation was accessed by the page. Updates document to keep track of mixed content.
+        (WebCore::MixedContentChecker::canRunInsecureContent):
+        Insecure content will be blocked if geolocation was accessed by the page. Updates document to keep track of mixed content.
+
 2016-05-26  Brady Eidson  <beidson@apple.com>
 
         Implement internals.observeGC to get called back when a Javascript object is GC'ed.
index 3ec4314..85ae40f 100644 (file)
@@ -43,6 +43,7 @@
 #include "SecurityOrigin.h"
 #include <wtf/CurrentTime.h>
 #include <wtf/Ref.h>
+#include <wtf/text/StringBuilder.h>
 
 namespace WebCore {
 
@@ -172,7 +173,7 @@ bool Geolocation::canSuspendForDocumentSuspension() const
 {
     return true;
 }
-
+    
 void Geolocation::suspend(ReasonForSuspension reason)
 {
     if (reason == ActiveDOMObject::PageCache) {
@@ -337,12 +338,44 @@ int Geolocation::watchPosition(RefPtr<PositionCallback>&& successCallback, RefPt
     return watchID;
 }
 
+static void logError(const String& target, const bool isSecure, const bool isMixedContent, Document* document)
+{
+    StringBuilder message;
+    message.append("[blocked] Access to geolocation was blocked over");
+    
+    if (!isSecure)
+        message.append(" insecure connection to ");
+    else if (isMixedContent)
+        message.append(" secure connection with mixed content to ");
+    else
+        return;
+    
+    message.append(target);
+    message.append(".\n");
+    document->addConsoleMessage(MessageSource::Security, MessageLevel::Error, message.toString());
+}
+
+bool Geolocation::shouldBlockGeolocationRequests()
+{
+    bool isSecure = SecurityOrigin::isSecure(document()->url());
+    bool hasMixedContent = document()->foundMixedContent();
+    bool isLocalFile = document()->url().isLocalFile();
+    if (securityOrigin()->canRequestGeolocation()) {
+        if (isLocalFile || (isSecure && !hasMixedContent))
+            return false;
+    }
+    
+    logError(securityOrigin()->toString(), isSecure, hasMixedContent, document());
+    return true;
+}
+
 void Geolocation::startRequest(GeoNotifier* notifier)
 {
-    if (!securityOrigin()->canRequestGeolocation()) {
+    if (shouldBlockGeolocationRequests()) {
         notifier->setFatalError(PositionError::create(PositionError::POSITION_UNAVAILABLE, ASCIILiteral(originCannotRequestGeolocationErrorMessage)));
         return;
     }
+    document()->setGeolocationAccessed();
 
     // Check whether permissions have already been denied. Note that if this is the case,
     // the permission state can not change again in the lifetime of this page.
index 0a68306..146e7d9 100644 (file)
@@ -72,6 +72,7 @@ public:
 
     void positionChanged();
     void setError(GeolocationError*);
+    bool shouldBlockGeolocationRequests();
 
 private:
     explicit Geolocation(ScriptExecutionContext*);
index 202bc09..07a2e61 100644 (file)
@@ -75,6 +75,10 @@ public:
     WEBCORE_EXPORT SecurityOrigin* securityOrigin() const;
 
     static SandboxFlags parseSandboxPolicy(const String& policy, String& invalidTokensErrorMessage);
+    bool foundMixedContent() const { return m_foundMixedContent; }
+    void setFoundMixedContent() { m_foundMixedContent = true; }
+    bool geolocationAccessed() const { return m_geolocationAccessed; }
+    void setGeolocationAccessed() { m_geolocationAccessed = true; }
 
 protected:
     SecurityContext();
@@ -90,6 +94,8 @@ private:
     SandboxFlags m_sandboxFlags;
     RefPtr<SecurityOriginPolicy> m_securityOriginPolicy;
     std::unique_ptr<ContentSecurityPolicy> m_contentSecurityPolicy;
+    bool m_foundMixedContent { false };
+    bool m_geolocationAccessed { false };
 };
 
 } // namespace WebCore
index 3656cb1..73923ad 100644 (file)
@@ -65,11 +65,13 @@ bool MixedContentChecker::canDisplayInsecureContent(SecurityOrigin* securityOrig
     if (!isMixedContent(securityOrigin, url))
         return true;
 
-    bool allowed = m_frame.settings().allowDisplayOfInsecureContent() || type == ContentType::ActiveCanWarn;
+    bool allowed = (m_frame.settings().allowDisplayOfInsecureContent() || type == ContentType::ActiveCanWarn) && !m_frame.document()->geolocationAccessed();
     logWarning(allowed, "display", url);
 
-    if (allowed)
+    if (allowed) {
+        m_frame.document()->setFoundMixedContent();
         client().didDisplayInsecureContent();
+    }
 
     return allowed;
 }
@@ -79,11 +81,13 @@ bool MixedContentChecker::canRunInsecureContent(SecurityOrigin* securityOrigin,
     if (!isMixedContent(securityOrigin, url))
         return true;
 
-    bool allowed = m_frame.settings().allowRunningOfInsecureContent();
+    bool allowed = m_frame.settings().allowRunningOfInsecureContent() && !m_frame.document()->geolocationAccessed();
     logWarning(allowed, "run", url);
 
-    if (allowed)
+    if (allowed) {
+        m_frame.document()->setFoundMixedContent();
         client().didRunInsecureContent(securityOrigin, url);
+    }
 
     return allowed;
 }