Bug 913734 - Stop consulting domain policies in CAPS. r=mrbkap
authorBobby Holley <bobbyholley@gmail.com>
Fri, 13 Dec 2013 19:15:43 -0800
changeset 160494 6c4dd8d796c672109929405770246a8acb40188f
parent 160493 22929b380e8458c9bc2816eef1bbfbf14e84472e
child 160495 4df87ccf0f5d6e6e54844aade43f1eb210fdbbe5
push id25834
push userphilringnalda@gmail.com
push dateSun, 15 Dec 2013 02:20:53 +0000
treeherdermozilla-central@9fcc6330dc69 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmrbkap
bugs913734
milestone29.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 913734 - Stop consulting domain policies in CAPS. r=mrbkap The whole LookupPolicy juggernaut is basically a mechanism for setting custom per-(protocol, origin, property, action) access control in the preferences service. There are two sets of preferences currently in all.js. One of them is set up for mailnews, for the mailbox:, imap:, and news: protocols. According to jst, this was designed as a whack-a-mole security mechanism for javascript running in HTML email. IIUC, we no longer allow JS to run at all in mailnews, so this is obsolete. The other mechanism appears to be our old-fashioned implementation of the same-origin policy, which has been obsoleted by the new compartment architecture. In addition, most of this stuff was obsoleted by the new dom bindings, since these DOM classes no longer go through XPCWrappedNativeJSOps, and thus no longer trigger these security checks at all. We stop using the infrastructure in this patch, and rip it out in the next one.
caps/src/nsScriptSecurityManager.cpp
--- a/caps/src/nsScriptSecurityManager.cpp
+++ b/caps/src/nsScriptSecurityManager.cpp
@@ -624,79 +624,59 @@ nsScriptSecurityManager::CheckPropertyAc
         return rv;
 
     if (!subjectPrincipal || subjectPrincipal == mSystemPrincipal)
         // We have native code or the system principal: just allow access
         return NS_OK;
 
     nsCOMPtr<nsIPrincipal> objectPrincipal;
 
+    // Even though security wrappers have handled our cross-origin access policy
+    // since FF4, we've still always called into this function, and have relied
+    // on the CAPS policy mechanism to manually approve each cross-origin property
+    // based on a whitelist in all.js. This whitelist has drifted out of sync
+    // with the canonical one in AccessCheck.cpp, but it's always been more
+    // permissive, so we never noticed. This machinery is going away, so for now
+    // let's just skip the check for cross-origin property accesses that we know
+    // we get right.
+    //
+    // Note that while it would be nice to just rely on aClassName here, it
+    // isn't set reliably by callers. :-(
+    const char *className = aClassName;
+    if (!className && jsObject) {
+        className = JS_GetClass(jsObject)->name;
+    }
+    if (className &&
+        (!strcmp(className, "Window") || !strcmp(className, "Location")))
+    {
+        return NS_OK;
+    }
+
     // Hold the class info data here so we don't have to go back to virtual
     // methods all the time
     ClassInfoData classInfoData(aClassInfo, aClassName);
 
