Bug 36 - Channels for rejected X sessions hang
Summary: Channels for rejected X sessions hang
Status: CLOSED FIXED
Alias: None
Product: Portable OpenSSH
Classification: Unclassified
Component: ssh (show other bugs)
Version: -current
Hardware: All SunOS
: P4 minor
Assignee: Markus Friedl
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2001-12-11 10:46 AEDT by Dan Astoorian
Modified: 2004-04-14 12:24 AEST (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Dan Astoorian 2001-12-11 10:46:20 AEDT
Whenever an X connection is rejected (e.g., "X11 connection uses different
authentication protocol" or "X11 auth data does not match fake data"), the
channel is not cleaned up.  chan_read_failed(c) is called by the ssh client, but
this merely puts the channel into CHAN_INPUT_WAIT_DRAIN forever.

As a result, the Xclient never sees its connection close, and the ssh session
doesn't end cleanly when the user's shell exits.

To reproduce:
	ssh -X hostname
	% env XAUTHORITY=/dev/null xterm  # rejected X connection
	[hangs]
	^C
	% exit
	[hangs]
	^CKilled by signal 2.

The patch below (written for 3.0.2p1, but I believe it should apply to -current)
calls chan_ibuf_empty() to send the input channel state to
CHAN_INPUT_WAIT_OCLOSE and send EOF over the channel: this fixes the symptom for
me, although it's not obvious to me that I've covered all cases.

========================================================================
--- channels.c.orig     Thu Oct 11 21:35:05 2001
+++ channels.c  Wed Dec  5 16:35:18 2001
@@ -872,7 +872,9 @@
 	} else if (ret == -1) {
 		debug("X11 rejected %d i%d/o%d", c->self, c->istate, c->ostate);
 		chan_read_failed(c);    /** force close? */
-		chan_write_failed(c);
+		chan_ibuf_empty(c);
+		if (compat20)
+			chan_write_failed(c);
 		debug("X11 closed %d i%d/o%d", c->self, c->istate, c->ostate);
 	}
 }
========================================================================

(Previously reported to <openssh-unix-dev@mindrot.org>.)
Comment 1 Markus Friedl 2001-12-29 00:35:24 AEDT
i'll look into this
Comment 2 Markus Friedl 2002-01-05 21:44:52 AEDT
i'm going to add this:


@@ -872,9 +872,17 @@
                else
                        channel_pre_open_15(c, readset, writeset);
        } else if (ret == -1) {
+               log("X11 connection rejected because of wrong authentication.");
                debug("X11 rejected %d i%d/o%d", c->self, c->istate, c->ostate);
-               chan_read_failed(c);    /** force close? */
-               chan_write_failed(c);
+               chan_read_failed(c);
+               buffer_clear(&c->input);
+               chan_ibuf_empty(c);
+               buffer_clear(&c->output);
+               /* for proto v1, the peer will send an IEOF */
+               if (compat20)
+                       chan_write_failed(c);
+               else
+                       c->type = SSH_CHANNEL_OPEN;
                debug("X11 closed %d i%d/o%d", c->self, c->istate, c->ostate);
        }
 }
@@ -1807,7 +1815,7 @@
        chan_rcvd_ieof(c);
 
        /* XXX force input close */
-       if (c->force_drain) {
+       if (c->force_drain && c->istate == CHAN_INPUT_OPEN) {
                debug("channel %d: FORCE input drain", c->self);
                c->istate = CHAN_INPUT_WAIT_DRAIN;
        }
Comment 3 Damien Miller 2004-04-14 12:24:17 AEST
Mass change of RESOLVED bugs to CLOSED