summaryrefslogtreecommitdiff
path: root/http.c
AgeCommit message (Collapse)AuthorFilesLines
2021-11-25run-command API users: use strvec_pushv(), not argv assignmentLibravatar Ævar Arnfjörð Bjarmason1-2/+3
Migrate those run-command API users that assign directly to the "argv" member to use a strvec_pushv() of "args" instead. In these cases it did not make sense to further refactor these callers, e.g. daemon.c could be made to construct the arguments closer to handle(), but that would require moving the construction from its cmd_main() and pass "argv" through two intermediate functions. It would be possible for a change like this to introduce a regression if we were doing: cp.argv = argv; argv[1] = "foo"; And changed the code, as is being done here, to: strvec_pushv(&cp.args, argv); argv[1] = "foo"; But as viewing this change with the "-W" flag reveals none of these functions modify variable that's being pushed afterwards in a way that would introduce such a logic error. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-10-18Merge branch 'ab/designated-initializers-more'Libravatar Junio C Hamano1-1/+1
Code clean-up. * ab/designated-initializers-more: builtin/remote.c: add and use SHOW_INFO_INIT builtin/remote.c: add and use a REF_STATES_INIT urlmatch.[ch]: add and use URLMATCH_CONFIG_INIT builtin/blame.c: refactor commit_info_init() to COMMIT_INFO_INIT macro daemon.c: refactor hostinfo_init() to HOSTINFO_INIT macro
2021-10-11Merge branch 'ab/http-pinned-public-key-mismatch'Libravatar Junio C Hamano1-0/+4
HTTPS error handling updates. * ab/http-pinned-public-key-mismatch: http: check CURLE_SSL_PINNEDPUBKEYNOTMATCH when emitting errors
2021-10-03Merge branch 'jk/http-redact-fix'Libravatar Junio C Hamano1-3/+3
Sensitive data in the HTTP trace were supposed to be redacted, but we failed to do so in HTTP/2 requests. * jk/http-redact-fix: http: match headers case-insensitively when redacting
2021-10-01urlmatch.[ch]: add and use URLMATCH_CONFIG_INITLibravatar Ævar Arnfjörð Bjarmason1-1/+1
Change the initialization pattern of "struct urlmatch_config" to use an *_INIT macro and designated initializers. Right now there's no other "struct" member of "struct urlmatch_config" which would require its own *_INIT, but it's good practice not to assume that. Let's also change this to a designated initializer while we're at it. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-09-27http: check CURLE_SSL_PINNEDPUBKEYNOTMATCH when emitting errorsLibravatar Ævar Arnfjörð Bjarmason1-0/+4
Change the error shown when a http.pinnedPubKey doesn't match to point the http.pinnedPubKey variable added in aeff8a61216 (http: implement public key pinning, 2016-02-15), e.g.: git -c http.pinnedPubKey=sha256/someNonMatchingKey ls-remote https://github.com/git/git.git fatal: unable to access 'https://github.com/git/git.git/' with http.pinnedPubkey configuration: SSL: public key does not match pinned public key! Before this we'd emit the exact same thing without the " with http.pinnedPubkey configuration". The advantage of doing this is that we're going to get a translated message (everything after the ":" is hardcoded in English in libcurl), and we've got a reference to the git-specific configuration variable that's causing the error. Unfortunately we can't test this easily, as there are no tests that require https:// in the test suite, and t/lib-httpd.sh doesn't know how to set up such tests. See [1] for the start of a discussion about what it would take to have divergent "t/lib-httpd/apache.conf" test setups. #leftoverbits 1. https://lore.kernel.org/git/YUonS1uoZlZEt+Yd@coredump.intra.peff.net/ Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-09-22http: match headers case-insensitively when redactingLibravatar Jeff King1-3/+3
When HTTP/2 is in use, we fail to correctly redact "Authorization" (and other) headers in our GIT_TRACE_CURL output. We get the headers in our CURLOPT_DEBUGFUNCTION callback, curl_trace(). It passes them along to curl_dump_header(), which in turn checks redact_sensitive_header(). We see the headers as a text buffer like: Host: ... Authorization: Basic ... After breaking it into lines, we match each header using skip_prefix(). This is case-sensitive, even though HTTP headers are case-insensitive. This has worked reliably in the past because these headers are generated by curl itself, which is predictable in what it sends. But when HTTP/2 is in use, instead we get a lower-case "authorization:" header, and we fail to match it. The fix is simple: we should match with skip_iprefix(). Testing is more complicated, though. We do have a test for the redacting feature, but we don't hit the problem case because our test Apache setup does not understand HTTP/2. You can reproduce the issue by applying this on top of the test change in this patch: diff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf index afa91e38b0..19267c7107 100644 --- a/t/lib-httpd/apache.conf +++ b/t/lib-httpd/apache.conf @@ -29,6 +29,9 @@ ErrorLog error.log LoadModule setenvif_module modules/mod_setenvif.so </IfModule> +LoadModule http2_module modules/mod_http2.so +Protocols h2c + <IfVersion < 2.4> LockFile accept.lock </IfVersion> @@ -64,8 +67,8 @@ LockFile accept.lock <IfModule !mod_access_compat.c> LoadModule access_compat_module modules/mod_access_compat.so </IfModule> -<IfModule !mod_mpm_prefork.c> - LoadModule mpm_prefork_module modules/mod_mpm_prefork.so +<IfModule !mod_mpm_event.c> + LoadModule mpm_event_module modules/mod_mpm_event.so </IfModule> <IfModule !mod_unixd.c> LoadModule unixd_module modules/mod_unixd.so diff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh index 1c2a444ae7..ff74f0ae8a 100755 --- a/t/t5551-http-fetch-smart.sh +++ b/t/t5551-http-fetch-smart.sh @@ -24,6 +24,10 @@ test_expect_success 'create http-accessible bare repository' ' git push public main:main ' +test_expect_success 'prefer http/2' ' + git config --global http.version HTTP/2 +' + setup_askpass_helper test_expect_success 'clone http repository' ' but this has a few issues: - it's not necessarily portable. The http2 apache module might not be available on all systems. Further, the http2 module isn't compatible with the prefork mpm, so we have to switch to something else. But we don't necessarily know what's available. It would be nice if we could have conditional config, but IfModule only tells us if a module is already loaded, not whether it is available at all. This might be a non-issue. The http tests are already optional, and modern-enough systems may just have both of these. But... - if we do this, then we'd no longer be testing HTTP/1.1 at all. I'm not sure how much that matters since it's all handled by curl under the hood, but I'd worry that some detail leaks through. We'd probably want two scripts running similar tests, one with HTTP/2 and one with HTTP/1.1. - speaking of which, a later test fails with the patch above! The problem is that it is making sure we used a chunked transfer-encoding by looking for that header in the trace. But HTTP/2 doesn't support that, as it has its own streaming mechanisms (the overall operation works fine; we just don't see the header in the trace). Furthermore, even with the changes above, this test still does not detect the current failure, because we see _both_ HTTP/1.1 and HTTP/2 requests, which confuse it. Quoting only the interesting bits from the resulting trace file, we first see: => Send header: GET /auth/smart/repo.git/info/refs?service=git-upload-pack HTTP/1.1 => Send header: Connection: Upgrade, HTTP2-Settings => Send header: Upgrade: h2c => Send header: HTTP2-Settings: AAMAAABkAAQCAAAAAAIAAAAA <= Recv header: HTTP/1.1 401 Unauthorized <= Recv header: Date: Wed, 22 Sep 2021 20:03:32 GMT <= Recv header: Server: Apache/2.4.49 (Debian) <= Recv header: WWW-Authenticate: Basic realm="git-auth" So the client asks for HTTP/2, but Apache does not do the upgrade for the 401 response. Then the client repeats with credentials: => Send header: GET /auth/smart/repo.git/info/refs?service=git-upload-pack HTTP/1.1 => Send header: Authorization: Basic <redacted> => Send header: Connection: Upgrade, HTTP2-Settings => Send header: Upgrade: h2c => Send header: HTTP2-Settings: AAMAAABkAAQCAAAAAAIAAAAA <= Recv header: HTTP/1.1 101 Switching Protocols <= Recv header: Upgrade: h2c <= Recv header: Connection: Upgrade <= Recv header: HTTP/2 200 <= Recv header: content-type: application/x-git-upload-pack-advertisement So the client does properly redact there, because we're speaking HTTP/1.1, and the server indicates it can do the upgrade. And then the client will make further requests using HTTP/2: => Send header: POST /auth/smart/repo.git/git-upload-pack HTTP/2 => Send header: authorization: Basic dXNlckBob3N0OnBhc3NAaG9zdA== => Send header: content-type: application/x-git-upload-pack-request And there we can see that the credential is _not_ redacted. This part of the test is what gets confused: # Ensure that there is no "Basic" followed by a base64 string, but that # the auth details are redacted ! grep "Authorization: Basic [0-9a-zA-Z+/]" trace && grep "Authorization: Basic <redacted>" trace The first grep does not match the un-redacted HTTP/2 header, because it insists on an uppercase "A". And the second one does find the HTTP/1.1 header. So as far as the test is concerned, everything is OK, but it failed to notice the un-redacted lines. We can make this test (and the other related ones) more robust by adding "-i" to grep case-insensitively. This isn't really doing anything for now, since we're not actually speaking HTTP/2, but it future-proofs the tests for a day when we do (either we add explicit HTTP/2 test support, or it's eventually enabled by default by our Apache+curl test setup). And it doesn't hurt in the meantime for the tests to be more careful. The change to use "grep -i", coupled with the changes to use HTTP/2 shown above, causes the test to fail with the current code, and pass after this patch is applied. And finally, there's one other way to demonstrate the issue (and how I actually found it originally). Looking at GIT_TRACE_CURL output against github.com, you'll see the unredacted output, even if you didn't set http.version. That's because setting it is only necessary for curl to send the extra headers in its HTTP/1.1 request that say "Hey, I speak HTTP/2; upgrade if you do, too". But for a production site speaking https, the server advertises via ALPN, a TLS extension, that it supports HTTP/2, and the client can immediately start using it. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-09-13http: don't hardcode the value of CURL_SOCKOPT_OKLibravatar Ævar Arnfjörð Bjarmason1-1/+1
Use the new git-curl-compat.h header to define CURL_SOCKOPT_OK to its known value if we're on an older curl version that doesn't have it. It was hardcoded in http.c in a15d069a198 (http: enable keepalive on TCP sockets, 2013-10-12). Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-09-13http: centralize the accounting of libcurl dependenciesLibravatar Ævar Arnfjörð Bjarmason1-14/+15
As discussed in 644de29e220 (http: drop support for curl < 7.19.4, 2021-07-30) checking against LIBCURL_VERSION_NUM isn't as reliable as checking specific symbols present in curl, as some distros have been known to backport features. However, while some of the curl_easy_setopt() arguments we rely on are macros, others are enum, and we can't assume that those that are macros won't change into enums in the future. So we're still going to have to check LIBCURL_VERSION_NUM, but by doing that in one central place and using a macro definition of our own, anyone who's backporting features can define it themselves, and thus have access to more modern curl features that they backported, even if they didn't bump the LIBCURL_VERSION_NUM. More importantly, as shown in a preceding commit doing these version checks makes for hard to read and possibly buggy code, as shown by the bug fixed there where we were conflating base 10 for base 16 when comparing the version. By doing them all in one place we'll hopefully reduce the chances of such future mistakes, furthermore it now becomes easier to see at a glance what the oldest supported version is, which makes it easier to reason about any future deprecation similar to the recent e48a623dea0 (Merge branch 'ab/http-drop-old-curl', 2021-08-24). Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-09-13http: correct curl version check for CURLOPT_PINNEDPUBLICKEYLibravatar Ævar Arnfjörð Bjarmason1-4/+4
In aeff8a61216 (http: implement public key pinning, 2016-02-15) a dependency and warning() was added if curl older than 7.44.0 was used, but the relevant code depended on CURLOPT_PINNEDPUBLICKEY, introduced in 7.39.0. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-09-13http: correct version check for CURL_HTTP_VERSION_2Libravatar Ævar Arnfjörð Bjarmason1-2/+2
In d73019feb44 (http: add support selecting http version, 2018-11-08) a dependency was added on CURL_HTTP_VERSION_2, but this feature was introduced in curl version 7.43.0, not 7.47.0, as the incorrect version check led us to believe. As looking through the history of that commit on the mailing list will reveal[1], the reason for this is that an earlier version of it depended on CURL_HTTP_VERSION_2TLS, which was introduced in libcurl 7.47.0. But the version that made it in in d73019feb44 had dropped the dependency on CURL_HTTP_VERSION_2TLS, but the corresponding version check was not corrected. The newest symbol we depend on is CURL_HTTP_VERSION_2. It was added in 7.33.0, but the CURL_HTTP_VERSION_2 alias we used was added in 7.47.0. So we could support an even older version here, but let's just correct the checked version. 1. https://lore.kernel.org/git/pull.69.git.gitgitgadget@gmail.com/ Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-09-13http: drop support for curl < 7.18.0 (again)Libravatar Ævar Arnfjörð Bjarmason1-2/+0
In 644de29e220 (http: drop support for curl < 7.19.4, 2021-07-30) we dropped support for curl < 7.19.4, so we can drop support for this non-obvious dependency on curl < 7.18.0. It's non-obvious because in curl's hex version notation 0x071800 is version 7.24.0, *not* 7.18.0, so at a glance this patch looks incorrect. But it's correct, because the existing version check being removed here is wrong. The check guards use of the following curl defines: CURLPROXY_SOCKS4 7.10 CURLPROXY_SOCKS4A 7.18.0 CURLPROXY_SOCKS5 7.10 CURLPROXY_SOCKS5_HOSTNAME 7.18.0 I.e. the oldest version that has these is in fact 7.18.0, not 7.24.0. That we were checking 7.24.0 is just an mistake in 6d7afe07f29 (remote-http(s): support SOCKS proxies, 2015-10-26), i.e. its author confusing base 10 and base 16. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-07-30http: rename CURLOPT_FILE to CURLOPT_WRITEDATALibravatar Ævar Arnfjörð Bjarmason1-3/+3
The CURLOPT_FILE name is an alias for CURLOPT_WRITEDATA, the CURLOPT_WRITEDATA name has been preferred since curl 7.9.7, released in May 2002[1]. 1. https://curl.se/libcurl/c/CURLOPT_WRITEDATA.html Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-07-30http: drop support for curl < 7.19.3 and < 7.17.0 (again)Libravatar Ævar Arnfjörð Bjarmason1-4/+0
Remove the conditional use of CURLAUTH_DIGEST_IE and CURLOPT_USE_SSL. These two have been split from earlier simpler checks against LIBCURL_VERSION_NUM for ease of review. According to https://github.com/curl/curl/blob/master/docs/libcurl/symbols-in-versions the CURLAUTH_DIGEST_IE flag became available in 7.19.3, and CURLOPT_USE_SSL in 7.17.0. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-07-30http: drop support for curl < 7.19.4Libravatar Jeff King1-50/+0
In the last commit we dropped support for curl < 7.16.0, let's continue that and drop support for versions older than 7.19.3. This allows us to simplify the code by getting rid of some "#ifdef"'s. Git was broken with vanilla curl < 7.19.4 from v2.12.0 until v2.15.0. Compiling with it was broken by using CURLPROTO_* outside any "#ifdef" in aeae4db174 (http: create function to get curl allowed protocols, 2016-12-14), and fixed in v2.15.0 in f18777ba6ef (http: fix handling of missing CURLPROTO_*, 2017-08-11). It's unclear how much anyone was impacted by that in practice, since as noted in [1] RHEL versions using curl older than that still compiled, because RedHat backported some features. Perhaps other vendors did the same. Still, it's one datapoint indicating that it wasn't in active use at the time. That (the v2.12.0 release) was in Feb 24, 2017, with v2.15.0 on Oct 30, 2017, it's now mid-2021. 1. http://lore.kernel.org/git/c8a2716d-76ac-735c-57f9-175ca3acbcb0@jupiterrise.com; followed-up by f18777ba6ef (http: fix handling of missing CURLPROTO_*, 2017-08-11) Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-07-30http: drop support for curl < 7.16.0Libravatar Jeff King1-55/+1
In the last commit we dropped support for curl < 7.11.1, let's continue that and drop support for versions older than 7.16.0. This allows us to get rid of some now-obsolete #ifdefs. Choosing 7.16.0 is a somewhat arbitrary cutoff: 1. It came out in October of 2006, almost 15 years ago. Besides being a nice round number, around 10 years is a common end-of-life support period, even for conservative distributions. 2. That version introduced the curl_multi interface, which gives us a lot of bang for the buck in removing #ifdefs RHEL 5 came with curl 7.15.5[1] (released in August 2006). RHEL 5's extended life cycle program ended on 2020-11-30[1]. RHEL 6 comes with curl 7.19.7 (released in November 2009), and RHEL 7 comes with 7.29.0 (released in February 2013). 1. http://lore.kernel.org/git/873e1f31-2a96-5b72-2f20-a5816cad1b51@jupiterrise.com Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-07-30http: drop support for curl < 7.11.1Libravatar Jeff King1-53/+0
Drop support for this ancient version of curl and simplify the code by allowing us get rid of some "#ifdef"'s. Git will not build with vanilla curl older than 7.11.1 due our use of CURLOPT_POSTFIELDSIZE in 37ee680d9b (http.postbuffer: allow full range of ssize_t values, 2017-04-11). This field was introduced in curl 7.11.1. We could solve these compilation problems with more #ifdefs, but it's not worth the trouble. Version 7.11.1 came out in March of 2004, over 17 years ago. Let's declare that too old and drop any existing ifdefs that go further back. One obvious benefit is that we'll have fewer conditional bits cluttering the code. This patch drops all #ifdefs that reference older versions (note that curl's preprocessor macros are in hex, so we're looking for 070b01, not 071101). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-05-21Merge branch 'cs/http-use-basic-after-failed-negotiate'Libravatar Junio C Hamano1-8/+7
Regression fix for a change made during this cycle. * cs/http-use-basic-after-failed-negotiate: Revert "remote-curl: fall back to basic auth if Negotiate fails" t5551: test http interaction with credential helpers
2021-05-19Revert "remote-curl: fall back to basic auth if Negotiate fails"Libravatar Jeff King1-8/+7
This reverts commit 1b0d9545bb85912a16b367229d414f55d140d3be. That commit does fix the situation it intended to (avoiding Negotiate even when the credentials were provided in the URL), but it creates a more serious regression: we now never hit the conditional for "we had a username and password, tried them, but the server still gave us a 401". That has two bad effects: 1. we never call credential_reject(), and thus a bogus credential stored by a helper will live on forever 2. we never return HTTP_NOAUTH, so the error message the user gets is "The requested URL returned error: 401", instead of "Authentication failed". Doing this correctly seems non-trivial, as we don't know whether the Negotiate auth was a problem. Since this is a regression in the upcoming v2.23.0 release (for which we're in -rc0), let's revert for now and work on a fix separately. (Note that this isn't a pure revert; the previous commit added a test showing the regression, so we can now flip it to expect_success). Reported-by: Ben Humphreys <behumphreys@atlassian.com> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-04-27Use the final_oid_fn to finalize hashing of object IDsLibravatar brian m. carlson1-1/+1
When we're hashing a value which is going to be an object ID, we want to zero-pad that value if necessary. To do so, use the final_oid_fn instead of the final_fn anytime we're going to create an object ID to ensure we perform this operation. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-03-30Merge branch 'cs/http-use-basic-after-failed-negotiate'Libravatar Junio C Hamano1-7/+8
When accessing a server with a URL like https://user:pass@site/, we did not to fall back to the basic authentication with the credential material embedded in the URL after the "Negotiate" authentication failed. Now we do. * cs/http-use-basic-after-failed-negotiate: remote-curl: fall back to basic auth if Negotiate fails
2021-03-26Merge branch 'js/http-pki-credential-store'Libravatar Junio C Hamano1-2/+11
The http codepath learned to let the credential layer to cache the password used to unlock a certificate that has successfully been used. * js/http-pki-credential-store: http: drop the check for an empty proxy password before approving http: store credential when PKI auth is used
2021-03-22remote-curl: fall back to basic auth if Negotiate failsLibravatar Christopher Schenk1-7/+8
When the username and password are supplied in a url like this https://myuser:secret@git.exampe/myrepo.git and the server supports the negotiate authenticaten method, git does not fall back to basic auth and libcurl hardly tries to authenticate with the negotiate method. Stop using the Negotiate authentication method after the first failure because if it fails on the first try it will never succeed. Signed-off-by: Christopher Schenk <christopher@cschenk.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-03-13use CALLOC_ARRAYLibravatar René Scharfe1-2/+2
Add and apply a semantic patch for converting code that open-codes CALLOC_ARRAY to use it instead. It shortens the code and infers the element size automatically. Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-03-11http: drop the check for an empty proxy password before approvingLibravatar John Szakmeister1-2/+1
credential_approve() already checks for a non-empty password before saving, so there's no need to do the extra check here. Signed-off-by: John Szakmeister <john@szakmeister.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-03-11http: store credential when PKI auth is usedLibravatar John Szakmeister1-0/+10
We already looked for the PKI credentials in the credential store, but failed to approve it on success. Meaning, the PKI certificate password was never stored and git would request it on every connection to the remote. Let's complete the chain by storing the certificate password on success. Likewise, we also need to reject the credential when there is a failure. Curl appears to report client-related certificate issues are reported with the CURLE_SSL_CERTPROBLEM error. This includes not only a bad password, but potentially other client certificate related problems. Since we cannot get more information from curl, we'll go ahead and reject the credential upon receiving that error, just to be safe and avoid caching or saving a bad password. Signed-off-by: John Szakmeister <john@szakmeister.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-02-22http: allow custom index-pack argsLibravatar Jonathan Tan1-7/+8
Currently, when fetching, packfiles referenced by URIs are run through index-pack without any arguments other than --stdin and --keep, no matter what arguments are used for the packfile that is inline in the fetch response. As a preparation for ensuring that all packs (whether inline or not) use the same index-pack arguments, teach the http subsystem to allow custom index-pack arguments. http-fetch has been updated to use the new API. For now, it passes --keep alone instead of --keep with a process ID, but this is only temporary because http-fetch itself will be taught to accept index-pack parameters (instead of using a hardcoded constant) in a subsequent commit. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-07-28strvec: fix indentation in renamed callsLibravatar Jeff King1-1/+1
Code which split an argv_array call across multiple lines, like: argv_array_pushl(&args, "one argument", "another argument", "and more", NULL); was recently mechanically renamed to use strvec, which results in mis-matched indentation like: strvec_pushl(&args, "one argument", "another argument", "and more", NULL); Let's fix these up to align the arguments with the opening paren. I did this manually by sifting through the results of: git jump grep 'strvec_.*,$' and liberally applying my editor's auto-format. Most of the changes are of the form shown above, though I also normalized a few that had originally used a single-tab indentation (rather than our usual style of aligning with the open paren). I also rewrapped a couple of obvious cases (e.g., where previously too-long lines became short enough to fit on one), but I wasn't aggressive about it. In cases broken to three or more lines, the grouping of arguments is sometimes meaningful, and it wasn't worth my time or reviewer time to ponder each case individually. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-07-28strvec: convert more callers away from argv_array nameLibravatar Jeff King1-3/+3
We eventually want to drop the argv_array name and just use strvec consistently. There's no particular reason we have to do it all at once, or care about interactions between converted and unconverted bits. Because of our preprocessor compat layer, the names are interchangeable to the compiler (so even a definition and declaration using different names is OK). This patch converts remaining files from the first half of the alphabet, to keep the diff to a manageable size. The conversion was done purely mechanically with: git ls-files '*.c' '*.h' | xargs perl -i -pe ' s/ARGV_ARRAY/STRVEC/g; s/argv_array/strvec/g; ' and then selectively staging files with "git add '[abcdefghjkl]*'". We'll deal with any indentation/style fallouts separately. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-06-25Merge branch 'jt/cdn-offload'Libravatar Junio C Hamano1-39/+43
The "fetch/clone" protocol has been updated to allow the server to instruct the clients to grab pre-packaged packfile(s) in addition to the packed object data coming over the wire. * jt/cdn-offload: upload-pack: fix a sparse '0 as NULL pointer' warning upload-pack: send part of packfile response as uri fetch-pack: support more than one pack lockfile upload-pack: refactor reading of pack-objects out Documentation: add Packfile URIs design doc Documentation: order protocol v2 sections http-fetch: support fetching packfiles by URL http-fetch: refactor into function http: refactor finish_http_pack_request() http: use --stdin when indexing dumb HTTP pack
2020-06-10http-fetch: support fetching packfiles by URLLibravatar Jonathan Tan1-7/+21
Teach http-fetch the ability to download packfiles directly, given a URL, and to verify them. The http_pack_request suite has been augmented with a function that takes a URL directly. With this function, the hash is only used to determine the name of the temporary file. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-06-10http: refactor finish_http_pack_request()Libravatar Jonathan Tan1-15/+16
finish_http_pack_request() does multiple tasks, including some housekeeping on a struct packed_git - (1) closing its index, (2) removing it from a list, and (3) installing it. These concerns are independent of fetching a pack through HTTP: they are there only because (1) the calling code opens the pack's index before deciding to fetch it, (2) the calling code maintains a list of packfiles that can be fetched, and (3) the calling code fetches it in order to make use of its objects in the same process. In preparation for a subsequent commit, which adds a feature that does not need any of this housekeeping, remove (1), (2), and (3) from finish_http_pack_request(). (2) and (3) are now done by a helper function, and (1) is the responsibility of the caller (in this patch, done closer to the point where the pack index is opened). Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-06-10http: use --stdin when indexing dumb HTTP packLibravatar Jonathan Tan1-22/+11
When Git fetches a pack using dumb HTTP, (among other things) it invokes index-pack on a ".pack.temp" packfile, specifying the filename as an argument. A future commit will require the aforementioned invocation of index-pack to also generate a "keep" file. To use this, we either have to use index-pack's naming convention (because --keep requires the pack's filename to end with ".pack") or to pass the pack through stdin. Of the two, it is simpler to pass the pack through stdin. Thus, teach http to pass --stdin to index-pack. As a bonus, the code is now simpler. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-06-05http: redact all cookies, teach GIT_TRACE_REDACT=0Libravatar Jonathan Tan1-24/+11
In trace output (when GIT_TRACE_CURL is true), redact the values of all HTTP cookies by default. Now that auth headers (since the implementation of GIT_TRACE_CURL in 74c682d3c6 ("http.c: implement the GIT_TRACE_CURL environment variable", 2016-05-24)) and cookie values (since this commit) are redacted by default in these traces, also allow the user to inhibit these redactions through an environment variable. Since values of all cookies are now redacted by default, GIT_REDACT_COOKIES (which previously allowed users to select individual cookies to redact) now has no effect. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-05-11http, imap-send: stop using CURLOPT_VERBOSELibravatar Jonathan Tan1-1/+7
Whenever GIT_CURL_VERBOSE is set, teach Git to behave as if GIT_TRACE_CURL=1 and GIT_TRACE_CURL_NO_DATA=1 is set, instead of setting CURLOPT_VERBOSE. This is to prevent inadvertent revelation of sensitive data. In particular, GIT_CURL_VERBOSE redacts neither the "Authorization" header nor any cookies specified by GIT_REDACT_COOKIES. Unifying the tracing mechanism also has the future benefit that any improvements to the tracing mechanism will benefit both users of GIT_CURL_VERBOSE and GIT_TRACE_CURL, and we do not need to remember to implement any improvement twice. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-04-19Sync with 2.26.2Libravatar Junio C Hamano1-0/+2
2020-04-19Git 2.26.2Libravatar Jonathan Nieder1-0/+1
This merges up the security fix from v2.17.5. Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
2020-04-19Git 2.25.4Libravatar Jonathan Nieder1-0/+1
This merges up the security fix from v2.17.5. Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
2020-04-19Git 2.24.3Libravatar Jonathan Nieder1-0/+1
This merges up the security fix from v2.17.5. Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
2020-04-19Git 2.22.4Libravatar Jonathan Nieder1-0/+1
This merges up the security fix from v2.17.5. Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
2020-04-19Git 2.21.3Libravatar Jonathan Nieder1-0/+1
This merges up the security fix from v2.17.5. Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
2020-04-19Git 2.20.4Libravatar Jonathan Nieder1-0/+1
This merges up the security fix from v2.17.5. Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
2020-04-19Git 2.19.5Libravatar Jonathan Nieder1-0/+1
This merges up the security fix from v2.17.5. Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
2020-04-19Git 2.18.4Libravatar Jonathan Nieder1-0/+1
This merges up the security fix from v2.17.5. Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
2020-04-19credential: parse URL without host as empty host, not unsetLibravatar Jeff King1-0/+1
We may feed a URL like "cert:///path/to/cert.pem" into the credential machinery to get the key for a client-side certificate. That credential has no hostname field, which is about to be disallowed (to avoid confusion with protocols where a helper _would_ expect a hostname). This means as of the next patch, credential helpers won't work for unlocking certs. Let's fix that by doing two things: - when we parse a url with an empty host, set the host field to the empty string (asking only to match stored entries with an empty host) rather than NULL (asking to match _any_ host). - when we build a cert:// credential by hand, similarly assign an empty string It's the latter that is more likely to impact real users in practice, since it's what's used for http connections. But we don't have good infrastructure to test it. The url-parsing version will help anybody using git-credential in a script, and is easy to test. Signed-off-by: Jeff King <peff@peff.net> Reviewed-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
2020-03-25Merge branch 'js/https-proxy-config'Libravatar Junio C Hamano1-5/+69
A handful of options to configure SSL when talking to proxies have been added. * js/https-proxy-config: http: add environment variable support for HTTPS proxies http: add client cert support for HTTPS proxies
2020-03-05http: add environment variable support for HTTPS proxiesLibravatar Jorge Lopez Silva1-0/+7
Add 4 environment variables that can be used to configure the proxy cert, proxy ssl key, the proxy cert password protected flag, and the CA info for the proxy. Documentation for the options was also updated. Signed-off-by: Jorge Lopez Silva <jalopezsilva@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-03-05http: add client cert support for HTTPS proxiesLibravatar Jorge Lopez Silva1-5/+62
Git supports performing connections to HTTPS proxies, but we don't support doing mutual authentication with them (through TLS). Add the necessary options to be able to send a client certificate to the HTTPS proxy. A client certificate can provide an alternative way of authentication instead of using 'ProxyAuthorization' or other more common methods of authentication. Libcurl supports this functionality already, so changes are somewhat minimal. The feature is guarded by the first available libcurl version that supports these options. 4 configuration options are added and documented, cert, key, cert password protected and CA info. The CA info should be used to specify a different CA path to validate the HTTPS proxy cert. Signed-off-by: Jorge Lopez Silva <jalopezsilva@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-02-10strbuf: add and use strbuf_insertstr()Libravatar René Scharfe1-2/+2
Add a function for inserting a C string into a strbuf. Use it throughout the source to get rid of magic string length constants and explicit strlen() calls. Like strbuf_addstr(), implement it as an inline function to avoid the implicit strlen() calls to cause runtime overhead. Helped-by: Taylor Blau <me@ttaylorr.com> Helped-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-01Merge branch 'cb/curl-use-xmalloc'Libravatar Junio C Hamano1-10/+8
HTTP transport had possible allocator/deallocator mismatch, which has been corrected. * cb/curl-use-xmalloc: remote-curl: unbreak http.extraHeader with custom allocators