-    //-- Look up the security policy for this class and subject domain
-    SecurityLevel securityLevel;
-    rv = LookupPolicy(subjectPrincipal, classInfoData, property, aAction,
-                      &securityLevel);
-    if (NS_FAILED(rv))
-        return rv;
-
-    if (securityLevel.level == SCRIPT_SECURITY_UNDEFINED_ACCESS)
-    {
-        // No policy found for this property so use the default of last resort.
-        // If we were called from somewhere other than XPConnect
-        // (no XPC call context), assume this is a DOM class. Otherwise,
-        // ask the ClassInfo.
-        if (!aCallContext || classInfoData.IsDOMClass())
-            securityLevel.level = SCRIPT_SECURITY_SAME_ORIGIN_ACCESS;
-        else
-            securityLevel.level = SCRIPT_SECURITY_NO_ACCESS;
-    }
-
-    if (SECURITY_ACCESS_LEVEL_FLAG(securityLevel))
-    // This flag means securityLevel is allAccess, noAccess, or sameOrigin
-    {
-        switch (securityLevel.level)
-        {
-        case SCRIPT_SECURITY_NO_ACCESS:
-            rv = NS_ERROR_DOM_PROP_ACCESS_DENIED;
-            break;
-
-        case SCRIPT_SECURITY_ALL_ACCESS:
-            rv = NS_OK;
-            break;
-
-        case SCRIPT_SECURITY_SAME_ORIGIN_ACCESS:
-            {
-                nsCOMPtr<nsIPrincipal> principalHolder;
-                if (jsObject)
-                {
-                    objectPrincipal = doGetObjectPrincipal(jsObject);
-                    if (!objectPrincipal)
-                        rv = NS_ERROR_DOM_SECURITY_ERR;
-                }
-                else
-                {
-                    NS_ERROR("CheckPropertyAccessImpl called without a target object or URL");
-                    return NS_ERROR_FAILURE;
-                }
-                if(NS_SUCCEEDED(rv))
-                    rv = CheckSameOriginDOMProp(subjectPrincipal, objectPrincipal,
-                                                aAction);
-                break;
-            }
-        default:
-            NS_ERROR("Bad Security Level Value");
+    // If we were called from somewhere other than XPConnect
+    // (no XPC call context), assume this is a DOM class. Otherwise,
+    // ask the ClassInfo.
+    if (aCallContext && !classInfoData.IsDOMClass()) {
+        rv = NS_ERROR_DOM_PROP_ACCESS_DENIED;
+    } else {
+        if (!jsObject) {
+            NS_ERROR("CheckPropertyAccessImpl called without a target object or URL");
             return NS_ERROR_FAILURE;
         }
-    }
-    else // if SECURITY_ACCESS_LEVEL_FLAG is false, securityLevel is a capability
-    {
-        rv = SubjectIsPrivileged() ? NS_OK : NS_ERROR_DOM_SECURITY_ERR;
+        nsCOMPtr<nsIPrincipal> principalHolder;
+        objectPrincipal = doGetObjectPrincipal(jsObject);
+        if (!objectPrincipal)
+            rv = NS_ERROR_DOM_SECURITY_ERR;
+
+        if (NS_SUCCEEDED(rv))
+            rv = CheckSameOriginDOMProp(subjectPrincipal, objectPrincipal,
+                                        aAction);
     }
 
     if (NS_SUCCEEDED(rv))
     {
         return rv;
     }
 
     //--See if the object advertises a non-default level of access
@@ -1428,29 +1408,16 @@ nsScriptSecurityManager::CheckLoadURIWit
         rv = NS_URIChainHasFlags(sourceURI,
                                  nsIProtocolHandler::URI_IS_UI_RESOURCE,
                                  &sourceIsChrome);
         NS_ENSURE_SUCCESS(rv, rv);
         if (sourceIsChrome) {
             return NS_OK;
         }
 
-        // Now check capability policies
-        static const char loadURIPrefGroup[] = "checkloaduri";
-        ClassInfoData nameData(nullptr, loadURIPrefGroup);
-
-        SecurityLevel secLevel;
-        rv = LookupPolicy(aPrincipal, nameData, EnabledID(),
-                          nsIXPCSecurityManager::ACCESS_GET_PROPERTY, &secLevel);
-        if (NS_SUCCEEDED(rv) && secLevel.level == SCRIPT_SECURITY_ALL_ACCESS)
-        {
-            // OK for this site!
-            return NS_OK;
-        }
-
         if (reportErrors) {
             ReportError(nullptr, errorTag, sourceURI, aTargetURI);
         }
         return NS_ERROR_DOM_BAD_URI;
     }
 
     // OK, everyone is allowed to load this, since unflagged handlers are
     // deprecated but treated as URI_LOADABLE_BY_ANYONE.  But check whether we
@@ -1583,39 +1550,17 @@ bool
 nsScriptSecurityManager::ScriptAllowed(JSObject *aGlobal)
 {
     MOZ_ASSERT(aGlobal);
     MOZ_ASSERT(JS_IsGlobalObject(aGlobal) || js::IsOuterObject(aGlobal));
     AutoJSContext cx;
     JS::RootedObject global(cx, js::UncheckedUnwrap(aGlobal, /* stopAtOuter = */ false));
 
     // Check the bits on the compartment private.
-    xpc::Scriptability& scriptability = xpc::Scriptability::Get(aGlobal);
-    if (!scriptability.Allowed()) {
-        return false;
-    }
-
-    // If the compartment is immune to script policy, we're done.
-    if (scriptability.IsImmuneToScriptPolicy()) {
-        return true;
-    }
-
-    // Check for a per-site policy.
-    static const char jsPrefGroupName[] = "javascript";
-    ClassInfoData nameData(nullptr, jsPrefGroupName);
-    SecurityLevel secLevel;
-    nsresult rv = LookupPolicy(doGetObjectPrincipal(global), nameData,
-                               EnabledID(),
-                               nsIXPCSecurityManager::ACCESS_GET_PROPERTY,
-                               &secLevel);
-    if (NS_FAILED(rv) || secLevel.level == SCRIPT_SECURITY_NO_ACCESS) {
-        return false;
-    }
-
-    return true;
+    return xpc::Scriptability::Get(aGlobal).Allowed();
 }
 
 ///////////////// Principals ///////////////////////
 NS_IMETHODIMP
 nsScriptSecurityManager::GetSubjectPrincipal(nsIPrincipal **aSubjectPrincipal)
 {
     nsresult rv;
     *aSubjectPrincipal = doGetSubjectPrincipal(&rv);