[webkit-changes] [111445] trunk/Source

Tue, 20 Mar 2012 15:02:22 -0700

Title: [111445] trunk/Source
Revision
111445
Author
[email protected]
Date
2012-03-20 15:02:08 -0700 (Tue, 20 Mar 2012)

Log Message

Restrict access to notifications for unique origins and file URLs with no local file access
https://bugs.webkit.org/show_bug.cgi?id=79704
<rdar://problem/10912430>

Reviewed by Adam Barth.

Source/WebCore:

Before checking or requesting permissions, we look at whether we can show notifications based on
the security context. If not, we short circuit and do not forward the request to the client.

* notifications/Notification.cpp:
(WebCore::Notification::Notification): Fix a bug where creating a notification goes through slightly
different logic for checking permissions than when checking permissions through the
notification center.
* notifications/NotificationCenter.cpp:
(WebCore::NotificationCenter::checkPermission): Check to see if the origin can show notifications.
(WebCore::NotificationCenter::requestPermission): If we know whether an origin can show notifications,
we asynchronously call the callback with that decision. Otherwise we ask the client.

Add new variable in the notification center to keep track of pending callbacks when we short-circuit
requestPermission().
* notifications/NotificationCenter.h:
(NotificationRequestCallback): Add new private class encapsulating the callback.
* notifications/NotificationCenter.cpp:
(WebCore::NotificationCenter::requestTimedOut): Remove the request from the set of pending callbacks.
(WebCore::NotificationCenter::NotificationRequest::createAndStartTimer): Because this is used when we already
know the decision, and are not asking the client to decide, we can immediately trigger a one-shot timer
to invoke the callback.
(WebCore::NotificationCenter::NotificationRequest::NotificationRequest):
(WebCore::NotificationCenter::NotificationRequest::startTimer):
(WebCore::NotificationCenter::NotificationRequest::timerFired): Invoke the callback and tell the notification
center.
* page/SecurityOrigin.h: Add new toRawString() method to return the string representation of the origin,
regardless of any restrictions that might otherwise cause toString() to return "null".
* page/SecurityOrigin.cpp:
(WebCore::SecurityOrigin::canShowNotifications): Added to return a tri-state regarding whether we know
the origin can show notifications. If the origin is unique, we never let it show. If the origin has
universal access, we always let it show. Otherwise the client should be asked.
(WebCore::SecurityOrigin::toString): Refactor to use toRawString().
(WebCore::SecurityOrigin::toRawString):

Added FIXME's regarding the naming of enforceFilePathSeparation().
* dom/Document.cpp:
(WebCore::Document::initSecurityContext): Also, tab reformatting.

* WebCore.exp.in: Export toRawString().

Source/WebKit2:

In the specific case where a file URL has restricted file access and is denied universal access,
SecurityOrigin::canShowNotifications() returns Ask, since it is not considered a unique origin.
The cached table of permissions held by the notification manager will typically not have an entry for
the toString() representation of these file URLs, which is "null", since that can also cover unique
origins, and it is possible that the client will want different permissions between the two types.

It is reasonable, however, for there to be an entry for "file://", so we use toRawString() to do the lookup.

* WebProcess/Notifications/WebNotificationManager.cpp:
(WebKit::WebNotificationManager::policyForOrigin):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (111444 => 111445)


