On 2025/05/13 10:14, izzy Meyer wrote:
> On Tue, 13 May 2025 13:51:33 +0100
> Stuart Henderson <stu@spacehopper.org> wrote:
> 
> > On 2025/05/13 07:11, izzy Meyer wrote:
> > > On Sat, 10 May 2025 19:04:11 +0000
> > > Lucas Gabriel Vuotto <lucas@sexy.is> wrote:
> > > 
> > > > On Sat, May 10, 2025 at 12:53:44PM -0500, izzy Meyer wrote:
> > > > > Ended up just patching the file for simplicity.
> > > > 
> > > > I tend to avoid patching as much as possible, but to each one
> > > > their own. You're the maintainer anyways.
> > > 
> > > This might make maintaining easier, but I couldn't get the -D flag
> > > in CFLAGS to apply correctly when setting REBOOT_CMD to
> > > "/sbin/shutdown -r now" in CFLAGS with -D. I must be missing
> > > something here:
> > 
> > You can do it with
> > 
> > MAKE_FLAGS =	CFLAGS="${CFLAGS} -I./Xm -I${X11BASE}/include
> > -I${LOCALBASE}/include -D'REBOOT_CMD=\"/sbin/shutdown -r now\"'"
> 
> Thanks! 
> 
> > but that's harder for maintenance because you don't get notified by
> > patch(1) if upstream changes that variable in their Makefile, which
> > you may need to adapt to.
> > 
> > The version with the patch is imho saner.
> 
> This is what I was thinking initially when doing it via a patch. I'll
> attach a new diff with this added back.
> 
> > > > This package is not installed with SUID root; warnings about
> > > > lacking SUID root can be ignored.
> > 
> > I don't particularly want to mess with my X setup to test it now, but
> > if that warning does still get displayed but is irrelevant after the
> > changes to the port, I would patch away the warning, rather than
> > adding to the pkg-readme telling people to ignore it.
> 
> Good catch. Duh, why didn't I think of this! xD
> 
> > : - To enable shutdown and reboot, the user should be in _shutdown
> > : group.
> > : - To enable suspend, the user needs to be able to run zzz(8).
> > : 
> > : Use usermod(8) to add the user to _shutdown group.
> > 
> > Might as well give an example for usermod:
> > 
> > 	# usermod -G _shutdown <username>
> > 
> > : Enable apmd(8) and check zzz(8) for details about the required
> > permissions.
> > 
> > Neither apmd(8) nor zzz(8) are really up-front about permissions.
> > 
> > zzz: "The protection modes on this socket govern which users may
> > access the APM functions"
> > 
> > apmd: "The socket is protected to mode 0660, UID 0, GID 0; this
> > protects access to suspend requests to authorized users only."
> > 
> > The experienced admin would realise what GID 0 is, but for user-level
> > docs it would be clearer to mention in the pkg-readme that the user
> > must be in group "wheel" for this to work -- however 'wheel' is a
> > significant escalation if the only reason is to permit sleep.
> 
> Fair point. This is a large escalation for a seemingly simple
> operation. 
> 
> Does this read better?

It does. Slight tweak though as we can remove some of the text and
group the shutdown/reboot-related text+command in one place,
and zzz-related text+commmand in another (and fix line-ending in
pkg/README which was missing EOL)

I think this is ok

Index: Makefile
===================================================================
RCS file: /cvs/ports/x11/emwm-utils/Makefile,v
diff -u -p -r1.2 Makefile
--- Makefile	8 May 2025 14:19:24 -0000	1.2
+++ Makefile	13 May 2025 15:21:00 -0000
@@ -3,6 +3,7 @@ COMMENT =	session manager and a toolches
 V =		1.3
 DISTNAME =	emwm-utils-src-${V}
 PKGNAME =	emwm-utils-${V}
+REVISION =	0
 
 CATEGORIES =	x11
 HOMEPAGE =	https://fastestcode.org/emwm.html
