From b2c239dd6ca4d4018654b7e9d8408d8bdd10b9d3 Mon Sep 17 00:00:00 2001 From: InterLinked1 <24227567+InterLinked1@users.noreply.github.com> Date: Fri, 27 Sep 2024 19:14:14 -0400 Subject: [PATCH] net_telnet: Improve terminal compatibility. * For some reason, even after sending IAC DO NAWS and receiving IAC WILL NAWS from the client, SyncTERM does not proceed with IAC SB NAWS to actually send us its terminal dimensions, which led us to unnecessarily falling back to prompting for dimensions. Work around this by sending another IAC DO NAWS in this case, which makes SyncTERM send them over. * Due to the above change, the received terminal type as read seems to include 3 non-printable characters at the end. * Properly handle the case when multiple commands are available in a single call to telnet_read_command by parsing them one at a time. * Request terminal type before the terminal dimensions. SyncTERM offers to send dimensions when we get the terminal type, so asking for the terminal type first slightly cuts down on the chattiness of the handshake. * Fix TERM for Windows Command Prompt. --- bbs/system.c | 9 ++++ nets/net_telnet.c | 109 ++++++++++++++++++++++++++++++++++++++-------- 2 files changed, 101 insertions(+), 17 deletions(-) diff --git a/bbs/system.c b/bbs/system.c index ac340b9..8424a76 100644 --- a/bbs/system.c +++ b/bbs/system.c @@ -795,6 +795,15 @@ static int __bbs_execvpe_fd(struct bbs_node *node, int usenode, int fdin, int fd /* Don't call tcgetsid here, it will fail */ bbs_debug(6, "sid: %d, tcpgrp: %d, term: %s\n", getsid(getpid()), tcgetpgrp(fd), S_IF(node->term)); snprintf(fullterm, sizeof(fullterm), "TERM=%s", S_OR(node->term, "xterm")); /* Many interactive programs will whine if $TERM is not set */ + if (node->term && !strcmp(node->term, "ANSI")) { + /* Windows Command Prompt (cmd.exe) advertises its term type as ANSI for telnet. + * Well, ANSI doesn't exist in the terminfo database, but ansi does, so use that instead. + * Note that Microsoft Telnet still works horribly, and most termcap/ncurses + * dependent brokens are going to be horribly broken. + * However, this IS the correct terminal definition to use. */ + bbs_debug(1, "Setting TERM to 'ansi' instead of 'ANSI' for compatibility\n"); + strcpy(fullterm + STRLEN("TERM="), "ansi"); /* Safe */ + } /* Save terminal settings to restore after execution */ memset(&term, 0, sizeof(term)); if (tcgetattr(node->slavefd, &term)) { diff --git a/nets/net_telnet.c b/nets/net_telnet.c index df4cef2..285e42b 100644 --- a/nets/net_telnet.c +++ b/nets/net_telnet.c @@ -145,28 +145,72 @@ static int telnet_read_command(int fd, unsigned char *buf, size_t len) #define telnet_process_command(node, settings, buf, len, res) __telnet_process_command(node, settings, buf, len, res, depth + 1) +/* IAC SE frequently indicates the end of a client's response for a command */ +static const unsigned char RESPONSE_FINALE[] = { IAC, SE }; +#define RESPONSE_FINALE_LEN 2 + +/* Forward declaration */ +static int __telnet_process_command(struct bbs_node *node, struct telnet_settings *settings, unsigned char *buf, size_t len, int res, int depth); + +static int telnet_process_command_additional(struct bbs_node *node, struct telnet_settings *settings, unsigned char *buf, size_t len, int res, int depth) +{ + if (!TELCMD_OK(buf[0]) || !TELCMD_OK(buf[1])) { + bbs_warning("Got out of bounds command: %d %d %d\n", buf[0], buf[1], buf[2]); + return 0; + } + if (!TELOPT_OK(buf[2])) { + bbs_warning("Got out of bounds option: %d %d %d\n", buf[0], buf[1], buf[2]); + return 0; + } + bbs_debug(3, "Processing additional Telnet command %s %s %s\n", telcmds[*buf - xEOF], telcmds[*(buf + 1) - xEOF], telopts[*(buf + 2)]); + return telnet_process_command(node, settings, buf, len, res); +} + static int __telnet_process_command(struct bbs_node *node, struct telnet_settings *settings, unsigned char *buf, size_t len, int res, int depth) { - if (depth > 3) { + if (depth > 4) { /* Prevent infinite recursion if the client replies with the same thing that triggered another command */ bbs_warning("Exceeded command stack depth %d\n", depth); return 0; } + bbs_assert(res >= 3); + if (buf[1] == DO && buf[2] == TELOPT_ECHO) { settings->rcv_noecho = 1; bbs_debug(3, "Client acknowledged local echo disable\n"); } else if (buf[1] == WILL && buf[2] == TELOPT_NAWS) { + if (node->dimensions) { + /* If we already got the dimensions, we don't want them again. + * If logic is added to process commands during a session and receive window size updates, + * this condition would need to be refined, but it would still be the case that we don't + * care to receive this more than once during initial negotiation. */ + bbs_debug(3, "Ignoring offer to send window dimensions since we already have them\n"); + if (telnet_send_command(node->wfd, DONT, TELOPT_NAWS)) { + return -1; + } + } if (!settings->sent_winsize) { if (telnet_send_command(node->wfd, DO, TELOPT_NAWS)) { return -1; } settings->sent_winsize = 1; } - /* Read terminal type, coming up next */ + /* Read terminal dimensions, coming up next */ res = telnet_read_command(node->rfd, buf, len); if (res > 0) { res = telnet_process_command(node, settings, buf, len, res); + } else { + /* Even after we send IAC DO NAWS and we receive IAC WILL NAWS from the client, + * SyncTERM doesn't seem to do IAC SB NAWS unless we repeat our IAC DO NAWS once more. */ + bbs_debug(3, "Failed to receive terminal dimensions, even though client offered to send it?\n"); + if (telnet_send_command(node->wfd, DO, TELOPT_NAWS)) { + return -1; + } + res = telnet_read_command(node->rfd, buf, len); + if (res > 0) { + res = telnet_process_command(node, settings, buf, len, res); + } } } else if (buf[1] == WONT && buf[2] == TELOPT_NAWS) { /* Client disabled NAWS, at our request, good. */ @@ -217,11 +261,40 @@ static int __telnet_process_command(struct bbs_node *node, struct telnet_setting return res; } else if (res > 0) { if (buf[1] == SB && buf[2] == TELOPT_TTYPE && buf[3] == TELQUAL_IS && res >= 6) { - bbs_debug(3, "Terminal type is %.*s\n", (int) res - 6, buf + 4); /* First 4 bytes are command, and last two are IAC SE */ - if (res - 6 < (int) len - 1) { - memcpy(buf, buf + 4, (size_t) res - 6); - buf[res - 6] = '\0'; - REPLACE(node->term, (char*) buf); + /* With SyncTERM, if we resend IAC DO NAWS (as we now do above, since for some reason it needs to be prompted to), + * SyncTERM will send the term type, followed by IAC SE IAC WILL TELOPT_NAWS. + * The IAC SE is the trailer to the response, but the IAC WILL TELOPT_NAWS (offering to send window dimensions) + * throws the accounting below off. It's also a bit odd, given we already received the dimensions + * and then send IAC DONT NAWS (and received IAC WONT NAWS) in response. + * That said, while odd, it is certainly legitimate to receive multiple commands in a single call to read() + * like this, and we need to parse appropriately. Specifically, + * rather than assuming IAC SE is at the end of whatever we just read, we need to actually look for IAC SE and stop there. + * + * Note that this doesn't actually happen anymore, since in telnet_handshake, + * we move asking for terminal type to BEFORE asking for dimensions, + * to avoid the extra exchange in the first place. */ + size_t length, cmdlen, nextlen; + unsigned char *termtype; + unsigned char *end; + termtype = buf + 4; /* First 4 bytes are command, and last two are IAC SE (IAC SB TERMINAL TYPE IAC SE) */ + end = memmem(termtype, (size_t) res, RESPONSE_FINALE, RESPONSE_FINALE_LEN); + if (!end) { + bbs_warning("Received command response does not send in IAC SE\n"); + bbs_dump_mem(buf, (size_t) res); + return -1; + } + /* IAC SB TTYPE IS syncterm IAC SE IAC WILL NAWS */ + cmdlen = (size_t) (end - buf) + RESPONSE_FINALE_LEN; + length = cmdlen - 6; /* Subtract 4 for command (IAC SB TERMINAL TYPE IS) and 2 for trailer (IAC SE) */ + bbs_debug(3, "Terminal type is %.*s\n", (int) length, termtype); + *end = '\0'; /* Replace IAC with NUL for strdup since we don't need it anymore */ + REPLACE(node->term, (char*) termtype); + /* If there is anything leftover, thanks to recursion, we can easily process a second command received */ + nextlen = (size_t) res - cmdlen; + if (nextlen > 0) { + buf += cmdlen; + len -= cmdlen; + res = telnet_process_command_additional(node, settings, buf, len, (int) nextlen, depth); } } else { bbs_warning("Foreign %d-byte response received in response to terminal type\n", res); @@ -239,7 +312,7 @@ static int __telnet_process_command(struct bbs_node *node, struct telnet_setting if (buf[1] == SB && buf[2] == TELOPT_TSPEED && buf[3] == TELQUAL_IS && res >= 3) { bbs_debug(3, "Terminal speed is %.*s\n", (int) res - 6, buf + 4); /* First 4 bytes are command, and last two are IAC SE */ if (res - 6 < (int) len - 1) { - memcpy(buf, buf + 4, (size_t) res - 6); + memmove(buf, buf + 4, (size_t) res - 6); buf[res - 6] = '\0'; node->reportedbps = (unsigned int) atoi((char*) buf); } @@ -303,6 +376,17 @@ static int telnet_handshake(struct bbs_node *node) return -1; } + bbs_debug(8, "Finished processing commands received at connection time\n"); + + /* RFC 1091 Terminal Type */ + if (telnet_send_command(node->wfd, DO, TELOPT_TTYPE)) { + return -1; + } + res = read_and_process_command(node, &settings, buf, sizeof(buf)); + if (res < 0) { + return res; + } + /* RFC 1073 Request window size */ if (!settings.sent_winsize) { settings.sent_winsize = 1; @@ -315,15 +399,6 @@ static int telnet_handshake(struct bbs_node *node) } } - /* RFC 1091 Terminal Type */ - if (telnet_send_command(node->wfd, DO, TELOPT_TTYPE)) { - return -1; - } - res = read_and_process_command(node, &settings, buf, sizeof(buf)); - if (res < 0) { - return res; - } - /* RFC 1079 Terminal Speed */ if (telnet_send_command(node->wfd, DO, TELOPT_TSPEED)) { return -1;