openssh unix dev - Mar 2004 - Patch for login exchange timeout

Here's a quick patch that adds a timeout (value set
from the connection timeout config parameter) for
identification exchange. The situation that this fixes
is the one where the sshd has a connection open (so
you can make a TCP connection) but does not put up a
banner (e.g. when the disk has crashed or when file
descriptors are exhausted on a machine). Sounds
unusual, but this happens with some regularity on
PlanetLab (www.planet-lab.org)... Anyway, that
situation is not caught be either the
ConnectionTimeout, TCPKeepAlive, or the ServerAlive
timeouts. This patch just sets an alarm prior to the
atomicio call that reads & processes the banner from
the sshd.

--Mic 
micbowman at comcast dot net


*** sshconnect.c	2004-01-27 02:21:27.000000000 -0800
--- sshconnect-new.c	2004-03-26 14:48:24.000000000
-0800
***************
*** 429,434 ****
--- 429,444 ----
  }
  
  /*
+  * Signal handler for the alarm after the login
grace period has expired.
+  */
+ static void
+ grace_alarm_handler(int sig)
+ {
+ 	/* Log error and exit. */
+ 	fatal("Timeout before authentication"); }
+ 
+ /*
   * Waits for the server identification string, and
sends our own
   * identification string.
   */
***************
*** 441,446 ****
--- 451,462 ----
  	int connection_out = packet_get_connection_out();
  	int minor1 = PROTOCOL_MINOR_1;
  
+ 	/* Setup a handler for banner prompt */
+ 	if (options.connection_timeout > 0) {
+ 	  signal(SIGALRM, grace_alarm_handler);
+ 	  alarm(options.connection_timeout);
+ 	}
+ 
  	/* Read other side\'s version identification. */
  	for (;;) {
  		for (i = 0; i < sizeof(buf) - 1; i++) {
***************
*** 466,471 ****
--- 482,494 ----
  	}
  	server_version_string = xstrdup(buf);
  
+ 	/* Clear the alarm */
+ 	if (options.connection_timeout > 0) {
+ 	  alarm(0);
+ 	  signal(SIGALRM,SIG_DFL);
+ 	}
+ 	
+ 
  	/*
  	 * Check that the versions match.  In future this
might accept
  	 * several versions and set appropriate flags to
handle them.



__________________________________
Do you Yahoo!?
Yahoo! Finance Tax Center - File online. File on time.
http://taxes.yahoo.com/filing.html

Mic Bowman wrote:
> Here's a quick patch that adds a timeout (value set
> from the connection timeout config parameter) for
> identification exchange. 
I think the idea is ok, but IMO the implementation should do
select+timeout rather than use signals.

Also, maybe the timeout for the banner exchange should be
ConnectionTimeout less the time actually used during the connect phase.

-d

I agree with select/timeout. This was a *fast*, minimal impact to the current
code kind of patch. Change it however you want.

--Mic
> Mic Bowman wrote:
> > Here's a quick patch that adds a timeout (value set
> > from the connection timeout config parameter) for
> > identification exchange. 
> 
> I think the idea is ok, but IMO the implementation should do
> select+timeout rather than use signals.
> 
> Also, maybe the timeout for the banner exchange should be
> ConnectionTimeout less the time actually used during the connect phase.
> 
> -d