On Mon, Feb 15, 2016 at 03:19:25PM -0500, Eric Sunshine wrote:
> On Mon, Feb 15, 2016 at 1:44 PM, brian m. carlson
> <sandals@xxxxxxxxxxxxxxxxxxxx> wrote:
> > Performing GSS-Negotiate authentication using Kerberos does not require
> > specifying a username or password, since that information is already
> > included in the ticket itself. However, libcurl refuses to perform
> > authentication if it has not been provided with a username and password.
> > Add an option, http.emptyAuth, that provides libcurl with an empty
> > username and password to make it attempt authentication anyway.
>
> I'm not familiar with this code, so let me know if my comments (below)
> are off the mark...
>
> > ---
> > diff --git a/http.c b/http.c
> > +++ b/http.c
> > @@ -299,14 +300,22 @@ static int http_options(const char *var, const char *value, void *cb)
> > static void init_curl_http_auth(CURL *result)
> > {
> > - if (!http_auth.username)
> > + if (!http_auth.username) {
> > + if (curl_empty_auth)
> > + curl_easy_setopt(result, CURLOPT_USERPWD, ":");
>
> Does this need to take LIBCURL_VERSION_NUM into account? Other code
> which uses CURLOPT_USERPWD only does so for certain versions of
> libcurl, otherwise CURLOPT_USERNAME and CURLOPT_PASSWORD is used.
This is the oldest version, which means it's the most compatible. Since
we don't need to consider odd usernames, it seemed silly to have both
cases present in the code. The benefit of using the pair of options is
that we don't leak memory in the non-empty auth case.
> > return;
> > + }
> >
> > credential_fill(&http_auth);
> >
> > @@ -827,7 +836,7 @@ struct active_request_slot *get_active_slot(void)
> > #ifdef LIBCURL_CAN_HANDLE_AUTH_ANY
> > curl_easy_setopt(slot->curl, CURLOPT_HTTPAUTH, http_auth_methods);
> > #endif
> > - if (http_auth.password)
> > + if (http_auth.password || curl_empty_auth)
> > init_curl_http_auth(slot->curl);
> >
> > return slot;
>
> Rather than sprinkling curl_empty_auth special cases here and there,
> would it be possible to simply set http_auth.username and
> http_auth.password to empty strings early on if they are not already
> set and curl_empty_auth is true, and then let the:
>
> strbuf_addf(&up, "%s:%s",
> http_auth.username, http_auth.password);
>
> in init_curl_http_auth() handle them in the normal fashion, with the
> end result being the same ":" set explicitly by this patch?
That would work. I was concerned about the credential_fill call
actually prompting the user, but it appears that it doesn't do that if
the password already exists. I don't know if we want to rely on that
functionality, though.
--
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187
Attachment:
signature.asc
Description: PGP signature