Bug 1164977 - Hoist app attributes into a struct on BasePrincipal and refer to them as 'origin attributes'. r=gabor
authorBobby Holley <bobbyholley@gmail.com>
Thu, 14 May 2015 12:24:03 -0700
changeset 265368 d4e6af40c5e87de3f2e91c6eeca96954d92c0166
parent 265367 2493c405317f9369d9cc573f0394bc67ae237380
child 265369 e18c56c6ade50594df27b3926df2b4f30eb00505
push id2126
push userjosea.olivera@gmail.com
push dateTue, 19 May 2015 14:12:35 +0000
reviewersgabor
bugs1164977
milestone41.0a1
Bug 1164977 - Hoist app attributes into a struct on BasePrincipal and refer to them as 'origin attributes'. r=gabor This sets the stage for the upcoming work for signed apps.
caps/BasePrincipal.cpp
caps/BasePrincipal.h
caps/nsNullPrincipal.cpp
caps/nsNullPrincipal.h
caps/nsPrincipal.cpp
caps/nsPrincipal.h
caps/nsScriptSecurityManager.cpp
caps/nsScriptSecurityManager.h
--- a/caps/BasePrincipal.cpp
+++ b/caps/BasePrincipal.cpp
@@ -71,55 +71,55 @@ BasePrincipal::GetIsNullPrincipal(bool* 
 {
   *aIsNullPrincipal = false;
   return NS_OK;
 }
 
 NS_IMETHODIMP
 BasePrincipal::GetJarPrefix(nsACString& aJarPrefix)
 {
-  MOZ_ASSERT(mAppId != nsIScriptSecurityManager::UNKNOWN_APP_ID);
+  MOZ_ASSERT(AppId() != nsIScriptSecurityManager::UNKNOWN_APP_ID);
 
-  mozilla::GetJarPrefix(mAppId, mIsInBrowserElement, aJarPrefix);
+  mozilla::GetJarPrefix(AppId(), IsInBrowserElement(), aJarPrefix);
   return NS_OK;
 }
 
 NS_IMETHODIMP
 BasePrincipal::GetAppStatus(uint16_t* aAppStatus)
 {
-  if (mAppId == nsIScriptSecurityManager::UNKNOWN_APP_ID) {
+  if (AppId() == nsIScriptSecurityManager::UNKNOWN_APP_ID) {
     NS_WARNING("Asking for app status on a principal with an unknown app id");
     *aAppStatus = nsIPrincipal::APP_STATUS_NOT_INSTALLED;
     return NS_OK;
   }
 
   *aAppStatus = nsScriptSecurityManager::AppStatusForPrincipal(this);
   return NS_OK;
 }
 
 NS_IMETHODIMP
 BasePrincipal::GetAppId(uint32_t* aAppId)
 {
-  if (mAppId == nsIScriptSecurityManager::UNKNOWN_APP_ID) {
+  if (AppId() == nsIScriptSecurityManager::UNKNOWN_APP_ID) {
     MOZ_ASSERT(false);
     *aAppId = nsIScriptSecurityManager::NO_APP_ID;
     return NS_OK;
   }
 
-  *aAppId = mAppId;
+  *aAppId = AppId();
   return NS_OK;
 }
 
 NS_IMETHODIMP
 BasePrincipal::GetIsInBrowserElement(bool* aIsInBrowserElement)
 {
-  *aIsInBrowserElement = mIsInBrowserElement;
+  *aIsInBrowserElement = IsInBrowserElement();
   return NS_OK;
 }
 
 NS_IMETHODIMP
 BasePrincipal::GetUnknownAppId(bool* aUnknownAppId)
 {
-  *aUnknownAppId = mAppId == nsIScriptSecurityManager::UNKNOWN_APP_ID;
+  *aUnknownAppId = AppId() == nsIScriptSecurityManager::UNKNOWN_APP_ID;
   return NS_OK;
 }
 
 } // namespace mozilla
--- a/caps/BasePrincipal.h
+++ b/caps/BasePrincipal.h
@@ -18,20 +18,17 @@ namespace mozilla {
  * default implementations and other commonalities between principal
  * implementations.
  *
  * We should merge nsJSPrincipals into this class at some point.
  */
 class BasePrincipal : public nsJSPrincipals
 {
 public:
-  BasePrincipal()
-    : mAppId(nsIScriptSecurityManager::NO_APP_ID)
-    , mIsInBrowserElement(false)
-  {}
+  BasePrincipal() {}
 
   enum DocumentDomainConsideration { DontConsiderDocumentDomain, ConsiderDocumentDomain};
   bool Subsumes(nsIPrincipal* aOther, DocumentDomainConsideration aConsideration);
 
   NS_IMETHOD Equals(nsIPrincipal* other, bool* _retval) final;
   NS_IMETHOD EqualsConsideringDomain(nsIPrincipal* other, bool* _retval) final;
   NS_IMETHOD Subsumes(nsIPrincipal* other, bool* _retval) final;
   NS_IMETHOD SubsumesConsideringDomain(nsIPrincipal* other, bool* _retval) final;
@@ -43,21 +40,42 @@ public:
   NS_IMETHOD GetAppId(uint32_t* aAppStatus) final;
   NS_IMETHOD GetIsInBrowserElement(bool* aIsInBrowserElement) final;
   NS_IMETHOD GetUnknownAppId(bool* aUnknownAppId) final;
 
   virtual bool IsOnCSSUnprefixingWhitelist() override { return false; }
 
   static BasePrincipal* Cast(nsIPrincipal* aPrin) { return static_cast<BasePrincipal*>(aPrin); }
 
+  struct OriginAttributes {
+    uint32_t mAppId;
+    bool mIsInBrowserElement;
+
+    OriginAttributes() : mAppId(nsIScriptSecurityManager::NO_APP_ID), mIsInBrowserElement(false) {}
+    OriginAttributes(uint32_t aAppId, bool aIsInBrowserElement)
+      : mAppId(aAppId), mIsInBrowserElement(aIsInBrowserElement) {}
+    bool operator==(const OriginAttributes& aOther) const
+    {
+      return mAppId == aOther.mAppId &&
+             mIsInBrowserElement == aOther.mIsInBrowserElement;
+    }
+    bool operator!=(const OriginAttributes& aOther) const
+    {
+      return !(*this == aOther);
+    }
+  };
+
+  const OriginAttributes& OriginAttributesRef() { return mOriginAttributes; }
+  uint32_t AppId() const { return mOriginAttributes.mAppId; }
+  bool IsInBrowserElement() const { return mOriginAttributes.mIsInBrowserElement; }
+
 protected:
   virtual ~BasePrincipal() {}
 
   virtual bool SubsumesInternal(nsIPrincipal* aOther, DocumentDomainConsideration aConsider) = 0;
 
   nsCOMPtr<nsIContentSecurityPolicy> mCSP;
-  uint32_t mAppId;
-  bool mIsInBrowserElement;
+  OriginAttributes mOriginAttributes;
 };
 
 } // namespace mozilla
 
 #endif /* mozilla_BasePrincipal_h */
--- a/caps/nsNullPrincipal.cpp
+++ b/caps/nsNullPrincipal.cpp
@@ -36,37 +36,35 @@ NS_IMPL_QUERY_INTERFACE_CI(nsNullPrincip
 NS_IMPL_CI_INTERFACE_GETTER(nsNullPrincipal,
                             nsIPrincipal,
                             nsISerializable)
 
 /* static */ already_AddRefed<nsNullPrincipal>
 nsNullPrincipal::CreateWithInheritedAttributes(nsIPrincipal* aInheritFrom)
 {
   nsRefPtr<nsNullPrincipal> nullPrin = new nsNullPrincipal();
-  nsresult rv = nullPrin->Init(aInheritFrom->GetAppId(),
-                               aInheritFrom->GetIsInBrowserElement());
+  nsresult rv = nullPrin->Init(Cast(aInheritFrom)->OriginAttributesRef());
   return NS_SUCCEEDED(rv) ? nullPrin.forget() : nullptr;
 }
 
 /* static */ already_AddRefed<nsNullPrincipal>
-nsNullPrincipal::Create(uint32_t aAppId, bool aInMozBrowser)
+nsNullPrincipal::Create(const OriginAttributes& aOriginAttributes)
 {
   nsRefPtr<nsNullPrincipal> nullPrin = new nsNullPrincipal();
-  nsresult rv = nullPrin->Init(aAppId, aInMozBrowser);
+  nsresult rv = nullPrin->Init(aOriginAttributes);
   NS_ENSURE_SUCCESS(rv, nullptr);
 
   return nullPrin.forget();
 }
 
 nsresult
-nsNullPrincipal::Init(uint32_t aAppId, bool aInMozBrowser)
+nsNullPrincipal::Init(const OriginAttributes& aOriginAttributes)
 {
-  MOZ_ASSERT(aAppId != nsIScriptSecurityManager::UNKNOWN_APP_ID);
-  mAppId = aAppId;
-  mIsInBrowserElement = aInMozBrowser;
+  mOriginAttributes = aOriginAttributes;
+  MOZ_ASSERT(AppId() != nsIScriptSecurityManager::UNKNOWN_APP_ID);
 
   mURI = nsNullPrincipalURI::Create();
   NS_ENSURE_TRUE(mURI, NS_ERROR_NOT_AVAILABLE);
 
   return NS_OK;
 }
 
 void
@@ -159,25 +157,25 @@ nsNullPrincipal::GetBaseDomain(nsACStrin
  */
 NS_IMETHODIMP
 nsNullPrincipal::Read(nsIObjectInputStream* aStream)
 {
   // Note - nsNullPrincipal use NS_GENERIC_FACTORY_CONSTRUCTOR_INIT, which means
   // that the Init() method has already been invoked by the time we deserialize.
   // This is in contrast to nsPrincipal, which uses NS_GENERIC_FACTORY_CONSTRUCTOR,
   // in which case ::Read needs to invoke Init().
-  nsresult rv = aStream->Read32(&mAppId);
+  nsresult rv = aStream->Read32(&mOriginAttributes.mAppId);
   NS_ENSURE_SUCCESS(rv, rv);
 
-  rv = aStream->ReadBoolean(&mIsInBrowserElement);
+  rv = aStream->ReadBoolean(&mOriginAttributes.mIsInBrowserElement);
   NS_ENSURE_SUCCESS(rv, rv);
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsNullPrincipal::Write(nsIObjectOutputStream* aStream)
 {
-  aStream->Write32(mAppId);
-  aStream->WriteBoolean(mIsInBrowserElement);
+  aStream->Write32(AppId());
+  aStream->WriteBoolean(IsInBrowserElement());
   return NS_OK;
 }
 
--- a/caps/nsNullPrincipal.h
+++ b/caps/nsNullPrincipal.h
@@ -49,21 +49,19 @@ public:
   NS_IMETHOD GetIsNullPrincipal(bool* aIsNullPrincipal) override;
   NS_IMETHOD GetBaseDomain(nsACString& aBaseDomain) override;
 
   // Returns null on failure.
   static already_AddRefed<nsNullPrincipal> CreateWithInheritedAttributes(nsIPrincipal *aInheritFrom);
 
   // Returns null on failure.
   static already_AddRefed<nsNullPrincipal>
-    Create(uint32_t aAppId = nsIScriptSecurityManager::NO_APP_ID,
-           bool aInMozBrowser = false);
+    Create(const OriginAttributes& aOriginAttributes = OriginAttributes());
 
-  nsresult Init(uint32_t aAppId = nsIScriptSecurityManager::NO_APP_ID,
-                bool aInMozBrowser = false);
+  nsresult Init(const OriginAttributes& aOriginAttributes = OriginAttributes());
 
   virtual void GetScriptLocation(nsACString &aStr) override;
 
  protected:
   virtual ~nsNullPrincipal() {}
 
   bool SubsumesInternal(nsIPrincipal* aOther, DocumentDomainConsideration aConsideration) override
   {
--- a/caps/nsPrincipal.cpp
+++ b/caps/nsPrincipal.cpp
@@ -73,30 +73,26 @@ nsPrincipal::nsPrincipal()
   , mDomainImmutable(false)
   , mInitialized(false)
 { }
 
 nsPrincipal::~nsPrincipal()
 { }
 
 nsresult
-nsPrincipal::Init(nsIURI *aCodebase,
-                  uint32_t aAppId,
-                  bool aInMozBrowser)
+nsPrincipal::Init(nsIURI *aCodebase, const OriginAttributes& aOriginAttributes)
 {
   NS_ENSURE_STATE(!mInitialized);
   NS_ENSURE_ARG(aCodebase);
 
   mInitialized = true;
 
   mCodebase = NS_TryToMakeImmutable(aCodebase);
   mCodebaseImmutable = URIIsImmutable(mCodebase);
-
-  mAppId = aAppId;
-  mIsInBrowserElement = aInMozBrowser;
+  mOriginAttributes = aOriginAttributes;
 
   return NS_OK;
 }
 
 void
 nsPrincipal::GetScriptLocation(nsACString &aStr)
 {
   mCodebase->GetSpec(aStr);
@@ -167,18 +163,18 @@ nsPrincipal::SubsumesInternal(nsIPrincip
 {
   MOZ_ASSERT(aOther);
 
   // For nsPrincipal, Subsumes is equivalent to Equals.
   if (aOther == this) {
     return true;
   }
 
-  if (!nsScriptSecurityManager::AppAttributesEqual(this, aOther)) {
-      return false;
+  if (OriginAttributesRef() != Cast(aOther)->OriginAttributesRef()) {
+    return false;
   }
 
   // If either the subject or the object has changed its principal by
   // explicitly setting document.domain then the other must also have
   // done so in order to be considered the same origin. This prevents
   // DNS spoofing based on document.domain (154930)
   nsresult rv;
   if (aConsideration == ConsiderDocumentDomain) {
@@ -375,17 +371,18 @@ nsPrincipal::Read(nsIObjectInputStream* 
   NS_ENSURE_SUCCESS(rv, rv);
 
   rv = NS_ReadOptionalObject(aStream, true, getter_AddRefs(supports));
   NS_ENSURE_SUCCESS(rv, rv);
 
   // This may be null.
   nsCOMPtr<nsIContentSecurityPolicy> csp = do_QueryInterface(supports, &rv);
 
-  rv = Init(codebase, appId, inMozBrowser);
+  OriginAttributes attrs(appId, inMozBrowser);
+  rv = Init(codebase, attrs);
   NS_ENSURE_SUCCESS(rv, rv);
 
   rv = SetCsp(csp);
   NS_ENSURE_SUCCESS(rv, rv);
 
   // need to link in the CSP context here (link in the URI of the protected
   // resource).
   if (csp) {
@@ -409,18 +406,18 @@ nsPrincipal::Write(nsIObjectOutputStream
   }
 
   rv = NS_WriteOptionalCompoundObject(aStream, mDomain, NS_GET_IID(nsIURI),
                                       true);
   if (NS_FAILED(rv)) {
     return rv;
   }
 
-  aStream->Write32(mAppId);
-  aStream->WriteBoolean(mIsInBrowserElement);
+  aStream->Write32(AppId());
+  aStream->WriteBoolean(IsInBrowserElement());
 
   rv = NS_WriteOptionalCompoundObject(aStream, mCSP,
                                       NS_GET_IID(nsIContentSecurityPolicy),
                                       true);
   if (NS_FAILED(rv)) {
     return rv;
   }
 
--- a/caps/nsPrincipal.h
+++ b/caps/nsPrincipal.h
@@ -29,19 +29,17 @@ public:
   NS_IMETHOD GetOrigin(nsACString& aOrigin) override;
   NS_IMETHOD CheckMayLoad(nsIURI* uri, bool report, bool allowIfInheritsPrincipal) override;
   NS_IMETHOD GetBaseDomain(nsACString& aBaseDomain) override;
   virtual bool IsOnCSSUnprefixingWhitelist() override;
 
   nsPrincipal();
 
   // Init() must be called before the principal is in a usable state.
-  nsresult Init(nsIURI* aCodebase,
-                uint32_t aAppId,
-                bool aInMozBrowser);
+  nsresult Init(nsIURI* aCodebase, const OriginAttributes& aOriginAttributes);
 
   virtual void GetScriptLocation(nsACString& aStr) override;
   void SetURI(nsIURI* aURI);
 
   static bool IsPrincipalInherited(nsIURI* aURI) {
     // return true if the loadee URI has
     // the URI_INHERITS_SECURITY_CONTEXT flag set.
     bool doesInheritSecurityContext;
--- a/caps/nsScriptSecurityManager.cpp
+++ b/caps/nsScriptSecurityManager.cpp
@@ -482,36 +482,16 @@ 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()));
-}
-
 NS_IMETHODIMP
 nsScriptSecurityManager::CheckLoadURIFromScript(JSContext *cx, nsIURI *aURI)
 {
     // Get principal of currently executing script.
     MOZ_ASSERT(cx == nsContentUtils::GetCurrentJSContext());
     nsIPrincipal* principal = nsContentUtils::SubjectPrincipal();
     nsresult rv = CheckLoadURIWithPrincipal(principal, aURI,
                                             nsIScriptSecurityManager::STANDARD);
@@ -1017,19 +997,19 @@ nsScriptSecurityManager::CreateCodebaseP
             NS_ENSURE_TRUE(principal, NS_ERROR_FAILURE);
         }
 
         principal.forget(result);
 
         return NS_OK;
     }
 
+    BasePrincipal::OriginAttributes attrs(aAppId, aInMozBrowser);
     nsRefPtr<nsPrincipal> codebase = new nsPrincipal();
-
-    nsresult rv = codebase->Init(aURI, aAppId, aInMozBrowser);
+    nsresult rv = codebase->Init(aURI, attrs);
     if (NS_FAILED(rv))
         return rv;
 
     NS_ADDREF(*result = codebase);
 
     return NS_OK;
 }
 
--- a/caps/nsScriptSecurityManager.h
+++ b/caps/nsScriptSecurityManager.h
@@ -75,32 +75,16 @@ 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);
-
     void DeactivateDomainPolicy();
 
 private:
 
     // GetScriptSecurityManager is the only call that can make one
     nsScriptSecurityManager();
     virtual ~nsScriptSecurityManager();