143575 - URL: http ignores username and password (http://user:passwd/domain) for auth cache

Status: RESOLVED FIXED

(Core :: Networking: HTTP, defect, P3)

Description

[James]

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.0rc1) Gecko/20020417
BuildID:    2002041711

Upon trying to access a certain website with a specific username and password,
integrated into the URL (ie "http://username1:[e-mail]")
the access is perfect.  However, if at a later stage an attempt to access the
same site, with a different username and password (ie
"http://username2:[e-mail]") is made, Mozilla
authenticates with the original combination (username1:password1) instead of the
new combination (username2:password2).  This problem is only cleared by closing
all browser, mail and other Mozilla windows (but not necessarily the QuickLaunch
program) and reopening a new instance of the Mozilla browser.

For example: Fred has the username "fred" and the password "abcd1234" to access
his customer management system (cms.fredsisp.com.au).  Fred accesses this
website with the URL: "http://fred:[e-mail]".  Fred wanders
off to do other work, and Bob (with the username of "bob" and the password of
"wxyz7890") sits down at Fred's PC, and tries to access the cms system with the
URL: "http://bob:[e-mail]".  Mozilla, however, authenticates
with Fred's username and password, ignoring Bob's username and password, despite
the fact they are explicitly stated in the URL.  The only way Bob can access the
site using his own username and password seems to be to close all Mozilla
windows (including browsers and mail, but not necessarily the QuickLauncher) and
open a new browser window.  Then Bob can log in, but once again he needs to
close all windows if Fred needs to log back in.

I hope that this bug report is helpful, and that I've filed it in the right
place.  If I can be of any more help, or there are any questions that I can
assist with, please email me ([e-mail]).  Thanks!  :)

