Closed Bug 654348 Opened 13 years ago Closed 12 years ago

XMLHttpRequest incorrectly caches credentials used to create WWW-Authenticate header for HTTP Basic authentication.

Categories

(Core :: Networking: HTTP, defect)

Product:
Core
Component:
Networking: HTTP
Version:
2.0 Branch
Platform:
All
Windows Vista
Type:
defect
Priority:
Not set
Severity:
major

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla19

People

(Reporter: drusso, Assigned: mayhemer)

References

Details

(Keywords: compat, regression, Whiteboard: [parity-ie][parity-safari][parity-chrome])

Attachments

(1 file)

v1
10.83 KB, patch
Biesinger
: review+
mayhemer
: checkin+
Details | Diff | Splinter Review 
User-Agent:       Mozilla/5.0 (Windows NT 6.0; rv:2.0.1) Gecko/20100101 Firefox/4.0.1
Build Identifier: Mozilla/5.0 (Windows NT 6.0; rv:2.0.1) Gecko/20100101 Firefox/4.0.1

When making an Ajax call to a resource requiring authentication, FireFox incorrectly caches credentials that are passed on the open() method. 

When XMLHttpRequest.open() is called and credentials are passed, the credentials are cached permanently for the remainder of the session if FireFox gets a response other than 401. 

Even if XMLHttpRequest.open() is called again and new credentials are used, FireFox sends the ones used on the very first call in the session to XMLHttpRequest.open().

This "breaks" applications that use a custom HTML form in conjunction with HTTP Basic authentication. This technique is described here:

http://www.peej.co.uk/articles/http-auth-with-html-forms.html


Reproducible: Always

Steps to Reproduce:
1. Create a server-side script that validates a user/password by processing the information FireFox sends in the Authorization header. 

2. If the Authorization header details are not present, the server-side script should respond with status:

401 Authorization Required

and send header:

WWW-Authenticate: Basic realm="YourRealm"

3. If the credentials are present but not valid, the server-side script should respond with 403 Not Authorized.

4. If the credentials are present and valid, the server-side script should respond with 200 OK.

5. Create a simple HTML form that collects the credentials and uses XMLHttpRequest (called from onsubmit event) to make a request to the server-side script, passing the credentials on the open() call.

======================

The website below has sample PHP and JavaScript code that can be used to create the above setup: 

http://www.peej.co.uk/articles/http-auth-with-html-forms.html

======================

6. Start a new session and enter invalid credentials into the form. 

7. After the first Ajax call returns with the 403 response, enter valid credentials. 

8. The invalid credentials from step 6 are still sent, even though new ones are now passed on XMLHttpRequest.open().

Actual Results:  
FireFox seems to permanently cache credentials (even when new ones are passed on XMLHttpRequest.open()) after any non-401 response is received as a result of the Ajax call.



Expected Results:  
FireFox should send the new credentials passed to XMLHttpRequest.open() in step 7.

FireFox 3.6 behaves as expected. 

I've tested the problem on various Windows Vista, Windows XP, and Windows 7 machines (both 32 and 64 bits) and FireFox 4 performs incorrectly in all cases. 

FireFox 3.6 performs correctly in all cases.
Version: unspecified → 4.0 Branch
Any feedback from a security developer?
I can confirm this bug exists in OSX.

