Bug 777467 - Update the same-origin policy for principals to include appid/isinbrowserelement. r=bholley
☠☠ backed out by 6eda62c9a13d ☠ ☠
authorMounir Lamouri <mounir.lamouri@gmail.com>
Mon, 22 Oct 2012 16:20:38 +0100
changeset 111165 fb62d8b9800aa73a0108832e32ee9ad95d6a7709
parent 111164 a2cdb1000e515efccf4884531088f62dd79b9a8b
child 111166 1ebb05c8a8c8a48dddc6bac6fe9ac03ab637562f
push id93
push usernmatsakis@mozilla.com
push dateWed, 31 Oct 2012 21:26:57 +0000
reviewersbholley
bugs777467
milestone19.0a1
Bug 777467 - Update the same-origin policy for principals to include appid/isinbrowserelement. r=bholley
caps/idl/nsIPrincipal.idl
caps/include/nsScriptSecurityManager.h
caps/src/nsPrincipal.cpp
caps/src/nsScriptSecurityManager.cpp
caps/tests/mochitest/Makefile.in
caps/tests/mochitest/test_app_principal_equality.html
--- a/caps/idl/nsIPrincipal.idl
+++ b/caps/idl/nsIPrincipal.idl
@@ -179,27 +179,57 @@ interface nsIPrincipal : nsISerializable
 
     /**
      * Returns the app id the principal is in, or returns
      * nsIScriptSecurityManager::NO_APP_ID if this principal isn't part of an
      * app.
      */
     readonly attribute unsigned long appId;
 
+    %{C++
+    uint32_t GetAppId()
+    {
+      uint32_t appId;
+      mozilla::DebugOnly<nsresult> rv = GetAppId(&appId);
+      MOZ_ASSERT(NS_SUCCEEDED(rv));
+      return appId;
+    }
+    %}
+
     /**
      * Returns true iif the principal is inside a browser element.
      */
     readonly attribute boolean isInBrowserElement;
 
+    %{C++
+    bool GetIsInBrowserElement()
+    {
+      bool isInBrowserElement;
+      mozilla::DebugOnly<nsresult> rv = GetIsInBrowserElement(&isInBrowserElement);
+      MOZ_ASSERT(NS_SUCCEEDED(rv));
+      return isInBrowserElement;
+    }
+    %}
+
     /**
      * Returns true if this principal has an unknown appId. This shouldn't
      * generally be used. We only expose it due to not providing the correct
      * appId everywhere where we construct principals.
      */
     readonly attribute boolean unknownAppId;
+
+    %{C++
+    bool GetUnknownAppId()
+    {
+      bool unkwnownAppId;
+      mozilla::DebugOnly<nsresult> rv = GetIsInBrowserElement(&unkwnownAppId);
+      MOZ_ASSERT(NS_SUCCEEDED(rv));
+      return unkwnownAppId;
+    }
+    %}
 };
 
 /**
  * If nsSystemPrincipal is too risky to use, but we want a principal to access 
  * more than one origin, nsExpandedPrincipals letting us define an array of 
  * principals it subsumes. So script with an nsExpandedPrincipals will gain
  * same origin access when at least one of its principals it contains gained 
  * sameorigin acccess. An nsExpandedPrincipal will be subsumed by the system
--- a/caps/include/nsScriptSecurityManager.h
+++ b/caps/include/nsScriptSecurityManager.h
@@ -343,16 +343,32 @@ public:
     HashPrincipalByOrigin(nsIPrincipal* aPrincipal);
 
     static bool
     GetStrictFileOriginPolicy()
     {
         return sStrictFileOriginPolicy;
     }
 
+    /**
+     * Returns true if the two principals share the same app attributes.
+     *
+     * App attributes are appId and the inBrowserElement flag.
+     * Two principals have the same app attributes if those information are
+     * equals.
+     * This method helps keeping principals from different apps isolated from
+     * each other. Also, it helps making sure mozbrowser (web views) and their
+     * parent are isolated from each other. All those entities do not share the
+     * same data (cookies, IndexedDB, localStorage, etc.) so we shouldn't allow
+     * violating that principle.
+     */
+    static bool
+    AppAttributesEqual(nsIPrincipal* aFirst,
+                       nsIPrincipal* aSecond);
+
 private:
 
     // GetScriptSecurityManager is the only call that can make one
     nsScriptSecurityManager();
     virtual ~nsScriptSecurityManager();
 
     bool SubjectIsPrivileged();
 
