Closed Bug 224653 Opened 21 years ago Closed 21 years ago

provide cross-platform NTLM auth implementation

Categories

(Core :: Networking, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla1.6beta

People

(Reporter: darin.moz, Assigned: darin.moz)

References

()

Details

Attachments

(1 file, 1 obsolete file)

this bug is about providing a cross-platform NTLM auth implementation that will
live in PSM (it leverages NSS) and can be used to provide NTLM auth for HTTP,
IMAP, SMTP, and POP3.

the implementation is based on http://davenport.sourceforge.net/ntlm.html

i filed this bug in an attempt to avoid the massive cc list of bug 23679... 
please do not mark this bug as a duplicate.  i'm hoping to get this in before
folks on that cc list get a chance to spam this bug.
Attached patch v1.0 patch (obsolete) (deleted) — Splinter Review
to summarize:

  o  this patch moves the NTLM authentication behind a new interface, called
     nsIAuthModule.  that interface defines a simple contract for challenge/
     response based authenticator.  the NTLM authenticator is implemented in
     PSM so that it can leverage NSS crypto functions (DES_ECB and MD5).

  o  the NTLM hash requires the MD4 algorithm.  since NSS does not provide MD4,
     the patch includes a clean (from spec) MPL'd MD4 implementation.  the code
     for MD4 is pretty small and straightforward (if you have the spec in front
     of you!).

  o  this patch adds support for sending either: LMv1 + NTLMv1 _or_ the NTLM2 
     session response.  there is no support for LMv2 or NTLMv2 since those modes
     cannot be negotiated (they must be manually configured on the client and
     server).  i will implement those modes if we discover cases where those are 
     required.  in all cases, the NTLM2 session response, if negotiated 
     successfully, is preferred over even LMv2 or NTLMv2, so i feel that it is 
     most useful to have NTLM2 session response supported and less useful to 
     support LMv2 and/or NTLMv2.
 
  o  i have discarded the NTLM SSPI code.  i think we are better off using our
     own code for NTLM authentication.  the reason for this is simple.  on older
     clients, like Win9x, only the LMv1 response is generated by SSPI.  that is
     the least secure way of encrypting the user's password.  it is definitely a
     good idea to use our code on Win9x since it gives us the opportunity to
     authenticate the user w/ a NTLM2 session response.  also, SSPI has been a
     source of mysterious crashes on Win9x (see bug 222849 for example).  
     moreover, older versions of WinNT do not support the NTLM2 session 
     response, so we are better off on some versions of WinNT if we use our 
     code.  ah!  but, the tradeoff is that we may need to implement LMv2/NTLMv2
     if a site is configured to only allow those modes of authentication.

  o  i've added netwerk/test/ReadNTLM.cpp, which is a simple little program that
     decodes the NTLM messages.  this is useful for debugging a NTLM session,
     and for figuring out what IE and SSPI are doing ;-)

  o  i've decided to keep the base64 encoding/decoding in nsHttpNTLMAuth.cpp. 
     i was tempted to put it inside the NTLM auth module (since all of the 4
     protocols i previously mentioned need the NTLM messages to be base64 
     encoded).  but, i think the base64 encoding belongs closer to the protocol
     implementations since the encoding is a protocol dependent aspect.  plus,
     i can eliminate a buffer copy by keeping the base64 encoding in HTTP land.
     for the mail protocols, we can try to share the code for base64 encoding/
     decoding.

  o  i removed the remnants of the LanManager single singon code since we 
     weren't using it anyways.  (i.e., we always prompt the user at least once 
     for their username and password.)

lastly, i've tested this against 1) a Win2k server and 2) a Squid proxy
configured to use winbindd to authenticate with a NT domain server (Samba in my
testcase).  and, i've tested this code under Win2k and Linux.  there may be
issues on big-endian platforms (such as OSX).  i have not yet tested this code
on OSX.  that's next on my list ;-)
Severity: normal → enhancement
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla1.6beta
Blocks: 200436
Comment on attachment 134744 [details] [diff] [review]
v1.0 patch

r=kaie on the changes to existing files in security/manager/ssl

I'm giving a general moa=kaie on adding the new nsNTLMAuth* and md4* files to
security/manager/ssl, because the new service you are adding is self contained.

Please note that NTLM authentication will remain to be active in the session,
if the user activates FIPS mode. A restart is required to disable NTLM. You
should make sure this is compliant with FIPS requirements, or find a way to
disable NTLM at the same time FIPS gets activated.

Regarding the use of PK11_ functions, please make sure that you are cleaning up
NSS resources correctly, in order to not break the "switch profile at runtime"
functionality.

For example, I do not see a call to PK11_FreeSlot, but I think you need to.
thanks for catching those mistakes kai!

  o  i'll add a PK11_IsFIPS() check in GetNextToken to have it fail if FIPS mode 
     has been enabled.

  o  and thanks for noticing the missing PK11_FreeSlot
Attachment #134744 - Attachment is obsolete: true
Attachment #135210 - Flags: superreview?(bryner)
Comment on attachment 135210 [details] [diff] [review]
v1.1 patch - revised per comments from kai

r/sr=bryner with the changes we discussed.
Attachment #135210 - Flags: superreview?(bryner) → superreview+
Thanks for the new patch, Darin!
My r=kaie/moa=kaie still stands.
patch checked in for 1.6 beta :)
Status: ASSIGNED → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
Blocks: 152883
Blocks: 23679
According to brad TBox, there are two "might be uised uninitialized" warnings in
the new code:

+netwerk/protocol/http/src/nsHttpNTLMAuth.cpp:104
+ `PRUint32 inBufLen' might be used uninitialized in this function

+security/manager/ssl/src/nsNTLMAuthModule.cpp:771
+ `SECItem*param' might be used uninitialized in this function
aleksey: thanks for the notice!
Blocks: 222849
i applied a fix for those warnings.  thanks again aleksey!
Hi, which CVS branch are these fixes in?  Would they have been built into the
latest nightlies?

Cheers,
Manik
Manik: only on HEAD. yes, nightlies contain it.
I know this one is resolved, but is there a reason why this couldn't be extended
to NNTP authentication in addition to HTTP, SMTP, IMAP and POP3?
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: