542318 - Firefox 3.6 needs constant re-auths when connecting through NTLM proxy with credentials different from currently logged in Windows user

Status: RESOLVED FIXED

(Core :: Networking: HTTP, defect)

Description

[Habeeb Dghaily]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US) AppleWebKit/532.0 (KHTML, like Gecko) Chrome/3.0.195.38 Safari/532.0
Build Identifier: Firefox 3.6

On Firefox 3.5.7, the browser would only ask for
authentication once when i open the browser (authentication-username&
password-is needed to access the internet in my college WLAN), after
input, the browser goes on doing things normally, not asking for it
every again untill i close and restart the browser. Once I upgraded to
3.6, it all got insanely terrible! it started asking for authentication
every time i opened a new tab to access a new website, and multiple
times per website. It was so annoying and unproductive that I had to
switch to chrome until you guys fix it or tell me how to fix it. The
security protocol is WPA2 enterprise with PEAP and MSCHAP v2 with a
certificate. Please help me since the add-ons of chrome aren't even
close in functionality to Firefox.
Thanks in advance

Reproducible: Always

Steps to Reproduce:
1.Connect to a WPA2 enterprise with PEAP and MSCHAP v2 WLAN
2.set proxy settings (.pac file)
3.type in internet website, not INtranet. the 3.6 will ask for username/password for every tab.
Actual Results:  
It keeps on asking for username/password even after i give it the first time (was enough in 3.5.7) and sometimes multiple times per page (if the page has external ad links)

Expected Results:  
It should have asked once for the password when i first tried to get on the internet, and then process the request/reply in the background without making life so miserable.

I tried Safe Mode, but it didn't work, hence the problem is not in the add-ons / plug ins.

Comment 1

[Johnathan Nightingale [:johnath]]

Hello Habeeb,

Thanks for providing the detail you have - there's clearly something strange going on here, but we should clear things up a little.  Firefox doesn't handle wireless protocols like WPA2, those are done by your operating system. Once connected, though, many wireless access points do direct you to a web based portal to prove that you deserve to be on their network, and I think what I hear you saying is that this authentication used to work once per session, but now it's prompting you much more frequently. I'm going to update the summary to reflect that.

Is it now prompting you on every page load, or just on many of them? Is there an interval involved, maybe?

