Finally kill off CheckSameOriginPrincipal, fix remaining callers to do the checks they really want to be doing. Fix screw-up in nsPrincipal::Equals if one principal has a cert and the other does not. Bug 418996, r=mrbkap,dveditz, sr=jst

caps/idl/nsIScriptSecurityManager.idl

@@ -42,7 +42,7 @@ interface nsIURI;
 interface nsIChannel;
 
 
-[scriptable, uuid(ce216cf7-3bcb-48ab-9ff8-d03a24f19ca5)]
+[scriptable, uuid(3fffd8e8-3fea-442e-a0ed-2ba81ae197d5)]
 interface nsIScriptSecurityManager : nsIXPCSecurityManager
 {
     ///////////////// Security Checks //////////////////
@@ -291,13 +291,6 @@ interface nsIScriptSecurityManager : nsIXPCSecurityManager
                             in nsIURI aTargetURI,
                             in boolean reportError);
 
-    /**
-     * Returns OK if aSourcePrincipal and aTargetPrincipal
-     * have the same "origin" (scheme, host, and port).
-     */
-    void checkSameOriginPrincipal(in nsIPrincipal aSourcePrincipal,
-                                  in nsIPrincipal aTargetPrincipal);
-
     /**
      * Returns the principal of the global object of the given context, or null
      * if no global or no principal.

caps/include/nsScriptSecurityManager.h

@@ -406,7 +406,10 @@ public:
     static nsresult 
     ReportError(JSContext* cx, const nsAString& messageTag,
                 nsIURI* aSource, nsIURI* aTarget);
-
+    static nsresult
+    CheckSameOriginPrincipal(nsIPrincipal* aSubject,
+                             nsIPrincipal* aObject,
+                             PRBool aIsCheckConnect);
 private:
 
     // GetScriptSecurityManager is the only call that can make one
@@ -441,11 +444,6 @@ private:
                             const char* aClassName, jsval aProperty,
                             void** aCachedClassPolicy);
 
-    nsresult
-    CheckSameOriginPrincipalInternal(nsIPrincipal* aSubject,
-                                     nsIPrincipal* aObject,
-                                     PRBool aIsCheckConnect);
-
     nsresult
     CheckSameOriginDOMProp(nsIPrincipal* aSubject, 
                            nsIPrincipal* aObject,

caps/src/nsPrincipal.cpp

@@ -249,13 +249,14 @@ nsPrincipal::Equals(nsIPrincipal *aOther, PRBool *aResult)
   }
 
   if (this != aOther) {
-    if (mCert) {
     PRBool otherHasCert;
     aOther->GetHasCertificate(&otherHasCert);
-      if (!otherHasCert) {
+    if (otherHasCert != (mCert != nsnull)) {
+      // One has a cert while the other doesn't.  Not equal.
       return NS_OK;
     }
 
+    if (mCert) {
       nsCAutoString str;
       aOther->GetFingerprint(str);
       *aResult = str.Equals(mCert->fingerprint);
@@ -292,8 +293,9 @@ nsPrincipal::Equals(nsIPrincipal *aOther, PRBool *aResult)
 
     // Codebases are equal if they have the same origin.
     *aResult =
-      NS_SUCCEEDED(nsScriptSecurityManager::GetScriptSecurityManager()
+      NS_SUCCEEDED(nsScriptSecurityManager::CheckSameOriginPrincipal(this,
-                   ->CheckSameOriginPrincipal(this, aOther));
+                                                                     aOther,
+                                                                     PR_FALSE));
     return NS_OK;
   }
 

caps/src/nsScriptSecurityManager.cpp

@@ -726,16 +726,6 @@ nsScriptSecurityManager::CheckSameOriginURI(nsIURI* aSourceURI,
     return NS_OK;
 }
 
-NS_IMETHODIMP
-nsScriptSecurityManager::CheckSameOriginPrincipal(nsIPrincipal* aSourcePrincipal,
-                                                  nsIPrincipal* aTargetPrincipal)
-{
-    return CheckSameOriginPrincipalInternal(aSourcePrincipal,
-                                            aTargetPrincipal,
-                                            PR_FALSE);
-}
-
-
 nsresult
 nsScriptSecurityManager::CheckPropertyAccessImpl(PRUint32 aAction,
                                                  nsAXPCNativeCallContext* aCallContext,
@@ -962,8 +952,9 @@ nsScriptSecurityManager::CheckPropertyAccessImpl(PRUint32 aAction,
     return rv;
 }
 
+/* static */
 nsresult
