256949 - browser not using the right credentials for proxy authentication (NTLM)

Status: RESOLVED FIXED

(Core :: Networking: HTTP, defect)

Description

[yavorvm]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8a3) Gecko/20040825
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8a3) Gecko/20040825

i am using an HTTP proxy, and i have saved my password for it.
After i installed the lates build from 25th Aug browser asks me not only on the
startup page but on every link followed and sometimes several times per page.
I don't enter anything just press enter and page continues to load or the link
is followed. But it is annoying

Reproducible: Always
Steps to Reproduce:
1.Set an HTTP proxy with no anonymous access
2. try to load a page
3. password manager window appear several times



Expected Results:  
once the password is provided there shouldn't be any more prompts

Comment 1

[yavorvm]

i see where the problem is
i am using proxy that i have different username  for, not the login name i use
in windows. However when i view the proxylog it has registered connection
attepmts with my windows logon name. Meaning the browser is not forwarding the
credentials right and proxy asks for authentication. That's why the prompt
occurs every time.

here is it as an example
windows login username: wintest
proxy login name proxytest (stored in password manager)
log says attempt from wintest 
connection shoudl be all made from proxytest user

Comment 2

[yavorvm]

it still exists in 26 build
i can not surf at all
request to the proxy are being made with my win logon name, not the one saved in
password manager for the proxy


earlier versions don't have this one

Comment 3

[Darin Fisher]

Perhaps this was caused by the patch for bug 231529.  Can you please try setting
the preference network.automatic-ntlm-auth.allow-proxies to false?  You can use
about:config to change the value of this preference.

I would also greatly appreciate it if you could attach a Mozilla HTTP log file
generated using the steps given here:

  http://www.mozilla.org/projects/netlib/http/http-debugging.html

Thanks!

Comment 4

[yavorvm]

thanks mister fisher

now it works fine. The proxy we are using is MS ISA array
now it won't ask.Just when the first window is started

So the automatic proxy authentication option is the one that makes my browser
authenticate with my windows logon name, not the one for my ISA server
Is this correct?

Do you still need the log file?
and do i have to do this on every new build?Is there going to be any fix for this? 



(In reply to comment #3)
> Perhaps this was caused by the patch for bug 231529.  Can you please try setting
> the preference network.automatic-ntlm-auth.allow-proxies to false?  You can use
> about:config to change the value of this preference.
> 
> I would also greatly appreciate it if you could attach a Mozilla HTTP log file
> generated using the steps given here:
> 
>   http://www.mozilla.org/projects/netlib/http/http-debugging.html
> 
> Thanks!

Comment 5

[roberto]

Attachment: firefox 1.0PR log

Here I attach the log file generated by firefox 1.0PR.I was requested for the
username and the password once while contacting http://www.google.it, once
while contacting http://forums.mozillazine.org, and a theird time with
http://texturizer.net/firefox/index.html.

Thanks for your help :)

Comment 6

[Darin Fisher]

yavorvm: no further information is needed.  i think i understand the full
problem now.

we need to fix this for firefox 1.0

Comment 7

[Darin Fisher]

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

Comment 8

[Darin Fisher]

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

Comment 9

[Darin Fisher]

Allen,

In bug 231529, you said:

> [snip]
> I can just turn the allow proxy setting off. But in this case it is not 
> possible to have completely silent NLTM. 
>
> IE doesn't have that problem though. I wonder how it does it.

I too am very interested in finding out how IE avoids asking you for your login
credentials in this case.  Would it be possible for you to capture a TCP packet
trace for me?  There's an excellent tool available for free from
www.ethereal.com that runs on windows.  If you do not wish to share the TCP
packet trace with everyone, then you can just send it directly to me and I will
keep its contents confidential.  I will only share what I find from reading the
log file.  If you can do this for me, then what would be very helpful is a TCP
packet trace of the login session with IE and two separate TCP packet traces of
the login session with Mozilla (once with the default preferences, and once with
network.automatic-ntlm-auth.allow-proxies set to false).  THANKS IN ADVANCE !!

Comment 10

[Allen Ziegenfus]

I'll do my best. Do you know of any other free apps other than ethereal? I've
had some problems with win32 ethereal crashing the whole machine and I'm not
excited about installing it on my laptop. 

Comment 11

[Allen Ziegenfus]

