Bug#423855: openssh: tty modes not set or checked properly
Package: openssh
Version: 1:4.3p2-9
Severity: normal
Tags: patch
In my quest to understand the following syslog message:
May 13 15:34:53 server sshd[8416]: Setting tty modes failed: Invalid argument
I discovered a bug in the way tty character size information is transmitted
from client to server. I am including a patch which corrects this and also
improves error reporting of tty modes which fail to be set.
First, the bug: openssh treats CS7/CS8 character sizes as simple flags, and
often sends, e.g. from the client:
debug3: tty_make_modes: 90 1
debug3: tty_make_modes: 91 1
where 90 is the opcode for CS7, and 91 the opcode for CS8. Obviously both
should not be sent with the same value (1), but this occurs because CS7/CS8
are not simple flags, but discrete enumerations:
(from /usr/include/bits/termios.h)
#define CS7 0000040
#define CS8 0000060
If the character size is CS8, then a simple mask for CS7 also yields a true
value, and vice versa. The correct way to detect character size is to mask
against the constant CSIZE, and then test the enumerations.
Probably it would have been better to transmit character size in a manner
similar to the way tty speeds are transmitted, but in the interests of
compatibility I think it may be better to retain the existing flag structure
but send the correct flags. The attached patch will cause the CS7 opcode to be
sent (only) when the tty mode is cs7, and CS8 (only) when cs8.
On the server side, receiving one or the other opcode (with true argument)
will now set the character size properly. Note that with existing (buggy)
clients, this has the effect of defaulting to CS8 when both opcodes are sent
-- later opcodes received take precedence over earlier ones.
Second, the improved error reporting: tcsetattr() is documented to return
success if *any* of the requested changes could be carried out. It follows
that some of the requested changes could have failed, but openssh does not
report anything unless tcsetattr() returns failure (implying *all* the
requested changes failed.)
The attached patch improves error reporting by itemizing every tty mode change
that failed.
Finally, I offer some observations. With the new error reporting in place, my
original syslog error message becomes:
May 14 06:26:24 server sshd[23556]: Setting tty mode PARENB (1) failed
I have determined this to be because my client tty (provided by X11 under OS X)
has by default the unusual mode: cs8 parenb.
Further testing reveals that I am quite unable to set any mode but cs8 -parenb
on the server-side tty of any ssh connection under Linux, even if the
client-side tty is correctly (and succesfully) set as cs7 parenb. This leads me
to question the purpose and validity of transmitting character size and parity
information across the network in the first place; is there any system in
existence where this information is useful? If not, it may be better to
remove/ignore these openssh opcodes altogether. The fact that the CS7/CS8
opcodes have never worked properly on many systems anyway is a good indication
they may not have any use. Never mind that CS5 and CS6 are completely ignored!
Cheers.
-- System Information:
Debian Release: 4.0
APT prefers stable
APT policy: (500, 'stable')
Architecture: i386 (i686)
Kernel: Linux 2.6.18-4-686 (SMP w/2 CPU cores)
Locale: LANG=C, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/bash
diff -ur openssh-4.3p2.orig/ttymodes.c openssh-4.3p2/ttymodes.c
--- openssh-4.3p2.orig/ttymodes.c 2005-08-16 04:32:10.000000000 -0700
+++ openssh-4.3p2/ttymodes.c 2007-05-14 05:18:47.000000000 -0700
@@ -53,6 +53,8 @@
#include "bufaux.h"
#define TTY_OP_END 0
+#define TTY_OP_CS7 90
+#define TTY_OP_CS8 91
/*
* uint32 (u_int) follows speed in SSH1 and SSH2
*/
@@ -325,6 +327,21 @@
#undef TTYCHAR
#undef TTYMODE
+ /* Store character size. */
+ switch (tio.c_cflag & CSIZE) {
+ case CS7:
+ debug3("tty_make_modes: %d %d", TTY_OP_CS7, 1);
+ buffer_put_char(&buf, TTY_OP_CS7);
+ put_arg(&buf, 1);
+ break;
+
+ case CS8:
+ debug3("tty_make_modes: %d %d", TTY_OP_CS8, 1);
+ buffer_put_char(&buf, TTY_OP_CS8);
+ put_arg(&buf, 1);
+ break;
+ }
+
end:
/* Mark end of mode data. */
buffer_put_char(&buf, TTY_OP_END);
@@ -342,7 +359,7 @@
void
tty_parse_modes(int fd, int *n_bytes_ptr)
{
- struct termios tio;
+ struct termios tio, ntio;
int opcode, baud;
int n_bytes = 0;
int failure = 0;
@@ -398,6 +415,20 @@
error("cfsetospeed failed for %d", baud);
break;
+ case TTY_OP_CS7:
+ n_bytes += arg_size;
+ if ((arg = get_arg()))
+ tio.c_cflag = (tio.c_cflag & ~CSIZE) | CS7;
+ debug3("tty_parse_modes: cs7 %d", arg);
+ break;
+
+ case TTY_OP_CS8:
+ n_bytes += arg_size;
+ if ((arg = get_arg()))
+ tio.c_cflag = (tio.c_cflag & ~CSIZE) | CS8;
+ debug3("tty_parse_modes: cs8 %d", arg);
+ break;
+
#define TTYCHAR(NAME, OP) \
case OP: \
n_bytes += arg_size; \
@@ -479,7 +510,32 @@
if (failure == -1)
return; /* Packet parsed ok but tcgetattr() failed */
- /* Set the new modes for the terminal. */
- if (tcsetattr(fd, TCSANOW, &tio) == -1)
- logit("Setting tty modes failed: %.100s", strerror(errno));
+ /* Set and verify the new modes for the terminal. */
+ tcsetattr(fd, TCSANOW, &tio);
+
+ if (tcgetattr(fd, &ntio) == -1) {
+ logit("tcgetattr: %.100s", strerror(errno));
+ return;
+ }
+
+ if (cfgetospeed(&ntio) != cfgetospeed(&tio))
+ logit("Setting tty ospeed (%d) failed", speed_to_baud(cfgetospeed(&tio)));
+ if (cfgetispeed(&ntio) != cfgetispeed(&tio))
+ logit("Setting tty ispeed (%d) failed", speed_to_baud(cfgetispeed(&tio)));
+
+#define TTYCHAR(NAME, OP) \
+ if (ntio.c_cc[NAME] != tio.c_cc[NAME]) \
+ logit("Setting tty char %s (%d) failed", #NAME, (int) tio.c_cc[NAME]);
+#define TTYMODE(NAME, FIELD, OP) \
+ if ((ntio.FIELD & NAME) != (tio.FIELD & NAME)) \
+ logit("Setting tty mode %s (%d) failed", #NAME, ((tio.FIELD & NAME) != 0));
+
+#include "ttymodes.h"
+
+#undef TTYCHAR
+#undef TTYMODE
+
+ if ((ntio.c_cflag & CSIZE) != (tio.c_cflag & CSIZE))
+ logit("Setting tty character size (%d) failed",
+ (tio.c_cflag & CSIZE) == CS7 ? 7 : 8);
}
diff -ur openssh-4.3p2.orig/ttymodes.h openssh-4.3p2/ttymodes.h
--- openssh-4.3p2.orig/ttymodes.h 2004-07-16 23:12:08.000000000 -0700
+++ openssh-4.3p2/ttymodes.h 2007-05-14 04:09:38.000000000 -0700
@@ -169,7 +169,5 @@
TTYMODE(ONLRET, c_oflag, 75)
#endif
-TTYMODE(CS7, c_cflag, 90)
-TTYMODE(CS8, c_cflag, 91)
TTYMODE(PARENB, c_cflag, 92)
TTYMODE(PARODD, c_cflag, 93)
Reply to: