Re: ssh client does not timeout if the network fails after ssh_connect

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 *);

_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev@mindrot.org
https://lists.mindrot.org/mailman/li...enssh-unix-dev