159015 - Implement windows NTLM authentication using SSPI

Status: VERIFIED FIXED

(Core :: Networking, enhancement)

Description

[Daniel van der Zee]

Bug 23679 (NTLM auth for HTTP) is an rfe for implementing crossplatform NTLM 
authentication, enabling mozilla to talk to MS web and proxy servers that are 
configured to use "windows integrated security". For the following 2 reasons, 
the windows implementation of this should use SSPI instead of a cross-platform 
solution:

1) SSPI can by used to send the credentials of the user running the browser 
without any user interaction. In an intranet situation (workstation and 
web/proxy server share the same user database) this means seamless 
authentication. IMHO, this is usually how windows authentication is used. I am 
not sure if Mozilla would be able to grab a hold of these credentials and use 
them for network authentication without using SSPI (I doubt it).

2) The first step of an SSPI driven authentication negotiates the 
authentication protocol. NTLM is but one possible outcomes. Two windows 2000 
machines might choose to use MS's kerebos implementation instead of NTLM. If 
memory serves me, it is possible to disable NTLM altogether in a pure windows 
2000+ domain, my guess is that then IIS/MS Proxy server will require kerebos 
instead of NTLM to authenticate the connection and the solutions proposed in 
the bug won't work. 

SSPI documentation can be found on (thanks to Jason Prell for the ptrs): 
http://www.microsoft.com/windows2000/techinfo/howitworks/security/sspi2000.asp
http://msdn.microsoft.com/library/en-us/security/security/initializing_sspi.asp

Comment 1

[Daniel van der Zee]

Wrong dependency. Sorry for the spam.

Comment 2

[Daniel van der Zee]

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

Comment 3

[pohl]

cc

Comment 4

[Jason Prell]

Added blocks 35159
Proxy: MS Proxy 2.0 and ISA auth fails (NTLM support needed)

Comment 5

[Martin Kutschker]

NTLM is also used for other protocols like POP3, IMAP and NNTP. I guess they do
LDAP authentication as well.

So I suggest using SSPI in a more general way and therefor changing the bug's
component accordingly to "Networking".

Comment 6

[Andrew Hagen]

This bug will be functionally comparable to the fix for bug 23679, but this will
be a Windows-only implementation, using the built-in Microsoft tools, namely SSPI. 

Multiple protocols, thus just Browser/networking. Reassign.

We have to assume that Microsoft won't change the API. If/when they make
changes, only then does that the dodgy API become an issue. Anyone have an idea
of the difficulty of this implementation, assuming the API remains the same? 

Comment 7

[Jason Prell]

I have found this helpful:

Documentation:
http://www.develop.com/kbrown/security/sample_sspibench.htm

Utility:
http://www.develop.com/kbrown/security/code/sspi_workbench.zip

I could help implement this in VB, but I don't know if that will help since 
mozilla is Java.

Comment 8

[Daniel van der Zee]

SSPI has been around since windows 95/NT4 (3.51?), the API itself is very 
unlikely to change. What could change is the way IIS expects the http headers 
to contain the SSPI generated data.

I have some SSPI client code lying around. If bug 23679 comes up with a working 
implementation, it means the rest of netwerk can handle this type of 
authentication (authenticate a persistent connection instead of a individual 
HTTP requests). Plugging in SSPI on windows would then be a semi-trivial task.

Had a brief e-mail exchange with [e-mail]. Bug 23679 is apparently 
being worked on but making a windows specific implementation is lower 
priority.  

Feel free to depend this one on bug 23679 and assign to me.

Comment 9

[Andrew Hagen]

Depends on the NTLM bug. Reassign. 

Comment 10

[Andrew Bartlett]

I beleive this is the 'correct' way to solve this issue on the MS Windows
platform.  By taking advantage of the same API MS uses, you ensure maximum
compatiblity.  NTLMSSP is not simple, and if we can avoid having to actually
deal with it at all (ie just pass blobs) then I think this is a big plus.

On other platforms, I'm proposing a Samba based solution that involves an
independent NTLMSSP implementation.

Comment 11

[Michael T. Babcock]

May I ask how this depends on 23679?

Comment 12

[Darin Fisher]

OK, i saw the light ;-)

... last night i hacked together a patch to get mozilla using SSPI and things
mostly work.  the patch i'm about to attach is by no means final.  it has many
serious problems, but i'm just submitting it here so people can try it out and
let me know if they discover any other problems that i'm not expecting.

Comment 13

[Darin Fisher]

Attachment: v0.000 patch