This bug also exists as specified above when using Digest authentication.
After some additional research, the problem is this (I'm not a Mozilla developer, so I may be missing something):

nsHttoChannelAuthProvider only participates at the beginning of a request. In typical usage, a username/password combination is tried, and left in the authCache, to be removed when a 401 response from the server causes the request to be repeated (a non-4XX response also leaves the info in the authCache, but this is acceptable, as the credentials can then be considered valid).

Because the AJAX/HTTPAuth hybrid solution mentioned above returns 403 on a bad username/password pair (as opposed to 401 on *no* username/password pair), nsHttpChannelAuthProvider::GetCredentialsForChallenge is never called a second time (as it would be on a typical 401 response), and thus the bad username/password pair is never cleared.

This is a bit of a structural issue - the username/password pair cached in nsHttpChannelAuthProvider should be cleared when nsHttpChannel::ProcessResponse receives a 403 (or other error that does not retry). There is no way to do this from nsHttpChannel, that I can easily see.

To contrast: Under normal circumstances, each new password attempt will (via  nsHttpChannelAuthProvider::GetCredentialsForChallenge) clear the previous password. In our cases, we are making an authenticated request via AJAX, and are returning 403 instead of 401 so the password dialog does not appear. The ChannelAuthProvider is then using the old password that did not get cleared on the previous failed attempt (since there was no 401 return to cause a second pass).

Ignoring the atypical nature of the use case described above, I think it goes without saying that when a 403 is received in response to a request with an "Authentication" header, the username and password pair that generated the 403 should not be reused.
I'm expereincing the same problem trying to authenticate multiple different users to the same URL

In my case I make connections to Exchange Webservices. The Authentication protocol used is NTLM and because of this no REALM is exchanged.

The authentication credentials are stored in a hashtable which uses the scheme, host, port and realm as key to store the credentials. See following part in file "nsHttpChannelAuthProvider.cpp" method "nsHttpChannelAuthProvider::GetCredentialsForChallenge"

    nsHttpAuthEntry *entry = nsnull;
    authCache->GetAuthEntryForDomain(scheme.get(), host, port,
                                     realm.get(), &entry);

After this the check is done if the identity is valid. When not valid and NOT identFromURI and matching credentials from cache the specified credentials are replaced by the cached credentials.

In the case where we try to create three connections to the same server with different usernames it will always replace the specified credentials by the cached version.


    // hold reference to the auth session state (in case we clear our
    // reference to the entry).
    nsCOMPtr<nsISupports> sessionStateGrip;
    if (entry)
        sessionStateGrip = entry->mMetaData;

    // for digest auth, maybe our cached nonce value simply timed out...
    PRBool identityInvalid;
    nsISupports *sessionState = sessionStateGrip;
    rv = auth->ChallengeReceived(mAuthChannel,
                                 challenge,
                                 proxyAuth,
                                 &sessionState,
                                 &*continuationState,
                                 &identityInvalid);
    sessionStateGrip.swap(sessionState);
    if (NS_FAILED(rv)) return rv;

    LOG(("  identity invalid = %d\n", identityInvalid));

    if (identityInvalid) {
        if (entry) {
            if (ident->Equals(entry->Identity())) {
                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,
                                          port, realm.get());
                entry = nsnull;
                ident->Clear();
            }
            else if (!identFromURI ||
                     nsCRT::strcmp(ident->User(),
                                   entry->Identity().User()) == 0) {
                LOG(("  taking identity from auth cache\n"));
                // the password from the auth cache is more likely to be
                // correct than the one in the URL.  at least, we know that it
                // works with the given username.  it is possible for a server
                // to distinguish logons based on the supplied password alone,
                // but that would be quite unusual... and i don't think we need
                // to worry about such unorthodox cases.
                ident->Set(entry->Identity());
                identFromURI = PR_FALSE;
                if (entry->Creds()[0] != '\0') {
                    LOG(("    using cached credentials!\n"));
                    creds.Assign(entry->Creds());
                    return entry->AddPath(path.get());
                }
            }
        }
        else if (!identFromURI) {
            // hmm... identity invalid, but no auth entry!  the realm probably
            // changed (see bug 201986).
            ident->Clear();
        }
As an extra addition. Specifying a different hostname to the same ip-address will make it work because in the cache the credentials are stored with a different key.
Component: Security → DOM: Mozilla Extensions
Product: Firefox → Core
QA Contact: firefox → general
Version: 4.0 Branch → 2.0 Branch
This sounds like a necko issue, not a DOM one.  Jeremy, thanks for the investigation!
Status: UNCONFIRMED → NEW
Component: DOM: Mozilla Extensions → Networking: HTTP
Ever confirmed: true
QA Contact: general → networking.http
(In reply to Jeremy Childs from comment #3)
> Ignoring the atypical nature of the use case described above, I think it
> goes without saying that when a 403 is received in response to a request
> with an "Authentication" header, the username and password pair that
> generated the 403 should not be reused.

According the spec and my experience, 403 has nothing to do with authentication.  If username/password provided is wrong then 401 has to be returned, not 403.


I'll take a look at this bug.
Assignee: nobody → honzab.moz
Status: NEW → ASSIGNED
What is the status of this bug? I ran into this issue this morning and was surprised to see this ticket is over a year old! Safari and and IE do not cache credentials with a 403 or 400 response. Apparently, Firefox 3.6 works correctly as well. 

Honza suggested that the server return a 401 error but I don't think that's the right approach - especially if you're trying to implement a custom login form. If the server returns a 401 after failing the initial login, Firefox opens prompts the user for their credentials with the standard pop-up. That is not what we want. What "we" are trying to do is to custom login form based on this technique:

http://www.peej.co.uk/articles/http-auth-with-html-forms.html

Is there any way we can get this resolved soon? It would be nice to maintain consistency with some of the other browsers and earlier versions of FF...
Just as an update, I tried to bypass the username/password params in the XMLHttpRequest.open() request and manually set the "Authorization" header. No luck. Firefox still passes old/invalid credentials even when username and password are set to null. Whatever credentials are being cached should be subordinate to the XMLHttpRequest.setRequestHeader() method. Here's the code:


//Set username/password
var username = "firefox";
var password = "mysecret";


//Create Authorization Header for Firefox
var headers = [];
var userAgent = navigator.userAgent.toLowerCase();
if (userAgent.indexOf("firefox") != -1) {
    headers.push(["Authorization", "Basic " + window.btoa(username + ":" + password)]);
    username = null;
    password = null;
}


//Instantiate HTTP Request
var request = ((window.XMLHttpRequest) ? new XMLHttpRequest() : new ActiveXObject("Microsoft.XMLHTTP"));
request.open("GET", url, true, username, password);
for (var i=0; i<headers.length; i++){
    var header = headers[i];
    request.setRequestHeader(header[0], header[1]);
}
request.send(null);


//Process Response
request.onreadystatechange = function(){
    if (request.readyState == 4) {
        console.log(request.status);
    }
}
Please note that this issue also prevents us from logging off programatically as described in this article:

http://www.nanodocumet.com/?p=6

And as cited as a workaround for this enhancement request:

https://bugzilla.mozilla.org/show_bug.cgi?id=287957#c20
I have come up with a temporary workaround (aka hack). Its not pretty but it seems to work (at least for me on FF 13). Hope this helps:

http://www.javaxt.com/Tutorials/Javascript/Form_Based_HTTP_Authentication
Our development team has now run headlong into this issue, and there seems to be no workaround for us to be able to Logout a user from our webapp. Our application is using Atlassian Crowd with the Apache-Crowd Connector integration to handle authentication and authorization, and, at present, basic auth is the only mechanism.

Example logout javascript:

    this.logoutUser = function() {
        $.ajax({
            type: 'GET',
            url: window.location.href,
            beforeSend : function(xhr) {
                var base64 = window.btoa('dummy:dummy');
                xhr.setRequestHeader("Authorization", "Basic " + base64);
            },
            success: function(data) {
                alert('Logged out');
            },
            error: function(xhr, ajaxOptions, thrownError) {
                console.log('Communication Error: An exception has occurred making request to URL "' + window.location.href + '": ' + xhr.statusText);
            },
            async: false,
            dataType: 'json',
            cache: false
        });
    }

When this runs in Chrome, the request uses the base64 of the 'dummy:dummy' string. In Firefox 15.0.1, on my Mac running OSX 10.7.4, the post uses the previous credentials every time, basically ignoring the entire point of logout.

Please, add my vote to getting this now incredibly ancient defect fixed. We will be asking our webapp users to use Chrome until this issue is resolved.

-thar
I think this is going to be necessary for the Gaia email app (specifically for ActiveSync). The corresponding bug for Gaia is: https://github.com/mozilla-b2g/gaia/issues/4823
blocking-basecamp: --- → ?
(In reply to Peter Borissow from comment #8)
> Apparently, Firefox 3.6 works correctly as well. 

Marking as regression, then. It would be helpful if somebody could confirm that Firefox 3.6 works as expected.

> Safari and and IE do not cache credentials with a 403 or 400 response.

Only 400, 401, and 403? Or, any 4xx response?

> Honza suggested that the server return a 401 error but I don't think that's
> the right approach - especially if you're trying to implement a custom login
> form. If the server returns a 401 after failing the initial login, Firefox
> opens prompts the user for their credentials with the standard pop-up. That
> is not what we want. What "we" are trying to do is to custom login form
> based on this technique:
> 
> http://www.peej.co.uk/articles/http-auth-with-html-forms.html
Keywords: compat, regression
Whiteboard: [parity-ie][parity-safari][parity-chrome]
(Copying my comment from bug 282547):

Sorry to be changing flags back and forth, but I found a series of magical incantations that satisfies my needs for the Gaia email app. Specifically, if I manually add the Authorization header *and* set mozAnon: true in the XHR constructor, things work as expected. This would still be nice-to-have, but I think it would only directly benefit me if bug 776171 were also fixed.
blocking-basecamp: ? → ---
I can reproduce the issue using STR from comment 0 when the username in requests in test 6 and 7 are the same (identical), but not when it is different for step 6 and step 7.  This is intentional, however I don't say it is always correct.

so doing
- "user" + "wrongpassword"
- "user" + "goodpassword"

leads to use of wrongpassword also for the second request (this bug?)

and doing
- "user_one" + "anypassowrd"
- "user_two" + "anypassowrd"

leads to a correct behavior, new (provided) credentials are sent.


I don't see a difference in code of Fx 3.6 from code of Fx 18 around this logic.

I also verified using 3.6 debug build and an automated test I have created for this bug that 3.6 behaves the same way as 18.


Unless a test case that works in 3.6 but doesn't in 4 (or 18, or 15) is provided I don't think this would be a regression.


I will test with some of the other browsers how they behave using php/js forms.


IMO, comment 12 is a different bug.  I think we overwrite user auth headers, that is a bug (not sure it has already been reported at the moment).
Will you be fixing the issue from comment 4?
(In reply to Kent James (:rkent) from comment #18)
> Will you be fixing the issue from comment 4?

It seems to be a different bug as well, more trickier.
Attached patch v1 — — Splinter Review 
Gecko prefers cached credentials over credentials given explicitly via URL for the same username.  Passing username and password via URL is the way we pass credentials from XHR open() method.  Hence this bug.

The patch is:
- introducing new flag nsIChannel::LOAD_EXPLICIT_CREDENTIALS
- when there are credentials in the URL and in the cache as well, this flag forces usage of the explicit (URL) credentials
- when the flag is not specified, behavior is unchanged
- xhr sets this flag on its channel and is the only consumer of the new flag
- automated test


(Gecko prefers cached creds because when URL passed creds are wrong, a password prompt pops up and user usually enters correct credentials.  We cache them on non-401 response.  So, it is illogical to use again the URL creds when it was detected as wrong before.  We prevent password prompt popups this way.)
Attachment #669532 - Flags: review?(cbiesinger)
Comment on attachment 669532 [details] [diff] [review]
v1

Review of attachment 669532 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/base/src/nsXMLHttpRequest.cpp
@@ +2956,5 @@
>  
>    if (mIsAnon) {
>      AddLoadFlags(mChannel, nsIRequest::LOAD_ANONYMOUS);
>    }
> +  else {

Why is this in an else?

::: netwerk/base/public/nsIChannel.idl
@@ +254,5 @@
> +     * we have cached previously. In some situations like form login using HTTP
> +     * auth via XmlHttpRequest we need to let consumers override these cached
> +     * credentials explicitely since sometimes 403 response is used to indicate
> +     * need for authentication instead of 401 that pops the dialog, undesired
> +     * in case a login form is used. And 403 doesn't delete cached credentials.

"And 403 doesn't delete cached credentials."

When I first read this, it wasn't clear to me what that sentence referred to, so perhaps rephrase as:

"This flag also makes a 403 response not delete cached credentials"
or something like that
Attachment #669532 - Flags: review?(cbiesinger) → review+
Er no, that's not right. But my point is the documentation should also mention that a _401_ reply won't clear the cache entry (the second change in your patch to the cpp)
Comment on attachment 669532 [details] [diff] [review]
v1

though you should also make sure that credentials from these requests will not get added to the auth cache, right?
(In reply to Honza Bambas (:mayhemer) from comment #20)
> (Gecko prefers cached creds because when URL passed creds are wrong, a
> password prompt pops up and user usually enters correct credentials.  We
> cache them on non-401 response.  So, it is illogical to use again the URL
> creds when it was detected as wrong before.  We prevent password prompt
> popups this way.)

Isn't that a problem though? If you're on a public computer, and Firefox has cached your credentials then whoever sits down at the computer next would be able to log in with your username.

I guess the caching would make sense for individual bad passwords, but not for *any* password. (If my actual password is X, and Firefox receives a different password Y via the URL, then the auth dialog pops up. Now if I type in X, then I'd consider it permissible for the browser to silently use X in the next request, if Y is still in the URL. But if the browser suddenly receives a third password Z instead, then I don't see how the browser can silently pretend that "this was the correct password". 

Also, I believe this bug is not only applicable to XHR. You run into the same problem if you try to make a JSONP request. If you do this, (dynamically injecting a SCRIPT element and setting the src attribute to whichever URL you wish to request), then Firefox will happily ignore the credentials you put in the URL, and just use the cached ones.

Finally, you only mention credentials passed in the URL. What about those passed by setting the Authorization header? They get overridden by the cached credentials as well.

It seems to me that the use of cached credentials just has to be done more intelligently (only if the password you get from the URL or authorization header is unchanged)
(In reply to Jesper Dam from comment #25)
> (In reply to Honza Bambas (:mayhemer) from comment #20)
> > (Gecko prefers cached creds because when URL passed creds are wrong, a
> > password prompt pops up and user usually enters correct credentials.  We
> > cache them on non-401 response.  So, it is illogical to use again the URL
> > creds when it was detected as wrong before.  We prevent password prompt
> > popups this way.)
> 
> Isn't that a problem though? If you're on a public computer, and Firefox has
> cached your credentials then whoever sits down at the computer next would be
> able to log in with your username.

This is a valid but not a new concern: when you navigate to URL http://user:pass@example.com/auth-page.html where the name and password are correct, then this URL gets "cached" in the browsers history listing where attackers can find it.  Hence the concern here is more about using Clear Recent History or Private Browsing, that both clear browsing history and authentication cache as well.  When you don't use CRH or PB then you are risking leak of your authentication info regardless the credentials are correct or not in the URL.

So I don't think this is a serious issue.

> Finally, you only mention credentials passed in the URL. What about those
> passed by setting the Authorization header? They get overridden by the
> cached credentials as well.

This is a bug we know of.  It definitely has to be fixed.  There is a workaround to prevent override just by setting LOAD_ANONYMOUS flag on the channel.
(In reply to Christian :Biesinger (don't email me, ping me on IRC) from comment #22)
> Comment on attachment 669532 [details] [diff] [review]
> v1
> 
> Review of attachment 669532 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: content/base/src/nsXMLHttpRequest.cpp
> @@ +2956,5 @@
> >  
> >    if (mIsAnon) {
> >      AddLoadFlags(mChannel, nsIRequest::LOAD_ANONYMOUS);
> >    }
> > +  else {
> 
> Why is this in an else?

Since it doesn't make sense for anonymous loads.  We don't use cache in that case.

> 
> ::: netwerk/base/public/nsIChannel.idl
> > +     * in case a login form is used. And 403 doesn't delete cached credentials.
> 

I will rephrase the comment.  Actually it is correct as it is, just hard to understand.

401 deletes the cached credentials, it is the only response code doing it.

(In reply to Christian :Biesinger (don't email me, ping me on IRC) from comment #24)
> Comment on attachment 669532 [details] [diff] [review]
> v1
> 
> though you should also make sure that credentials from these requests will
> not get added to the auth cache, right?

Hmm.. why exactly?  

We store them to the cache to make state-full schemes work ; cache is used to store state objects (maybe we should change it).

I think it is time to revisit this code as well as testing it.  We are missing automated tests for NTLM, Digest, Negotiate...
https://hg.mozilla.org/mozilla-central/rev/1be13ecfd80b
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: