openssh PTY allocation

Morty Abzug morty at frakir.org
Tue Aug 2 15:51:06 EST 2011


Thanks.  I tried this.  It only works for one of the two devices I've
been testing with.  The device that works runs ScreenOS 6.3.0r7.0.
The device that's still broken runs ScreenOS 5.3.0r3.0.

Knocking the threshold down from 256 to 128, though, yields joy with
both devices.  129 and 130 work, while 131 doesn't; presumably the
success of 129 and 130 is due to fenceposts.  Presumably the earlier
version of ScreenOS had a version with a 128 limit; ScreenOS ran into
interop problems at the time; ScreenOS coders bumped it to 256; and
now they/we are having interop problems again.  Maybe some even-older
version of ScreenOS had an even lower limit.  Hence why I'm thinking
that it might be better to make PTY allocation failure a non-fatal
error rather than trying to guess the other side's worst case buffer
len.

Note that this doesn't do anything for scp.  The compat hack stuff
does not seem to be available within scp.c, probably because it
doesn't look directly at the ssh banner.

Combining your compat concept with my earlier patch yields:

diff -ur openssh-5.8p2-orig/clientloop.c openssh-5.8p2-morty-p4/clientloop.c
--- openssh-5.8p2-orig/clientloop.c	Sun Jan 16 12:18:35 2011
+++ openssh-5.8p2-morty-p4/clientloop.c	Tue Aug  2 05:35:36 2011
@@ -1982,7 +1982,11 @@
 			memset(&ws, 0, sizeof(ws));
 
 		channel_request_start(id, "pty-req", 1);
-		client_expect_confirm(id, "PTY allocation", 1);
+		if (datafellows & SSH_BUG_SCREENOS_PTY) {
+			client_expect_confirm(id, "PTY allocation", 0);
+		} else {
+			client_expect_confirm(id, "PTY allocation", 1);
+		}
 		packet_put_cstring(term != NULL ? term : "");
 		packet_put_int((u_int)ws.ws_col);
 		packet_put_int((u_int)ws.ws_row);
diff -ur openssh-5.8p2-orig/compat.c openssh-5.8p2-morty-p4/compat.c
--- openssh-5.8p2-orig/compat.c	Mon Nov  3 08:20:14 2008
+++ openssh-5.8p2-morty-p4/compat.c	Tue Aug  2 05:35:36 2011
@@ -148,6 +148,8 @@
 					SSH_BUG_IGNOREMSG },
 		{ "*SSH Compatible Server*",			/* Netscreen */
 					SSH_BUG_PASSWORDPAD },
+               { "NetScreen",
+                                       SSH_BUG_SCREENOS_PTY },
 		{ "*OSU_0*,"
 		  "OSU_1.0*,"
 		  "OSU_1.1*,"
diff -ur openssh-5.8p2-orig/compat.h openssh-5.8p2-morty-p4/compat.h
--- openssh-5.8p2-orig/compat.h	Mon Nov  3 08:20:14 2008
+++ openssh-5.8p2-morty-p4/compat.h	Tue Aug  2 05:35:36 2011
@@ -58,6 +58,7 @@
 #define SSH_OLD_FORWARD_ADDR	0x01000000
 #define SSH_BUG_RFWD_ADDR	0x02000000
 #define SSH_NEW_OPENSSH		0x04000000
+#define SSH_BUG_SCREENOS_PTY	0x08000000
 
 void     enable_compat13(void);
 void     enable_compat20(void);
diff -ur openssh-5.8p2-orig/scp.c openssh-5.8p2-morty-p4/scp.c
--- openssh-5.8p2-orig/scp.c	Thu Jan  6 11:41:21 2011
+++ openssh-5.8p2-morty-p4/scp.c	Tue Aug  2 05:35:36 2011
@@ -273,7 +273,6 @@
 			addargs(&args, "-l");
 			addargs(&args, "%s", remuser);
 		}
-		addargs(&args, "--");
 		addargs(&args, "%s", host);
 		addargs(&args, "%s", cmd);
 