Reproducible: Always
Steps to Reproduce:
1. Access a password protected site by including the username and password in
the URL (ie http://username1:[e-mail]).
2. Without closing the browser window, access the exact same site, with a
different username and password in the URL (ie
http://username2:[e-mail]).

Actual Results:  Mozilla will authenticate the second time around using
'username1' and 'password1', despite the fact that 'username2' and 'password2'
are explicitly requested.

Expected Results:  After attempting to log in the first time with 'username1'
and 'password1' in the URL (ie http://username1:[e-mail]), and
then attempting to log in the second time with 'username2' and 'password2' in
the URL (ie http://username2:[e-mail]), I think that Mozilla
should detect the fact that the URL has explicitly stated a new username and
password, and supply the new username and password instead.

Comment 1

[Darin Fisher]

hmm... yeah, makes sense that you are seeing this bug.  we do in fact give
preference to the username:password that worked before.

i need to think about how best to solve this... simply preferring the one in the
URL may not always be best.

Comment 2

[Kathleen :Brade]

Darin--in what circumstances should the ones in the url not be used?
I expect that this could also affect publishing?

Comment 3

[Darin Fisher]

mass futuring of untargeted bugs

Comment 4

[John Levon]

bug 140645 ?

Comment 5

[benc]

The underyling behavior might be the same, but this example uses user|pass info
in the URl.

HTTP or some password manager-level problem?

Comment 6

[Casey Crabb]

Attachment: A fix where the cached auth data is deleted when there is auth data in the uri

This patch causes the cached http auth entry to be deleted when there is a
different username specified in the URI. This causes an additional 401 hit to
the server on changing users.

Comment 7

[Casey Crabb]

Attachment: Second attempt to solve the problem, address specifing username only.

This patch takes out the printf's I left in, changes the bools to PRBools and
augments the check for asking the user for their name/password. The check now
works like this:  if(the uri doesn't have a user, or has a user but doesn't
have a password) then ask for a user and password.  It doesn't prefill the user
in the box like it should. There is a bug open about this [66177].

Also, there is no way that I can see to tell the difference between
http://user@host/path and http://user:@host/path. There is a bug open about
this as well [32761].

Darin -- go ahead and tear apart the patch and let me know what needs to be
fixed.

Comment 8

[Casey Crabb]

Attachment: Patch against the trunk

This patch is against the mozilla trunk. The previous patches were against
mozilla 1.3. Sorry for the spam.

Comment 9

[Darin Fisher]

Comment on attachment 118776 [details] [diff] [review]
Patch against the trunk

>Index: nsHttpChannel.cpp

>+        if (!entry && (ident->IsEmpty() || (nsCRT::strlen(ident->User()) > 0 && nsCRT::strlen(ident->Password()) == 0))) {

same as:

    if (!entry && (nsCRT::strlen(ident->Password()) == 0)) {

no??  keep in mind that it can be valid to have an empty password.  if the
URL says "http://[e-mail]/" are you sure you don't want to try just sending
an empty password?


>+        if(URIContainsUserPass()) {
...
>+            mURI->GetUsername(buf);

you're copying the username out of mURI twice.	don't do that! ;-)


>+            NS_UnescapeURL(buf);
>+            iUser = ToNewUnicode(NS_ConvertASCIItoUCS2(buf));
>+            if(nsCRT::strcmp(aUser, iUser)) {
>+                authCache->ClearAuthEntry(mConnectionInfo->Host(), mConnectionInfo->Port(), entry->Realm());
>+                return;

you're leaking iUser!  also, what if ToNewUnicode fails?

instead of calling ToNewUnicode you could do this:

  NS_ConvertASCIItoUCS2 iUser(buf);
  if (iUser.Equals(aUser)) {
      // ...
  }

Comment 10

[Casey Crabb]

Attachment: Patch for specified credentials overriding cache

This patch covers a two things:

1) the credentials specified on the uri take precedence over the credentials in
cache.

2) the authenication dialog is now prefilled.

I also update the URL bar with the username that they finally authenticate
with; I clear the password from the URL bar.

Comment 11

[benc]

Trying for better summary.

nominated nsbeta, we have a couple auth bugs that are hard to separate, but I
think this might be the missing behavior we need...

Comment 12

[Casey Crabb]

Attachment: Patch against latest CVS as of May 21 2003

This cleanly applies to latest CVS. Ready for review.

Comment 13

[Samir Gehani]

adt: nsbeta1-

CC'ing Mitch for a security evaluation of this defect.

Comment 14

[Mitchell Stoltz (not reading bugmail)]

There's a moderately serious security problem here. Since we've got a patch,
let's get it in for 1.4 if possible.

Comment 15

[Darin Fisher]

*** Bug 206707 has been marked as a duplicate of this bug. ***

Comment 16

[Mitchell Stoltz (not reading bugmail)]

After talking to Darin, I no longer think this is moderately serious. It's a
"nice to have" for the next release, not a "must have."

Comment 17

[Darin Fisher]

ok, the patch is actually addressing multiple problems.  i think we should break
it into separate parts (though i seem to recall okaying their combination).

the first part should just deal with this bug specifically.  we need to address
the problem that http://foo:[e-mail]/ means try sending foo:foo.  while the
current patch takes a pretty good stab at solving this problem, i think i would
solve it in a slightly different way.  specifically, i don't agree with clearing
the auth cache when a non-matching username:password is provided in the URL
(maybe we only want to do this when the username differs).

at any rate, i'm planning to revise the patch shortly to solve this specific bug
only at first.  we should create another bug for the issue of prepopulating the
password prompt.

Comment 18

[Darin Fisher]

Attachment: v2 patch

ok, this patch is a wittled down version of the previous patch.  all it does is
guarantee that we try the username given in the URL before any username found
in the auth cache.  a couple things that this patch changes:

1) if the username given by the URL matches the username given in the auth
cache, then use the auth cache (including the password from the auth cache).

2) when challenged, if we have not yet tried the username from the URL, then
always try it.	do not use the auth cache entry unless the auth cache entry is
for the same username as that found in the URL.

3) when attempting to authenticate with the username given in the URL, do not
add the username:password from the URL into the auth cache.  this is an
important step since it preserves the original auth cache entry, which may
still be valid.  moreover, we may still want to fallover to the auth cache
entry if the username from the URL happens to be invalid.

NOTE:

a) the issue of falling over to the username found in the auth cache is bug
144256.  i don't want to deal with that bug at the moment, so i'm preserving
our current behavior w.r.t. that bug.  moreover, there are a number of tricky
issues to deal with regarding such a change.

b) i have removed the changes in the previous patch which attempted to prefill
the password prompt.  to do that correctly, we would need to modify
nsIAuthPrompt since the username and password parameters are |out| params only.
 the previous patch was using them like |inout| params which they are not.  the
previous patch would have not worked with JavaScript for example.  i think such
a change should be deferred to another bug since anyways there are many things
to consider with such a change, including the bugs having to do with why the
current behavior is the way it is.

Comment 19

[Darin Fisher]

Comment on attachment 126403 [details] [diff] [review]
v2 patch

please see comment #18 for an explanation of this patch.

Comment 20

[Alec Flett]

Comment on attachment 126403 [details] [diff] [review]
v2 patch

sr=alecf

Comment 21

[Darin Fisher]

fixed-on-trunk

we should use a different bug for the other changes covered by the original patch.

Comment 22

[Matthias Versen [:Matti]]

*** Bug 220309 has been marked as a duplicate of this bug. ***