Bug 1955775 - Don't include domain in ContentPrincipal::GetHashValue. r=nika,ckerschb

As it can mutate over the lifetime of the principal.

Differential Revision: https://phabricator.services.mozilla.com/D242653

caps/BasePrincipal.cpp

@@ -1618,3 +1618,9 @@ void BasePrincipal::WriteJSONProperty(JSONWriter& aWriter,
 }
 
 }  // namespace mozilla
+
+uint32_t nsIPrincipal::GetHashValue() const {
+  auto* bp = mozilla::BasePrincipal::Cast(this);
+  return mozilla::HashGeneric(bp->GetOriginNoSuffixHash(),
+                              bp->GetOriginSuffixHash());
+}

caps/ContentPrincipal.cpp

@@ -308,17 +308,6 @@ bool ContentPrincipal::MayLoadInternal(nsIURI* aURI) {
   return false;
 }
 
-uint32_t ContentPrincipal::GetHashValue() {
-  MOZ_ASSERT(mURI, "Need a principal URI");
-
-  nsCOMPtr<nsIURI> uri;
-  GetDomain(getter_AddRefs(uri));
-  if (!uri) {
-    GetURI(getter_AddRefs(uri));
-  };
-  return NS_SecurityHashURI(uri);
-}
-
 NS_IMETHODIMP
 ContentPrincipal::GetDomain(nsIURI** aDomain) {
   if (!GetHasExplicitDomain()) {

caps/ContentPrincipal.h

@@ -22,7 +22,6 @@ class JSONWriter;
 class ContentPrincipal final : public BasePrincipal {
  public:
   NS_IMETHOD QueryInterface(REFNSIID aIID, void** aInstancePtr) override;
-  uint32_t GetHashValue() override;
   NS_IMETHOD GetURI(nsIURI** aURI) override;
   NS_IMETHOD GetDomain(nsIURI** aDomain) override;
   NS_IMETHOD SetDomain(nsIURI* aDomain) override;

caps/ExpandedPrincipal.cpp

@@ -106,10 +106,6 @@ bool ExpandedPrincipal::MayLoadInternal(nsIURI* uri) {
   return false;
 }
 
-uint32_t ExpandedPrincipal::GetHashValue() {
-  MOZ_CRASH("extended principal should never be used as key in a hash map");
-}
-
 NS_IMETHODIMP
 ExpandedPrincipal::GetURI(nsIURI** aURI) {
   *aURI = nullptr;

caps/ExpandedPrincipal.h

@@ -37,7 +37,6 @@ class ExpandedPrincipal : public nsIExpandedPrincipal,
     return nsJSPrincipals::Release();
   };
   NS_IMETHOD QueryInterface(REFNSIID aIID, void** aInstancePtr) override;
-  uint32_t GetHashValue() override;
   NS_IMETHOD GetURI(nsIURI** aURI) override;
   NS_IMETHOD GetDomain(nsIURI** aDomain) override;
   NS_IMETHOD SetDomain(nsIURI* aDomain) override;

caps/NullPrincipal.cpp

@@ -157,8 +157,6 @@ nsresult NullPrincipal::GetScriptLocation(nsACString& aStr) {
  * nsIPrincipal implementation
  */
 
-uint32_t NullPrincipal::GetHashValue() { return (NS_PTR_TO_INT32(this) >> 2); }
-
 NS_IMETHODIMP
 NullPrincipal::GetURI(nsIURI** aURI) {
   nsCOMPtr<nsIURI> uri = mURI;

caps/NullPrincipal.h

@@ -40,7 +40,6 @@ class NullPrincipal final : public BasePrincipal {
   static PrincipalKind Kind() { return eNullPrincipal; }
 
   NS_IMETHOD QueryInterface(REFNSIID aIID, void** aInstancePtr) override;
-  uint32_t GetHashValue() override;
   NS_IMETHOD GetURI(nsIURI** aURI) override;
   NS_IMETHOD GetIsOriginPotentiallyTrustworthy(bool* aResult) override;
   NS_IMETHOD GetDomain(nsIURI** aDomain) override;

caps/PrincipalHashKey.h

@@ -43,8 +43,7 @@ class PrincipalHashKey : public PLDHashEntryHdr {
     return aKey;
   }
   static PLDHashNumber HashKey(const nsIPrincipal* aKey) {
-    const auto* bp = BasePrincipal::Cast(aKey);
-    return HashGeneric(bp->GetOriginNoSuffixHash(), bp->GetOriginSuffixHash());
+    return aKey->GetHashValue();
   }
 
   enum { ALLOW_MEMMOVE = true };

caps/SystemPrincipal.cpp

@@ -65,8 +65,6 @@ nsresult SystemPrincipal::GetScriptLocation(nsACString& aStr) {
 // Methods implementing nsIPrincipal //
 ///////////////////////////////////////
 
-uint32_t SystemPrincipal::GetHashValue() { return NS_PTR_TO_INT32(this); }
-
 NS_IMETHODIMP
 SystemPrincipal::GetURI(nsIURI** aURI) {
   *aURI = nullptr;

caps/SystemPrincipal.h

@@ -43,7 +43,6 @@ class SystemPrincipal final : public BasePrincipal, public nsISerializable {
 
   NS_DECL_NSISERIALIZABLE
   NS_IMETHOD QueryInterface(REFNSIID aIID, void** aInstancePtr) override;
-  uint32_t GetHashValue() override;
   NS_IMETHOD GetURI(nsIURI** aURI) override;
   NS_IMETHOD GetDomain(nsIURI** aDomain) override;
   NS_IMETHOD SetDomain(nsIURI* aDomain) override;

caps/nsIPrincipal.idl

@@ -95,11 +95,16 @@ interface nsIPrincipal : nsISupports
     boolean equalsURI(in nsIURI aOtherURI);
 
     /**
-     * Returns a hash value for the principal.
+     * Returns a hash value for the principal. Guaranteed to be equal
+     * for two principals which are Equals(...).
+     *
+     * Implemented in BasePrincipal.cpp.
      *
      * May be called from any thread.
      */
-    [notxpcom, nostdcall] readonly attribute unsigned long hashValue;
+    %{C++
+    uint32_t GetHashValue() const;
+    %}
 
     /**
      * The principal URI to which this principal pertains.  This is

caps/tests/gtest/TestPrincipalHash.cpp

@@ -0,0 +1,27 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+#include "gtest/gtest.h"
+#include "mozilla/gtest/MozAssertions.h"
+#include "nsIPrincipal.h"
+#include "nsScriptSecurityManager.h"
+#include "nsNetUtil.h"
+
+TEST(PrincipalHash, DocumentDomain)
+{
+  nsCOMPtr<nsIScriptSecurityManager> ssm =
+      nsScriptSecurityManager::GetScriptSecurityManager();
+  nsCOMPtr<nsIPrincipal> principal;
+  nsresult rv =
+      ssm->CreateContentPrincipalFromOrigin("https://sub.mozilla.org"_ns, getter_AddRefs(principal));
+  EXPECT_NS_SUCCEEDED(rv);
+
+  const auto hash = principal->GetHashValue();
+
+  nsCOMPtr<nsIURI> domain;
+  rv = NS_NewURI(getter_AddRefs(domain), "https://mozilla.org"_ns);
+  EXPECT_NS_SUCCEEDED(rv);
+  principal->SetDomain(domain);
+
+  ASSERT_EQ(principal->GetHashValue(), hash) << "Principal hash shouldn't change";
+}

caps/tests/gtest/moz.build

@@ -9,6 +9,7 @@ UNIFIED_SOURCES += [
     "TestNullPrincipalPrecursor.cpp",
     "TestOriginAttributes.cpp",
     "TestPrincipalAttributes.cpp",
+    "TestPrincipalHash.cpp",
     "TestPrincipalSerialization.cpp",
     "TestRedirectChainURITruncation.cpp",
     "TestScriptSecurityManager.cpp",