Hello,

        This patch adds utmpx support in st, which means that st sessions
will be visible using who or, who is the correct behaviour of a terminal
emulator, but this means that the binary needs have setgid, which is
something we have to thing carefully.

          Other important about this patch is the portability of it. I have
used the POSIX definitions, but as far as I know, there are some BSD that
don't support very well this functions (specially OpenBSD). There are
different solutions for this:

          - A hell of ifdef inside in st.c
          - A pty.c where all this portability stuff is present.
          - A compat file (or one for each problematic System), where
            this interfaces are adapted.

Please give your opinion, and don't worry if it is necessary modify this
patch.

Sincerely,


Roberto E. Vargas Caballero
>From 0a9788863aaf76aa7e4cb87cfa99a549f81ae87d Mon Sep 17 00:00:00 2001
From: "Roberto E. Vargas Caballero" <k...@shike2.com>
Date: Mon, 8 Oct 2012 21:03:54 +0200
Subject: Initialize environment variables to correct values

Calling st without SHELL variable caused a segmentation fault, because no
test was done about this variable. It is responsability of the terminal
emulator set correct values for LOGNAME and USER, and the value for the
variable WINDOWID (It contains a numeric identifier of the window that
contains the terminal. (This allows X-aware applications that are run from
console to do some operations over the window, e.g. changing its icon.)

This patch doesn't overwrite values in SHELL and HOME because maybe the user
put some values here which can be useful for him.
---
 st.c |   37 ++++++++++++++++++++++++++++++++-----
 1 file changed, 32 insertions(+), 5 deletions(-)

diff --git a/st.c b/st.c
index 197dd79..46db942 100644
--- a/st.c
+++ b/st.c
@@ -5,6 +5,7 @@
 #include <fcntl.h>
 #include <limits.h>
 #include <locale.h>
+#include <pwd.h>
 #include <stdarg.h>
 #include <stdbool.h>
 #include <stdio.h>
@@ -261,6 +262,7 @@ static void die(const char *, ...);
 static void draw(void);
 static void redraw(void);
 static void drawregion(int, int, int, int);
+static void initenv(void);
 static void execsh(void);
 static void sigchld(int);
 static void run(void);
@@ -881,10 +883,6 @@ execsh(void) {
 	char **args;
 	char *envshell = getenv("SHELL");
 
-	unsetenv("COLUMNS");
-	unsetenv("LINES");
-	unsetenv("TERMCAP");
-
 	signal(SIGCHLD, SIG_DFL);
 	signal(SIGHUP, SIG_DFL);
 	signal(SIGINT, SIG_DFL);
@@ -893,7 +891,6 @@ execsh(void) {
 	signal(SIGALRM, SIG_DFL);
 
 	DEFAULT(envshell, SHELL);
-	putenv("TERM="TNAME);
 	args = opt_cmd ? opt_cmd : (char*[]){envshell, "-i", NULL};
 	execvp(args[0], args);
 	exit(EXIT_FAILURE);
@@ -914,6 +911,35 @@ sigchld(int a) {
 }
 
 void
+initenv(void) {
+	const struct passwd *pass = getpwuid(getuid());
+	char buff[sizeof(long) * 8];
+
+	if(!pass)
+		die("Process is running with an incorrect uid\n");
+
+	unsetenv("COLUMNS");
+	unsetenv("LINES");
+	unsetenv("TERMCAP");
+	/*
+	 * We assume the first input in passwd with this uid
+	 * is the correct. If you have more of one user with the same
+	 * uid you suck a lot.
+	 */
+	setenv("LOGNAME", pass->pw_name, 1);
+	setenv("USER", pass->pw_name, 1);
+	setenv("SHELL", pass->pw_shell, 0);
+	setenv("HOME", pass->pw_dir, 0);
+	putenv("TERM=" TNAME);
+	/*
+	 * we can be sure that DISPLAY is correct because we don't have
+	 * a -diplay option
+	 */
+	sprintf(buff, "%lu", xw.win);
+	setenv("WINDOWID", buff, 1);
+}
+
+void
 ttynew(void) {
 	int m, s;
 	struct winsize w = {term.row, term.col, 0, 0};
@@ -935,6 +961,7 @@ ttynew(void) {
 			die("ioctl TIOCSCTTY failed: %s\n", SERRNO);
 		close(s);
 		close(m);
+		initenv();
 		execsh();
 		break;
 	default:
-- 
1.7.10.4

>From 607a9cf9b4a61fb77bfefba7d166f0b8681079a2 Mon Sep 17 00:00:00 2001
From: "Roberto E. Vargas Caballero" <k...@shike2.com>
Date: Tue, 9 Oct 2012 18:29:27 +0200
Subject: Add utmp stuff

When it is initiated a terminal emulator it is also created a new tty
(pseudo) and a new login is done. This imply that the usual job done by
login program must be done in the emulator, basically updating utmpx
database. With this patch sessions created by the emulator will be shown in
w or who.

Writing in utmpx means having enough privileges for writing in it, so the
binary needs have setgid. In this case we need this special privileges only
for two functions call, so it is not a security risk. Also, st can continue
working without setgid bit, but in this case utmpx stuff will not work.

utmpx stuff was standardised by POSIX in 2001, but some BSD distribution
lack good support for this. FreeBSD added this functions in January of 2012
and OpenBSD doesn't support them.
---
 Makefile |    3 ++
 st.c     |  100 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 99 insertions(+), 4 deletions(-)

diff --git a/Makefile b/Makefile
index 8fc9674..8680e04 100644
--- a/Makefile
+++ b/Makefile
@@ -5,6 +5,7 @@ include config.mk
 
 SRC = st.c
 OBJ = ${SRC:.c=.o}
+GROUP = utmp
 
 all: options st
 
@@ -44,6 +45,8 @@ install: all
 	@mkdir -p ${DESTDIR}${PREFIX}/bin
 	@cp -f st ${DESTDIR}${PREFIX}/bin
 	@chmod 755 ${DESTDIR}${PREFIX}/bin/st
+	@chgrp $(GROUP) ${DESTDIR}${PREFIX}/bin/st
+	@chmod g+s ${DESTDIR}${PREFIX}/bin/st
 	@echo installing manual page to ${DESTDIR}${MANPREFIX}/man1
 	@mkdir -p ${DESTDIR}${MANPREFIX}/man1
 	@sed "s/VERSION/${VERSION}/g" < st.1 > ${DESTDIR}${MANPREFIX}/man1/st.1
diff --git a/st.c b/st.c
index 46db942..8641a7a 100644
--- a/st.c
+++ b/st.c
@@ -1,8 +1,10 @@
 /* See LICENSE for licence details. */
 #define _XOPEN_SOURCE 600
+#define _POSIX_SAVED_IDS
 #include <ctype.h>
 #include <errno.h>
 #include <fcntl.h>
+#include <grp.h>
 #include <limits.h>
 #include <locale.h>
 #include <pwd.h>
@@ -19,6 +21,7 @@
 #include <sys/types.h>
 #include <sys/wait.h>
 #include <time.h>
+#include <utmpx.h>
 #include <unistd.h>
 #include <X11/Xatom.h>
 #include <X11/Xlib.h>
@@ -266,6 +269,9 @@ static void initenv(void);
 static void execsh(void);
 static void sigchld(int);
 static void run(void);
+static void delutmp(void);
+static void addutmp(int);
+static struct utmpx *findutmp(int);
 
 static void csidump(void);
 static void csihandle(void);
@@ -374,6 +380,9 @@ static STREscape strescseq;
 static int cmdfd;
 static pid_t pid;
 static Selection sel;
+static gid_t egid, gid;
+struct utmpx utmp;
+struct passwd *pass;
 static int iofd = -1;
 static char **opt_cmd = NULL;
 static char *opt_io = NULL;
@@ -900,6 +909,7 @@ void
 sigchld(int a) {
 	int stat = 0;
 
+	delutmp();
 	if(waitpid(pid, &stat, 0) < 0)
 		die("Waiting for pid %hd failed: %s\n",	pid, SERRNO);
 
@@ -910,14 +920,86 @@ sigchld(int a) {
 	}
 }
 
+
+/*
+ * From utmp(5)
+ * xterm and other terminal emulators directly create a USER_PROCESS
+ * record and generate the ut_id  by using the string that suffix part of
+ * the terminal name (the characters  following  /dev/[pt]ty). If they find
+ * a DEAD_PROCESS for this ID, they recycle it, otherwise they create a new
+ * entry.  If they can, they will mark it as DEAD_PROCESS on exiting and it
+ * is advised that they null ut_line, ut_time, ut_user, and ut_host as well.
+*/
+
+struct utmpx *
+findutmp(int type) {
+	struct utmpx *r;
+
+	utmp.ut_type = type;
+	setutxent();
+	for(;;) {
+		/*
+		 * we can not use getutxline because we can search in
+		 * DEAD_PROCESS to
+		 */
+		if(!(r = getutxid(&utmp)))
+			break;
+		if(!strcmp(r->ut_line, utmp.ut_line))
+			break;
+		memset(r, 0, sizeof(*r)); /* for Solaris, IRIX64 and HPUX */
+	}
+	return r;
+}
+
+void
+addutmp(int fd) {
+	uint id;
+	char *pts = ptsname(fd), *cp, buf[5] = {'x'};
+
+	for(cp = pts + strlen(pts) - 1; isdigit(*cp); --cp)
+		/* nothing */;
+	if((id = atoi(++cp)) > 999 || strlen(pts + 5) > sizeof(utmp.ut_line))
+		die("Incorrect pts name %s\n", pts);
+	sprintf(buf + 1, "%03d", id);
+	strncpy(utmp.ut_id, buf, 4);
+	strcpy(utmp.ut_line, pts + 5);   /* remove /dev/ part of the string */
+
+	if(!findutmp(DEAD_PROCESS))
+		findutmp(USER_PROCESS);
+
+	utmp.ut_type = USER_PROCESS;
+	strcpy(utmp.ut_user, pass->pw_name);
+	utmp.ut_pid = pid;
+	utmp.ut_tv.tv_sec = time(NULL);
+	utmp.ut_tv.tv_usec = 0;
+	 /* don't use no standard fields host and session */
+
+	setgid(egid);
+	if(!pututxline(&utmp))
+		perror("add utmp entry");
+	setgid(gid);
+	endutxent();
+}
+
+void
+delutmp(void) {
+	struct utmpx *r;
+
+	setutxent();
+	if((r = getutxline(&utmp)) != NULL) {
+		r->ut_type = DEAD_PROCESS;
+		r->ut_tv.tv_usec = r->ut_tv.tv_sec = 0;
+		setgid(egid);
+		pututxline(r);
+		setgid(gid);
+	}
+	endutxent();
+}
+
 void
 initenv(void) {
-	const struct passwd *pass = getpwuid(getuid());
 	char buff[sizeof(long) * 8];
 
-	if(!pass)
-		die("Process is running with an incorrect uid\n");
-
 	unsetenv("COLUMNS");
 	unsetenv("LINES");
 	unsetenv("TERMCAP");
@@ -943,7 +1025,13 @@ void
 ttynew(void) {
 	int m, s;
 	struct winsize w = {term.row, term.col, 0, 0};
+	uid_t id = getuid();
 
+	pass = getpwuid(id);
+	if(!pass || !pass->pw_name ||
+	   strlen(pass->pw_name) + 1 > sizeof(utmp.ut_user)) {
+		die("Process is running with an incorrect uid %d\n", id);
+	}
 	/* seems to work fine on linux, openbsd and freebsd */
 	if(openpty(&m, &s, NULL, NULL, &w) < 0)
 		die("openpty failed: %s\n", SERRNO);
@@ -967,6 +1055,7 @@ ttynew(void) {
 	default:
 		close(s);
 		cmdfd = m;
+		addutmp(cmdfd);
 		signal(SIGCHLD, sigchld);
 		if(opt_io) {
 			iofd = (!strcmp(opt_io, "-")) ?
@@ -2761,6 +2850,9 @@ main(int argc, char *argv[]) {
 	int i, bitm, xr, yr;
 	uint wr, hr;
 
+	egid = getegid();
+	gid = getgid();
+	setgid(gid);
 	xw.fw = xw.fh = xw.fx = xw.fy = 0;
 	xw.isfixed = False;
 
-- 
1.7.10.4

Reply via email to