And the patch actually attached here.

On Mon, Apr 01, 2013 at 11:01:42PM +0800, Matt Johnston wrote:
> Hi,
> 
> The attached attached patch against 2013.56 should fix it, or
> https://secure.ucc.asn.au/hg/dropbear/rev/70811267715c
> 
> Dropbear wasn't running cleanup handlers when it exited due
> to the TCP connection being closed.
> 
> Matt
> 
> On Thu, Mar 28, 2013 at 07:24:55PM +0800, Matt Johnston wrote:
> > I think that -K on the server should be enough. On the
> > server can you run "tcpdump -i eth0 -w cap1.cap port 22",
> > get a ssh session going, pull out the cable, wait 10
> > minutes, then send me the capture?
> > 
> > Could you also check that the Dropbear process for the
> > connection is still running after the connection should have
> > been finished. It's possible that the process is exiting but
> > the session cleanup code isn't working correctly. The whole
> > debug log might give me an idea what's going on.
> > 
> > Cheers,
> > Matt
> > 
> > On Thu, Mar 28, 2013 at 09:56:02AM +0100, Mattias Walström wrote:
> > > Thanks for your responses, all your suggestions imply that you should do 
> > > something
> > > in the client (set keepalive on client end), but shouldn't the server 
> > > itself be able to
> > > decide if a client is dead (can't OpenSSH do this?).
> > > 
> > > If I do the -K 15 -I 20 on the server end only, this will close the 
> > > connection when
> > > the OpenSSH client has not sent any characters in 20s. I expected the 
> > > keepalive to be
> > > two way, that the server got responses on these packages as well, is that 
> > > not the case?
> > > 
> > > Regards
> > >  Mattias
> > 
> > > >>On Wed, Mar 27, 2013 at 11:24 AM, Mattias Walström <
> > > >>mattias.walst...@westermo.se> wrote:
> > > >>
> > > >>>Hi!
> > > >>>I am running dropbear 2013.56, connecting to the server with a PC but
> > > >>>not performing a clean close (I pulled my ethernet cable), this caused
> > > >>>dropbear to never drop its connection.
> > > >>>
> > > >>>Looking at the utmp entries, I could see that the connection never got
> > > >>>dropped,
> > > >>>the utmp entries was kept forever, and running with debug indicates 
> > > >>>that
> > > >>>also.
> > > >>>  Tried to use -K to send keepalive, but it just keeps sending 
> > > >>> keepalives
> > > >>>to the peer,
> > > >>>even it is no longer there, and not possible to reach. Shouldn't
> > > >>>the connection be dropped if the keepalive does not reach its 
> > > >>>destination?
> > > >>>
> > > >>>I know there is the -I option, but that does not really do what I want,
> > > >>>I want the connection to be tear down when the peer is unreachable, not
> > > >>>when the user has been idle for a while.
> > > >>>
> > > >>>Regards
> > > >>>  Mattias
> > > >>>
> > > 
diff -r 1b8b2b9d6e94 cli-main.c
--- a/cli-main.c	Thu Mar 21 23:29:04 2013 +0800
+++ b/cli-main.c	Mon Apr 01 22:59:26 2013 +0800
@@ -98,8 +98,7 @@
 	}
 
 	/* Do the cleanup first, since then the terminal will be reset */
-	cli_session_cleanup();
-	common_session_cleanup();
+	session_cleanup();
 
 	_dropbear_log(LOG_INFO, fmtbuf, param);
 
diff -r 1b8b2b9d6e94 cli-session.c
--- a/cli-session.c	Thu Mar 21 23:29:04 2013 +0800
+++ b/cli-session.c	Mon Apr 01 22:59:26 2013 +0800
@@ -41,6 +41,7 @@
 static void cli_sessionloop();
 static void cli_session_init();
 static void cli_finished();
+static void cli_session_cleanup(void);
 
 struct clientsession cli_ses; /* GLOBAL */
 
