Hi Steve,

On Mon, Feb 9, 2015 at 11:37 PM, Steve Holme <steve_holme_at_hotmail.com> wrote:
>
> On Sun, 8 Feb 2015, Isaac Boukris wrote:
>
> > Also, it doesn't seem to have been the initial intention - see this quote from
> > the comment in the source (http.c): "Then we set the picked one to the want
> > one, and if this is one single bit it'll be used instantly."
>
> From my knowledge in this area that doesn't seem quite right and if we can save an extra round trip, and it is valid to do so, then great ;-)

It works well in my tests and according to RFC 4559 (SPNEGO) it should
be perfectly valid.
In fact, logically the initial 401 isn't necessary as it doesn't
initialize the GSS-API but only offers 'Negotiate' authentication.

See these quotes from the RFC (which is actually quite short and concise):

"The initial WWW-Authenticate header will not carry any gssapi-data.
...
A client may initiate a connection to the server with an
"Authorization" header containing the initial token for the server.
This form will bypass the initial 401 error from the server when the
client knows that the server will accept the Negotiate HTTP
authentication type."

> > I've been working this weekend on the attached patch, with these changes
> > most of the logic is done at 'Curl_output_negotiate' instead of
> > 'Curl_input_negotiate' which only takes server's answer. It saves the extra
> > round trip when using Negotiate (either explicitly or in CURLAUTH_ANY flow)
> > and aligns the behavior with other protocols.
>
> Cool - I'll try and take a look at your patch in more detail over the next few days.

Thanks a lot!

> > I've run quite several tests (with valgrind) of NTLM and Kerberos with Heimdal
> > Kerberos GSSAPI library (also tested NTLM inside Negotiate with Heimdal's
> > NTLMSSP).
>
> Was that your own tests or did you run the curl Test Suite?

I've run my own tests against a Windows 2003 server (I'll checkout
about curl's test suite).

> > When investigating, I noticed the new code seem not to add '+1' to
> > 'spn_token.length' compare to the old code.
>
> That might have been me :( but which commit are you referring to?

I think it is commit 1cbc8fd3d1a95b90a1585d57d7af37c73cda2fc1 but
can't tell for sure.

The old code was:
token.length = strlen(service) + 1 + strlen(proxy ? conn->proxy.name :
conn->host.name) + 1;

Which equals to 'strlen +1' that I'd assume accounts for NULL termination.
But to be honest, I ain't that familiar with GSS-API internals and
only based my comment on the old code and the referenced
documentation.

> > Note, just before sending I noticed these changes would probably impact
> > on 'http_negotiate_sspi.c' as well... I'll look at it if necessary.
>
> Yes - It will probably need fixing there to - however, it is worth noting that:
>
> I wanted to align the SPNEGO GSS-API and SSPI code for the next release, but it was dependent on a little rework of the authentication code for HTTP and SASL and unfortunately I ran out of time for v7.41.0. In that respect I only managed to align the return codes in these functions :(
>
> As such I want to move SPNEGO code from HTTP into the new authentication module. This module will then allow the GSS-API code and SSPI based code to be called from a single HTTP source module, for example, the current http_negotiate.c (Just as the "NTLM" mechanism is called for HTTP and SASL and the "GSSAPI" mechanism for SASL).

I think that makes good sense.

> Given this, would you prefer to attempt a fix in the SSPI code, fixing both at the same time, or wait for my changes in March which should mean a single fix?

As the logic of the changes should be the same I'll give it a try in
the following days (and use the opportunity to practice some windows
programming).
I am actually quite eager to advance and get this ready as I wish for
more people to review and test so we could correct and improve as
necessary.

Furthermore, while researching I've come up with a few additional
ideas for possible (related) improvements which I'd like to
investigate and try atop of this fix.

Thanks and regards,
Isaac B.

-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette: http://curl.haxx.se/mail/etiquette.html
Received on 2015-02-10