Idletimeout patch

[Jani Jaakkola]

On Mon, 20 Aug 2001, Markus Friedl wrote:

> i don't think that both dispatch.c and packet.c should be touched.

I agree, but I could not find any other way to hook to both incoming
and outgoing packets with their types known (at least without doing
de-assembly of allready assembled packets).

> openssh-current can parse time.

Yes, I noticed this in Kevin Steves patch. I have the cvs version of
openssh-current and could provide the patch against it too (given a few
hours a time) if we can agree how this should be implemented.

> > +/* Called by dispatch.c and packet.c whenever packets are sent or received.
> > + * This function decides whenever server idletimeout should be reset */
> > +void idletimeout_check(int type, int received) {
> > +	case SSH2_MSG_KEXINIT:         /* Ignore rekeying */
> > +	case SSH2_MSG_NEWKEYS:
> > +	case SSH2_MSG_KEX_DH_GEX_REQUEST_OLD:
> > +	case SSH2_MSG_KEX_DH_GEX_GROUP:
> > +	case SSH2_MSG_KEX_DH_GEX_INIT:
> > +	case SSH2_MSG_KEX_DH_GEX_REPLY:
> > +	case SSH2_MSG_KEX_DH_GEX_REQUEST:
>
> there are ranges defined for KEX packets.
>
> not that kex packets are only defined for protocol 2
> so you break ssh1 with this patch.

I don't think so and I actually tested that it works. Those messages are
never seen in the version 1 protocol, so the case will never be taken.
And even if it would, it would only skip the reset of idletimeout, which
would not be a big deal.

> > +	case SSH_MSG_NONE:             /* Ignore dummies */
>
> this is not allowed on the wire. it's a protocol violation.

OK, I never did bother to read the spec.

> > +			return;
> > +		}
> > +	}
> > +	/* Reset idletime */
> > +	time(&idletime_last);
> > +}
>
> ^^^ i don't like this at all. i think it should be
> simpler to have a positive list. e.g. all packets
> related to interactive traffic, e.g channel/stdin/out/err
> packets reset the counter, control messages don't).

OK, That would probably be simpler and easier to maintain. I will fix
that.

> so many changes.....

Yes, I too thought that it would be easier.  Maybe I should make it a
configure option and tag it as 'experimental' (which off course would make
the patch even bigger).

- Jani