{"thread":{"id":"55155","subject":"[PATCH v2] remote-curl: fall back to basic auth if Negotiate fails","startedAt":"2021-02-16T16:58:24Z","lastAt":"2021-03-22T16:18:48Z","messageCount":3,"participants":["Christopher via GitGitGadget","Christopher Schenk"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"417074","messageId":"pull.849.v2.git.1613494656636.gitgitgadget@gmail.com","threadId":"55155","inReplyTo":"pull.849.git.1611921008282.gitgitgadget@gmail.com","subject":"[PATCH v2] remote-curl: fall back to basic auth if Negotiate fails","fromName":"Christopher via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-02-16T16:57:36Z","receivedAt":"2021-02-16T16:58:24Z","isPatch":true,"sender":{"key":"name:Christopher","avatar":null},"body":"From: Christopher Schenk \n\nWhen the username and password are supplied in a url like this\nhttps://myuser:secret@git.exampe/myrepo.git and the server supports the\nnegotiate authenticaten method git does not fall back to basic auth and\nlibcurl hardly tries to authenticate with the negotiate method.\n\nStop using the Negotiate authentication method after the first failure\nbecause if it fails on the first try it will never succeed.\n\nV1 of this patch somehow did not make it to the mailing list so i will\ntry to send this patch again\n\nSigned-off-by: Christopher Schenk \n---\n remote-curl: fall back to basic auth if Negotiate fails\n \n When the username and password are supplied in a url like this\n https://myuser:secret@git.exampe/myrepo.git and the server supports the\n negotiate authenticaten method git does not fall back to basic auth and\n libcurl hardly tries to authenticate with the negotiate method.\n \n Stop using the Negotiate authentication method after the first failure\n because if it fails on the first try it will never succeed.\n \n Signed-off-by: Christopher Schenk christopher@cschenk.net\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-849%2Fchschenk%2Fkerberos-basic-fallback-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-849/chschenk/kerberos-basic-fallback-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/849\n\nRange-diff vs v1:\n\n 1: 285a8a568444 ! 1: 7bfc0b431910 remote-curl: fall back to basic auth if Negotiate fails\n @@ Commit message\n Stop using the Negotiate authentication method after the first failure\n because if it fails on the first try it will never succeed.\n \n + V1 of this patch somehow did not make it to the mailing list so i will\n + try to send this patch again\n +\n Signed-off-by: Christopher Schenk \n \n ## http.c ##\n\n\n http.c | 8 ++++++++\n 1 file changed, 8 insertions(+)\n\ndiff --git a/http.c b/http.c\nindex 8b23a546afdf..36f113d46c23 100644\n--- a/http.c\n+++ b/http.c\n@@ -1642,6 +1642,14 @@ static int handle_curl_result(struct slot_results *results)\n \t\treturn HTTP_MISSING_TARGET;\n \telse if (results->http_code == 401) {\n \t\tif (http_auth.username && http_auth.password) {\n+#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY\n+\t\t\tif (results->auth_avail & CURLAUTH_GSSNEGOTIATE) {\n+\t\t\t\thttp_auth_methods &= ~CURLAUTH_GSSNEGOTIATE;\n+\t\t\t\thttp_auth_methods &= results->auth_avail;\n+\t\t\t\thttp_auth_methods_restricted = 1;\n+\t\t\t\treturn HTTP_REAUTH;\n+\t\t\t}\n+#endif\n \t\t\tcredential_reject(&http_auth);\n \t\t\treturn HTTP_NOAUTH;\n \t\t} else {\n\nbase-commit: 71ca53e8125e36efbda17293c50027d31681a41f\n-- \ngitgitgadget\n"},{"id":"419937","messageId":"pull.849.v3.git.1616413876663.gitgitgadget@gmail.com","threadId":"55155","inReplyTo":"pull.849.v2.git.1613494656636.gitgitgadget@gmail.com","subject":"[PATCH v3] remote-curl: fall back to basic auth if Negotiate fails","fromName":"Christopher via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-03-22T11:51:16Z","receivedAt":"2021-03-22T11:52:10Z","isPatch":true,"sender":{"key":"name:Christopher","avatar":null},"body":"From: Christopher Schenk \n\nWhen the username and password are supplied in a url like this\nhttps://myuser:secret@git.exampe/myrepo.git and the server supports the\nnegotiate authenticaten method, git does not fall back to basic auth and\nlibcurl hardly tries to authenticate with the negotiate method.\n\nStop using the Negotiate authentication method after the first failure\nbecause if it fails on the first try it will never succeed.\n\nSigned-off-by: Christopher Schenk \n---\n remote-curl: fall back to basic auth if Negotiate fails\n \n When the username and password are supplied in a url like this\n https://myuser:secret@git.exampe/myrepo.git and the server supports the\n negotiate authenticaten method git does not fall back to basic auth and\n libcurl hardly tries to authenticate with the negotiate method.\n \n Stop using the Negotiate authentication method after the first failure\n because if it fails on the first try it will never succeed.\n \n Signed-off-by: Christopher Schenk christopher@cschenk.net\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-849%2Fchschenk%2Fkerberos-basic-fallback-v3\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-849/chschenk/kerberos-basic-fallback-v3\nPull-Request: https://github.com/gitgitgadget/git/pull/849\n\nRange-diff vs v2:\n\n 1: 7bfc0b431910 ! 1: 52de7fa42f88 remote-curl: fall back to basic auth if Negotiate fails\n @@ Commit message\n \n When the username and password are supplied in a url like this\n https://myuser:secret@git.exampe/myrepo.git and the server supports the\n - negotiate authenticaten method git does not fall back to basic auth and\n + negotiate authenticaten method, git does not fall back to basic auth and\n libcurl hardly tries to authenticate with the negotiate method.\n \n Stop using the Negotiate authentication method after the first failure\n because if it fails on the first try it will never succeed.\n \n - V1 of this patch somehow did not make it to the mailing list so i will\n - try to send this patch again\n -\n Signed-off-by: Christopher Schenk \n \n ## http.c ##\n @@ http.c: static int handle_curl_result(struct slot_results *results)\n + \t} else if (missing_target(results))\n \t\treturn HTTP_MISSING_TARGET;\n \telse if (results->http_code == 401) {\n - \t\tif (http_auth.username && http_auth.password) {\n +#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY\n -+\t\t\tif (results->auth_avail & CURLAUTH_GSSNEGOTIATE) {\n -+\t\t\t\thttp_auth_methods &= ~CURLAUTH_GSSNEGOTIATE;\n -+\t\t\t\thttp_auth_methods &= results->auth_avail;\n -+\t\t\t\thttp_auth_methods_restricted = 1;\n -+\t\t\t\treturn HTTP_REAUTH;\n -+\t\t\t}\n ++\t\thttp_auth_methods &= ~CURLAUTH_GSSNEGOTIATE;\n ++\t\tif (results->auth_avail) {\n ++\t\t\thttp_auth_methods &= results->auth_avail;\n ++\t\t\thttp_auth_methods_restricted = 1;\n ++\t\t\treturn HTTP_REAUTH;\n ++\t\t}\n +#endif\n + \t\tif (http_auth.username && http_auth.password) {\n \t\t\tcredential_reject(&http_auth);\n \t\t\treturn HTTP_NOAUTH;\n \t\t} else {\n +-#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY\n +-\t\t\thttp_auth_methods &= ~CURLAUTH_GSSNEGOTIATE;\n +-\t\t\tif (results->auth_avail) {\n +-\t\t\t\thttp_auth_methods &= results->auth_avail;\n +-\t\t\t\thttp_auth_methods_restricted = 1;\n +-\t\t\t}\n +-#endif\n + \t\t\treturn HTTP_REAUTH;\n + \t\t}\n + \t} else {\n\n\n http.c | 15 ++++++++-------\n 1 file changed, 8 insertions(+), 7 deletions(-)\n\ndiff --git a/http.c b/http.c\nindex 8b23a546afdf..4b4cfee8185c 100644\n--- a/http.c\n+++ b/http.c\n@@ -1641,17 +1641,18 @@ static int handle_curl_result(struct slot_results *results)\n \t} else if (missing_target(results))\n \t\treturn HTTP_MISSING_TARGET;\n \telse if (results->http_code == 401) {\n+#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY\n+\t\thttp_auth_methods &= ~CURLAUTH_GSSNEGOTIATE;\n+\t\tif (results->auth_avail) {\n+\t\t\thttp_auth_methods &= results->auth_avail;\n+\t\t\thttp_auth_methods_restricted = 1;\n+\t\t\treturn HTTP_REAUTH;\n+\t\t}\n+#endif\n \t\tif (http_auth.username && http_auth.password) {\n \t\t\tcredential_reject(&http_auth);\n \t\t\treturn HTTP_NOAUTH;\n \t\t} else {\n-#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY\n-\t\t\thttp_auth_methods &= ~CURLAUTH_GSSNEGOTIATE;\n-\t\t\tif (results->auth_avail) {\n-\t\t\t\thttp_auth_methods &= results->auth_avail;\n-\t\t\t\thttp_auth_methods_restricted = 1;\n-\t\t\t}\n-#endif\n \t\t\treturn HTTP_REAUTH;\n \t\t}\n \t} else {\n\nbase-commit: 71ca53e8125e36efbda17293c50027d31681a41f\n-- \ngitgitgadget\n"},{"id":"419955","messageId":"2d995558-1fce-824f-ae65-bcbd3b744cb2@cschenk.net","threadId":"55155","inReplyTo":"xmqq35xvpr8q.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v2] remote-curl: fall back to basic auth if Negotiate fails","fromName":"Christopher Schenk","fromEmail":"christopher@cschenk.net","sentAt":"2021-03-22T16:08:12Z","receivedAt":"2021-03-22T16:18:48Z","isPatch":true,"sender":{"key":"christopher@cschenk.net","avatar":null},"body":"\n\nOn 2/16/21 11:44 PM, Junio C Hamano wrote:\n> \"Christopher via GitGitGadget\" writes:\n> \n>> From: Christopher Schenk \n>>\n>> When the username and password are supplied in a url like this\n>> https://myuser:secret@git.exampe/myrepo.git and the server supports the\n>> negotiate authenticaten method git does not fall back to basic auth and\n> \n> s/method git/method, git/;\n> \n>> libcurl hardly tries to authenticate with the negotiate method.\n> \n> Thanks.\n> \n>> Stop using the Negotiate authentication method after the first failure\n>> because if it fails on the first try it will never succeed.\n> \n> Is this patch needed because we are using cURL library incorrectly,\n> or is it the limitation of the cURL library?\nI'm no cURL expert, but in my opinon this is a limitation of the cURL \nLibary.\n> \n>> V1 of this patch somehow did not make it to the mailing list so i will\n>> try to send this patch again\n> \n> The last paragraph does not belong to the commit log message; if\n> nobody on the list saw the \"v1\", as far as the project is concerned,\n> it never happened.\nI have adapted the commit message accordingly.\n> \n>> Signed-off-by: Christopher Schenk \n>> ---\n> \n>> diff --git a/http.c b/http.c\n>> index 8b23a546afdf..36f113d46c23 100644\n>> --- a/http.c\n>> +++ b/http.c\n>> @@ -1642,6 +1642,14 @@ static int handle_curl_result(struct slot_results *results)\n>> \t\treturn HTTP_MISSING_TARGET;\n>> \telse if (results->http_code == 401) {\n>> \t\tif (http_auth.username && http_auth.password) {\n>> +#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY\n>> +\t\t\tif (results->auth_avail & CURLAUTH_GSSNEGOTIATE) {\n>> +\t\t\t\thttp_auth_methods &= ~CURLAUTH_GSSNEGOTIATE;\n>> +\t\t\t\thttp_auth_methods &= results->auth_avail;\n>> +\t\t\t\thttp_auth_methods_restricted = 1;\n>> +\t\t\t\treturn HTTP_REAUTH;\n>> +\t\t\t}\n>> +#endif\n>> \t\t\tcredential_reject(&http_auth);\n>> \t\t\treturn HTTP_NOAUTH;\n>> \t\t} else {\n> \n> Hmph, is this an extension to what 4dbe6646 (remote-curl: fall back\n> to Basic auth if Negotiate fails, 2015-01-08) tried to do? What\n> happens on the \"else\" side after the context of this patch, which\n> seems to have come from:\n> \n> - 4dbe6646 (remote-curl: fall back to Basic auth if Negotiate\n> fails, 2015-01-08)\n> \n> - 840398fe (http: restrict auth methods to what the server\n> advertises, 2017-02-22), and\n> \n> - 40a18fc7 (http: add an \"auto\" mode for http.emptyauth,\n> 2017-02-25),\n> \n> looks essentially the same as what this patch is adding, and I am\n> wondering if there is a room for simplification. It almost looks\n> to me that the only difference between \"credential fully given\" and\n> other case is if we \"reject\" the credential after this patch.\n> \n> Asking contributors who made these past contributions for input.\n> \n> Thanks.\n> \n\nI have simplified the code and sent the patch again.\n\nThanks.\n"}]}