(In reply to comment #10)
> I'll do my best. Do you know of any other free apps other than ethereal? I've
> had some problems with win32 ethereal crashing the whole machine and I'm not
> excited about installing it on my laptop. 

Actually forget that. I think I was thinking of nmap for windows. In any case
I've sent you the logs. 

Comment 12

[Darin Fisher]

> I've sent you the logs. 

I analyzed the log files, and low-and-behold it is true.  IE is magically
selecting the correct domain instead of using the local machine's name as the
domain.  Mozilla is using Microsoft's SSPI API here, and we don't have any
control over the domain it selects to use.  I guess this means that IE is using
some private Win32 API or something like that :-(

The one thing we can do is fix the prompting issue.  Investigating...

Comment 13

[Darin Fisher]

Attachment: v0 experimental patch

This is an experimental patch that should solve the problem.  What I found was
that we were always trying SSPI and then failing over to our built-in NTLM
mech.  As a result, we were invalidating any stored login credentials each time
the SSPI mech failed.  This patch makes us remember the fact that the SSPI mech
failed (on a per auth domain basis) so that we don't needlessly retry it.  This
should fix the bug, but I've not had a chance to test it yet.  If anyone is
able to build Mozilla (or Firefox) and test out this patch, that would be most
appreciated.  Meanwhile, I'll try to find a testcase for this.

Comment 14

[Christopher Nebergall]

(In reply to comment #12)
> > I've sent you the logs. 
> 
> I analyzed the log files, and low-and-behold it is true.  IE is magically
> selecting the correct domain instead of using the local machine's name as the
> domain.

Does a user's password for the domain have to be the same as the users password
for the local machine for the login to be transparent?

Comment 15

[Darin Fisher]

> Does a user's password for the domain have to be the same as the users password
> for the local machine for the login to be transparent?

That's a good question, but in this case the fact that the domains are different
is enough to indicate that SSPI is not using the same logon credentials as IE. 
If the domains were the same, then I'd be suspicious of the password.

Comment 16

[chris hofmann]

we should either disable the feature or fix the bug ... but we shouldn't ship
1.0 as is  blocking avaiary +

sounds like the patch is close.

Comment 17

[Darin Fisher]

Attachment: patch to disable pref by default

in case we don't get a chance to complete a fix for this feature, we should
disable the pref by default.  this patch can be used to do that.

Comment 18

[Christian :Biesinger (don't email me, ping me on IRC)]

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

Comment 19

[Christian :Biesinger (don't email me, ping me on IRC)]

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

Comment 20

[Andy Townsend]

Is the "v0 experimental patch" that is attached above in a build anywhere?  I'm 
guessing that this problem is the same as that in bug 259854 and would like to 
test if it resolves the problem there, but don't have a chance to do a Windows 
build of either Firefox or Mozilla currently, so I'm not able to test the patch 
as it stands, unfortunately.

Comment 21

[Darin Fisher]

Attachment: v1 patch

Ok, the v0 patch was not quite sufficient.  This patch solves the problem
according to my testing.

The main problem occurs when a server responds with 'WWW-Authenticate:
Negotiate NTLM'.  We'll try Negotiate, find that the server (or proxy) is not
on our whitelist (or not supported in the case of a proxy), and then we'll try
NTLM.  We'll try the system NTLM authenticator, and that will fail.  After that
we will repeat from the top, trying Negotiate first and then failing over to
NTLM.  However, in the process of doing so we do not clear
nsHttpChannel::mAuthContinuationState.	As a result,
nsHttpNegotiateAuth::ChallengeReceived is passed the 'continuationState' param
that was generated by the NTLM authenticator!  The Negotiate auth code doesn't
know any better, and thinks that the continuationState is its own.  It happily
uses the continuationState, bypassing its whitelist!  Since the
continuationState supports nsIAuthModule, the Negotiate auth code happily
invokes the NTLM nsIAuthModule for NTLM credentials, which in the case of the
system NTLM auth module does not require user input to return a non-empty
result.  So, we end up in a loop, calling upon the system NTLM authenticator
repeatedly.  Each subsequent network request contains a malformed
'Authorization: Negotiate XXX' where XXX is a NTLMSSP message :-( 

The solution is to make sure that we clear mAuthContinuationState when
switching to a different auth type.  We also need to make sure that we stick
with the same auth type once we start using one on a given channel.  Otherwise,
the logic to failover from system NTLM to mozilla NTLM won't be triggered,
since it depends on the value of mAuthContinuationState.

Given all that, this patch is fairly straightforward.

Comment 22

[Darin Fisher]

NOTE: this bug can cause the browser to inadvertantly send the user's system
logon to servers, and that would be a privacy concern except for the fact that
it can only happen after the user incorrectly enters their domain logon into an
auth prompt.  so, i think that the potential threat to users is very low. 
nonetheless this is a serious bug that needs to be fixed for firefox 1.0 and the
next release off the 1.7 branch.

Comment 23

[Allen Ziegenfus]

I guess I'm missing one thing. If I've already put the correct credentials into
password manager, why does it bother figuring out the credentials on its own?

Comment 24

[Darin Fisher]

> I guess I'm missing one thing. If I've already put the correct credentials into
> password manager, why does it bother figuring out the credentials on its own?

Yeah, you'd think it would be that simple.  However, we don't currently check
password manager from the HTTP code.  Instead, the password manager is actually
implementing the prompt interface, and it magically inserts whatever saved
password you may have into that prompt.  At the networking level we do not
really know about password manager.  That said, we probably should teach the
HTTP code to check password manager as you suggest.

Comment 25

[chris hofmann]

could use some quick review help on this one so we can figure out what to do
soon.  thx

Comment 26

[Christopher Nebergall]

Review comming up.

Comment 27

[Christopher Nebergall]

Unfortunately, I've been out of the office for a few days and won’t be back in 
till Monday, so I don't currently have access to a build environment or I'd do 
some testing on my own. (Once I can get a nightly build of this I could perform 
some of the tests remotely.)

Wouldn't the same users who have problems with auto sending NTLM because they 
use a different system login than domain credentials have the same problem with 
auto-sending Negotiate? I don't believe Negotiate can currently fail over to 
another authentication type when a server side failure occurs. We should try 
the same trick there as this patch by using a session state variable to mark 
that we have tried Negotiate before and failed.   This fix would also be 
required if the client and server don’t support a common list of authentication 
types for Negotiate (SPNEGO).   I currently have this problem because I can’t 
support NTLM at the server so if Mozilla in Windows tries Negotiate-NTLM rather 
than Negotiate-Kerberos there is no smooth way for the server to force the 
browser to fail over to another authentication type (in my case basic 
authentication.)    My current work around for this bug is that I remove www-
authenticate: Negotiate from the headers and re-send the prompt to the user 
only containing www-authenticate:  basic.  (This works against IE also), this 
workaround will be broken by this patch which can not handle the current 
authentication type from being dropped from the www-authenticate list from the 
server.    One way to change the code below to handle the auth type missing is 
to make it more flexible by not handling the auth type checking in the loop but 
instead do a sanity check in each httpAuthenticator class individually by 
having the first line of the ChallengeReceived function do a sanity check to 
see if the current channel mAuthType is suitable, if not error out.   Then make 
basic not do the sanity check.   I hope one the solutions will be possible for 
Firefox 1.0 but I know time is short.

        if (NS_SUCCEEDED(rv)) {
             //
+            // if we've already selected an auth type from a previous challenge
+            // received while processing this channel, then skip others until
+            // we find a challenge corresponding to the previously tried auth
+            // type.
+            //
+            if (!mAuthType.IsEmpty() && authType != mAuthType)
+                continue;

#ifdef XP_WIN
         //
+        // our session state is non-null to indicate that we've flagged
+        // this auth domain as not accepting the system's default login.
+        //

You should clarify your comment to specify what exactly the auth domain is. It 
would be the channel (connection?), correct?   Also, wouldn't the system's 
default credentials be retried with every new channel? Maybe in the future we 
could add a method of using the auth cache so we don't retry the same 
authentication types over and over with every new connection (another new bug).
 
More to come…

Comment 28

[Christopher Nebergall]

> Wouldn't the same users who have problems with auto sending NTLM because they 
> use a different system login than domain credentials have the same problem 
with 
> auto-sending Negotiate? 

Sorry to respond to my own question but according to the following comment, my 
problem may have already been fixed.  I have no way to verify the fix currently.

http://lxr.mozilla.org/mozilla/source/extensions/negotiateauth/nsNegotiateAuthSS
PI.cpp#283


else {
284         // If there is no input token, then we are starting a new
285         // authentication sequence.  If we have already initialized our
286         // security context, then we're in trouble because it means that the
287         // first sequence failed.  We need to bail or else we might end up 
in
288         // an infinite loop.
289         if (mCtxt.dwLower || mCtxt.dwUpper) {
290             LOG(("Cannot restart authentication sequence!"));
291             return NS_ERROR_UNEXPECTED;
292         }
293 
294         ctxIn = NULL;
295     }

Comment 29

[Christopher Nebergall]

        if (NS_SUCCEEDED(rv)) {
             //
+            // if we've already selected an auth type from a previous challenge
+            // received while processing this channel, then skip others until
+            // we find a challenge corresponding to the previously tried auth
+            // type.
+            //
+            if (!mAuthType.IsEmpty() && authType != mAuthType)
+                continue;

Wouldn't this code break support using support for 2 different authentication
types during the same connection for request based auth types?

For example in IIS or Apache I could set support for Digest authentication for
one directory and Http basic authentication on another.  With this patch I don't
think I could hit both of those directories in the same connection because when
I hit the second directory it would look for the authentication type of the
first then fail because it couldn't find it. 

A fix for this would be to check to make sure that the auth type you used last
on the connection is actually present before enforcing that it is used.  Do that
and r=cneberg

I still would also like to see the checking of whether mAuthType is valid be
done in the nsIHttpAuthenticator class before or only when mContinuation state
is used.  That way the use of request based authentication types are not forced
to be dependendent on the use of a connection based state variable, but that can
probably wait for another bug.

Comment 30

[Darin Fisher]

> Wouldn't this code break support using support for 2 different authentication
> types during the same connection for request based auth types?

no, the channel object corresponds to an individual URI.  it does not correspond
to the connection.  if fetching a URI involves multiple HTTP transactions, then
the state of that sequence of transactions can be stored on the channel.  here,
mAuthType is stored on the channel, so it only applies to a particular URI being
fetched.

in other words, if the user reloads the page, mAuthType is reset to empty, and
the whole process starts over.  no history is kept between channels other than
what is stored in the auth cache in the form of session state.

moreover, i designed the mAuthType checking so that it would reset mAuthType to
empty whenever ChallengeReceived or GenerateCredentials returns an error.  the
one thing i did not do is recycle over the list of challenges, when the auth
type i'm looking for isn't present.  perhaps that would make the patch more
robust and better address the failover to basic auth requirement.

Comment 31

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 161793 [details] [diff] [review]
v1 patch

sr=bzbarsky if Christopher is OK with this.

Comment 32

[Christopher Nebergall]

>no, the channel object corresponds to an individual URI.  it does not
>correspond to the connection. 

Thanks for the clarifications. 

> one thing i did not do is recycle over the list of challenges, when the auth
> type i'm looking for isn't present.  perhaps that would make the patch more
> robust and better address the failover to basic auth requirement.

Sounds good.  r=cneberg

Comment 33

[Asa Dotzler [:asa]]

Comment on attachment 161793 [details] [diff] [review]
v1 patch

a=asa for aviary and 1.7 branch checkin.

Comment 34

[Darin Fisher]

Attachment: v1.1 patch

revised patch w/ proper failover when expected auth type not found in list of
challenges.  i modified the GetCredentials function to call itself again if it
didn't get any credentials and if mAuthType is non-empty.  i clear mAuthType
before calling GetCredentials again so as to avoid infinite recursion.	

unfortunately, my test server is down right now, so i cannot directly confirm
this revision to the patch :(

Comment 35

[Darin Fisher]

ok, my tests show that the v1.1 patch does the right thing.  i'll go ahead and
land that one.

Comment 36

[Darin Fisher]

fixed-on-trunk

Comment 37

[Darin Fisher]

fixed-aviary1.0, fixed1.7.x

Comment 38

[David G King]

It seems to me that this patch has caused the following :-

nsHttpNTLMAuth.cpp
Building deps for nsHttpNTLMAuth.cpp
/cygdrive/e/mozilla/source/mozilla/build/cygwin-wrapper g++ -march=athlon-xp -mm
mx -msse -mno-cygwin -o nsHttpNTLMAuth.o -c -DOSTYPE=\"WINNT5.1\" -DOSARCH=\"WIN
NT\" -I/cygdrive/e/mozilla/source/mozilla/netwerk/protocol/http/src/../../../bas
e/src -I../../../../dist/include/xpcom -I../../../../dist/include/string -I../..
/../../dist/include/pref -I../../../../dist/include/nkcache -I../../../../dist/i
nclude/mimetype -I../../../../dist/include/intl -I../../../../dist/include/unich
arutil -I../../../../dist/include/caps -I../../../../dist/include/xpconnect -I..
/../../../dist/include/js -I../../../../dist/include/uconv -I../../../../dist/in
clude/necko -I../../../../dist/include -I../../../../dist/include/nspr
-fno-rtti -fno-exceptions -Wall -Wconversion -Wpointer-arith -Wcast-align -Wover
loaded-virtual -Wsynth -Wno-ctor-dtor-privacy -Wno-non-virtual-dtor -Wno-long-lo
ng -pedantic -mms-bitfields -pipe  -DDEBUG -D_DEBUG -DDEBUG_David -DTRACING -g
 -DX_DISPLAY_MISSING=1 -DMOZILLA_VERSION=\"1.8a5\" -DHAVE_SNPRINTF=1 -D_WINDOWS=
1 -D_WIN32=1 -DWIN32=1 -DXP_WIN=1 -DXP_WIN32=1 -DHW_THREADS=1 -DWINVER=0x400 -DS
TDC_HEADERS=1 -DWIN32_LEAN_AND_MEAN=1 -DNO_X11=1 -D_X86_=1 -DD_INO=d_ino -DSTDC_
HEADERS=1 -Duid_t=int -Dgid_t=int -DHAVE_DIRENT_H=1 -DHAVE_GETOPT_H=1 -DHAVE_MEM
ORY_H=1 -DHAVE_UNISTD_H=1 -DHAVE_MALLOC_H=1 -DHAVE_LIBM=1 -DNO_X11=1 -DMMAP_MISS
ES_WRITES=1 -DHAVE_STRERROR=1 -DHAVE_SNPRINTF=1 -DHAVE_MEMMOVE=1 -DHAVE_RINT=1 -
DVA_COPY=va_copy -DHAVE_VA_COPY=1 -DMOZ_DEFAULT_TOOLKIT=\"windows\" -DMOZ_DISTRI
BUTION_ID=\"org.mozilla\" -DMOZ_APP_NAME=\"mozilla\" -DOJI=1 -DIBMBIDI=1 -DMOZ_V
IEW_SOURCE=1 -DMOZ_XPINSTALL=1 -DMOZ_JSLOADER=1 -DMOZ_MATHML=1 -DMOZ_LOGGING=1 -
DDETECT_WEBSHELL_LEAKS=1 -DHAVE___CXA_DEMANGLE=1 -DMOZ_DEMANGLE_SYMBOLS=1 -DMOZ_
USER_DIR=\"Mozilla\" -DMOZ_XUL=1 -DMOZ_PROFILESHARING=1 -DMOZ_PROFILELOCKING=1 -
DMOZ_DLL_SUFFIX=\".dll\" -DJS_THREADSAFE=1 -DNS_PRINT_PREVIEW=1 -DNS_PRINTING=1
-DMOZ_REFLOW_PERF=1 -DMOZ_REFLOW_PERF_DSP=1 -DMOZILLA_LOCALE_VERSION=\"1.8a\" -D
MOZILLA_REGION_VERSION=\"1.8a\" -DMOZILLA_SKIN_VERSION=\"1.5\"  -D_MOZILLA_CONFI
G_H_ -DMOZILLA_CLIENT /cygdrive/e/mozilla/source/mozilla/netwerk/protocol/http/s
rc/nsHttpNTLMAuth.cpp
e:/mozilla/source/mozilla/netwerk/protocol/http/src/nsHttpNTLMAuth.cpp:210: erro
r: extra `;'
make[6]: *** [nsHttpNTLMAuth.o] Error 1
make[6]: Leaving directory `/cygdrive/e/mozilla/source/mozilla/netwerk/protocol/
http/src'
make[5]: *** [libs] Error 2
make[5]: Leaving directory `/cygdrive/e/mozilla/source/mozilla/netwerk/protocol/
http'
make[4]: *** [libs] Error 2
make[4]: Leaving directory `/cygdrive/e/mozilla/source/mozilla/netwerk/protocol'

make[3]: *** [libs] Error 2
make[3]: Leaving directory `/cygdrive/e/mozilla/source/mozilla/netwerk'
make[2]: *** [libs] Error 2
make[2]: Leaving directory `/cygdrive/e/mozilla/source/mozilla'
make[1]: *** [alldep] Error 2
make[1]: Leaving directory `/cygdrive/e/mozilla/source/mozilla'
make: *** [alldep] Error 2

Comment 39

[Darin Fisher]

MingW bustage fixed on trunk, 1.7 branch, and aviary 1.0 branch

marking FIXED

Comment 40

[David G King]

Yup, that fixed the MinGW bustage. Thanks.

Comment 41

[Darin Fisher]

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

Comment 42

[Artem Skoretskiy]

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

Comment 43

[Jo Hermans]

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

Comment 44

[Jo Hermans]

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

Comment 45

[Jo Hermans]

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

Comment 46

[Jo Hermans]

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

Comment 47

[Darin Fisher]

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