[CC'ng dolske because problems with remembered passwords and proxy auth prompts and things always remind me of him.]

Comment 2

[Justin Dolske [:Dolske]]

A log per https://wiki.mozilla.org/Firefox:Password_Manager_Debugging would help sort out what's happening.

Comment 3

[Habeeb Dghaily]

Hello Johnathan and Justin,

You are correct, the problem is not with the WPA2, which is clearly set by the network and windows is manages it. I just mentioned them to clarify and be as much correct as possible about the problem. I don't know much about code writing or the like, I don't know anything to be exact. I sent my problem to the Hendrix, and a reply email told me to file a bug, and I did. It's a great thing that you actually have people working on the problems, not like in IE.
As for your question, It prompts for every tab i open, and sometimes many times per page (i'm thinking probably because of external ads or something), but not one tab goes without prompting for it. the interval is probably less than a second between each. But once it stops, it doesn't ask for more authentication unless i open a new page.
As for the log Justin mentioned, I activated. How can I attach the file here, so you guys can take a better look at it. Can i make it a .txt file and attach it for help.
Many thanks.

Comment 4

[Johnathan Nightingale [:johnath]]

(In reply to comment #3)
> As for the log Justin mentioned, I activated. How can I attach the file here,
> so you guys can take a better look at it. Can i make it a .txt file and attach
> it for help.

Yeah - if you followed the instructions and ended up with a log file, you can click the "add an attachment" link up above the first comment to attach it here.

Comment 5

[Habeeb Dghaily]

okay. but how do i make a log file from the Error Console? I know that i can upload a file.

Comment 6

[Habeeb Dghaily]

Attachment: Text file of error counsel

Comment 7

[Habeeb Dghaily]

Console2 on mozdev.org is not compatible with 3.6 to copy them all. i copied as much as i could

Comment 8

[Justin Dolske [:Dolske]]

Hmm, the password manager log looks fairly normal, except for it getting another request for a proxy authentication after the first one is completed.

Can we get some more logging from you? See http://www.mozilla.org/projects/netlib/http/http-debugging.html -- it's a little outdated, but just set NSPR_LOG_MODULES and NSPR_LOG_FILE as it shows, and then run firefox.exe from the command prompt.

Comment 9

[Habeeb Dghaily]

The problem is there only in 3.6. It's not there in 3.5.7 (which my friends use, in the same university network), and it's present on the 3.6 that other people I know have updated. It's also not present in Chrome, Safari, or IE.

I'll get a longer log file as soon as possible.

Comment 10

[Habeeb Dghaily]

Attachment: HTTP Log file

This is a .txt file compressed. the original one was 8 MB, and it wouldn't let me upload it.

Comment 11

[Honza Bambas (:mayhemer)]

Habeeb, thanks for the log, I'm looking into it.

The proxy requires NTML auth.  I'm afraid the problem lies in this hunk:

https://bugzilla.mozilla.org/attachment.cgi?id=389653&action=diff#a/netwerk/protocol/http/src/nsHttpChannel.cpp_sec10

According to the log we receive "NTML" challenge (type 1 message), we use saved credentials to create "NTML <BASE64_CREDS>" to answer and re-request, but we still get "NTML" challenge in answer from the server (like auth would not succeed).  At that moment we ask again for credentials.  

My theory is that we failure to authenticate with saved credentials because we throw away a keep alive connection that NTML auth bounds to, but this is unconfirmed.  Continuing with analyzes.

Comment 12

[Honza Bambas (:mayhemer)]

Just confirmed with IIS and a virtual directory configured for Windows Integrated Authentication only access.  I changed the code to forcibly throw away the connection and it seems I get the described behavior (many auth prompts for a single page with lot of sub requests).

The hunk I refer to in comment 11 was added just as a workaround to make some leaks coming from nsHttpConnectionManager go away.  We can revert the hunk as a quick fix (actually revert to 1.9.1 and older code which is stable) and disable the test introducing the leak.  To really fix the leak I'm afraid the change might be too risky and complicated to take on a stable branch.

This should block next 1.9.2.x release.

Will have a testing build and patch soon.

@Habeeb: I'll soon add a URL to an experimental build to test, would you be willing to install it and run to check the problem is fixed?

Comment 13

[Honza Bambas (:mayhemer)]

Attachment: possible patch

Removing the culprit, disabling the test.

Comment 14

[Habeeb Dghaily]

Yes sure, I'll be more than glad to test it, even though i didn't understand anything of what you just wrote! lol
Is the file you last attached the patch I need to test?
should i run it in safe mode?
take care

Comment 15

[Honza Bambas (:mayhemer)]

Habeeb, please download and run this installation file.  It is installer of modified Firefox (called Namoroka) with the fix.  Then run it instead of your standard Firefox installation and give it a try.

https://build.mozilla.org/tryserver-builds/[e-mail]-542318/install/sea/542318-win32.installer.exe

Thank you!

Comment 16

[Habeeb Dghaily]

Alright, I installed Namoroka. Played around with it for a little while. Then I did the logging from the cmd like you guys told me to do with Firefox. and I got a log file. 19 MB of logging to be exact.
It's better, but the problem's not gone. in 3.6 it would, for instance, ask for the login when i clicked on any link, say from Facebook Home to Facebook Profile. Or when clicking on any link on TechCrunch. Gmail was a disaster. Now, it still asks for the login. But once i put them when loading facebook, it stops asking when i browse around the website. But still asks for every new site. It still asks for the logins every time I open an email in Gmail. It's a little better, but not by much.

Comment 17

[Habeeb Dghaily]

Attachment: Log for Namoroka

Comment 18

[Honza Bambas (:mayhemer)]

Habeeb, I'm so far not wiser from this log.  Could you please do the following:

set NSPR_LOG_MODULES=timestamp,nsHttp:5,nsSocketTransport:5

Run Namoroka to create a log and just open two different web pages to have two prompts for authentication and nothing more, then close Namoroka and upload the file.

Then, run Firefox 3.5.x to create a log, open the same two web sites as you did in Namoroka (actually do exactly the same things as in Namoroka before), close Firefox and upload the file.

This will help me to find differences in logs and hope figure out what's wrong.

Thanks for help.

Comment 19

[Habeeb Dghaily]

Alright, I did as you told me. Created two separate log files. The same ad-ons i have installed on both namoroka and 3.5.7. It opened into my homepage (www.aub.edu.lb) which is an intranet website and doesn't require any authentication (the browser did request authentication, which is due to the add ons). I then opened nytimes.com and zdnet.com
Both logs are in the zip file

Comment 20

[Habeeb Dghaily]

Attachment: Log for Namoroka and firefox

The log files in a .zip file

Comment 21

[Honza Bambas (:mayhemer)]

Cause of this bug:

- we have a valid credentials cached from a previous successful NTLM authentication
- we do a request, get 407 with "NTLM" challenge
- in nsHttpChannel::GetCredentialsForChallenge, we extract auth cache entry from the auth cache, we call ChallengeReceived on nsHttpNTLMAuth class
- that returns identityInvalid = 0; there is difference from 1.9.1 code that returns 1 !
- based on this fact the nsHttpChannel code IS NOT filling mProxyIdent (holding username, password etc info) with the auth cache entry NOR asking user to provide it, i.e. mProxyIdent remains empty
- we call GenCredsAndSetEntry that calls nsHttpNTLMAuth::GenerateCredentials (that generates type 1 NTML message) and we also do SetAuthEntry on the auth cache that overrides the existing (valid) cache entry with an empty one !
- we repeat the request, server answers with 407 "NTLM <CHALLENGE>"
- we enter nsHttpChannel::GetCredentialsForChallenge again, get now empty cache entry from the auth cache, and call ChallengeReceived
- that returns identityInvalid = 0
- we again call nsHttpNTLMAuth::GenerateCredentials that now generates the challenge answer based on credentials it gets, but we pass an empty mProxyInfo to it => the generated credentials are wrong, based on an empty user name and password
- server answers with 407 "NTML", telling us to start over again
- we enter nsHttpChannel::GetCredentialsForChallenge third time, loading again the empty cache entry from auth cache, and call ChallengeReceived
- now it returns identityInvalid = 1
- we have a cache entry that is identical to mProxyIdent (both empty); based on this we delete the cache entry as it is considered as invalid and clear the identity
- bellow in the nsHttpChannel code we prompt the user for an identity based on fact we do not have a cache entry now

Comment 22

[Honza Bambas (:mayhemer)]

And Habeeb, thanks for the last two logs, it helped me to figure this out.

Comment 23

[Honza Bambas (:mayhemer)]

And the piece of log reflecting first 6 steps from comment 21:

0[92c140]: nsHttpChannel::ProcessAuthentication [this=6202ff0 code=407]
0[92c140]: nsHttpChannel::PrepareForAuthentication [this=6202ff0]
0[92c140]: nsHttpChannel::GetAuthenticator [this=6202ff0]
0[92c140]: nsHttpChannel::GetCredentialsForChallenge [this=6202ff0 proxyAuth=1 challenges=NTLM]
0[92c140]: nsHttpAuthCache::GetAuthEntryForDomain [key=http://proxya.aub.edu.lb:3128 realm=]
0[92c140]: nsHttpNTLMAuth::ChallengeReceived [ss=0 cs=0]
0[92c140]: Force use of generic ntlm auth module: 0
0[92c140]: Default credentials allowed for proxy: 1
0[92c140]:   identity invalid = 0
0[92c140]: nsHttpNTLMAuth::GenerateCredentials
0[92c140]: nsHttpAuthCache::SetAuthEntry [key=http://proxya.aub.edu.lb:3128 realm= path= metadata=0]

Comment 24

[Honza Bambas (:mayhemer)]

(In reply to comment #21)
> - we again call nsHttpNTLMAuth::GenerateCredentials that now generates the
> challenge answer based on credentials it gets, but we pass an empty mProxyInfo
> to it => the generated credentials are wrong, based on an empty user name and
> password

Hmm.. as I read the code further, nsAuthSSPI module is trying to use credentials of the user currently logged in through the system API.  The code of nsHttpNTLMAuth seems to be correct but the problem is that we actually throw away the auth cache entry with this approach and then after logging in with user default creds failed we don't have stored user creds in the auth cache anymore.

Comment 25

[Honza Bambas (:mayhemer)]

As a workaround switch "network.automatic-ntlm-auth.allow-proxies" pref to false.  Confirmed by Habeeb in a private email.

Comment 26

[Jim Mathies [:jimm]]

Is thi

Comment 27

[Jim Mathies [:jimm]]

(In reply to comment #26)
> Is thi

Sorry, was going to ask if the NTLM issue was a dupe, we have existing bugs related to failed proxy login & repeated prompting - 

bug 478018
bug 439463
bug 286099
bug 318253

Comment 28

[Honza Bambas (:mayhemer)]

Attachment: v1

This is confirmed fix for this bug.  I have checked with reporter that he is using a local Windows account credentials different from those needed for NTLM proxy authentication, that's why attempt to use the SSPI password failed.

Comment 29

[Jim Mathies [:jimm]]

Comment on attachment 424621 [details] [diff] [review]
v1

I'm not the right reviewer for this as I'm not a peer in netwerk.

Comment 30

[Boris Zbarsky [:bzbarsky]]

Hmm....  I might be a peer, but I have no idea how this code works.  Jim, if you understand this patch, I'm happy to delegate the review to you.

Otherwise, Honza, can you explain what the patch is changing and why it's the right change in general?  I did read comment 21, but it's not clear to me which parts of that process this patch changes.

Comment 31

[Honza Bambas (:mayhemer)]

(In reply to comment #30)
> Honza, can you explain what the patch is changing and why it's the
> right change in general?  I did read comment 21, but it's not clear to me which
> parts of that process this patch changes.

First round of NTLM auth is trying to use credentials of actively logged in user, using SSPI API, so in nsHttpChannel::GetCredentialsForChallenge [1] first call to nsHttpNTLMAuth::ChallengeReceived returns identityInvalid = FALSE.  Based on this we DO NOT fill mProxyIdent with ident from the cache entry taken form the auth cache, NOR we ask user to provide it, i.e. mProxyIdent remains empty.  Then we continue by call to nsHttpChannel::GenCredsAndSetEntry that calls nsHttpNTLMAuth::GenerateCredentials and then authCache->SetAuthEntry.  SetAuthEntry then OVERWRITES the entry's ident with a blank credentials.  After we fail to authenticate with SSPI password, we try to use auth cache entry.  But based on condition at [2] we delete mProxyIdent and the cache entry and ask user to provide credentials again.  mProxyIdent is empty and the cache entry as well, i.e. they are identical.

My patch prevents the OVERWRITE of cached ident when ident passed to authCache->SetAuthEntry is empty.  Then we can use the cached entry on the second round, preventing any prompts.

bz, feel free to ask on IRC, this is complicated to explain.

[1] http://mxr.mozilla.org/mozilla-central/source/netwerk/protocol/http/src/nsHttpChannel.cpp#3475
[2] http://mxr.mozilla.org/mozilla-central/source/netwerk/protocol/http/src/nsHttpChannel.cpp#3491 (ident is at that moment a reference to mProxyIdent member)

Comment 32

[Boris Zbarsky [:bzbarsky]]

OK.  Honza and I talked this over and I think I now understand the problem, at least.

The basic setup is that we treat NTLM internally as two different auth methods: the first one tries the system credentials, and the second one tries the user-prompt-provided credentials.  When we make a request and get a 401 we try auth methods in order until we find one that works.  We do NOT simply retry the last auth method that worked.  In the digest/basic case this works out ok because after the first 401 we cache the request header that needs to be sent to authenticate, so there are no more 401s.  However in the NTLM case every single request gets a 401 response and has to be followed with a second request.  So for every pageload we start at the beginning of the auth method list and walk forward, and the first method will clobber the "good" data we had cached from the second method around.

Comment 33

[Boris Zbarsky [:bzbarsky]]

So as I see it, we have the following options to fix this:

1)  Instead of starting at the beginning of the challenge list, start with the
    challenge that succeeded last time.  I'm not sure whether this is safe and
    such, or even allowed by the spec.
2)  Instead of caching a single identity, cache one identity per challenge; use
    the cached identity for the given challenge.  In particular, that would mean
    two cached identities for NTLM.  This involves figuring out when to clear
    the whole list of cached identities and when to only clear a single cached
    entry, right?
3)  Never cache the OS-provided NTLM identity, since we can always just get it
    from the OS.  This is less general than #2 or #1, but may be safer.
4)  In the identity cache, allow caching two separate username/passwords
    depending on the challenge we're working with (again, treating NTLM as two
    separate challenges).  That involves keeping track of which NTLM challenge
    we're doing through a bunch of code, right?

Architecturally, #2 seems like the right way to fix this to me.

The attached patch is sort of a variant of #4, which makes the assumption that the ident is always empty for the OS-default NTLM and that it's never empty in any other cases, right?  Oh, and that OS-defaut NTLM is always immediately followed by prompt-the-user NTLM.  Right?

Comment 34

[Honza Bambas (:mayhemer)]

(In reply to comment #33)
> So as I see it, we have the following options to fix this:
> 
> 1)  Instead of starting at the beginning of the challenge list, start with the
>     challenge that succeeded last time.  I'm not sure whether this is safe and
>     such, or even allowed by the spec.
> 2)  Instead of caching a single identity, cache one identity per challenge; use
>     the cached identity for the given challenge.  In particular, that would
> mean
>     two cached identities for NTLM.  This involves figuring out when to clear
>     the whole list of cached identities and when to only clear a single cached
>     entry, right?
> 3)  Never cache the OS-provided NTLM identity, since we can always just get it
>     from the OS.  This is less general than #2 or #1, but may be safer.
> 4)  In the identity cache, allow caching two separate username/passwords
>     depending on the challenge we're working with (again, treating NTLM as two
>     separate challenges).  That involves keeping track of which NTLM challenge
>     we're doing through a bunch of code, right?
> 
> Architecturally, #2 seems like the right way to fix this to me.
> 
Actually all solutions, 1-4, need some identification that ntlm has used system to get credentials.  This leads me to my other idea of letting GenerateCredentials return a flag "DO_NOT_CACHE_IDENTITY_AS_I_AM_USING_THE_SYSTEM_ONE".  There is no way to distinguish the challenge, as it is always "NTLM".  It is just our decision to use SSPI API or not.

