cURL / Mailing Lists / curl-library / Single Mail

curl-library

Re: [PATCH 2/5] Use SPNEGO for HTTP Negotiate

From: Michael Osipov <1983-01-06_at_gmx.net>
Date: Sat, 12 Jul 2014 11:06:39 +0200

Am 2014-07-12 02:34, schrieb David Woodhouse:
> On Fri, 2014-07-11 at 20:15 +0200, Michael Osipov wrote:
>> Am 2014-07-11 13:28, schrieb David Woodhouse:
>>> From: David Woodhouse <David.Woodhouse_at_intel.com>
>>>
>>
>> You can safely remove this from http_negotiate.c because the caller
>> already checks that:
>>
>> if(checkprefix("GSS-Negotiate", header)) {
>> protocol = "GSS-Negotiate";
>> gss = TRUE;
>> }
>> else if(checkprefix("Negotiate", header)) {
>> protocol = "Negotiate";
>> gss = FALSE;
>> }
>
> Yes, and I agree that 'GSS-Negotiate' should die.
>
> We'll end up wanting to add very similar logic to differentiate between
> Negotiate and Kerberos though, and it'll be 'use_spnego' that gets set
> or cleared depending on which one we see.

Good.

>> I don't like that code change. It can be done better.
>>
>> In curl_gssapi.h you should do:
>>
>> #ifdef HAVE_GSSAPI
>> #ifndef SPNEGO_MECHANISM
>> static gss_OID_desc spnego_mech_oid = { 6, "\x2b\x06\x01\x05\x05\x02" };
>> #define SPNEGO_MECHANISM &spnego_mech_oid
>> #endif
>> #ifndef KRB5_MECHANISM
>> static gss_OID_desc krb5_mech_oid = { 6, ... };
>> #define KRB5_MECHANISM &krb5_mech_oid
>> #endif
>
> Now you've defined a separate copy of spnego_mech_oid in every C file
> that includes curl_gssapi.h. Potentially unused.
>
> Surely you'd want it to be defined *once* in curl_gssapi.c and then
> exported?

Now I need some advise because I am not a daily C hacker. If the
curl_gssapi.h is not a good place -- granted.
What is it then? I won't run in any compilation problems if I put those
defines into the curl_gssapi.h? All I wanted is tht every consumer can
use thoes defines w/o definining them over an oder again.

How would your proposal look like?

> Doing something like this was my first inclination, to keep the
> signature of Curl_gss_init_sec_context() closer to that of the real
> gss_init_sec_context(), but I figured that a simple 'use_spnego' was
> probably cleaner in the end.
>
> That said, I don't care too much. If you want to do it your way then
> please go ahead and I'll insert your patch in my sequence instead.
>
>> This gives you the ability to use any mech and clearly indicate which is
>> used, for FTP and SOCKS GSS_KRB5_MECHANISM and for HTTP
>> GSS_SPNEGO_MECHANISM. You mave even define NTLM_MECHISM for your custom
>> GSS NTLMSSP.
>
> I don't think we'll be implementing an alternative to ntlm_wb using
> gssapi+gss-ntlmssp any time soon. The boolean for SPNEGO or not ought to
> be fine.

I will add your repo as remote and replay your commits on top of master.
Then I will make my improvements and share. I'd like you to review them,
if they are fine. I'll go ahead next week and test them.

If all is fine, we cann provide an altered patch set to the mailing list.

Is that OK?

Michael

-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette: http://curl.haxx.se/mail/etiquette.html
Received on 2014-07-12