On Wed, Jul 17, 2002 at 12:18:05PM -0700, [EMAIL PROTECTED] wrote:
> Daniel Kouril wrote:
> > Hi all
> > 
> > I'm not sure if this is the right mailling list for such an announcement but
> > I think it is the most appropriate I was able to find. Please let me know if 
> > there's a better place.
> > 
> > We have implemented a Kerberos support for Mozilla (and the Apache http
> > server). It's available at
> > http://meta.cesnet.cz/software/heimdal/negotiate.en.html
> > 
> > The solution is based on draft 'draft-brezak-spnego-http-03.txt', which
> > defines how Kerberos is used in Microsoft www components. It is based on
> > using of the GSS-API (RFC 2743) and SPNEGO (RFC 2478). 
> > 
> > I would very appreciate if someone who understands the libnecko code would
> > overview the implementation since I'm affraid there are some ugly
> > constructions there (caused by the fact that we are not C++ programmers and
> > also don't know the mozilla code well).
> > 
> > I'm looking forward to any comments.
> > 
> > --
> > Dan Kouril
> > 
> > 

First of all, sorry for my delay. I was on vacation and then at a conference
with poor Internet connectivity so I couldn't response earlier.

P.S.
I'm very glad I ever found someone who has overviewed the patch and sent me
comments. I know that the code requires a lot of fixes but since I'm not a
C++ nor Mozilla expert I need someone more familiar with the enviroment.
Thank you.

> 
> i only took a brief look at your patch for mozilla, and i spotted a 
> couple problems...
> 
> 1) the makefiles include full paths to libraries:
> 
> +             /usr/heimdal/lib/libgssapi.a \
> +             /usr/heimdal/lib/libkrb5.a \
> +             /usr/heimdal/lib/libasn1.a \
> +             /usr/heimdal/lib/libroken.a \
> +             /usr/lib/libdb.a \
> +             /usr/lib/libresolv.a \
> +             /software/openssl-0.9.6/lib/libcrypto.a \
> 
> this is not the right way to specify libraries.  moreover, pulling in
> openssl is probably the wrong solution given that mozilla already has 
> it's own crypto library (NSS).  you should talk to the NSS folks about
> getting access to the crypto functions you need.

I agree that hardcoded path and libraries are not nice. In the future I'm
going to change the configuration scripts and exports some variables from the
configuration stage. 

Regarding the Openssl libcrypto, the patch doesn't use any function from the
library directly. The library is required by the Kerberos libraries thus it
cannot be replaced by the NSS libs.

> 
> 2) non-english comments are fairly useless to the necko maintainers (and 
> most mozilla developers), so you should really try to translate 
> everything to english as best as you can.  otherwise, some good 
> information that you have documented may be overlooked when someone else 
> tries to modify the code.

Definetely, sorry for inconvenience, I'll fix it later on.

> 
> 3) some of your mods are contained within #ifdef GSSAPI and some of your 
> mods are outside #ifdef GSSAPI.  seems like you should either keep all 
> of your mods inside #ifdef GSSAPI or eliminate #ifdef GSSAPI altogether.
> i.e., please be consistent.

We enclosed only really GSS-related stuff within #ifdef GSSAPI. The other
part of the patch, which are outside #ifdef GSSAPI is more general, which
could be also used while implementing other authentication methods.

> 
> 4) limiting the length of the server_name to 1024 chars seems bad.  how 
> about this instead:
> 
>     hostname.Insert(NS_LITEREAL_CSTRING("khttp@"), 0);
> 
> this modifies hostname to include the "khttp@" prefix.  use 
> hostname.Length() to determine strlen for hostname.

Thanks, I'll fix it.

> 
> 5) i'm concerned about your modifications to nsHttpChannel... but i 
> don't have time to review them carefully right now.  how about moving 
> this patch into a bugzilla bug?

OK, I'll prepare a new version reflecting your comments and try submitting it
into the bugzilla (perhaps next week).

--
Dan

Reply via email to