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

[Jiaying Zhang]

Hi Dan,

Thanks for your reply.

Yes, it can, but there is nothing to do with that (except for at least
> Linux,
> which always returns remaining timeout in the select() timeout parameter).
> Fortunately signals are relatively rare, so the timeout deviation is IMHO
> not
> essential. There are also other possible reasons for deviation, for
> example
> sending the packet to another end while network is down.


Agree. I was thinking to record the start time and subtract the elapsed time
when an interrupt happens. But I guess we can save that trouble since
signals are rare.


> > I also noticed that in server_alive_check() function, we check
> > if (++server_alive_timeouts > options.server_alive_count_max). Should it
> be
> > >=? According to the comment in ssh_config, if ServerAliveInterval
> equals to
> > 30 and ServerAliveCountMax equals to 3, ssh disconnects after 90
> seconds.
> > But with the current server_alive_check, we actually disconnects after
> 120
> > seconds.
> >
> Not right. Pay attention at '++server_alive_timeouts >
> options.server_alive_count_max' expression. server_alive_timeouts is
> pre-incremented there, that is it's incremented value is used while
> evaluating
> the expression, so the count begins with 1 rather than 0 there.


But still, when options.server_alive_count_max=1, we should exit after the
first timeout.

Some notices I (unfortunately) forgot to meantion previously:
>
> > diff -puN sshconnect.c~old sshconnect.c
> > --- sshconnect.c~old    2007-07-25 16:59:36.000000000 -0700
> > +++ sshconnect.c        2007-07-26 11:46:48.000000000 -0700
> > @@ -404,6 +404,28 @@ ssh_exchange_identification(void)
> >         int minor1 = PROTOCOL_MINOR_1;
> >         u_int i, n;
> >
> > +       if (options.server_alive_interval) {
> > +               fd_set rfds;
> > +               struct timeval timeo;
> > +               int read_timeouts, ret;
> > +
> > +               FD_SET(connection_in, &rfds);
>                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> (1) This should be placed inside the cycle before select() since on error
> (even
> EINTR) rfds along with timeo becomes undefined according to spec.


Thanks for catching this.

Jiaying