{"thread":{"id":"59256","subject":"Bug: git behind proxy is broken in 2.34.1","startedAt":"2023-02-16T18:03:52Z","lastAt":"2023-02-17T00:43:49Z","messageCount":5,"participants":["tm-uzr3z@entrap.de","Jeff King","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"472213","messageId":"c0862f86061c220af289ce812d57f9c7.squirrel@www.entrap.de","threadId":"59256","inReplyTo":null,"subject":"Bug: git behind proxy is broken in 2.34.1","fromName":"","fromEmail":"tm-uzr3z@entrap.de","sentAt":"2023-02-16T17:55:42Z","receivedAt":"2023-02-16T18:03:52Z","isPatch":false,"sender":{"key":"tm-uzr3z@entrap.de","avatar":null},"body":"Hi all,\n\nI just realized that git cloning behind a webproxy is broken in version\n2.34.1 (Ubuntu 22.04).\n\nThe environment bash variables http_proxy, https_proxy, HTTP_PROXY and\nHTTPS_PROXY are all set with the value\n\"http://myusername:mypassword@ourwebproxy:3128/\".\n\ngit gives me the following error message on cloning:\n$git clone https://github.com/XXXX/YYYY\nfatal: unable to access 'https://github.com/XXXX/YYYY/': Received HTTP\ncode 407 from proxy after CONNECT\n\nFor example wget or curl http/https requests in the same shell work\nwithout any issues and use the same proxy settings from the environment\nvariables.\n\nIn pcap traces I see that git requests the URL through the proxy, receives\nan http 407 authentication required, and repeats the same request again\nwithout credentials, which gets denied a second time.\n\nOn another very old machine with git 1.9.1 it requests the URL through the\nproxy, receives an http 407 authentication required, and repeats the\nrequest with credentials, which is allowed then.\n\nEven with git config --global and --system variables http.proxy and\nhttps.proxy the authentication required reply is ignored.\n\nThe git configuration is all default, except the proxy variables.\nHope this helps to reproduce and fix this issue. Thank you!\n\nBest regards\nHolger\n\n"},{"id":"472226","messageId":"Y+6WOF+OHX4u94Z2@coredump.intra.peff.net","threadId":"59256","inReplyTo":"c0862f86061c220af289ce812d57f9c7.squirrel@www.entrap.de","subject":"Re: Bug: git behind proxy is broken in 2.34.1","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-02-16T20:46:48Z","receivedAt":"2023-02-16T20:46:52Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Feb 16, 2023 at 06:55:42PM +0100, tm-uzr3z@entrap.de wrote:\n\n> I just realized that git cloning behind a webproxy is broken in version\n> 2.34.1 (Ubuntu 22.04).\n> \n> The environment bash variables http_proxy, https_proxy, HTTP_PROXY and\n> HTTPS_PROXY are all set with the value\n> \"http://myusername:mypassword@ourwebproxy:3128/\".\n> \n> git gives me the following error message on cloning:\n> $git clone https://github.com/XXXX/YYYY\n> fatal: unable to access 'https://github.com/XXXX/YYYY/': Received HTTP\n> code 407 from proxy after CONNECT\n\nHmm. We do not have any test coverage for proxies at all. I took a stab\nat adding some, and it was not too bad. But...it works!\n\nHowever, I think there may be a number of possible configurations here.\nIn my tests, it is an http proxy over http, so it is issuing a GET with\na full URL in it, rather than using CONNECT.\n\nSo it may be that CONNECT is broken, but http-over-http proxying is not.\nI couldn't get a working CONNECT set up at all (before even worrying\nabout the authentication step).\n\n> On another very old machine with git 1.9.1 it requests the URL through the\n> proxy, receives an http 407 authentication required, and repeats the\n> request with credentials, which is allowed then.\n\nThat could be good news, as that would mean we might be able to bisect\nbetween 1.9.1 and 2.34.1. However, since you said it was an older\nmachine, presumably other packages are old, too. I would not be\nsurprised if the difference is actually in libcurl, not Git.\n\nStill, if you're able to build Git from source and bisect, that would be\nworth trying.\n\n-Peff\n"},{"id":"472227","messageId":"Y+6YgALh6L9m6rSX@coredump.intra.peff.net","threadId":"59256","inReplyTo":"Y+6WOF+OHX4u94Z2@coredump.intra.peff.net","subject":"[PATCH] add basic http proxy tests","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-02-16T20:56:32Z","receivedAt":"2023-02-16T20:56:36Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Feb 16, 2023 at 03:46:48PM -0500, Jeff King wrote:\n\n> Hmm. We do not have any test coverage for proxies at all. I took a stab\n> at adding some, and it was not too bad.\n\nHere is that patch. Even though it didn't lead me anywhere useful in\ndebugging your problem, it may be worth picking up anyway just to get\nbetter test coverage.\n\n-- >8 --\nSubject: [PATCH] add basic http proxy tests\n\nWe do not test our http proxy functionality at all in the test suite, so\nthis is a pretty big blind spot. Let's at least add a basic check that\nwe can go through an authenticating proxy to perform a clone.\n\nA few notes on the implementation:\n\n - I'm using a single apache instance to proxy to itself. This seems to\n work fine in practice, and we can check with a test that this rather\n unusual setup is doing what we expect.\n\n - I've put the proxy tests into their own script, and it's the only\n one which loads the apache proxy config. If any platform can't\n handle this (e.g., doesn't have the right modules), the start_httpd\n step should fail and gracefully skip the rest of the script (but all\n the other http tests in existing scripts will continue to run).\n\n - I used a separate passwd file to make sure we don't ever get\n confused between proxy and regular auth credentials. It's using the\n antiquated crypt() format. This is a terrible choice security-wise\n in the modern age, but it's what our existing passwd file uses, and\n should be portable. It would probably be reasonable to switch both\n of these to bcrypt, but we can do that in a separate patch.\n\n - On the client side, we test two situations with credentials: when\n they are present in the url, and when the username is present but we\n prompt for the password. I think we should be able to handle the\n case that _neither_ is present, but an HTTP 407 causes us to prompt\n for them. However, this doesn't seem to work. That's either a bug,\n or at the very least an opportunity for a feature, but I punted on\n it for now. The point of this patch is just getting basic coverage,\n and we can explore possible deficiencies later.\n\n - this doesn't work with LIB_HTTPD_SSL. This probably would be\n valuable to have, as https over an http proxy is totally different\n (it uses CONNECT to tunnel the session). But adding in\n mod_proxy_connect and some basic config didn't seem to work for me,\n so I punted for now. Much of the rest of the test suite does not\n currently work with LIB_HTTPD_SSL either, so we shouldn't be making\n anything much worse here.\n\nSigned-off-by: Jeff King \n---\n t/lib-httpd.sh | 7 +++++++\n t/lib-httpd/apache.conf | 16 ++++++++++++++++\n t/lib-httpd/proxy-passwd | 1 +\n t/t5563-http-proxy.sh | 41 ++++++++++++++++++++++++++++++++++++++++\n 4 files changed, 65 insertions(+)\n create mode 100644 t/lib-httpd/proxy-passwd\n create mode 100755 t/t5563-http-proxy.sh\n\ndiff --git a/t/lib-httpd.sh b/t/lib-httpd.sh\nindex 5d2d56c445..059fd74adb 100644\n--- a/t/lib-httpd.sh\n+++ b/t/lib-httpd.sh\n@@ -25,6 +25,7 @@\n # LIB_HTTPD_DAV enable DAV\n # LIB_HTTPD_SVN enable SVN at given location (e.g. \"svn\")\n # LIB_HTTPD_SSL enable SSL\n+# LIB_HTTPD_PROXY enable proxy\n #\n # Copyright (c) 2008 Clemens Buchacher \n #\n@@ -133,6 +134,7 @@ install_script () {\n prepare_httpd() {\n \tmkdir -p \"$HTTPD_DOCUMENT_ROOT_PATH\"\n \tcp \"$TEST_PATH\"/passwd \"$HTTPD_ROOT_PATH\"\n+\tcp \"$TEST_PATH\"/proxy-passwd \"$HTTPD_ROOT_PATH\"\n \tinstall_script incomplete-length-upload-pack-v2-http.sh\n \tinstall_script incomplete-body-upload-pack-v2-http.sh\n \tinstall_script error-no-report.sh\n@@ -176,6 +178,11 @@ prepare_httpd() {\n \t\t\texport LIB_HTTPD_SVN LIB_HTTPD_SVNPATH\n \t\tfi\n \tfi\n+\n+\tif test -n \"$LIB_HTTPD_PROXY\"\n+\tthen\n+\t\tHTTPD_PARA=\"$HTTPD_PARA -DPROXY\"\n+\tfi\n }\n \n enable_http2 () {\ndiff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf\nindex 51a4fbcf62..e31293a45f 100644\n--- a/t/lib-httpd/apache.conf\n+++ b/t/lib-httpd/apache.conf\n@@ -47,6 +47,22 @@ Protocols h2c\n \tLoadModule authz_host_module modules/mod_authz_host.so\n \n \n+\n+\n+\tLoadModule proxy_module modules/mod_proxy.so\n+\n+\n+\tLoadModule proxy_http_module modules/mod_proxy_http.so\n+\n+ProxyRequests On\n+\n+\tAuthType Basic\n+\tAuthName \"proxy-auth\"\n+\tAuthUserFile proxy-passwd\n+\tRequire valid-user\n+\n+\n+\n \n \tLoadModule authn_core_module modules/mod_authn_core.so\n \ndiff --git a/t/lib-httpd/proxy-passwd b/t/lib-httpd/proxy-passwd\nnew file mode 100644\nindex 0000000000..77c25138e0\n--- /dev/null\n+++ b/t/lib-httpd/proxy-passwd\n@@ -0,0 +1 @@\n+proxuser:2x7tAukjAED5M\ndiff --git a/t/t5563-http-proxy.sh b/t/t5563-http-proxy.sh\nnew file mode 100755\nindex 0000000000..9da5134614\n--- /dev/null\n+++ b/t/t5563-http-proxy.sh\n@@ -0,0 +1,41 @@\n+#!/bin/sh\n+\n+test_description=\"test fetching through http proxy\"\n+\n+. ./test-lib.sh\n+. \"$TEST_DIRECTORY\"/lib-httpd.sh\n+\n+LIB_HTTPD_PROXY=1\n+start_httpd\n+\n+test_expect_success 'setup repository' '\n+\ttest_commit foo &&\n+\tgit init --bare \"$HTTPD_DOCUMENT_ROOT_PATH/repo.git\" &&\n+\tgit push --mirror \"$HTTPD_DOCUMENT_ROOT_PATH/repo.git\"\n+'\n+\n+setup_askpass_helper\n+\n+# sanity check that our test setup is correctly using proxy\n+test_expect_success 'proxy requires password' '\n+\ttest_config_global http.proxy $HTTPD_DEST &&\n+\ttest_must_fail git clone $HTTPD_URL/smart/repo.git 2>err &&\n+\tgrep \"error.*407\" err\n+'\n+\n+test_expect_success 'clone through proxy with auth' '\n+\ttest_when_finished \"rm -rf clone\" &&\n+\ttest_config_global http.proxy http://proxuser:proxpass@$HTTPD_DEST &&\n+\tGIT_TRACE_CURL=$PWD/trace git clone $HTTPD_URL/smart/repo.git clone &&\n+\tgrep -i \"Proxy-Authorization: Basic \" trace\n+'\n+\n+test_expect_success 'clone can prompt for proxy password' '\n+\ttest_when_finished \"rm -rf clone\" &&\n+\ttest_config_global http.proxy http://proxuser@$HTTPD_DEST &&\n+\tset_askpass nobody proxpass &&\n+\tGIT_TRACE_CURL=$PWD/trace git clone $HTTPD_URL/smart/repo.git clone &&\n+\texpect_askpass pass proxuser\n+'\n+\n+test_done\n-- \n2.39.2.920.gf3beb43ccf\n\n"},{"id":"472240","messageId":"xmqqttzlm8uz.fsf@gitster.g","threadId":"59256","inReplyTo":"Y+6YgALh6L9m6rSX@coredump.intra.peff.net","subject":"Re: [PATCH] add basic http proxy tests","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-02-17T00:30:28Z","receivedAt":"2023-02-17T00:30:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Here is that patch. Even though it didn't lead me anywhere useful in\n> debugging your problem, it may be worth picking up anyway just to get\n> better test coverage.\n> ...\n> - I'm using a single apache instance to proxy to itself. This seems to\n> work fine in practice, and we can check with a test that this rather\n> unusual setup is doing what we expect.\n\nCute.\n\n> - I've put the proxy tests into their own script, and it's the only\n> one which loads the apache proxy config. If any platform can't\n> handle this (e.g., doesn't have the right modules), the start_httpd\n> step should fail and gracefully skip the rest of the script (but all\n> the other http tests in existing scripts will continue to run).\n\nNice. I have to move it from 5563 to 5564, though.\n\n> - On the client side, we test two situations with credentials: when\n> they are present in the url, and when the username is present but we\n> prompt for the password. I think we should be able to handle the\n> case that _neither_ is present, but an HTTP 407 causes us to prompt\n> for them. However, this doesn't seem to work. That's either a bug,\n> or at the very least an opportunity for a feature, but I punted on\n> it for now. The point of this patch is just getting basic coverage,\n> and we can explore possible deficiencies later.\n\nOK.\n\nThanks.\n"},{"id":"472242","messageId":"Y+7NwQer53hP9ekm@coredump.intra.peff.net","threadId":"59256","inReplyTo":"xmqqttzlm8uz.fsf@gitster.g","subject":"Re: [PATCH] add basic http proxy tests","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-02-17T00:43:45Z","receivedAt":"2023-02-17T00:43:49Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Feb 16, 2023 at 04:30:28PM -0800, Junio C Hamano wrote:\n\n> > - I've put the proxy tests into their own script, and it's the only\n> > one which loads the apache proxy config. If any platform can't\n> > handle this (e.g., doesn't have the right modules), the start_httpd\n> > step should fail and gracefully skip the rest of the script (but all\n> > the other http tests in existing scripts will continue to run).\n> \n> Nice. I have to move it from 5563 to 5564, though.\n\nAh, yeah. These number collisions are annoying.\n\nAlso, I note that our http client tests have spilled over into t556x\nnow, making the t556x_common script a bit of a misnomer (it handles\nhttp-backend stuff). I'm not sure if we should be reordering more (which\nis a lot of hassle) or perhaps just giving that helper a script a more\ndescriptive name. Or doing nothing. It is not that big a deal, just\nsomething I noticed while searching for an unused number.\n\n-Peff\n"}]}