Idletimeout patch

[Markus Friedl]

On Mon, Aug 20, 2001 at 04:30:17PM +0300, Jani Jaakkola wrote:
> diff -ru openssh-2.9p2.orig/dispatch.c openssh-2.9p2/dispatch.c
> --- openssh-2.9p2.orig/dispatch.c	Mon Feb 19 12:51:08 2001
> +++ openssh-2.9p2/dispatch.c	Mon Aug 20 15:00:38 2001
> @@ -69,9 +69,10 @@
>  			if (type == SSH_MSG_NONE)
>  				return;
>  		}
> -		if (type > 0 && type < DISPATCH_MAX && dispatch[type] != NULL)
> +		if (type > 0 && type < DISPATCH_MAX && dispatch[type] != NULL) {
> +		        idletimeout_check(type,1);
>  			(*dispatch[type])(type, plen, ctxt);
> -		else
> +		} else
>  			packet_disconnect("protocol error: rcvd type %d", type);
>  		if (done != NULL && *done)
>  			return;
> diff -ru openssh-2.9p2.orig/packet.c openssh-2.9p2/packet.c
> --- openssh-2.9p2.orig/packet.c	Fri Apr  6 02:26:33 2001
> +++ openssh-2.9p2/packet.c	Mon Aug 20 15:01:06 2001
> @@ -383,6 +383,7 @@
>  		packet_start2(type);
>  	else
>  		packet_start1(type);
> +        idletimeout_check(type,0);
>  }

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

> +               case sIdleTimeout:
> +			arg = strdelim(&cp);
> +			if (!arg || *arg == '\0')
> +				fatal("%s line %d: Missing IdleTimeout argument",
> +					filename,linenum);
> +			options->idletimeout=atoi(arg);
> +			switch(arg[strlen(arg)-1]) {
> +				case 'w': options->idletimeout*=7;
> +				case 'd': options->idletimeout*=24;
> +				case 'h': options->idletimeout*=60;
> +				case 'm': options->idletimeout*=60;
> +				case 's': 
> +				case '0': case '1': case '2': case '3': 
> +				case '4': case '5': case '6': case '7': 
> +				case '8': case '9':
> +	                               break;
> +				default:
> +					fatal("%s line %d: Invalid IdleTimeout argument",
> +						filename,linenum);
> +			}
> +			break;

openssh-current can parse time.

> +/* 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.

> +	case SSH_MSG_NONE:             /* Ignore dummies */

this is not allowed on the wire. it's a protocol violation.

> +	    debug("Server i/o idlecheck ignored msg %d, received=%d",
> +		  type,received);
> +	    return;
> +	}
> +
> +	if (received) {
> +	        /* The following packets do not reset idletimeout on input */
> +	        switch(type) {
> +		case SSH2_MSG_CHANNEL_FAILURE: /* used for client alive */
> +		        debug("Server i idlecheck ignored msg %d",type);
> +			return;
> +		}
> +	} else {
> +	        /* The following packets do not reset idletimeout on output */
> +	        switch(type) {
> +		case SSH2_MSG_CHANNEL_REQUEST: /* used for client alive */
> +		        debug("Server o idlecheck ignored msg %d",type);
> +			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).

> +
>  /*
>   * Sleep in select() until we can do something.  This will initialize the
>   * select masks.  Upon return, the masks will indicate which descriptors
> @@ -193,7 +234,9 @@
>  	struct timeval tv, *tvp;
>  	int ret;
>  	int client_alive_scheduled = 0;
> -
> +	/* Secs until idletimeout, zero if no idletimeout */
> +	int max_time_seconds=0; 
> +	
>  	/*
>  	 * if using client_alive, set the max timeout accordingly, 
>  	 * and indicate that this particular timeout was for client
> @@ -208,6 +251,24 @@
>  	} else 
>  	        client_alive_scheduled = 0;
>  
> +
> +	if (options.idletimeout>0) {
> +		if (idletime_last==0) {
> +			/* Initialize idletime_last */		
> +			time(&idletime_last);  
> +		}
> +		/* Schedule idletimeout if no other timeouts are scheduled.
> +		 * Idletimeouts are the longest and it is not a big deal,
> +		 * if they are missed by few seconds. */
> +		if (max_time_milliseconds == 0) {
> +			time_t diff=time(NULL)-idletime_last;
> +			if (diff>=options.idletimeout)
> +				max_time_seconds=1;
> +			else 
> +				max_time_seconds=options.idletimeout-diff;
> +		}
> +	}
> +	
>  	/* When select fails we restart from here. */
>  retry_select:
>  
> @@ -258,10 +319,17 @@
>  	if (child_terminated && packet_not_very_much_data_to_write())
>  		if (max_time_milliseconds == 0 || client_alive_scheduled)
>  			max_time_milliseconds = 100;
> -
> -	if (max_time_milliseconds == 0)
> -		tvp = NULL;
> -	else {
> +	
> +	if (max_time_milliseconds == 0) {
> +		/* Use max_time_seconds only if max_time_milliseconds is
> +		 * not set */
> +		if (max_time_seconds>0) {
> +			tv.tv_sec=max_time_seconds;
> +			tv.tv_usec=0;
> +			tvp=&tv;
> +		} else
> +			tvp = NULL;
> +	} else {
>  		tv.tv_sec = max_time_milliseconds / 1000;
>  		tv.tv_usec = 1000 * (max_time_milliseconds % 1000);
>  		tvp = &tv;
> @@ -301,7 +369,14 @@
>  				packet_disconnect(
>  					"No open channels after timeout!");
>  		}
> -	} 
> +	}
> +
> +	/* Check if idletimeout has happened */
> +	if (options.idletimeout>0 && ret==0 && 
> +	    time(NULL)-idletime_last>options.idletimeout) {
> +	            packet_disconnect("Idletimeout.");
> +		    options.idletimeout=0;
> +	}
>  }

so many changes.....