@@ -322,7 +321,6 @@
 			addargs(&args, "-l");
 			addargs(&args, "%s", remuser);
 		}
-		addargs(&args, "--");
 		addargs(&args, "%s", host);
 		addargs(&args, "%s", cmd);
 
@@ -601,12 +599,12 @@
 				host = cleanhostname(argv[i]);
 				suser = NULL;
 			}
-			xasprintf(&bp, "%s -f -- %s", cmd, src);
+			xasprintf(&bp, "%s -f %s", cmd, src);
 			if (do_cmd(host, suser, bp, &remin, &remout) < 0)
 				exit(1);
 			(void) xfree(bp);
 			host = cleanhostname(thost);
-			xasprintf(&bp, "%s -t -- %s", cmd, targ);
+			xasprintf(&bp, "%s -t %s", cmd, targ);
 			if (do_cmd2(host, tuser, bp, remin, remout) < 0)
 				exit(1);
 			(void) xfree(bp);
@@ -641,7 +639,6 @@
 			} else {
 				host = cleanhostname(argv[i]);
 			}
-			addargs(&alist, "--");
 			addargs(&alist, "%s", host);
 			addargs(&alist, "%s", cmd);
 			addargs(&alist, "%s", src);
@@ -652,7 +649,7 @@
 				errs = 1;
 		} else {	/* local to remote */
 			if (remin == -1) {
-				xasprintf(&bp, "%s -t -- %s", cmd, targ);
+				xasprintf(&bp, "%s -t %s", cmd, targ);
 				host = cleanhostname(thost);
 				if (do_cmd(host, tuser, bp, &remin,
 				    &remout) < 0)
@@ -685,7 +682,6 @@
 				addargs(&alist, "-r");
 			if (pflag)
 				addargs(&alist, "-p");
-			addargs(&alist, "--");
 			addargs(&alist, "%s", argv[i]);
 			addargs(&alist, "%s", argv[argc-1]);
 			if (do_local_cmd(&alist))
@@ -705,7 +701,7 @@
 				suser = pwd->pw_name;
 		}
 		host = cleanhostname(host);
-		xasprintf(&bp, "%s -f -- %s", cmd, src);
+		xasprintf(&bp, "%s -f %s", cmd, src);
 		if (do_cmd(host, suser, bp, &remin, &remout) < 0) {
 			(void) xfree(bp);
 			++errs;


On Fri, Jul 29, 2011 at 05:59:09PM +1000, Damien Miller wrote:
> Try this compat hack:
> 
> 
> Index: ttymodes.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/ssh/ttymodes.c,v
> retrieving revision 1.29
> diff -u -p -r1.29 ttymodes.c
> --- ttymodes.c	2 Nov 2008 00:16:16 -0000	1.29
> +++ ttymodes.c	29 Jul 2011 07:58:29 -0000
> @@ -295,8 +295,11 @@ tty_make_modes(int fd, struct termios *t
>  	put_arg(&buf, tio.c_cc[NAME]);
>  
>  #define TTYMODE(NAME, FIELD, OP) \
> -	buffer_put_char(&buf, OP); \
> -	put_arg(&buf, ((tio.FIELD & NAME) != 0));
> +	if (!compat20 || (datafellows & SSH_BUG_SCREENOS_PTY) == 0 || \
> +	    buffer_len(&buf) < 256 - 5) { \
> +		buffer_put_char(&buf, OP); \
> +		put_arg(&buf, ((tio.FIELD & NAME) != 0)); \
> +	}
>  
>  #include "ttymodes.h"
>  
> Index: compat.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/ssh/compat.c,v
> retrieving revision 1.78
> diff -u -p -r1.78 compat.c
> --- compat.c	11 Sep 2008 14:22:37 -0000	1.78
> +++ compat.c	29 Jul 2011 07:58:29 -0000
> @@ -146,6 +146,8 @@ compat_datafellows(const char *version)
>  					SSH_BUG_IGNOREMSG },
>  		{ "*SSH Compatible Server*",			/* Netscreen */
>  					SSH_BUG_PASSWORDPAD },
> +		{ "NetScreen",
> +					SSH_BUG_SCREENOS_PTY },
>  		{ "*OSU_0*,"
>  		  "OSU_1.0*,"
>  		  "OSU_1.1*,"
> Index: compat.h
> ===================================================================
> RCS file: /cvs/src/usr.bin/ssh/compat.h,v
> retrieving revision 1.42
> diff -u -p -r1.42 compat.h
> --- compat.h	11 Sep 2008 14:22:37 -0000	1.42
> +++ compat.h	29 Jul 2011 07:58:29 -0000
> @@ -58,6 +58,7 @@
>  #define SSH_OLD_FORWARD_ADDR	0x01000000
>  #define SSH_BUG_RFWD_ADDR	0x02000000
>  #define SSH_NEW_OPENSSH		0x04000000
> +#define SSH_BUG_SCREENOS_PTY	0x08000000
>  
>  void     enable_compat13(void);
>  void     enable_compat20(void);
> 
> On Thu, 28 Jul 2011, Morty Abzug wrote:
> 
> > On Thu, Jul 28, 2011 at 06:00:38PM +0200, Gert Doering wrote:
> > > Hi,
> > > 
> > > On Thu, Jul 28, 2011 at 11:52:47AM -0400, Morty Abzug wrote:
> > > > On Wed, Jul 27, 2011 at 05:25:05PM +1000, Damien Miller wrote:
> > > > 
> > > > > The problem is a bug in ScreenOS, it refuses pty-req channel requests
> > > > > when the tty modes blob exceeds 256 bytes in length. If you want a
> > > > > workaround that preserves the usability of the tty, then comment out
> > > > > a couple of less-important modes in ttymodes.h and recompile
> > > > 
> > > > Any suggestions on which modes are less important?
> > > 
> > > In that context, I think CS7, PARENB, PARODDB, IXON, IXOFF, IXANY, IUCLC,
> > > PARMRK would be the ones I'd skip, given that use of 7-bit and parity
> > > terminals is unlikely, and that the netscreens are not going to honour
> > > xon/xoff flow control (IXON/IXOFF/IXANY) anyway.
> > 
> > Thanks.
> > 
> > I tested with #ifdef all of the above (CS7, PARENB, PARODDB, IXON,
> > IXOFF, IXANY, IUCLC, and PARMRK.)  This worked to get to one of our
> > firewalls (ScreenOS 6.3.0r7.0) but not another (ScreenOS 5.3.0r3.0).
> > So the problem appears to depend to some extent on ScreenOS version or
> > some other variable that is device-specific.
> > 
> > Meanwhile, I have that other workaround, i.e. to make the ssh client
> > not consider PTY allocation failure a fatal exit.  It appears to work
> > for all of our ScreenOS devices.
> > 
> > Questions/comments:
> > 
> > (1) From a patch perspective, which approach is preferable -- making
> >     PTY allocation failure not a fatal error, or commenting out a
> >     bunch of ttymodes?  [Assuming a set of ttymodes can be found that
> >     makes this work, of course.]  I would lean towards the former
> >     approach, since it seems inherently more robust/consistent.
> > 
> > (2) Commenting out stuff in ttymodes.h thing appears to be a
> >     compile-time option.  Is there a way to make it a run-time option?
> > 
> > (3) What would be a good name for an option to workaround this?  I
> >     lean towards ExitOnTTYFailure.
> > 
> > (4) What would be a good name for an option to workaround the scp "--"
> >     problem?
> > 
> > - Morty
> > 

-- 
                           Mordechai T. Abzug
Linux red-sonja 2.6.31-23-generic #75-Ubuntu SMP Fri Mar 18 18:16:06 UTC 2011 x86_64 GNU/Linux
If you have any trouble sounding condescending, find a Unix user to show you
 how it's done. -Scott Adams (of Dilbert fame)


More information about the openssh-unix-dev mailing list