Bug 1548804 - Remove origin suffix isolation for proxy credentials when setting authentication cache entry, r=valentin
authorHonza Bambas <honzab.moz@firemni.cz>
Tue, 14 May 2019 13:40:30 +0000
changeset 535673 3502dda7734c88243a809b1a3441971adfd649ee
parent 535672 cc1bf8afa084e6e2ca2623714470bb065128e072
child 535674 5f0d8832067f71dc9e0bcc4c0b03dc5458d57ea1
push id2082
push userffxbld-merge
push dateMon, 01 Jul 2019 08:34:18 +0000
treeherdermozilla-release@2fb19d0466d2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersvalentin
bugs1548804
milestone68.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 1548804 - Remove origin suffix isolation for proxy credentials when setting authentication cache entry, r=valentin Differential Revision: https://phabricator.services.mozilla.com/D30911
netwerk/protocol/http/nsHttpAuthCache.cpp
netwerk/protocol/http/nsHttpChannelAuthProvider.cpp
netwerk/protocol/http/nsHttpChannelAuthProvider.h
--- a/netwerk/protocol/http/nsHttpAuthCache.cpp
+++ b/netwerk/protocol/http/nsHttpAuthCache.cpp
@@ -46,121 +46,129 @@ static bool StrEquivalent(const char16_t
 }
 
 //-----------------------------------------------------------------------------
 // nsHttpAuthCache <public>
 //-----------------------------------------------------------------------------
 
 nsHttpAuthCache::nsHttpAuthCache()
     : mDB(128), mObserver(new OriginClearObserver(this)) {
+  LOG(("nsHttpAuthCache::nsHttpAuthCache %p", this));
+
   nsCOMPtr<nsIObserverService> obsSvc = services::GetObserverService();
   if (obsSvc) {
     obsSvc->AddObserver(mObserver, "clear-origin-attributes-data", false);
   }
 }
 
 nsHttpAuthCache::~nsHttpAuthCache() {
+  LOG(("nsHttpAuthCache::~nsHttpAuthCache %p", this));
+
   DebugOnly<nsresult> rv = ClearAll();
   MOZ_ASSERT(NS_SUCCEEDED(rv));
   nsCOMPtr<nsIObserverService> obsSvc = services::GetObserverService();
   if (obsSvc) {
     obsSvc->RemoveObserver(mObserver, "clear-origin-attributes-data");
     mObserver->mOwner = nullptr;
   }
 }
 
 nsresult nsHttpAuthCache::GetAuthEntryForPath(const char* scheme,
                                               const char* host, int32_t port,
                                               const char* path,
                                               nsACString const& originSuffix,
                                               nsHttpAuthEntry** entry) {
-  LOG(("nsHttpAuthCache::GetAuthEntryForPath [key=%s://%s:%d path=%s]\n",
-       scheme, host, port, path));
+  LOG(("nsHttpAuthCache::GetAuthEntryForPath %p [path=%s]\n", this, path));
 
   nsAutoCString key;
   nsHttpAuthNode* node = LookupAuthNode(scheme, host, port, originSuffix, key);
   if (!node) return NS_ERROR_NOT_AVAILABLE;
 
   *entry = node->LookupEntryByPath(path);
+  LOG(("  returning %p", *entry));
   return *entry ? NS_OK : NS_ERROR_NOT_AVAILABLE;
 }
 
 nsresult nsHttpAuthCache::GetAuthEntryForDomain(const char* scheme,
                                                 const char* host, int32_t port,
                                                 const char* realm,
                                                 nsACString const& originSuffix,
                                                 nsHttpAuthEntry** entry)
 
 {
-  LOG(("nsHttpAuthCache::GetAuthEntryForDomain [key=%s://%s:%d realm=%s]\n",
-       scheme, host, port, realm));
+  LOG(("nsHttpAuthCache::GetAuthEntryForDomain %p [realm=%s]\n", this, realm));
 
   nsAutoCString key;
   nsHttpAuthNode* node = LookupAuthNode(scheme, host, port, originSuffix, key);
   if (!node) return NS_ERROR_NOT_AVAILABLE;
 
   *entry = node->LookupEntryByRealm(realm);
+  LOG(("  returning %p", *entry));
   return *entry ? NS_OK : NS_ERROR_NOT_AVAILABLE;
 }
 
 nsresult nsHttpAuthCache::SetAuthEntry(const char* scheme, const char* host,
                                        int32_t port, const char* path,
                                        const char* realm, const char* creds,
                                        const char* challenge,
                                        nsACString const& originSuffix,
                                        const nsHttpAuthIdentity* ident,
                                        nsISupports* metadata) {
   nsresult rv;
 
-  LOG(
-      ("nsHttpAuthCache::SetAuthEntry [key=%s://%s:%d realm=%s path=%s "
-       "metadata=%p]\n",
-       scheme, host, port, realm, path, metadata));
+  LOG(("nsHttpAuthCache::SetAuthEntry %p [realm=%s path=%s metadata=%p]\n",
+       this, realm, path, metadata));
 
   nsAutoCString key;
   nsHttpAuthNode* node = LookupAuthNode(scheme, host, port, originSuffix, key);
 
   if (!node) {
     // create a new entry node and set the given entry
     node = new nsHttpAuthNode();
+    LOG(("  new nsHttpAuthNode %p for key='%s'", node, key.get()));
     rv = node->SetAuthEntry(path, realm, creds, challenge, ident, metadata);
     if (NS_FAILED(rv))
       delete node;
     else
       mDB.Put(key, node);
     return rv;
   }
 
   return node->SetAuthEntry(path, realm, creds, challenge, ident, metadata);
 }
 
 void nsHttpAuthCache::ClearAuthEntry(const char* scheme, const char* host,
                                      int32_t port, const char* realm,
                                      nsACString const& originSuffix) {
   nsAutoCString key;
   GetAuthKey(scheme, host, port, originSuffix, key);
+  LOG(("nsHttpAuthCache::ClearAuthEntry %p key='%s'\n", this, key.get()));
   mDB.Remove(key);
 }
 
 nsresult nsHttpAuthCache::ClearAll() {
-  LOG(("nsHttpAuthCache::ClearAll\n"));
+  LOG(("nsHttpAuthCache::ClearAll %p\n", this));
   mDB.Clear();
   return NS_OK;
 }
 
 //-----------------------------------------------------------------------------
 // nsHttpAuthCache <private>
 //-----------------------------------------------------------------------------
 
 nsHttpAuthNode* nsHttpAuthCache::LookupAuthNode(const char* scheme,
                                                 const char* host, int32_t port,
                                                 nsACString const& originSuffix,
                                                 nsCString& key) {
   GetAuthKey(scheme, host, port, originSuffix, key);
-  return mDB.Get(key);
+  nsHttpAuthNode* result = mDB.Get(key);
+
+  LOG(("nsHttpAuthCache::LookupAuthNode %p key='%s' found node=%p", this,
+       key.get(), result));
+  return result;
 }
 
 NS_IMPL_ISUPPORTS(nsHttpAuthCache::OriginClearObserver, nsIObserver)
 
 NS_IMETHODIMP
 nsHttpAuthCache::OriginClearObserver::Observe(nsISupports* subject,
                                               const char* topic,
                                               const char16_t* data_unicode) {
@@ -172,16 +180,18 @@ nsHttpAuthCache::OriginClearObserver::Ob
     return NS_ERROR_FAILURE;
   }
 
   mOwner->ClearOriginData(pattern);
   return NS_OK;
 }
 
 void nsHttpAuthCache::ClearOriginData(OriginAttributesPattern const& pattern) {
+  LOG(("nsHttpAuthCache::ClearOriginData %p", this));
+
   for (auto iter = mDB.Iter(); !iter.Done(); iter.Next()) {
     const nsACString& key = iter.Key();
 
     // Extract the origin attributes suffix from the key.
     int32_t colon = key.FindChar(':');
     MOZ_ASSERT(colon != kNotFound);
     nsDependentCSubstring oaSuffix = StringHead(key, colon);
 
--- a/netwerk/protocol/http/nsHttpChannelAuthProvider.cpp
+++ b/netwerk/protocol/http/nsHttpChannelAuthProvider.cpp
@@ -394,24 +394,24 @@ nsresult nsHttpChannelAuthProvider::GenC
   if (NS_FAILED(rv)) return rv;
 
     // don't log this in release build since it could contain sensitive info.
 #ifdef DEBUG
   LOG(("generated creds: %s\n", *result));
 #endif
 
   return UpdateCache(auth, scheme, host, port, directory, realm, challenge,
-                     ident, *result, generateFlags, sessionState);
+                     ident, *result, generateFlags, sessionState, proxyAuth);
 }
 
 nsresult nsHttpChannelAuthProvider::UpdateCache(
     nsIHttpAuthenticator* auth, const char* scheme, const char* host,
     int32_t port, const char* directory, const char* realm,
     const char* challenge, const nsHttpAuthIdentity& ident, const char* creds,
-    uint32_t generateFlags, nsISupports* sessionState) {
+    uint32_t generateFlags, nsISupports* sessionState, bool aProxyAuth) {
   nsresult rv;
 
   uint32_t authFlags;
   rv = auth->GetAuthFlags(&authFlags);
   if (NS_FAILED(rv)) return rv;
 
   // find out if this authenticator allows reuse of credentials and/or
   // challenge.
@@ -421,19 +421,23 @@ nsresult nsHttpChannelAuthProvider::Upda
       0 != (authFlags & nsIHttpAuthenticator::REUSABLE_CHALLENGE);
 
   bool saveIdentity =
       0 == (generateFlags & nsIHttpAuthenticator::USING_INTERNAL_IDENTITY);
 
   // this getter never fails
   nsHttpAuthCache* authCache = gHttpHandler->AuthCache(mIsPrivate);
 
-  nsCOMPtr<nsIChannel> chan = do_QueryInterface(mAuthChannel);
   nsAutoCString suffix;
-  GetOriginAttributesSuffix(chan, suffix);
+  if (!aProxyAuth) {
+    // We don't isolate proxy credentials cache entries with the origin suffix
+    // as it would only annoy users with authentication dialogs popping up.
+    nsCOMPtr<nsIChannel> chan = do_QueryInterface(mAuthChannel);
+    GetOriginAttributesSuffix(chan, suffix);
+  }
 
   // create a cache entry.  we do this even though we don't yet know that
   // these credentials are valid b/c we need to avoid prompting the user
   // more than once in case the credentials are valid.
   //
   // if the credentials are not reusable, then we don't bother sticking
   // them in the auth cache.
   rv = authCache->SetAuthEntry(scheme, host, port, directory, realm,
@@ -1388,17 +1392,17 @@ NS_IMETHODIMP nsHttpChannelAuthProvider:
   ParseRealm(mCurrentChallenge.get(), realm);
 
   rv = GetAuthorizationMembers(mProxyAuth, scheme, host, port, directory, ident,
                                unusedContinuationState);
   if (NS_FAILED(rv)) return rv;
 
   rv = UpdateCache(auth, scheme.get(), host, port, directory.get(), realm.get(),
                    mCurrentChallenge.get(), *ident, aGeneratedCreds, aFlags,
-                   aSessionState);
+                   aSessionState, mProxyAuth);
   MOZ_ASSERT(NS_SUCCEEDED(rv));
   mCurrentChallenge.Truncate();
 
   rv = ContinueOnAuthAvailable(nsDependentCString(aGeneratedCreds));
   MOZ_ASSERT(NS_SUCCEEDED(rv));
   return NS_OK;
 }
 
--- a/netwerk/protocol/http/nsHttpChannelAuthProvider.h
+++ b/netwerk/protocol/http/nsHttpChannelAuthProvider.h
@@ -126,17 +126,18 @@ class nsHttpChannelAuthProvider final : 
 
   // Store credentials to the cache when appropriate aFlags are set.
   MOZ_MUST_USE nsresult UpdateCache(nsIHttpAuthenticator* aAuth,
                                     const char* aScheme, const char* aHost,
                                     int32_t aPort, const char* aDirectory,
                                     const char* aRealm, const char* aChallenge,
                                     const nsHttpAuthIdentity& aIdent,
                                     const char* aCreds, uint32_t aGenerateFlags,
-                                    nsISupports* aSessionState);
+                                    nsISupports* aSessionState,
+                                    bool aProxyAuth);
 
  private:
   nsIHttpAuthenticableChannel* mAuthChannel;  // weak ref
 
   nsCOMPtr<nsIURI> mURI;
   nsCOMPtr<nsProxyInfo> mProxyInfo;
   nsCString mHost;
   int32_t mPort;