ssh client does not timeout if the network fails after ssh_connect but before ssh_exchange_identification, even with Alive options set

Damien Miller djm at mindrot.org
Fri Jul 27 12:15:20 EST 2007


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