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

Wrong dependency. Sorry for the spam.

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

cc

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

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".

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?

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.

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.

Depends on the NTLM bug. Reassign.

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.

May I ask how this depends on 23679?

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.

this patch is at best a hack.

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 :)

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!

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 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!

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 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.

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

I don't understand, this is just SSPI or cross-platform NTLM module? How about bug 23679 comment 161?

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

> 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 :-/

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.

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.

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 !

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

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 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 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.

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.

revised per previous comments.

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

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 on attachment 118353 [details] [diff] [review] v1.3 patch ok, the rest of this looks good! sr=alecf

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

Comment on attachment 118465 [details] [diff] [review] v1.4 patch r=cathleen :-)

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

this patch includes only the NTLM SSPI portion.

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 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?

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 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.

thanks to kaie for providing better PSM hooks.

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

alright! final patch is in! marking FIXED =)

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

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.

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.

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.

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?

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)

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)

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.

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 ?

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)