patch for bug 232567 "Warn when HTTP URL auth information isn't necessary or when it's provided" r=dougt sr=dveditz

modules/libpref/src/init/all.js

@@ -536,9 +536,6 @@ pref("network.http.proxy.pipelining", false);
 // Max number of requests in the pipeline
 pref("network.http.pipelining.maxrequests" , 4);
 
-// Whether to warn if the user connects to a site with authentication
-// information that isn't required.
-pref("network.http.confirm_superfluous_auth",true);
 // </http>
 
 // This preference controls whether or not internationalized domain names (IDN)

netwerk/protocol/http/src/nsHttpChannel.cpp

@@ -96,6 +96,7 @@ nsHttpChannel::nsHttpChannel()
     , mTransactionReplaced(PR_FALSE)
     , mUploadStreamHasHeaders(PR_FALSE)
     , mAuthRetryPending(PR_FALSE)
+    , mSuppressDefensiveAuth(PR_FALSE)
     , mResuming(PR_FALSE)
 {
     LOG(("Creating nsHttpChannel @%x\n", this));
@@ -689,6 +690,10 @@ nsHttpChannel::ProcessResponse()
     // notify nsIHttpNotify implementations
     gHttpHandler->OnExamineResponse(this);
 
+    // handle unused username and password in url (see bug 232567)
+    if (httpStatus != 401 && httpStatus != 407)
+        CheckForSuperfluousAuth();
+
     // handle different server response categories
     switch (httpStatus) {
     case 200:
@@ -749,6 +754,7 @@ nsHttpChannel::ProcessResponse()
         if (NS_FAILED(rv)) {
             LOG(("ProcessAuthentication failed [rv=%x]\n", rv));
             CloseCacheEntry(NS_ERROR_ABORT);
+            CheckForSuperfluousAuth();
             rv = ProcessNormal();
         }
         break;
@@ -807,7 +813,7 @@ nsHttpChannel::ProcessNormal()
     // expiration time (bug 87710).
     if (mCacheEntry) {
         rv = InitCacheEntry();
-        if (NS_FAILED(rv)) return rv;
+        if (NS_FAILED(rv)) return rv; // XXX this early return prevents OnStartRequest from firing!
     }
 
     // Check that the server sent us what we were asking for
@@ -2245,6 +2251,23 @@ nsHttpChannel::GetCredentialsForChallenge(const char *challenge,
         }
     }
 
+    // If the defensive auth pref is set, then we'll warn the user before
+    // automatically using the identity from the URL to automatically log
+    // them into a site (see bug 232567).
+    if (identFromURI) {
+        // ask the user...
+        if (!ConfirmAuth(NS_LITERAL_STRING("AutomaticAuth"), PR_FALSE)) {
+            // calling cancel here sets our mStatus and aborts the HTTP
+            // transaction, which prevents OnDataAvailable events.
+            Cancel(NS_ERROR_ABORT);
+            // this return code alone is not equivalent to Cancel, since
+            // it only instructs our caller that authentication failed.
+            // without an explicit call to Cancel, our caller would just
+            // load the page that accompanies the HTTP auth challenge.
+            return NS_ERROR_ABORT;
+        }
+    }
+
     //
     // get credentials for the given user:pass
     //
@@ -2438,16 +2461,111 @@ nsHttpChannel::PromptForIdentity(const char *scheme,
                                                key.get(),
                                                nsIAuthPrompt::SAVE_PASSWORD_PERMANENTLY,
                                                &user, &pass, &retval);
-    if (NS_FAILED(rv))
-        return rv;
+    if (NS_FAILED(rv)) return rv;
+
+    // remember that we successfully showed the user an auth dialog
+    if (!proxyAuth)
+        mSuppressDefensiveAuth = PR_TRUE;
+
     if (!retval || !user || !pass)
-        return NS_ERROR_ABORT;
+        rv = NS_ERROR_ABORT;
+    else
+        SetIdent(ident, authFlags, user, pass);
+  
+    if (user) nsMemory::Free(user);
+    if (pass) nsMemory::Free(pass);
+    return rv;
+}
 
