Re: [PATCH v2] remote-curl: fall back to Basic auth if Negotiate fails
- From
brian m. carlson <sandals@crustytoothpaste.net>
- Date
- Jan 3, 2015, 17:45 UTC
- Message-ID
- <20150103174509.GA1025060@vauxhall.crustytoothpaste.net>
- In-Reply-To
- <20150103111922.GB27793@peff.net>
On Sat, Jan 03, 2015 at 06:19:23AM -0500, Jeff King wrote:
>This pattern gets repeated in several places. Now that >http_passwordless_auth is a global, can we handle it automatically for >the callers, as below (which, aside from compiling, is completely >untested by me)?
This looks good (although I haven't tested it).
Show 7 quoted lines
>Note that this is in a slightly different boat than credential_fill. >Ideally we would also handle picking up credentials on behalf of the >callers of get_curl_handle/handle_curl_result. But that may involve >significant work and/or prompting the user, which we _must_ avoid if we >do not know if we are going to retry the request (and only the caller >knows that for sure). However, in the case of http_passwordless_auth, we >are just setting a flag, so it's OK to do it preemptively.
Right. We already prompt the user for a username and password in that case, so we already have the credentials that we need.
Show 20 quoted lines
>diff --git a/http.c b/http.c
>index 040f362..2bbcdf1 100644
>--- a/http.c
>+++ b/http.c
>@@ -62,6 +62,8 @@ static const char *user_agent;
>
> static struct credential cert_auth = CREDENTIAL_INIT;
> static int ssl_cert_password_required;
>+/* Should we allow non-password-based authentication (e.g. GSSAPI)? */
>+static int http_passwordless_auth = 1;
>
> static struct curl_slist *pragma_header;
> static struct curl_slist *no_pragma_header;
>@@ -318,7 +320,12 @@ static CURL *get_curl_handle(void)
> curl_easy_setopt(result, CURLOPT_NETRC, CURL_NETRC_OPTIONAL);
> #endif
> #ifdef LIBCURL_CAN_HANDLE_AUTH_ANY
>- curl_easy_setopt(result, CURLOPT_HTTPAUTH, CURLAUTH_ANY);
>+ {
>+ int flags = CURLAUTH_ANY;I think this needs to be unsigned long or it can cause undefined behavior, since libcurl uses unsigned long in the flags. I'll fix that up when I reroll. I'll need your sign-off since it will essentially be your work.
Show 25 quoted lines
>+ if (!http_passwordless_auth)
>+ flags &= ~CURLAUTH_GSSNEGOTIATE;
>+ curl_easy_setopt(result, CURLOPT_HTTPAUTH, flags);
>+ }
> #endif
>
> if (http_proactive_auth)
>@@ -870,6 +877,7 @@ int handle_curl_result(struct slot_results *results)
> credential_reject(&http_auth);
> return HTTP_NOAUTH;
> } else {
>+ http_passwordless_auth = 0;
> return HTTP_REAUTH;
> }
> } else {
>
>
>Note that you could probably drop http_passwordless_auth completely, and
>just keep a:
>
> static int http_auth_methods = CURLAUTH_ANY;
>
>and then drop CURLAUTH_GSSNEGOTIATE from it instead of setting the
>passwordless_auth flag to 0 (again, it happens in one place, so I don't
>know that it needs an extra layer of abstraction).This does seem to handle both cases well. It also has the pleasant side effect of being static.
-- brian m. carlson / brian with sandals: Houston, Texas, US +1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187