this patch is at best a hack.

Comment 14

[Darin Fisher]

things that still need to be done:

1- implement auth prompt with domain field.  currently, you must enter your
username as "username\domain"  ... this is just for testing purposes ;-)

2- try default credentials first before prompting user.

3- come up with a better way of suppressing Authorization headers on an already
authenticated connection.  current method is a total hack.

4- aggressively send first auth header if URL looks like it will require
authentication.  currently we only do NTLM auth when challenged.

5- i hacked around the problem of needing to reuse the same connection for the
second auth header by delaying the second HTTP transaction until the first is
finished (instead of firing it off as soon as we read the header with the
challenge as is usually done).  it turns out that our next transaction stands a
very good chance of being sent out over the same connection that the first
transaction was sent out over.  so, this hack mostly works.  it will probably
fail if we are authenticating multiple connections at once.  to get this right,
i really need to tag the transactions with some extra state information and have
the connection machinery check that and try to do the right thing from there.

at any rate, i'm anxious to hear how this patch behaves in the real world as is :)

Comment 15

[Andrew Bartlett]

Just one comment - in Windows, it's traditionally DOMAIN\username when entering
things in a single box.

It's great to see this finally moving!

Comment 16

[Darin Fisher]

Attachment: phase 1 patch

ok, i cleaned up the previous patch, and architecturally things are looking
better.  for example, i'm not sending default credentials first before
prompting the user.  some big things still remain to be done:

