{"thread":{"id":"29678","subject":"Patchset NTLM-Authentication","startedAt":"2012-02-21T08:07:59Z","lastAt":"2012-02-21T19:02:34Z","messageCount":5,"participants":["Schmidt, Marco","Thomas Rast","Junio C Hamano","Daniel Stenberg"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"185059","messageId":"4CDEC141B5583D408E79F2931DB7708301802B70@GSX300A.mxchg.m.corp","threadId":"29678","inReplyTo":null,"subject":"Patchset NTLM-Authentication","fromName":"Schmidt, Marco","fromEmail":"marco.schmidt@cassidian.com","sentAt":"2012-02-21T08:07:59Z","receivedAt":"2012-02-21T08:07:59Z","isPatch":false,"sender":{"key":"marco.schmidt@cassidian.com","avatar":null},"body":"Support NTLM-Authentication against proxy.\n\nI have include a minimal patch set to support NTLM authentification\nagainst a proxy server. \n\nThe patch adds a boolean configuration variable \"http.proxyauthany\" to\nenable CURLOPT_PROXYAUTH = CURLAUTH_ANY.\n\nI'm not conform with your patch submitting policy. I had no chance to\ntrack the git repository from my current location.\n\n\nMarco Schmidt\n\n\nFrom 8a3892eca69a0c603a53b3d3aa052f45c4a5b648 Mon Sep 17 00:00:00 2001\nFrom: Marco Schmidt <Marco.Schmidt@cassidian.com>\nDate: Fri, 27 Jan 2012 13:08:44 +0100\nSubject: [PATCH 2/4] =?UTF-8?q?Einf=FChren=20der=20HTTP=20Proxy=20Authentifi?=\n =?UTF-8?q?zierung?=\nMIME-Version: 1.0\nContent-Type: text/plain; charset=UTF-8\nContent-Transfer-Encoding: 8bit\n\n---\n http.c |   11 +++++++++++\n 1 files changed, 11 insertions(+), 0 deletions(-)\n\ndiff --git a/http.c b/http.c\nindex f3b2c90..dc51d41 100644\n--- a/http.c\n+++ b/http.c\n@@ -41,6 +41,7 @@ static long curl_low_speed_limit = -1;\n static long curl_low_speed_time = -1;\n static int curl_ftp_no_epsv;\n static const char *curl_http_proxy;\n+static int curl_http_proxyauthany = 0;\n static const char *curl_cookie_file;\n static char *user_name, *user_pass, *description;\n static const char *user_agent;\n@@ -222,6 +223,10 @@ static int http_options(const char *var, const char *value, void *cb)\n \t}\n \tif (!strcmp(\"http.proxy\", var))\n \t\treturn git_config_string(&curl_http_proxy, var, value);\n+\t\n+\tif (!strcmp(\"http.proxyauthany\", var))  {\n+\t\tcurl_http_proxyauthany = git_config_bool(var, value);\n+\t}\n \n \tif (!strcmp(\"http.cookiefile\", var))\n \t\treturn git_config_string(&curl_cookie_file, var, value);\n@@ -330,6 +335,12 @@ static CURL *get_curl_handle(void)\n \n \tif (curl_http_proxy)\n \t\tcurl_easy_setopt(result, CURLOPT_PROXY, curl_http_proxy);\n+\t\n+\tif (curl_http_proxyauthany) {\n+\t\tcurl_easy_setopt(result, CURLOPT_PROXYAUTH, CURLAUTH_ANY);\n+\t\tprintf(\"CURLOPT_PROXYAUTH!!!!\");\n+\t}\n+\n \n \treturn result;\n }\n-- \n1.7.8\n\n\n\nFrom 2b7336c4959fc4df30da9e68f652f2c5941f0c91 Mon Sep 17 00:00:00 2001\nFrom: Marco Schmidt <Marco.Schmidt@cassidian.com>\nDate: Fri, 27 Jan 2012 14:35:15 +0100\nSubject: [PATCH 3/4] First working step\n\n---\n http.c |    9 ++++++++-\n 1 files changed, 8 insertions(+), 1 deletions(-)\n\ndiff --git a/http.c b/http.c\nindex dc51d41..3dbd390 100644\n--- a/http.c\n+++ b/http.c\n@@ -173,6 +173,8 @@ static int git_config_path(const char **result,\n \n static int http_options(const char *var, const char *value, void *cb)\n {\n+\tfprintf(stderr, \"http_options: %s = %s\\n\", var, value);\n+\t\n \tif (!strcmp(\"http.sslverify\", var)) {\n \t\tcurl_ssl_verify = git_config_bool(var, value);\n \t\treturn 0;\n@@ -226,6 +228,8 @@ static int http_options(const char *var, const char *value, void *cb)\n \t\n \tif (!strcmp(\"http.proxyauthany\", var))  {\n \t\tcurl_http_proxyauthany = git_config_bool(var, value);\n+\t\tfprintf(stderr, \"http_options: curl_http_proxyauthany = %i\", curl_http_proxyauthany);\n+\t\treturn 0;\n \t}\n \n \tif (!strcmp(\"http.cookiefile\", var))\n@@ -336,9 +340,12 @@ static CURL *get_curl_handle(void)\n \tif (curl_http_proxy)\n \t\tcurl_easy_setopt(result, CURLOPT_PROXY, curl_http_proxy);\n \t\n+\tfprintf(stderr, \"DEBUG: curl_http_proxyauthany\\n\");\n+\t\n+\t/* http proxy could be set from enviroment - set CURLOPT_PROXYAUTH */\n \tif (curl_http_proxyauthany) {\n \t\tcurl_easy_setopt(result, CURLOPT_PROXYAUTH, CURLAUTH_ANY);\n-\t\tprintf(\"CURLOPT_PROXYAUTH!!!!\");\n+\t\tfprintf(stderr, \"curl_easy_setopt(result, CURLOPT_PROXYAUTH, CURLAUTH_ANY);\\n\");\n \t}\n \n \n-- \n1.7.8\n\n\n\nFrom 55b76b5f658904d240db29bb0d59f8c1dcedd4b5 Mon Sep 17 00:00:00 2001\nFrom: Marco Schmidt <Marco.Schmidt@cassidian.com>\nDate: Fri, 27 Jan 2012 14:40:57 +0100\nSubject: [PATCH 4/4] remove debug information\n\n---\n http.c |    8 ++------\n 1 files changed, 2 insertions(+), 6 deletions(-)\n\ndiff --git a/http.c b/http.c\nindex 3dbd390..fbc52f0 100644\n--- a/http.c\n+++ b/http.c\n@@ -173,8 +173,6 @@ static int git_config_path(const char **result,\n \n static int http_options(const char *var, const char *value, void *cb)\n {\n-\tfprintf(stderr, \"http_options: %s = %s\\n\", var, value);\n-\t\n \tif (!strcmp(\"http.sslverify\", var)) {\n \t\tcurl_ssl_verify = git_config_bool(var, value);\n \t\treturn 0;\n@@ -228,7 +226,6 @@ static int http_options(const char *var, const char *value, void *cb)\n \t\n \tif (!strcmp(\"http.proxyauthany\", var))  {\n \t\tcurl_http_proxyauthany = git_config_bool(var, value);\n-\t\tfprintf(stderr, \"http_options: curl_http_proxyauthany = %i\", curl_http_proxyauthany);\n \t\treturn 0;\n \t}\n \n@@ -340,13 +337,12 @@ static CURL *get_curl_handle(void)\n \tif (curl_http_proxy)\n \t\tcurl_easy_setopt(result, CURLOPT_PROXY, curl_http_proxy);\n \t\n-\tfprintf(stderr, \"DEBUG: curl_http_proxyauthany\\n\");\n-\t\n+#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY\t\n \t/* http proxy could be set from enviroment - set CURLOPT_PROXYAUTH */\n \tif (curl_http_proxyauthany) {\n \t\tcurl_easy_setopt(result, CURLOPT_PROXYAUTH, CURLAUTH_ANY);\n-\t\tfprintf(stderr, \"curl_easy_setopt(result, CURLOPT_PROXYAUTH, CURLAUTH_ANY);\\n\");\n \t}\n+#endif\n \n \n \treturn result;\n-- \n1.7.8\n\n"},{"id":"185064","messageId":"87vcn0h77a.fsf@thomas.inf.ethz.ch","threadId":"29678","inReplyTo":"4CDEC141B5583D408E79F2931DB7708301802B70@GSX300A.mxchg.m.corp","subject":"Re: Patchset NTLM-Authentication","fromName":"Thomas Rast","fromEmail":"trast@inf.ethz.ch","sentAt":"2012-02-21T09:22:17Z","receivedAt":"2012-02-21T09:22:17Z","isPatch":false,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"\"Schmidt, Marco\" <Marco.Schmidt@cassidian.com> writes:\n\n> I'm not conform with your patch submitting policy. I had no chance to\n> track the git repository from my current location.\n\nYou should read Documentation/SubmittingPatches.  If your location does\nnot let you use any git:// or git+http:// (\"smart http\") transports, you\ncan use the wereHamster's bundler service https://bundler.caurea.org/ to\nget a bundle that you can download over http.  (If you cannot download\n*anything* over http, what is your internet access good for?)\n\nLooking at the attachments:\n\n[0002-Einf-hren-der-HTTP-Proxy-Authentifizierung.patch]\n[0003-First-working-step.patch]\n[0004-remove-debug-information.patch]\n\n* You seem to have forgotten the first patch.  As it stands, the 0002\n  one does not apply to my tree.\n\n* The commit messages are useless.  The second one at least says\n  \"introduce HTTP proxy authentication\" which tells me something about\n  *what it does*, but in German.  It should also tell me why and how and\n  possible other considerations.  See SubmittingPatches.\n\n* The commits should \"spring into perfect existence\".  That is, in the\n  above you should squash 3 and 4 into 2 so that 2 starts out working\n  (presumably -- I'm only looking at the subjects) .  You can use rebase\n  -i to clean everything up.\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"185073","messageId":"8762f05n9q.fsf_-_@thomas.inf.ethz.ch","threadId":"29678","inReplyTo":"4CDEC141B5583D408E79F2931DB7708301802BE7@GSX300A.mxchg.m.corp","subject":"Re: Patchset NTLM-Authentication","fromName":"Thomas Rast","fromEmail":"trast@inf.ethz.ch","sentAt":"2012-02-21T13:28:17Z","receivedAt":"2012-02-21T13:28:17Z","isPatch":false,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"\"Schmidt, Marco\" <Marco.Schmidt@cassidian.com> writes:\n\n> Thanks for your reply. I have used the wereHamster bundler service and\n> created the following patch. May I ask you to test the patch and check\n> again for more style violations?\n\nPlease keep these discussions public, so that others can learn and\n(heaven forbid...) point out _my_ mistakes.\n\nI'll discuss the file you attached below.  Keep in mind that unless\nthere is a compelling reason not to do that, you should send your\npatches inline.  This allows for much easier review on the list.\n\n> Subject: [PATCH] Allow NTLM-Authentication against a http-proxy server - Add\n>  config option \"http.proxyauthany\" - Set CURLOPT_PROXYAUTH\n>  to CURLAUTH_ANY if option curl_http_proxyauthany is true.\n> \n> ---\n\nYou can already see that you did not adhere to the standard format of a\ncommit message: one line summary, followed by a blank line, followed by\na long description.  So the above might have been written (dropping the\nSubject: pseudoheader tag now):\n\n} Allow NTLM-Authentication against a http-proxy server\n} \n} - Add config option \"http.proxyauthany\"\n} \n} - Set CURLOPT_PROXYAUTH to CURLAUTH_ANY if option\n}   curl_http_proxyauthany is true.\n\nNow bear in mind that I have no clue about curl (except that it\ndownloads stuff ;-) and especially about its authentication parts.  But\nallow me to sketch a different commit message that highlights what I\nwould like to see answered.  I am merely using the knowledge I could\nglean from 2min of googling around; I am not saying that anything of it\nis correct, so you should fix it as needed!\n\n  http/curl: let user configure \"any\" proxy authentication\n\n  Normally, curl uses only the \"basic\" authentication scheme when\n  talking to proxies, which may not be desirable (it sends the password\n  in cleartext) or sufficient (the author needs NTLM authentication for\n  his proxy).\n\n  Introduce the config setting http.proxyAuthAny.  When enabled, we tell\n  curl to use any authentication scheme supported by the proxy.\n\n  This mostly parallels http.authAny which was introduced in b8ac923\n  (Add an option for using any HTTP authentication scheme, not only\n  basic, 2009-11-27).  http.authAny was removed, and its feature\n  unconditionally enabled, in 525ecd2 (Remove http.authAny, 2009-12-28).\n  However the reasoning of the latter does not apply here because XXXX.\n\n\nSome notes on the code/patch: There was some trailing whitespace, please\nmake sure you remove it.\n\nUsually we introduce a setting (command line option or environment\nvariable) that can be tweaked to override the configuration.  b8ac923\ndid this if you need some inspiration :-)  Otherwise you should make an\nargument why this is not needed.\n\n +#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY\n> +\tif (curl_http_proxyauthany) {\n> +\t\tcurl_easy_setopt(result, CURLOPT_PROXYAUTH, CURLAUTH_ANY);\n> +\t}\n> +#endif\n\nMy google is failing me as to when this feature was introduced or how\ncommon curls without it still are.  But you could be extra doubly nice\nto the user and warn if s/he enabled the feature, but git cannot adhere\nto the request because curl doesn't support it.  (Then again b8ac923\ndidn't go that far either.)\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"185091","messageId":"7vfwe4gj4n.fsf@alter.siamese.dyndns.org","threadId":"29678","inReplyTo":"8762f05n9q.fsf_-_@thomas.inf.ethz.ch","subject":"Re: Patchset NTLM-Authentication","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-02-21T18:02:16Z","receivedAt":"2012-02-21T18:02:16Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thomas Rast <trast@inf.ethz.ch> writes:\n\n>   This mostly parallels http.authAny which was introduced in b8ac923\n>   (Add an option for using any HTTP authentication scheme, not only\n>   basic, 2009-11-27).  http.authAny was removed, and its feature\n>   unconditionally enabled, in 525ecd2 (Remove http.authAny, 2009-12-28).\n>   However the reasoning of the latter does not apply here because XXXX.\n\nThanks, Thomas.\n\nI think this paragraph is essential, especially the XXXX part, if we were\nto accept the proposed change and keep the new configuration.  Otherwise\nwe won't know what to do when somebody proposes to unconditionally enable\nthis ;-)\n\nIf it turns out that we can set CURLOPT_PROXYAUTH always to CURLAUTH_ANY\nwithout compromising security, then an explanation why this does not have\nto be optional, similar to what justified 525ecd2, needs to be there\ninstead, and the patch needs to be tweaked to drop the configuration bits.\n\nMarco, I extracted your patch in the attachment and took a look at it\nbefore composing the above response.\n\n - Your log message seems to be indented by two spaces for some strange\n   reason;\n\n - it does not have any justification like the example Thomas gave\n   you; and\n\n - it also is missing your S-o-b.\n\nCare to re-roll one more time?\n"},{"id":"185094","messageId":"alpine.DEB.2.00.1202211954160.5167@tvnag.unkk.fr","threadId":"29678","inReplyTo":"7vfwe4gj4n.fsf@alter.siamese.dyndns.org","subject":"Re: Patchset NTLM-Authentication","fromName":"Daniel Stenberg","fromEmail":"daniel@haxx.se","sentAt":"2012-02-21T19:02:34Z","receivedAt":"2012-02-21T19:02:34Z","isPatch":false,"sender":{"key":"daniel@haxx.se","avatar":"https://gravatar.com/avatar/69fdca87edd17cee21ca2e79fc2ff671d644603c3dc27167430f3cd3dbab7ba8?d=mp&s=160"},"body":"On Tue, 21 Feb 2012, Junio C Hamano wrote:\n\n> If it turns out that we can set CURLOPT_PROXYAUTH always to CURLAUTH_ANY \n> without compromising security, then an explanation why this does not have to \n> be optional, similar to what justified 525ecd2, needs to be there instead, \n> and the patch needs to be tweaked to drop the configuration bits.\n\nAllow me to provide some libcurl info on this!\n\nSetting it to ANY will unconditionally cause an extra roundtrip which you can \navoid if you know what auth type the proxy wants and you set it at once. With \nANY set, libcurl will first \"probe\" the proxy to figure out which type to use \nand then go on and actually do it in a second request (and possibly even a \nthird request in some cases).\n\nIt can actually be seen as a security _improvement_ in some cases where for \nexample Basic auth (user+password sent as plain text) can be avoided in \npreference to a more secure mechanism, but I think that's a rather rare case \nfor git.\n\nIMO, if ANY is considered fine for normal host authentication I think it could \nbe considered fine for proxy authentication as well.\n\n-- \n\n  / daniel.haxx.se\n"}]}