Am 2014-07-12 17:58, schrieb David Woodhouse:
> [...]
>>> So what *do* we want to do on top of the patch set I posted? Just add
>>> support for '{Proxy,WWW}-Authenticate: Kerberos'?
>>
>> I would rather do that after this patch has been tested, approved and
>> committed. This is the safest way to implement that improvement on top.
>> I don't like to fix two things in one big patch. It ends up in a mess.
>
> Pfft. It's a set of 7 patches in my tree already; what's wrong with
> making it 8? :)

That maybe true but in my opinion this can be done this way:

rename http_negoatiate.c to http_gssapi.c and your are almost done. A
few modified signatures and the very same code does Kerberos and SPNEGO
almost for free.

Now let's get back to the patch. I am half way through your patch. Code
looks good with a few glitches. I have improved those and modified a
lots of the boiler-plate code. I compiles flawlessly on Linux Mint, make
test runs fine too. The non-GSS-API version runs fine.
I will test the entire code by the end of the next week at work. So
these changes are still pending.

Please have a look:
https://github.com/michael-o/curl/commit/b78ad621d45f537dfde745e961427257f1e1fc2d

Work is based on top of your patches.

There is another issue with the code I'd like you to examine with your C
knowledge, mine is rather limited. The entire auth loop workflow is,
unfortunately, spread over several files/places which makes it hard to read.

Curl_http_input_auth():

It receives an auth challenge from the server and passes to
Curl_input_negotiate but it does not init the context to NO_CONTEXT but
simply passes a NULL pointer.
After the first round trip, a mutual token is received but nowhere is
saved that the whether auth is actually complete or continue is needed.
The enum state does not really help. It does not reflect the looping.
If there would be a check, you could already call Curl_cleanup_negotiate
here and leaving an additional call with Curl_http_done in case of
failures. Alternatively, you would call it after all failures.
Moreover, I fail to see the gss_release_buffer on the input_token when
the server sent one, is Curl_safefree(input_token.value) enough?

output_auth_headers():

> negdata->state = GSS_AUTHNONE;

This blind assumes that we always have only one way auth.

> if((authstatus->picked == CURLAUTH_NEGOTIATE) &&
> negdata->context && !GSS_ERROR(negdata->status)) {
> auth="Negotiate";
> result = Curl_output_negotiate(conn, proxy);
> if(result)
> return result;
> authstatus->done = TRUE;

This is also wrong, the auth is not complete, CONTINUE_NEEDED is
completely ignored. The client must wait for the mutual auth response.
There is a multi flag, we should set it to TRUE.

> negdata->state = GSS_AUTHSENT;
> }

Curl_output_negotiate():

> gss_release_buffer(&discard_st, &neg_ctx->output_token);
> neg_ctx->output_token.value = NULL;
> neg_ctx->output_token.length = 0;

I do not think that release and the assignments are necessary. Release
ought be enough.

I am curious about your comments,

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