-    SetIdent(ident, authFlags, user, pass);
+PRBool
+nsHttpChannel::ConfirmAuth(const nsString &bundleKey, PRBool doYesNoPrompt)
+{
+    // skip prompting the user if
+    //   1) we've already prompted the user
+    //   2) we're not a toplevel channel
+    //   3) the userpass length is less than the "phishy" threshold
 
-    nsMemory::Free(user);
-    nsMemory::Free(pass);
-    return NS_OK;
+    if (mSuppressDefensiveAuth || !(mLoadFlags & LOAD_INITIAL_DOCUMENT_URI))
+        return PR_TRUE;
+
+    nsresult rv;
+    nsCAutoString userPass;
+    rv = mURI->GetUserPass(userPass);
+    if (NS_FAILED(rv) || (userPass.Length() < gHttpHandler->PhishyUserPassLength()))
+        return PR_TRUE;
+
+    // we try to confirm by prompting the user.  if we cannot do so, then
+    // assume the user said ok.  this is done to keep things working in
+    // embedded builds, where the string bundle might not be present, etc.
+
+    nsCOMPtr<nsIStringBundleService> bundleService =
+            do_GetService(NS_STRINGBUNDLE_CONTRACTID);
+    if (!bundleService)
+        return PR_TRUE;
+
+    nsCOMPtr<nsIStringBundle> bundle;
+    bundleService->CreateBundle(NECKO_MSGS_URL, getter_AddRefs(bundle));
+    if (!bundle)
+        return PR_TRUE;
+
+    nsCAutoString host;
+    rv = mURI->GetHost(host);
+    if (NS_FAILED(rv))
+        return PR_TRUE;
+
+    nsCAutoString user;
+    rv = mURI->GetUsername(user);
+    if (NS_FAILED(rv))
+        return PR_TRUE;
+
+    NS_ConvertUTF8toUTF16 ucsHost(host), ucsUser(user);
+    const PRUnichar *strs[2] = { ucsHost.get(), ucsUser.get() };
+
+    nsXPIDLString msg;
+    bundle->FormatStringFromName(bundleKey.get(), strs, 2, getter_Copies(msg));
+    if (!msg)
+        return PR_TRUE;
+    
+    nsCOMPtr<nsIPrompt> prompt;
+    GetCallback(NS_GET_IID(nsIPrompt), getter_AddRefs(prompt));
+    if (!prompt)
+        return PR_TRUE;
+
+    PRBool confirmed;
+    if (doYesNoPrompt) {
+        PRInt32 choice;
+        rv = prompt->ConfirmEx(nsnull, msg,
+                               nsIPrompt::BUTTON_TITLE_YES * nsIPrompt::BUTTON_POS_0 +
+                               nsIPrompt::BUTTON_TITLE_NO  * nsIPrompt::BUTTON_POS_1,
+                               nsnull, nsnull, nsnull, nsnull, nsnull, &choice);
+        if (NS_FAILED(rv))
+            return PR_TRUE;
+
+        confirmed = choice == 0;
+    }
+    else {
+        rv = prompt->Confirm(nsnull, msg, &confirmed);
+        if (NS_FAILED(rv))
+            return PR_TRUE;
+    }
+
+    return confirmed;
+}
+
+void
+nsHttpChannel::CheckForSuperfluousAuth()
+{
+    // check whether authentication was provided, even if not required.
+    // if so, prompt the user as to whether to continue, as this might be an
+    // attempt to spoof a different site (see bug 232567).
+    if (!mAuthRetryPending) {
+        // ask user...
+        if (!ConfirmAuth(NS_LITERAL_STRING("SuperfluousAuth"), PR_TRUE)) {
+            // calling cancel here sets our mStatus and aborts the HTTP
+            // transaction, which prevents OnDataAvailable events.
+            Cancel(NS_ERROR_ABORT);
+        }
+    }
 }
 
 void
