Bug 80732 - [patch] getty(8) and telnetd(8) ignore the 'if' option of gettytab(5) and don't print initial message from a file (by default /etc/issue) before the login prompt
Summary: [patch] getty(8) and telnetd(8) ignore the 'if' option of gettytab(5) and don...
Status: Open
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: unspecified
Hardware: Any Any
: Normal Affects Only Me
Assignee: freebsd-bugs mailing list
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2005-05-07 14:30 UTC by Rostislav Krasny
Modified: 2017-12-31 22:35 UTC (History)
0 users

See Also:


Attachments
file.diff (1020 bytes, patch)
2005-05-07 14:30 UTC, Rostislav Krasny
no flags Details | Diff
file.diff (1.06 KB, patch)
2005-05-07 14:30 UTC, Rostislav Krasny
no flags Details | Diff
telnetd.patch (4.39 KB, patch)
2005-05-09 16:28 UTC, Rostislav Krasny
no flags Details | Diff
telnetd.patch.txt (4.60 KB, text/plain)
2005-05-09 16:48 UTC, Rostislav Krasny
no flags Details
telnetd2.patch (4.00 KB, patch)
2005-05-27 03:22 UTC, Rostislav Krasny
no flags Details | Diff
main.c.diff (852 bytes, patch)
2005-06-08 05:05 UTC, Rostislav Krasny
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Rostislav Krasny 2005-05-07 14:30:02 UTC
By default /etc/gettytab have following lines:

default:\
        :cb:ce:ck:lc:fd#1000:im=\r\n%s/%m (%h) (%t)\r\n\r\n:sp#1200:\
        :if=/etc/issue:

The 'if' option specifies a text file that their contents will be processed like contents of a 'im' gettytab(5) option. Both getty(8) and telnetd(8) ignore the 'if' option, the first because of a bug in the code and the second because it is not implemented. However, the telnetd(8) manual page states that it is implemented.

How-To-Repeat: Just create the /etc/issue file with some text in it and see how telnetd(8) and getty(8) work. You don't have to reboot when you test getty(8). Instead you can press [Ctrl]+[D] to restart getty(8) or [Enter] to just make it reprint the full login prompt. Also, you might need to enable inetd(8) and eding an /etc/inetd.conf to make telnetd working.
Comment 1 Maxim Konovalov 2005-05-07 14:50:14 UTC
Does your patch correlate with bin/23562?

http://www.freebsd.org/cgi/query-pr.cgi?pr=bin/23562

-- 
Maxim Konovalov
Comment 2 Rostislav Krasny 2005-05-07 16:41:58 UTC
On 5/7/05, Maxim Konovalov <maxim@macomnet.ru> wrote:
> Does your patch correlate with bin/23562?
>=20
> http://www.freebsd.org/cgi/query-pr.cgi?pr=3Dbin/23562

The bin/23562 is about same bugs of getty(8) and telnetd(8). It have
very old patch for telnetd(8) and no patch for getty(8).

The history of the patch in bin/23562 is mysterious. Originally it was
introduced in bin/6492 and then commited, many years ago:
http://www.freebsd.org/cgi/cvsweb.cgi/src/libexec/telnetd/Attic/telnetd.c.d=
iff?r1=3D1.7.2.4&r2=3D1.7.2.5
Three years later (August 2001) it was removed during: "Feature
merging and diff reduction between this code and crypto telnet":
http://www.freebsd.org/cgi/cvsweb.cgi/src/libexec/telnetd/Attic/telnetd.c.d=
iff?r1=3D1.29&r2=3D1.30
But the removing was not complete. telnetd.c still includes
<sys/mman.h> that had been need for mmap() and munmap() and manual
page of telnetd(8) still states that 'if' option of gettytab is
supported. The patch in bin/23562 was proposed in February 2001, i.e.
before it was removed in revision 1.30.
In July 2003 the sources of telnetd(8) were moved to
src/contrib/telnet where the revision numbering is different.

Personally I think my patch ot telnetd.c is better and the the
inclusion of <sys/mman.h> could be also removed from the telnetd.c as
needless.
Comment 3 Rostislav Krasny 2005-05-09 16:28:12 UTC
This is an updated version of the telnetd(8) patch. It solves two more problems:
1. if a lexically cultivated or original contents of /etc/issue file
or 'im' parameter of /etc/gettytab is longer than BUFSIZ - 1 (1023
bytes) then telnetd will crash
2. if the last byte of the /etc/issue file or the 'im' parameter of
/etc/gettytab is a '%' char then telnetd will jump over the
null-terminator of the string.

