Bug 1309438 - Don't reuse sticky connection after authentication failure. r=jduell
authorHonza Bambas <honzab.moz@firemni.cz>
Fri, 25 Nov 2016 03:33:00 -0500
changeset 324511 8300b41fe20b187bdf37f6b351c3342474a4a2db
parent 324510 9444e271b391686b1238a20c20175b8bd7d7b400
child 324512 efad76d74c06a7c0c29b43dc3d87ac5e9b8443a2
push id24
push usermaklebus@msu.edu
push dateTue, 20 Dec 2016 03:11:33 +0000
reviewersjduell
bugs1309438
milestone53.0a1
Bug 1309438 - Don't reuse sticky connection after authentication failure. r=jduell
netwerk/protocol/http/nsHttpChannel.cpp
netwerk/protocol/http/nsHttpChannel.h
netwerk/protocol/http/nsHttpChannelAuthProvider.cpp
netwerk/protocol/http/nsHttpChannelAuthProvider.h
netwerk/protocol/http/nsIHttpAuthenticableChannel.idl
--- a/netwerk/protocol/http/nsHttpChannel.cpp
+++ b/netwerk/protocol/http/nsHttpChannel.cpp
@@ -5507,16 +5507,52 @@ NS_IMETHODIMP nsHttpChannel::OnAuthCance
         if (NS_FAILED(rv))
             mTransactionPump->Cancel(rv);
     }
 
     mProxyAuthPending = false;
     return NS_OK;
 }
 
+NS_IMETHODIMP nsHttpChannel::CloseStickyConnection()
+{
+    LOG(("nsHttpChannel::CloseStickyConnection this=%p", this));
+
+    // Require we are between OnStartRequest and OnStopRequest, because
+    // what we do here takes effect in OnStopRequest (not reusing the
+    // connection for next authentication round).
+    if (!mIsPending) {
+        LOG(("  channel not pending"));
+        NS_ERROR("CloseStickyConnection not called before OnStopRequest, won't have any effect");
+        return NS_ERROR_UNEXPECTED;
+    }
+
+    MOZ_ASSERT(mTransaction);
+    if (!mTransaction) {
+        return NS_ERROR_UNEXPECTED;
+    }
+
+    if (!(mCaps & NS_HTTP_STICKY_CONNECTION ||
+          mTransaction->Caps() & NS_HTTP_STICKY_CONNECTION)) {
+        LOG(("  not sticky"));
+        return NS_OK;
+    }
+
+    RefPtr<nsAHttpConnection> conn = mTransaction->GetConnectionReference();
+    if (!conn) {
+        LOG(("  no connection"));
+        return NS_OK;
+    }
+
+    // This turns the IsPersistent() indicator on the connection to false,
+    // and makes us throw it away in OnStopRequest.
+    conn->DontReuse();
+    return NS_OK;
+}
+
 //-----------------------------------------------------------------------------
 // nsHttpChannel::nsISupports
 //-----------------------------------------------------------------------------
 
 NS_IMPL_ADDREF_INHERITED(nsHttpChannel, HttpBaseChannel)
 NS_IMPL_RELEASE_INHERITED(nsHttpChannel, HttpBaseChannel)
 
 NS_INTERFACE_MAP_BEGIN(nsHttpChannel)
--- a/netwerk/protocol/http/nsHttpChannel.h
+++ b/netwerk/protocol/http/nsHttpChannel.h
@@ -105,16 +105,17 @@ public:
     NS_IMETHOD GetProxyMethodIsConnect(bool *aProxyMethodIsConnect) override;
     NS_IMETHOD GetServerResponseHeader(nsACString & aServerResponseHeader) override;
     NS_IMETHOD GetProxyChallenges(nsACString & aChallenges) override;
     NS_IMETHOD GetWWWChallenges(nsACString & aChallenges) override;
     NS_IMETHOD SetProxyCredentials(const nsACString & aCredentials) override;
     NS_IMETHOD SetWWWCredentials(const nsACString & aCredentials) override;
     NS_IMETHOD OnAuthAvailable() override;
     NS_IMETHOD OnAuthCancelled(bool userCancel) override;
+    NS_IMETHOD CloseStickyConnection() override;
     // Functions we implement from nsIHttpAuthenticableChannel but are
     // declared in HttpBaseChannel must be implemented in this class. We
     // just call the HttpBaseChannel:: impls.
     NS_IMETHOD GetLoadFlags(nsLoadFlags *aLoadFlags) override;
     NS_IMETHOD GetURI(nsIURI **aURI) override;
     NS_IMETHOD GetNotificationCallbacks(nsIInterfaceRequestor **aCallbacks) override;
     NS_IMETHOD GetLoadGroup(nsILoadGroup **aLoadGroup) override;
     NS_IMETHOD GetRequestMethod(nsACString& aMethod) override;