I'm not that well familiar with the continuation state and session state objects, but probably the session state object (that seems to stick to the cache entry for the browser lifetime) might keep info that system credentials failed to auth last time and bypass to use them. We would not get to this trouble again.

> The attached patch is sort of a variant of #4, which makes the assumption that
> the ident is always empty for the OS-default NTLM and that it's never empty in
> any other cases, right?

Yes.  The risk on my patch might be that it doesn't let us delete the identity from the cache entry while we desire to keep the cache entry itself...

> Oh, and that OS-defaut NTLM is always immediately
> followed by prompt-the-user NTLM.  Right?

As we don't have other option, yes.

Comment 35

[Boris Zbarsky [:bzbarsky]]

> This leads me to my other idea of letting GenerateCredentials return a flag
> "DO_NOT_CACHE_IDENTITY_AS_I_AM_USING_THE_SYSTEM_ONE".

I think this is probably the way to go...

Comment 36

[Sven]

Hi guys,

I am seeing the same issue here with one of the proxy products I support.
current FF 3.6 is prompting for credentials over and over again and keeps on using the local credentials instead of the manual credentials entered for the next connection.

Any chance to get a build with the patch mentioned above to test it? i got a test lab setup that can easily reproduce the issue and it should be easy and quick to test a patch if I can get a build.

