Am 2014-07-15 14:19, schrieb David Woodhouse:
On Tue, 2014-07-15 at 13:18 +0200, Michael Osipov wrote:
Am 2014-07-13 22:22, schrieb David Woodhouse:
On Sun, 2014-07-13 at 11:31 +0200, Michael Osipov wrote:
Please have a look:
https://github.com/michael-o/curl/commit/b78ad621d45f537dfde745e961427257f1e1fc2d
Work is based on top of your patches.
That really wants splitting into individual patches to make it readable.
David,
I have split the patch apart and added some more bugfixes I did not
notice before.
Please have a look again:
https://github.com/michael-o/curl/compare/a6bf4636e4...1047baf0e3
I'll test that by the end of the week and make a complete patch proposal
if everything is fine.
Michael Osipov (7):
Added missing ifdef to Curl_http_done if GSS-API or SSPI is not available
I've merged that fix into the patch which introduced that bug now; thanks.
Add macros for the most common GSS-API mechs and pass them to
That commit subject is truncated (you can't wrap lines there). And I
don't like the patch either. I think this wants to be an enum, as
discussed. That way we can end up presenting the same API for our GSSAPI
and SSPI implementations, and the code which *uses* them can be the
same.
Remove checkprefix("GSS-Negotiate")
OK... but you're about to add half of this back again to handle
'WWW-Authenticate: Kerberos'. You'll need the 'protocol' member of
negotiatedata back again then, and the 'gss' member becomes 'spnego',
right? So perhaps it makes sense to remove GSS-Negotiate and add
Kerberos in the *same* patch, rather than in separate patches? Or at
least do them in consecutive patches.
The issues about would actually require to rename the struct to
something like gssdata from negotiatedata because it is not tied to
SPNEGO anymore -- and yes, in that case an enum would be wise.
E.g.:
#if defined(HAVE_GSSAPI) || defined(USE_WINDOWS_SSPI)
struct gssdata {
/* When doing GSS-based auth, we first need to send a token
and then validate the received one. */
enum { GSS_AUTHNONE, GSS_AUTHRECV, GSS_AUTHSENT } state;
enum { KERBEROS, SPNEGO, NTLM } mech;
#ifdef HAVE_GSSAPI
OM_uint32 status;
gss_ctx_id_t context;
gss_name_t server_name;
gss_buffer_desc output_token;
#else
#ifdef USE_WINDOWS_SSPI
DWORD status;
CtxtHandle *context;
CredHandle *credentials;
char server_name[1024];
size_t max_token_length;
BYTE *output_token;
size_t output_token_length;
#endif
#endif
};
#endif
#ifdef USE_HTTP_NEGOTIATE
gssdata->state = GSS_AUTHNONE;
if((authstatus->picked == CURLAUTH_NEGOTIATE) &&
gssdata->context && !GSS_ERROR(gssdata->status)) {
auth="Negotiate";
gssdata->mech = SPNEGO;
result = Curl_output_gss(conn, proxy);
if(result)
return result;
authstatus->done = TRUE;
gssdata->state = GSS_AUTHSENT;
}
else
#endif
While you are right about "add half of this back again", it not just
like add that enum and you are done. I you must add the define for the
CURLAUTH_, add CURL_VERSION_, register both at the appropriate spots,
add the command line options, etc. That might result in an additional
series of commits. That's why I have abstained from.
For such a change, I'd like to have a commiters consent. This is some
serious work.
Improve inline GSS-API naming in code documentation
Not so keen on this one either. I think 'GSSAPI' was better than 'GSS-API'.
The official name is GSS-API, I have that already patched in previous
ticket in the configure.ac. So that is an intentional correction.
About the truncation: I will take care of, it seems like GitHub limits
then to 72 chars.
-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette: http://curl.haxx.se/mail/etiquette.html