--- a/netwerk/protocol/http/nsHttpChannelAuthProvider.cpp
+++ b/netwerk/protocol/http/nsHttpChannelAuthProvider.cpp
@@ -74,16 +74,17 @@ nsHttpChannelAuthProvider::nsHttpChannel
     , mIsPrivate(false)
     , mProxyAuthContinuationState(nullptr)
     , mAuthContinuationState(nullptr)
     , mProxyAuth(false)
     , mTriedProxyAuth(false)
     , mTriedHostAuth(false)
     , mSuppressDefensiveAuth(false)
     , mCrossOrigin(false)
+    , mConnectionBased(false)
     , mHttpHandler(gHttpHandler)
 {
 }
 
 nsHttpChannelAuthProvider::~nsHttpChannelAuthProvider()
 {
     MOZ_ASSERT(!mAuthChannel, "Disconnect wasn't called");
 }
@@ -787,16 +788,28 @@ nsHttpChannelAuthProvider::GetCredential
                                  &sessionState,
                                  &*continuationState,
                                  &identityInvalid);
     sessionStateGrip.swap(sessionState);
     if (NS_FAILED(rv)) return rv;
 
     LOG(("  identity invalid = %d\n", identityInvalid));
 
+    if (mConnectionBased && identityInvalid) {
+        // If the flag is set and identity is invalid, it means we received the first
+        // challange for a new negotiation round after negotiating a connection based
+        // auth failed (invalid password).
+        // The mConnectionBased flag is set later for the newly received challenge,
+        // so here it reflects the previous 401/7 response schema.
+        mAuthChannel->CloseStickyConnection();
+        mConnectionBased = false;
+    }
+
+    mConnectionBased = !!(authFlags & nsIHttpAuthenticator::CONNECTION_BASED);
+
     if (identityInvalid) {
         if (entry) {
             if (ident->Equals(entry->Identity())) {
                 if (!identFromURI) {
                     LOG(("  clearing bad auth cache entry\n"));
                     // ok, we've already tried this user identity, so clear the
                     // corresponding entry from the auth cache.
                     authCache->ClearAuthEntry(scheme.get(), host,
@@ -1300,16 +1313,24 @@ NS_IMETHODIMP nsHttpChannelAuthProvider:
 {
     LOG(("nsHttpChannelAuthProvider::OnAuthCancelled [this=%p channel=%p]",
         this, mAuthChannel));
 
     mAsyncPromptAuthCancelable = nullptr;
     if (!mAuthChannel)
         return NS_OK;
 
+    // When user cancels or auth fails we want to close the connection for
+    // connection based schemes like NTLM.  Some servers don't like re-negotiation
+    // on the same connection.
+    if (mConnectionBased) {
+        mAuthChannel->CloseStickyConnection();
+        mConnectionBased = false;
+    }
+
     if (userCancel) {
         if (!mRemainingChallenges.IsEmpty()) {
             // there are still some challenges to process, do so
             nsresult rv;
 
             // Get rid of current continuationState to avoid reusing it in
             // next challenges since it is no longer relevant.
             if (mProxyAuth) {
--- a/netwerk/protocol/http/nsHttpChannelAuthProvider.h
+++ b/netwerk/protocol/http/nsHttpChannelAuthProvider.h
@@ -169,17 +169,18 @@ private:
     // response. Used in OnAuthAvailable and OnAuthCancelled callbacks.
     uint32_t                          mProxyAuth                : 1;
     uint32_t                          mTriedProxyAuth           : 1;
     uint32_t                          mTriedHostAuth            : 1;
     uint32_t                          mSuppressDefensiveAuth    : 1;
 
     // If a cross-origin sub-resource is being loaded, this flag will be set.
     // In that case, the prompt text will be different to warn users.
-    uint32_t                          mCrossOrigin              : 1;
+    uint32_t                          mCrossOrigin : 1;
+    uint32_t                          mConnectionBased : 1;
 
     RefPtr<nsHttpHandler>           mHttpHandler;  // keep gHttpHandler alive
 
     // A variable holding the preference settings to whether to open HTTP
     // authentication credentials dialogs for sub-resources and cross-origin
     // sub-resources.
     static uint32_t                   sAuthAllowPref;
     nsCOMPtr<nsICancelable>           mGenerateCredentialsCancelable;
--- a/netwerk/protocol/http/nsIHttpAuthenticableChannel.idl
+++ b/netwerk/protocol/http/nsIHttpAuthenticableChannel.idl
@@ -100,9 +100,16 @@ interface nsIHttpAuthenticableChannel : 
      * Notifies that the prompt was cancelled. It is called asynchronously
      * from nsIHttpChannelAuthProvider::processAuthentication if that method
      * returns NS_ERROR_IN_PROGRESS.
      *
      * @param userCancel
      *        If the user was cancelled has cancelled the authentication prompt.
      */
     void onAuthCancelled(in boolean userCancel);
+
+    /**
+     * Tells the channel to drop and close any sticky connection, since this
+     * connection oriented schema cannot be negotiated second time on
+     * the same connection.
+     */
+    void closeStickyConnection();
 };