From: Stuart Henderson Subject: Re: MAINTAINER FIX: x11/emwm-utils: Enable unpriv shutdown/reboot/suspend and add pkg-readme To: izzy Meyer Cc: ports@openbsd.org, Lucas Gabriel Vuotto Date: Tue, 13 May 2025 16:22:54 +0100 On 2025/05/13 10:14, izzy Meyer wrote: > On Tue, 13 May 2025 13:51:33 +0100 > Stuart Henderson wrote: > > > On 2025/05/13 07:11, izzy Meyer wrote: > > > On Sat, 10 May 2025 19:04:11 +0000 > > > Lucas Gabriel Vuotto 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 > > > > : 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 + +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