Index | Thread | Search

From:
Kirill A. Korinsky <kirill@korins.ky>
Subject:
Re: sysutils/tmate: add missed pledge
To:
Theo Buehler <tb@theobuehler.org>
Cc:
rafael@sizeofvoid.org, ports@openbsd.org
Date:
Tue, 22 Apr 2025 22:29:21 +0200

Download raw body.

Thread
On Tue, 22 Apr 2025 09:17:31 +0200,
Theo Buehler <tb@theobuehler.org> wrote:
> 
> Also, adding a pledge without giving a clear root cause is never the
> right thing to do. At a minimum a ktrace should be provided.
> 
> Why does this violate the inet pledge? Why does this work on 7.6? What
> exactly changed in 7.7 so that it no longer works?
> 
> These are questions that should be asked answered *before* commit.
> 

Thanks for challenge it.

Indeed, my patch was wrong and here no need to touch pledge().

The cause is claudio@ works on libutil's imsg. Because it includes renaming
of imsg_init -> imsgbuf_init, it leads configure to a conclusion that tmate
needs compat/imsg.c and compat/imsg-buffer.c where additional socket() is
used.

So, here a diff which revert pledge() changes, and improved compatibility
with new imsg API.

Ok?

Index: sysutils/tmate/Makefile
===================================================================
RCS file: /cvs/ports/sysutils/tmate/Makefile,v
diff -u -p -r1.24 Makefile
--- sysutils/tmate/Makefile	22 Apr 2025 07:07:57 -0000	1.24
+++ sysutils/tmate/Makefile	22 Apr 2025 20:23:39 -0000
@@ -4,7 +4,7 @@ GH_ACCOUNT =	tmate-io
 GH_PROJECT =	tmate
 GH_TAGNAME =	2.4.0
 CATEGORIES =	sysutils
-REVISION =	8
+REVISION =	9
 
 SITES.p =	https://github.com/tmate-io/tmate/commit/
 PATCHFILES.p =	tmate-bad-fingerprint{cbec43f56dfb48c2fb6e00faa2cb85443d4b7d8f}.patch \