@@ -2509,6 +2627,13 @@ nsHttpChannel::SetAuthorizationHeader(nsHttpAuthCache *authCache,
         if (creds[0]) {
             LOG(("   adding \"%s\" request header\n", header.get()));
             mRequestHead.SetHeader(header, nsDependentCString(creds));
+
+            // suppress defensive auth prompting for this channel since we know
+            // that we already prompted at least once this session.  we only do
+            // this for non-proxy auth since the URL's userpass is not used for
+            // proxy auth.
+            if (header == nsHttp::Authorization)
+                mSuppressDefensiveAuth = PR_TRUE;
         }
         else
             ident.Clear(); // don't remember the identity

netwerk/protocol/http/src/nsHttpChannel.h

@@ -158,6 +158,8 @@ private:
     void     ParseRealm(const char *challenge, nsACString &realm);
     void     GetIdentityFromURI(PRUint32 authFlags, nsHttpAuthIdentity&);
     nsresult PromptForIdentity(const char *scheme, const char *host, PRInt32 port, PRBool proxyAuth, const char *realm, const char *authType, PRUint32 authFlags, nsHttpAuthIdentity &);
+    PRBool   ConfirmAuth(const nsString &bundleKey, PRBool doYesNoPrompt);
+    void     CheckForSuperfluousAuth();
     void     SetAuthorizationHeader(nsHttpAuthCache *, nsHttpAtom header, const char *scheme, const char *host, PRInt32 port, const char *path, nsHttpAuthIdentity &ident);
     void     AddAuthorizationHeaders();
     nsresult GetCurrentPath(nsACString &);
@@ -231,6 +233,7 @@ private:
     PRUint32                          mTransactionReplaced      : 1;
     PRUint32                          mUploadStreamHasHeaders   : 1;
     PRUint32                          mAuthRetryPending         : 1;
+    PRUint32                          mSuppressDefensiveAuth    : 1;
     PRUint32                          mResuming                 : 1;
 
     class nsContentEncodings : public nsIUTF8StringEnumerator

netwerk/protocol/http/src/nsHttpHandler.cpp

@@ -140,6 +140,7 @@ nsHttpHandler::nsHttpHandler()
     , mMaxPersistentConnectionsPerProxy(4)
     , mMaxPipelinedRequests(2)
     , mRedirectionLimit(10)
+    , mPhishyUserPassLength(16)
     , mLastUniqueID(NowInSeconds())
     , mSessionStartTime(0)
     , mUserAgentIsDirty(PR_TRUE)
@@ -973,6 +974,12 @@ nsHttpHandler::PrefsChanged(nsIPrefBranch *prefs, const char *pref)
             mEnablePersistentHttpsCaching = cVar;
     }
 
+    if (PREF_CHANGED(HTTP_PREF("phishy-userpass-length"))) {
+        rv = prefs->GetIntPref(HTTP_PREF("phishy-userpass-length"), &val);
+        if (NS_SUCCEEDED(rv))
+            mPhishyUserPassLength = (PRUint8) CLAMP(val, 0, 0xff);
+    }
+
     //
     // INTL options
     //

netwerk/protocol/http/src/nsHttpHandler.h

@@ -92,6 +92,7 @@ public:
     PRUint16       MaxRequestAttempts()      { return mMaxRequestAttempts; }
     const char    *DefaultSocketType()       { return mDefaultSocketType.get(); /* ok to return null */ }
     nsIIDNService *IDNConverter()            { return mIDNConverter; }
+    PRUint32       PhishyUserPassLength()    { return mPhishyUserPassLength; }
     
     PRBool         IsPersistentHttpsCachingEnabled() { return mEnablePersistentHttpsCaching; }
 
@@ -233,6 +234,12 @@ private:
 
     PRUint8  mRedirectionLimit;
 
+    // we'll warn the user if we load an URL containing a userpass field
+    // unless its length is less than this threshold.  this warning is
+    // intended to protect the user against spoofing attempts that use
+    // the userpass field of the URL to obscure the actual origin server.
+    PRUint8  mPhishyUserPassLength;
+
     nsCString mAccept;
     nsCString mAcceptLanguages;
     nsCString mAcceptEncodings;
@@ -270,7 +277,7 @@ private:
     // if true allow referrer headers between secure non-matching hosts
     PRPackedBool   mSendSecureXSiteReferrer;
 
-    // Persitent HTTPS caching flag
+    // Persistent HTTPS caching flag
     PRPackedBool   mEnablePersistentHttpsCaching;
 };