Re: [PATCH] http: add option to try authentication without username

From: brian m. carlson <sandals@crustytoothpaste.net>
Date: Feb 15, 2016, 20:29 UTC
Message-ID: <20160215202937.GA57185@vauxhall.crustytoothpaste.net>
In-Reply-To: <CAPig+cTr1eW1KLsZGpY98hUhJ2EHdPopz9C_gTztZRdNPBQTmQ@mail.gmail.com>

On Mon, Feb 15, 2016 at 03:19:25PM -0500, Eric Sunshine wrote:

Show 26 quoted lines

> On Mon, Feb 15, 2016 at 1:44 PM, brian m. carlson
> <sandals@crustytoothpaste.net> 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.

Show 25 quoted lines

> >                 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