ssh client does not timeout if the network fails after ssh_connect but before ssh_exchange_identification, even with Alive options set
Jiaying Zhang
jiayingz at google.com
Fri Jul 27 12:51:42 EST 2007
Thanks for the updates.
Jiaying
On 7/26/07, Damien Miller <djm at mindrot.org> wrote:
>
> On Wed, 25 Jul 2007, Jiaying Zhang wrote:
>
> > Hello again,
> >
> > Here is the patch I came up with to prevent the hanging in
> > ssh_exchange_identification. I tested it a little bit and it seems to
> have
> > solved the problem. Could anyone help to have a look at the patch?
> Thanks a
> > lot!
>
> Here is a patch that I did a while ago to make ConnectTimeout apply
> to banner exchange too (updated to -current):
>
> Index: ssh.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/ssh/ssh.c,v
> retrieving revision 1.300
> diff -u -p -r1.300 ssh.c
> --- ssh.c 14 Jun 2007 22:48:05 -0000 1.300
> +++ ssh.c 27 Jul 2007 02:14:11 -0000
> @@ -202,7 +202,7 @@ main(int ac, char **av)
> char *p, *cp, *line, buf[256];
> struct stat st;
> struct passwd *pw;
> - int dummy;
> + int dummy, timeout_ms;
> extern int optind, optreset;
> extern char *optarg;
> struct servent *sp;
> @@ -666,13 +666,19 @@ main(int ac, char **av)
> if (options.control_path != NULL)
> control_client(options.control_path);
>
> + timeout_ms = options.connection_timeout * 1000;
> +
> /* Open a connection to the remote host. */
> if (ssh_connect(host, &hostaddr, options.port,
> - options.address_family, options.connection_attempts,
> + options.address_family, options.connection_attempts,
> &timeout_ms,
> + options.tcp_keep_alive,
> original_effective_uid == 0 && options.use_privileged_port,
> options.proxy_command) != 0)
> exit(255);
>
> + if (timeout_ms > 0)
> + debug3("timeout: %d ms remain after connect", timeout_ms);
> +
> /*
> * If we successfully made the connection, load the host private
> key
> * in case we will need it later for combined rsa-rhosts
> @@ -748,7 +754,8 @@ main(int ac, char **av)
> signal(SIGPIPE, SIG_IGN); /* ignore SIGPIPE early */
>
> /* Log into the remote system. This never returns if the login
> fails. */
> - ssh_login(&sensitive_data, host, (struct sockaddr *)&hostaddr,
> pw);
> + ssh_login(&sensitive_data, host, (struct sockaddr *)&hostaddr,
> + pw, timeout_ms);
>
> /* We no longer need the private host keys. Clear them now. */
> if (sensitive_data.nkeys != 0) {
> Index: sshconnect.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/ssh/sshconnect.c,v
> retrieving revision 1.200
> diff -u -p -r1.200 sshconnect.c
> --- sshconnect.c 10 Oct 2006 10:12:45 -0000 1.200
> +++ sshconnect.c 27 Jul 2007 02:14:11 -0000
> @@ -64,6 +64,23 @@ extern pid_t proxy_command_pid;
> static int show_other_keys(const char *, Key *);
> static void warn_changed_key(Key *);
>
> +static void
> +ms_subtract_time(struct timeval *start, int *ms)
> +{
> + struct timeval diff, finish;
> +
> + gettimeofday(&finish, NULL);
> + timersub(&finish, start, &diff);
> + *ms -= (diff.tv_sec * 1000) + (diff.tv_usec / 1000);
> +}
> +
> +static void
> +ms_to_timeval(struct timeval *tv, int ms)
> +{
> + tv->tv_sec = ms / 1000;
> + tv->tv_usec = (ms % 1000) * 1000;
> +}
> +
> /*
> * Connect to the given ssh server using a proxy command.
> */
> @@ -207,30 +224,36 @@ ssh_create_socket(int privileged, struct
>
> static int
> timeout_connect(int sockfd, const struct sockaddr *serv_addr,
> - socklen_t addrlen, int timeout)
> + socklen_t addrlen, int *timeoutp)
> {
> fd_set *fdset;
> - struct timeval tv;
> + struct timeval tv, t_start;
> socklen_t optlen;
> int optval, rc, result = -1;
>
> - if (timeout <= 0)
> - return (connect(sockfd, serv_addr, addrlen));
> + gettimeofday(&t_start, NULL);
> +
> + if (*timeoutp <= 0) {
> + result = connect(sockfd, serv_addr, addrlen);
> + goto done;
> + }
>
> set_nonblock(sockfd);
> rc = connect(sockfd, serv_addr, addrlen);
> if (rc == 0) {
> unset_nonblock(sockfd);
> - return (0);
> + result = 0;
> + goto done;
> + }
> + if (errno != EINPROGRESS) {
> + result = -1;
> + goto done;
> }
> - if (errno != EINPROGRESS)
> - return (-1);
>
> fdset = (fd_set *)xcalloc(howmany(sockfd + 1, NFDBITS),
> sizeof(fd_mask));
> FD_SET(sockfd, fdset);
> - tv.tv_sec = timeout;
> - tv.tv_usec = 0;
> + ms_to_timeval(&tv, *timeoutp);
>
> for (;;) {
> rc = select(sockfd + 1, NULL, fdset, NULL, &tv);
> @@ -269,6 +292,11 @@ timeout_connect(int sockfd, const struct
> }
>
> xfree(fdset);
> +
> + done:
> + if (result == 0 && *timeoutp > 0)
> + ms_subtract_time(&t_start, timeoutp);
> +
> return (result);
> }
>
> @@ -285,8 +313,8 @@ timeout_connect(int sockfd, const struct
> */
> int
> ssh_connect(const char *host, struct sockaddr_storage * hostaddr,
> - u_short port, int family, int connection_attempts,
> - int needpriv, const char *proxy_command)
> + u_short port, int family, int connection_attempts, int *timeout_ms,
> + int want_keepalive, int needpriv, const char *proxy_command)
> {
> int gaierr;
> int on = 1;
> @@ -339,7 +367,7 @@ ssh_connect(const char *host, struct soc
> continue;
>
> if (timeout_connect(sock, ai->ai_addr,
> ai->ai_addrlen,
> - options.connection_timeout) >= 0) {
> + timeout_ms) >= 0) {
> /* Successful connection. */
> memcpy(hostaddr, ai->ai_addr,
> ai->ai_addrlen);
> break;
> @@ -366,7 +394,7 @@ ssh_connect(const char *host, struct soc
> debug("Connection established.");
>
> /* Set SO_KEEPALIVE if requested. */
> - if (options.tcp_keep_alive &&
> + if (want_keepalive &&
> setsockopt(sock, SOL_SOCKET, SO_KEEPALIVE, (void *)&on,
> sizeof(on)) < 0)
> error("setsockopt SO_KEEPALIVE: %.100s", strerror(errno));
> @@ -382,7 +410,7 @@ ssh_connect(const char *host, struct soc
> * identification string.
> */
> static void
> -ssh_exchange_identification(void)
> +ssh_exchange_identification(int timeout_ms)
> {
> char buf[256], remote_version[256]; /* must be same size! */
> int remote_major, remote_minor, mismatch;
> @@ -390,16 +418,44 @@ ssh_exchange_identification(void)
> int connection_out = packet_get_connection_out();
> int minor1 = PROTOCOL_MINOR_1;
> u_int i, n;
> + size_t len;
> + int fdsetsz, remaining, rc;
> + struct timeval t_start, t_remaining;
> + fd_set *fdset;
> +
> + fdsetsz = howmany(connection_in + 1, NFDBITS) * sizeof(fd_mask);
> + fdset = xcalloc(1, fdsetsz);
>
> /* Read other side's version identification. */
> + remaining = timeout_ms;
> for (n = 0;;) {
> for (i = 0; i < sizeof(buf) - 1; i++) {
> - size_t len = atomicio(read, connection_in,
> &buf[i], 1);
> + if (timeout_ms > 0) {
> + gettimeofday(&t_start, NULL);
> + ms_to_timeval(&t_remaining, remaining);
> + FD_SET(connection_in, fdset);
> + rc = select(connection_in + 1, fdset,
> NULL,
> + fdset, &t_remaining);
> + ms_subtract_time(&t_start, &remaining);
> + if (rc == 0 || remaining <= 0)
> + fatal("Connection timed out during
> "
> + "banner exchange");
> + if (rc == -1) {
> + if (errno == EINTR)
> + continue;
> +
> fatal("ssh_exchange_identification: "
> + "select: %s",
> strerror(errno));
> + }
> + }
> +
> + len = atomicio(read, connection_in, &buf[i], 1);
>
> if (len != 1 && errno == EPIPE)
> - fatal("ssh_exchange_identification:
> Connection closed by remote host");
> + fatal("ssh_exchange_identification: "
> + "Connection closed by remote host");
> else if (len != 1)
> - fatal("ssh_exchange_identification: read:
> %.100s", strerror(errno));
> + fatal("ssh_exchange_identification: "
> + "read: %.100s", strerror(errno));
> if (buf[i] == '\r') {
> buf[i] = '\n';
> buf[i + 1] = 0;
> @@ -410,7 +466,8 @@ ssh_exchange_identification(void)
> break;
> }
> if (++n > 65536)
> - fatal("ssh_exchange_identification: No
> banner received");
> + fatal("ssh_exchange_identification: "
> + "No banner received");
> }
> buf[sizeof(buf) - 1] = 0;
> if (strncmp(buf, "SSH-", 4) == 0)
> @@ -418,6 +475,7 @@ ssh_exchange_identification(void)
> debug("ssh_exchange_identification: %s", buf);
> }
> server_version_string = xstrdup(buf);
> + xfree(fdset);
>
> /*
> * Check that the versions match. In future this might accept
> @@ -926,7 +984,7 @@ verify_host_key(char *host, struct socka
> */
> void
> ssh_login(Sensitive *sensitive, const char *orighost,
> - struct sockaddr *hostaddr, struct passwd *pw)
> + struct sockaddr *hostaddr, struct passwd *pw, int timeout_ms)
> {
> char *host, *cp;
> char *server_user, *local_user;
> @@ -941,7 +999,7 @@ ssh_login(Sensitive *sensitive, const ch
> *cp = (char)tolower(*cp);
>
> /* Exchange protocol version identification strings with the
> server. */
> - ssh_exchange_identification();
> + ssh_exchange_identification(timeout_ms);
>
> /* Put the connection into non-blocking mode. */
> packet_set_nonblocking();
> Index: sshconnect.h
> ===================================================================
> RCS file: /cvs/src/usr.bin/ssh/sshconnect.h,v
> retrieving revision 1.23
> diff -u -p -r1.23 sshconnect.h
> --- sshconnect.h 3 Aug 2006 03:34:42 -0000 1.23
> +++ sshconnect.h 27 Jul 2007 02:14:11 -0000
> @@ -33,10 +33,10 @@ struct Sensitive {
>
> int
> ssh_connect(const char *, struct sockaddr_storage *, u_short, int, int,
> - int, const char *);
> + int *, int, int, const char *);
>
> void
> -ssh_login(Sensitive *, const char *, struct sockaddr *, struct passwd *);
> +ssh_login(Sensitive *, const char *, struct sockaddr *, struct passwd *,
> int);
>
> int verify_host_key(char *, struct sockaddr *, Key *);
>
>
More information about the openssh-unix-dev
mailing list