Thanks
Sven

Comment 37

[Boris Zbarsky [:bzbarsky]]

Sven, the attached patch isn't quite right; Honza is working on a better one.  If you really want I can spin up a test build with the above patch, of course.  Just let me know.

Comment 38

[Honza Bambas (:mayhemer)]

Attachment: v2

Final patch, currently uploaded to try server. I will post a URI here when it is done, anyone, feel free to test it.

Comment 39

[Sven]

Hi Guys,

thanks for the feedback.
I am standing by for the test version and will fire it up here as soon as i get it.

I just wanted to reconfirm one thing.
I know that in FF 3.6 the way NTLM auth is handled has been changed. The problem discussed here only occurs if I use the new (default) way for NTLM. If i set the pref network.auth.force-generic-ntlm set to “true” then the problem goes away. Is this expected and would the fix be expected to address the "new" way?

thanks
sven

Comment 40

[Honza Bambas (:mayhemer)]

(In reply to comment #39)
> Is this expected and would the fix be expected to address the "new"
> way?

For the first question: yes. For the second question, if I understand correctly: yes as well.

Comment 41

[Sven]

perfect, thank you Honza.
Please let me know when the testbuild is available.

Comment 42

[Honza Bambas (:mayhemer)]

Attachment: v2.1

Adjusted patch, fixing build failures.

Reference to the windows test build installer with this patch: https://build.mozilla.org/tryserver-builds/[e-mail]-542318-4/install/sea/

Comment 43

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 425285 [details] [diff] [review]
v2.1

>+++ b/netwerk/protocol/http/public/nsIHttpAuthenticator.idl
>+[scriptable, uuid(36402c9d-c280-4860-b4b0-2e7eb35b0aaf)]

Do we plan to backport this to 3.6, btw?  If so, I assume we'll need to add an
interface instead of revving, right?

>+    /**
>+     * Indicates that the authenticator has used internal source of identity
>+     * telling the consumer it must not cache/use identity it currently holds
>+     * because it might not be valid and would overwrite the cached identity.
>+     * See bug 542318 comment 32.

Perhaps:

  Indicates that the authenticator has used an out-of-band or internal source
  of identity and tells the consumer that it must not cache the returned
  identity because it might not be valid and would overwrite the cached
  identity.  See bug 542318 comment 32.
  
>+++ b/netwerk/protocol/http/src/nsHttpAuthCache.cpp
>@@ -408,21 +408,33 @@ nsHttpAuthEntry::Set(const char *path,
>+        // if we are not given an identity but our cached identity has not been
>+        // so far initialized (is currently empty), do so here by filling it
>+        // with nulls
>+        // some other code expects that mIdent keeps empty null terminated
>+        // strings in it after this code has been executed, what following
>+        // call ensures.

Perhaps:

  If we are not given an identity and our cached identity has not been
  initialized yet (so is currently empty), initialize it now by filling it with
  nulls.  We need to do that because consumers expect that mIdent is
  initialized after this function returns.

r=bzbarsky with those changes.

Comment 44

[Honza Bambas (:mayhemer)]

Attachment: v2.2 [Checkin comment 49]

Adjusted to review comments.

And yes, we have to backport it to 1.9.2, I'll have a patch for that soon, not modifying the interface.

Comment 45

[Honza Bambas (:mayhemer)]

Sven, Habeeb, have you anyone tried the test build?  Would be great to get confirmed from you the patch fixes the problem.

Download it here: https://build.mozilla.org/tryserver-builds/[e-mail]-542318-4/install/sea/

Comment 46

[Sven]

Hi Honza,

I just finished by tests here and the fix seems to be working perfectly.
I got screwed first because Namoroka uses the same prefs as my in parallel installed firefox 3.6 and that had the new ntlm stuff disabled.
Anyway, long story short, I am back to default prefs, FF 3.6 is prompting over and over and your fix build does not.
Did some tcpdumps on it and it looks all good.

Thanks so much for the quick fix.
Any kind of ETA for an official release for the patch?

thanks
sven

Comment 47

[Honza Bambas (:mayhemer)]

Sven, thanks for testing.  I first have to land the patch on the trunk that is currently closed because of the 1.9.3a1 freeze.  I hope it opens soon.  Then, I'll create a patch for 1.9.2 branch (Firefox 3.6) and request approval, so, it should get to one of upcoming 3.6.x minor releases, I don't have a picture of exact timing.

Comment 48

[Honza Bambas (:mayhemer)]

Attachment: v2.2. for 1.9.2 branch [Checkin comment 50]

Comment 49

[Honza Bambas (:mayhemer)]

Comment on attachment 425302 [details] [diff] [review]
v2.2 [Checkin comment 49]

http://hg.mozilla.org/mozilla-central/rev/ab20dd6c7d09

Comment 50

[Honza Bambas (:mayhemer)]

Comment on attachment 425637 [details] [diff] [review]
v2.2. for 1.9.2 branch [Checkin comment 50]

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/b9bcdca316b0

Comment 51

[Marco Zehe (:MarcoZ)]

Verified by affected user using a try-server build. See comment#46.