ssh: 'auto' variant to select between 'ssh' and 'simple'

Android's "repo" tool is a tool for managing a large codebase consisting of multiple smaller repositories, similar to Git's submodule feature. Starting with Git 94b8ae5a (ssh: introduce a 'simple' ssh variant, 2017-10-16), users noticed that it stopped handling the port in ssh:// URLs. The cause: when it encounters ssh:// URLs, repo pre-connects to the server and sets GIT_SSH to a helper ".repo/repo/git_ssh" that reuses that connection. Before 94b8ae5a, the helper was assumed to support OpenSSH options for lack of a better guess and got passed a -p option to set the port. After that patch, it uses the new default of a simple helper that does not accept an option to set the port. The next release of "repo" will set GIT_SSH_VARIANT to "ssh" to avoid that. But users of old versions and of other similar GIT_SSH implementations would not get the benefit of that fix. So update the default to use OpenSSH options again, with a twist. As observed in 94b8ae5a, we cannot assume that $GIT_SSH always handles OpenSSH options: common helpers such as travis-ci's dpl[*] are configured using GIT_SSH and do not accept OpenSSH options. So make the default a new variant "auto", with the following behavior: 1. First, check for a recognized basename, like today. 2. If the basename is not recognized, check whether $GIT_SSH supports OpenSSH options by running $GIT_SSH -G <options> <host> This returns status 0 and prints configuration in OpenSSH if it recognizes all <options> and returns status 255 if it encounters an unrecognized option. A wrapper script like exec ssh -- "$@" would fail with ssh: Could not resolve hostname -g: Name or service not known , correctly reflecting that it does not support OpenSSH options. The command is run with stdin, stdout, and stderr redirected to /dev/null so even a command that expects a terminal would exit immediately. 3. Based on the result from step (2), behave like "ssh" (if it succeeded) or "simple" (if it failed). This way, the default ssh variant for unrecognized commands can handle both the repo and dpl cases as intended. This autodetection has been running on Google workstations since 2017-10-23 with no reported negative effects. [*] https://github.com/travis-ci/dpl/blob/6c3fddfda1f2a85944c544446b068bac0a77c049/lib/dpl/provider.rb#L215 Reported-by: William Yan <wyan@google.com> Improved-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jonathan Nieder committed Nov 20, 2017 at 13:30 UTC 0da0e49ba12225684b75e86a4c9344ad121652cb
3 files changed +62 -17
Documentation/config.txt
+16 -10
@@ -2081,16 +2081,22 @@ matched against are those given directly to Git commands. This means any URLs
2081 visited as a result of a redirection do not participate in matching.
2082
2083 ssh.variant::
2084 - Depending on the value of the environment variables `GIT_SSH` or
2085 - `GIT_SSH_COMMAND`, or the config setting `core.sshCommand`, Git
2086 - auto-detects whether to adjust its command-line parameters for use
2087 - with ssh (OpenSSH), plink or tortoiseplink, as opposed to the default
2088 - (simple).
2089 -+
2090 -The config variable `ssh.variant` can be set to override this auto-detection;
2091 -valid values are `ssh`, `simple`, `plink`, `putty` or `tortoiseplink`. Any
2092 -other value will be treated as normal ssh. This setting can be overridden via
2093 -the environment variable `GIT_SSH_VARIANT`.
2084 + By default, Git determines the command line arguments to use
2085 + based on the basename of the configured SSH command (configured
2086 + using the environment variable `GIT_SSH` or `GIT_SSH_COMMAND` or
2087 + the config setting `core.sshCommand`). If the basename is
2088 + unrecognized, Git will attempt to detect support of OpenSSH
2089 + options by first invoking the configured SSH command with the
2090 + `-G` (print configuration) option and will subsequently use
2091 + OpenSSH options (if that is successful) or no options besides
2092 + the host and remote command (if it fails).
2093 ++
2094 +The config variable `ssh.variant` can be set to override this detection.
2095 +Valid values are `ssh` (to use OpenSSH options), `plink`, `putty`,
2096 +`tortoiseplink`, `simple` (no options except the host and remote command).
2097 +The default auto-detection can be explicitly requested using the value
2098 +`auto`. Any other value is treated as `ssh`. This setting can also be
2099 +overridden via the environment variable `GIT_SSH_VARIANT`.
2100 +
2101 The current command-line parameters used for each variant are as
2102 follows:
connect.c
+25 -7
@@ -788,6 +788,7 @@ static const char *get_ssh_command(void)
788 }
789
790 enum ssh_variant {
791 + VARIANT_AUTO,
792 VARIANT_SIMPLE,
793 VARIANT_SSH,
794 VARIANT_PLINK,
@@ -795,14 +796,16 @@ enum ssh_variant {
796 VARIANT_TORTOISEPLINK,
797 };
798
798 -static int override_ssh_variant(enum ssh_variant *ssh_variant)
799 +static void override_ssh_variant(enum ssh_variant *ssh_variant)
800 {
801 const char *variant = getenv("GIT_SSH_VARIANT");
802
803 if (!variant && git_config_get_string_const("ssh.variant", &variant))
803 - return 0;
804 + return;
805
805 - if (!strcmp(variant, "plink"))
806 + if (!strcmp(variant, "auto"))
807 + *ssh_variant = VARIANT_AUTO;
808 + else if (!strcmp(variant, "plink"))
809 *ssh_variant = VARIANT_PLINK;
810 else if (!strcmp(variant, "putty"))
811 *ssh_variant = VARIANT_PUTTY;
@@ -812,18 +815,18 @@ static int override_ssh_variant(enum ssh_variant *ssh_variant)
815 *ssh_variant = VARIANT_SIMPLE;
816 else
817 *ssh_variant = VARIANT_SSH;
815 -
816 - return 1;
818 }
819
820 static enum ssh_variant determine_ssh_variant(const char *ssh_command,
821 int is_cmdline)
822 {
822 - enum ssh_variant ssh_variant = VARIANT_SIMPLE;
823 + enum ssh_variant ssh_variant = VARIANT_AUTO;
824 const char *variant;
825 char *p = NULL;
826
826 - if (override_ssh_variant(&ssh_variant))
827 + override_ssh_variant(&ssh_variant);
828 +
829 + if (ssh_variant != VARIANT_AUTO)
830 return ssh_variant;
831
832 if (!is_cmdline) {
@@ -982,6 +985,21 @@ static void fill_ssh_args(struct child_process *conn, const char *ssh_host,
985 variant = determine_ssh_variant(ssh, 0);
986 }
987
988 + if (variant == VARIANT_AUTO) {
989 + struct child_process detect = CHILD_PROCESS_INIT;
990 +
991 + detect.use_shell = conn->use_shell;
992 + detect.no_stdin = detect.no_stdout = detect.no_stderr = 1;
993 +
994 + argv_array_push(&detect.args, ssh);
995 + argv_array_push(&detect.args, "-G");
996 + push_ssh_options(&detect.args, &detect.env_array,
997 + VARIANT_SSH, port, flags);
998 + argv_array_push(&detect.args, ssh_host);
999 +
1000 + variant = run_command(&detect) ? VARIANT_SIMPLE : VARIANT_SSH;
1001 + }
1002 +
1003 argv_array_push(&conn->args, ssh);
1004 push_ssh_options(&conn->args, &conn->env_array, variant, port, flags);
1005 argv_array_push(&conn->args, ssh_host);
t/t5601-clone.sh
+21
@@ -369,6 +369,12 @@ test_expect_success 'variant can be overriden' '
369 expect_ssh myhost src
370 '
371
372 +test_expect_success 'variant=auto picks based on basename' '
373 + copy_ssh_wrapper_as "$TRASH_DIRECTORY/plink" &&
374 + git -c ssh.variant=auto clone -4 "[myhost:123]:src" ssh-auto-clone &&
375 + expect_ssh "-4 -P 123" myhost src
376 +'
377 +
378 test_expect_success 'simple is treated as simple' '
379 copy_ssh_wrapper_as "$TRASH_DIRECTORY/simple" &&
380 git clone -4 "[myhost:123]:src" ssh-bracket-clone-simple &&
@@ -381,6 +387,21 @@ test_expect_success 'uplink is treated as simple' '
387 expect_ssh myhost src
388 '
389
390 +test_expect_success 'OpenSSH-like uplink is treated as ssh' '
391 + write_script "$TRASH_DIRECTORY/uplink" <<-EOF &&
392 + if test "\$1" = "-G"
393 + then
394 + exit 0
395 + fi &&
396 + exec "\$TRASH_DIRECTORY/ssh$X" "\$@"
397 + EOF
398 + test_when_finished "rm -f \"\$TRASH_DIRECTORY/uplink\"" &&
399 + GIT_SSH="$TRASH_DIRECTORY/uplink" &&
400 + test_when_finished "GIT_SSH=\"\$TRASH_DIRECTORY/ssh\$X\"" &&
401 + git clone "[myhost:123]:src" ssh-bracket-clone-sshlike-uplink &&
402 + expect_ssh "-p 123" myhost src
403 +'
404 +
405 test_expect_success 'plink is treated specially (as putty)' '
406 copy_ssh_wrapper_as "$TRASH_DIRECTORY/plink" &&
407 git clone "[myhost:123]:src" ssh-bracket-clone-plink-0 &&