--- trunk/Source/WebCore/ChangeLog	2012-03-20 21:54:15 UTC (rev 111444)
+++ trunk/Source/WebCore/ChangeLog	2012-03-20 22:02:08 UTC (rev 111445)
@@ -1,3 +1,51 @@
+2012-03-20  Jon Lee  <[email protected]>
+
+        Restrict access to notifications for unique origins and file URLs with no local file access
+        https://bugs.webkit.org/show_bug.cgi?id=79704
+        <rdar://problem/10912430>
+
+        Reviewed by Adam Barth.
+
+        Before checking or requesting permissions, we look at whether we can show notifications based on
+        the security context. If not, we short circuit and do not forward the request to the client.
+
+        * notifications/Notification.cpp:
+        (WebCore::Notification::Notification): Fix a bug where creating a notification goes through slightly
+        different logic for checking permissions than when checking permissions through the
+        notification center.
+        * notifications/NotificationCenter.cpp:
+        (WebCore::NotificationCenter::checkPermission): Check to see if the origin can show notifications.
+        (WebCore::NotificationCenter::requestPermission): If we know whether an origin can show notifications,
+        we asynchronously call the callback with that decision. Otherwise we ask the client.
+
+        Add new variable in the notification center to keep track of pending callbacks when we short-circuit
+        requestPermission().
+        * notifications/NotificationCenter.h:
+        (NotificationRequestCallback): Add new private class encapsulating the callback.
+        * notifications/NotificationCenter.cpp:
+        (WebCore::NotificationCenter::requestTimedOut): Remove the request from the set of pending callbacks.
+        (WebCore::NotificationCenter::NotificationRequest::createAndStartTimer): Because this is used when we already
+        know the decision, and are not asking the client to decide, we can immediately trigger a one-shot timer
+        to invoke the callback.
+        (WebCore::NotificationCenter::NotificationRequest::NotificationRequest):
+        (WebCore::NotificationCenter::NotificationRequest::startTimer):
+        (WebCore::NotificationCenter::NotificationRequest::timerFired): Invoke the callback and tell the notification
+        center.
+        * page/SecurityOrigin.h: Add new toRawString() method to return the string representation of the origin,
+        regardless of any restrictions that might otherwise cause toString() to return "null".
+        * page/SecurityOrigin.cpp:
+        (WebCore::SecurityOrigin::canShowNotifications): Added to return a tri-state regarding whether we know
+        the origin can show notifications. If the origin is unique, we never let it show. If the origin has
+        universal access, we always let it show. Otherwise the client should be asked.
+        (WebCore::SecurityOrigin::toString): Refactor to use toRawString().
+        (WebCore::SecurityOrigin::toRawString):
+
+        Added FIXME's regarding the naming of enforceFilePathSeparation().
+        * dom/Document.cpp:
+        (WebCore::Document::initSecurityContext): Also, tab reformatting.
+
+        * WebCore.exp.in: Export toRawString().
+
 2012-03-20  Andreas Kling  <[email protected]>
 
         Remove unused CSSPrimitiveValue constructors.

Modified: trunk/Source/WebCore/WebCore.exp.in (111444 => 111445)


--- trunk/Source/WebCore/WebCore.exp.in	2012-03-20 21:54:15 UTC (rev 111444)
+++ trunk/Source/WebCore/WebCore.exp.in	2012-03-20 22:02:08 UTC (rev 111445)
@@ -1283,6 +1283,7 @@
 __ZNK7WebCore14ScrollableArea23mouseEnteredContentAreaEv
 __ZNK7WebCore14ScrollableArea23mouseMovedInContentAreaEv
 __ZNK7WebCore14SecurityOrigin10canDisplayERKNS_4KURLE
+__ZNK7WebCore14SecurityOrigin11toRawStringEv
 __ZNK7WebCore14SecurityOrigin18databaseIdentifierEv
 __ZNK7WebCore14SecurityOrigin5equalEPKS0_
 __ZNK7WebCore14SecurityOrigin8toStringEv

Modified: trunk/Source/WebCore/dom/Document.cpp (111444 => 111445)


--- trunk/Source/WebCore/dom/Document.cpp	2012-03-20 21:54:15 UTC (rev 111444)
+++ trunk/Source/WebCore/dom/Document.cpp	2012-03-20 22:02:08 UTC (rev 111445)
@@ -4717,19 +4717,20 @@
 
     if (Settings* settings = this->settings()) {
         if (!settings->isWebSecurityEnabled()) {
-          // Web security is turned off.  We should let this document access every
-          // other document.  This is used primary by testing harnesses for web
-          // sites.
-          securityOrigin()->grantUniversalAccess();
-
+            // Web security is turned off. We should let this document access every
+            // other document. This is used primary by testing harnesses for web
+            // sites.
+            securityOrigin()->grantUniversalAccess();
         } else if (settings->allowUniversalAccessFromFileURLs() && securityOrigin()->isLocal()) {
-          // Some clients want file:// URLs to have universal access, but that
-          // setting is dangerous for other clients.
-          securityOrigin()->grantUniversalAccess();
+            // Some clients want file:// URLs to have universal access, but that
+            // setting is dangerous for other clients.
+            securityOrigin()->grantUniversalAccess();
         } else if (!settings->allowFileAccessFromFileURLs() && securityOrigin()->isLocal()) {
-          // Some clients want file:// URLs to have even tighter restrictions by
-          // default, and not be able to access other local files.
-          securityOrigin()->enforceFilePathSeparation();
+            // Some clients want file:// URLs to have even tighter restrictions by
+            // default, and not be able to access other local files.
+            // FIXME 81578: The naming of this is confusing. Files with restricted access to other local files
+            // still can have other privileges that can be remembered, thereby not making them unique origins.
+            securityOrigin()->enforceFilePathSeparation();
         }
     }

