Closed Bug 17578 Opened 25 years ago Closed 20 years ago

I want Kerberos authentication and TGT forwarding [using GSSAPI]

Categories

(Core :: Networking, enhancement, P2)

Product:
Core
Component:
Networking
Type:
enhancement

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla1.7beta

People

(Reporter: spreitze, Assigned: darin.moz)

References

Details

Attachments

(4 files, 22 obsolete files)

IETF Draft describing their HTTP Negotiate method
13.39 KB, text/plain
Details
Error from startup of debug build
127.76 KB, text/plain
Details
v3.1 patch
39.08 KB, patch
Details | Diff | Splinter Review
another attempt at doing mutual authentication correctly
8.17 KB, patch
Details | Diff | Splinter Review 
I'd like to see browser and server able to use Kerberos for authentication and
authorization delegation.  This would probably break down as using the new
Kerberos option in TLS (as specified in RFC 2712) for the authentication, plus a
new flavor of HTTP "authentication" to convey the TGT.
Status: NEW → ASSIGNED
http://stonecold.unity.ncsu.edu/software/mod_auth_kerb/ is an Apache module that
works with kmosaic.  It might be useful as a test environment.
Bulk moving all Browser Security bugs to new Security: General component.  The 
previous Security component for Browser will be deleted.
Component: Security → Security: General
Changing Qa contact to myself.
QA Contact: dshea → junruh
Whiteboard: [from mwelch@netscape.com]
I'm leaving Netscape, so I'm reassigning all my pending bugs to David Drinan.
I've added "[from mwelch@netscape.com]" to the status whiteboard for each of
these bugs for easier searching.
Assignee: mwelch → ddrinan
Status: ASSIGNED → NEW
Target Milestone: --- → Future
Changing component to Security:Crypto.
Component: Security: General → Security: Crypto
Whiteboard: [from mwelch@netscape.com]
This would be a killer feature for some big environments. Way cool.
Changing product from Browser:Security:Crypto --> PSM 2.0
Component: Security: Crypto → Client Library
Product: Browser → PSM
Target Milestone: Future → ---
Version: other → 2.0
Target Milestone: --- → Future
Attached patch proposed patch. (obsolete) — — Splinter Review 

    
    

    
  
Mass assigning QA to ckritzer.
QA Contact: junruh → ckritzer

    
  
QA Contact: ckritzer → junruh

    
    

    
  
Just thought ya'lld like to know that Kerberos authentication works with the BU
studentlink in RC1.

I'm not sure if that is due to a PSM change or what, but it's great to have, as
I was forced to open IE whenever I wanted to buy convenience points so I could
go get tasty campus treats.  :)  Now I can just open a new tab!  ;)

    
    

    
  
Scott -- the weblogin system we use at BU is independent of the feature 
requested by this bug. It works by getting your login/password in plain text 
and sending that (over SSL) to a central server for authentication against 
kerberos. In other words, it's a kludge. *This* bug suggests 
actually "kerberizing" mozilla, so that it can get kerberos credentials of its 
own.

(The studentlink started working, by the way, because BU OIT updated the code 
to a newer and better version.)

    
    

    
  
OK, I'm looking at taking this on, but might take a different approach than that
outlined by the original poster.  My initial take would be to extend the current
WWW-Authentication mechanism to include a KerberosV5 mechanism.  This seems to
be the approach taken by the mod_auth_kerb Apache module.  So, this would be
simple Authentication, and would not be used to initiate a TLS secure exchange.
 The HTTP handshake would be unchanged except that the response would include
the user's Kerberos ticket, which would be pulled from the environment.  If a
ticket didn't exist, the user would be prompted as with the current basic
authentication implementation.  In looking at the nsHttpChannel and nsHttp*Auth
code this looks pretty straightforward (until I get to the cross-platform issues
of getting the ticket from the environment).  Does this make sense?  Does anyone
know how Microsoft is handling Kerberos tickets with IE and IIS?  I don't want
to deviate from an existing implementation unless I have to.

    
    

    
  
I guess I'm going to answer my own question (sorry for the SPAM).  I found an
Internet Draft document by John Brezak that outlines the IE/IIS interactions and
their use of SPNEGO that is triggered by the "Negotiate" option to
WWW-Authenticate.  The assumption is that SPNEGO is using GSS-API, which is an
available API on all of the mozilla platforms, I believe.  The only concern is
that GSS-API will require server tickets, I believe, which is more complicated
to manage.  Anybody have any strong feelings one way or the other about this?

    
    

    
  
SPNEGO is a GSSAPI mechanism that negotiates the "real" security mechanism to be 
used for authentication.  Implementing Microsoft's draft would be the ideal 
solution as it would allow for folks who access IIS servers (also enabled with 
the new Auth-Negotiate support) to use Mozilla instead of IE.  Also, this 
approach avoids common pitfalls of implementing Kerberos authentication - using
cookies or some other grossly insecure method for transmitting the security
credentials.

PLEASE implement Brezak's draft!   Doing so will require the creation of an
SPNEGO GSS-API mechanism (unless someone wants to donate one) as this is not 
included with MIT or Hiemdal Kerberos distributions at this time.

Because its GSSAPI based and should not need to directly access the 
Kerberos API's,  the solution should not require the developer to choose 
one Kerberos implementation over another, just choose one that has a 
working GSSAPI KRB5 mechanism that SPNEGO can choose to use.

The only additional library linking would be against libgss. 
I think....



    
    

    
  
Reasigning
Assignee: ddrinan → ssaux
Version: 2.0 → 2.4

    
    

    
  
Since this one is reassigned (does that mean it'll get worked on?), what about
bug 28200? 

    
    

    
  
I'm interested in helping get SPNEGO working with Mozilla.

Could one of the more seasoned bugzilla developers comment on the correct
approach?  

1.  Work with Samba developers to provide a method of accessing their spnego
routines. This was suggested by one of the Samba developers as a method of
integrating with their NTLM implementation
(http://bugzilla.mozilla.org/show_bug.cgi?id=23679#c134).  He seemed to have
some ideas about how to deal with the licensing issues (Mozilla can't link w/
gpl'ed code). Can Samba's spnego do kerberos delegation?

2. Have mozilla link directly with Heimdal or MIT kerberos and add support in 
ourselves.  This seems to be the approach take here 

http://meta.cesnet.cz/software/heimdal/negotiate.en.html

This code does not include a SPNEGO implementation, but instead uses the raw gss
data.  If we choose to go this route can moz's asn parsing code be accessed from
netwerk? This would allow us to easy write the required SPNEGO wrapper for kerberos.

3. Help with bug 180049?  IMO this is the best long term approach and it will
make things a lot easier in the future for fixing bugs like these.

    
    

    
  
As Christopher Nebergall mentioned in his comment, we developed an open source
implementation of the Negotiate method used by Microsoft for Kerberos
authentication on the web. I've just updated and rewritten it a bit so that
Mozilla 1.1 is now supported. It's available from
http://meta.cesnet.cz/software/heimdal/negotiate.en.html

I'd be very glad if anyone familiar with Mozilla and C++ could have a look at
the code.

    
    

    
  


  
I'm adding the patch (for mozilla 1.1), which is also available from
http://meta.cesnet.cz/software/heimdal/negotiate.en.html

    
    

    
  
I've been using some of the ASN1 code from the public api's of NSS to add the 
actual DER encoded SPNEGO wrapper for the Kerberos call. If anyone's interested 
I can post my preliminary code here in a day or two for feedback from other 
developers, but its for -testing purposes only- (until I can get the required 
approvals from my company to donate the code).

Using NSS's ASN1 code does add the odd dependency that this Authentication type 
would require PSM to compile.

Questions.

Should there be support for both NEGOTIATE authtype w/ Kerberos support 
(Brezak's draft) and a KERBEROS authtype(read base64'd token from gss-init-sec-
context)?

I'm also interested in working on a patch to nsIHttpAuthenticator so Kerberos 
support can be added with an XPI and not require compiling mozilla.  This is 
not to say that I don't want to work towards native support in Mozilla, just 
that I want a working solution until then.  In testing I've reworked some of 
the above patch to install as an XPI and only problems I come across are the 
required changes to nsIHttpAuthenticator and nsHttpChannel.  I mentioned the 
problems with the current api in bug 180049.  Anyone who can help with generic 
methods for solving these problems please post suggestions on that bug.

    
    

    
  
Recent conversations on the IETF KRB5 Working Group mailing list might be of
interest here.  In the process of developing a GSSAPI SPNEGO implementation
(according to IETF RFC 2478), I discovered the Microsoft's own SPNEGO (a.k.a
"Negotiate") mechanism is badly broken and does not follow the IETF specification.
There are workarounds for the problem, but trying to decipher the ASN.1 based 
on what *should* be present according to the SPNEGO spec will lead to problems.
Likewise, communicating with MS and following RFC 2478 will also not work
without some changes. 

Basically - do not send or expect to receive the MechListMIC field when talking
to Microsoft SSPI/Negotiate clients.

Search thru this file for "SPNEGO" references to get all the details, including
a response from Microsoft:
ftp://ftp.ietf.org/ietf-mail-archive/krb-wg/current

If you are developing a patch for Mozilla that will interoperate, it should use
the generic GSSAPI public interface so it can compile and work on any system
that has a GSSAPI library (i.e. Solaris 2.[8,9], or systems with MIT KRB5 or
Heimdal Kerberos installed).

There should be NO reason for the mozilla source to try and do the ASN.1 stuff
or bypass the GSSAPI layer, this would be a really ugly hack.
Much better to write a real SPNEGO mechanism and talk directly to GSSAPI.

There are several people working on open source SPNEGO mechanisms (MIT KRb5
team, Heimdal) that might be helpful in kickstarting this particular project.

    
    

    
  
To Christopher Nebergall (comment #20)
I think the usage of only SPNEGO is sufficient for Kerberos support. The variant
of "raw" krb5 was choosen as workaround in situation where no SPNEGO
implementation was available. Since this method is used only by our
implementation there is no need to keep backwards compatibility (I think that
the patch is used by a quite limited set of users).

The plugin solution should be perhaps the best one in long term. But I believe
that the possible Negotiate plugin would be very similar to current
implementation of the nsHttpGssapiAuth class. So first I would prefer to have a
stable (and perhaps MS compatible) implementation of the Negotiate mechanism.
For this to really work an SPNEGO implementation is needed. I agree with Wyllys
Ingersoll that such an implementation should be independend on mozilla sources
(but perhaps might be created by means of NSS public API). Unfortunatelly, I've
 heard of several people working open source SPNEGO implementations but I
haven't seen any code yet :-( Would anyone have some link?

    
    

    
  
>Since this method is used only by our
>implementation there is no need to keep backwards compatibility (I think that
>the patch is used by a quite limited set of users).

Network sniff's of IIS 5 list Kerberos as a supported authentication type.  I 
have no idea how they use it, but it is in the list of auth types sent as part 
of the 401.  Negotiate of course is also listed.

I agree that an independent implementation of SPNEGO would probably be the 
cleanest long term approach but I've never seen one outside of Samba and 
Samba's implementation does not use the GSS-API. There are rumors that both 
Heimdal and MIT want to add it, but I've found no patches on the web.

In the short term don't overlook how easy it is to write a Mozilla SPNEGO 
implementation which calls the systems gss-kerberos implementation (heimdal, 
MIT or whatever).  Two of the big problems in writing an SPNEGO implemtation 
have already been solved my Mozilla 1.) available, easy to use ASN1 DER 
encoding/decoding libraries, 2.) An api which allows supporting multiple 
authentication mechanisms.  To support just kerberos it only took a few ASN 
template structures and ~10 lines of code.  To truly support the whole 
mechanism using both Kerberos and NTLM (when there is an implementation) would 
not be much more difficult.  

The only real problems are where to keep state information for the connection 
between requests and how to get Mozilla to call the auth handlers after a 
successful request so mutual authentication becomes possible.  These problems 
must still be solved even if an external SPNEGO mechism is used.

    
    

    
  
I have an SPNEGO mechanism that I may attempt to give back to MIT that will work
with MS.
If that works, then you could eventually just use that mechanism. 

GSSAPI offers a layered, abtract security API that should support whatever
mechanisms
SPNEGO is a specific GSSAPI mechanism that negotiates the "real" security mechanism
to be used.   None of these mechanisms shoudl have any knowledge of each other nor
should they be architecturally linked in any way.    Heimdal and MIT both offer
GSSAPI
with KRB5, but their approaches are quite different.  If you must choose, I
would recommend
choosing the MIT GSSAPI implementation as MIT KRB5 is probably the most prevalent
KRB5 distribution.   Heimdal has nice features as well, but I dont think its as
commonly
used as MIT (at least in the US).

Applications should never talk directly to the specific mechanism's API (i.e.
krb5_gss_init_sec_context), but should always talk directly to the GSSAPI layer
(gss_init_sec_context).  The mechanism used may be specified thru the use of the
correct OID or by having the system choose the default.

As far as state information goes, all GSSAPI sessions require an opaque context
token to be passed 
to the various function calls, so the gssapi context struct could just be part
of whatever other 
context is passed around with the mozilla connection information.

    
    

    
  
Regarding comments 21-24, before you go too far down the road of making 
mozilla depend on GSSAPI, have you contacted the PSM module owner about
this idea?  Is the module owner amenable to this proposed contribution?
I imagine the module owner will ask questions like these:

How will mozilla build or run on systems that don't have GSSAPI headers
and libraries? 
Will there need to be new UI for mozilla to do Krb5 based authentication?
Are you planning to do that too?  

    
    

    
  
To Christopher (comment #23):
- I don't have any idea what's the Kerberos method offered by IIS. At this
moment I'd stick only with Negotiate method, whose specification is published.
- Concerning the SPNEGO implementation above Mozilla API, would it be possible
to implement it as a standalone library exporting GSS-API functions (or their
subset)? In this way it would be possible to keep the Negotiate implementation
separate from the GSS-API stuff.
- I also don't know how the mentioned incompatibility of the MS SPNEGO
implementation will influence the implementation of the Negotiate method - would
some "hooks" be needed?

To wyllys (comment #24)
- You really have an SPNEGO implementation and you're willing to share it with
us? It would be great. 
- Concerning the interoperability MIT and Heimdal, according to my experience
their GSS-API implementation are very compatible and if your application is
properly written it really doesn't matter which implementation you will use. So
 it shouldn't be important if we use Heimdal or MIT for development.
(unfortunatelly, the GSS-API spec. doesn't solve all problems, e.g. credential
storing must still be done via the mechanism API), but it shouldn't be important
for Mozilla and SPNEGO.

To nelson (comment #25)
- I haven't contacted anyone from PSM but I absolutely agree we should do it.
But I'm not sure if we should talk to the PSM people since the Negotiate patch
alter the Necko part? 
- Should we move this discussion to some mailinglist? The bugzilla interface 
seems to me a bit uncomfortable for this purpose. What's the right list for this
discussion (mozilla-security@mozilla.org)?

    
    

    
  
FYI: a new version of the apache module supporting both password and native
(i.e. Negotiate) Krb5 authentication is available at
http://meta.cesnet.cz/software/heimdal/negotiate.en.html

A new patch for Mozilla 1.2.1 is there as well.

    
    

    
  
I'm all for moving the conversation to mozilla-security@mozilla.org.  I'll post 
my comments there.

    
  
QA Contact: junruh → bmartin

    
    

    
  
In depth explanation and sample code to implement SPNEGO from Microsoft

http://msdn.microsoft.com/library/default.asp?url=/library/en-
us/dnsecure/html/http-sso-3.asp

    
    

    
  
A new version of the Negotiate patch for Mozilla 1.3 is available from
http://meta.cesnet.cz/software/heimdal/negotiate.en.html
This new patch contains some cleanups, especially all new code is wrapped within
#ifdef statements, it also adds registering of the GSSAPI module with a new
component ID, and contains a support for the configure script (adds a new option
--with-gssapi).

    
    

    
  


  
Patch for Mozilla 1.4 checkouted from the main CVS branch on 5.5.2003. Hopefuly
prepared for review.

        Attachment #107471 -
        Attachment is obsolete: true

    
  

        Attachment #122480 -
        Flags: review?(darin)

    
    

    
  
i'm glad to see that nsIHttpAuthenticator is almost sufficient for what you
need.  however...

1- changes to nsHttpChannel should be made generic so they don't need to be
specific to #ifdef GSSAPI.  we need to generalize nsHttpChannel, so there is no
need to add special cases like this.  this should be a separate patch.

2- if #1 is done, then your auth module could live as a completely separate
XPCOM component.  this would be ideal since it has link dependencies on a new
lib.  perhaps you could even start by hosting your XPCOM component on
mozdev.org??  alternatively, this component could perhaps live under
mozilla/extensions if it were to be part of the standard mozilla release at some
point.

    
  

        Attachment #122480 -
        Flags: review?(darin) → review-

    
    

    
  
#1: Strictly speaking, the change of nsHttpChannel is not needed for the
Negotiate authentication. It's only meant as a handler for multiple
authentication methods sent by the server. I saw bug #180049 and your patch
"authentication api update" attached to this bug seems to add exactly that
functionality as my adaptation of nsHttpChannel. Any chance of getting that
patch to the Mozilla CVS?

#2: I think situation is very similar to NTLM authentication, which is included
in Mozilla tree (also the code is similar). In fact the Negotiate method can
support NTLM as well (provided NTLM GSA-API implementation is available) and it
is supported by Microsoft products. I don't opose the suggestion  of creating a
separate XPCOM component but I believe it should be kept "close" to the NTLM
support. Your opionions?

    
    

    
  
The patch has no code in PSM. Who should own this bug?


    
    

    
  
FYI, I've opened a new project on the mozdev site aiming to support of the
Negotiate HTTP method (especially with regards to Kerberos). See
http://negotiateauth.mozdev.org/ for more information.

What is still needed, however, is support in mozilla for handling multiple
authentication methods sent by the server (e.g. something suggested by bug #180049

    
    

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

    
    

    
  
I've ported and updated the patch from Dan to work with Mozilla 1.5,
and made some significant fixes so that it works with MS Windows 2000
IIS servers that request "Negotiate" authentication.  The code as written
was not requesting the proper AD service (khttp/ instead of HTTP/), nor
was it recognizing the correct Negotiation string ("GSS-Negotiate" instead
of "Negotiate") to work with IIS.

One thing to note:  Though the MS documentation states that SPNEGO is
needed to authenticate correctly, this is not strictly true.  
If the client initiates the GSSAPI session using a KRB5 GSSAPI token
(instead of being wrapped in SPNEGO), IIS will accept it and
the authentication can still succeed despite not having used SPNEGO.

This means that the fix can work with MIT KRB5, Heimdal KRB5, Solaris SEAM, and
anyone else who has a working GSSAPI layer and KRB5 mechanism.  SPNEGO will
obviously work as a mechanism, but not many distributions have SPNEGO at this time.

I am working on getting the fixes prepared as a single "patch" file and testing
compilation on RH Linux (with MIT KRB5) and Solaris (with the bundled GSSAPI
library and SEAM KRB5 mechanism).

    
    

    
  
Wyllys: i'm very interested in helping bring kerberos authentication to mozilla
(for HTTP anyways).  i can help you get this in the tree.  just attach your
updated patch, and i'll begin reviewing :)

you should probably include your implementation in security/manager/ssl/src,
assuming kaie is okay with that.

    
    

    
  
The code I have now needs a couple of small tweaks to configure and compile 
clean on a system that has MIT KRB5 only installed.  My initial testing was 
Solaris, which has its own GSSAPI libraries that differ in name & location
from the stuff from MIT (though the API is the same).  I should be able to 
post a patch by the end of the week.   I'm not sure how well it conforms to 
the coding style/standards, but since my work is based on an existing patch,
it should be close.  I have added lots of comments to help those not 
too familiar with GSSAPI.

Why would the code go in security/manager/ssl/src ?  Its not clear to me
how that relates here.  The patch is currently mostly in 
netwerk/protocols/http/src.

    
    

    
  
wyllys: so there are maybe three places where this code could maybe live:

1) netwerk/protocol/http/src

   this is where the other http authentication protocols live, so why not put
   this one there too?  definitely a good point.  the downside to putting the
   code here is that it increases the size of libnecko.so, and most users won't
   need this code.

2) extensions/negotiateauth

   this may make more sense since this implies that the negotiate auth code
   would live as a separate DSO, loaded only when the user needs it.  also,
   if it is the case that other protocols besides HTTP would potentially use
   negotiate auth, then it should certainly not live next to the HTTP 
   implementation.  for example, is it the case that SMTP, IMAP, or POP can
   use negotiate auth?  i know they can for NTLM, and for this reason NTLM
   code is being factored out of necko.

3) security/manager/ssl/src

   ... which brings me to this location.  currently, i am working on moving
   the necko NTLM implementation into PSM.  the reasons for doing so are two
   fold: a) i need to access some NSS routines in order to implement NTLM on
   non-Win32 platforms, and b) i wanted to factor out the NTLM request/response 
   generation code from the actual code which implements nsIHttpAuthenticator.  
   again, this is important since we may want to add code to support NTLM 
   authentication with the mail protocols.