1- i haven't made any UI changes, so i took Andrew's advice and made it so that
you must supply "domain\username" in the prompt dialog.  if i'm lucky, maybe 2
or 3 other people will know how to enter things correctly into this prompt
without reading this bug report (or digging through the release notes if i
don't fix this by 1.4 alpha) :-/

2- we continue to only send out NTLM credentials if challenged.  this can mean
one extra round trip, but i'd like to take the perf hit for now (i don't think
it'll be huge).  by doing this we avoid the problem of having to detect an
already authenticated channel.	yes, this shouldn't be difficult, but given the
way things are organized, it is not as trivial as it should be.

3- same hack in place that depends on request order to reuse the right
connection.  i'm pretty sure this won't hold up.

despite the remaining problems, i think this patch stands on its own.  i'm
going to move forward and try to land this for 1.4 alpha.  i'll then try to
clean up the remaining issues (especially the UI) in a subsequent patch.

Comment 17

[Darin Fisher]

Comment on attachment 116892 [details] [diff] [review]
phase 1 patch

bbaetz: can you please review this.  at least from  a design point of view. 
nsIHttpAuthenticator needed to be seriously wacked. thx!

Comment 18

[Jason Prell]

The UI shouldn't be a big deal at all. The 3rd 'Domain:' field is actually used when invoking 'Integrated Windows Authentication'. Anyone who is using a domain account that is using Windows XP with IE 6 or any version of Windows with NS (any version) will have to enter the 'Domain\Username'.

The 3rd 'Domain:' field only appears in pre-Windows XP clients if the 'Integrated Windows Authentication' is enabled as an authentication method in IIS.

You shouldn't need to worry about the UI if you do not want to. :)

Comment 19

[Bradley Baetz (:bbaetz)]

Comment on attachment 116892 [details] [diff] [review]
phase 1 patch

>Index: public/nsIHttpAuthenticator.idl

How would an ASCII-art state diagram look? It may be too complex, bu if not, it
would be good to add here

>Index: src/nsHttpAuthCache.cpp

> 
>+static inline void
>+GetAuthKey(const char *host, PRInt32 port, nsCString &key)
>+{
>
>+    key.Assign(nsDependentCString(host) + nsPrintfCString(":%d", port));

nsDependentCString(host) + NS_LITERAL_CSTRING(":") + <Something>(port)

I don't know what the <something> is - prdtoa, perhaps? Save on the :%d parsing
each time.

I skipped over the rest, though - I don't have time, and can't test it anyway.

>Index: src/nsHttpNTLMAuth.cpp

>@@ -0,0 +1,364 @@
>+/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
>+/* ***** BEGIN LICENSE BLOCK *****
>+ * Version: NPL 1.1/GPL 2.0/LGPL 2.1

New files should be MPL-tru-licensed.

Comment 20

[Darin Fisher]

finally got a hold of a copy of MS Proxy to test this on, and there are
definitely some issues to be ironed out.

Comment 21

[Manko]

I don't understand, this is just SSPI or cross-platform NTLM module?

How about bug 23679 comment 161?

Comment 22

[dan]

I would like to test this patch but am not sure how to implement it.  I am
currently developing an ASP intranet site that uses "windows intergrated security" 
please advise...

Thanks,
Dan

Comment 23

[[e-mail]]

> I ... made it so that you must supply "domain\username" in the prompt dialog.

You mean there's no default for the domain?
Trust Microsoft to make things difficult :-/

Comment 24

[Darin Fisher]

Attachment: v1.1 patch

ok, this patch seems to pass all my stress tests.  it's is really ugly still,
and needs some major cleanup... so no reviewing yet ;-)  but, if someone wants
to try out this patch, this is the one to test.

Comment 25

[Darin Fisher]

oh fun!  so i submitted that last patch via a NTLM proxy, and i noticed that it
POSTed the entire patch twice to the proxy server.  the first POST request was
rejected with a 407, but the second POST request succeeded (and was passed on to
bugzilla).  we have the same problem with Basic and Digest auth... we so need to
implement the HTTP/1.1 "expect 100 continue" method of uploading.

Comment 26

[lode leroy]

I just downloaded the nightly, and I guess your patch was in it...
Anyway, I can now pass through our proxy which before I could not,
so the patch seems to be working... 
Thanks... I can now finally get back to my favorite browser :-)

WinXP, build 2003031714

Comment 27

[Matthias Versen [:Matti]]

no, the patch isn't checked into the tree !

Comment 28

[Darin Fisher]

Attachment: v1.2 patch

cleaned up a bit.  addressed bbaetz's review comment.  ready for more reviews.

Comment 29

[Doug Turner (:dougt)]

Comment on attachment 117630 [details] [diff] [review]
v1.2 patch

I am about 1/2 way done.

Do we warn the user before we send the response to the auth request?  
Sending the username, domain name, and workgroup in the clear to 
anyone who asks seans like it should have a alert dialog.


i would wait a milestone before tagging the nsIHttpAuthenticator as
UNDER_REVIEW.

In the comments of nsIHttpAuthenticator, you should make it explict who is
going to 
be prompting the user.

We should build this feature into a configurable build option:


+ifeq ($(OS_ARCH),WINNT)
+CPPSRCS		+= \
+		nsHttpNTLMAuth.cpp \
+		$(NULL)
+endif


Make it so, or drop the comment. :-)
+#define NS_HTTP_STICKY_CONNECTION    (1<<2)
+// XXX bah! this name could be better :(


I really don't like this procedure.  It returns TRUE if i pass a="liar",
b=nsnull.

+StrEquivalent(const PRUnichar *a, const PRUnichar *b)

instead of using calloc, just assign a null after PL_Base64Encode returns
+    // use calloc, since PL_Base64Encode does not null terminate.

Fix your copyright dates.


static void ParseUserDomain(PRUnichar *buf,
			    const PRUnichar **user,
			    const PRUnichar **domain)

if buf is ever null, ParseUserDomain will crash.  I think that the result of
PromptUsernameAndPassword 
could get you into this condition



Maybe you should explain this where you got the numbers :-) :

+	 // decode into the input secbuffer
+	 ib.BufferType = SECBUFFER_TOKEN;
+	 ib.cbBuffer = (len * 3)/4;


Why the changes from PRPackedBool to PRUint32

-    PRPackedBool		     mConnected;
-    PRPackedBool		     mHaveStatusLine;
-    PRPackedBool		     mHaveAllHeaders;
-    PRPackedBool		     mTransactionDone;
-    PRPackedBool		     mResponseIsComplete;  // ==
mTransactionDone && NS_SUCCEEDED(mStatus) ?
-    PRPackedBool		     mDidContentStart;
-    PRPackedBool		     mNoContent;	   // expecting an
empty entity body?
-    PRPackedBool		     mReceivedData;
-    PRPackedBool		     mDestroying;
+    // state flags
+    PRUint32			     mClosed		 : 1;
+    PRUint32			     mDestroying	 : 1;
+    PRUint32			     mConnected 	 : 1;
+    PRUint32			     mHaveStatusLine	 : 1;
+    PRUint32			     mHaveAllHeaders	 : 1;
+    PRUint32			     mTransactionDone	 : 1;
+    PRUint32			     mResponseIsComplete : 1;
+    PRUint32			     mDidContentStart	 : 1;
+    PRUint32			     mNoContent 	 : 1; // expecting an
empty entity body
+    PRUint32			     mReceivedData	 : 1;
+    PRUint32			     mStatusEventPending : 1;

Comment 30

[Alec Flett]

Comment on attachment 117630 [details] [diff] [review]
v1.2 patch

+static PRBool
+StrEquivalent(const PRUnichar *a, const PRUnichar *b)
+{
+    if (a && b)
+	 return nsCRT::strcmp(a, b) == 0;
+
+    if (a && a[0] == '\0')
+	 return PR_TRUE;
+
+    if (b && b[0] == '\0')
+	 return PR_TRUE;
+
+    return PR_FALSE;
+}
+

wait, doesn't that mean that "foo" and "" are equivalent? 

maybe you meant something like if (a && b && a[0] == b[0] == '\0') return
PR_TRUE?

I'm looking at the AuthCache stuff now, then onto NTLM

Comment 31

[Alec Flett]

Comment on attachment 117630 [details] [diff] [review]
v1.2 patch

duh, shaver straightened me out on my last comment, ignore that..
 The rest of this looks good though.. can we have some kind of a pref and
configure definition so this can be disabled at both build time and runtime? 

The build time stuff is for bloat, obviously, but the runtime stuff would cover
people who don't want their credentials just sent unconditionally .. I guess
dougt covered that too :)

you should document why StrEquivalent works, just because both dougt and I fell
into the same confusion and I'd hate to see someone come along and try to 'fix'
it :)
(something along the lines of
// a has value, so b must be null
...
// b has value so a must be null
...

As for the changing of GenerateCredentials to contain user/pass/domain - is
there any reason you can't just pass around nsHttpAuthIdentity like you do in
PromptForIdentity? or perhaps if this is going to be frozen (and thus you can't
use concrete classes) could you just wrap an interface around the identity, or
abstract the domain into some sort of "extra" user data so that if some other
authentication mechanism shows up and wants to hijack the "domain" parameter.. 

almost done...

Comment 32

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Mind you, that check will also report that NULL and NULL aren't equivalent,
though they're each equivalent to "".  Not what I would expect.

Comment 33

[Darin Fisher]

thanks for the comments guys.  i've applied changes to my local tree, minus the
following:

1- no fancy build changes at this time.  that can happen later.  anyways, this
has very low footprint already.

2- dougt: i changed nsHttpTransaction to store its flags as bit fields instead
of bytes.  in the end this shaves off a few DWORDS.

3- alecf: good idea about COM'ifying the auth identity structure, but i think
i'd prefer to wait on that.  it isn't necessary at the moment.  maybe something
that can be done when we want to freeze this interface.

mike: good catch.. thx!  i'll fix that.

Comment 34

[Darin Fisher]

Attachment: v1.3 patch

revised per previous comments.

Comment 35

[Martin Kutschker]

Alec (#31), I think "realm" is the term used in other protocols for "domain".

Comment 36

[Darin Fisher]

martin: sort of.  actually, with NTLM the "domain" is something the user must
enter, whereas with other protocols the "realm" is something specified in the
server challenge.

Comment 37

[Alec Flett]

Comment on attachment 118353 [details] [diff] [review]
v1.3 patch

ok, the rest of this looks good! sr=alecf

Comment 38

[Darin Fisher]

Attachment: v1.4 patch

updated the patch to apply cleanly to the trunk and applied changes per the
review comments.

Comment 39

[Cathleen]

Comment on attachment 118465 [details] [diff] [review]
v1.4 patch

r=cathleen  :-)

Comment 40

[Darin Fisher]

landed everything except nsHttpNTLMAuth.{h,cpp} and nsNetModule/nsNetCID
changes.  these are still pending approval.

Comment 41

[Darin Fisher]

Attachment: remaining portion of patch that didn't land before the tree closed for 1.4 alpha

this patch includes only the NTLM SSPI portion.

Comment 42

[Darin Fisher]

Comment on attachment 118602 [details] [diff] [review]
remaining portion of patch that didn't land before the tree closed for 1.4 alpha

requesting drivers approval for 1.4 alpha.  this patch has r=dougt,cathleen &
sr=alecf.

Comment 43

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 118602 [details] [diff] [review]
remaining portion of patch that didn't land before the tree closed for 1.4 alpha

>+ifeq ($(OS_ARCH),WINNT)
>+CPPSRCS		+= \
>+		nsHttpNTLMAuth.cpp \
>+		$(NULL)
>+ifdef MOZ_PSM
>+REQUIRES += pipnss
>+DEFINES += -DHAVE_PSM
>+endif
>+endif

I don't think this will work in a clobber build, will it?  (Given the order of
client.mk, designed to prevent dependencies like this one.)

>+#ifdef HAVE_PSM
...
>+static PRBool
>+IsFIPSEnabled()
>+{
>+    nsresult rv;
>+    nsCOMPtr<nsIPKCS11ModuleDB> pkcs =
>+            do_GetService("@mozilla.org/security/pkcs11moduledb;1", &rv);

Shouldn't this function exist even if HAVE_PSM isn't defined, so that it car
return false if the service is present, to allow for a case where the security
DLLs (which can, after all, be installed drop-in) are added later?  I'd think
you could do_GetService outside of the |#ifdef HAVE_PSM| and then QI to the
right interface and test inside of it?

Comment 44

[Darin Fisher]

dbaron: the issue i ran into is that if MOZ_PSM is not defined, then
nsIPKCS11ModuleDB.h will not be exported!  so, i do unfortunately need some kind
of compile time check :(

hmm... any suggestions?

Comment 45

[Asa Dotzler [:asa]]

Comment on attachment 118602 [details] [diff] [review]
remaining portion of patch that didn't land before the tree closed for 1.4 alpha

a=asa (on behalf of drivers) for checkin to 1.4a.

Comment 46

[Darin Fisher]

Attachment: updated final patch

thanks to kaie for providing better PSM hooks.

Comment 47

[Kai Engert (:KaiE:)]

Comment on attachment 118671 [details] [diff] [review]
updated final patch

I compared this patch with the different version of the patch.

r=kaie on the new portions added, and carrying forward the other reviews

Comment 48

[Darin Fisher]

alright!  final patch is in!  marking FIXED =)

Comment 49

[Manik Surtani]

What about NTLM for Moz running on Linux machines, or is this a separate
bugzilla issue?

Comment 50

[Martin Kutschker]

Manik,
 this is bug is about SSPI, which is a Windows 2000 API. For a Linux solution
see bug #171500. It seems that winbindd is not favoured by SAMBA developers.
OTOH SAMBA provides no other means for non-GPL programs.

For a discussion of the general topic see bug #23679.

Comment 51

[Jonathan Woods]

Great work - lack of NTLM support has for ages been one great reason why
corporates haven't dared consider moving away from IE.

But can I just check that there's no information leakage going on?  IE rightly
has a preference which goes something like "authenticate only in intranet zone",
which prevents NTLM credentials from leaking out to nasty sites on the Internet.
 Does your implementation do the same?  It's not enough to rely on only
authenticating when challenged, because of course the challenge can come from
anywhere.

Comment 52

[Darin Fisher]

mozilla will never send your NT logon credentials without first consulting you.
 thereafter, it will automatically send them to the same domain per the
protection space matching rules defined by RFC2617.  in the future, we may look
into offering the default credentials using some kind of heuristic like same
intranet or proxy-
only, etc.

Comment 53

[dave]

Hi - Like many others I'm really excited to have this in Moz and I've been
testing it out.
On one site it works fine. On a second site, I get a prompt: 

Enter Username and password for "" at myurl.example.com

In user name I have tried US\bob  and then my password in the password field

After hitting enter the prompt just returns over and over again. I think a clue
to the issue might be that the prompt is for "". On the site that works it lists
the hostname. eg/ Enter Username and password for "myurl.example.com" at
myurl.example.com

Am I missing something here?

Comment 54

[Rich Unger]

Just curious: does this put the machinery in place to support SPA, which is
basically NTLM over SMTP, in the mail/news client?

(More info at http://www.kuro5hin.org/story/2002/4/28/1436/66154)

Comment 55

[Darin Fisher]

dave: please see bug 199644#c4

rich: short answer is not really.  feel free to file a bug ;-)

Comment 56

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

Comment on attachment 118353 [details] [diff] [review]
v1.3 patch

(clearing review request on obsolete patch)

Comment 57

[benc]

VERIFIED: commercial 2003-04-29-04, Win 98.

I'm in the process of moving some testing to Win2K, so I'll check the NT end soon.

BTW, can someone confirm for me: Although 1.4a was released a couple days after
this checkin, this was not in for the 1.4a milestone release.

Comment 58

[Serge Gautherie (:sgautherie)]

Re comment 52 ! (and may be 31 ?)

Could there be a preference, with default to 'false' if you prefer, to let
Mozilla authenticate itself without prompting the user, as MsIE does ?

Comment 59

[christian.fruehling]

using mozilla 1.6b I still don't seem to be able to get out of my corporate 
network. Do I need to switch NTLM explicitly on, somehow? (I can get out with 
IE)