Index: sysutils/tmate/patches/patch-client_c
===================================================================
RCS file: sysutils/tmate/patches/patch-client_c
diff -N sysutils/tmate/patches/patch-client_c
--- sysutils/tmate/patches/patch-client_c	22 Apr 2025 07:07:57 -0000	1.1
+++ /dev/null	1 Jan 1970 00:00:00 -0000
@@ -1,23 +0,0 @@
-Add missing inet promise.
-
-Index: client.c
---- client.c.orig
-+++ client.c
-@@ -391,7 +391,7 @@ client_main(struct event_base *base, int argc, char **
- 	 *
- 	 * "sendfd" is dropped later in client_dispatch_wait().
- 	 */
--	if (pledge("stdio unix sendfd proc exec tty", NULL) != 0)
-+	if (pledge("stdio unix sendfd proc exec tty inet", NULL) != 0)
- 		fatal("pledge failed");
- #endif
- 
-@@ -652,7 +652,7 @@ client_dispatch_wait(struct imsg *imsg, const char *sh
- 	 * get the first message from the server.
- 	 */
- 	if (!pledge_applied) {
--		if (pledge("stdio unix proc exec tty", NULL) != 0)
-+		if (pledge("stdio unix proc exec tty inet", NULL) != 0)
- 			fatal("pledge failed");
- 		pledge_applied = 1;
- 	};
Index: sysutils/tmate/patches/patch-configure-ac
===================================================================
RCS file: /cvs/ports/sysutils/tmate/patches/patch-configure-ac,v
diff -u -p -r1.1 patch-configure-ac
--- sysutils/tmate/patches/patch-configure-ac	20 Feb 2024 12:01:49 -0000	1.1
+++ sysutils/tmate/patches/patch-configure-ac	22 Apr 2025 20:23:39 -0000
@@ -1,5 +1,6 @@
---- configure.ac.orig	Sun Nov 17 07:09:38 2019
-+++ configure.ac	Sat Feb  3 15:47:48 2024
+Index: configure.ac
+--- configure.ac.orig
++++ configure.ac
 @@ -201,7 +201,7 @@ fi
  
  PKG_CHECK_MODULES(
@@ -18,3 +19,18 @@
  fi
  
  PKG_CHECK_MODULES(
+@@ -310,11 +310,11 @@ fi
+ AC_SUBST(XOPEN_DEFINES)
+ 
+ # Look for imsg in libutil. compat/imsg.c is linked by Makefile.am if missing.
+-AC_SEARCH_LIBS(imsg_init, util, found_imsg_init=yes, found_imsg_init=no)
+-if test "x$found_imsg_init" = xyes; then
++AC_SEARCH_LIBS(imsgbuf_init, util, found_imsgbuf_init=yes, found_imsgbuf_init=no)
++if test "x$found_imsgbuf_init" = xyes; then
+ 	AC_DEFINE(HAVE_IMSG)
+ fi
+-AM_CONDITIONAL(NO_IMSG, [test "x$found_imsg_init" = xno])
++AM_CONDITIONAL(NO_IMSG, [test "x$found_imsgbuf_init" = xno])
+ 
+ # Look for forkpty in libutil. compat/forkpty-*.c is linked if not found.
+ AC_SEARCH_LIBS(forkpty, util, found_forkpty=yes, found_forkpty=no)
Index: sysutils/tmate/patches/patch-proc_c
===================================================================
RCS file: sysutils/tmate/patches/patch-proc_c
diff -N sysutils/tmate/patches/patch-proc_c
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ sysutils/tmate/patches/patch-proc_c	22 Apr 2025 20:23:39 -0000
@@ -0,0 +1,72 @@
+Index: proc.c
+--- proc.c.orig
++++ proc.c
+@@ -59,8 +59,10 @@ proc_event_cb(__unused int fd, short events, void *arg
+ 	struct imsg	 imsg;
+ 
+ 	if (!(peer->flags & PEER_BAD) && (events & EV_READ)) {
+-		if (((n = imsg_read(&peer->ibuf)) == -1 && errno != EAGAIN) ||
+-		    n == 0) {
++		if ((n = imsgbuf_read(&peer->ibuf)) == -1)
++			fatal("imsgbuf_read");
++
++		if (n == 0) {
+ 			peer->dispatchcb(NULL, peer->arg);
+ 			return;
+ 		}
+@@ -74,8 +76,6 @@ proc_event_cb(__unused int fd, short events, void *arg
+ 			log_debug("peer %p message %d", peer, imsg.hdr.type);
+ 
+ 			if (peer_check_version(peer, &imsg) != 0) {
+-				if (imsg.fd != -1)
+-					close(imsg.fd);
+ 				imsg_free(&imsg);
+ 				break;
+ 			}
+@@ -86,13 +86,13 @@ proc_event_cb(__unused int fd, short events, void *arg
+ 	}
+ 
+ 	if (events & EV_WRITE) {
+-		if (msgbuf_write(&peer->ibuf.w) <= 0 && errno != EAGAIN) {
++		if (imsgbuf_write(&peer->ibuf) == -1) {
+ 			peer->dispatchcb(NULL, peer->arg);
+ 			return;
+ 		}
+ 	}
+ 
+-	if ((peer->flags & PEER_BAD) && peer->ibuf.w.queued == 0) {
++	if ((peer->flags & PEER_BAD) && imsgbuf_queuelen(&peer->ibuf) == 0) {
+ 		peer->dispatchcb(NULL, peer->arg);
+ 		return;
+ 	}
+@@ -133,7 +133,7 @@ proc_update_event(struct tmuxpeer *peer)
+ 	event_del(&peer->event);
+ 
+ 	events = EV_READ;
+-	if (peer->ibuf.w.queued > 0)
++	if (imsgbuf_queuelen(&peer->ibuf) > 0)
+ 		events |= EV_WRITE;
+ 	event_set(&peer->event, peer->ibuf.fd, events, proc_event_cb, peer);
+ 
+@@ -246,7 +246,11 @@ proc_add_peer(struct tmuxproc *tp, int fd,
+ 	peer->dispatchcb = dispatchcb;
+ 	peer->arg = arg;
+ 
+-	imsg_init(&peer->ibuf, fd);
++	if (imsgbuf_init(&peer->ibuf, fd) == -1)
++		fatal("imsgbuf_init");
++
++	imsgbuf_allow_fdpass(&peer->ibuf);
++
+ 	event_set(&peer->event, fd, EV_READ, proc_event_cb, peer);
+ 
+ 	log_debug("add peer %p: %d (%p)", peer, fd, arg);
+@@ -261,7 +265,7 @@ proc_remove_peer(struct tmuxpeer *peer)
+ 	log_debug("remove peer %p", peer);
+ 
+ 	event_del(&peer->event);
+-	imsg_clear(&peer->ibuf);
++	imsgbuf_clear(&peer->ibuf);
+ 
+ 	close(peer->ibuf.fd);
+ 	free(peer);
Index: sysutils/tmate/patches/patch-server-client_c
===================================================================
RCS file: sysutils/tmate/patches/patch-server-client_c
diff -N sysutils/tmate/patches/patch-server-client_c
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ sysutils/tmate/patches/patch-server-client_c	22 Apr 2025 20:23:39 -0000
@@ -0,0 +1,14 @@
+Index: server-client.c
+--- server-client.c.orig
++++ server-client.c
+@@ -1238,8 +1238,8 @@ server_client_dispatch_identify(struct client *c, stru
+ 	case MSG_IDENTIFY_STDIN:
+ 		if (datalen != 0)
+ 			fatalx("bad MSG_IDENTIFY_STDIN size");
+-		c->fd = imsg->fd;
+-		log_debug("client %p IDENTIFY_STDIN %d", c, imsg->fd);
++		c->fd = imsg_get_fd(imsg);
++		log_debug("client %p IDENTIFY_STDIN %d", c, c->fd);
+ 		break;
+ 	case MSG_IDENTIFY_ENVIRON:
+ 		if (datalen == 0 || data[datalen - 1] != '\0')