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

[Dan Yefimov]

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!
> 
> --- sshconnect.c~old    2007-07-25 10:44:26.000000000 -0700
> +++ sshconnect.c        2007-07-25 14:45:57.000000000 -0700
> @@ -404,9 +404,26 @@ ssh_exchange_identification(void)
>         int minor1 = PROTOCOL_MINOR_1;
>         u_int i, n;
> 
> +       if (options.server_alive_interval) {
> +               fd_set rfds;
> +               struct timeval timeo = { .tv_usec=0 };
> +               int read_timeouts, ret;
> +
> +               FD_SET(connection_in, &rfds);
> +               for (read_timeouts = 0;;) {
> +                       timeo.tv_sec = options.server_alive_interval;
> +                       ret = select(connection_in+1, &rfds, NULL, NULL,
> &timeo);
> +                       if (ret < 0) {

I think you should handle errno == EINTR case here restarting select(), 
otherwise any arriving signal would make ssh fail here.

> +                               fatal("ssh_exchange_identification: select
> read error: %.100s", strerror(errno));
> +                       } else if (ret == 0) {
> +                               if (++read_timeouts >=
> options.server_alive_count_max)
> +                                       fatal("ssh_exchange_identification:
> Timeout, server not responding");
> +                       } else
> +                               break;
> +               }
> +
> +       }
>         /* Read other side's version identification. */
> -       struct timeval timeo = { .tv_sec=10, .tv_usec=0 };
> -       setsockopt(connection_in, SOL_SOCKET, SO_SNDTIMEO, &timeo,
> sizeof(timeo));
>         for (n = 0;;) {
>                 for (i = 0; i < sizeof(buf) - 1; i++) {
>                         size_t len = atomicio(read, connection_in, &buf[i],
> 1);
> @@ -490,6 +507,25 @@ ssh_exchange_identification(void)
>             compat20 ? PROTOCOL_MAJOR_2 : PROTOCOL_MAJOR_1,
>             compat20 ? PROTOCOL_MINOR_2 : minor1,
>             SSH_VERSION);
> +       if (options.server_alive_interval) {
> +               fd_set wfds;
> +               struct timeval timeo = { .tv_usec=0 };
> +               int write_timeouts, ret;
> +
> +               FD_SET(connection_out, &wfds);
> +               for (write_timeouts = 0;;) {
> +                       timeo.tv_sec = options.server_alive_interval;
> +                       ret = select(connection_out+1, NULL, &wfds, NULL,
> &timeo);
> +                       if (ret < 0) {

The same notice as above here.

> +                               fatal("ssh_exchange_identification: select
> write error: %.100s", strerror(errno));
> +                       } else if (ret == 0) {
> +                               if (++write_timeouts >=
> options.server_alive_count_max)
> +                                       fatal("ssh_exchange_identification:
> Timeout, server not responding");
> +                       } else
> +                               break;
> +               }
> +
> +       }
>         if (atomicio(vwrite, connection_out, buf, strlen(buf)) !=
> strlen(buf))
>                 fatal("write: %.100s", strerror(errno));
>         client_version_string = xstrdup(buf);
> 
-- 

    Sincerely Your, Dan.