@@ -142,6 +143,7 @@
 
 	/* For printing "remote host closed" for the user */
 	ses.remoteclosed = cli_remoteclosed;
+	ses.extra_session_cleanup = cli_session_cleanup;
 	ses.buf_match_algo = cli_buf_match_algo;
 
 	/* packet handlers */
@@ -278,7 +280,7 @@
 
 }
 
-void cli_session_cleanup() {
+static void cli_session_cleanup(void) {
 
 	if (!sessinitdone) {
 		return;
@@ -296,8 +298,7 @@
 
 static void cli_finished() {
 
-	cli_session_cleanup();
-	common_session_cleanup();
+	session_cleanup();
 	fprintf(stderr, "Connection to %s@%s:%s closed.\n", cli_opts.username,
 			cli_opts.remotehost, cli_opts.remoteport);
 	exit(cli_ses.retval);
diff -r 1b8b2b9d6e94 common-session.c
--- a/common-session.c	Thu Mar 21 23:29:04 2013 +0800
+++ b/common-session.c	Mon Apr 01 22:59:26 2013 +0800
@@ -225,7 +225,7 @@
 }
 
 /* clean up a session on exit */
-void common_session_cleanup() {
+void session_cleanup() {
 	
 	TRACE(("enter session_cleanup"))
 	
@@ -234,6 +234,10 @@
 		TRACE(("leave session_cleanup: !sessinitdone"))
 		return;
 	}
+
+	if (ses.extra_session_cleanup) {
+		ses.extra_session_cleanup();
+	}
 	
 	m_free(ses.session_id);
 	m_burn(ses.keys, sizeof(struct key_context));
diff -r 1b8b2b9d6e94 session.h
--- a/session.h	Thu Mar 21 23:29:04 2013 +0800
+++ b/session.h	Mon Apr 01 22:59:26 2013 +0800
@@ -44,7 +44,7 @@
 
 void common_session_init(int sock_in, int sock_out);
 void session_loop(void(*loophandler)());
-void common_session_cleanup();
+void session_cleanup();
 void session_identification();
 void send_msg_ignore();
 
@@ -58,7 +58,6 @@
 
 /* Client */
 void cli_session(int sock_in, int sock_out);
-void cli_session_cleanup();
 void cleantext(unsigned char* dirtytext);
 
 /* crypto parameters that are stored individually for transmit and receive */
@@ -178,6 +177,7 @@
 	void(*remoteclosed)(); /* A callback to handle closure of the
 									  remote connection */
 
+	void(*extra_session_cleanup)(); /* client or server specific cleanup */
 
 	struct AuthState authstate; /* Common amongst client and server, since most
 								   struct elements are common */
diff -r 1b8b2b9d6e94 svr-session.c
--- a/svr-session.c	Thu Mar 21 23:29:04 2013 +0800
+++ b/svr-session.c	Mon Apr 01 22:59:26 2013 +0800
@@ -72,6 +72,13 @@
 	NULL /* Null termination is mandatory. */
 };
 
+static void
+svr_session_cleanup(void)
+{
+	/* free potential public key options */
+	svr_pubkey_options_cleanup();
+}
+
 void svr_session(int sock, int childpipe) {
 	char *host, *port;
 	size_t len;
@@ -103,6 +110,7 @@
 
 	/* set up messages etc */
 	ses.remoteclosed = svr_remoteclosed;
+	ses.extra_session_cleanup = svr_session_cleanup;
 
 	/* packet handlers */
 	ses.packettypes = svr_packettypes;
@@ -160,11 +168,8 @@
 	if (svr_ses.server_pid == getpid())
 #endif
 	{
-		/* free potential public key options */
-		svr_pubkey_options_cleanup();
-
 		/* must be after we've done with username etc */
-		common_session_cleanup();
+		session_cleanup();
 	}
 
 	exit(exitcode);

Reply via email to