ssh: introduce a 'simple' ssh variant

When using the 'ssh' transport, the '-o' option is used to specify an environment variable which should be set on the remote end. This allows git to send additional information when contacting the server, requesting the use of a different protocol version via the 'GIT_PROTOCOL' environment variable like so: "-o SendEnv=GIT_PROTOCOL". Unfortunately not all ssh variants support the sending of environment variables to the remote end. To account for this, only use the '-o' option for ssh variants which are OpenSSH compliant. This is done by checking that the basename of the ssh command is 'ssh' or the ssh variant is overridden to be 'ssh' (via the ssh.variant config). Other options like '-p' and '-P', which are used to specify a specific port to use, or '-4' and '-6', which are used to indicate that IPV4 or IPV6 addresses should be used, may also not be supported by all ssh variants. Currently if an ssh command's basename wasn't 'plink' or 'tortoiseplink' git assumes that the command is an OpenSSH variant. Since user configured ssh commands may not be OpenSSH compliant, tighten this constraint and assume a variant of 'simple' if the basename of the command doesn't match the variants known to git. The new ssh variant 'simple' will only have the host and command to execute ([username@]host command) passed as parameters to the ssh command. Update the Documentation to better reflect the command-line options sent to ssh commands based on their variant. Reported-by: Jeffrey Yasskin <jyasskin@google.com> Signed-off-by: Brandon Williams <bmwill@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Brandon Williams committed Oct 16, 2017 at 10:55 UTC 94b8ae5aca41538cf78918663f8b8294f6dcaa21
5 files changed +111 -61
Documentation/config.txt
+23 -4
@@ -2084,12 +2084,31 @@ 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 plink or tortoiseplink, as opposed to the default (OpenSSH).
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;
2090 -valid values are `ssh`, `plink`, `putty` or `tortoiseplink`. Any other value
2091 -will be treated as normal ssh. This setting can be overridden via the
2092 -environment variable `GIT_SSH_VARIANT`.
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`.
2094 ++
2095 +The current command-line parameters used for each variant are as
2096 +follows:
2097 ++
2098 +--
2099 +
2100 +* `ssh` - [-p port] [-4] [-6] [-o option] [username@]host command
2101 +
2102 +* `simple` - [username@]host command
2103 +
2104 +* `plink` or `putty` - [-P port] [-4] [-6] [username@]host command
2105 +
2106 +* `tortoiseplink` - [-P port] [-4] [-6] -batch [username@]host command
2107 +
2108 +--
2109 ++
2110 +Except for the `simple` variant, command-line parameters are likely to
2111 +change as git gains new features.
2112
2113 i18n.commitEncoding::
2114 Character encoding the commit messages are stored in; Git itself
Documentation/git.txt
+4 -5
@@ -518,11 +518,10 @@ other
518 If either of these environment variables is set then 'git fetch'
519 and 'git push' will use the specified command instead of 'ssh'
520 when they need to connect to a remote system.
521 - The command will be given exactly two or four arguments: the
522 - 'username@host' (or just 'host') from the URL and the shell
523 - command to execute on that remote system, optionally preceded by
524 - `-p` (literally) and the 'port' from the URL when it specifies
525 - something other than the default SSH port.
521 + The command-line parameters passed to the configured command are
522 + determined by the ssh variant. See `ssh.variant` option in
523 + linkgit:git-config[1] for details.
524 +
525 +
526 `$GIT_SSH_COMMAND` takes precedence over `$GIT_SSH`, and is interpreted
527 by the shell, which allows additional arguments to be included.
connect.c
+61 -47
@@ -776,37 +776,44 @@ static const char *get_ssh_command(void)
776 return NULL;
777 }
778
779 -static int override_ssh_variant(int *port_option, int *needs_batch)
779 +enum ssh_variant {
780 + VARIANT_SIMPLE,
781 + VARIANT_SSH,
782 + VARIANT_PLINK,
783 + VARIANT_PUTTY,
784 + VARIANT_TORTOISEPLINK,
785 +};
786 +
787 +static int override_ssh_variant(enum ssh_variant *ssh_variant)
788 {
781 - char *variant;
789 + const char *variant = getenv("GIT_SSH_VARIANT");
790
783 - variant = xstrdup_or_null(getenv("GIT_SSH_VARIANT"));
784 - if (!variant &&
785 - git_config_get_string("ssh.variant", &variant))
791 + if (!variant && git_config_get_string_const("ssh.variant", &variant))
792 return 0;
793
788 - if (!strcmp(variant, "plink") || !strcmp(variant, "putty")) {
789 - *port_option = 'P';
790 - *needs_batch = 0;
791 - } else if (!strcmp(variant, "tortoiseplink")) {
792 - *port_option = 'P';
793 - *needs_batch = 1;
794 - } else {
795 - *port_option = 'p';
796 - *needs_batch = 0;
797 - }
798 - free(variant);
794 + if (!strcmp(variant, "plink"))
795 + *ssh_variant = VARIANT_PLINK;
796 + else if (!strcmp(variant, "putty"))
797 + *ssh_variant = VARIANT_PUTTY;
798 + else if (!strcmp(variant, "tortoiseplink"))
799 + *ssh_variant = VARIANT_TORTOISEPLINK;
800 + else if (!strcmp(variant, "simple"))
801 + *ssh_variant = VARIANT_SIMPLE;
802 + else
803 + *ssh_variant = VARIANT_SSH;
804 +
805 return 1;
806 }
807
802 -static void handle_ssh_variant(const char *ssh_command, int is_cmdline,
803 - int *port_option, int *needs_batch)
808 +static enum ssh_variant determine_ssh_variant(const char *ssh_command,
809 + int is_cmdline)
810 {
811 + enum ssh_variant ssh_variant = VARIANT_SIMPLE;
812 const char *variant;
813 char *p = NULL;
814
808 - if (override_ssh_variant(port_option, needs_batch))
809 - return;
815 + if (override_ssh_variant(&ssh_variant))
816 + return ssh_variant;
817
818 if (!is_cmdline) {
819 p = xstrdup(ssh_command);
@@ -825,19 +832,22 @@ static void handle_ssh_variant(const char *ssh_command, int is_cmdline,
832 free(ssh_argv);
833 } else {
834 free(p);
828 - return;
835 + return ssh_variant;
836 }
837 }
838
832 - if (!strcasecmp(variant, "plink") ||
833 - !strcasecmp(variant, "plink.exe"))
834 - *port_option = 'P';
839 + if (!strcasecmp(variant, "ssh") ||
840 + !strcasecmp(variant, "ssh.exe"))
841 + ssh_variant = VARIANT_SSH;
842 + else if (!strcasecmp(variant, "plink") ||
843 + !strcasecmp(variant, "plink.exe"))
844 + ssh_variant = VARIANT_PLINK;
845 else if (!strcasecmp(variant, "tortoiseplink") ||
836 - !strcasecmp(variant, "tortoiseplink.exe")) {
837 - *port_option = 'P';
838 - *needs_batch = 1;
839 - }
846 + !strcasecmp(variant, "tortoiseplink.exe"))
847 + ssh_variant = VARIANT_TORTOISEPLINK;
848 +
849 free(p);
850 + return ssh_variant;
851 }
852
853 /*
@@ -937,8 +947,7 @@ struct child_process *git_connect(int fd[2], const char *url,
947 conn->in = conn->out = -1;
948 if (protocol == PROTO_SSH) {
949 const char *ssh;
940 - int needs_batch = 0;
941 - int port_option = 'p';
950 + enum ssh_variant variant;
951 char *ssh_host = hostandport;
952 const char *port = NULL;
953 transport_check_allowed("ssh");
@@ -965,10 +974,9 @@ struct child_process *git_connect(int fd[2], const char *url,
974 die("strange hostname '%s' blocked", ssh_host);
975
976 ssh = get_ssh_command();
968 - if (ssh)
969 - handle_ssh_variant(ssh, 1, &port_option,
970 - &needs_batch);
971 - else {
977 + if (ssh) {
978 + variant = determine_ssh_variant(ssh, 1);
979 + } else {
980 /*
981 * GIT_SSH is the no-shell version of
982 * GIT_SSH_COMMAND (and must remain so for
@@ -979,32 +987,38 @@ struct child_process *git_connect(int fd[2], const char *url,
987 ssh = getenv("GIT_SSH");
988 if (!ssh)
989 ssh = "ssh";
982 - else
983 - handle_ssh_variant(ssh, 0,
984 - &port_option,
985 - &needs_batch);
990 + variant = determine_ssh_variant(ssh, 0);
991 }
992
993 argv_array_push(&conn->args, ssh);
994
990 - if (get_protocol_version_config() > 0) {
995 + if (variant == VARIANT_SSH &&
996 + get_protocol_version_config() > 0) {
997 argv_array_push(&conn->args, "-o");
998 argv_array_push(&conn->args, "SendEnv=" GIT_PROTOCOL_ENVIRONMENT);
999 argv_array_pushf(&conn->env_array, GIT_PROTOCOL_ENVIRONMENT "=version=%d",
1000 get_protocol_version_config());
1001 }
1002
997 - if (flags & CONNECT_IPV4)
998 - argv_array_push(&conn->args, "-4");
999 - else if (flags & CONNECT_IPV6)
1000 - argv_array_push(&conn->args, "-6");
1001 - if (needs_batch)
1003 + if (variant != VARIANT_SIMPLE) {
1004 + if (flags & CONNECT_IPV4)
1005 + argv_array_push(&conn->args, "-4");
1006 + else if (flags & CONNECT_IPV6)
1007 + argv_array_push(&conn->args, "-6");
1008 + }
1009 +
1010 + if (variant == VARIANT_TORTOISEPLINK)
1011 argv_array_push(&conn->args, "-batch");
1003 - if (port) {
1004 - argv_array_pushf(&conn->args,
1005 - "-%c", port_option);
1012 +
1013 + if (port && variant != VARIANT_SIMPLE) {
1014 + if (variant == VARIANT_SSH)
1015 + argv_array_push(&conn->args, "-p");
1016 + else
1017 + argv_array_push(&conn->args, "-P");
1018 +
1019 argv_array_push(&conn->args, port);
1020 }
1021 +
1022 argv_array_push(&conn->args, ssh_host);
1023 } else {
1024 transport_check_allowed("file");
t/t5601-clone.sh
+21 -5
@@ -309,8 +309,8 @@ test_expect_success 'clone checking out a tag' '
309 setup_ssh_wrapper () {
310 test_expect_success 'setup ssh wrapper' '
311 cp "$GIT_BUILD_DIR/t/helper/test-fake-ssh$X" \
312 - "$TRASH_DIRECTORY/ssh-wrapper$X" &&
313 - GIT_SSH="$TRASH_DIRECTORY/ssh-wrapper$X" &&
312 + "$TRASH_DIRECTORY/ssh$X" &&
313 + GIT_SSH="$TRASH_DIRECTORY/ssh$X" &&
314 export GIT_SSH &&
315 export TRASH_DIRECTORY &&
316 >"$TRASH_DIRECTORY"/ssh-output
@@ -318,7 +318,7 @@ setup_ssh_wrapper () {
318 }
319
320 copy_ssh_wrapper_as () {
321 - cp "$TRASH_DIRECTORY/ssh-wrapper$X" "${1%$X}$X" &&
321 + cp "$TRASH_DIRECTORY/ssh$X" "${1%$X}$X" &&
322 GIT_SSH="${1%$X}$X" &&
323 export GIT_SSH
324 }
@@ -362,10 +362,26 @@ test_expect_success 'bracketed hostnames are still ssh' '
362 expect_ssh "-p 123" myhost src
363 '
364
365 -test_expect_success 'uplink is not treated as putty' '
365 +test_expect_success 'OpenSSH variant passes -4' '
366 + git clone -4 "[myhost:123]:src" ssh-ipv4-clone &&
367 + expect_ssh "-4 -p 123" myhost src
368 +'
369 +
370 +test_expect_success 'variant can be overriden' '
371 + git -c ssh.variant=simple clone -4 "[myhost:123]:src" ssh-simple-clone &&
372 + expect_ssh myhost src
373 +'
374 +
375 +test_expect_success 'simple is treated as simple' '
376 + copy_ssh_wrapper_as "$TRASH_DIRECTORY/simple" &&
377 + git clone -4 "[myhost:123]:src" ssh-bracket-clone-simple &&
378 + expect_ssh myhost src
379 +'
380 +
381 +test_expect_success 'uplink is treated as simple' '
382 copy_ssh_wrapper_as "$TRASH_DIRECTORY/uplink" &&
383 git clone "[myhost:123]:src" ssh-bracket-clone-uplink &&
368 - expect_ssh "-p 123" myhost src
384 + expect_ssh myhost src
385 '
386
387 test_expect_success 'plink is treated specially (as putty)' '
t/t5700-protocol-v1.sh
+2
@@ -147,6 +147,8 @@ test_expect_success 'push with file:// using protocol v1' '
147 test_expect_success 'setup ssh wrapper' '
148 GIT_SSH="$GIT_BUILD_DIR/t/helper/test-fake-ssh" &&
149 export GIT_SSH &&
150 + GIT_SSH_VARIANT=ssh &&
151 + export GIT_SSH_VARIANT &&
152 export TRASH_DIRECTORY &&
153 >"$TRASH_DIRECTORY"/ssh-output
154 '