so, i may have jumped the gun a bit by suggesting that the negotiate auth code
live in PSM (security/manager/ssl/src), because clearly it does not have a
dependency on NSS.  rather it has a dependency on GSSAPI, and that is something
independent.  i think option #2 makes the most sense for negotiate auth.  in
fact, option #1 is definitely a bad choice since it would force users of mozilla
to have the right GSSAPI library installed in order to simply load libnecko.so.
 that could be a major compatibility constraint.  so, short of dynamically
loading the GSSAPI library and resolving each symbol manually, i think simply
moving the link dependency into a xpcom library (libnegotiateauth.so), which is
dynamically loaded, would solve this problem elegantly.

extensions/finger is a simple extension, which you might find useful as a guide
to learn how to properly write a mozilla extension.

    
    

    
  
Darin -
  I agree,  #2 seems like the right place.  I was thinking that GSSAPI by itself
would not be useful to some people and thus it could not be enabled by default.
In fact, it could not be enabled by default if it was built into necko because 
those that do not have a GSSAPI library would not be able to run the browser at
all due to the missing library.
   Having it be configurable and loadable is the right solution.  I will look at
moving the code into the extensions area  before I submit.




    
    

    
  
Wyllys: I've already converted Daniel's patch into an extension.  If you post
all our changes here, I can merge my extentions changes in.  Bug 180049 was just
checked in and that was all that was required so no necko changes were not required.

    
    

    
  
My current version of the Negotiate patch is available from
negotiateauth.mozdev.org. My goal was to create an stand-alone XPCOM component,
which would implement this method so that it's not necessary to patch and
recompile mozilla to get support of Kerberos (and others GSS mechanims) to
Mozilla. I'm happy to hear that bug #180049 was solved. I'd very like to
incorporate all your changes to the module. Christopher and Wyllys would you be
so kind as to send me your adaptations?


    
    

    
  
I'd be very concerned about including anyone's simple implementation of SPNEGO
in this piece of code.  SPNEGO is designed to work as a GSSAPI plugin, it should
not be included as part of this patch.   The only API needed by this
patch is the pure GSS-API code, if the system has Kerberos *OR* SPNEGO, the
server should still accept the credentials.  I have verified this testing
with MS Windows 2000  IIS servers.   Implementing SPNEGO directly here is
clearly the WRONG thing to do.


    
    

    
  


  
I'm not sure that my additions are correctly "c-styled" for mozilla, if not
acceptable, I will rework them, but I wanted to post the patch because there
has been some interest lately in seeing what was done.

Note that I also included updates to configure.in so that the gssapi stuff can
be detected and configured at build time. 

As always, autoconf 2.13 is needed, 2.5X will not work here.

steps: patch, autoconf, configure, build.

    
    

    
  

      
      
      
      

        Attached patch
          
          
        minor update to previous patch (obsolete)
        — 
          — Splinter Review
      

    


  
Minor update to previous patch to correct gssapi files included.

    
    

    
  
Comment on attachment 134483 [details] [diff] [review]
Fixes for the "Negotiate" authentication code to work with MS IIS correctly

obsolete, see updated patch

        Attachment #134483 -
        Attachment is obsolete: true

    
    

    
  
wyllys: thanks for the updated patch...

some quick comments:

1) the files are not in mozilla/extensions.  they still appear to be in necko
(network/protocol/http/src).

2) there are modifications to necko included in this patch.  according to
cneberg (comment #42), these changes should not be necessary.  why are those
changes needed?

3) we want to avoid #ifdef/#ifndef GSSAPI being added in parts of the core
mozilla code.  as soon as you do that you break the possibility of ever being
able to just drop your library into a build of mozilla that does not have those
defined.

    
    

    
  