-nsScriptSecurityManager::CheckSameOriginPrincipalInternal(nsIPrincipal* aSubject,
+nsScriptSecurityManager::CheckSameOriginPrincipal(nsIPrincipal* aSubject,
                                                   nsIPrincipal* aObject,
                                                   PRBool aIsCheckConnect)
 {
@@ -1035,8 +1026,19 @@ nsScriptSecurityManager::CheckSameOriginDOMProp(nsIPrincipal* aSubject,
                                                 PRUint32 aAction,
                                                 PRBool aIsCheckConnect)
 {
-    nsresult rv = CheckSameOriginPrincipalInternal(aSubject, aObject,
+    nsresult rv;
-                                                   aIsCheckConnect);
+    if (aIsCheckConnect) {
+        // Don't do equality compares, just do a same-origin compare,
+        // since the object principal isn't a real principal, just a
+        // GetCodebasePrincipal() on whatever URI we started with.
+        rv = CheckSameOriginPrincipal(aSubject, aObject, aIsCheckConnect);
+    } else {
+        PRBool subsumes;
+        rv = aSubject->Subsumes(aObject, &subsumes);
+        if (NS_SUCCEEDED(rv) && !subsumes) {
+            rv = NS_ERROR_DOM_PROP_ACCESS_DENIED;
+        }
+    }
     
     if (NS_SUCCEEDED(rv))
         return NS_OK;
@@ -1696,9 +1698,12 @@ nsScriptSecurityManager::CheckFunctionAccess(JSContext *aCx, void *aFunObj,
     if (!object)
         return NS_ERROR_FAILURE;        
 
-    // Note that CheckSameOriginPrincipalInternal already does an equality
+    PRBool subsumes;
-    // comparison on subject and object, so no need for us to do it.
+    rv = subject->Subsumes(object, &subsumes);
-    return CheckSameOriginPrincipalInternal(subject, object, PR_TRUE);
+    if (NS_SUCCEEDED(rv) && !subsumes) {
+        rv = NS_ERROR_DOM_PROP_ACCESS_DENIED;
+    }
+    return rv;
 }
 
 NS_IMETHODIMP

js/src/xpconnect/src/XPCCrossOriginWrapper.cpp

@@ -250,14 +250,14 @@ IsValFrame(JSObject *obj, jsval v, XPCWrappedNative *wn)
   return domwin != nsnull;
 }
 
-// Returns whether the currently executing code has the same origin as the
+// Returns whether the currently executing code is allowed to access
-// wrapper. Uses nsIScriptSecurityManager::CheckSameOriginPrincipal.
+// the wrapper.  Uses nsIPrincipal::Subsumes.
 // |cx| must be the top context on the context stack.
-// If the two principals have the same origin, returns NS_OK. If they differ,
+// If the subject is allowed to access the object returns NS_OK. If not,
 // returns NS_ERROR_DOM_PROP_ACCESS_DENIED, returns another error code on
 // failure.
 nsresult
-IsWrapperSameOrigin(JSContext *cx, JSObject *wrappedObj)
+CanAccessWrapper(JSContext *cx, JSObject *wrappedObj)
 {
   // Get the subject principal from the execution stack.
   nsIScriptSecurityManager *ssm = XPCWrapper::GetSecurityManager();
@@ -279,6 +279,8 @@ IsWrapperSameOrigin(JSContext *cx, JSObject *wrappedObj)
 
   // If we somehow end up being called from chrome, just allow full access.
   // This can happen from components with xpcnativewrappers=no.
+  // Note that this is just an optimization to avoid getting the
+  // object principal in this case, since Subsumes() would return true.
   if (isSystem) {
     return NS_OK;
   }
@@ -296,7 +298,12 @@ IsWrapperSameOrigin(JSContext *cx, JSObject *wrappedObj)
   }
 
   // Now, we have our two principals, compare them!
-  return ssm->CheckSameOriginPrincipal(subjectPrin, objectPrin);
+  PRBool subsumes;
+  rv = subjectPrin->Subsumes(objectPrin, &subsumes);
+  if (NS_SUCCEEDED(rv) && !subsumes) {
+    rv = NS_ERROR_DOM_PROP_ACCESS_DENIED;
+  }
+  return rv;
 }
 
 static JSBool
@@ -333,7 +340,7 @@ XPC_XOW_FunctionWrapper(JSContext *cx, JSObject *obj, uintN argc, jsval *argv,
     return ThrowException(NS_ERROR_ILLEGAL_VALUE, cx);
   }
 
-  nsresult rv = IsWrapperSameOrigin(cx, JSVAL_TO_OBJECT(funToCall));
+  nsresult rv = CanAccessWrapper(cx, JSVAL_TO_OBJECT(funToCall));
   if (NS_FAILED(rv) && rv != NS_ERROR_DOM_PROP_ACCESS_DENIED) {
     return ThrowException(rv, cx);
   }
@@ -551,7 +558,7 @@ XPC_XOW_AddProperty(JSContext *cx, JSObject *obj, jsval id, jsval *vp)
   if (!wrappedObj) {
     return ThrowException(NS_ERROR_ILLEGAL_VALUE, cx);
   }
-  nsresult rv = IsWrapperSameOrigin(cx, wrappedObj);
+  nsresult rv = CanAccessWrapper(cx, wrappedObj);
   if (NS_FAILED(rv)) {
     if (rv == NS_ERROR_DOM_PROP_ACCESS_DENIED) {
       // Can't override properties on foreign objects.
@@ -571,7 +578,7 @@ XPC_XOW_DelProperty(JSContext *cx, JSObject *obj, jsval id, jsval *vp)
   if (!wrappedObj) {
     return ThrowException(NS_ERROR_ILLEGAL_VALUE, cx);
   }
-  nsresult rv = IsWrapperSameOrigin(cx, wrappedObj);
+  nsresult rv = CanAccessWrapper(cx, wrappedObj);
   if (NS_FAILED(rv)) {
     if (rv == NS_ERROR_DOM_PROP_ACCESS_DENIED) {
       // Can't delete properties on foreign objects.
@@ -620,7 +627,7 @@ XPC_XOW_GetOrSetProperty(JSContext *cx, JSObject *obj, jsval id, jsval *vp,
   if (!wrappedObj) {
     return ThrowException(NS_ERROR_ILLEGAL_VALUE, cx);
   }
-  nsresult rv = IsWrapperSameOrigin(cx, wrappedObj);
+  nsresult rv = CanAccessWrapper(cx, wrappedObj);
   if (NS_FAILED(rv)) {
     if (rv != NS_ERROR_DOM_PROP_ACCESS_DENIED) {
       return JS_FALSE;
@@ -734,7 +741,7 @@ XPC_XOW_Enumerate(JSContext *cx, JSObject *obj)
     // Nothing to enumerate.
     return JS_TRUE;
   }
-  nsresult rv = IsWrapperSameOrigin(cx, wrappedObj);
+  nsresult rv = CanAccessWrapper(cx, wrappedObj);
   if (NS_FAILED(rv)) {
     if (rv == NS_ERROR_DOM_PROP_ACCESS_DENIED) {
       // Can't enumerate on foreign objects.
@@ -760,7 +767,7 @@ XPC_XOW_NewResolve(JSContext *cx, JSObject *obj, jsval id, uintN flags,
     return JS_TRUE;
   }
 
-  nsresult rv = IsWrapperSameOrigin(cx, wrappedObj);
+  nsresult rv = CanAccessWrapper(cx, wrappedObj);
   if (NS_FAILED(rv)) {
     if (rv != NS_ERROR_DOM_PROP_ACCESS_DENIED) {
       return JS_FALSE;
@@ -840,7 +847,7 @@ XPC_XOW_Convert(JSContext *cx, JSObject *obj, JSType type, jsval *vp)
   }
 
   // Note: JSTYPE_VOID and JSTYPE_STRING are equivalent.
-  nsresult rv = IsWrapperSameOrigin(cx, wrappedObj);
+  nsresult rv = CanAccessWrapper(cx, wrappedObj);
   if (NS_FAILED(rv) &&
       (rv != NS_ERROR_DOM_PROP_ACCESS_DENIED ||
        (type != JSTYPE_STRING && type != JSTYPE_VOID))) {
@@ -908,7 +915,7 @@ XPC_XOW_Call(JSContext *cx, JSObject *obj, uintN argc, jsval *argv, jsval *rval)
     // Nothing to call.
     return JS_TRUE;
   }
-  nsresult rv = IsWrapperSameOrigin(cx, wrappedObj);
+  nsresult rv = CanAccessWrapper(cx, wrappedObj);
   if (NS_FAILED(rv)) {
     if (rv == NS_ERROR_DOM_PROP_ACCESS_DENIED) {
       // Can't call.
@@ -939,7 +946,7 @@ XPC_XOW_Construct(JSContext *cx, JSObject *obj, uintN argc, jsval *argv,
     // Nothing to construct.
     return JS_TRUE;
   }
-  nsresult rv = IsWrapperSameOrigin(cx, wrappedObj);
+  nsresult rv = CanAccessWrapper(cx, wrappedObj);
   if (NS_FAILED(rv)) {
     if (rv == NS_ERROR_DOM_PROP_ACCESS_DENIED) {
       // Can't construct.
@@ -963,7 +970,7 @@ JS_STATIC_DLL_CALLBACK(JSBool)
 XPC_XOW_HasInstance(JSContext *cx, JSObject *obj, jsval v, JSBool *bp)
 {
   JSObject *iface = GetWrappedObject(cx, obj);
-  nsresult rv = IsWrapperSameOrigin(cx, iface);
+  nsresult rv = CanAccessWrapper(cx, iface);
   if (NS_FAILED(rv)) {
     if (rv == NS_ERROR_DOM_PROP_ACCESS_DENIED) {
       // Don't do this test across origins.
@@ -1093,7 +1100,7 @@ XPC_XOW_toString(JSContext *cx, JSObject *obj, uintN argc, jsval *argv,
     return JS_TRUE;
   }
 
-  nsresult rv = IsWrapperSameOrigin(cx, wrappedObj);
+  nsresult rv = CanAccessWrapper(cx, wrappedObj);
   if (rv == NS_ERROR_DOM_PROP_ACCESS_DENIED) {
     nsIScriptSecurityManager *ssm = XPCWrapper::GetSecurityManager();
     if (!ssm) {

js/src/xpconnect/src/XPCWrapper.h

@@ -79,7 +79,7 @@ XPC_XOW_WrapperMoved(JSContext *cx, XPCWrappedNative *innerObj,
                      XPCWrappedNativeScope *newScope);
 
 nsresult
-IsWrapperSameOrigin(JSContext *cx, JSObject *wrappedObj);
+CanAccessWrapper(JSContext *cx, JSObject *wrappedObj);
 
 inline JSBool
 XPC_XOW_ClassNeedsXOW(const char *name)
@@ -208,7 +208,7 @@ public:
     }
 
     JSObject *wrappedObj = JSVAL_TO_OBJECT(v);
-    nsresult rv = IsWrapperSameOrigin(cx, wrappedObj);
+    nsresult rv = CanAccessWrapper(cx, wrappedObj);
     if (NS_FAILED(rv)) {
       JS_ClearPendingException(cx);
       return nsnull;