@@ -20,7 +21,8 @@ WANTLIB += X11 Xinerama Xm Xrandr Xss Xt
 LIB_DEPENDS =	x11/motif
 
 MAKE_FLAGS =	RCDIR=${PREFIX}/lib/X11 \
-		CFLAGS="${CFLAGS} -I./Xm -I${X11BASE}/include -I${LOCALBASE}/include"
+		CFLAGS="${CFLAGS} -DUNPRIVILEGED_SHUTDOWN \
+			-I./Xm -I${X11BASE}/include -I${LOCALBASE}/include"
 
 FAKE_FLAGS =	PREFIX=${WRKINST}${PREFIX} \
 		APPLRESDIR=${WRKINST}${PREFIX}/lib/X11/app-defaults \
Index: patches/patch-src_smconf_h
===================================================================
RCS file: patches/patch-src_smconf_h
diff -N patches/patch-src_smconf_h
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ patches/patch-src_smconf_h	13 May 2025 15:21:00 -0000
@@ -0,0 +1,12 @@
+Index: src/smconf.h
+--- src/smconf.h.orig
++++ src/smconf.h
+@@ -33,7 +33,7 @@
+ #endif /* SHUTDOWN_CMD */
+ 
+ #ifndef REBOOT_CMD
+-#define REBOOT_CMD      "/sbin/reboot"
++#define REBOOT_CMD      "/sbin/shutdown -r now"
+ #endif
+ 
+ #ifndef SUSPEND_CMD
Index: patches/patch-src_smmain_c
===================================================================
RCS file: /cvs/ports/x11/emwm-utils/patches/patch-src_smmain_c,v
diff -u -p -r1.1 patch-src_smmain_c
--- patches/patch-src_smmain_c	8 May 2025 14:20:34 -0000	1.1
+++ patches/patch-src_smmain_c	13 May 2025 15:21:00 -0000
@@ -61,3 +61,21 @@ Index: src/smmain.c
  
  	memset(pwb,0,strlen(pwb));
  	XmTextFieldSetString(wpasswd,"");
+@@ -988,7 +1009,8 @@ static Boolean set_privileges(Boolean elevate)
+ 	if(!initialized){
+ 		orig_uid = geteuid();
+ 		orig_gid = getegid();
+-
++		/* BSD-auth handles authentication, no SUID needed.  */
++#ifndef __OpenBSD__
+ 		if(orig_uid != 0){
+ 			log_msg("%s must be setuid root to enable "
+ 				"screen locking capabilities.\n",bin_name);
+@@ -996,6 +1018,7 @@ static Boolean set_privileges(Boolean elevate)
+ 			can_elevate = False;
+ 			return False;
+ 		}
++#endif /* __OpenBSD__ */
+ 		initialized = True;
+ 		can_elevate = True;
+ 	}
Index: pkg/PLIST
===================================================================
RCS file: /cvs/ports/x11/emwm-utils/pkg/PLIST,v
diff -u -p -r1.2 PLIST
--- pkg/PLIST	8 May 2025 14:19:24 -0000	1.2
+++ pkg/PLIST	13 May 2025 15:21:00 -0000
@@ -10,3 +10,4 @@ lib/X11/app-defaults/XmToolbox
 lib/X11/toolboxrc
 @man man/man1/xmsm.1
 @man man/man1/xmtoolbox.1
+share/doc/pkg-readmes/${PKGSTEM}
Index: pkg/README
===================================================================
RCS file: pkg/README
diff -N pkg/README
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ pkg/README	13 May 2025 15:21:00 -0000
@@ -0,0 +1,15 @@
++-------------------------------------------------------------------------------
+| Running ${PKGSTEM} on OpenBSD
++-------------------------------------------------------------------------------
+
+To enable shutdown and reboot, the user should be in _shutdown group:
+
+	# usermod -G _shutdown <username>
+
+To enable suspend, the user needs to be able to run zzz(8).
+In order to do that, the user must be in group "wheel", and apmd(8)
+must be running:
+
+	# rcctl enable apmd
+	# rcctl start apmd
+	# usermod -G wheel <username>