The patch is attached to this email.

P.S. Original patch of getty(8) (of the main.c file) is still good and
desirable.
Comment 4 Rostislav Krasny 2005-05-09 16:48:51 UTC
In my previous email the patch has been encoded by base64 by Gmail.
I'm sending the patch again after renaming it to *.txt file. I hope
this time Gmail will not use base64 encoding.
Comment 5 Maxim Konovalov 2005-05-09 17:07:45 UTC
Added to the audit trail.

diff -ur telnetd.orig/ext.h /usr/src/contrib/telnet/telnetd/ext.h
--- telnetd.orig/ext.h	Mon May  9 16:41:45 2005
+++ /usr/src/contrib/telnet/telnetd/ext.h	Mon May  9 17:01:30 2005
@@ -112,8 +112,7 @@
 #endif
 	process_slc(unsigned char, unsigned char, cc_t),
 	ptyflush(void),
-	putchr(int),
-	putf(char *, char *),
+	putf(char *, char *, size_t),
 	recv_ayt(void),
 	send_do(int, int),
 	send_dont(int, int),
diff -ur telnetd.orig/telnetd.c /usr/src/contrib/telnet/telnetd/telnetd.c
--- telnetd.orig/telnetd.c	Mon May  9 16:42:13 2005
+++ /usr/src/contrib/telnet/telnetd/telnetd.c	Mon May  9 17:01:45 2005
@@ -42,7 +42,6 @@
 #include "telnetd.h"
 #include "pathnames.h"

-#include <sys/mman.h>
 #include <err.h>
 #include <libutil.h>
 #include <paths.h>
@@ -740,6 +739,7 @@
 	char *HE;
 	char *HN;
 	char *IM;
+	char *IF;
 	int nfd;

 	/*
@@ -900,22 +900,40 @@
 	 */

 	if (getent(defent, "default") == 1) {
-		char *cp=defstrs;
+		char *cp = defstrs;

 		HE = Getstr("he", &cp);
 		HN = Getstr("hn", &cp);
-		IM = Getstr("im", &cp);
-		if (HN && *HN)
+		IF = Getstr("if", &cp);
+		if (HN != NULL && *HN != 0)
 			(void) strlcpy(host_name, HN, sizeof(host_name));
-		if (IM == 0)
-			IM = strdup("");
+		if (IF != NULL) {
+			int if_fd;
+
+			if ((if_fd = open(IF, O_RDONLY)) != -1) {
+				struct stat if_fst;
+
+				fstat(if_fd, &if_fst);
+				IM = malloc(if_fst.st_size + 1);
+				read(if_fd, IM, if_fst.st_size);
+				IM[if_fst.st_size] = 0;
+				close(if_fd);
+			} else {
+				IF = NULL;
+			}
+		}
+		if (IF == NULL) {
+			IM = Getstr("im", &cp);
+			if (IM == NULL)
+				IM = strdup("");
+		}
 	} else {
 		IM = strdup(DEFAULT_IM);
-		HE = 0;
+		HE = NULL;
 	}
 	edithost(HE, host_name);
 	if (hostinfo && *IM)
-		putf(IM, ptyibuf2);
+		putf(IM, ptyibuf2, BUFSIZ);

 	if (pcc)
 		(void) strncat(ptyibuf2, ptyip, pcc+1);
diff -ur telnetd.orig/utility.c /usr/src/contrib/telnet/telnetd/utility.c
--- telnetd.orig/utility.c	Mon May  9 16:42:22 2005
+++ /usr/src/contrib/telnet/telnetd/utility.c	Mon May  9 17:01:38 2005
@@ -66,11 +66,9 @@
  * also flush the pty input buffer (by dropping its data) if it becomes
  * too full.
  */
