The default bug view has changed. See this FAQ.

Implement windows NTLM authentication using SSPI

VERIFIED FIXED in mozilla1.4alpha

Status

()

Core
Networking
--
enhancement
VERIFIED FIXED
15 years ago
7 years ago

People

(Reporter: Daniel van der Zee, Assigned: Darin Fisher)

Tracking

Trunk
mozilla1.4alpha
x86
Windows 2000
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 6 obsolete attachments)

(Reporter)

Description

15 years ago
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
(Reporter)

Updated

15 years ago
Depends on: 23679
(Reporter)

Comment 1

15 years ago
Wrong dependency. Sorry for the spam.
Depends on: 74246
No longer depends on: 23679
(Reporter)

Comment 2

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

Comment 3

15 years ago
cc

Comment 4

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

Comment 5

15 years ago
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

15 years ago
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? 
Assignee: darin → new-network-bugs
Component: Networking: HTTP → Networking
Keywords: mozilla1.2, nsbeta1
QA Contact: tever → benc

Comment 7

15 years ago
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.
(Reporter)

Comment 8

15 years ago
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 darin@netscape.com. 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

15 years ago
Depends on the NTLM bug. Reassign. 
Assignee: new-network-bugs → zee
Depends on: 23679

Comment 10

15 years ago
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

15 years ago
May I ask how this depends on 23679?
(Assignee)

Comment 12

14 years ago
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.
Assignee: daniel → darin
Target Milestone: --- → mozilla1.4alpha
(Assignee)

Comment 13

14 years ago
Created attachment 116584 [details] [diff] [review]
v0.000 patch

this patch is at best a hack.
(Assignee)

Comment 14

14 years ago
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

14 years ago
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!
(Assignee)

Comment 16

14 years ago
Created attachment 116892 [details] [diff] [review]
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.
Attachment #116584 - Attachment is obsolete: true
(Assignee)

Comment 17

14 years ago
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!
Attachment #116892 - Flags: review?(bbaetz)

Comment 18

14 years ago
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. :)
(Assignee)

Updated

14 years ago
Blocks: 60304
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.
(Assignee)

Comment 20

14 years ago
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

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

How about bug 23679 comment 161?

Comment 22

14 years ago
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

14 years ago
> 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 :-/

Updated

14 years ago
Summary: Implement windows authentication using SSPI → Implement windows NTLM authentication using SSPI
(Assignee)

Comment 24

14 years ago
Created attachment 117179 [details] [diff] [review]
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.
Attachment #116892 - Attachment is obsolete: true
(Assignee)

Comment 25

14 years ago
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.
Status: NEW → ASSIGNED

Comment 26

14 years ago
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
no, the patch isn't checked into the tree !
(Assignee)

Comment 28

14 years ago
Created attachment 117630 [details] [diff] [review]
v1.2 patch

cleaned up a bit.  addressed bbaetz's review comment.  ready for more reviews.
Attachment #117179 - Attachment is obsolete: true
(Assignee)

Updated

14 years ago
Attachment #117630 - Flags: superreview?(alecf)
Attachment #117630 - Flags: review?(dougt)
(Assignee)

Updated

14 years ago
Attachment #116892 - Flags: review?(bbaetz)
(Assignee)

Updated

14 years ago
Blocks: 180049

Comment 29

14 years ago
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

14 years ago
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

14 years ago
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...
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.
(Assignee)

Comment 33

14 years ago
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.
(Assignee)

Comment 34

14 years ago
Created attachment 118353 [details] [diff] [review]
v1.3 patch

revised per previous comments.
Attachment #117630 - Attachment is obsolete: true
(Assignee)

Updated

14 years ago
Attachment #118353 - Flags: superreview?(alecf)
Attachment #118353 - Flags: review?(cathleen)

Comment 35

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

Comment 36

14 years ago
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

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

ok, the rest of this looks good! sr=alecf
Attachment #118353 - Flags: superreview?(alecf) → superreview+
(Assignee)

Comment 38

14 years ago
Created attachment 118465 [details] [diff] [review]
v1.4 patch

updated the patch to apply cleanly to the trunk and applied changes per the
review comments.
Attachment #118353 - Attachment is obsolete: true

Updated

14 years ago
Attachment #117630 - Flags: superreview?(alecf)
Attachment #117630 - Flags: review?(dougt)

Comment 39

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

r=cathleen  :-)
Attachment #118465 - Flags: review+
(Assignee)

Comment 40

14 years ago
landed everything except nsHttpNTLMAuth.{h,cpp} and nsNetModule/nsNetCID
changes.  these are still pending approval.
(Assignee)

Comment 41

14 years ago
Created attachment 118602 [details] [diff] [review]
remaining portion of patch that didn't land before the tree closed for 1.4 alpha

this patch includes only the NTLM SSPI portion.
(Assignee)

Comment 42

14 years ago
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.
Attachment #118602 - Flags: approval1.4a?
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?
(Assignee)

Comment 44

14 years ago
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

14 years ago
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.
Attachment #118602 - Flags: approval1.4a? → approval1.4a+
(Assignee)

Comment 46

14 years ago
Created attachment 118671 [details] [diff] [review]
updated final patch

thanks to kaie for providing better PSM hooks.
Attachment #118602 - Attachment is obsolete: true
(Assignee)

Updated

14 years ago
Attachment #118671 - Flags: superreview?(dbaron)
Attachment #118671 - Flags: review?(kaie)

Comment 47

14 years ago
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
Attachment #118671 - Flags: review?(kaie) → review+
Attachment #118671 - Flags: superreview?(dbaron) → superreview+
(Assignee)

Comment 48

14 years ago
alright!  final patch is in!  marking FIXED =)
Status: ASSIGNED → RESOLVED
Last Resolved: 14 years ago
Resolution: --- → FIXED

Comment 49

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

Comment 50

14 years ago
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

14 years ago
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.
(Assignee)

Comment 52

14 years ago
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

14 years ago
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

14 years ago
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)
(Assignee)

Comment 55

14 years ago
dave: please see bug 199644#c4

rich: short answer is not really.  feel free to file a bug ;-)
Comment on attachment 118353 [details] [diff] [review]
v1.3 patch

(clearing review request on obsolete patch)
Attachment #118353 - Flags: review?(cathleen)

Comment 57

14 years ago
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.
Status: RESOLVED → VERIFIED
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

14 years ago
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)
You need to log in before you can comment on or make changes to this bug.