diff options
author | Junio C Hamano <gitster@pobox.com> | 2017-12-06 20:23:45 +0300 |
---|---|---|
committer | Junio C Hamano <gitster@pobox.com> | 2017-12-06 20:23:45 +0300 |
commit | ef470364448b6d5de159c3a961c1b1e8283d1dbc (patch) | |
tree | b52a8b297d62ad5c14a8251f4942c5dc2ae99601 | |
parent | 4c6dad0059b2b5d1ea996ccf67f93224955b07b4 (diff) | |
parent | 233cd282ad71e667082bae45b3a73e947daa158b (diff) |
Merge branch 'jn/ssh-wrappers'
The ssh-variant 'simple' introduced earlier broke existing
installations by not passing --port/-4/-6 and not diagnosing an
attempt to pass these as an error. Instead, default to
automatically detect how compatible the GIT_SSH/GIT_SSH_COMMAND is
to OpenSSH convention and then error out an invocation to make it
easier to diagnose connection errors.
* jn/ssh-wrappers:
connect: correct style of C-style comment
ssh: 'simple' variant does not support --port
ssh: 'simple' variant does not support -4/-6
ssh: 'auto' variant to select between 'ssh' and 'simple'
connect: split ssh option computation to its own function
connect: split ssh command line options into separate function
connect: split git:// setup into a separate function
connect: move no_fork fallback to git_tcp_connect
ssh test: make copy_ssh_wrapper_as clean up after itself
-rw-r--r-- | Documentation/config.txt | 26 | ||||
-rw-r--r-- | connect.c | 320 | ||||
-rwxr-xr-x | t/t5601-clone.sh | 70 | ||||
-rwxr-xr-x | t/t5603-clone-dirname.sh | 2 |
4 files changed, 266 insertions, 152 deletions
diff --git a/Documentation/config.txt b/Documentation/config.txt index 64bdce8435..c1598ee703 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -2108,16 +2108,22 @@ matched against are those given directly to Git commands. This means any URLs visited as a result of a redirection do not participate in matching. ssh.variant:: - Depending on the value of the environment variables `GIT_SSH` or - `GIT_SSH_COMMAND`, or the config setting `core.sshCommand`, Git - auto-detects whether to adjust its command-line parameters for use - with ssh (OpenSSH), plink or tortoiseplink, as opposed to the default - (simple). -+ -The config variable `ssh.variant` can be set to override this auto-detection; -valid values are `ssh`, `simple`, `plink`, `putty` or `tortoiseplink`. Any -other value will be treated as normal ssh. This setting can be overridden via -the environment variable `GIT_SSH_VARIANT`. + By default, Git determines the command line arguments to use + based on the basename of the configured SSH command (configured + using the environment variable `GIT_SSH` or `GIT_SSH_COMMAND` or + the config setting `core.sshCommand`). If the basename is + unrecognized, Git will attempt to detect support of OpenSSH + options by first invoking the configured SSH command with the + `-G` (print configuration) option and will subsequently use + OpenSSH options (if that is successful) or no options besides + the host and remote command (if it fails). ++ +The config variable `ssh.variant` can be set to override this detection. +Valid values are `ssh` (to use OpenSSH options), `plink`, `putty`, +`tortoiseplink`, `simple` (no options except the host and remote command). +The default auto-detection can be explicitly requested using the value +`auto`. Any other value is treated as `ssh`. This setting can also be +overridden via the environment variable `GIT_SSH_VARIANT`. + The current command-line parameters used for each variant are as follows: @@ -582,12 +582,25 @@ static int git_tcp_connect_sock(char *host, int flags) #endif /* NO_IPV6 */ -static void git_tcp_connect(int fd[2], char *host, int flags) +/* + * Dummy child_process returned by git_connect() if the transport protocol + * does not need fork(2). + */ +static struct child_process no_fork = CHILD_PROCESS_INIT; + +int git_connection_is_socket(struct child_process *conn) +{ + return conn == &no_fork; +} + +static struct child_process *git_tcp_connect(int fd[2], char *host, int flags) { int sockfd = git_tcp_connect_sock(host, flags); fd[0] = sockfd; fd[1] = dup(sockfd); + + return &no_fork; } @@ -761,8 +774,6 @@ static enum protocol parse_connect_url(const char *url_orig, char **ret_host, return protocol; } -static struct child_process no_fork = CHILD_PROCESS_INIT; - static const char *get_ssh_command(void) { const char *ssh; @@ -777,6 +788,7 @@ static const char *get_ssh_command(void) } enum ssh_variant { + VARIANT_AUTO, VARIANT_SIMPLE, VARIANT_SSH, VARIANT_PLINK, @@ -784,14 +796,16 @@ enum ssh_variant { VARIANT_TORTOISEPLINK, }; -static int override_ssh_variant(enum ssh_variant *ssh_variant) +static void override_ssh_variant(enum ssh_variant *ssh_variant) { const char *variant = getenv("GIT_SSH_VARIANT"); if (!variant && git_config_get_string_const("ssh.variant", &variant)) - return 0; + return; - if (!strcmp(variant, "plink")) + if (!strcmp(variant, "auto")) + *ssh_variant = VARIANT_AUTO; + else if (!strcmp(variant, "plink")) *ssh_variant = VARIANT_PLINK; else if (!strcmp(variant, "putty")) *ssh_variant = VARIANT_PUTTY; @@ -801,18 +815,18 @@ static int override_ssh_variant(enum ssh_variant *ssh_variant) *ssh_variant = VARIANT_SIMPLE; else *ssh_variant = VARIANT_SSH; - - return 1; } static enum ssh_variant determine_ssh_variant(const char *ssh_command, int is_cmdline) { - enum ssh_variant ssh_variant = VARIANT_SIMPLE; + enum ssh_variant ssh_variant = VARIANT_AUTO; const char *variant; char *p = NULL; - if (override_ssh_variant(&ssh_variant)) + override_ssh_variant(&ssh_variant); + + if (ssh_variant != VARIANT_AUTO) return ssh_variant; if (!is_cmdline) { @@ -851,11 +865,181 @@ static enum ssh_variant determine_ssh_variant(const char *ssh_command, } /* - * This returns a dummy child_process if the transport protocol does not - * need fork(2), or a struct child_process object if it does. Once done, - * finish the connection with finish_connect() with the value returned from - * this function (it is safe to call finish_connect() with NULL to support - * the former case). + * Open a connection using Git's native protocol. + * + * The caller is responsible for freeing hostandport, but this function may + * modify it (for example, to truncate it to remove the port part). + */ +static struct child_process *git_connect_git(int fd[2], char *hostandport, + const char *path, const char *prog, + int flags) +{ + struct child_process *conn; + struct strbuf request = STRBUF_INIT; + /* + * Set up virtual host information based on where we will + * connect, unless the user has overridden us in + * the environment. + */ + char *target_host = getenv("GIT_OVERRIDE_VIRTUAL_HOST"); + if (target_host) + target_host = xstrdup(target_host); + else + target_host = xstrdup(hostandport); + + transport_check_allowed("git"); + + /* + * These underlying connection commands die() if they + * cannot connect. + */ + if (git_use_proxy(hostandport)) + conn = git_proxy_connect(fd, hostandport); + else + conn = git_tcp_connect(fd, hostandport, flags); + /* + * Separate original protocol components prog and path + * from extended host header with a NUL byte. + * + * Note: Do not add any other headers here! Doing so + * will cause older git-daemon servers to crash. + */ + strbuf_addf(&request, + "%s %s%chost=%s%c", + prog, path, 0, + target_host, 0); + + /* If using a new version put that stuff here after a second null byte */ + if (get_protocol_version_config() > 0) { + strbuf_addch(&request, '\0'); + strbuf_addf(&request, "version=%d%c", + get_protocol_version_config(), '\0'); + } + + packet_write(fd[1], request.buf, request.len); + + free(target_host); + strbuf_release(&request); + return conn; +} + +/* + * Append the appropriate environment variables to `env` and options to + * `args` for running ssh in Git's SSH-tunneled transport. + */ +static void push_ssh_options(struct argv_array *args, struct argv_array *env, + enum ssh_variant variant, const char *port, + int flags) +{ + if (variant == VARIANT_SSH && + get_protocol_version_config() > 0) { + argv_array_push(args, "-o"); + argv_array_push(args, "SendEnv=" GIT_PROTOCOL_ENVIRONMENT); + argv_array_pushf(env, GIT_PROTOCOL_ENVIRONMENT "=version=%d", + get_protocol_version_config()); + } + + if (flags & CONNECT_IPV4) { + switch (variant) { + case VARIANT_AUTO: + BUG("VARIANT_AUTO passed to push_ssh_options"); + case VARIANT_SIMPLE: + die("ssh variant 'simple' does not support -4"); + case VARIANT_SSH: + case VARIANT_PLINK: + case VARIANT_PUTTY: + case VARIANT_TORTOISEPLINK: + argv_array_push(args, "-4"); + } + } else if (flags & CONNECT_IPV6) { + switch (variant) { + case VARIANT_AUTO: + BUG("VARIANT_AUTO passed to push_ssh_options"); + case VARIANT_SIMPLE: + die("ssh variant 'simple' does not support -6"); + case VARIANT_SSH: + case VARIANT_PLINK: + case VARIANT_PUTTY: + case VARIANT_TORTOISEPLINK: + argv_array_push(args, "-6"); + } + } + + if (variant == VARIANT_TORTOISEPLINK) + argv_array_push(args, "-batch"); + + if (port) { + switch (variant) { + case VARIANT_AUTO: + BUG("VARIANT_AUTO passed to push_ssh_options"); + case VARIANT_SIMPLE: + die("ssh variant 'simple' does not support setting port"); + case VARIANT_SSH: + argv_array_push(args, "-p"); + break; + case VARIANT_PLINK: + case VARIANT_PUTTY: + case VARIANT_TORTOISEPLINK: + argv_array_push(args, "-P"); + } + + argv_array_push(args, port); + } +} + +/* Prepare a child_process for use by Git's SSH-tunneled transport. */ +static void fill_ssh_args(struct child_process *conn, const char *ssh_host, + const char *port, int flags) +{ + const char *ssh; + enum ssh_variant variant; + + if (looks_like_command_line_option(ssh_host)) + die("strange hostname '%s' blocked", ssh_host); + + ssh = get_ssh_command(); + if (ssh) { + variant = determine_ssh_variant(ssh, 1); + } else { + /* + * GIT_SSH is the no-shell version of + * GIT_SSH_COMMAND (and must remain so for + * historical compatibility). + */ + conn->use_shell = 0; + + ssh = getenv("GIT_SSH"); + if (!ssh) + ssh = "ssh"; + variant = determine_ssh_variant(ssh, 0); + } + + if (variant == VARIANT_AUTO) { + struct child_process detect = CHILD_PROCESS_INIT; + + detect.use_shell = conn->use_shell; + detect.no_stdin = detect.no_stdout = detect.no_stderr = 1; + + argv_array_push(&detect.args, ssh); + argv_array_push(&detect.args, "-G"); + push_ssh_options(&detect.args, &detect.env_array, + VARIANT_SSH, port, flags); + argv_array_push(&detect.args, ssh_host); + + variant = run_command(&detect) ? VARIANT_SIMPLE : VARIANT_SSH; + } + + argv_array_push(&conn->args, ssh); + push_ssh_options(&conn->args, &conn->env_array, variant, port, flags); + argv_array_push(&conn->args, ssh_host); +} + +/* + * This returns the dummy child_process `no_fork` if the transport protocol + * does not need fork(2), or a struct child_process object if it does. Once + * done, finish the connection with finish_connect() with the value returned + * from this function (it is safe to call finish_connect() with NULL to + * support the former case). * * If it returns, the connect is successful; it just dies on errors (this * will hopefully be changed in a libification effort, to return NULL when @@ -865,7 +1049,7 @@ struct child_process *git_connect(int fd[2], const char *url, const char *prog, int flags) { char *hostandport, *path; - struct child_process *conn = &no_fork; + struct child_process *conn; enum protocol protocol; /* Without this we cannot rely on waitpid() to tell @@ -881,50 +1065,7 @@ struct child_process *git_connect(int fd[2], const char *url, printf("Diag: path=%s\n", path ? path : "NULL"); conn = NULL; } else if (protocol == PROTO_GIT) { - struct strbuf request = STRBUF_INIT; - /* - * Set up virtual host information based on where we will - * connect, unless the user has overridden us in - * the environment. - */ - char *target_host = getenv("GIT_OVERRIDE_VIRTUAL_HOST"); - if (target_host) - target_host = xstrdup(target_host); - else - target_host = xstrdup(hostandport); - - transport_check_allowed("git"); - - /* These underlying connection commands die() if they - * cannot connect. - */ - if (git_use_proxy(hostandport)) - conn = git_proxy_connect(fd, hostandport); - else - git_tcp_connect(fd, hostandport, flags); - /* - * Separate original protocol components prog and path - * from extended host header with a NUL byte. - * - * Note: Do not add any other headers here! Doing so - * will cause older git-daemon servers to crash. - */ - strbuf_addf(&request, - "%s %s%chost=%s%c", - prog, path, 0, - target_host, 0); - - /* If using a new version put that stuff here after a second null byte */ - if (get_protocol_version_config() > 0) { - strbuf_addch(&request, '\0'); - strbuf_addf(&request, "version=%d%c", - get_protocol_version_config(), '\0'); - } - - packet_write(fd[1], request.buf, request.len); - - free(target_host); - strbuf_release(&request); + conn = git_connect_git(fd, hostandport, path, prog, flags); } else { struct strbuf cmd = STRBUF_INIT; const char *const *var; @@ -946,8 +1087,6 @@ struct child_process *git_connect(int fd[2], const char *url, conn->use_shell = 1; conn->in = conn->out = -1; if (protocol == PROTO_SSH) { - const char *ssh; - enum ssh_variant variant; char *ssh_host = hostandport; const char *port = NULL; transport_check_allowed("ssh"); @@ -969,57 +1108,7 @@ struct child_process *git_connect(int fd[2], const char *url, strbuf_release(&cmd); return NULL; } - - if (looks_like_command_line_option(ssh_host)) - die("strange hostname '%s' blocked", ssh_host); - - ssh = get_ssh_command(); - if (ssh) { - variant = determine_ssh_variant(ssh, 1); - } else { - /* - * GIT_SSH is the no-shell version of - * GIT_SSH_COMMAND (and must remain so for - * historical compatibility). - */ - conn->use_shell = 0; - - ssh = getenv("GIT_SSH"); - if (!ssh) - ssh = "ssh"; - variant = determine_ssh_variant(ssh, 0); - } - - argv_array_push(&conn->args, ssh); - - if (variant == VARIANT_SSH && - get_protocol_version_config() > 0) { - argv_array_push(&conn->args, "-o"); - argv_array_push(&conn->args, "SendEnv=" GIT_PROTOCOL_ENVIRONMENT); - argv_array_pushf(&conn->env_array, GIT_PROTOCOL_ENVIRONMENT "=version=%d", - get_protocol_version_config()); - } - - if (variant != VARIANT_SIMPLE) { - if (flags & CONNECT_IPV4) - argv_array_push(&conn->args, "-4"); - else if (flags & CONNECT_IPV6) - argv_array_push(&conn->args, "-6"); - } - - if (variant == VARIANT_TORTOISEPLINK) - argv_array_push(&conn->args, "-batch"); - - if (port && variant != VARIANT_SIMPLE) { - if (variant == VARIANT_SSH) - argv_array_push(&conn->args, "-p"); - else - argv_array_push(&conn->args, "-P"); - - argv_array_push(&conn->args, port); - } - - argv_array_push(&conn->args, ssh_host); + fill_ssh_args(conn, ssh_host, port, flags); } else { transport_check_allowed("file"); if (get_protocol_version_config() > 0) { @@ -1041,11 +1130,6 @@ struct child_process *git_connect(int fd[2], const char *url, return conn; } -int git_connection_is_socket(struct child_process *conn) -{ - return conn == &no_fork; -} - int finish_connect(struct child_process *conn) { int code; diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh index ef94af9fcc..0f895478f0 100755 --- a/t/t5601-clone.sh +++ b/t/t5601-clone.sh @@ -306,23 +306,21 @@ test_expect_success 'clone checking out a tag' ' test_cmp fetch.expected fetch.actual ' -setup_ssh_wrapper () { - test_expect_success 'setup ssh wrapper' ' - rm -f "$TRASH_DIRECTORY/ssh$X" && - cp "$GIT_BUILD_DIR/t/helper/test-fake-ssh$X" \ - "$TRASH_DIRECTORY/ssh$X" && - GIT_SSH="$TRASH_DIRECTORY/ssh$X" && - export GIT_SSH && - export TRASH_DIRECTORY && - >"$TRASH_DIRECTORY"/ssh-output - ' -} +test_expect_success 'set up ssh wrapper' ' + cp "$GIT_BUILD_DIR/t/helper/test-fake-ssh$X" \ + "$TRASH_DIRECTORY/ssh$X" && + GIT_SSH="$TRASH_DIRECTORY/ssh$X" && + export GIT_SSH && + export TRASH_DIRECTORY && + >"$TRASH_DIRECTORY"/ssh-output +' copy_ssh_wrapper_as () { rm -f "${1%$X}$X" && cp "$TRASH_DIRECTORY/ssh$X" "${1%$X}$X" && + test_when_finished "rm $(git rev-parse --sq-quote "${1%$X}$X")" && GIT_SSH="${1%$X}$X" && - export GIT_SSH + test_when_finished "GIT_SSH=\"\$TRASH_DIRECTORY/ssh\$X\"" } expect_ssh () { @@ -346,8 +344,6 @@ expect_ssh () { (cd "$TRASH_DIRECTORY" && test_cmp ssh-expect ssh-output) } -setup_ssh_wrapper - test_expect_success 'clone myhost:src uses ssh' ' git clone myhost:src ssh-clone && expect_ssh myhost src @@ -369,23 +365,50 @@ test_expect_success 'OpenSSH variant passes -4' ' expect_ssh "-4 -p 123" myhost src ' -test_expect_success 'variant can be overriden' ' - git -c ssh.variant=simple clone -4 "[myhost:123]:src" ssh-simple-clone && - expect_ssh myhost src +test_expect_success 'variant can be overridden' ' + copy_ssh_wrapper_as "$TRASH_DIRECTORY/putty" && + git -c ssh.variant=putty clone -4 "[myhost:123]:src" ssh-putty-clone && + expect_ssh "-4 -P 123" myhost src ' -test_expect_success 'simple is treated as simple' ' +test_expect_success 'variant=auto picks based on basename' ' + copy_ssh_wrapper_as "$TRASH_DIRECTORY/plink" && + git -c ssh.variant=auto clone -4 "[myhost:123]:src" ssh-auto-clone && + expect_ssh "-4 -P 123" myhost src +' + +test_expect_success 'simple does not support -4/-6' ' copy_ssh_wrapper_as "$TRASH_DIRECTORY/simple" && - git clone -4 "[myhost:123]:src" ssh-bracket-clone-simple && - expect_ssh myhost src + test_must_fail git clone -4 "myhost:src" ssh-4-clone-simple +' + +test_expect_success 'simple does not support port' ' + copy_ssh_wrapper_as "$TRASH_DIRECTORY/simple" && + test_must_fail git clone "[myhost:123]:src" ssh-bracket-clone-simple ' test_expect_success 'uplink is treated as simple' ' copy_ssh_wrapper_as "$TRASH_DIRECTORY/uplink" && - git clone "[myhost:123]:src" ssh-bracket-clone-uplink && + test_must_fail git clone "[myhost:123]:src" ssh-bracket-clone-uplink && + git clone "myhost:src" ssh-clone-uplink && expect_ssh myhost src ' +test_expect_success 'OpenSSH-like uplink is treated as ssh' ' + write_script "$TRASH_DIRECTORY/uplink" <<-EOF && + if test "\$1" = "-G" + then + exit 0 + fi && + exec "\$TRASH_DIRECTORY/ssh$X" "\$@" + EOF + test_when_finished "rm -f \"\$TRASH_DIRECTORY/uplink\"" && + GIT_SSH="$TRASH_DIRECTORY/uplink" && + test_when_finished "GIT_SSH=\"\$TRASH_DIRECTORY/ssh\$X\"" && + git clone "[myhost:123]:src" ssh-bracket-clone-sshlike-uplink && + expect_ssh "-p 123" myhost src +' + test_expect_success 'plink is treated specially (as putty)' ' copy_ssh_wrapper_as "$TRASH_DIRECTORY/plink" && git clone "[myhost:123]:src" ssh-bracket-clone-plink-0 && @@ -434,12 +457,14 @@ test_expect_success 'ssh.variant overrides plink detection' ' ' test_expect_success 'GIT_SSH_VARIANT overrides plink detection to plink' ' + copy_ssh_wrapper_as "$TRASH_DIRECTORY/plink" && GIT_SSH_VARIANT=plink \ git clone "[myhost:123]:src" ssh-bracket-clone-variant-3 && expect_ssh "-P 123" myhost src ' test_expect_success 'GIT_SSH_VARIANT overrides plink to tortoiseplink' ' + copy_ssh_wrapper_as "$TRASH_DIRECTORY/plink" && GIT_SSH_VARIANT=tortoiseplink \ git clone "[myhost:123]:src" ssh-bracket-clone-variant-4 && expect_ssh "-batch -P 123" myhost src @@ -451,9 +476,6 @@ test_expect_success 'clean failure on broken quoting' ' git clone "[myhost:123]:src" sq-failure ' -# Reset the GIT_SSH environment variable for clone tests. -setup_ssh_wrapper - counter=0 # $1 url # $2 none|host diff --git a/t/t5603-clone-dirname.sh b/t/t5603-clone-dirname.sh index d5af758129..13b5e5eb9b 100755 --- a/t/t5603-clone-dirname.sh +++ b/t/t5603-clone-dirname.sh @@ -11,7 +11,9 @@ test_expect_success 'setup ssh wrapper' ' git upload-pack "$TRASH_DIRECTORY" EOF GIT_SSH="$TRASH_DIRECTORY/ssh-wrapper" && + GIT_SSH_VARIANT=ssh && export GIT_SSH && + export GIT_SSH_VARIANT && export TRASH_DIRECTORY ' |