-
-    void
-ttloop()
+void
+ttloop(void)
 {
-
     DIAG(TD_REPORT, output_data("td: ttloop\r\n"));
     if (nfrontp - nbackp > 0) {
 	netflush();
@@ -393,22 +391,6 @@
 	editedhost[sizeof editedhost - 1] = '\0';
 }

-static char *putlocation;
-
-static void
-putstr(const char *s)
-{
-
-	while (*s)
-		putchr(*s++);
-}
-
-void
-putchr(int cc)
-{
-	*putlocation++ = cc;
-}
-
 #ifdef __FreeBSD__
 static char fmtstr[] = { "%+" };
 #else
@@ -416,11 +398,12 @@
 #endif

 void
-putf(char *cp, char *where)
+putf(char *cp, char *where, size_t where_size)
 {
 	char *slash;
 	time_t t;
 	char db[100];
+	char ch_str[2] = {0, 0};
 #ifdef __FreeBSD__
 	static struct utsname kerninfo;

@@ -428,19 +411,13 @@
 		uname(&kerninfo);
 #endif

-	putlocation = where;
-
-	while (*cp) {
-		if (*cp =='\n') {
-			putstr("\r\n");
-			cp++;
-			continue;
+	while (*cp != 0) {
+		if (*cp == '\n') {
+			strlcat(where, "\r\n", where_size);
 		} else if (*cp != '%') {
-			putchr(*cp++);
-			continue;
-		}
-		switch (*++cp) {
-
+			*ch_str = *cp;
+			strlcat(where, ch_str, where_size);
+		} else switch (*++cp) {
 		case 't':
 #ifdef	STREAMSPTY
 			/* names are like /dev/pts/2 -- we want pts/2 */
@@ -448,46 +425,42 @@
 #else
 			slash = strrchr(line, '/');
 #endif
-			if (slash == (char *) 0)
-				putstr(line);
+			if (slash == NULL)
+				strlcat(where, line, where_size);
 			else
-				putstr(&slash[1]);
+				strlcat(where, &slash[1], where_size);
 			break;
-
 		case 'h':
-			putstr(editedhost);
+			strlcat(where, editedhost, where_size);
 			break;
-
 		case 'd':
 #ifdef __FreeBSD__
 			setlocale(LC_TIME, "");
 #endif
 			(void)time(&t);
 			(void)strftime(db, sizeof(db), fmtstr, localtime(&t));
-			putstr(db);
+			strlcat(where, db, where_size);
 			break;
-
 #ifdef __FreeBSD__
 		case 's':
-			putstr(kerninfo.sysname);
+			strlcat(where, kerninfo.sysname, where_size);
 			break;
-
 		case 'm':
-			putstr(kerninfo.machine);
+			strlcat(where, kerninfo.machine, where_size);
 			break;
-
 		case 'r':
-			putstr(kerninfo.release);
+			strlcat(where, kerninfo.release, where_size);
 			break;
-
 		case 'v':
-			putstr(kerninfo.version);
+			strlcat(where, kerninfo.version, where_size);
 			break;
 #endif
-
 		case '%':
-			putchr('%');
+			*ch_str = '%';
+			strlcat(where, ch_str, where_size);
 			break;
+		case 0:
+			return;
 		}
 		cp++;
 	}
Comment 6 Maxim Konovalov 2005-05-25 07:40:52 UTC
Why didn't you check the return code from malloc(3) and read(2)?
Could you please fix that?  Could you remove all style(9) changes
from your diff?

-- 
Maxim Konovalov
Comment 7 Rostislav Krasny 2005-05-27 03:22:06 UTC
Maxim Konovalov wrote:
> Why didn't you check the return code from malloc(3) and read(2)?
> Could you please fix that?  Could you remove all style(9) changes
> from your diff?

Fixed. Thank you for the comment. New diff of telnetd(8) is attached to 
this email (inline).

P.S. The original diff of getty(8) doesn't need any change so far.
Comment 8 Rostislav Krasny 2005-06-08 05:05:23 UTC
During a private discussion with Maxim Konovalov I've realized that the 
original getty(8) actually does print the IF file (/etc/issue) but only 
once - before the very first IM message. When I did make the getty(8) 
and the telnetd(8) patches I thought that the IM is really an initial 
message and there should be no super-initial message before it; i.e. if 
the IF file is defined and accessible, its contents should be a 
replacement of the IM.

I still think so, but also I think getty(8) and telnetd(8) should behave 
by the same way during a login process. Try doing following experiment 
with the original getty(8) on any pseudo-terminal:

1. Press the Enter key a few times. After each pressing you'll see the 
IM message before each "login:" prompt.
2. Try login with a wrong password or a wrong username and after you get 
a new "login:" prompt press the Enter key a few times again. Now you'll 
see only the "login:" prompts, without the IM message before any of them.

If you do the same experiment with the original telnetd(8) you see the 
IM message only once - before the very first "login:" prompt.

Therefore, in addition to the last version of telnetd(8) patch, I made a 
new version of getty(8) patch that makes getty(8) to be with the same 
behavior like telnetd(8).
Comment 9 Eitan Adler freebsd_committer freebsd_triage 2017-12-31 08:00:49 UTC
For bugs matching the following criteria:

Status: In Progress Changed: (is less than) 2014-06-01

Reset to default assignee and clear in-progress tags.

Mail being skipped