From 316bdcd4afc5a270ac17e923abe58c0912411241 Mon Sep 17 00:00:00 2001 From: Uwe Kleine-König Date: Wed, 7 Jun 2023 17:53:34 +0200 Subject: mux: Fix debug output for comport subnegotiation parameters MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes: a94d4b8bff00 ("telnet: make rfc2217 handling more correct") Signed-off-by: Uwe Kleine-König --- mux.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/mux.c b/mux.c index b7535b8..22c04be 100644 --- a/mux.c +++ b/mux.c @@ -85,14 +85,14 @@ static int do_com_port_option(struct ios_ops *ios, unsigned char *buf, int len) break; case SET_CONTROL_SC: i++; - dbg_printf("SET_CONTROL_SC 0x%02x ", buf[i]); + dbg_printf("SET_CONTROL_SC 0x%02x ", buf[2]); break; case NOTIFY_LINESTATE_SC: dbg_printf("NOTIFY_LINESTATE_SC "); break; case NOTIFY_MODEMSTATE_SC: i++; - dbg_printf("NOTIFY_MODEMSTATE_SC 0x%02x ", buf[i]); + dbg_printf("NOTIFY_MODEMSTATE_SC 0x%02x ", buf[2]); break; case FLOWCONTROL_SUSPEND_SC: dbg_printf("FLOWCONTROL_SUSPEND_SC "); @@ -110,7 +110,7 @@ static int do_com_port_option(struct ios_ops *ios, unsigned char *buf, int len) dbg_printf("PURGE_DATA_SC "); break; default: - dbg_printf("??? %d ", buf[i]); + dbg_printf("??? %d ", buf[1]); break; } -- cgit v1.2.3 From 91182380f2234ad629238bf7d323b8087ba9fddf Mon Sep 17 00:00:00 2001 From: Uwe Kleine-König Date: Wed, 7 Jun 2023 23:57:46 +0200 Subject: mux: Handle parsing subnegotiations including IACs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The IACs in a subnegotiation are quoted, too. So unquote while parsing. If the remote rfc2217 side sends (raw) SB COM_PORT_CONTROL NOTIFY_MODEMSTATE_SC 0xff 0xff IAC SE the double 0xff is to be interpreted as a single 0xff (as IAC = 0xff). Instead the second 0xff was interpreted as quoting the following IAC and so the end of the subnegotiation is missed making microcom exit with Incomplete SB string Reported-by: James Puderer Fixes: 1334a93b2718 ("implement telnet rfc2217 mode (experimental), update help") Signed-off-by: Uwe Kleine-König --- mux.c | 84 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 77 insertions(+), 7 deletions(-) diff --git a/mux.c b/mux.c index 22c04be..a6284ca 100644 --- a/mux.c +++ b/mux.c @@ -27,6 +27,48 @@ #define BUFSIZE 1024 +static ssize_t get(unsigned char *buf, unsigned char *out, size_t len) +{ + if (!len) + return -1; + + if (buf[0] == IAC) { + if (len < 1) + return -1; + if (buf[1] == IAC) { + *out = IAC; + return 2; + } + return -1; + } else { + *out = buf[0]; + return 1; + } +} + +static size_t getl(unsigned char *buf, uint32_t *out, size_t len) +{ + *out = 0; + int i; + size_t offset = 0; + + for (i = 0; i < 4; ++i) { + ssize_t getres; + unsigned char c; + + getres = get(buf + offset, &c, len - offset); + if (getres < 0) + return getres; + + *out <<= 8; + *out |= c; + + offset += getres; + } + + return offset; +} + /* This is called with buf[-2:0] being IAC SB COM_PORT_OPTION */ static int do_com_port_option(struct ios_ops *ios, unsigned char *buf, int len) { @@ -70,9 +112,17 @@ static int do_com_port_option(struct ios_ops *ios, unsigned char *buf, int len) dbg_printf("PURGE_DATA_CS "); break; case SET_BAUDRATE_SC: - dbg_printf("SET_BAUDRATE_SC %d ", - buf[2] << 24 | buf[3] << 16 | buf[4] << 8 | buf[5]); - i += 4; + { + uint32_t baudrate; + ssize_t getres = getl(buf + 2, &baudrate, len - 2); + + if (getres < 0) { + fprintf(stderr, "Incomplete or broken SB (SET_BAUDRATE_SC)\n"); + return getres; + } + dbg_printf("SET_BAUDRATE_SC %u ", baudrate); + i += getres;; + } break; case SET_DATASIZE_SC: dbg_printf("SET_DATASIZE_SC "); @@ -84,15 +134,35 @@ static int do_com_port_option(struct ios_ops *ios, unsigned char *buf, int len) dbg_printf("SET_STOPSIZE_SC "); break; case SET_CONTROL_SC: - i++; - dbg_printf("SET_CONTROL_SC 0x%02x ", buf[2]); + { + unsigned char ctrl; + ssize_t getres = get(buf + 2, &ctrl, len - 2); + + if (getres < 0) { + fprintf(stderr, "Incomplete or broken SB (SET_CONTROL_SC)\n"); + return getres; + } + + dbg_printf("SET_CONTROL_SC 0x%02x ", ctrl); + i += getres; + } break; case NOTIFY_LINESTATE_SC: dbg_printf("NOTIFY_LINESTATE_SC "); break; case NOTIFY_MODEMSTATE_SC: - i++; - dbg_printf("NOTIFY_MODEMSTATE_SC 0x%02x ", buf[2]); + { + unsigned char ms; + ssize_t getres = get(buf + 2, &ms, len - 2); + + if (getres < 0) { + fprintf(stderr, "Incomplete or broken SB (NOTIFY_MODEMSTATE_SC)\n"); + return getres; + } + + dbg_printf("NOTIFY_MODEMSTATE_SC 0x%02x ", ms); + i += getres; + } break; case FLOWCONTROL_SUSPEND_SC: dbg_printf("FLOWCONTROL_SUSPEND_SC "); -- cgit v1.2.3 From 704f5416235f0d9b24aa55ad59654a7630b64bc1 Mon Sep 17 00:00:00 2001 From: Uwe Kleine-König Date: Thu, 8 Jun 2023 00:08:38 +0200 Subject: telnet: Properly quote IAC when sending SET_BAUDRATE negotiation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the baudrate is to be set to say 130816 (= 0x1ff00) this has to be send (raw) as: IAC SB COM_PORT_CONTROL SET_BAUDRATE_CS 0x00 0x01 0xff 0xff 0x00 IAC SE This is explicitly mentioned in RFC855: Finally, if parameters in an option "subnegotiation" include a byte with a value of 255, it is necessary to double this byte in accordance the general TELNET rules. Signed-off-by: Uwe Kleine-König --- telnet.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/telnet.c b/telnet.c index c32a2d4..4a66b95 100644 --- a/telnet.c +++ b/telnet.c @@ -39,13 +39,21 @@ static ssize_t telnet_read(struct ios_ops *ios, void *buf, size_t count) static int telnet_set_speed(struct ios_ops *ios, unsigned long speed) { + unsigned char buf2[14] = {IAC, SB, TELNET_OPTION_COM_PORT_CONTROL, SET_BAUDRATE_CS}; + size_t offset = 4; + int i; + + for (i = 0; i < 4; ++i) { + buf2[offset] = (speed >> (24 - 8 * i)) & 0xff; + if (buf2[offset++] == IAC) + buf2[offset++] = IAC; + } - unsigned char buf2[] = {IAC, SB, TELNET_OPTION_COM_PORT_CONTROL, SET_BAUDRATE_CS, 0, 0, 0, 0, IAC, SE}; - int *speedp = (int *)&buf2[4]; + buf2[offset++] = IAC; + buf2[offset++] = SE; - *speedp = htonl(speed); dbg_printf("-> IAC SB COM_PORT_CONTROL SET_BAUDRATE_CS 0x%lx IAC SE\n", speed); - write(ios->fd, buf2, 10); + write(ios->fd, buf2, offset); return 0; } -- cgit v1.2.3