Modified: trunk/Source/WebCore/notifications/Notification.cpp (111444 => 111445)


--- trunk/Source/WebCore/notifications/Notification.cpp	2012-03-20 21:54:15 UTC (rev 111444)
+++ trunk/Source/WebCore/notifications/Notification.cpp	2012-03-20 22:02:08 UTC (rev 111445)
@@ -58,8 +58,7 @@
     , m_state(Idle)
     , m_notificationCenter(provider)
 {
-    ASSERT(m_notificationCenter->client());
-    if (m_notificationCenter->client()->checkPermission(context) != NotificationClient::PermissionAllowed) {
+    if (m_notificationCenter->checkPermission() != NotificationClient::PermissionAllowed) {
         ec = SECURITY_ERR;
         return;
     }
@@ -80,8 +79,7 @@
     , m_state(Idle)
     , m_notificationCenter(provider)
 {
-    ASSERT(m_notificationCenter->client());
-    if (m_notificationCenter->client()->checkPermission(context) != NotificationClient::PermissionAllowed) {
+    if (m_notificationCenter->checkPermission() != NotificationClient::PermissionAllowed) {
         ec = SECURITY_ERR;
         return;
     }

Modified: trunk/Source/WebCore/notifications/NotificationCenter.cpp (111444 => 111445)


--- trunk/Source/WebCore/notifications/NotificationCenter.cpp	2012-03-20 21:54:15 UTC (rev 111444)
+++ trunk/Source/WebCore/notifications/NotificationCenter.cpp	2012-03-20 22:02:08 UTC (rev 111445)
@@ -37,7 +37,7 @@
 
 #include "Document.h"
 #include "NotificationClient.h"
-#include "VoidCallback.h"
+#include "SecurityOrigin.h"
 #include "WorkerContext.h"
 
 namespace WebCore {
@@ -57,6 +57,17 @@
 {
     if (!client() || !scriptExecutionContext())
         return NotificationClient::PermissionDenied;
+
+    switch (scriptExecutionContext()->securityOrigin()->canShowNotifications()) {
+    case SecurityOrigin::Always:
+        return NotificationClient::PermissionAllowed;
+    case SecurityOrigin::Never:
+        return NotificationClient::PermissionDenied;
+    case SecurityOrigin::Ask:
+        return m_client->checkPermission(scriptExecutionContext());
+    }
+
+    ASSERT_NOT_REACHED();
     return m_client->checkPermission(scriptExecutionContext());
 }
 
@@ -64,6 +75,18 @@
 {
     if (!client() || !scriptExecutionContext())
         return;
+
+    switch (scriptExecutionContext()->securityOrigin()->canShowNotifications()) {
+    case SecurityOrigin::Always:
+    case SecurityOrigin::Never: {
+        m_callbacks.add(NotificationRequestCallback::createAndStartTimer(this, callback));
+        return;
+    }
+    case SecurityOrigin::Ask:
+        return m_client->requestPermission(scriptExecutionContext(), callback);
+    }
+
+    ASSERT_NOT_REACHED();
     m_client->requestPermission(scriptExecutionContext(), callback);
 }
 
@@ -78,6 +101,36 @@
     m_client = 0;
 }
 
+void NotificationCenter::requestTimedOut(NotificationCenter::NotificationRequestCallback* request)
+{
+    m_callbacks.remove(request);
+}
+
+PassRefPtr<NotificationCenter::NotificationRequestCallback> NotificationCenter::NotificationRequestCallback::createAndStartTimer(NotificationCenter* center, PassRefPtr<VoidCallback> callback)
+{
+    RefPtr<NotificationCenter::NotificationRequestCallback> requestCallback = adoptRef(new NotificationCenter::NotificationRequestCallback(center, callback));
+    requestCallback->startTimer();
+    return requestCallback.release();
+}
+
+NotificationCenter::NotificationRequestCallback::NotificationRequestCallback(NotificationCenter* center, PassRefPtr<VoidCallback> callback)
+    : m_notificationCenter(center)
+    , m_timer(this, &NotificationCenter::NotificationRequestCallback::timerFired)
+    , m_callback(callback)
+{
+}
+
+void NotificationCenter::NotificationRequestCallback::startTimer()
+{
+    m_timer.startOneShot(0);
+}
+
+void NotificationCenter::NotificationRequestCallback::timerFired(Timer<NotificationCenter::NotificationRequestCallback>*)
+{
+    m_callback->handleEvent();
+    m_notificationCenter->requestTimedOut(this);
+}
+
 } // namespace WebCore
 
 #endif // ENABLE(NOTIFICATIONS) || ENABLE(LEGACY_NOTIFICATIONS)

Modified: trunk/Source/WebCore/notifications/NotificationCenter.h (111444 => 111445)


--- trunk/Source/WebCore/notifications/NotificationCenter.h	2012-03-20 21:54:15 UTC (rev 111444)
+++ trunk/Source/WebCore/notifications/NotificationCenter.h	2012-03-20 22:02:08 UTC (rev 111445)
@@ -35,6 +35,8 @@
 #include "ExceptionCode.h"
 #include "Notification.h"
 #include "ScriptExecutionContext.h"
+#include "Timer.h"
+#include "VoidCallback.h"
 #include <wtf/OwnPtr.h>
 #include <wtf/PassRefPtr.h>
 #include <wtf/RefCounted.h>
@@ -83,7 +85,23 @@
 private:
     NotificationCenter(ScriptExecutionContext*, NotificationClient*);
 
+    class NotificationRequestCallback : public RefCounted<NotificationRequestCallback> {
+    public:
+        static PassRefPtr<NotificationRequestCallback> createAndStartTimer(NotificationCenter*, PassRefPtr<VoidCallback>);
+        void startTimer();
+        void timerFired(Timer<NotificationRequestCallback>*);
+    private:
+        NotificationRequestCallback(NotificationCenter*, PassRefPtr<VoidCallback>);
+
+        RefPtr<NotificationCenter> m_notificationCenter;
+        Timer<NotificationRequestCallback> m_timer;
+        RefPtr<VoidCallback> m_callback;
+    };
+
+    void requestTimedOut(NotificationRequestCallback*);
+
     NotificationClient* m_client;
+    HashSet<RefPtr<NotificationRequestCallback> > m_callbacks;
 };
 
 } // namespace WebCore

Modified: trunk/Source/WebCore/page/SecurityOrigin.cpp (111444 => 111445)


--- trunk/Source/WebCore/page/SecurityOrigin.cpp	2012-03-20 21:54:15 UTC (rev 111444)
+++ trunk/Source/WebCore/page/SecurityOrigin.cpp	2012-03-20 22:02:08 UTC (rev 111445)
@@ -355,6 +355,15 @@
     return true;
 }
 
+SecurityOrigin::Policy SecurityOrigin::canShowNotifications() const
+{
+    if (m_universalAccess)
+        return Always;
+    if (isUnique())
+        return Never;
+    return Ask;
+}
+
 void SecurityOrigin::grantLoadLocalResources()
 {
     // This function exists only to support backwards compatibility with older
@@ -386,12 +395,15 @@
 {
     if (isUnique())
         return "null";
+    if (m_protocol == "file" && m_enforceFilePathSeparation)
+        return "null";
+    return toRawString();
+}
 
-    if (m_protocol == "file") {
-        if (m_enforceFilePathSeparation)
-            return "null";
+String SecurityOrigin::toRawString() const
+{
+    if (m_protocol == "file")
         return "file://";
-    }
 
     StringBuilder result;
     result.reserveCapacity(m_protocol.length() + m_host.length() + 10);

Modified: trunk/Source/WebCore/page/SecurityOrigin.h (111444 => 111445)


--- trunk/Source/WebCore/page/SecurityOrigin.h	2012-03-20 21:54:15 UTC (rev 111444)
+++ trunk/Source/WebCore/page/SecurityOrigin.h	2012-03-20 22:02:08 UTC (rev 111445)
@@ -39,6 +39,12 @@
 
 class SecurityOrigin : public ThreadSafeRefCounted<SecurityOrigin> {
 public:
+    enum Policy {
+        Never = 0,
+        Always,
+        Ask
+    };
+
     static PassRefPtr<SecurityOrigin> create(const KURL&);
     static PassRefPtr<SecurityOrigin> createUnique();
 
@@ -115,6 +121,7 @@
     bool canAccessCookies() const { return !isUnique(); }
     bool canAccessPasswordManager() const { return !isUnique(); }
     bool canAccessFileSystem() const { return !isUnique(); }
+    Policy canShowNotifications() const;
 
     // Technically, we should always allow access to sessionStorage, but we
     // currently don't handle creating a sessionStorage area for unique
@@ -135,6 +142,8 @@
     bool isUnique() const { return m_isUnique; }
 
     // Marks a file:// origin as being in a domain defined by its path.
+    // FIXME 81578: The naming of this is confusing. Files with restricted access to other local files
+    // still can have other privileges that can be remembered, thereby not making them unique.
     void enforceFilePathSeparation();
 
     // Convert this SecurityOrigin into a string. The string
@@ -149,6 +158,10 @@
     // we shouldTreatURLSchemeAsNoAccess.
     String toString() const;
 
+    // Similar to toString(), but does not take into account any factors that
+    // could make the string return "null".
+    String toRawString() const;
+
     // Serialize the security origin to a string that could be used as part of
     // file names. This format should be used in storage APIs only.
     String databaseIdentifier() const;

Modified: trunk/Source/WebKit2/ChangeLog (111444 => 111445)


--- trunk/Source/WebKit2/ChangeLog	2012-03-20 21:54:15 UTC (rev 111444)
+++ trunk/Source/WebKit2/ChangeLog	2012-03-20 22:02:08 UTC (rev 111445)
@@ -1,3 +1,22 @@
+2012-03-20  Jon Lee  <[email protected]>
+
+        Restrict access to notifications for unique origins and file URLs with no local file access
+        https://bugs.webkit.org/show_bug.cgi?id=79704
+        <rdar://problem/10912430>
+
+        Reviewed by Adam Barth.
+
+        In the specific case where a file URL has restricted file access and is denied universal access,
+        SecurityOrigin::canShowNotifications() returns Ask, since it is not considered a unique origin.
+        The cached table of permissions held by the notification manager will typically not have an entry for
+        the toString() representation of these file URLs, which is "null", since that can also cover unique
+        origins, and it is possible that the client will want different permissions between the two types.
+
+        It is reasonable, however, for there to be an entry for "file://", so we use toRawString() to do the lookup.
+
+        * WebProcess/Notifications/WebNotificationManager.cpp:
+        (WebKit::WebNotificationManager::policyForOrigin):
+
 2012-03-20  Alexey Proskuryakov  <[email protected]>
 
         WebProcess should use private temporary and cache directories

Modified: trunk/Source/WebKit2/WebProcess/Notifications/WebNotificationManager.cpp (111444 => 111445)


--- trunk/Source/WebKit2/WebProcess/Notifications/WebNotificationManager.cpp	2012-03-20 21:54:15 UTC (rev 111444)
+++ trunk/Source/WebKit2/WebProcess/Notifications/WebNotificationManager.cpp	2012-03-20 22:02:08 UTC (rev 111445)
@@ -95,8 +95,9 @@
 #if ENABLE(NOTIFICATIONS) || ENABLE(LEGACY_NOTIFICATIONS)
     if (!origin)
         return NotificationClient::PermissionNotAllowed;
-    
-    HashMap<String, bool>::const_iterator it = m_permissionsMap.find(origin->toString());
+
+    ASSERT(!origin->isUnique());
+    HashMap<String, bool>::const_iterator it = m_permissionsMap.find(origin->toRawString());
     if (it != m_permissionsMap.end())
         return it->second ? NotificationClient::PermissionAllowed : NotificationClient::PermissionDenied;
 #endif

_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to