--- a/caps/src/nsPrincipal.cpp
+++ b/caps/src/nsPrincipal.cpp
@@ -258,44 +258,52 @@ NS_IMETHODIMP
 nsPrincipal::GetOrigin(char **aOrigin)
 {
   return GetOriginForURI(mCodebase, aOrigin);
 }
 
 NS_IMETHODIMP
 nsPrincipal::Equals(nsIPrincipal *aOther, bool *aResult)
 {
+  *aResult = false;
+
   if (!aOther) {
     NS_WARNING("Need a principal to compare this to!");
-    *aResult = false;
     return NS_OK;
   }
 
-  if (this != aOther) {
-
-    // Codebases are equal if they have the same origin.
-    *aResult =
-      NS_SUCCEEDED(nsScriptSecurityManager::CheckSameOriginPrincipal(this,
-                                                                     aOther));
+  if (aOther == this) {
+    *aResult = true;
     return NS_OK;
   }
 
-  *aResult = true;
+  // Codebases are equal if they have the same origin.
+  *aResult = NS_SUCCEEDED(
+               nsScriptSecurityManager::CheckSameOriginPrincipal(this, aOther));
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsPrincipal::EqualsIgnoringDomain(nsIPrincipal *aOther, bool *aResult)
 {
-  if (this == aOther) {
+  *aResult = false;
+
+  if (!aOther) {
+    NS_WARNING("Need a principal to compare this to!");
+    return NS_OK;
+  }
+
+  if (aOther == this) {
     *aResult = true;
     return NS_OK;
   }
 
-  *aResult = false;
+  if (!nsScriptSecurityManager::AppAttributesEqual(this, aOther)) {
+    return NS_OK;
+  }
 
   nsCOMPtr<nsIURI> otherURI;
   nsresult rv = aOther->GetURI(getter_AddRefs(otherURI));
   if (NS_FAILED(rv)) {
     return rv;
   }
 
   NS_ASSERTION(mCodebase,
--- a/caps/src/nsScriptSecurityManager.cpp
+++ b/caps/src/nsScriptSecurityManager.cpp
@@ -885,16 +885,20 @@ nsScriptSecurityManager::CheckSameOrigin
                                                   nsIPrincipal* aObject)
 {
     /*
     ** Get origin of subject and object and compare.
     */
     if (aSubject == aObject)
         return NS_OK;
 
+    if (!AppAttributesEqual(aSubject, aObject)) {
+        return NS_ERROR_DOM_PROP_ACCESS_DENIED;
+    }
+
     // Default to false, and change if that turns out wrong.
     bool subjectSetDomain = false;
     bool objectSetDomain = false;
     
     nsCOMPtr<nsIURI> subjectURI;
     nsCOMPtr<nsIURI> objectURI;
 
     aSubject->GetDomain(getter_AddRefs(subjectURI));
@@ -945,16 +949,36 @@ nsScriptSecurityManager::HashPrincipalBy
 {
     nsCOMPtr<nsIURI> uri;
     aPrincipal->GetDomain(getter_AddRefs(uri));
     if (!uri)
         aPrincipal->GetURI(getter_AddRefs(uri));
     return SecurityHashURI(uri);
 }
 
+/* static */ bool
+nsScriptSecurityManager::AppAttributesEqual(nsIPrincipal* aFirst,
+                                            nsIPrincipal* aSecond)
+{
+    MOZ_ASSERT(aFirst && aSecond, "Don't pass null pointers!");
+
+    uint32_t firstAppId = nsIScriptSecurityManager::UNKNOWN_APP_ID;
+    if (!aFirst->GetUnknownAppId()) {
+        firstAppId = aFirst->GetAppId();
+    }
+
+    uint32_t secondAppId = nsIScriptSecurityManager::UNKNOWN_APP_ID;
+    if (!aSecond->GetUnknownAppId()) {
+        secondAppId = aSecond->GetAppId();
+    }
+
+    return ((firstAppId == secondAppId) &&
+            (aFirst->GetIsInBrowserElement() == aSecond->GetIsInBrowserElement()));
+}
+
 nsresult
 nsScriptSecurityManager::CheckSameOriginDOMProp(nsIPrincipal* aSubject,
                                                 nsIPrincipal* aObject,
                                                 uint32_t aAction)
 {
     nsresult rv;
     bool subsumes;
     rv = aSubject->Subsumes(aObject, &subsumes);
--- a/caps/tests/mochitest/Makefile.in
+++ b/caps/tests/mochitest/Makefile.in
@@ -11,16 +11,17 @@ relativesrcdir  = @relativesrcdir@
 
 include $(DEPTH)/config/autoconf.mk
 
 MOCHITEST_FILES = 	test_bug423375.html \
                 test_bug246699.html \
                 test_bug292789.html \
                 test_bug470804.html \
                 test_disallowInheritPrincipal.html \
+                test_app_principal_equality.html \
                 $(NULL)
 
 # extendedOrigin test doesn't work on Windows, see bug 776296.
 ifneq ($(OS_ARCH),WINNT)
 MOCHITEST_CHROME_FILES = test_principal_extendedorigin_appid_appstatus.html \
                          $(NULL)
 endif
 
new file mode 100644
--- /dev/null
+++ b/caps/tests/mochitest/test_app_principal_equality.html
@@ -0,0 +1,85 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=777467
+-->
+<head>
+  <meta charset="utf-8">
+  <title>Test app principal's equality</title>
+  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=777467">Mozilla Bug 777467</a>
+<p id="display"></p>
+<script>
+  // Initialization.
+  SpecialPowers.addPermission("browser", true, document);
+  SpecialPowers.addPermission("embed-apps", true, document);
+
+  var previousPrefs = {
+    mozBrowserFramesEnabled: undefined,
+    oop_by_default: undefined,
+  };
+  try {
+    previousPrefs.mozBrowserFramesEnabled = SpecialPowers.getBoolPref('dom.mozBrowserFramesEnabled');
+  } catch(e) {}
+
+  try {
+    previousPrefs.oop_by_default = SpecialPowers.getBoolPref('dom.ipc.browser_frames.oop_by_default');
+  } catch(e) {}
+
+  SpecialPowers.setBoolPref('dom.mozBrowserFramesEnabled', true);
+  SpecialPowers.setBoolPref("dom.ipc.browser_frames.oop_by_default", false);
+</script>
+<div id="content" style="display: none;">
+  <iframe src="error404"></iframe>
+  <iframe mozbrowser src="error404"></iframe>
+  <iframe mozapp="http://example.org/manifest.webapp" mozbrowser src="error404"></iframe>
+</div>
+<pre id="test">
+<script type="application/javascript">
+
+/** Test for app principal's equality **/
+
+SimpleTest.waitForExplicitFinish();
+
+function canAccessDocument(win) {
+  var result = true;
+  try {
+    win.document;
+  } catch(e) {
+    result = false;
+  }
+  return result;
+}
+
+addLoadEvent(function() {
+  // Test the witness frame (we can access same-origin frame).
+  is(canAccessDocument(frames[0]), true,
+     "should be able to access the first frame");
+
+  // Test different app/browserElement frames.
+  for (var i=1; i<frames.length; ++i) {
+    is(canAccessDocument(frames[i]), false,
+       "should not be able to access the other frames");
+  }
+
+  // Cleanup.
+  if (previousPrefs.mozBrowserFramesEnabled !== undefined) {
+    SpecialPowers.setBoolPref('dom.mozBrowserFramesEnabled', previousPrefs.mozBrowserFramesEnabled);
+  }
+  if (previousPrefs.oop_by_default !== undefined) {
+    SpecialPowers.setBoolPref("dom.ipc.browser_frames.oop_by_default", previousPrefs.oop_by_default);
+  }
+
+  SpecialPowers.removePermission("browser", window.document);
+  SpecialPowers.removePermission("embed-apps", window.document);
+
+  SimpleTest.finish();
+});
+
+</script>
+</pre>
+</body>
+</html>