summaryrefslogtreecommitdiff
path: root/connect.c
AgeCommit message (Collapse)AuthorFilesLines
2017-09-07connect: release strbuf on error return in git_connect()Libravatar Rene Scharfe1-1/+3
Reduce the scope of the variable cmd and release it before returning early. Signed-off-by: Rene Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-08-04Merge tag 'v2.13.5' into maintLibravatar Junio C Hamano1-0/+11
2017-08-01Merge tag 'v2.12.4' into maintLibravatar Junio C Hamano1-0/+11
2017-07-30Merge tag 'v2.10.4' into maint-2.11Libravatar Junio C Hamano1-0/+11
Git 2.10.4
2017-07-30Merge tag 'v2.9.5' into maint-2.10Libravatar Junio C Hamano1-0/+11
Git 2.9.5
2017-07-30Merge tag 'v2.7.6' into maint-2.8Libravatar Junio C Hamano1-0/+11
Git 2.7.6
2017-07-28connect: reject paths that look like command line optionsLibravatar Jeff King1-0/+3
If we get a repo path like "-repo.git", we may try to invoke "git-upload-pack -repo.git". This is going to fail, since upload-pack will interpret it as a set of bogus options. But let's reject this before we even run the sub-program, since we would not want to allow any mischief with repo names that actually are real command-line options. You can still ask for such a path via git-daemon, but there's no security problem there, because git-daemon enters the repo itself and then passes "." on the command line. Signed-off-by: Jeff King <peff@peff.net> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-07-28connect: reject dashed arguments for proxy commandsLibravatar Jeff King1-0/+5
If you have a GIT_PROXY_COMMAND configured, we will run it with the host/port on the command-line. If a URL contains a mischievous host like "--foo", we don't know how the proxy command may handle it. It's likely to break, but it may also do something dangerous and unwanted (technically it could even do something useful, but that seems unlikely). We should err on the side of caution and reject this before we even run the command. The hostname check matches the one we do in a similar circumstance for ssh. The port check is not present for ssh, but there it's not necessary because the syntax is "-p <port>", and there's no ambiguity on the parsing side. It's not clear whether you can actually get a negative port to the proxy here or not. Doing: git fetch git://remote:-1234/repo.git keeps the "-1234" as part of the hostname, with the default port of 9418. But it's a good idea to keep this check close to the point of running the command to make it clear that there's no way to circumvent it (and at worst it serves as a belt-and-suspenders check). Signed-off-by: Jeff King <peff@peff.net> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-07-28connect: factor out "looks like command line option" checkLibravatar Jeff King1-1/+1
We reject hostnames that start with a dash because they may be confused for command-line options. Let's factor out that notion into a helper function, as we'll use it in more places. And while it's simple now, it's not clear if some systems might need more complex logic to handle all cases. Signed-off-by: Jeff King <peff@peff.net> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-07-28connect: reject ssh hostname that begins with a dashLibravatar Junio C Hamano1-0/+3
When commands like "git fetch" talk with ssh://$rest_of_URL/, the code splits $rest_of_URL into components like host, port, etc., and then spawns the underlying "ssh" program by formulating argv[] array that has: - the path to ssh command taken from GIT_SSH_COMMAND, etc. - dashed options like '-batch' (for Tortoise), '-p <port>' as needed. - ssh_host, which is supposed to be the hostname parsed out of $rest_of_URL. - then the command to be run on the other side, e.g. git upload-pack. If the ssh_host ends up getting '-<anything>', the argv[] that is used to spawn the command becomes something like: { "ssh", "-p", "22", "-<anything>", "command", "to", "run", NULL } which obviously is bogus, but depending on the actual value of "<anything>", will make "ssh" parse and use it as an option. Prevent this by forbidding ssh_host that begins with a "-". Noticed-by: Joern Schneeweisz of Recurity Labs Reported-by: Brian at GitLab Signed-off-by: Junio C Hamano <gitster@pobox.com> Reviewed-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-06-24Merge branch 'bw/config-h'Libravatar Junio C Hamano1-0/+1
Fix configuration codepath to pay proper attention to commondir that is used in multi-worktree situation, and isolate config API into its own header file. * bw/config-h: config: don't implicitly use gitdir or commondir config: respect commondir setup: teach discover_git_directory to respect the commondir config: don't include config.h by default config: remove git_config_iter config: create config.h
2017-06-15config: don't include config.h by defaultLibravatar Brandon Williams1-0/+1
Stop including config.h by default in cache.h. Instead only include config.h in those files which require use of the config system. Signed-off-by: Brandon Williams <bmwill@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-06-13Merge branch 'jk/connect-symref-info-leak-fix' into maintLibravatar Junio C Hamano1-1/+1
Leakfix. * jk/connect-symref-info-leak-fix: connect.c: fix leak in parse_one_symref_info()
2017-06-05Merge branch 'jk/connect-symref-info-leak-fix'Libravatar Junio C Hamano1-1/+1
Leakfix. * jk/connect-symref-info-leak-fix: connect.c: fix leak in parse_one_symref_info()
2017-05-26connect.c: fix leak in parse_one_symref_info()Libravatar Jeff King1-1/+1
If we successfully parse a symref value like "HEAD:refs/heads/master", we add the result to a string list. But because the string list is marked STRING_LIST_INIT_DUP, the string list code will make a copy of the string and add the copy. This patch fixes it by adding the entry with string_list_append_nodup(), which lets the string list take ownership of our newly allocated string. There are two alternatives that seem like they would work, but aren't the right solution. The first is to initialize the list with the "NODUP" initializer. That would avoid the copy, but then the string list would not realize that it owns the strings. When we eventually call string_list_clear(), it would not free the strings, causing a leak. The second option would be to use the normal string_list_append(), but free the local copy in our function. We can't do this because the local copy actually contains _two_ strings; the symref name and its target. We point to the target pointer via the "util" field, and its memory must last as long as the string list does. You may also wonder whether it's safe to ever free the local copy, since the target points into it. The answer is yes, because we duplicate it in annotaate_refs_with_symref_info before clearing the string list. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-04-26Merge branch 'sf/putty-w-args'Libravatar Junio C Hamano1-1/+3
Plug a memleak. * sf/putty-w-args: connect.c: fix leak in handle_ssh_variant
2017-04-20connect.c: fix leak in handle_ssh_variantLibravatar Jeff King1-1/+3
When we see an error from split_cmdline(), we exit the function without freeing the copy of the command string we made. This was sort-of introduced by 22e5ae5c8 (connect.c: handle errors from split_cmdline, 2017-04-10). The leak existed before that, but before that commit fixed the bug, we could never trigger this else clause in the first place. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-04-19Merge branch 'sf/putty-w-args'Libravatar Junio C Hamano1-1/+1
* sf/putty-w-args: connect.c: handle errors from split_cmdline
2017-04-16connect.c: handle errors from split_cmdlineLibravatar Jeff King1-1/+1
Commit e9d9a8a4d (connect: handle putty/plink also in GIT_SSH_COMMAND, 2017-01-02) added a call to split_cmdline(), but checks only for a non-zero return to see if we got any output. Since the function returns negative values (and a NULL argv) on error, we end up dereferencing NULL and segfaulting. Arguably we could report on the parsing error here, but it's probably not worth it. This is a best-effort attempt to see if we are using plink. So we can simply return here with "no, it wasn't plink" and let the shell actually complain about the bogus quoting. Reported-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-03-31Rename sha1_array to oid_arrayLibravatar brian m. carlson1-4/+4
Since this structure handles an array of object IDs, rename it to struct oid_array. Also rename the accessor functions and the initialization constant. This commit was produced mechanically by providing non-Documentation files to the following Perl one-liners: perl -pi -E 's/struct sha1_array/struct oid_array/g' perl -pi -E 's/\bsha1_array_/oid_array_/g' perl -pi -E 's/SHA1_ARRAY_INIT/OID_ARRAY_INIT/g' Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-03-31Make sha1_array_append take a struct object_id *Libravatar brian m. carlson1-2/+2
Convert the callers to pass struct object_id by changing the function declaration and definition and applying the following semantic patch: @@ expression E1, E2; @@ - sha1_array_append(E1, E2.hash) + sha1_array_append(E1, &E2) @@ expression E1, E2; @@ - sha1_array_append(E1, E2->hash) + sha1_array_append(E1, E2) Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-02-10connect.c: stop conflating ssh command names and overridesLibravatar Junio C Hamano1-13/+32
dd33e07766 ("connect: Add the envvar GIT_SSH_VARIANT and ssh.variant config", 2017-02-01) attempted to add support for configuration and environment variable to override the different handling of port_option and needs_batch settings suitable for variants of the ssh implementation that was autodetected by looking at the ssh command name. Because it piggybacked on the code that turns command name to specific override (e.g. "plink.exe" and "plink" means port_option needs to be set to 'P' instead of the default 'p'), yet it defined a separate namespace for these overrides (e.g. "putty" can be usable to signal that port_option needs to be 'P'), however, it made the auto-detection based on the command name less robust (e.g. the code now accepts "putty" as a SSH command name and applies the same override). Separate the code that interprets the override that was read from the configuration & environment from the original code that handles the command names, as they are in separate namespaces, to fix this confusion. This incidentally also makes it easier for future enhancement of the override syntax (e.g. "port_option=p,needs_batch=1" may want to be accepted as a more explicit syntax) without affecting the code for auto-detection based on the command name. While at it, update the return type of the handle_ssh_variant() helper function to void; the caller does not use it, and the function does not return any meaningful value. Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-02-01connect: Add the envvar GIT_SSH_VARIANT and ssh.variant configLibravatar Segev Finer1-3/+8
This environment variable and configuration value allow to override the autodetection of plink/tortoiseplink in case that Git gets it wrong. [jes: wrapped overly-long lines, factored out and changed get_ssh_variant() to handle_ssh_variant() to accomodate the change from the putty/tortoiseplink variables to port_option/needs_batch, adjusted the documentation, free()d value obtained from the config.] Signed-off-by: Segev Finer <segev208@gmail.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-02-01git_connect(): factor out SSH variant handlingLibravatar Johannes Schindelin1-26/+46
We handle plink and tortoiseplink as OpenSSH replacements, by passing the correct command-line options when detecting that they are used. To let users override that auto-detection (in case Git gets it wrong), we need to introduce new code to that end. In preparation for this code, let's factor out the SSH variant handling into its own function, handle_ssh_variant(). Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-01-26connect: rename tortoiseplink and putty variablesLibravatar Junio C Hamano1-10/+13
One of these two may have originally been named after "what exact SSH implementation do we have?" so that we can tweak the command line options for that exact implementation. But "putty=1" no longer means "We are using the plink SSH implementation that comes with PuTTY" these days. It is set when we guess that either PuTTY plink or Tortoiseplink is in use. Rename them after what effect is desired. The current 'putty' option is about using "-P <port>" when OpenSSH would use "-p <port>", so rename it to 'port_option' whose value is either 'p' or 'P". The other one is about passing an extra command line option "-batch", so rename it to 'needs_batch'. [jes: wrapped overly-long line] Signed-off-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-01-25connect: handle putty/plink also in GIT_SSH_COMMANDLibravatar Segev Finer1-7/+16
Git for Windows has special support for the popular SSH client PuTTY: when using PuTTY's non-interactive version ("plink.exe"), we use the -P option to specify the port rather than OpenSSH's -p option. TortoiseGit ships with its own, forked version of plink.exe, that adds support for the -batch option, and for good measure we special-case that, too. However, this special-casing of PuTTY only covers the case where the user overrides the SSH command via the environment variable GIT_SSH (which allows specifying the name of the executable), not GIT_SSH_COMMAND (which allows specifying a full command, including additional command-line options). When users want to pass any additional arguments to (Tortoise-)Plink, such as setting a private key, they are required to either use a shell script named plink or tortoiseplink or duplicate the logic that is already in Git for passing the correct style of command line arguments, which can be difficult, error prone and annoying to get right. This patch simply reuses the existing logic and expands it to cover GIT_SSH_COMMAND, too. Note: it may look a little heavy-handed to duplicate the entire command-line and then split it, only to extract the name of the executable. However, this is not a performance-critical code path, and the code is much more readable this way. Signed-off-by: Segev Finer <segev208@gmail.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-10-31Merge branch 'ls/filter-process'Libravatar Junio C Hamano1-1/+1
The smudge/clean filter API expect an external process is spawned to filter the contents for each path that has a filter defined. A new type of "process" filter API has been added to allow the first request to run the filter for a path to spawn a single process, and all filtering need is served by this single process for multiple paths, reducing the process creation overhead. * ls/filter-process: contrib/long-running-filter: add long running filter example convert: add filter.<driver>.process option convert: prepare filter.<driver>.process option convert: make apply_filter() adhere to standard Git error handling pkt-line: add functions to read/write flush terminated packet streams pkt-line: add packet_write_gently() pkt-line: add packet_flush_gently() pkt-line: add packet_write_fmt_gently() pkt-line: extract set_packet_header() pkt-line: rename packet_write() to packet_write_fmt() run-command: add clean_on_exit_handler run-command: move check_pipe() from write_or_die to run_command convert: modernize tests convert: quote filter names in error messages
2016-10-17pkt-line: rename packet_write() to packet_write_fmt()Libravatar Lars Schneider1-1/+1
packet_write() should be called packet_write_fmt() because it is a printf-like function that takes a format string as first parameter. packet_write_fmt() should be used for text strings only. Arbitrary binary data should use a new packet_write() function that is introduced in a subsequent patch. Suggested-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Lars Schneider <larsxschneider@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-09-26Merge branch 'va/i18n-more'Libravatar Junio C Hamano1-4/+4
Even more i18n. * va/i18n-more: i18n: stash: mark messages for translation i18n: notes-merge: mark die messages for translation i18n: ident: mark hint for translation i18n: i18n: diff: mark die messages for translation i18n: connect: mark die messages for translation i18n: commit: mark message for translation
2016-09-21Merge branch 'jt/accept-capability-advertisement-when-fetching-from-void'Libravatar Junio C Hamano1-6/+26
JGit can show a fake ref "capabilities^{}" to "git fetch" when it does not advertise any refs, but "git fetch" was not prepared to see such an advertisement. When the other side disconnects without giving any ref advertisement, we used to say "there may not be a repository at that URL", but we may have seen other advertisement like "shallow" and ".have" in which case we definitely know that a repository is there. The code to detect this case has also been updated. * jt/accept-capability-advertisement-when-fetching-from-void: connect: advertized capability is not a ref connect: tighten check for unexpected early hang up tests: move test_lazy_prereq JGIT to test-lib.sh
2016-09-19i18n: connect: mark die messages for translationLibravatar Vasco Almeida1-4/+4
Mark messages passed to die() in die_initial_contact(). Update test to reflect changes. Signed-off-by: Vasco Almeida <vascomalmeida@sapo.pt> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-09-09connect: advertized capability is not a refLibravatar Jonathan Tan1-0/+14
When cloning an empty repository served by standard git, "git clone" produces the following reassuring message: $ git clone git://localhost/tmp/empty Cloning into 'empty'... warning: You appear to have cloned an empty repository. Checking connectivity... done. Meanwhile when cloning an empty repository served by JGit, the output is more haphazard: $ git clone git://localhost/tmp/empty Cloning into 'empty'... Checking connectivity... done. warning: remote HEAD refers to nonexistent ref, unable to checkout. This is a common command to run immediately after creating a remote repository as preparation for adding content to populate it and pushing. The warning is confusing and needlessly worrying. The cause is that, since v3.1.0.201309270735-rc1~22 (Advertise capabilities with no refs in upload service., 2013-08-08), JGit's ref advertisement includes a ref named capabilities^{} to advertise its capabilities on, while git's ref advertisement is empty in this case. This allows the client to learn about the server's capabilities and is needed, for example, for fetch-by-sha1 to work when no refs are advertised. This also affects "ls-remote". For example, against an empty repository served by JGit: $ git ls-remote git://localhost/tmp/empty 0000000000000000000000000000000000000000 capabilities^{} Git advertises the same capabilities^{} ref in its ref advertisement for push but since it never did so for fetch, the client didn't need to handle this case. Handle it. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Helped-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-09-09connect: tighten check for unexpected early hang upLibravatar Jonathan Nieder1-6/+12
A server hanging up immediately to mark access being denied does not send any .have refs, shallow lines, or anything else before hanging up. If the server has sent anything, then the hangup is unexpected. That is, if the server hangs up after a shallow line but before sending any refs, then git should tell me so: fatal: The remote end hung up upon initial contact instead of suggesting an access control problem: fatal: Could not read from remote repository. Please make sure you have the correct access rights and the repository exists. Noticed while examining this code. This case isn't likely to come up in practice but tightening the check makes the code easier to read and manipulate. Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-07-06connect: read $GIT_SSH_COMMAND from config fileLibravatar Nguyễn Thái Ngọc Duy1-1/+14
Similar to $GIT_ASKPASS or $GIT_PROXY_COMMAND, we also read from config file first then fall back to $GIT_SSH_COMMAND. This is useful for selecting different private keys targetting the same host (e.g. github) Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-03-16Merge branch 'cn/deprecate-ssh-git-url'Libravatar Junio C Hamano1-2/+2
The two alternative ways to spell "ssh://" transport have been deprecated for a long time. The last mention of them has finally removed from the documentation. * cn/deprecate-ssh-git-url: Disown ssh+git and git+ssh
2016-03-09Disown ssh+git and git+sshLibravatar Carlos Martín Nieto1-2/+2
Some people argue that these were silly from the beginning (see http://thread.gmane.org/gmane.comp.version-control.git/285590/focus=285601 for example), but we have to support them for compatibility. That doesn't mean we have to show them in the documentation. These were already left out of the main list, but a reference in the main manpage was left, so remove that. Also add a note to discourage their use if anybody goes looking for them in the source code. Signed-off-by: Carlos Martín Nieto <cmn@dwim.me> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-02-12connect & http: support -4 and -6 switches for remote operationsLibravatar Eric Wong1-0/+8
Sometimes it is necessary to force IPv4-only or IPv6-only operation on networks where name lookups may return a non-routable address and stall remote operations. The ssh(1) command has an equivalent switches which we may pass when we run them. There may be old ssh(1) implementations out there which do not support these switches; they should report the appropriate error in that case. rsync support is untouched for now since it is deprecated and scheduled to be removed. Signed-off-by: Eric Wong <normalperson@yhbt.net> Reviewed-by: Torsten Bögershausen <tboegi@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-11-20get_remote_heads: convert to struct object_idLibravatar brian m. carlson1-10/+12
Replace an unsigned char array with struct object_id and express several hard-coded constants in terms of GIT_SHA1_HEXSZ. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Jeff King <peff@peff.net>
2015-11-20Convert struct ref to use object_id.Libravatar brian m. carlson1-1/+1
Use struct object_id in three fields in struct ref and convert all the necessary places that use it. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Jeff King <peff@peff.net>
2015-10-20Merge branch 'jk/war-on-sprintf'Libravatar Junio C Hamano1-1/+1
Many allocations that is manually counted (correctly) that are followed by strcpy/sprintf have been replaced with a less error prone constructs such as xstrfmt. Macintosh-specific breakage was noticed and corrected in this reroll. * jk/war-on-sprintf: (70 commits) name-rev: use strip_suffix to avoid magic numbers use strbuf_complete to conditionally append slash fsck: use for_each_loose_file_in_objdir Makefile: drop D_INO_IN_DIRENT build knob fsck: drop inode-sorting code convert strncpy to memcpy notes: document length of fanout path with a constant color: add color_set helper for copying raw colors prefer memcpy to strcpy help: clean up kfmclient munging receive-pack: simplify keep_arg computation avoid sprintf and strcpy with flex arrays use alloc_ref rather than hand-allocating "struct ref" color: add overflow checks for parsing colors drop strcpy in favor of raw sha1_to_hex use sha1_to_hex_r() instead of strcpy daemon: use cld->env_array when re-spawning stat_tracking_info: convert to argv_array http-push: use an argv_array for setup_revisions fetch-pack: use argv_array for index-pack / unpack-objects ...
2015-10-14Merge branch 'tk/typofix-connect-unknown-proto-error'Libravatar Junio C Hamano1-1/+1
* tk/typofix-connect-unknown-proto-error: connect: fix typo in result string of prot_name()
2015-10-05Sync with 2.6.1Libravatar Junio C Hamano1-0/+6
2015-09-28Sync with 2.3.10Libravatar Junio C Hamano1-0/+5
2015-09-25convert trivial sprintf / strcpy calls to xsnprintfLibravatar Jeff King1-1/+1
We sometimes sprintf into fixed-size buffers when we know that the buffer is large enough to fit the input (either because it's a constant, or because it's numeric input that is bounded in size). Likewise with strcpy of constant strings. However, these sites make it hard to audit sprintf and strcpy calls for buffer overflows, as a reader has to cross-reference the size of the array with the input. Let's use xsnprintf instead, which communicates to a reader that we don't expect this to overflow (and catches the mistake in case we do). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-09-25connect: fix typo in result string of prot_name()Libravatar Tobias Klauser1-1/+1
Replace 'unkown' with 'unknown'. Signed-off-by: Tobias Klauser <tklauser@distanz.ch> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-09-23transport: add a protocol-whitelist environment variableLibravatar Jeff King1-0/+5
If we are cloning an untrusted remote repository into a sandbox, we may also want to fetch remote submodules in order to get the complete view as intended by the other side. However, that opens us up to attacks where a malicious user gets us to clone something they would not otherwise have access to (this is not necessarily a problem by itself, but we may then act on the cloned contents in a way that exposes them to the attacker). Ideally such a setup would sandbox git entirely away from high-value items, but this is not always practical or easy to set up (e.g., OS network controls may block multiple protocols, and we would want to enable some but not others). We can help this case by providing a way to restrict particular protocols. We use a whitelist in the environment. This is more annoying to set up than a blacklist, but defaults to safety if the set of protocols git supports grows). If no whitelist is specified, we continue to default to allowing all protocols (this is an "unsafe" default, but since the minority of users will want this sandboxing effect, it is the only sensible one). A note on the tests: ideally these would all be in a single test file, but the git-daemon and httpd test infrastructure is an all-or-nothing proposition rather than a test-by-test prerequisite. By putting them all together, we would be unable to test the file-local code on machines without apache. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-09-08git_connect: clarify conn->use_shell flagLibravatar Jeff King1-9/+13
When executing user-specified programs, we generally always want to use a shell, for flexibility and consistency. One big exception is executing $GIT_SSH, which for historical reasons must not use a shell. Once upon a time the logic in git_connect looked like: if (protocol == PROTO_SSH) { ... setup ssh ... } else { ... setup local connection ... conn->use_shell = 1; } But over time the PROTO_SSH block has grown, and the "local" block has shrunk so that it contains only conn->use_shell; it's easy to miss at the end of the large block. Moreover, PROTO_SSH now also sometimes sets use_shell, when the new GIT_SSH_COMMAND is used. Let's just set conn->use_shell when we're setting up the "conn" struct, and unset it (with a comment) in the historical GIT_SSH case. This will make the flow easier to follow. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-09-04git_connect: clear GIT_* environment for sshLibravatar Jeff King1-2/+2
When we "switch" to another local repository to run the server side of a fetch or push, we must clear the variables in local_repo_env so that our local $GIT_DIR, etc, do not pollute the upload-pack or receive-pack that is executing in the "remote" repository. We have never done so for ssh connections. For the most part, nobody has noticed because ssh will not pass unknown environment variables by default. However, it is not out of the question for a user to configure ssh to pass along GIT_* variables using SendEnv/AcceptEnv. We can demonstrate the problem by using "git -c" on a local command and seeing its impact on a remote repository. This config ends up in $GIT_CONFIG_PARAMETERS. In the local case, the config has no impact, but in the ssh transport, it does (our test script has a fake ssh that passes through all environment variables; this isn't normal, but does simulate one possible setup). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-06-05Merge branch 'bc/connect-plink' into maintLibravatar Junio C Hamano1-21/+33
The connection initiation code for "ssh" transport tried to absorb differences between the stock "ssh" and Putty-supplied "plink" and its derivatives, but the logic to tell that we are using "plink" variants were too loose and falsely triggered when "plink" appeared anywhere in the path (e.g. "/home/me/bin/uplink/ssh"). * bc/connect-plink: connect: improve check for plink to reduce false positives t5601: fix quotation error leading to skipped tests connect: simplify SSH connection code path
2015-05-19Merge branch 'bc/connect-plink'Libravatar Junio C Hamano1-21/+33
The connection initiation code for "ssh" transport tried to absorb differences between the stock "ssh" and Putty-supplied "plink" and its derivatives, but the logic to tell that we are using "plink" variants were too loose and falsely triggered when "plink" appeared anywhere in the path (e.g. "/home/me/bin/uplink/ssh"). * bc/connect-plink: connect: improve check for plink to reduce false positives t5601: fix quotation error leading to skipped tests connect: simplify SSH connection code path