I didn't put the changes in extensions because Christopher said that he
would merge my fixes as they were since he had just added the extensions code
very recently (also from comment #42, unless I misunderstood).

I agree that the #ifdef GSSAPI stuff should be removed since the auth module
is going to be an extension instead of merged with necko.

I did add code to nsHttpChannel.cpp that is important and cannot be
put into the extension.  The code changes in nsHttpChannel::ProcessResponse(),
when the Server returns status code 200 but also includes the WWW-Authenticate:
Negotiate header with additional token information following the "Negotiate"
string, that code must be processed by the "negotiate" extension in order for
the token exchange to complete correctly.   GSSAPI exchanges are not guaranteed
to be complete in a single round trip, often additional token exchanges are
needed to complete correctly. 






    
    

    
  
>I didn't put the changes in extensions because Christopher said that he
>would merge my fixes as they were since he had just added the extensions code
>very recently (also from comment #42, unless I misunderstood).

ah, ok... the confusion is on my end then.  i misunderstood.


>I agree that the #ifdef GSSAPI stuff should be removed since the auth module
>is going to be an extension instead of merged with necko.
>
>I did add code to nsHttpChannel.cpp that is important and cannot be
>put into the extension.  The code changes in nsHttpChannel::ProcessResponse(),
>when the Server returns status code 200 but also includes the WWW-Authenticate:
>Negotiate header with additional token information following the "Negotiate"
>string, that code must be processed by the "negotiate" extension in order for
>the token exchange to complete correctly.   GSSAPI exchanges are not guaranteed
>to be complete in a single round trip, often additional token exchanges are
>needed to complete correctly. 

Ack!  WWW-Authenticate sent with a 200 response.  Can we deviate from the
standards any further?  Is there any documentation explaining the Negotiate HTTP
authentication protocol?  I'd like to read about this, so I can better
understand all that is required of an authentication plugin.  Thanks!

    
    

    
  


  
See attachment for description of Microsoft's Negotiate method.  The draft is
expired, but its still technically correct.

Also see this page at MSDN for more information:
http://msdn.microsoft.com/library/default.asp?url=/library/en-us/dnsecure/html/http-sso-1.asp

    
    

    
  
Basically in any GSSAPI transacation using Negotiate a WWW-Authenticate with
HTTP 200 would mean the server is satisfied, but the client is still missing
something it asked about.  In Kerberos that is probably mutual auth.  If you
don't care about Mutual auth, you wouldn't need it.   IE seems to ignore the
response as well. Although IIS does send it.

My patch's solution is to optionally require (prefs.js option) SSL for it to
automatically authenticate.

I think server authentication would be useful in some contexts, but I don't
think it should hold up user-only authentication using the GSS-API.

I will attach an upated patch tonight or tomorrow morning.

    
    

    
  
Correct, the final Negotiation token sent by the server in a "200" response is
to complete the mutual authentication step for Kerberos.  I think it should be 
handled just to be correct, there may be cases where mutual auth fails (due to a
security breach) in which case the browser should report that auth failed and
not send the authentication response.  This prevents the browser from exposing
the user's creds to a bad-guy impersonating a real web server.



    
    

    
  
>there may be cases where mutual auth fails (due to a
>security breach) in which case the browser should report that auth failed and
>not send the authentication response.  This prevents the browser from exposing
>the user's creds to a bad-guy impersonating a real web server.

But that's too late, you have already sent your token(s) and since the session
key is not used to protect the channel, the server (or man in the middle) needs
nothing else to impersonate you (the kerberos tokens can only be replayed up
until the max clock skew ~5 Minutes).  The only question left is will you fail
over to some other auth type and possibly give them more information (like your
password).   I just think SSL should be required by default, and eventually the
best solution would be a security zones like IE has.

    
    

    
  
cc'ing caillon since "security zones" were mentioned ;)

    
    

    
  
One thing I noticed while poking around is that setAuthorization header in
nsHttpChannel.cpp doesn't obey the nsIHttpAuthenticator::REUSABLE_CREDENTIALS
auth flag.  It will always cache the credentials.  This will cause some replay
errors on the server.

  if ((!creds[0] || identFromURI) && challenge[0]) {
2369             nsCAutoString unused;
2370             rv = ParseChallenge(challenge, unused, getter_AddRefs(auth));
2371             if (NS_SUCCEEDED(rv)) {
2372                 nsISupports *sessionState = entry->mMetaData;
2373                 rv = auth->GenerateCredentials(this, challenge,
2374                                                header ==
nsHttp::Proxy_Authorization,
2375                                                ident.Domain(),
2376                                                ident.User(),
2377                                                ident.Password(),
2378                                                &sessionState,
2379                                                &mAuthContinuationState,
2380                                                getter_Copies(temp));
2381 
2382                 entry->mMetaData.swap(sessionState);
2383                 if (NS_SUCCEEDED(rv)) {
2384                     creds = temp.get();
2385 
2386                     //
2387                     // set the cached authorization credentials to
2388                     // those we used (in case we used the URI
2389                     // specified credentials)
2390                     rv = authCache->SetAuthEntry(host, port, path,
2391                                                  entry->Realm(),
2392                                                  creds, challenge,
2393                                                  ident, sessionState);
2394                 }
2395             }
2396         }
2397         if (creds[0]) {
2398             LOG(("   adding \"%s\" request header\n", header.get()));
2399             mRequestHead.SetHeader(header, nsDependentCString(creds));
2400         }
2401         else
2402             ident.Clear(); // don't remember the identity

    
    

    
  

      
      
      
      

        Attached patch
          
          
        SPNEGO patch (obsolete)
        — 
          — Splinter Review
      

    


  
I see that work have started here again. Nice!

I have from original patch updated it with SPNEGO so that it works with
IIS. I can now do automatic login from Mozilla under Solaris to an IIS
server, and to my own Unix-web server that also have negotiate/SPNEGO in it.
I have been using it for over a month now.

The above patch is for mozilla 1.4, but can also be used on 1.4.1 and 1.5.

The code is just for compiling under Solaris. But adding how to find
libraries and include files on other plattforms should be easy.
I have not compared how my changes compares to the latests patches that
I now see have been added to this bug. I hope my patch can be merged
with the other new stuff.

About my patch:
In the code is included a number of source files for handling
ASN1 parsing and SPNEGO parsing. These are from microsoft who
have published them on the web site to help people get
SPNEGO working. I do not know if Mozilla folks do not want to
use them because Microsoft have created them, though it is quite
easy to write your own. These files can also help enable you to
implement SPNEGO in apache, if you like.
The source code come from a 3 part article starting at
http://msdn.microsoft.com/library/default.asp?url=/library/en-us/dnsecure/html/h

ttp-sso-1.asp

Some changes to nsHTTPGssapiAuth.cpp:
LogGssError now prints both major and minor code. The minor did not
always explain what was wrong.

I have removed the questions of need to free some strings where
I think it is not needed.
I have simplified code in ChallengeRecieved and
GenerateCredentials removing continuationState handling
as I do not think that is needed.
As we do not do mutual authentication and send Kerberos data in
initial packet, we should never get a "Negotiate data" back -
if we do we cannot handle that. We could add that later, if needed.
It works for me as it is now.

There are some comments in the code that hopefully explain some
of my choices.

In GetAuthFlags I removed REUSABLE_CHALLENGE as that
results in Mozilla generating Negotiate data for every
http query on the same connection. It looks like this makes the
CONNECTION_BASED not work as it should.

    
    

    
  
I hate to be a PITA about this, but I strongly believe that this code
should NOT contain any SPNEGO or KRB5 code.  All that should be
done when the "Negotiate" is performed is the gss_init_sec_context
call (and other supporting GSSAPI calls).   If the underlying GSS 
system supports SPNEGO or Kerberos, the negotation will succeed. 

Especially because if you write the local SPNEGO
to be compliant with Microsoft's implementation, you end up with
a non-standard (i.e. in violation of the IETF RFC specification)
implementation.  The SPNEGO code from MS is broken in several ways
(Using the wrong KRB5 Mechanism OID for one, putting incorrect data
in the MechListMIC field for another).

Also, Mutual Authentication is useful to verify that the server and/or
KDC are not being spoofed and should be used.  Its not hard to handle
the continuation token.

SPNEGO is intended to be layered *under* the GSS-API layer, the app should
not be aware of the tokens being exchanged by GSS-API, putting ASN.1 
code directly in here violates the abstraction.

Putting local SPNEGO code into the browser will also cause problems 
on systems that have existing SPNEGO mechanisms already included
that work properly with GSS-API.

If there is strong concensus that SPNEGO code should be included here,
it should be a user-configurable option whether or not to use the
included spnego or use the system supplied GSSAPI mechanisms.





    
    

    
  

      
      
      
      

        Attached patch
          
          
        Moved into Extensions (obsolete)
        — 
          — Splinter Review
      

    


  
Here is an updated version of the patch from Wyllys (originally from Daniel)
which moved Negotiate to extensions/negotiateauth.   It compiles fine, but
there are some linking dependencies I haven’t working which would require
addition changes to the configure script.

I’ve had some trouble building against MIT Kerberos 1.2.7 (maybe it would work
better with 1.3.1?).   Kerb 1.2.7 (at least on Linux) doesn’t define
GSS_C_NT_HOSTBASED_SERVICE and configure seems to think I should have it and
compiled it in so it won’t load.

When I hacked it up to make it compile, the code failed in
gss_init_sec_context.after retrieving a service ticket but before, creating any
output tokens.	I didn't have a change to look into this, it just gave a
generic failure error.
 
Another problem we are going to have if MIT Kerberos is configured to use DNS
lookups, then any Kerberized app will also require libresolv.	There will have
to be some magic done in the configure script to figure out if libresolv is
required.

I removed most of the modified code in nsHttpChannel, because I don’t believe
it is required anymore for the current trunk.	I left the mutual
authentication check there till Darin comments on whether he will accept it.

In order to try it out, you must apply the patch, then run autoconf  and place
the following in your .mozconfig

ac_add_options --enable-extensions=default,irc,negotiateauth
ac_add_options --enable-gssapi
ac_add_options --with-gssapi=<Kerberos Directory>

I also added a command to the make file for negotiateauth, so “make XPI” will
make a redistributable XPI.   Feel free to leave this out of the final patch,
but it helps with testing.

Other things I noticed in that if we want the same behavior as IE we will need
the following flags.

*flags = CONNECTION_BASED | REUSABLE_CHALLENGE | IDENTITY_IGNORED;

    
    

    
  
+# The Initial Developer of the Original Code is
+# Netscape Communications.
+# Portions created by the Initial Developer are Copyright (C) 2001
+# the Initial Developer. All Rights Reserved.
+#
+# Contributor(s):
+#  Brian Ryner <bryner@netscape.com>

looks like the copyright notices need to be updated.  there's another one that
is likewise bogus.

as for the nsHttpChannel changes, i'm thinking about how best to incorporate
those.  i think it might be sufficient to call ChallengeReceived on the
authenticator when a 200 OK response carries with it a challenge blob.  though
it is not actually a challenge, this seems like the right way to handle this
situation.  short of that, i might add a new method to nsIHttpAuthenticator, but
i'd prefer to avoid breaking the binary contract of that interface.

i was also wondering if other protocols, besides HTTP, such as IMAP or SMTP can
use Negotiate auth.  i know that those protocols can use NTLM auth.  if it turns
out that Negotiate auth can be used with other protocols, then maybe we should
factor things out a little differently.

    
    

    
  
*  MIT 1.3.X (finally!) fixes the missing GSS_C_NT_HOSTBASED_SERVICE problem.

* The comment above the code added to ProcessResponse is incorrect, there is no
check for "Content-Length == 0" and that part of the comment should be removed.
 I thought I had removed this from the patch before I submitted it, but I must
have left it in by accident.

* I don't believe MS uses the same sort of "Negotiate" authentication in other
protocols.  They do use SPNEGO negotation in other places, for example secure
LDAP authentication uses SASL + GSSAPI + SPNEGO + (NTLM | Kerberos).  I don't
know if it is used for IMAP or SMTP.  I do know that IMAP can support SASL,
which in turn can use a GSSAPI plugin.

* On Solaris, you can use the "-z lazyload" option when linking with libresolv
so that it will only load if necessary, I don't know if this option exists with
gnu ld.   The other choice might be to just link with libresolv all the time,
its not that costly if its never used.  Or, just add a new "AC_TRY_COMPILE"
block to test for the need to have libresolv present.

* Finally - THANKS FOR INCLUDING THIS ONE!


    
    

    
  
<I might add a new method to nsIHttpAuthenticator

I agree, I think it will require a new method to nsIHttpAuthenticator in order
to use mutual auth. The above patch assumes that if the server sends a negotiate
response, that the client should process it to check if mutual auth was
successful.  If the server does not send send a negotiate response (because the
server is mis-configured or is not legitimate), the client will not notice that
the response was skipped and proceed without the response.  There needs to be
some state kept on the client side which will not allow it to proceed with the
current authenticator if mutual auth is required but never completes.  That
would require keeping track of the current authentication scheme between
requests, and whether the scheme requires more information from the server.

    
    

    
  
Bug 224749 added to fix a problem with the REUSABLE_CREDENTIALS flag being ignored.

    
    

    
  
The patch does keep state in the "nsGssapiSessionState" object, I should have
added a flag to track whether or not the mutual auth completed correctly and
checked for this before allowing the session to complete.  It would be easy
enough to add to that class and make things right.  That state object was
created so that the gssapi context pointer could be carried around when mutual
authentication was being done and the gss_init_sec_context returned the
CONTINUE_NEEDED status. I should have been more careful marking the session as
properly authenticated.  The fix is easy, though.

    
    

    
  
I agree with  Wyllys Ingersoll that SPNEGO should not be in the code, if it
is available natively. I included it in my patch as I have waited a very
long time for Sun to include it and finaly I gave up and added it myself.
If the most important systems now does include it, we do not have to
include it. I do not want this patch to force me to once again have to
wait for Sun to include SPNEGO before being able to use it.

Looking at the "moved to extensions" patch I wonder if you have fixed
the code in nsHttpChannel.cpp so it is able to try several authentication
mechanisms? Like in older patches. It is needed as more then one
can be specified and "negotiate" may fail so fallback to one other
is needed. Like I today get negoatiate and basic, and if negotiate
fails (for example due to not logged in with Kerberos) mozilla
prompts for user/password and uses basic.

    
    

    
  
Systems with Samba 3.0 can use SPNEGO via the ntlm_auth helper program. 
Documentation is sparse (bug me if you really are intending to use it) but
ntlm_auth does act as an SPNEGO client and server.

This also allows the use of NTLMSSP within SPNEGO, and allows for Mozilla to
follow Samba as we better understand bits and peices of this protocol.

Andrew Bartlett
(Samba Team)

    
    

    
  
In reply to comment 65:

bug #180049 fixed the problem you are referring to on the trunk.   We are still
waiting to see if it gets checked into the 1.4 branch.

Bug 224749 added as a dependency, this is a new problem on the trunk which will
prevent this patch from working. (It doesn't exist on the 1.4 branch)

I won't have time to update the patch with suggestions from other's comments for
a couple of weeks.   If I could pass it on to someone else for the time being. 
Maybe someone could volunteer on the list so we don't duplicate efforts? Wyllys?
Daniel? Someone else?
Depends on: 224749

    
    

    
  
If you agree I'd place the code on the negotiateauth.mozdev.org site. Now I'm
trying to make it work with Heimdal and I believe I'll be able to spend some
time on it in the future too.

    
    

    
  
It sounds like there are several possible paths to get SPNEGO functionality -
The MS code, SAMBA, Heimdal, etc.   I should add that a full blown
implementation is probably going to be in a future Solaris release ( I cannot be
more specific than that).   Im not sure of the best way to configure the code so
that it will succeed on various platforms.  I think it should be left generic so
that the local GSSAPI configuration dictates what is available and the browser
itself doesnt care where SPNEGO comes from. 

It should be noted that SPNEGO is *NOT* required for MS interoperability to
succeed [ See comment #37]. Its really not necessary to implement SPNEGO, as
long as Kerberos is available, the browser can just send GSSAPI/KRB5 tokens and
MS IIS will accept them as a response to the "Negotiate" authentication request.

The code I submitted (based on Daniel's code) has a check to see if the system
has support for SPNEGO, if so it uses it, otherwise it just used kerberos.  This
eliminates the need for any SPNEGO dependencies.

    
    

    
  
I'd also like to mention that current version of the kerberos module for Apache
(available from modauthkerb.sf.net) also doesn't require the use of SPNEGO and
it's able to work with both SPNEGO and plain Krb5 GSSAPI tokens. So SPNEGO is
not required for support of Kerberos in HTTP authentication (I'm not mentioning
situations where more GSS-API methods will be required)

    
    

    
  
>The code I submitted (based on Daniel's code) has a check to see if the system
>has support for SPNEGO, if so it uses it, otherwise it just used kerberos.  This
>eliminates the need for any SPNEGO dependencies.

Does it only use Kerberos or does it let gssapi select what to use?
One problem I had when doing my patches was that Sun by default do not use
Kerberos as default mechanism. This resulted in som diffie_hellman mech
was used instead resulting in not being able to do SPNEGO in one query/response.
And without SPNEGO Kerberos would never be tried.
While something else than Kerberos might be used in the future, both SPNEGO
and without should start with Kerberos to avoid failures or multiple
query/response to find correct mech.

The good thing with Kerberos is that just like when using authentication
Basic, it can be done with just one query and one response over the net.

    
    

    
  
In Solaris 9 and future releases, Kerberos will be the default mechanism.  In
earlier releases, DH was the default *until* the unbundled SEAM package was
installed in which case kerberos should have been made the default.  Local
administrative policy dictates the order of the mechanisms that gss will use in
Solaris through the editing of the /etc/gss/mech configuration file.

The gss initiator *may* specify that it wants Kerberos specifically (instead of
using GSS_C_NULL_OID, specify the correct Kerberos OID structure in the
init_sec_context call) if SPNEGO is not available in which case the system will
find Kerberos even if it is not configured as the default mechanism.

I agree, if SPNEGO is not available, the client should try to specify Kerberos
explicitly instead of letting the system choose the default, thats an easy fix,
though.

    
    

    
  
In reply to comment 68:

Any changes I made to your patch are to be covered under normal mozilla
licensing so do with it as you please.  I'm assuming your patch and everyone
elses suggested changes are also covered under normal licesensing.  Someone let
me know if that assumption is incorrect.

    
    

    
  
Which version of the patch is being considered?  I am willing to make fixes to
the patch I submitted, but I dont want to conflict with whatever code the
maintainers are working from.  Could someone post the code that is currently
under consideration so that fixes can be made to "current" code ?





    
    

    
  
John Brezak (Microsoft) has just published an updated draft describing the
HTTP-Negotiate protocol to the IETF Kerberos Working group.  It does not appear
to be any different from what was previously published, but at least the
document is now current (as opposed to "expired").

http://www.ietf.org/internet-drafts/draft-brezak-kerberos-http-00.txt

Also, can someone respond to my previous request for a pointer to the latest
code so I can make the requested fixes?


    
    

    
  
Is the final patch above ("Moved into Extensions") going to be included as part
of the next release or is someone waiting for additional changes of some sort?

I downloaded the latest code via CVS and did not see any of this stuff included
in the tree yet, so I'm curious as to the status.  

    
    

    
  

      
      
      
      

        Attached patch
          
          
        Updated code for extensions area (obsolete)
        — 
          — Splinter Review
      

    


  
This patch updates various autoconf files (configure.in and
config/autoconf.mk.in) to detect GSSAPI installations better (Solaris, MIT, and
Heimdal).  Also, it includes the latest code from Daniel.   Patches
"extensions" area so that "negotiateauth" extension module is created.

    
  

        Attachment #134485 -
        Attachment is obsolete: true

    
    

    
  
if the latest patch is ready for review, then please let me know, and i will
take a look.  at this point, it seems unlikely that it will make 1.6 final.

reassigning to myself for eventual checkin.
Assignee: ssaux → darin
Component: Client Library → Networking
Product: PSM → Browser
Target Milestone: Future → mozilla1.7alpha
Version: 2.4 → Trunk

    
    

    
  
Go ahead and review it, I want to move this along and be done with it:)


    
  

        Attachment #136299 -
        Flags: review?(darin)

    
    

    
  
What concerns the patch to be reviewed, I'm suggesting to replace function  
nsGssapiSessionState::nsGssapiSessionState() with:


nsGssapiSessionState::nsGssapiSessionState()
{
        OM_uint32 minstat, majstat;
        gss_buffer_desc buffer;
        gss_OID_set mech_set;
        int mech_found = 0;
        unsigned int i;
        gss_OID item;

        mCtx = GSS_C_NO_CONTEXT;
        mech_oid = &gss_krb5_mech_oid_desc;

        //
        // Now, look at the list of supported mechanisms,
        // if SPNEGO is found, then use it.
        // Otherwise, set the desired mechanism to krb5
        //
        // Using Kerberos directly (instead of negotiating
        // with SPNEGO) may work in some cases depending
        // on how smart the server side is.
        //

        majstat = gss_indicate_mechs(&minstat, &mech_set);
        if (GSS_ERROR(majstat))
           return;

        for (i=0; i<mech_set->count; i++) {
           item = &mech_set->elements[i];
           if (item->length == gss_spnego_mech_oid_desc.length &&
                !memcmp(item->elements, gss_spnego_mech_oid_desc.elements,
                        item->length)) {
              mech_oid = &gss_spnego_mech_oid_desc;
              break;
           }
        }
        (void) gss_release_oid_set(&minstat, &mech_set);
}

add lines 

static gss_OID_desc gss_krb5_mech_oid_desc =
{9, (void *)"\x2a\x86\x48\x86\xf7\x12\x01\x02\x02"};

static gss_OID_desc gss_spnego_mech_oid_desc =
{6, (void *)"\x2b\x06\x01\x05\x05\x02"};

to the beginining the file

and remove all occurences of the gss_release_oid() call. These changes removes
dependances on functions which are not part of the GSS-API specs (and set
directly krb5 mechanism if spnego is not found, as mentioned in comment #72.

I'd also suggest to replace the nsCString types with EmbedString to allow the
component to build with standalone gecko SDK.

    
    

    
  
err, you should not put stuff that can fail into a constructor. instead, add a
::Init method, you can get xpcom to automatically call that when your component
is instantiated.

    
    

    
  
I suppose you mean the GSS_ERROR(majstat) checking result of
gss_indicate_mechs() in nsGssapiSessionState::nsGssapiSessionState(). But
failure of this call doesn't influence the initialization of the object, it only
causes that the krb5 mechanism will be used by the component and no other
mechanisms (especialy SPNEGO) will be looked for in the system.

    
    

    
  
I'm OK with the suggested change.  Daniel is correct, the failure of the
gss_indicate_mechs call does not cause the whole instantiation to fail, it just
causes the mech_oid to default to Kerberos instead of continue looking for
SPNEGO, which is acceptable.


    
    

    
  
If some actually has an RFC compliant implementation of SPNEGO for Mozilla to
pick up, will it even work with Microsoft’s implementation?  If not we should
always just use GSS Kerberos.

    
    

    
  
A "future release" of Solaris may very well have an RFC-compliant SPNEGO
mechanism.   It "may" also allow for a configurable flag (in the gss config
file) that turns it into a non-RFC-compliant-but-Microsoft-compatible SPNEGO
implementation.   

A strictly RFC compliant SPNEGO mech will NOT interoperate with Microsoft's
current SSPI-Negotiate mechanism.  However, they (Microsoft) are aware of the 
problem(s) and may choose to correct it in a future release.

Since its easy enough to support SPNEGO if its available, I think the code
should be left in place.   In the future, when new GSS mechanisms (SPKM, LIPKEY)
become available, negotiation of the security mech may actually become a
significant choice (or maybe not).


    
    

    
  
ah - yeah, this block:
        if (GSS_ERROR(majstat))
           return;

was indeed what I was referring to. Thanks for the explanation.

    
    

    
  
In comment 62 and comment 64 we discussed the problems with the way this code
implements mutual auth.   If the server doesn't send a negotiate token, Mozilla
won't try and process the mutual auth response but will proceed anyway. 

I suggest that mutual auth be moved out of this patch and another bug opened on
how to address mutual auth in a generic way that all authentication plugins can
take advantage of. If possible there should never be mechanism specific code
outside of the plugin.

Reiterating my comment 54:

In addition, SPNEGO is susceptible to Man in the Middle attacks even with mutual
auth (at least for the max clock skew of the Init-sec-context blob).  Mutual
auth just ensures that the server is out there, not that the traffic is not
being captured and replayed by a third party in the middle.  Since the user is
never prompted he/she never gets to make the call whether they trust the remote
site enough to send their credentials to it, it just happens automatically and
transparently.   If the site is not legitimate it's too late. Actually prompting
the user would defeat the purpose of this patch, so their should be a pref where
the user can specify to which protocols, servers, or domains he trusts enough to
auto send his tokens.  This is basically the model Microsoft has used to keep
Negotiate in the Local Intranet Zone.

Pref Example

user_pref("browser.negotiateauth.urls","https://*.somesite.com;https://*.someothersite.com")

For Kerberos at least, most sites don't have a huge number of shared keys with
foreign KDC's, so usually they will only be able to get tickets for one, or at
least a small set of machines in different DNS domains.  So for Kerberos just
having a pref for requiring SSL, would be a good start because the acceptable
machine list is implicit to the protocol. The problem is that in practice most
users will just turn it off, so something like the list of trusted DNS domains
is needed in the long run.  

You will probably at least want to add IDENITY_IGNORED to your set of flags. 
You may also want CONNECTION_BASED if you want to make it exactly like
Microsoft's. Also you could add REUSABLE_CHALLENGE, but I'm not sure if it's
appropriate or not, it may break if SPNEGO is ever used for something besides
Kerberos.


    
    

    
  
Im inclined to agree about removing the mutual authentication option until we
can work out the details of how to correctly handle it.

I'll go ahead and update the patch to add the flags as you suggest as well.

I'll also remove the code from nsHttpChannel.cpp as its only needed when mutual
auth is requested, for non-mutual auth, the context initiation should complete
in  a single round-trip.


    
    

    
  
Regarding comment 87:

Of what I could understand, we should not need to have things
like trusted zones or trusted domains when using Kerberos.

There will be no tokens to send unless the remote host is
in the same Kerberos domain as the one I have logged in to, or the
other domain has a trust with my Kerberos domain.
You cannot get a ticket to send unless you have an account
in the remote domain and can get a ticket for that domain.

It may be problems with SPNEGO, but not with Kerberos. Or
have I missunderstood Kerberos?

And if I only can get a ticket to send to a server in a domain
my Kerberos domain trusts, then there is no need of having
a preference defining which domain to trust (or require them to be
SSL as Kerberos is already encrypted).


    
    

    
  

      
      
      
      

        Attached patch
          
          
        Updated patch  (obsolete)
        — 
          — Splinter Review
      

    


  
Updated with suggestions from Comment #87 and #88.

- MUTUAL_AUTHENTICATION flag is no longer used
- Changes to nsHttpChannel.cpp removed, all changes are isolated in the
extension
- IDENTITY_IGNORE | CONNECTION_BASED flags added to authentication handler.
- Cleaned up nsGssapiSessionState::nsGssapiSessionState() as suggested in
comment #80

        Attachment #136299 -
        Attachment is obsolete: true

    
    

    
  
You are correct about domains and machines you can access are implicit to the
kerberos protocol.   The problem is will you have to go through an untrusted
domain to get to your trusted one? Mozilla currently has no way to make that
assumption.   Are you sitting in a coffee shop with your laptop, in which case
there is no reason to trust DNS, the proxy server or anything else (maybe its
unlikely that you'll even be able to get kerberos credentials in such an
environment, but I don't really want to base security on that assumption)   You
will likely want to use SSL even to a site you considered trusted if you have to
go through an untrused network to get there. To refine what I suggested before,
the really only important part of the pref I suggested is that you can specifiy
which domains, are local and which are remote (or which ones I really trust and
which ones I don't). So if DNS siteA is my homesite, I may decide MITM attacks
are unlikely, so in that case I really don't need SSL, but but DNS SiteB, which
is a site I have a keberos shared key with is at another university.  To get to
siteB my connection must traverse the internet, where MITM attacks are more
likely, then I would want SSL.  

Another thing is that kerberos shared keys should not imply that I trust the
networks of KDC's from other realms.   My university could share keys with every
University in the United States, that does not mean I trust the networks of each
of these universities, or even that they trust me to log into any of their
machines. It's just that I now have a way of asserting my identity to them.  So
 I should be able to decide on a case by case basis if MITM middle attacks are a
viable threat, and so require SSL, or not require SSL. But in the general case,
I would be happy if we could just require it or not require it for everything,
but I'm sure most people would just use the option to not require SSL because
they would assume that SPNEGO will give them many of the same protections as
SSL, which is not true.

>Kerberos is already encrypted

Also note, that SPNEGO is for authentication only, the shared keys transferred
back and forth during the gss-init-sec-context and gss-accept-sec-context dance
are never used again.  Since the shared keys are not used to protect the
channel, there are very few of the normal kerberbos or SSL protections. There
are no Integrity, privacy, or Man-in-the-middle protections.  You need to add
SSL to the mix if you want those.

    
  

        Attachment #136361 -
        Flags: review?(darin)

    
    

    
  
Having an option to specify trusted domains would be nice.  Microsoft added it
for good reason.  I think that the whole concept of using Kerberos and HTTP
together was only ever intended for intranets and not necessarily for going over
the internet itself.  SSL protection is highly desirable in either case, though
not a requirement for successful authentication (obviously).  This feature is
important to advance the adoption of mozilla as a standard desktop browser for
intranets in companies where they have a Kerberos infrastructure and want to
extend SSO to their internal websites.  Currently, only IE is capable of doing
secure web logins this way.

Just to clarify, SPNEGO is a "pseudo-mechanism" that allows servers and clients
to negotiate a common security mechanism when both sides are not aware of the
others capabilities.  The security of the authentication is dependent upon the
security of the mechanism that is chosen, SPNEGO itself does not offer any
security beyond what is available in the security mechanisms chosen.

Kerberos does provide replay protection, though, so Im not sure if a replay
attack by a MITM is possible when Kerberos is the mech that is negotiated.


    
    

    
  
>>Kerberos does provide replay protection, though, so Im not sure if a replay
attack by a MITM is possible when Kerberos is the mech that is negotiated.

It can if the advesary owns the proxy server or DNS you are using. They control
what information runs through them (with DNS I mean they redirect your traffic
to one of their own machines), they will just interecpt all of your
communications and not send them on, at least until they are done altering them.
 They will just steal info from the WWW-Authenticate header and reform the data
portion to their liking.   They can even do the same with the response, you will
get a mutual authenticate token, but again they get to read and change the reply
before they send it to you.  Replay caches have very little value in this senario.

Replay caches are useful if you don't go through a compromised proxy and DNS is
legit, then its just a race to see who can get their data to the webserver
first. The orginal version of the data or their altered version.  At least in
this senario web server logs may point to something funny going on.

    
    

    
  
I don't like this part of the patch, gss_str_to_oid was removed from
gssapiv2

+	// Create an OID for SPNEGO mechanism.
+	//
+	buffer.value = (char *)"{ 1 3 6 1 5 5 2 }";
+	buffer.length = strlen((char *)buffer.value);
+#ifdef HAVE_GSS_STR_TO_OID
+	majstat = gss_str_to_oid(&minstat, &buffer, &mech_oid);
+#else
+	majstat = 10000; /* XXX simulate error */
+#endif
+	if (majstat == GSS_S_COMPLETE) {


Why not just replace it with

static gss_OID_desc mech_oid_desc =
	{6, (void *)"\x2b\x06\x01\x05\x05\x02"};
gss_OID mech_oid = &mech_oid_desc;

?

As a comment to #85, Heimdal (current not released yet) includes a SPENGO
that interrops with MS SPENGO (that is used in IIS). There is a test http
client (that is just built, not installed) that include with heimdal
distribution that tests this.

The goal is to get a rfc compliant SPNEGO.

    
    

    
  
In response to Comment #94 - the gss_str_to_oid stuff was removed in the patch
that I posted this morning.

In response to Comment #93:
I believe the scenario being discussed is where a DNS server is corrupted to
direct kerberos traffic to an "evil" KDC server that issues bogus tickets.
This is a general problem for Kerberos, but Im not sure that scenario you
are describing is valid.  

Assuming that the HTTP server has a local keytab with a service key stored that
came from the original (good) KDC,  it will reject the tickets that were 
modified by the MITM because the encrypted authenticator part of the bogus
ticket will not be decryptable using the http servers real key.  To be
successful, the MITM would need to have access to the HTTP service key on the 
web server.  IN that case, all is lost because the web server is already 
compromised.

Anyway, its probably a discussion best taken off line, perhaps to the IETF krb5
WG list or the MIT developers list.



    
    

    
  
so the new patch is an improvement

but I still think nsGssapiSessionState is broken, in the current state it will
always select the krb5 mech

shouldn't it look for SPNEGO

-	mech_oid = &gss_krb5_mech_oid_desc;
+	mech_oid = &gss_spengo_mech_oid_desc;

and then when not found revert to let the gss implemetion guess

if (!found)
  mech_oid = GSS_C_NO_OID;

?

also the variable buffer in that function is unused

    
    

    
  
There is a problem with my loop that looks for SPNEGO.
If SPNEGO is available, it should be used, otherwise, default
to KRB5.  KRB5 is the fallback default.
I think its OK to set mech_oid = &gss_krb5_mech_oid_desc, as long
as it switches to SPNEGO later if possible.

If this fix is OK, I'll update the patch sometime next week:

Fix:
+	//
+	// Now, look at the list of supported mechanisms,
+	// if SPNEGO is found, then use it.
+	// Otherwise, set the desired mechanism to
+	// GSS_C_NO_OID and let the system try to use
+	// the default mechanism.
+	//
+	// Using Kerberos directly (instead of negotiating
+	// with SPNEGO) may work in some cases depending
+	// on how smart the server side is.
+	//
+	majstat = gss_indicate_mechs(&minstat, &mech_set);
+	if (GSS_ERROR(majstat))
+		return;
+	for (i=0; i<mech_set->count && !mech_found; i++) {
+		item = &mech_set->elements[i];	
+		if (item->length == gss_spnego_mech_oid_desc.length &&
+		    !memcmp(item->elements, gss_spnego_mech_oid_desc.elements,
+			item->length)) {
+			mech_found = 1;
+                       mech_oid = &gss_spnego_mech_oid_desc;
+			break;
+		}
+	}

    
    

    
  
Not an evil KDC but an evil web server posing as the end web server but really
just altering and relaying the traffic. All tickets are valid.  There is no
cryptographic connection between the auth tokens and the data being sent, you
can switch out the data portion without trouble.

But my point is that we need at least SSL and something like this added to
ChallengeReceived or GenerateCredentials would do it.

You could do the following to get the pref

nsCOMPtr<nsIPref> prefs(do_GetService(NS_PREF_CONTRACTID, &rv));
prefs->GetBoolPref("network.http.auth.spnego.requireSSL", &requireSSL)

And the following to test if its SSL
rv = channel->GetURI(getter_AddRefs(uri));
uri->SchemeIs("https", &usingSSL);

Also we probably don't want to send delegated credentials to everything.  Most
people will not ever require it, but some will.  There should be a pref for that
as well.  The biggest problem with delegating constantly (besides security) is
that MIT kerberos will hit the KDC with every request of the web server to get a
cred to delegate (these are not generally cached).   So with a page with 4
images, a user will hit the KDC 5 times (the html page + the 4 images) and get a
new TGT with each request.

    
    

    
  
I am not sure the right way is to have a pref like:
user_pref("browser.negotiateauth.urls","https://*.somesite.com;https://*.someothersite.com")
because it is not SPNEGO that is the problem. Instead it is the
authentication mechanism.

Would it not be better if the user could for each authentication mechanism,
like Kerberos, NTLM, "basic",..., define what should be applied for it?
I see that MS IE have things like: enable, disable, prompt.
I would like to be able for each mechanism set: disabled, enabled for all,
enabled for a restricted case, or prompted for ok.
This way I could define, for example, that Kerberos is ok in domain xxx.yy,
NTLM is never OK and "basic" is allowed in domain xxx.yy over SSL.

The general user will not understand negotiate but might better understand
the mechanism names. Also, as allowing negotiate would allow any
mechanism negotiated to be used, I really could not allow negoatiate
to be used at all unless I know that the negotiate protocol will never
negotiate an unsafe mechanism.

When you use SPNEGO you need to be able to tell it which mechanisms
it is allowed to negotiate (and you cannot do this in a general global
configuration file as it depends on who you negotiate with).



    
    

    
  


  
I've updated the patch to reflect the changes mentioned in comment #97.

        Attachment #136361 -
        Attachment is obsolete: true

    
    

    
  
Comment on attachment 136869 [details] [diff] [review]
Updated to reflect changes in mech_oid selection


Any chance we can get this fixed in 1.6B instead of waiting another 3 or 4
months for the next release?

        Attachment #136869 -
        Flags: review?(darin)

    
    

    
  
+       //
+       // Now, look at the list of supported mechanisms,
+       // if SPNEGO is found, then use it.
+       // Otherwise, set the desired mechanism to
+       // GSS_C_NO_OID and let the system try to use
+       // the default mechanism.
+       //
+       // Using Kerberos directly (instead of negotiating
+       // with SPNEGO) may work in some cases depending
+       // on how smart the server side is.
+       //

the comment is not correct, GSS_C_NO_OID isn't used but the kerberos oid directly

    
    

    
  
In Reply to comment 99: Yes, most of my security assumptions were based on
SPNEGO negotiating Kerberos, and that may not be the case.  It does make sense,
to split up the authentication protocols for SPNEGO under different preferences,
the problem is I don't know if that is possible in any sort of cross
implementation manner.  Is there a standard API for SPNEGO that at the
application level you can specify what can be negotiated or is all of that
buried in SPNEGO or OS Specific config files?  Spnego should not be enabled by
default if we don't know what a user the user's machine has been configured to
support, what if for whatever reason, sending a cleartext password was what was
negotiated, that would be terrible for security.   Microsoft mixed NTLM and
Kerberos for negotiate, I personally would not want to treat them the same, or
put them at the same level of trust for auto-sending authentication tokens
without user input.   

So if we decided to use SPNEGO, there should be a way to limit to only to a
particular protocol and DNS space (User configured for his/her Intranet DNS
space) and disabled by default, till user decides he wants/trusts it based on an
informed decision on how SPNEGO on his system is configured. 

If we just support Kerberos, the same is true, but it should also be turned off
be default or at least require SSL, be default until a user tweaks some config
setting and tells it otherwise.

Also does anyone have any links that point other Kerberos web standards besides
the Microsoft's draft?  NCSA's httpd seemed to support this, but I've only found
sketchy details.

http://hoohoo.ncsa.uiuc.edu/docs/tutorials/howto/kerberos.html 

    
    

    
  
Because SPNEGO is a GSSAPI mechanism, the API should be the GSS-API (v2), it
should definitely NOT be a separate API.

Defining what mechanisms SPNEGO will negotiate is a system-wide policy decision,
it should probably not be made on an application-by-application basis.  Though,
one can see why it might be desirable to do so in some cases, I would tend
towards leaving that as an administrator's policy decision.  SPNEGO will
negotiate whatever the system is configured to offer in union with whatever
credentials are available for that user.  No sense offering a mechanism for
which the user cannot acquire credentials.

Also, see: http://www.ietf.org/internet-drafts/draft-nystrom-http-sasl-09.txt
for an alternate HTTP authentication method that is more popular within the
IETF security circles.   The SPNEGO method proposed by Microsoft is an
"individual submission" and is not to be treated as a standard of any sort, its
merely a submission that describes an existing technology, it has not been
adopted by any working groups for development into a "standard".

In fact, the KRB5, GSSAPI, and HTTP people within the IETF are not very
enthusiastic about the MS implementation.  Unfortunately, its already out there
and now people are wanting to use it.

there is







    
    

    
  


  
I've updated 'nsHttpGssapiAuth.cpp' with a couple of minor fixes:
1 - don't call gss_release_oid on 'mech_oid' because mech_oid is no longer
    dynamically allocated.
2 - fixed the mech selection loop to select SPNEGO if its found (as was
    originally intended).  As written in the original patch it was not
    actually selecting SPNEGO if available.  
    The default is still to choose KRB5, only use SPNEGO if the system says
    its available.

    
    

    
  
Is this extension going to be reviewed for 1.7?

    
    

    
  
this extension is going to be reviewed for 1.7.  it is on my plate to get it
into the tree.

    
  

        Attachment #136299 -
        Flags: review?(darin)

    
  

        Attachment #136361 -
        Flags: review?(darin)

    
    

    
  
>AC_CHECK_HEADERS(gssapi/gssapi.h, [GSSAPI_INCLUDES="-I/usr/include/gssapi"])  
 GSSAPI=1

i know this line is only evaluated in the solaris builds, but isn't it possible
that gssapi/gssapi.h might be under /usr/local/include instead of /usr/include?
 i ask because it seems like AC_CHECK_HEADER is going to check more than just
/usr/include.

 	
>	if test "$GSSAPI" = "1" ; then
>		AC_DEFINE(GSSAPI)
>		MOZ_EXTENSIONS="$MOZ_EXTENSIONS negotiateauth"	
>	fi

i noticed that you are unconditionally building the negotiateauth extension if
GSSAPI exists.  shouldn't it be the case that you only check for GSSAPI if the
negotiateauth extension has been listed as an extension to be built?  (i.e., via
configure --enable-extensions=default,negotiateauth)... though we probably do
want to make negotiateauth be built by default.  the point is that someone could
also type "configure --enable-extensions=default,-negotiateauth" to disable
negotiateauth.  we should not override that.


the patch includes some files that should not be there:

  nsIHttpAuthenticator.idl
  nsIHttpAuthenticator.h
  Makefile

please let me know if you have included modifications to these files.  i'm
guessing that isn't the case ;)


>-. ${srcdir}/config/chrome-versions.sh
>-AC_SUBST(MOZILLA_LOCALE_VERSION)
>-AC_SUBST(MOZILLA_REGION_VERSION)

i see these lines being removed from configure.in.  i presume that this wasn't
meant to be included in the patch.

    
    

    
  
In Solaris, we very much want to use the standard GSSAPI header and library
bundled with the OS (under /usr/include) and not someone's alternate version in 
/usr/local.  If the standard gssapi headers and library is not found, it should 
default to looking in /usr/local.  But for 99% of the Solaris population
(Solaris 2.6 and later), the Solaris GSSAPI (/usr/include/gssapi/gssapi.h and
/usr/lib/libgss.so.1) should be preferred and highly encouraged.  Solaris has
the only GSSAPI library that is actually "pluggable" and will dynamically load
mechanisms other than KRB5.  MIT KRB5 includes a GSSAPI library, but it really
only supports KRB5.   Future Solaris versions may include an SPNEGO plugin as
well as others, so I'd like Solaris users to use the Solaris code wherever possible.


I agree with your 2nd point - we probably do want negotiateauth to be on by
default, but it certainly should be de-configurable as well.  

Finally - I did not make changes to those files you listed (they should not have
been part of the patch) and I did not mean to include that last bit of change to
configure.in, you can disregard that stuff. Its probably a "diff" **** from when
I generated the patch.

thanks for looking at this stuff...

    
    

    
  
Comment on attachment 137883 [details] [diff] [review]
Updated nsHttpGssapiAuth.cpp file

>/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
>/* The contents of this file are subject to the Mozilla Public License Version
> * 1.1 (the "License"); you may not use this file except in compliance with
> * the License. You may obtain a copy of the License at
> * http://www.mozilla.org/MPL/
> *
> * Software distributed under the License is distributed on an "AS IS" basis,
> * WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License
> * for the specific language governing rights and limitations under the
> * License.
> *
> * The Original Code is the Negotiateauth
> *
> * The Initial Developer of the Original Code is Daniel Kouril.
> * Portions created by the Initial Developer are Copyright (C) 2003
> * the Initial Developer. All Rights Reserved.
> *
> * Contributor(s):
> *   Daniel Kouril <kouril@ics.muni.cz> (original author)
> *   Wyllys Ingersoll <wyllys.ingersoll@sun.com>
> *   Christopher Nebergall <cneberg@sandia.gov>
> */

have you considered licensing this file under the new MPL/GPL/LGPL tri-license?
there is an effort underway to make most of mozilla fall under that license.
see http://www.mozilla.org/MPL/relicensing-faq.html for more info.


>/* this #define must run before prlog.h is included */
>#define FORCE_PR_LOG 1

are you sure you want to enable logging in release builds?

typically, you want to do this:

#ifdef MOZ_LOGGING
#define FORCE_PR_LOG
#endif

that way "configure --disable-logging" will work :-)


>#if defined(PR_LOGGING)
>
>    PRLogModuleInfo *gHttpLog = nsnull;
>    static PRLogModuleInfo* gNegotiateLog = nsnull;
>
>#endif

declaring "gHttpLog" doesn't make any sense here.



>class nsGssapiSessionState : public nsISupports
>{
>public:
>    NS_DECL_ISUPPORTS
>
>    nsGssapiSessionState();
>    virtual ~nsGssapiSessionState() {

destructor does not need to be virtual.  instances of nsGssapiSessionState
are only ever deleted via the Release method which is defined on this class
so it does not need a virtual destructor.


>    NS_IMETHOD Reset() {

this method does not need to be virtual.  NS_IMETHOD declares it to be
virtual.  better to just declare this method like so:

     nsresult Reset()

hmm... though it looks like it never fails, so why not just return |void|?
yes, in fact you never check the return value, so

     void Reset()

would be best :)


>nsHttpGssapiAuth::nsHttpGssapiAuth()
>{
>   NS_INIT_ISUPPORTS();

NS_INIT_ISUPPORTS is no longer needed in Mozilla code.	it is defined
as an empty macro now.


>nsHttpGssapiAuth::~nsHttpGssapiAuth()
>{
>}

define this destructor inline... and be sure to NOT declare it virtual
for the reasons i gave above for nsGssapiSessionState.



>    if (!session) {
>	session = new nsGssapiSessionState();
>	if (!session)
>		return(NS_ERROR_OUT_OF_MEMORY);

nit: looks like a tab character crept in here.	please de-tab
source files.




perhaps this method should be wrapped in a #ifdef MOZ_LOGGING block.

>void
>nsHttpGssapiAuth::LogGssError(OM_uint32 maj_stat, OM_uint32 min_stat, char *prefix)
>{
...
>   // LOG(("%s", ToNewCString(error)));
>   LOG(("%s\n", error.get()));
>}

remove the ToNewCString comment.




>nsHttpGssapiAuth::GenerateCredentials(nsIHttpChannel *httpChannel,
...
>   NS_ENSURE_ARG_POINTER(creds);

no need for a runtime check here.  a debug assertion is sufficient.
"garbage in equals garbage out" as far as i'm concerned.

    NS_ASSERTION(creds, "null param");


>   major_status = gss_import_name(&minor_status,
>                                 &input_token,
>#ifdef HAVE_GSS_C_NT_HOSTBASED_SERVICE
>                                 GSS_C_NT_HOSTBASED_SERVICE,
>#else
>				 gss_nt_service_name,
>#endif
>                                 &server);

nit: indent gss_nt_service_name by one more space ;)


>   input_token.value = 0;
>   input_token.length = 0;
>   if (GSS_ERROR(major_status)) {
>      LogGssError(major_status, minor_status,
>		(char *)"gss_import_name() failed");
>      return NS_ERROR_FAILURE;
>   }

i would recomment defined LogGssError like this:

#ifdef MOZ_LOGGING
void
nsHttpGssapiAuth::LogGssError(OM_uint32 maj_stat, OM_uint32 min_stat, char
*prefix)
{
...
}
#else
#define LogGssError(x,y,z)
#endif

that way you don't have to add #ifdef MOZ_LOGGING where ever LogGssError
is called.


>   if (len > strlen(NEGOTIATE_AUTH)) {
>	challenge += strlen(NEGOTIATE_AUTH);

most compilers will optimize away strlen of a string literal, but it might
be better to use sizeof to help the compiler.  how about defining a macro
like this:

#define NEGOTIATE_AUTH_LEN  (sizeof(NEGOTIATE_AUTH)-1)


>        input_token.length = (len * 3)/4;
>	input_token.value = malloc(input_token.length);
>	if (!input_token.value)
>		return (NS_ERROR_OUT_OF_MEMORY);

nit: fix indentation


>	if (PL_Base64Decode(challenge, len,
>		(char *) input_token.value) == 0) {
>		free(input_token.value);
>		return(NS_ERROR_UNEXPECTED);
>	}

nit: fix indentation


>      (void) gss_release_name(&minor_status, &server);

nit: i'm not a big fan of prefixing functions with (void) "when i choose
not to store the return value"... in the case of this source file, the
convention is only sometimes followed.	seems to me that it would be 
better to keep things consistent.  either always prefix with (void) or
never prefix with void :-)


>   // allocate a buffer sizeof("Negotiate" + " " + b64output_token + "\0")
>   *creds = (char *) malloc (strlen(NEGOTIATE_AUTH) + 1 + strlen(encoded_token) + 1);

since this value is being passed out of your module, you should allocate 
memory using nsMemory::Alloc instead.



this last code block seems like it could be simplified to this:


   nsresult rv = NS_ERROR_OUT_OF_MEMORY;

   char *encoded_token = PL_Base64Encode((char *)output_token.value,
					 output_token.length,
					 nsnull);
   if (!encoded_token)
     goto end;

   LOG(("Sending a token of length %d\n", output_token.length));

   // allocate a buffer sizeof("Negotiate" + " " + b64output_token + "\0")
   *creds = (char *) nsMemory::Alloc(NEGOTIATE_AUTH_LEN + 1 +
strlen(encoded_token) + 1);
   if (!*creds)
     goto end;

   sprintf(*creds, "%s %s", NEGOTIATE_AUTH, encoded_token);
   rv = NS_OK;

 end:
   if (encoded_token)
     PR_Free(encoded_token);

   (void) gss_release_buffer(&minor_status, &output_token);
   (void) gss_release_name(&minor_status, &server);

   LOG(("returning the call"));

   return rv;
 }


though i'm not a huge fan of using goto statements, in this case i think
using a goto is reasonable since it keeps all your cleanup code in one
place.

        Attachment #137883 -
        Flags: review-

    
    

    
  
>are you sure you want to enable logging in release builds?

let me clarify that statement.  mozilla.org's release builds currently define
MOZ_LOGGING, but some folks may want to build trimmed down executables that do
not define MOZ_LOGGING :-)

    
    

    
  
What about the security problems with the patch I discussed previously?

1. It should give the user the option of requiring or not requiring SSL before
to automatically sending their tokens to the server.   This really should be a
client side decision.  I'm also hoping we default it requiring SSL, if a user
actually understands enough about the pref to go look for it, then he's welcome
to disable it.

user_pref("network.http.auth.spnego.requireSSL", true).

2. If is possible that any other protocol besides kerberos will slip in because
of the OS's ability to choose SPNEGO and negotiate a different possibly less
secure protocol(NTLM?), the user should be able to define his trusted DNS (or
IP) space.  The security of the browser should not rely on the configuration of
the OS it is running on.  The default should probably be no trusted DNS or IP
space.  

Example Configuration
user_pref("browser.negotiateauth.urls","https://*.somesite.com;https://192.168.*")

I volunteer to write this code, if necessary.  Since Wyllys is the patch
maintainer I won't move on this unless he approves, but I hope Darin will look
at my suggestions before accepting the patch as is.  I could, of couse, open new
bugs on these later, but if possible we shouldn't have nightly builds floating
around with inadequate security which could have been built in originally.

    
    

    
  
If Christopher wants to add the code to add those user_pref options, I think
that would be great.  They are good ideas, I just don't have the time to
do the work right now and I don't want to delay the adoption of this 
extension even further.   I don't consider them to be show-stopper problems,
but rather as enhancements that can be fixed later (or now, if time permits).

Regarding the code review comments,  I will take a look at the code
and update the patch accordingly.

thanks...

    
    

    
  
Regarding the copyright issue - Daniel Kouril is the original author, so I'll
defer to him regarding the copyright.  I'm fine with using the new standard
mozilla licensing as suggested.



    
    

    
  
(In reply to comment #113)
> If Christopher wants to add the code to add those user_pref options, I think
> that would be great.  They are good ideas, I just don't have the time to
> do the work right now and I don't want to delay the adoption of this 
> extension even further.   I don't consider them to be show-stopper problems,
> but rather as enhancements that can be fixed later (or now, if time permits).

chris, if you can make these enhancements happen in a timely manner, then i'm
all for including them.  i confess that i don't understand all the details (yet)
about what you were suggesting, but i agree that we shouldn't miss the
opportunity to build a secure system from the start.

    
    

    
  
In comment 112:
> I'm also hoping we default it requiring SSL, if a user
> actually understands enough about the pref to go look for it, then he's
> welcome to disable it.
> user_pref("network.http.auth.spnego.requireSSL", true).

> The default should probably be no trusted DNS or IP space.  
> user_pref("browser.negotiateauth.urls","https://*.somesite.com")

The above is ok if there is a user interface to set this up.
Otherwise I do not want the default to require SSL as we use this for our
internal net and do not want to teach everybody how to hack prefs.js
to get things to work.
Is Kerberos in spnego unencrypted?

    
    

    
  
> Is Kerberos in spnego unencrypted?

No, but the (http)data isn't bound to the spnego request, so if
see a spnego request on the wire you can do a reply attack (if
you can force the first session to be dropped)

http-spnego really needs to be run over a secure transport

    
    

    
  


  
Attached is the updated patch file.  I implemented the changes suggested by
Darren in his review.

    
  

        Attachment #136869 -
        Flags: review?(darin)

    
    

    
  
ok, a couple issues remain with this patch:

1) i don't see how it will support "configure
--enable-extensions=default,-negotiateauth" in case i want to compile without
negotiateauth extension.

2) need Daniel Kouril to agree to the MPL/GPL/LGPL tri-licensing.

3) need cneberg to complete the pref changes, etc.

milestone 1.7 alpha is fast approaching.  we can still land this during 1.7
beta, but it would be best to wrap things up quickly.

    
    

    
  
Wylls what OS/Kerberos combinations have you tried to compile Mozilla with after
applying this patch?  I can help test in Linux. I would like to get this option
enabled in the nightly and release builds if at all possible.

Pertinant Combinations (some neat to support and some are actually nightly build
platforms)

Default RH 7.3 Default install comes with dynamic libs for MIT 1.2.4 and puts
them in /usr/lib/kerberos. I think that's the LINUX nightlies and releases are
built on.

Sun OS 8 + Sun Kerberos (What Version??)

Sun OS 9 + Sun Kerberos  (What Version??)

Sun OS 8 + MIT Kerberos (what Version??) - Neat to have

Sun OS 8 + MIT Kerberos (what Version??)  - Neat to have

Mac OS X.Something (What version?) / What version of Kerberos does it come with?
 This is also a nightly build platform.

    
    

    
  
(In reply to comment #120)
> Wylls what OS/Kerberos combinations have you tried to compile Mozilla with after
> applying this patch?  

I have compiled and tested this on MacOS X 1.3.1 (Panther) using an Apache web
server with the latest mod_auth_kerb, which supports negotiate authentication.

    
    

    
  
I have tested the on the following:

Solaris 9 with SEAM 1.0.3 (i.e. bundled Solaris Kerberos bits)
- I did not test on older Solaris releases, but am confident
that it would work just as well because GSSAPI has been part of
Solaris since 2.7 (or maybe even 2.6, I would have to check).

RedHat Linux with MIT KRB5 1.3.1 installed in /usr/local

I don't have access to other systems (Mac OSX or other Linux distro's).

The configure script changes I added include an option for specifying the
location of the gssapi headers and libraries (--with-gssapi=PFX) 
to accomodate other locations, so I don't know that further changes are needed.



    
    

    
  
(In reply to comment #119)
> ok, a couple issues remain with this patch:
> 
> 1) i don't see how it will support "configure
> --enable-extensions=default,-negotiateauth" in case i want to compile without
> negotiateauth extension.
> 

I will post a fix for the configure.in script to address this.

> 2) need Daniel Kouril to agree to the MPL/GPL/LGPL tri-licensing.

I emailed Daniel.  If we don't hear from him in a reasonable time,
I think we should go with the new tri-license and not let this
be a show-stopper.

> 
> 3) need cneberg to complete the pref changes, etc.

Chris' changes are definitely good to have, but not necessary
for moving forward.  If he has an update in time for  the next
milestone, then thats great, but if not we should not delay
getting the existing work included in its current state (code
review pending, of course).


> 
> milestone 1.7 alpha is fast approaching.  we can still land this during 1.7
> beta, but it would be best to wrap things up quickly.

Agreed.




    
    

    
  
> 2) need Daniel Kouril to agree to the MPL/GPL/LGPL tri-licensing.

I have no problem with this change

    
    

    
  

      
      
      
      

        Attached patch
          
          
        Complete updated patch (obsolete)
        — 
          — Splinter Review
      

    


  
Patch updated as follows:

1. copyright notice updated in all new files to reflect tri-license.
2. Modified configure.in to allow for the negotiateauth extension to be
   disabled as suggested by reviewer.
3. patch created against code downloaded from CVS last night.

    
  

        Attachment #136869 -
        Attachment is obsolete: true

        Attachment #137883 -
        Attachment is obsolete: true

        Attachment #140107 -
        Attachment is obsolete: true

    
    

    
  
>RedHat Linux with MIT KRB5 1.3.1 installed in /usr/local

Unless this compiles with MIT 1.2.4 I don't think it'll compile on the Linux
tinder box. If necessary, we could up the requirements, but I'm sure 80% of
redhat user's are using the MIT version that their linux distro came with.

I'm working on the added features, I'll add them to the newly updated patch you
wrote and re-submit it.

    
    

    
  
The GSS-API has not changed from 1.2.X to 1.3.X so compiling with MIT 1.2.4 
should not be an issue.  I did a build today on a RedHat system with 1.2.7
installed and it worked fine.  

2 minor fixes to the most recently submitted patch were needed to get it to find 
the proper headers, though:

[For new 'configure.in' script ]
+        dnl
+        dnl If GSSAPI libs were found, look for headers and try to
+        dnl determine if the GSS_C_NT_HOSTBASED_SERVICE value is
+        dnl defined.  Older MIT releases did not define this correctly.
+        dnl
+        if test -n "$GSSAPI_LIBS"; then
+            AC_CHECK_HEADERS([gssapi/gssapi_generic.h])
                               ^^^^^^^
+            AC_TRY_COMPILE([ #include <gssapi/gssapi.h> ],
+		[ gss_OID oid = GSS_C_NT_HOSTBASED_SERVICE; ],
+		[AC_DEFINE(HAVE_GSS_C_NT_HOSTBASED_SERVICE)],[])
+        fi

[Update to extensions/negotiateauth/nsHttpGssapiAuth.cpp ]

+
+#include <gssapi.h>
+#ifdef HAVE_GSSAPI_GSSAPI_GENERIC_H
             ^^^^^^^
+#include <gssapi/gssapi_generic.h> 
+#endif
+



    
    

    
  

      
      
      
      

        Attached patch
          
          
        working version (obsolete)
        — 
          — Splinter Review
      

    


  
I've attached my untested version with with the pref changes and the changes to
the config files mentioned in the previous comment.  Everything compiles fine,
but for some reason when I compiled against Kerberos 1.2.4 - I couldn't start
the browser, even if I removed the libnegotiateauth.so file, the previous
profile, and component.reg.  I've tried on two seperate machines.  I'll try
again with 1.3.1 when I get a chance. 

The two prefs are listed below. They create themselves as empty strings if they
aren't found. 

NegotiateAuth.delegationURIs 
NegotiateAuth.trustedURIs

By default the module is not enabled, because I have no idea what, if anything
would be a safe default because I have no idea what SPNEGO on that particular
machine may support.

Here are some examples of a valid pref, they can be whole domains, sub-domains,
or even specific machines.  You can prepend a protocol to any of these if you
only want it to work for HTTP or HTTPS.  Even putting a protocol by itself is
valid if you want it to work for every site using that protocol (https for
example). The list is semi-colon (;) seperated.

NegotiateAuth.trustedURIs "www.somesite.com;https://wholedomain.com;https://"

If you want it to work everywhere you can do this
NegotiateAuth.trustedURIs "http://;https://"

NegotiateAuth.delegationURIs works the same way but it is for a list of hosts,
domains, etc that you trust enough to delegate to.   This way we are not
delgating to every web server.	 In order to delegate to a machine it of course
must be matched up to something in both prefs.

    
    

    
  

      
      
      
      

        Attached patch
          
          
        test version (obsolete)
        — 
          — Splinter Review
      

    


  
I've attached my untested version with with the pref changes and the changes to
the config files mentioned in the previous comment.  Everything compiles fine,
but for some reason when I compiled against Kerberos 1.2.4 - I couldn't start
the browser, even if I removed the libnegotiateauth.so file, the previous
profile, and component.reg.  I've tried on two seperate machines.  I'll try
again with 1.3.1 when I get a chance. 

The two prefs are listed below. They create themselves as empty strings if they
aren't found. 

NegotiateAuth.delegationURIs 
NegotiateAuth.trustedURIs

By default the module is not enabled, because I have no idea what, if anything
would be a safe default because I have no idea what SPNEGO on that particular
machine may support.

Here are some examples of a valid pref, they can be whole domains, sub-domains,
or even specific machines.  You can prepend a protocol to any of these if you
only want it to work for HTTP or HTTPS.  Even putting a protocol by itself is
valid if you want it to work for every site using that protocol (https for
example). The list is semi-colon (;) seperated.

NegotiateAuth.trustedURIs "www.somesite.com;https://wholedomain.com;https://"

If you want it to work everywhere you can do this
NegotiateAuth.trustedURIs "http://;https://"

NegotiateAuth.delegationURIs works the same way but it is for a list of hosts,
domains, etc that you trust enough to delegate to.   This way we are not
delgating to every web server.	 In order to delegate to a machine it of course
must be matched up to something in both prefs.

    
    

    
  
I've been having trouble with recent builds (RH Linux), they build fine but the
browser never starts.    I don't think its related to this patch, though.  The
same code compiled and worked fine in Solaris x86, fails to start in Linux.  

Regarding the new prefs, just to be sure I understand:

NegotiateAuth.delegationURIs == list of URI patterns.
  - if the URI matches this list, then the Delgation flag is enabled
    thus making it possible for the users creds to be forwarded if
    possible. 

NegotiateAuth.trustedURIs == list of URI patterns
  - If the URI matches this list, Negotiation is allowed to proceed.
    Is this correct?  
    If so, then the default value for this list should be such that
    ALL URIs match and are allowed to proceed with negotiation.
    Users should be encouraged to tighten this control at their
    discretion, but initially it should not be restricted.
    Looking at the code submitted, it looks like if this pref is empty, then 
    nsHttpGssapiAuth::ChallengeReceived is always going to return
    NS_ERROR_FAILURE because 'match' will return FALSE and the negotiation
    will not proceed.  The default value for this should be
    such that all HTTP and HTTPS URIs match and are allowed to
    proceed with negotiation.
    I think the default should be:
       NegotiateAuth.trustedURIs="http://;https://"

Someone really needs to test this latest bit pretty soon.   I don't have
a Windows server that uses Auth-Negotiate handy anymore (it was 
recently decommissioned), but I hope to get another one up and
running soon.  

    
    

    
  
(In reply to comment #130)
   
>I've been having trouble with recent builds (RH Linux), they build fine but the
>browser never starts.    I don't think its related to this patch, though.  The
>same code compiled and worked fine in Solaris x86, fails to start in Linux.  

I tried with the nightly source tarball from yesterday and the official Mozilla
1.6 source, but I'll try again.

> Regarding the new prefs, just to be sure I understand:
> 
> NegotiateAuth.delegationURIs == list of URI patterns.
>   - if the URI matches this list, then the Delgation flag is enabled
>     thus making it possible for the users creds to be forwarded if
>     possible. 

Yes

> NegotiateAuth.trustedURIs == list of URI patterns
>   - If the URI matches this list, Negotiation is allowed to proceed.
>     Is this correct?

Yes
  
>     If so, then the default value for this list should be such that
>     ALL URIs match and are allowed to proceed with negotiation.
>     Users should be encouraged to tighten this control at their
>     discretion, but initially it should not be restricted.

You always need to default to secure. Only a small percent of people will ever
find this pref so unless the default is a secure configuration, it will almost
never be changed. Since this patch includes SPNEGO support we have no idea what
protocol's it may be autosending without the user's consent.  There is no way to
make an informed decision about the security impact of doing what you suggest.

If enabling SPNEGO was a pref but Kerberos was the default, we could make an
informed decision about the protocol because we actually know something about
it, but SPNEGO is a big unknown.   For example if the default was kerberos and
not SPNEGO.  I would feel more comfortable defaulting to "https://".

I understand my suggestions will limit the visiblity of the plugin, but I'm
uncertain how else to secure it.  Suggestions?
 
> Someone really needs to test this latest bit pretty soon.   I don't have
> a Windows server that uses Auth-Negotiate handy anymore (it was 
> recently decommissioned), but I hope to get another one up and
> running soon.  

I should be able to test everything on Monday.

    
    

    
  
It's a wonderful idea to create the world's most secure system possible.  I
assume therefore that you will include in this patch, a default that disables
basic authentication too?

In all seriousness, I get worried when I see good patches like this (which will
provide massive gains in system security and interopability) pushed to one side
because they are not infinitly secure.  The problem is, if the system is not
using SPNEGO, then what are they using?  

SPNEGO is indeed a DOG - schemes like SASL at least have a flags system for how
much protection each mech provides, and allows the admin to set 'nothing that
exposes a plaintext pw' or similar flags.  However, please do not let that get
in the way of the widespread useability of this system, particularly in the
corperate environment.

Andrew Bartlett
Samba Team

    
    

    
  
The middle ground solution to our dicussions is to only have it work
automatically for the Intranet.  The problem is there is no gui, or functions in
Mozilla that will give us this.  It is a security zones discussion and it
belongs in bug 169106.  Also, no one has listed what protocols they are going to
support in SPNEGO, MS choose Kerberos and NTLM.   I assume for compatiblity
reasons someone else will choose this combination as well. Up to this point
mozilla.org has choosen NOT to enable autosending of NTLM tokens without user
input. Discussions on why we should or shouldn't support autosending NTLM tokens
is Bug 231529.  I don't even know what other protocol's people may want to
support in SPNEGO because we are not using a Mozilla implementation, so I'm
assuming NTLM support in SPNEGO is probably one of the least security painful
protocols people will implement, but the list might be worse.  

The problem with SPNEGO is we need to limit the use of the protocol being
autosent to the most security senstive protocol that it can negotiate and there
is no way to know what that is.  Only the user is qualified to decide this, not us.
Depends on: 169106

    
    

    
  
SPNEGO only negotiates available GSSAPI protocols.  I don't know of any GSSAPI
modules that do NTLM (except for the Microsoft SSPI, obviously).    

I think we have a philosophical difference in  opinion as to who governs the
security policy on the system.  IMO, the administrator sets the security policy
for the system and this includes configuring what GSSAPI protocols  are
available for negotiation via SPNEGO.  I do not believe this should be a
user-level decision, as most users will opt for the path of least resistance,
which usually means disabling any hint of security because it usually involves
an extra few keystrokes.

Anyway, I think much of this debate is philosophical.  At this time, Kerberos is
the predominant GSSAPI security mechanism available on most unix platforms. 
SPNEGO is available on some, but even so, it usually just presents Kerberos as
the only negotiable mechanism.  So, at this time, I don't think its worth the
time and effort to configure a new 'pref' to allow for tweaking of the SPNEGO
mechanisms to be negotiated.  Leave the patch as it currently stands - it
presents SPNEGO if possible, else it defaults to KRB5.  NTLM is not an issue
unless someone can point to a GSSAPI-NTLM module that is supported on a platform
that also supports SPNEGO and KRB5. 

The mechanisms that SPNEGO presents should be configured external to mozilla in
a manner similar to however the system configures its GSSAPI offerings (example:
See the /etc/gss/mech file in Solaris).  SPNEGO and GSSAPI are not intrinsically
tied to mozilla, obviously, so the browser should not break the administration
model for those protocols by allowing another path for configuring them beyond
what the OS itself already offers.

I strongly dislike the "TrustedURIs" preference if it is going to default
to basically disabling the whole extension.   

Is important to keep an eye on the ball here, which is to give Mozilla equal
footing with IE in terms of supported authentication mechanisms, and, most
importantly, bringing REAL single-sign-on capability for web browsing to the
Unix world.   Limiting this ability by overengineering security at this point is
not going to help anyone.  Im in favor of TrustedURIs if it defaults to being
wide-open, as it currently is in IE.    Default support for the Intranet is also
acceptable to me since I believe that primary function of this method is for
intranet usage anyway and less-so on the intERnet.  SSO for internet usually
involves something more complicated like  Passport.

I really want to wrap this up and see it appear in an actual release, I think we
are close if we can agree on these final details and get someone to test Chris'
latest additions.




    
    

    
  
>I'm in favor of TrustedURIs if it defaults to being wide-open, as it currently
is in IE.

IE defaults to the Intranet, they choose to not leave it wide open. If a site is
not in the intranet zone, IE prompts the user with a password dialog in response
to Negotiate (instead of continuing to auto-send user tokens without user input,
which is what we are dicussing here.) This dialog is interesting, it is not for
Basic (it won't fall below Negotiate and use Basic if even if both Negotiate and
Basic our present), it will try to get a kerberos ticket for the user name and
password entered (even if they are different than the person logged in), and if
Kerberos can't be achieved, then NTLM data is created from that
username/password and the NTLM data is sent.

    
    

    
  
Since mozilla cannot currently restrict things based on zones such as "intranet"
as you describe in comment 133, then we should not try to restrict the URIs by
default.   Until someone writes an NTLM GSSAPI mechanism that is widely
distributed with mozilla, the downgrade attack is purely hypothetical anyway.

TrustedURIs is still a nice feature to have, but given the current state of the
browser and the availability of pluggable GSSAPI frameworks that support SPNEGO
and NTLM, I think making it default to tightly restricting the use of
auth=negotiation is unecessary and it will make it harder for people to use this
feature.







    
    

    
  
Have you checked how MS Windows handles this?  My understanding is that if you
use SPNEGO, you will be able to do an NTLMSSP authentication, if that's what the
server wants.  (This is certainly how it operates under CIFS)

Also, I expect NTLMSSP to be available under SPENGO on posix platforms.  Certain
proprietory GSSAPI libs already provide it.

    
    

    
  
It works that way under CIFS because MS provides KRB5 and NTLM SSP modules for
SPNEGO to negotiate.  As far as I know (and this may well be wrong) I don't know
of any NTLM GSSAPI modules in the Unix/POSIX world that are widely available. 
SPNEGO is completely independent from the underlying mechanisms that it
negotiates.  If the server says "I support KRB5 and NTLM", but the client only
supports KRB5, then KRB5 will be used. There is no requirement that SPNEGO
implementations support NTLM or that both sides support the same set, just that
there is 1 overlapping mechanism that is supported. 

What POSIX platforms offer NTLM GSSAPI mechanisms?   Linux - no.  Solaris - no,
HP-UX ???,  AIX - ???,  ***BSD - ???.  Even so, its not really relevant here. 
The  security that SPNEGO negotiates is not of any concern to mozilla.  As long
as this  module is capable of offering KRB5 *OR* SPNEGO, the rest should fall
into place.   Also, this feature does not even require SPNEGO, it will offer
KRB5 GSSAPI tokens if SPNEGO is not available on the system.

If someone wants to create an NTLM-GSSAPI module and offer it to the world on
various platforms under a friendly license, then it should work just fine. A
proper SPNEGO mechanism should be able to handle the addition of new mechanisms
later without a problem. 

The original intention of this bug was to  make Kerberos authentication possible
. This patch solves that problem in a way that is compatible with MS and uses
open standards that can be extended. I dont think that the lack of NTLM
negotation should influence the adoption of this particular feature.



    
    

    
  
>I dont think that the lack of NTLM negotation should influence the adoption of
>this particular feature.

it won't.


so what version of the patch are we trying for now?  chris: are you going to
polish off attachment 140271 [details] [diff] [review] ("test version") or are we going to go with
attachment 140190 [details] [diff] [review] ("Complete updated patch").


also, i would like to make sure that when this patch lands the Mozilla.org build
systems are setup to enable GSSAPI support on all platforms that support it.  it
looks like OSX (Panther at least) includes the headers and libraries.  and i
think there are redhat packages for linux that i can install (please correct me
if i'm wrong).  what about windows?  am i correct to understand that this code
is not going to work under windows?  do we need a different SSPI based solution
under windows?

    
    

    
  
I'll get the test version' polished off today.  As i'm sure you've seen there is
some disagreement about the defaults.   I can summarize the issues and I'll do
my best to represent both sides in the next message.  Then just let us know what
you are willing to accept, we'll set the defaults to whatever you choose and
submit it.

>am i correct to understand that this code
>is not going to work under windows?  do we need a different SSPI based solution
>under windows?

Yes we probably do need an SSPI solution for Windows.  Most people won't have
MIT kerberos installed on their Windows box (although it would work).

    
    

    
  
An SSPI version of this same code needs to be done.  I've done some coding with
SSPI and GSSAPI interop in the past, its not too difficult.  I would even go so
far as to volunteer to work on this at some point, but I don't currently have an
Win2K server setup to build or test on.  So, initially, the new auth method will
only work on Unix platforms that have the necessary GSSAPI support.

    
    

    
  

      
      
      
      

        Attached patch
          
          
        updated (obsolete)
        — 
          — Splinter Review
      

    


  
My updates now work fine, but there is a problem. After applying the patch
mozilla will still not start in Linux.	I have no idea why this is, I don't
believe the problem to be in my additions (or any of the .cpp files).  

I tested by building mozilla twice, once was a stock Moz 1.6 with no gssapi or
negotiate options, and once with moz 1.6 with all of the required options
gssapi and extention options enabled. The negotiateauth version will not start,
although it compiles fine. The stock mozilla 1.6 built without authnegotiate
built starts fine.   I copied the libnegotiateauth.so file into the stock
mozilla 1.6 build and the auth module even works fine. That's how I did my
testing.

I'm not sure where the problem lies, my guess is that it has something to do
with the configure scripts. Is it possible that since my kerberos 1.3 libs and
headers made it into the default build configs for every piece of mozilla that
the mozilla build processes is somehow getting confused and compiling in the
wrong header or library at some point in the build process? Again there are no
compile errors. In my next post I'll attach the assertions I'm seeing, which
appear whether I've left libnegotiateauth.so file in the build or have moved it
out.

I'm not a configure expert so I'm not sure how to just use the KRB5 lib
directories and include files in the negotiateauth extention directory, but the
would probably be the next logical test.

Build env

MIT kerberos 1.3.1

./gmake --version
GNU Make 3.79.1
 
./gtk-config --version
1.2.10
 
./glib-config --version
1.2.10
 
./libIDL-config --version
0.6.8
 
./m4 --version
1.4.1
 
./automake --version
automake (GNU automake) 1.5
 
./autoconf --version (GNU autoconf)
Autoconf version 2.13
 
gcc (GCC) 3.2.3
   gcc-compile options
   "./configure --prefix=/usr/local/gcc_3.2.x_static/ --disable-shared
--with-pic"

        Attachment #134742 -
        Attachment is obsolete: true

        Attachment #140270 -
        Attachment is obsolete: true

        Attachment #140271 -
        Attachment is obsolete: true

    
    

    
  
I noticed this same problem but had not yet figured out where it was coming from.
I did not see the same problem when I built the identical code with
Solaris x86 and the native solaris GSSAPI headers/libs.  I only noticed
a problem when using MIT code on Linux.  


    
    

    
  
i'll try building and running the patch on a Fedora Linux box w/ version 1.3.1
of the MIT krb5 implementation.  i believe i may have access to a Win2k server
for testing.

    
    

    
  
+NS_IMETHODIMP
+nsHttpGssapiAuth::GetAuthFlags(PRUint32 *flags)
+{
+  //
+  // GSSAPI creds should not be reused across multiple requests.
+  // Only perform the negotiation when it is explicitly requested
+  // by the server.  Thus, do *NOT* use the "REUSABLE_CREDENTIALS"
+  // flag here.
+  //
+  *flags = REQUEST_BASED; 
+  return NS_OK;
+}

this is interesting.  i thought i remember a previous version of this patch
where CONNECTION_BASED was returned.  the fact that this returns REQUEST_BASED
means that NTLM will not work with this authenticator.  NTLM authenticates
individual TCP connections, and with the REQUEST_BASED flag, Necko will not
guarantee that the same connection is used when sending the Type3 NTLM
authorization header as was used when the Type1 NTLM authorization header was sent.

are you sure the Negotiate auth protocol does not require CONNECTION_BASED?  is
this ok for Krb5 authentication?

    
    

    
  
I'm not exactly sure where the flags went, but this is what I think
they should be:

  *flags = REQUEST_BASED | CONNECTION_BASED | IDENTITY_IGNORED; 
  return NS_OK;

I think the above is correct.  I think that when I re-built the patch
I somehow lost track of this piece.

    
    

    
  
Comment #90 indicates when the CONNECTION_BASED and IDENTITY_IGNORED flags were
added.  


    
    

    
  
> *flags = REQUEST_BASED | CONNECTION_BASED | IDENTITY_IGNORED; 

it does not make sense to set both of these flags.  you should set one or the
other but not both.

    
    

    
  
(In reply to comment #149)
> > *flags = REQUEST_BASED | CONNECTION_BASED | IDENTITY_IGNORED; 
> 
> it does not make sense to set both of these flags.  you should set one or the
> other but not both.

it's late ;)  what i meant was that it does not make sense to set both
REQUEST_BASED and CONNECTION_BASED.  pick CONNECTION_BASED if you are
authenticating individual connections and you need to have GenerateCredentials
called more than once for the same connection.  (note: CONNECTION_BASED implies
that your authenticator is not RFC 2617 compliant.)  otherwise, pick REQUEST_BASED.

    
    

    
  
REQUEST_BASED is the way to go then.
Thanks for clarifying, Im not so clear on how these various flags work.




    
    

    
  
ok, then we are accepting that NTLM will not be supported by this Negotiate auth
module.  i just want to make sure we are clear on that point.  it helps, i
think, eliminate some of the concerns expressed previously about a GSSAPI
implementation making use of NTLM.  by setting REQUEST_BASED instead of
CONNECTION_BASED, we are preventing NTLM from working.

    
    

    
  
I think NTLM would work if there were a GSSAPI implementation of it that SPNEGO 
could offer in the negotiation, but since there isn't, then its not really an 
issue.    Even so, I think NTLM is less interesting and definitely less secure
than Kerberos so if NTLM is forced out of the picture, its a Good Thing.

The thing to remember with this scheme is that the authentication mechanisms must 
fit into the GSSAPI framework in order to be able to participate.  NTLM auth
that does not frame its tokens in GSSAPI is not eligible for the negotiation
regardless  of the flags.

I expect that when the Windows/SSPI code is written, that NTLM will be a valid
choice since Windows has an NTLM SSPI module to use as a negotiation option.

    
    

    
  
>I expect that when the Windows/SSPI code is written, that NTLM will be a valid
>choice since Windows has an NTLM SSPI module to use as a negotiation option.

no, it will not be an option if we do not specify CONNECTION_BASED.  the point
is that necko needs to do something special to support NTLM.

here's how NTLM works:

  C: GET /blah ...
  S: 401 ...
     WWW-Authenticate: NTLM

at this point the client typically opens a new connection (but that's not really
important).

  C: GET /blah ...
     Authorization: NTLM (... Type1 messsage ...)
  S: 401 ...
     WWW-Authenticate: NTLM (... Type2 message ...)

now, on the same connection that the client sent the Type1 message (<-- this is
crucial), the client sends a Type3 message:

  C: GET /blah ...
     Authorization: NTLM (... Type3 message ...)
  S: 200 OK ...

since Necko controls opening, reusing, and closing connections, it needs to be
told about this special requirement of NTLM.  so, if the Negotiate auth handler
says it is REQUEST_BASED, then there is no way at all for it to support NTLM.

this whole issue makes me wonder if Negotiate auth doesn't require
CONNECTION_BASED as well.  NOTE: a request based authenticator should be able to
specify CONNECTION_BASED and be ok... also, it may happen that REQUEST_BASED
appears to behave like CONNECTION_BASED except when Necko is heavily loaded down
with requests for a particular host.

    
    

    
  
Microsoft's implementation is defintely connection based, and if multiple round
trips are required - which many negotiations may require, this implementation
will require it as well.

    
    

    
  
I've been convinced that SPNEGO (instead of Kerberos V5) is currently of little
threat for the same reason it is currently of little value, there are almost no
implementations and the ones that do exist only support KerberosV5 which is what
you get if you don't support SPNEGO (minus some overhead). Did anyone with an
RFC compliant SPNEGO implementation ever check to see if it worked against IIS,
or will that be broken?

We will of course need to re-dicuss the threats if we make an SSPI solution.

Even making the assumption that Kerberos V5 will be Negotiated, it still should
be over SSL.  (default should be NegotiateAuth.trustedURIs="https://") Many
sites with a kerberos implementation also have a kerberos shared key with
another site, which means you may be asked to present your token out side of
your intranet, and I don't think that should be able to happen withoug SSL
without user consent.  The token being sent is valuable if it can be stolen
(Details: Only for ~5 minutes, and only a DNS spoof, or a man in the middle type
attack, and it can't be replayed if the server has a replay cache enabled).  The
risk is small I admit, but it is up the the browser to protect the user's
credentials and not autosend them unprotected without the user's input. 

I understand that this is somewhat painful for sites that have choosen to not
protect internal resources using SSL, but the pain will go away if bug 169106 is
fixed and user can choose through a GUI exactly what they trust.

    
    

    
  
I have tested with SPNEGO against IIS successfully.  Of course, I had to hack my
SPNEGO implementation to interoperate with MS because their SPNEGO (e.g.
SSPI-Negotiate) mechanism is not RFC compliant.  However, it does work if
certain bits of the RFC are ignored or completely violated :)

I am OK with allowing the default to be NegotiateAuth.trustedURIs="https://"

Has anyone figured out why mozilla will not start in Linux when the extension
is enabled?  This was working up until some of the more recent patches were
added, but Im still not sure what is causing the problem.





    
    

    
  
i haven't had a chance to test under linux yet... i probably won't have a chance
until thursday :(

    
    

    
  
On an intranet, how many people actually manage to get IIS to talk HTTPS?

My worry is that we have just newtered this patch again.  What we really need is
the security zones - trustedURIs seems such a compleatly wrong way to approach this.

    
    

    
  
(In reply to comment #159)
>  What we really need is the security zones - trustedURIs seems such a
compleatly >  wrong way to approach this.

Well I think everyone probably agree's with you (me included), but security
zones doesn't even have a target milestone. So if you want this feature in the
near future, trustedURI's is the way to get it.

    
    

    
  
i agree.  at least for 1.7 alpha/beta, let's keep our aim simple.  this way
there is some hope that folks can use this patch.

btw, on an unrelated note, i suspect that in the future we will want to hide
GSSAPI behind nsIAuthModule, and then make nsHttpNTLMAuth.cpp support both
Negotiate and NTLM auth modules.  the API for nsIAuthModule may need to be rev'd
to realize this goal, and that can happen after this initial patch lands. 
having a Negotiate auth implemented behind an interface like nsIAuthModule that
has nothing to do with HTTP makes it possible to incorporate this into IMAP,
POP3, etc.

    
    

    
  
But that is what TrustedURIs it - a gui-less, likely-undocumented implementation
of security zones.  And anyway, why are all SSL servers trustworthy?

I hate to be one of the many 'community members', cheering from the sidelines
about how things should be fixed (as a member of the Samba Team, I certainly
know the other end of that pointy stick), but as anything more than a 'half way'
point, this seems really broken.


    
    

    
  
(In reply to comment #162)
>And anyway, why are all SSL servers trustworthy?

They aren't, that is why I've been so adamant that the SPNEGO must negotiate a
protocol as strong or stronger than Kerberos.  The reason Kerberos fits well
here is that if the web site is not registered in your KDC or a KDC which your
site shares a cross site key with, you won't get a ticket for it and so you will
not even try to send it an authenticator or any other sensitive data.
ChallengeRecieved will fail and other auth methods will be tried if available,
all of the other methods HTTP Basic, NTLM, and Digest, all require user input,
the user must now make the decision whether or not to trust the site.

We are not trusting the site if it is protected under SSL, we are trusting that
its DNS name has not been spoofed, and then we rely on the normal Kerberos
mechanisms to determine if this site has been registered in a KDC (which can
include Active Directory) with which the user is affiliated.  Basically the
Kerberos protocol has the notion of which servers you should give your
credentials to built-in, but it can't make the decision of whether you should
use SSL or not, because that depends on how much you trust the networks that the
data is going to have to cross.

    
    

    
  
(In reply to comment #161)
> i agree.  at least for 1.7 alpha/beta, let's keep our aim simple.  this way
> there is some hope that folks can use this patch.


I favor leaving the TrustedURIs list wide open, but will compromise by
restricting it to HTTPS if the feature is documented well enough that 
people know how to remove this restriction to get it to work with 
unencrypted servers.

> 
> btw, on an unrelated note, i suspect that in the future we will want to hide
> GSSAPI behind nsIAuthModule, and then make nsHttpNTLMAuth.cpp support both
> Negotiate and NTLM auth modules.  the API for nsIAuthModule may need to be rev'd
> to realize this goal, and that can happen after this initial patch lands. 
> having a Negotiate auth implemented behind an interface like nsIAuthModule that
> has nothing to do with HTTP makes it possible to incorporate this into IMAP,
> POP3, etc.


I think this is an excellent idea.  I was just talking to some people today who
want to use GSSAPI or SASL/GSSAPI to talk to secure IMAP servers, so moving this
to a more general location is a good idea for the future.



    
    

    
  
I do not want to require SSL as a default.

There will be no Kerberos data sent unless I have trusts so there will be nothing
sent over Internet unless I have set up a trust.

Internaly I suspect most companies do not have SSL due to the complexities
with certificate that requires. And it is internally most will want to use
Kerberos login. Those who want to use Kerberos/SPNEGO over Internet
will enable SSL if they think it is needed. A good documentation with both
info on how to enable Kerberos use with Mozilla as well as description of
what the security risks are, is needed to go with this addition to Mozilla.

    
    

    
  
(In reply to comment #165)
> I do not want to require SSL as a default.
> There will be no Kerberos data sent unless I have trusts so there will be 
nothing
> sent over Internet unless I have set up a trust.
> Internaly I suspect most companies do not have SSL due to the complexities
> with certificate that requires. And it is internally most will want to use
> Kerberos login. Those who want to use Kerberos/SPNEGO over Internet
> will enable SSL if they think it is needed.

In general users will not change a pref to get added security, they will only 
change it to get added functionality.   We are also fighting a loosing battle, 
may users will hear the word kerberos and say why would I need SSL anyway?  You 
can try to make it clear in documentation, most users won't read it.  

> A good documentation with both info on how to enable Kerberos use with 
> Mozilla as well as description of
> what the security risks are, is needed to go with this addition to Mozilla.

That's a good idea.  Keep in mind that RedHat users at least will not have to 
do anything to get SPNEGO working (expect typing kinit and have a valid 
krb5.conf file) as long as they downloaded the binary of Mozilla from 
mozilla.org and their web servers are set up correctly. If they don't read the 
documentation they could be possibly sending SPNEGO tokens on demand inside or 
outside their network without even knowing it.  These users will not know that 
they should have tweaked a pref to get added security, they didn't even know 
the feature existed.

    
    

    
  
I've been giving this alot of thought lately, and I've come to believe that
requiring SSL is not a good idea for now.  At least not until there is some sort
of easy-to-understand interface for users who want to lessen the restriction.
I know I said back in Comment #164 that I would compromise and allow for HTTPS
to be the default,  but I've come to believe that doing so would cripple this
feature for 99% of the public who don't know how or where to go to tweak various
user_pref items that are not presented in one of the GUI preferences windows. 
If they are used to accessing an intranet website with IE and then try to do it
with the new mozilla because they read that it has support, they will be very
disappointed to see that it doesn't work out of the box and will likely not know
how to fix it (even if its documented clearly).

If the site being accessed is deemed important, it probably already requires the
use of SSL anyway, it maybe should not even be a client-side
decision.   I understand the risk of the spoof attack, but I'm not convinced
that that risk outweighs the fact that restricting it by default effectively
makes this feature useless to most people.

Now, if someone wants to make additions to one of the "Privacy & Security" pages
so that the new options are easily visible & modifyable, I would change my
opinion (uh, yet again), but just adding another "secret" user_pref item is not 
terribly helpful.

Finally - I have a new patch that seems to work in Linux (no longer fails to
start the browser after being compiled) and Solaris.  I need to do further
testing, but should post it sometime Monday.

What do we need to do to bring this issue to closure and get to the final code
review steps?  

    
    

    
  
> I know I said back in Comment #164 that I would compromise and allow for HTTPS
> to be the default,  but I've come to believe that doing so would cripple this
> feature for 99% of the public who don't know how or where to go to tweak various
> user_pref items that are not presented in one of the GUI preferences windows. 
> If they are used to accessing an intranet website with IE and then try to do 

Keep in mind to tweak this pref all you have to do is go to about:config and if
you've tried to run the extention previously and failed, the required pref will
  be at the top of the list and in bold.  Double click the pref and you can
re-set it in the gui to anything you want.  I'm sure most people all ready know
this trick, but I wanted to put it down just in case people were judging the
difficulty of setting this pref by assuming a user had to actually hand edit
pref files.  They don't.  Not the gui we want for this, but it's not as bad as
it sounds either.

> with the new mozilla because they read that it has support, they will be very
> disappointed to see that it doesn't work out of the box and will likely not know
> how to fix it (even if its documented clearly).

I believe you underestimate Unix users.  (The only user group that will benefit
from the current patch.)

> If the site being accessed is deemed important, it probably already requires the
> use of SSL anyway, it maybe should not even be a client-side
> decision.

It has to be a client side decision as well, you can't prevent a man in the
middle attack by only requiring SSL at the server.   If the client doesn't know
the site he is going to is under SSL, the man in the middle can just spoof a
non-ssl version of the site read/edit the data from the client and send and
receive the repiles to/from the actual SSL encrypted site.

> Finally - I have a new patch that seems to work in Linux (no longer fails to
> start the browser after being compiled) and Solaris.  I need to do further
> testing, but should post it sometime Monday.

Good work! That was a strange error, I'll be interested in seeing what caused it.

> 
> What do we need to do to bring this issue to closure and get to the final code
> review steps?  

The security decision about defaults is of course up to Darin, but whatever the
decision, we do we need to make sure that the feature can be turned off, by
leaving the pref there but with an empty string.  (The default would only be
used if the pref was not there at all.  Then presumably, it would be
automatically set to the default value for future use.)

    
    

    
  
>> with the new mozilla because they read that it has support, they will be very
>> disappointed to see that it doesn't work out of the box and will likely not know
>> how to fix it (even if its documented clearly).

>I believe you underestimate Unix users.  (The only user group that will benefit
>from the current patch.)

As there is an increased number of ordinary users on Unix, it would help making
people move to the Unix desktop if things work out of the box.
To have mozilla autmatically login using negotiate without changing preferences
by hacking in a file (or using about:config) would be a nice thing to
show the normal end user.


    
    

    
  
perhaps (and i'm not necessarily advocating this solution...) we could solve the
security problem by prompting the user when the site is not SSL.  we could give
the user a checkbox that allows them to remember this site.  that makes it very
easy for the user to build up the whitelist.  i think this would help minimize
the annoyance factor, and in the future we could extend the permissions database
and provide UI (perhaps as part of the security zones UI) to provide a better
solution.  thoughts?

    
    

    
  
(In reply to comment #170)

User's like to Click OK on every box they see, so we would have to be careful 
about the wording, but we might be able to come up with something.

    
    

    
  
FYI, I'm working on revising the patch...

    
    

    
  
ok, here's what i'm thinking:

it appears that the current patch (attachment 140465 [details] [diff] [review]) prevents negotiate auth
whenever a host (or sub-domain of that host) does not appear on the "hostlist".
and, i noticed that the same test is in both ChallengeReceived and
GenerateCredentials.  there's no point to duplicating the test as far as i can
tell.  and, moreover... it seems that GenerateCredentials set GSS_C_DELEG_FLAG
only when a match has been found.  but, GenerateCredentials is only called if a
match was found when ChallengeReceived was called (they are always called in a
pair).  so, i think we should do the following:

always set GSS_C_DELEG_FLAG in GenerateCredentials.  make ChallengeReceived fail
if the following conditions are met:

 (1) channel is not HTTPS
 (2) auth request is not for a proxy server
 (3) permissions database does not have an "ALLOW" entry for the host and the   
     user chooses not to allow host via a prompt

this gives us a good path moving forward b/c we can hope for a generic UI for
all permissions in the permissions database.  also, it means that we don't have
to futz with host & sub-domain matching.

i'm working to revise the patch.  i cleaned up the configure.in code quite a
bit, and i'll be posting a new patch here for testing.  i'd really appreciate
help testing it when it is ready.  thanks!

    
    

    
  

      
      
      
      

        Attached patch
          
          
        Updated 2/9/04 (obsolete)
        — 
          — Splinter Review
      

    


  
Sorry for the duplicate effort, but I updated the patch over the weekend and
spent today testing/debugging it.

        Attachment #140190 -
        Attachment is obsolete: true

    
    

    
  
(In reply to comment #173)
> so, i think we should do the following: 
> always set GSS_C_DELEG_FLAG in GenerateCredentials. 

I don't think it should be set up this way. The two lists were never designed to
be identical.  It was designed so NegotiateAuth.trustedURIs would most likely
contain a whole domain (or just a protocol) and NegotiateAuth.DelegateURIs would
be a very tiny subset, probably just one or two hosts.

Kerberos delegation is triggered by the GSS_C_DELEG_FLAG.  That is the security
equivalent to giving your password to the server for ~8 hours before it becomes
useless. During this time the server can effectively be "you", and do anything
that you can. This is a VERY useful feature at times, but it is not something
you want to give to every server.  In addition, most users won't want to do this
or even know what it is, it increases the size of each request by ~1K, and in
MIT Kerberos it puts a lot of traffic on your KDC (one request and reply from
the KDC before EVERY request). I think that's the reason no one ever said much
about it being hidden away in a pref, at least until a suitable permanent GUI
can be put in place.

So I suggest continuing as you stated, but turning off GSS_C_DELEG_FLAG by
default and keeping the pref to turn it back on a per host basis. Then we can
figure out a better GUI for it later.

    
    

    
  
- I agree with Chris - definitely do NOT set the DELEG flag in all cases,
  its dangerous at worst and will cause alot of failures for sites that
  do not automatically issue forwardable tickets.

- The test in ChallengeReceived is different from the test in
GenerateCredentials.  The one in ChallengeReceived checks to see if the URI 
matches the TrustedURIs list.  The one in GenerateCredentials checks to see if 
its in the "DelegationURIs" list.  Its not a duplicated test.

- I question the value in passing delegated creds to the web server in any case.
Setting the DELEG flag "always" is not going to work.  It only succeeds if the
Kerberos TGT was created with the "Forwardable" flag - which is not always the
default.  If the "F" flag is not set and the GSSAPI client tries to send with
the DELEG flag, the authentication will fail.  I left it in place in my latest
patch because it *might* be useful in rare circumstances, but I am not really in
favor of passing TGTs to a web server.  Unless that server is capable of 
assuming the identity of the user and using the delegated creds for something
more than fetching pages, its not useful at all.

- This auth method should never work for a proxy server, the spec is only for
web server authentication, not proxy auth.

- I left the default for TrustedURIs to be "https://;http://" in my patch. I
know this is still under discussion, but thats my preference at this point and
I had to stick something in there.

Lets try not to veer to far from the current solution at this point, I think its
95% ready, we just need to come up with a solution for the remaining couple of 
items:
   1. Default value for TrustedURIs ?  Include "http://" or not ?
   2. Delegation - is it useful or dangerous in web authentication ?



    
    

    
  
(In reply to comment #176)
 
> - I question the value in passing delegated creds to the web server in any case.
> Setting the DELEG flag "always" is not going to work.  It only succeeds if the
> Kerberos TGT was created with the "Forwardable" flag - which is not always the
> default.  If the "F" flag is not set and the GSSAPI client tries to send with
> the DELEG flag, the authentication will fail. 

In MIT kerberos Authentication will still succeed it just won't send the
delegated credential.

 I left it in place in my latest
> patch because it *might* be useful in rare circumstances, but I am not really in
> favor of passing TGTs to a web server. 

Delegation is one of the main reasons I want this patch. Although, I think it
should be kept (maybe forever) in an obscure pref.

Yes, delegation is risky, but its much less risky than sending your password to
the server. (The only other way to delegate to the server).

Here are a few uses:

IIS server is starting to use these creds for lots of things, especially since
most of the MS infrastructure is getting kerberized.  The web server no longer
has to be trusted to access network resources, instead it only has permissions
to access what the user's temporary kerberos credentials have access to.  (File
shares, MS Exchange, and MS Sql server, other web servers). 

Also CGI's in apache could use these creds to check email, scp files around,
access NFS mounts, access Oracle, execute commands on a remote host (ssh), or
another web server (like a portal would).  And you get all of this while having
an Apache server which has no special priveleges if no users have recently
logged in.  I believe this makes it much more secure than users sending
re-usable passwords to it which are good for a MUCH longer time.

> - This auth method should never work for a proxy server, the spec is only for
> web server authentication, not proxy auth.

In fact, if the connection is not SSL, and IE detects that it is being asked to
send SPNEGO through a proxy server, it won't do it.  IE will not do non-ssl
SPNEGO connections through a proxy server.

    
    

    
  
>Sorry for the duplicate effort, but I updated the patch over the weekend and
>spent today testing/debugging it.   

Ack!  I hope it hasn't changed much...

>I don't think it should be set up this way. The two lists were never designed
>to be identical.

OK, I majorly overlooked the fact that those two functions were looking up
different preferences!!  Sorry for adding noise to this bug :-(

Thanks for explaining the issues concerning delegation.

I still think that using the permissions backend is the right thing to do.  I
think that ensures that we are using the right backend for this.. even well into
the future.  It isn't much more work.  In fact, it is less work.  All we need to
do is define a permission bit for delegation, and then we are set.

    
    

    
  
> - This auth method should never work for a proxy server, the spec is only for
> web server authentication, not proxy auth.


ok, then we need to return a failure code when isProxyAuth is true.  i'll add
that to the patch i'm modifying.

wyllys: i think i understand your recent changes.  your changes appear to
include making the patch apply cleanly, adding the trustedURIs default value,
and some other minor fixup.  can you please let me know if you changed anything
else?  thx!

    
    

    
  
can someone explain why mCtx is part of the "session state" and not the
"continuation state"?  if this were SSPI, i would expect it to be part of the
continuation state, which is a property stored on the connection.  session state
is stored in a global location and is shared by all connections that use
negotiate auth.  since negotiate auth has multiple stages, it seems like this
could cause problems... couldn't there be multiple contexts for different
connections?

    
    

    
  
(In reply to comment #177)
>
> Here are a few uses:
> 
> IIS server is starting to use these creds for lots of things, especially since
> most of the MS infrastructure is getting kerberized.  The web server no longer
> has to be trusted to access network resources, instead it only has permissions
> to access what the user's temporary kerberos credentials have access to.  (File
> shares, MS Exchange, and MS Sql server, other web servers). 

Agreed, IIS is one product that does properly support delegation.

> 
> Also CGI's in apache could use these creds to check email, scp files around,
> access NFS mounts, access Oracle, execute commands on a remote host (ssh), or
> another web server (like a portal would).  And you get all of this while having
> an Apache server which has no special priveleges if no users have recently
> logged in.  I believe this makes it much more secure than users sending
> re-usable passwords to it which are good for a MUCH longer time.

Yes, in theory this is correct.  Though Unix in general lacks alot of the
functionality needed to do proper impersonation.






    
    

    
  
(In reply to comment #179)
> > - This auth method should never work for a proxy server, the spec is only for
> > web server authentication, not proxy auth.
> 
> 
> ok, then we need to return a failure code when isProxyAuth is true.  i'll add
> that to the patch i'm modifying.

Thanks!

> 
> wyllys: i think i understand your recent changes.  your changes appear to
> include making the patch apply cleanly, adding the trustedURIs default value,
> and some other minor fixup.  can you please let me know if you changed anything
> else?  thx!

Thats about it.  

Regarding the mCtx and the session state - I'm not sure if moving it to
continuatoin state is correct or not.  I seem to recall trying this
in one of the early patches and I found it to be incorrect for
some reason.   I will test it locally and see if it makes a difference,
if it works the same I will repost with the change.




    
    

    
  
> I still think that using the permissions backend is the right thing to do.  I
> think that ensures that we are using the right backend for this.. even well into
> the future.  It isn't much more work.  In fact, it is less work.  All we need to
> do is define a permission bit for delegation, and then we are set.

A few things to keep in mind for a future direction as were talking about the
permissions backend. I think most people will want the ablitilty to put a domain
(or subdomain) in the gui, so at some point I think we will want whole domain
matching back. (I think the permission manager already works this way).  They
will might also want the ablity to put a optional protocol in with each entry.

Also, in SSPI you actually want to turn the delegation flag on at all times. 
The reason is even though you always ask for delegation you won't get it unless
the KDC Administrator trusted the service enough to put a special flag on the
service called OK-AS-DELEGATE.  It's hint to clients that the KDC admin believes
that that a particular host actually needs your credential, and that he/she
actually trusts it enough for you to give it to them. It's a pretty good system,
I've even implemented it in MIT kerberos, so if possible, I'd like to take
advantage of it there also.


    
    

    
  
(In reply to comment #183)
> > I still think that using the permissions backend is the right thing to do.  I
> > think that ensures that we are using the right backend for this.. even well into
> > the future.  It isn't much more work.  In fact, it is less work.  All we need to
> > do is define a permission bit for delegation, and then we are set.
> 
> A few things to keep in mind for a future direction as were talking about the
> permissions backend. I think most people will want the ablitilty to put a domain
> (or subdomain) in the gui, so at some point I think we will want whole domain
> matching back. (I think the permission manager already works this way).  They
> will might also want the ablity to put a optional protocol in with each entry.
> 

I agree, domain matching is a desirable feature.  Thats what I do like about the
current patch - it matches URI substrings which gives the user alot of
flexibility in how they configure this feature.

> Also, in SSPI you actually want to turn the delegation flag on at all times. 
> The reason is even though you always ask for delegation you won't get it unless
> the KDC Administrator trusted the service enough to put a special flag on the
> service called OK-AS-DELEGATE.  It's hint to clients that the KDC admin believes
> that that a particular host actually needs your credential, and that he/she
> actually trusts it enough for you to give it to them. It's a pretty good system,
> I've even implemented it in MIT kerberos, so if possible, I'd like to take
> advantage of it there also.
> 

Have you considered donating this back to MIT?  It sounds like a good feature
that they might consider.





    
    

    
  
(In reply to comment #181)
> Yes, in theory this is correct.  Though Unix in general lacks alot of the
> functionality needed to do proper impersonation.

Just curious what you mean here? Isn't it problem of the authentication (or
authorisation) mechanism rather than OS?

    
    

    
  
(In reply to comment #184)
> I agree, domain matching is a desirable feature.  Thats what I do like about the
> current patch - it matches URI substrings which gives the user alot of
> flexibility in how they configure this feature.

The permission manager does that to. You could add youcorp.com to it, and it
will accept something.yourcorp.com too. It doesn't work the other way, so
yourcorp.com.evil.com doesn't get accepted.
So to prevent code duplication, i think it is a good idea to just use the perm
manager.
Maybe it needs to be extended to accept protocols though, but that is fixable.

    
    

    
  
I'm not able to build the current patch with Heimdal (the configure script seemd
to fail to detect correctly Heimdal's enviroment) and the build fails. Before
starting looking into this, I'd like to ask what's the purpose for having both
the configure options --with-gssapi=DIR and --enable-gssapi. Isn't the latter
useless (I suppose when I specify I want to build the negotiateauth extension
the GSSAPI libraries are available either at default place or where specified by
--with-gssapi=DIR). What's --enable-gssapi good for than?

    
    

    
  
>Maybe it needs to be extended to accept protocols though, but that is fixable. 

yeah, this is the only real issue with the permission manager at the moment.  i
think we can live with that for the first installment of this patch.

using the permission manager without UI means that instead of hand modifying
prefs.js files, users will need to hand modify their hostperm.1 file.  of
course, hand modifying these files is really undesirable.  what would be ideal
is for us to create some UI for this.  the UI could even start out as a mozdev
extension.

also, the permission manager is capable of storing the delegation flag per host
as well.  in fact, nsIPermissionManager allows a permission value to be any
number between 1 and 15, where 0, 1, and 2 have predefined meanings:
UNKNOWN_ACTION, ALLOW_ACTION, and DENY_ACTION respectively.  in my patch, i have
used the 3rd bit to indicate the delegation flag.  in other words, a value of 5
stored in the permissions database means "allow action" and "allow delegation".

> I'm not able to build the current patch with Heimdal..

Daniel, in my version of the patch i have eliminated --enable-gssapi.  instead,
if negotiateauth is specified as an extension (and i have made it a default
extension), then configure.in will try to look for gssapi.  if you specify
--with-gssapi, then it only configures GSSAPI_DIR.  i'll be posting a revised
patch shortly...

    
    

    
  
(In reply to comment #187)
> I'm not able to build the current patch with Heimdal (the configure script seemd
> to fail to detect correctly Heimdal's enviroment) and the build fails. Before
> starting looking into this, I'd like to ask what's the purpose for having both
> the configure options --with-gssapi=DIR and --enable-gssapi. Isn't the latter
> useless (I suppose when I specify I want to build the negotiateauth extension
> the GSSAPI libraries are available either at default place or where specified by
> --with-gssapi=DIR). What's --enable-gssapi good for than?

--with-gssapi was meant for sites with the GSSAPI stuff installed in 
a non-standard location.  Obviously, "with-gssapi" implies "enable-gssapi", so
both are not necessary.   I envisioned "--enable-gssapi" to be sufficient for
most sites since the configure script was supposed to be smart enough to figure
out where to find the libraries.  I did testing with Heimdal a while ago, but
not lately, so I'm not surprized there is some breakage.





    
    

    
  

      
      
      
      

        Attached patch
          
          
        v2 patch (obsolete)
        — 
          — Splinter Review
      

    


  
here's an updated patch that leverages the permission manager as we've been
discussing.  it adds a dialog with a checkbox allowing users to suppress the
dialog in the future.  there is of course no UI (yet) to manage the remembered
settings.

        Attachment #40197 -
        Attachment is obsolete: true

        Attachment #122480 -
        Attachment is obsolete: true

        Attachment #134646 -
        Attachment is obsolete: true

        Attachment #140465 -
        Attachment is obsolete: true

        Attachment #141010 -
        Attachment is obsolete: true

    
    

    
  
Daniel, Wyllys, Chris: can you guys please take a look at this patch, try it
out, and make sure it works (and is reasonable to each of you).  thanks!

mvl: can you please review the way i'm using the permission manager?  thanks!

    
    

    
  
I like the permission manager use. It worked fine in Solaris.

The configure.in script needs a small fix for the Solaris "-lgss" case:
Add "-L" to the GSSAPI_LIBS definition.
...
    if test -z "$GSSAPI_LIBS" ; then
        AC_CHECK_LIB(gss, gss_init_sec_context,
            [GSSAPI_LIBS="-L$GSSAPI_DIR/lib -lgss"],)
                         ^^^^
    fi
...

Also, I am posting a new nsHttpGssapiAuth.cpp file that uses
a ContinuationState instead of SessionState to hold the gss context 
record.  I think this is more correct and it works fine in my testing.


    
    

    
  


  
Minor fix to Darin's most recent patch, this one uses continuationState instead
of sessionState

    
    

    
  


  
As before, use continuationState instead of sessionState.
Previous attachment had a bug.

        Attachment #141074 -
        Attachment is obsolete: true

    
    

    
  


  
thx wyllys!

    
  

        Attachment #141066 -
        Attachment is obsolete: true

        Attachment #141075 -
        Attachment is obsolete: true

    
    

    
  
If you don't check the box to remember your decision, it will prompt you both 
in ChallengeRecieved and in GenerateCredentials.   You should never prompt a 
user in generatecreentials because it is called with EVERY request.

For some reason it always DELEGATES. This the following statement is always 
true. (I don't yet understand why.)

if (perm & NEGOTIATE_PERM_DELEGATE) {
+        LOG(("  using GSS_C_DELEG_FLAG\n"));
+        req_flags |= GSS_C_DELEG_FLAG;
+    }

The message you put in the popup is somewhat misleading. The webserver that you 
are trying to access will almost always be a webserver you know and trust, the 
issue is that its not over SSL.

I'll have more time tomorrow to look at this.

    
    

    
  
Also if you click no and save the response, it will also never prompt you for 
SPNEGO again not even over SSL.

    
    

    
  
chris: thanks for catching those problems... i can solve the double prompting
problem easily, but the other problem is more difficult.  unfortunately, the
permission manager doesn't allow us to distinguish different protocols on the
same host.  i have some ideas about how to solve that problem...

    
    

    
  
(In reply to comment #185)
> (In reply to comment #181)
> > Yes, in theory this is correct.  Though Unix in general lacks alot of the
> > functionality needed to do proper impersonation.
> 
> Just curious what you mean here? Isn't it problem of the authentication (or
> authorisation) mechanism rather than OS?

The question is what to do with the delegated credentials once the server has
accepted them.  In an Active Directory environment, a users credentials
(Tickets) contain lots of extra information (the infamous PAC data) that the
server can use to further impersonate the authenticated user and take actions on
their behalf in separate threads.  A unix process can crudely assume a user's
identity by calling "seteuid(newuid)", performing some actions, and switching
back to its original (most likely 'root') user identity - OR it can fork
a new process under the uid of the authenticated user and perform actions
as if it were that user, using the delegated credentials. 

My original point was that Active Directory has a much better system for
handling user impersonation and delegation of privileges and credentials.
In Unix its alot easier to abuse the delegated credentials, intentionally or by
poorly designed services that are using them.

    
    

    
  
I succeeded in building the patch with Heimdal (I'll send modifications to
discuss shortly). When accessing an Negotiate-secured page page I've noticed
annoying behaviour when the server is contacted two times when I push the reload
button. Sniffing the communication I recognized that the first http GET request
doesn't contain the Authorization: Negotiate XXX header, the server replies with
WWW-Authenticace Negotiate sent in an 401 reply, and only this message is
replied by mozilla with correct Authorization: Negotiate header. Excerpt from my
log (both records generated by a single clicking the Reload button):

147.251.3.54 - - [12/Feb/2004:11:46:45 +0100] "GET /secure-gss/ HTTP/1.0" 401 506
147.251.3.54 - kouril@ICS.MUNI.CZ [12/Feb/2004:11:46:45 +0100] "GET /secure-gss/
HTTP/1.0" 304 0

Is it possible that the browser remembers the method required from previous
communications without these additional messages? I guess the problem is caused
by the REUSABLE_CREDENTIALS flag not being set for Negotiate, but don't know
what's the best way to correct (since the credentials shouldn't be reused,
certainly).

    
    

    
  
> I succeeded in building the patch with Heimdal (I'll send modifications to
> discuss shortly).

these are only typos corrections:
--- configure.in        Thu Feb 12 13:19:27 2004
+++ configure.in.dan    Thu Feb 12 13:08:07 2004
@@ -3764,6 +3764,7 @@
         LIBS="$LIBS -lgssapi_krb5 -lkrb5 -lk5crypto -lcom_err"
         AC_CHECK_LIB(gssapi_krb5, gss_init_sec_context,
             [GSSAPI_LIBS="-L$GSSAPI_DIR/lib -lgssapi_krb5 -lkrb5 -lk5crypto
-lcom_err"],)
+        LIBS="$_SAVE_LIBS"
     fi
     dnl
     dnl If GSS libs were STILL not found, try looking for Heimdal
@@ -3772,12 +3773,12 @@
     if test -z "$GSSAPI_LIBS" ; then
         if test -x "$GSSAPI_DIR/bin/krb5-config" ; then
             krb5cfg="$GSSAPI_DIR/bin/krb5-config"
-            TMP_GSSAPI_LIBS=`$krbcfg --libs gssapi 2>/dev/null`
-            TMP_GSSAPI_INCLUDES=`$krbcfg --cflags gssapi 2>/dev/null`
+            TMP_GSSAPI_LIBS=`$krb5cfg --libs gssapi 2>/dev/null`
+            TMP_GSSAPI_INCLUDES=`$krb5cfg --cflags gssapi 2>/dev/null`
 
             LIBS="$LIBS $TMP_GSSAPI_LIBS"
             AC_CHECK_LIB(gssapi, gss_init_sec_context,
-                [GSSAPI_LIBS="$TMP_GSSAPI_LIBS",
+                [GSSAPI_LIBS="$TMP_GSSAPI_LIBS"
                  GSSAPI_INCLUDES="$TMP_GSSAPI_INCLUDES"],
                 [GSSAPI_LIBS=""])
         fi

    
    

    
  
(In reply to comment #201)
> > I succeeded in building the patch with Heimdal (I'll send modifications to
> > discuss shortly).
> 
> these are only typos corrections:
> --- configure.in        Thu Feb 12 13:19:27 2004
> +++ configure.in.dan    Thu Feb 12 13:08:07 2004
> @@ -3764,6 +3764,7 @@
>          LIBS="$LIBS -lgssapi_krb5 -lkrb5 -lk5crypto -lcom_err"
>          AC_CHECK_LIB(gssapi_krb5, gss_init_sec_context,
>              [GSSAPI_LIBS="-L$GSSAPI_DIR/lib -lgssapi_krb5 -lkrb5 -lk5crypto
> -lcom_err"],)
> +        LIBS="$_SAVE_LIBS"
>      fi
>      dnl
>      dnl If GSS libs were STILL not found, try looking for Heimdal
> @@ -3772,12 +3773,12 @@
>      if test -z "$GSSAPI_LIBS" ; then
>          if test -x "$GSSAPI_DIR/bin/krb5-config" ; then
>              krb5cfg="$GSSAPI_DIR/bin/krb5-config"
> -            TMP_GSSAPI_LIBS=`$krbcfg --libs gssapi 2>/dev/null`
> -            TMP_GSSAPI_INCLUDES=`$krbcfg --cflags gssapi 2>/dev/null`
> +            TMP_GSSAPI_LIBS=`$krb5cfg --libs gssapi 2>/dev/null`
> +            TMP_GSSAPI_INCLUDES=`$krb5cfg --cflags gssapi 2>/dev/null`

Good catch - I was close :)

>  
>              LIBS="$LIBS $TMP_GSSAPI_LIBS"
>              AC_CHECK_LIB(gssapi, gss_init_sec_context,
> -                [GSSAPI_LIBS="$TMP_GSSAPI_LIBS",
> +                [GSSAPI_LIBS="$TMP_GSSAPI_LIBS"

Also, good catch.

>                   GSSAPI_INCLUDES="$TMP_GSSAPI_INCLUDES"],
>                  [GSSAPI_LIBS=""])
>          fi



    
    

    
  
(In reply to comment #201)
> these are only typos corrections:

and here's something more questionable:

--- configure.in        Thu Feb 12 13:19:27 2004
+++ configure.in.dan    Thu Feb 12 13:08:07 2004
@@ -3795,8 +3796,9 @@
         dnl Try to determine if the GSS_C_NT_HOSTBASED_SERVICE value is
         dnl defined.  Older MIT releases did not define this correctly.
         dnl
+        GSSAPI_INCLUDES="$GSSAPI_INCLUDES $GSSAPI_INCLUDES/gssapi"
         AC_CHECK_HEADERS([gssapi/gssapi_generic.h])
-        AC_TRY_COMPILE([ #include <gssapi/gssapi.h> ],
+        AC_TRY_COMPILE([ #include <gssapi.h> ],
             [ gss_OID oid = GSS_C_NT_HOSTBASED_SERVICE; ],
             [AC_DEFINE(HAVE_GSS_C_NT_HOSTBASED_SERVICE)],[])
     fi
--- extensions/negotiateauth/nsHttpGssapiAuth.cpp.dan   Thu Feb 12 13:08:07 2004
+++ extensions/nnn/nsHttpGssapiAuth.cpp Thu Feb 12 13:19:27 2004
@@ -54,7 +54,7 @@
 #endif

 #include <stdlib.h>
-#include <gssapi.h>
+#include <gssapi/gssapi.h>
 #ifdef HAVE_GSSAPI_GSSAPI_GENERIC_H
 #include <gssapi/gssapi_generic.h>
 #endif


I suggest to include just gssapi.h without any directory specified. This is much
more portable since e.g. Heimdal doesn't store the gssapi.h file in a separate
subdirectory. Similarly for other GSSAPI implementations (at least GSI according
to my knowledge)

    
    

    
  
Thanks for the feedback guys...
Status: NEW → ASSIGNED
Priority: P3 → P2
Target Milestone: mozilla1.7alpha → mozilla1.7beta

    
    

    
  
Darin - Do you need any more code updates or information from anyone to get this
into 1.7beta?  Its been quiet here for a while and I wanted to follow up to see
what the status is.



    
    

    
  
Wyllys: i should be posting a revised patch early this week.  (i've been busy
with other things.)

    
    

    
  
I noticed a small bug in the most recent patch posted by Darin.
In configure.in

+    dnl
+    dnl If GSSAPI libs were not found, remove extension from the list to be
+    dnl built.
+    dnl
+    if test -z "$GSSAPI_LIBS" ; then
+        AC_MSG_WARN([Cannot build negotiateauth without GSSAPI. Removing
negotatiate from MOZ_EXTENSIONS.])
+        MOZ_EXTENSIONS=`echo $MOZ_EXTENSIONS | sed -e 's|xmlterm||'`

                                                          ^^^^^^^^

That should probably be "negotiateauth" instead of "xmlterm", eh?


    
    

    
  
> That should probably be "negotiateauth" instead of "xmlterm", eh?

yes, thank you for pointing that out.  i will correct that before posting a new
patch.


    
    

    
  
To summarize and focus on the current concern...

So, the steps are basically:

* client sends a get
* server sends auth required
* client asks some library for a ticket.
* library does some stuff and response with a blob that the client sends back to
server.
* server verifies the user based on the received blob.

These step are perfectly valid and if implemented correctly are secure.  There
are security problems with the next step.  Since this implementation isn't going
to encrypt the traffic between the server and client with the session key and we
are considering allowing keberos over http (non-ssl) connections, 
man-in-the-middle attacks is possible.

This isn't that big of a deal since the end goal, i presume, is to use this
system to replace basic auth with is susceptible to both man-in-the-middle AND
replay attacks.

The concern here is that with Basic Auth, you will get a password dialog at
least once per session and with the kerberos local client we aren't sure we will
get any prompt.  If there isn't a prompt at some point during the session, an
attacker can cause a page load and act as the man-in-the-middle without ever
having the user aware of this.  

Does this summarize the concern properly?  

    
    

    
  
> Does this summarize the concern properly?  
Yes, good summary.

    
    

    
  
well, i would have thought it would be the responiblity of the kerberos library
to ensure that access to single signon data is restricted.  Any GSSAPI experts
care to comment?

In any case, we could just display a dialog similar to those which are displayed
during https loads (such as mixed content, page transition from secure to
insecure and visa versa).  But, I am not sure that these dialog protect the user
any better than not having them (assumption is that most users just blindly
click OK).

    
    

    
  
(In reply to comment #211)
> well, i would have thought it would be the responiblity of the kerberos library
> to ensure that access to single signon data is restricted.  Any GSSAPI experts
> care to comment?

What do you mean by "restricted" ?   A user and service credentials (tickets)
are protected
with standard file permissions so that others may not read (or write) your tickets.

MITM attacks are possible for all HTTP authentication methods that are not
bound by SSL.   This is an inherent weakness in the HTTP protocol.  GSSAPI/Kerberos
at least removes the Replay Attack scenario (outside of the typical 5 minute
time sync window).   Mutual authentication is also possible with
GSSAPI/Kerberos, so 
the client may verify that it is talking to the "real" server.

> 
> In any case, we could just display a dialog similar to those which are displayed
> during https loads (such as mixed content, page transition from secure to
> insecure and visa versa).  But, I am not sure that these dialog protect the user
> any better than not having them (assumption is that most users just blindly
> click OK).

I see little benefit to adding another dialog popup.  




    
    

    
  
At this point, I'm leaning toward ripping out the nsIPermissionManager code and
the prompting I added in the previous patch.  I think a preference based
solution as cneberg originally implemented may be best.  It will be easier for
IS departments and end-users to configure.  The nsIPermissionManager based
solution is not ideal since the settings are not easily edited by end-users or
IS departments.  I'm also very concerned about the fast approaching 1.7 beta
deadline, and I want to get something into the tree.

The big question is: what should the default value of "trustedURIs" be set to? 
We've debated back and forth on this issue.  I think it comes down to
expectations... i.e., do we need to solve the MITM exploit at this level?

My sense is that we should start out with a more restrictive solution.  We start
out with only allowing kerberos auth over HTTPS, and then we document this
feature in the release notes.

Sound like a plan?

    
    

    
  
> "to ensure that access to single signon data is restricted."

I meant basically, shouldn't the Kerberos local client worry about this
prompting the user for creditials?  

Darin, restricting this authenication scheme to https is a good idea -- even
better with a preference based whitelist of hosts that an IS person can add to.
 With that, I do not think you need to have any kind of dialog.

    
    

    
  
(In reply to comment #213)
> At this point, I'm leaning toward ripping out the nsIPermissionManager code and
> the prompting I added in the previous patch.  I think a preference based
> solution as cneberg originally implemented may be best.  It will be easier for
> IS departments and end-users to configure.  The nsIPermissionManager based
> solution is not ideal since the settings are not easily edited by end-users or
> IS departments.  I'm also very concerned about the fast approaching 1.7 beta
> deadline, and I want to get something into the tree.

YES.  ME TOO!

> 
> The big question is: what should the default value of "trustedURIs" be set to? 
> We've debated back and forth on this issue.  I think it comes down to
> expectations... i.e., do we need to solve the MITM exploit at this level?
> 
> My sense is that we should start out with a more restrictive solution.  We start
> out with only allowing kerberos auth over HTTPS, and then we document this
> feature in the release notes.
> 
> Sound like a plan?

Thats fine with me, but I would like it to be clearly documented so that
people will know how to enable the feature for non-SSL sites and that
they will know that the support actually exists.

Maybe for the next release, the security panel could be modified
to show the options for this feature.  Perhaps an "Authentication"
panel in the "security & privacy" section of the configuration dialog
where various authentication related options could be placed.





    
    

    
  
(In reply to comment #215)
> Maybe for the next release, the security panel could be modified
> to show the options for this feature.  Perhaps an "Authentication"
> panel in the "security & privacy" section of the configuration dialog
> where various authentication related options could be placed.

getting UI changes into 1.7 for this feature is going to be difficult.  i think
we may end up resorting to release notes + about:config as the configuration
solution for 1.7 :-(

    
    

    
  

      
      
      
      

        Attached patch
          
          
        v3 patch (obsolete)
        — 
          — Splinter Review
      

    


  
ok, here's the revised patch.  it includes the autoconf changes plus changes
for the recent talk about prefs, etc.

folks, please look this over and let me know if you see something out of wack.

i don't have a kerberos setup, so i have not been able to fully test this
patch.	however, i have tested the whitelisting pref feature, and it appears to
work correctly.

        Attachment #141086 -
        Attachment is obsolete: true

    
  

        Attachment #143338 -
        Flags: superreview?(bryner)

        Attachment #143338 -
        Flags: review?(dougt)

    
    

    
  
setting blocking 1.7b flag to get this feature on drivers' radar.
Flags: blocking1.7b?

    
    

    
  
There are a couple of bugs in the configure.in script dealing with the location
of the gssapi 
header files.    GSSAPI headers are almost always located in a "gssapi" subdirectory
such as "/usr/kerberos/include/gssapi" or "/usr/include/gssapi".

Startng at line 5872:
+    dnl
+    dnl Try to set GSSAPI_INCLUDES ...
+    dnl
+    AC_CHECK_HEADERS(gssapi.h,
+        [GSSAPI_INCLUDES="-I$GSSAPI_DIR/include"])
+    if test -z "$GSSAPI_INCLUDES" ; then
+        AC_CHECK_HEADERS(gssapi/gssapi.h,
+            [GSSAPI_INCLUDES="-I$GSSAPI_DIR/include"])
+    fi

Later, at line 5782:
+        AC_CHECK_HEADERS([gssapi/gssapi_generic.h])
+        AC_TRY_COMPILE([ #include <gssapi/gssapi.h> ],
+            [ gss_OID oid = GSS_C_NT_HOSTBASED_SERVICE; ],
+            [AC_DEFINE(HAVE_GSS_C_NT_HOSTBASED_SERVICE)],[
+             AC_MSG_WARN([GSS_C_NT_HOSTBASED_SERVICE not found])
+            ])

Use AC_MSG_WARN instead of AC_MSG_ERROR or else the config will
fail on systems that don't have GSS_C_NT_HOSTBASED_SERVICE, which is
not the intention of that test.



    
    

    
  
thanks for the feedback wyllys.  i'll fix up those autoconf issues shortly.

    
    

    
  
nelson,  can you also review...  we are looking to get many eyes on this since
time is short to get reviewed and integrated for Mozilla 1.7beta

    
    

    
  
(In reply to comment #219)
> There are a couple of bugs in the configure.in script dealing with the location
> of the gssapi 
> header files.    GSSAPI headers are almost always located in a "gssapi"
subdirectory
> such as "/usr/kerberos/include/gssapi" or "/usr/include/gssapi".

Not always. Heimdal (and e.g. Globus as an example of non-krb5 GSSAPI
implementation) store the header (they only export gssapi.h) in the include/
subdirectory. I tested the v3 patch with Heimdal and it builds like a charm,
including the configuration stage. So please keep this scenario in mind when
doing any changes to the configure script.

    
    

    
  
I can still see the problem described in comment #200. Mozilla isn't able to
remember that the Negotiate method is required to access previously visited
pages (or pages which are lower in the directory hieararchy). If I have e.g. a
page containing images (stored in the same directory as the page), mozilla first
authenticates to receive this pahe and then tries to get all the images without
the Negotiate header, and only after the server replies with 401, it sends
correct headers. Similarly when I push the reload button, mozilla first try
accessing without the Authorization: header beining set.

Is it possible to make mozilla work similarly as in e.g. case of Basic
authentication, when mozilla remembers that a particular URL (and any other
(lower) URLs starting with this URL) requires authentication and sends the
username/password pair automaticaly?

    
    

    
  
Provided this patch gets approved and commited to the main trunk, are you
planning to ship it as part of standard binary Mozilla distributions? If so,
will the GSSAPI libraries staticaly linked to the binary, or dynamicaly leaving
the possibility for the users to choose their local installation?

    
    

    
  
(In reply to comment #222)
> (In reply to comment #219)
> > There are a couple of bugs in the configure.in script dealing with the location
> > of the gssapi 
> > header files.    GSSAPI headers are almost always located in a "gssapi"
> subdirectory
> > such as "/usr/kerberos/include/gssapi" or "/usr/include/gssapi".
> 
> Not always. Heimdal (and e.g. Globus as an example of non-krb5 GSSAPI
> implementation) store the header (they only export gssapi.h) in the include/
> subdirectory. I tested the v3 patch with Heimdal and it builds like a charm,
> including the configuration stage. So please keep this scenario in mind when
> doing any changes to the configure script.

The configure script  uses 'krb5-config --cflags gssapi' to determine the
correct paths when using Heimdal, so this should not be a problem as long
as that script works as expected.




    
    

    
  
(In reply to comment #224)
> Provided this patch gets approved and commited to the main trunk, are you
> planning to ship it as part of standard binary Mozilla distributions? If so,
> will the GSSAPI libraries staticaly linked to the binary, or dynamicaly leaving
> the possibility for the users to choose their local installation?

It should definitely be dynamic, IMO.  static libs are BAD.

    
    

    
  
(In reply to comment #226)
> The configure script  uses 'krb5-config --cflags gssapi' to determine the
> correct paths when using Heimdal, so this should not be a problem as long
> as that script works as expected.

Some installation doesn't have the krb5-config script installed but still have
correct GSSAPI development enviroment available (e.g. Heimdal in OpenBSD).
Moreover another (non-krb5) GSSAPI installations don't have such a script at
all.  In these cases the configure script should still be able to detect the
headers/libraries.

    
    

    
  
(In reply to comment #226)
> It should definitely be dynamic, IMO.  static libs are BAD.

I agree, if it doesn't conflict with some Mozilla installation/deployment
requirements. 


    
    

    
  
> Some installation doesn't have the krb5-config script installed but still have
> correct GSSAPI development enviroment available (e.g. Heimdal in OpenBSD).
> Moreover another (non-krb5) GSSAPI installations don't have such a script at
> all.  In these cases the configure script should still be able to detect the
> headers/libraries.


Regardless, I think the current configure script and code should support the 
gssapi.h file being in either */include or */include/gssapi/

From configure.in:
.....
    dnl
    dnl Try to set GSSAPI_INCLUDES ...
    dnl
    AC_CHECK_HEADERS(gssapi.h,
        [GSSAPI_INCLUDES="-I$GSSAPI_DIR/include"])
    if test -z "$GSSAPI_INCLUDES" ; then
        AC_CHECK_HEADERS(gssapi/gssapi.h,
            [GSSAPI_INCLUDES="-I$GSSAPI_DIR/include"])
    fi
    CPPFLAGS="$_SAVE_CPPFLAGS $GSSAPI_INCLUDES"
....

The "AC_CHECK_HEADERS" tests set the following definitions:
HAVE_GSSAPI_H or HAVE_GSSAPI_GSSAPI_H

And from ntHttpGssapiAuth.cpp:
#if defined(HAVE_GSSAPI_H)
#include <gssapi.h>
#elif defined(HAVE_GSSAPI_GSSAPI_H)
#include <gssapi/gssapi.h>
#endif

So, it appears that the alternate locations for gssapi.h are handled.





    
    

    
  
(In reply to comment #229)
> Regardless, I think the current configure script and code should support the 
> gssapi.h file being in either */include or */include/gssapi/

I agree

> So, it appears that the alternate locations for gssapi.h are handled.

Yes, it does. Reacting to your message saying that most of the installation use
the gssapi subdirectory, I just wanted to note that there are another
installations not doing so, and that behaviour of the configure should be retained.


    
    

    
  
the current patch builds libnegotiateauth.so, which will be loaded dynamically
by mozilla when it encounters a page that requires "Negotiate" authentication. 
if dlopen fails, then the negotiateauth feature will simply be ignored by
mozilla.  it would obviously fail to load if the necessary gssapi libraries are
not present on the end user's system.

this same build configuration will still be used when the main mozilla code is
compiled and linked into a single executable (the so called static build of
mozilla, which firefox uses).

as for getting mozilla builds to ship with this feature, if we're successful
getting the patch in for 1.7, then i'll work on making sure that the official
linux builds include this component.  we might enable it for OSX as well.  all
other unix builds are community contributed, so we'd need to talk to those folks
to see about enabling this in their builds.

    
    

    
  
Comment on attachment 143338 [details] [diff] [review]
v3 patch

>+void
>+nsGssapiContinuationState::Reset()
>+{
>+    if (mCtx != GSS_C_NO_CONTEXT)
>+    {

Be consistent about brace style.


>+    if (len > kNegotiateLen) {
>+        challenge += kNegotiateLen;
>+        while (*challenge == ' ') challenge++;
>+        len = strlen(challenge);
>+
>+        input_token.length = (len * 3)/4;

I assume this does not need to be null terminated.

>+    char *hostList;
>+    if (NS_FAILED(prefs->GetCharPref(pref, &hostList)) || !hostList)
>+        return PR_FALSE;
>+

I think you're leaking hostList.

>+    // pseudo-BNF
>+    // ----------
>+    //
>+    // url-list       base-url ( base-url "," LWS )*
>+    // base-url       ( scheme-part | host-part | scheme-part host-part )
>+    // scheme-part    scheme "://"
>+    // host-part      host [":" port]
>+    //
>+    // for example:
>+    //   "https://, http://office.foo.com"
>+    //
>+
>+    char *start = hostList, *end;
>+    for (;;) {
>+        // skip past any whitespace
>+        while (*start == ' ' || *start == '\t')
>+            ++start;
>+        end = strchr(start, ',');
>+        if (!end)
>+            end = start + strlen(start);
>+        if (start == end)
>+            break;
>+        if (MatchesBaseURI(scheme, host, port, Substring(start, end)))

Is there extra overhead from constructing a Substring object here? I notice
it's manipulated in MatchesBaseURI mostly with standard libc string functions. 
Would it be faster to just use strncmp() in MatchesBaseURI, or is Substring()
really close to no overhead?

>+    // if we didn't parser out a host, then assume we got a match.

"parse"

sr=bryner with those points addressed.

        Attachment #143338 -
        Flags: superreview?(bryner) → superreview+

    
    

    
  
thanks bryner!  i also removed MODULE_NAME from Makefile.in since we want this
component to remain dynamic even in static mozilla builds.

    
  

        Attachment #143338 -
        Flags: review?(dougt) → review+

    
    

    
  
Comment on attachment 143338 [details] [diff] [review]
v3 patch

requesting drivers approval for 1.7b

        Attachment #143338 -
        Flags: approval1.7b?

    
    

    
  

      
      
      
      

        Attached patch
          
          
        v3.1 patch
        — 
          — Splinter Review
      

    


  
here's the revised patch.  i've tested this on rh9 and fc1.  in both cases the
configure fu correctly detects the kerberos headers and libs.  this should be
sufficient to enable us to build linux and osx nighties with kerberos support
included.  if we need to make further autoconf changes for other platforms, we
can do so as a follow-up patch.

        Attachment #143338 -
        Attachment is obsolete: true

    
    

    
  
Comment on attachment 143338 [details] [diff] [review]
v3 patch

a=chofmann for 1.7b

        Attachment #143338 -
        Flags: approval1.7b? → approval1.7b+

    
    

    
  
v3.1 patch checked in for Mozilla 1.7 beta.  Now to ensure that the build
machines have GSSAPI installed :-)

this bug remains open for the SSPI work.

    
    

    
  
chofmann suggested that i open a new bug for the WIN32 implementation.  he makes
the point that we want to be able to have a closed bug for bug tracking and
testing purposes.

so, marking FIXED.

see bug 237586 for the SSPI implementation for Windows.
Blocks: 237586
Status: ASSIGNED → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Summary: I want Kerberos authentication and TGT forwarding → I want Kerberos authentication and TGT forwarding [using GSSAPI]

    
  
Flags: blocking1.7b?

    
    

    
  
While it's very nice to see Mozilla now supports Kerberos authentication, I'm having a tough time 
finding documentation on how to use it. What Apache module(s) are supported? What about IIS? If 
documentation is available, where is it available? If no documentation has been written would you like 
some help writing it?

    
    

    
  
By default, mozilla will only respond to requests for "Negotiate" authentication
over an https connection.  However, this can be overridden by setting the
following configuration parameters (see the "about:config" page to quickly find
and set these):

network.negotiate-auth.delegation-uris   "http://,https://"
network.negotiate-auth.trusted-uris:     "http://,https://"

It is highly recommended that sites that use Negotiate authentication
also protect those connections with TLS (SSL) and do proper certificate
validation on the TLS session.

These options can obviously be made alot more specific.  For example,
if you only want to trust specific sites, add their URLs in a comma
separated list.

ex:  http://foo.bar.com,https://foo.bar.com

There are several apache modules out there that seem to work OK.
Daniel Kouril (the original author of this extension) has one
at: http://modauthkerb.sourceforge.net that will work.

I am also working on a simple module that just supports the
Negotiate method and removes the dependencies on the Kerberos API.

However, until one of these modules gets accepted into Apache
"officially", results may vary depending on the quality of the
implemented module.

Microsoft IIS also has this support already, enable "Integrated Windows
Authentication" on the directories or pages and make sure your
client system is set up to use the Active Directory KDC and it should
work.


    
    

    
  
nalin@redhat.com says that we're still requesting mutual auth and not dealing
with bad data that is sent back from the server.  He recommends that we either

1. Not request mutual auth.

or

2. Verify the identity of the server if we're requesting it.

Yes, this is hashed out above in the bug but it's hard to follow the thread.  Is
there a valid reason that we're requesting it and not dealing with it?  Nalin
does have a patch that does most of the hard work in verifying it, minus the UI
required to inform the user that the server is Evil.

    
    

    
  
(In reply to comment #241)
>Is there a valid reason that we're requesting it and not dealing with it?  Nalin
> does have a patch that does most of the hard work in verifying it, minus the UI
> required to inform the user that the server is Evil.

The problem was addressed in an very early version of this fix, but it involved
making changes to netwerk/protocol/http/src/nsHttpChannel.cpp to handle the case
where you get a "200" response which also contains WWW-Authenticate header that
has the final GSSAPI token data needed to complete the mutual-auth.
(Take a look at the patch submitted 2003-10-30, it has the changes in there
for handling this situation.)  At the time, there were objection from some on
the mozilla.org team (I think) to making this change because it was considered
"non-standard" and there was a strong desire to keep the extension code
completely isolated in the extension module area.  I thought we had agreed to
disable MUTUAL_AUTH until a later time when the mutual auth fix could be addressed.

If it is now considered acceptable to modify the nsHttpChannel code to handle
this strange case, then I am in favor of adding proper support for Mutual
authentication.  If not, then just turn off the MUTUAL_AUTH flag in the
gss_init_sec_context call and don't to mutual auth.

I would like to see the Nalin's patch, can it be added to the record for this
bug just for reference?




    
    

    
  
I agree.  Turn Mutual auth off until we can do it correctly, we are just 
wasting bandwidth in our current version.

Let's open a different bug for adding mutual auth support so we can hash it out 
there and this bug can remain closed (it's already way too long).

    
    

    
  
ok, i filed the following bugs:

  bug 239734 - negotiateauth extension should not request mutual auth
  bug 239735 - add kerberos mutual auth support to the negotiateauth extension

the first bug is targeting 1.7 final, and the second is futured.

patches welcome :)

    
    

    
  


  
The patch adds an additional parameter to
nsIHttpAuthenticator::generateCredentials which can be used to indicate that
the server must supply additional data, and a field to nsHttpChannel which is
used to keep track of this state.  Initially, this flag is false, but is set to
true if gss_init_sec_context returns GSS_C_CONTINUE_NEEDED.  If a 2xx or 3xx
response is received (I'm not certain whether or not the spec requires this,
but mod_auth_kerb is happy to perform mutual auth for a redirect), the flag is
checked.  Handling of errors is very rough, so it still needs work, if this is
even the right approach.

    
    

    
  
Nalin: Thank you for the patch, but I'd greatly appreciate it if you would
attach it to bug 239735 instead so we can discuss it there.  This bug is closed.
 Thx!!

    
    

    
  
In order to get ticket forwarding to work on Mac OS X 10.3 I had to create /etc/krb5.conf. The following 
worked for me:

[libdefaults]
        default_realm = FOO.RH.RIT.EDU

[realms]
        FOO.RH.RIT.EDU = {
                kdc = foo.rh.rit.edu:88
                default_domain = foo.rh.rit.edu
        }

[domain_realm]
        foo.rh.rit.edu = FOO.RH.RIT.EDU

Is there an easier/different way to get ticket passing to work or is creating the conf file a given?

    
    

    
  
On Mac OS X, the equivalent of /etc/krb5.conf is found at /Library/Preferences/edu.mit.Kerberos. It 
uses exactly the same config options as krb5.conf. You can symlink to edu.mit.Kerberos to make this 
work at the moment.

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: